kubernetes-sigs / aws-ebs-csi-driver

CSI driver for Amazon EBS https://aws.amazon.com/ebs/
Apache License 2.0
995 stars 797 forks source link

Enable VAC feature gate for Kustomize deployments #2240

Open ConnorJC3 opened 1 day ago

ConnorJC3 commented 1 day ago

What type of PR is this?

/kind feature

What is this PR about? / Why do we need it?

Fixes #2238

After discussion, we decided for a consistent experience and because the downsides are significant if a user attempts to use it while disabled on the sidecars, we will enable the VAC feature gate for the Kustomize deployment.

How was this change tested?

Manually tested + CI

Does this PR introduce a user-facing change?

[TODO AFTER GENERATING RELEASE NOTES: Move to "Urgent Upgrade Notes" section] Enables the `VolumeAttributesClass` by default for the Kustomize deployment. If you are deploying using the Kustomize manifests on a cluster that does not have the `VolumeAttributesClass` feature gate enabled on the control plane, you may see harmless extra failures related to the feature in the csi-provisioner and/or csi-resizer logs.
k8s-ci-robot commented 1 day ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from connorjc3. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
github-actions[bot] commented 1 day ago

Code Coverage Diff

This PR does not change the code coverage

AndrewSirenko commented 1 day ago

you may see harmless extra failures related to the feature in the csi-provisioner and/or csi-resizer logs

Did we confirm this in the manual test? Or should I spin up a non-VAC enabled cluster

ConnorJC3 commented 1 day ago

Did we confirm this in the manual test? Or should I spin up a non-VAC enabled cluster

I did a super basic test but extra eyes cannot hurt.

torredil commented 1 day ago

/retest

ConnorJC3 commented 1 day ago

/test

k8s-ci-robot commented 1 day ago

@ConnorJC3: The /test command needs one or more targets. The following commands are available to trigger required jobs:

The following commands are available to trigger optional jobs:

Use /test all to run all jobs.

In response to [this](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/pull/2240#issuecomment-2494297096): >/test Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
ConnorJC3 commented 1 day ago

Tested make e2e/external-kustomize locally because this PR won't run the kustomize job:

Ran 91 of 6899 Specs in 314.219 seconds
SUCCESS! -- 91 Passed | 0 Failed | 0 Pending | 6808 Skipped