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

Conversation

ddysher
Copy link
Contributor

@ddysher ddysher commented Jun 10, 2017

Based on #135. Review the last commit will suffice.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 10, 2017
@ddysher
Copy link
Contributor Author

ddysher commented Jun 10, 2017

@msau42

@msau42
Copy link
Contributor

msau42 commented Jun 12, 2017

/lgtm

@wongma7 wongma7 merged commit 4cf8cf3 into kubernetes-retired:master Jun 12, 2017
// 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......

cofyc pushed a commit to cofyc/external-storage that referenced this pull request Dec 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants