search

kubernetes-csi / external-snapshotter

Sidecar container that watches Kubernetes Snapshot CRD objects and triggers CreateSnapshot/DeleteSnapshot against a CSI endpoint.
Apache License 2.0
463 stars 361 forks source link

Correctly detect newly created Volume[Group]SnapshotClasses #1100

Closed leonardoce closed 1 month ago

leonardoce commented 1 month ago

What type of PR is this?

/kind bug

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change /kind cleanup /kind design /kind documentation /kind failing-test /kind feature /kind flake

What this PR does / why we need it:

This PR makes the controller process newly created default VolumeSnapshotClasses and VolumeGroupSnapshotClasses, correctly using them or raising an error when appropriate.

Which issue(s) this PR fixes:

Fixes #1099

Special notes for your reviewer:

The controller is using an informer to get VolumeGroupSnapshotClasses and VolumeSnapshotClasses. Unfortunately, objects returned by that will not have the API Kind info set (see https://github.com/kubernetes/client-go/issues/308).

This prevented the IsDefaultAnnotation function from working correctly.

This patch avoids relying on the object Kind to know whether a VolumeGroupSnapshotClass or a VolumeSnapshotClass is set as default.

The semantics of the groupsnapshot.storage.kubernetes.io/is-default-class have been preserved, with true being true and everything else being false (this is slightly different from Kubernetes).

Does this PR introduce a user-facing change?:

The controller will now detect new default VolumeSnapshotClasses and VolumeGroupSnapshotClasses.
If multiple classes exist for the same CSI driver, VolumeSnapshot and VolumeGroupSnapshots
will be marked as failed when provisioned dynamically.
xing-yang commented 1 month ago

Please rebase.

leonardoce commented 1 month ago

Done @xing-yang

xing-yang commented 1 month ago

/lgtm /approve

k8s-ci-robot commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leonardoce, xing-yang

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-csi/external-snapshotter/blob/master/OWNERS)~~ [xing-yang] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment