Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

indicate volume expansion is possible #189

Merged
merged 1 commit into from
Jul 27, 2021

Conversation

ekristen
Copy link
Contributor

By adding allowVolumeExpansion this will prevent errors if someone changes the PVC capacity size on an already provisioned PVC, since local-path-provisioner does not enforce disk usage anyways, it seems reasonable to allow the value to change without throwing an error.

However this will not update the status of the PVC, it'll still show the original amount. The project would need some sort of resource watch and update status to reflect status from spec.

@innobead
Copy link
Collaborator

right now, as you know, local-path-provisioner does not support allowVolumeExpansion. For this change, it is partial. So I would like to know more what kind of errors you would see when changing the size of PVC? It will be good to create an issue/request to describe the problem you encountered and the expected you would like to achieve.

@ekristen
Copy link
Contributor Author

@innobead sure, I can do that. My point is that since it doesn't enforce size there's no reason to throw an error if someone tries to resize, even if the spec -> status doesn't get updated, this prevents an error being thrown.

@ekristen
Copy link
Contributor Author

Issue opened and referenced. My point if I haven't made it clear is that since the local path provisioner doesn't enforce disk usage, or check for disk usage, or provisions actual disk space, why prevent the volume size for changing?

In my use case I am programmatically creating PVCs using the storage class and if the underlying setting in my app changes for the default disk size all the api calls break because the provisioner says it doesn't support expansion when in fact it could and should since it doesn't actually do any provisioning around size anyways. :)

Thanks for your time.

@flokli
Copy link

flokli commented Jul 23, 2021

What's the status of this?

If we don't enforce resource limits/quotas anyways, could we just set this, as this PR already does?

Copy link
Collaborator

@innobead innobead left a comment

Choose a reason for hiding this comment

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

LGTM

@innobead
Copy link
Collaborator

Issue opened and referenced. My point if I haven't made it clear is that since the local path provisioner doesn't enforce disk usage, or check for disk usage, or provisions actual disk space, why prevent the volume size for changing?

In my use case I am programmatically creating PVCs using the storage class and if the underlying setting in my app changes for the default disk size all the api calls break because the provisioner says it doesn't support expansion when in fact it could and should since it doesn't actually do any provisioning around size anyways. :)

Thanks for your time.

@ekristen thanks for the explanation and contribution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants