-
Notifications
You must be signed in to change notification settings - Fork 38.9k
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
CSIStorageCapacity beta API #99641
CSIStorageCapacity beta API #99641
Conversation
448641a
to
9664f14
Compare
7ab8d1e
to
177ba61
Compare
c8a2aa9
to
b52d558
Compare
b083624
to
dde88e3
Compare
Defaults and validation are such that the field has to be set when the feature is enabled, just as for the other boolean fields. This was missing in some tests, which was okay as long as they ran with the feature disabled. Once it gets enabled, validation will flag the missing field as error. Other tests didn't run at all.
The v1alpha1 API is left in place for now to ease the migration.
That the object was registered depending on the feature gate was called out as unusual during the 1.21 review. Previously, all beta storage APIs were unders such feature gate checks, but its better to drop that to be consistent with the rest of Kubernetes.
This is the result of UPDATE_BOOTSTRAP_POLICY_FIXTURE_DATA=true go test k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy after enabling the CSIStorageCapacity feature. This enables additional RBAC entries for reading CSIDriver and CSIStorageCapacity.
We still need the alpha API for testing with current external-provisioner, but then should remove it shortly after the 1.21 release once the external-provisioner is updated to use the beta API. That VolumeAttachment is still defined in the alpha API looks like an oversight. To minimize changes during the beta graduation of CSIStorageCapacity it is left in place with a deprecation notification in 1.21.
If available, then the MaximumVolumeSize is a better indicator whether creating a volume has a chance to succeed than the total (?) Capacity, which is potentially larger and less well-defined.
I need to rebase and regenerate one file... working on it. |
The tag is currently mostly ignored by the tooling, but its usage is encouraged (kubernetes#99641 (comment)).
With the feature now in beta, tests should run in the normal jobs.
dde88e3
to
4c7e4c6
Compare
/lgtm The feature-gate tagging discussion can continue offline and we can update the tags once the KEP is merged. |
Oh @pohly can you also update the release note saying the alpha capacity objects are also deprecated? |
Done. |
/retest |
@pohly: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
The tag is currently mostly ignored by the tooling, but its usage is encouraged (kubernetes/kubernetes#99641 (comment)). Kubernetes-commit: 39103c188f5d17a0baffb90c122f4751e5b33b93
/triage accepted |
What type of PR is this?
/kind feature
/kind api-change
What this PR does / why we need it:
We want to promote CSIStorageCapacity to beta in Kubernetes 1.21.
Special notes for your reviewer:
The new v1beta1 API might get extended before Kubernetes 1.21 gets released if the new CSI spec 1.4.0 gets released quickly enough. Right now, this PR does not include that change (yet).
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: