Skip to content
This repository has been archived by the owner on Oct 21, 2020. It is now read-only.

Add test to verify storageclass #154

Merged
merged 4 commits into from
Jun 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ spec:
- name: provisioner
image: "gcr.io/msau-k8s-dev/local-volume-provisioner:latest"
imagePullPolicy: Always
securityContext:
privileged: true
volumeMounts:
- name: discovery-vol
mountPath: "/local-disks"
Expand Down
4 changes: 2 additions & 2 deletions local-volume/provisioner/pkg/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ type RuntimeConfig struct {
type LocalPVConfig struct {
Name string
HostPath string
Capacity uint64
StorageClass string
ProvisionerName string
AffinityAnn string
Expand All @@ -83,8 +84,7 @@ func CreateLocalPVSpec(config *LocalPVConfig) *v1.PersistentVolume {
Spec: v1.PersistentVolumeSpec{
PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimDelete,
Capacity: v1.ResourceList{
// TODO: detect capacity
v1.ResourceName(v1.ResourceStorage): resource.MustParse("10Gi"),
v1.ResourceName(v1.ResourceStorage): *resource.NewQuantity(int64(config.Capacity), resource.BinarySI),
},
PersistentVolumeSource: v1.PersistentVolumeSource{
Local: &v1.LocalVolumeSource{
Expand Down
14 changes: 10 additions & 4 deletions local-volume/provisioner/pkg/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,13 @@ func (d *Discoverer) discoverVolumesAtPath(class, relativePath string) {
glog.Errorf("Mount path %q validation failed: %v", filePath, err)
continue
}
// TODO: detect capacity
d.createPV(file, relativePath, class)

availByte, err := d.VolUtil.GetFsAvailableByte(filePath)
if err != nil {
glog.Errorf("Path %q fs stats error: %v", filePath, err)
continue
}
d.createPV(file, relativePath, class, availByte)
}
}
}
Expand All @@ -131,14 +136,15 @@ func generatePVName(file, node, class string) string {
return fmt.Sprintf("local-pv-%x", h.Sum32())
}

func (d *Discoverer) createPV(file, relativePath, class string) {
func (d *Discoverer) createPV(file, relativePath, class string, availByte uint64) {
pvName := generatePVName(file, d.Node.Name, class)
outsidePath := filepath.Join(d.HostDir, relativePath, file)

glog.Infof("Found new volume at host path %q, creating Local PV %q", outsidePath, pvName)
glog.Infof("Found new volume at host path %q with capacity %d, creating Local PV %q", outsidePath, availByte, pvName)
pvSpec := common.CreateLocalPVSpec(&common.LocalPVConfig{
Name: pvName,
HostPath: outsidePath,
Capacity: availByte,
StorageClass: class,
ProvisionerName: d.Name,
AffinityAnn: d.nodeAffinityAnn,
Expand Down
53 changes: 42 additions & 11 deletions local-volume/provisioner/pkg/discovery/discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ type testConfig struct {
func TestDiscoverVolumes_Basic(t *testing.T) {
vols := map[string][]*util.FakeFile{
"dir1": {
{Name: "mount1", Hash: 0xaaaafef5},
{Name: "mount2", Hash: 0x79412c38},
{Name: "mount1", Hash: 0xaaaafef5, Capacity: 100 * 1024},
{Name: "mount2", Hash: 0x79412c38, Capacity: 100 * 1024 * 1024},
},
"dir2": {
{Name: "mount1", Hash: 0xa7aafa3c},
Expand Down Expand Up @@ -311,13 +311,40 @@ func verifyProvisionerName(t *testing.T, pv *v1.PersistentVolume) {
}
}

func verifyCapacity(t *testing.T, createdPV *v1.PersistentVolume, expectedPV *testPVInfo) {
capacity, ok := createdPV.Spec.Capacity[v1.ResourceStorage]
if !ok {
t.Errorf("Unexpected empty resource storage")
}
capacityInt, ok := capacity.AsInt64()
if !ok {
t.Errorf("Unable to convert resource storage into int64")
}
if uint64(capacityInt) != expectedPV.capacity {
t.Errorf("Expected capacity %d, got %d", expectedPV.capacity, capacityInt)
}
}

// testPVInfo contains all the fields we are intested in validating.
type testPVInfo struct {
pvName string
path string
capacity uint64
storageClass string
}

func verifyCreatedPVs(t *testing.T, test *testConfig) {
expectedPVs := map[string]string{}
expectedPVs := map[string]*testPVInfo{}
for dir, files := range test.expectedVolumes {
for _, file := range files {
pvName := fmt.Sprintf("local-pv-%x", file.Hash)
path := filepath.Join(testHostDir, dir, file.Name)
expectedPVs[pvName] = path
expectedPVs[pvName] = &testPVInfo{
pvName: pvName,
path: path,
capacity: file.Capacity,
storageClass: findSCName(t, dir, test),
}
}
}

Expand All @@ -328,21 +355,25 @@ func verifyCreatedPVs(t *testing.T, test *testConfig) {
t.Errorf("Expected %v created PVs, got %v", expectedLen, actualLen)
}

for pvName, pv := range createdPVs {
expectedPath, found := expectedPVs[pvName]
for pvName, createdPV := range createdPVs {
expectedPV, found := expectedPVs[pvName]
if !found {
t.Errorf("Did not expect created PVs %v", pvName)
}
if pv.Spec.PersistentVolumeSource.Local.Path != expectedPath {
t.Errorf("Expected path %q, got %q", expectedPath, expectedPath)
if createdPV.Spec.PersistentVolumeSource.Local.Path != expectedPV.path {
t.Errorf("Expected path %q, got %q", expectedPV.path, createdPV.Spec.PersistentVolumeSource.Local.Path)
}
if createdPV.Spec.StorageClassName != expectedPV.storageClass {
t.Errorf("Expected storage class %q, got %q", expectedPV.storageClass, createdPV.Spec.StorageClassName)
}
_, exists := test.cache.GetPV(pvName)
if !exists {
t.Errorf("PV %q not in cache", pvName)
}
// TODO: verify storage class
verifyProvisionerName(t, pv)
verifyNodeAffinity(t, pv)

verifyProvisionerName(t, createdPV)
verifyNodeAffinity(t, createdPV)
verifyCapacity(t, createdPV, expectedPV)
}
}

Expand Down
36 changes: 35 additions & 1 deletion local-volume/provisioner/pkg/util/volume_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"os"
"path/filepath"

"golang.org/x/sys/unix"

"github.com/golang/glog"
)

Expand All @@ -34,6 +36,9 @@ type VolumeUtil interface {

// Delete all the contents under the given path, but not the path itself
DeleteContents(fullPath string) error

// Get available capacity for fs on full path
GetFsAvailableByte(fullPath string) (uint64, error)
}

var _ VolumeUtil = &volumeUtil{}
Expand Down Expand Up @@ -99,6 +104,18 @@ func (u *volumeUtil) DeleteContents(fullPath string) error {
return nil
}

// GetFsAvailableByte returns available capacity in byte about a mounted filesystem.
// fullPath is the pathname of any file within the mounted filesystem. Capacity
// returned here is total capacity available to non-root users, and does not include
// fs reserved space.
func (u *volumeUtil) GetFsAvailableByte(fullPath string) (uint64, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this AvailableByte different from available bytes inside kubernetes/pkg/volume/util/fs.go:33 FsInfo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out. I didn't know there is such a util in kubernetes :) You can take a look at discussion here: #135 (comment) Basically, available from that package returns all available spaces block wise, but there are certain amount of blocks can't be used by normal user. AvailableByte removes those.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ddysher @msau42 would it make sense to modify the volume util package to match this definition of availablebytes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at FsInfo, I think it returns all the info that you need here. You could do (usage + available) to get the equivalent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, usage + available is basically the idea.

@ianchakeres available in fs.go means remaining bytes available to be consumed, while available here means capacity that can be used by normal users. so we can't change the definition of available in fs.go. It may worth taking a look if returning a new field for availablebytes is necessary though, but i don't know if there is use case. Even if there is, it can be calculated by just using usage+available.

But you raise another IMPORTANT problem, that we should change to a better name......

var s unix.Statfs_t
if err := unix.Statfs(fullPath, &s); err != nil {
return 0, err
}
return uint64(s.Frsize) * (s.Blocks - s.Bfree + s.Bavail), nil
}

var _ VolumeUtil = &FakeVolumeUtil{}

// FakeVolumeUtil is a stub interface for unit testing
Expand All @@ -114,7 +131,8 @@ type FakeFile struct {
Name string
IsNotDir bool
// Expected hash value of the PV name
Hash uint32
Hash uint32
Capacity uint64
}

// NewFakeVolumeUtil returns a VolumeUtil object for use in unit testing
Expand Down Expand Up @@ -163,6 +181,22 @@ func (u *FakeVolumeUtil) DeleteContents(fullPath string) error {
return nil
}

func (u *FakeVolumeUtil) GetFsAvailableByte(fullPath string) (uint64, error) {
dir, file := filepath.Split(fullPath)
dir = filepath.Clean(dir)
files, found := u.directoryFiles[dir]
if !found {
return 0, fmt.Errorf("Directory %q not found", dir)
}

for _, f := range files {
if file == f.Name {
return f.Capacity, nil
}
}
return 0, fmt.Errorf("File %q not found", fullPath)
}

// AddNewFiles adds the given files to the current directory listing
// This is only for testing
func (u *FakeVolumeUtil) AddNewFiles(mountDir string, dirFiles map[string][]*FakeFile) {
Expand Down