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

Fix resource exhausted typo #578

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ebblake
Copy link

@ebblake ebblake commented Jan 21, 2025

What type of PR is this?
Bug fix to the specification.

What this PR does / why we need it:
Inconsistent specs are hard to implement.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce an API-breaking change?:

none

The spec is inconsistent with itself, claiming in some places that
RESOURCE_EXHAUSTED is 8, and in others that it is 13.  The canonical
source [1] confirms that 8 is correct.

[1] https://github.com/grpc/grpc-go/blob/master/codes/codes.go

Signed-off-by: Eric Blake <[email protected]>
@ebblake
Copy link
Author

ebblake commented Jan 21, 2025

I found this while trying to determine what error to report. CreateVolume mentions both RESOURCE_EXHAUSTED and OUT_OF_RANGE but without a clear statement of which is better when running out of space; while CreateSnapshot clearly wants RESOURCE_EXHAUSTED but ControllerExpandVolume wants OUT_OF_RANGE. In my view, OUT_OF_RANGE applies to things like "requested size needs to be rounded larger than requested limit" or "limit is smaller than the source volume size" (something you can't fix no matter how often you retry with the same arguments), while RESOURCE_EXHAUSTED is for errors that may or may not be transient if more storage resources are freed up in the meantime). I also note that https://github.com/kubernetes/enhancements/blob/master/keps/sig-storage/1790-recover-resize-failure/README.md#making-allocatedresourcestatus-not-change-unnecessarily-for-every-error-in-131 mentioned OUT_OF_RANGE but not RESOURCE_EXHAUSTED as a plausible reason to back off the retry loop.

@jdef
Copy link
Member

jdef commented Jan 23, 2025

thanks for the PR. please sign the CLA if you've not done so already.

@ebblake
Copy link
Author

ebblake commented Jan 23, 2025

thanks for the PR. please sign the CLA if you've not done so already.

I have: kubernetes-sigs/contributor-playground#1452
Or is there a different CLA for this project?

@jdef
Copy link
Member

jdef commented Jan 23, 2025 via email

@ebblake
Copy link
Author

ebblake commented Jan 23, 2025

Diff. See the Ccla file in top dir

Thanks - I'm chasing down whether Red Hat has already signed such a file, and if so, whether it is blanket for all employees or whether I need to be added to a list.

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.

2 participants