DeleteVolumeGroupSnapshot API - part 1 #882

RaunakShah · 2023-08-15T11:49:35Z

What type of PR is this?
/kind feature

What this PR does / why we need it:

This PR adds logic to delete volume group snapshots.

Currently, Only the volume group snapshots and corresponding volume group snapshot contents are deleted. The logic to delete individual snapshots will be added in a separate PR.

Testing

Dynamically volume group snapshot:

Create a volume group snapshot

% kubectl create -f volumegroupsnapshot-example.yaml 
volumegroupsnapshot.groupsnapshot.storage.k8s.io/my-group-snapshot created
% kubectl get vgsc
NAME                                                    READYTOUSE   DELETIONPOLICY   DRIVER                VOLUMEGROUPSNAPSHOTCLASS   VOLUMEGROUPSNAPSHOTNAMESPACE   VOLUMEGROUPSNAPSHOT   AGE
groupsnapcontent-29f97dbc-d2a7-4204-b356-98a8c29870fb   true         Delete           hostpath.csi.k8s.io   vgs-class                  default                        my-group-snapshot     10s
% kubectl get vgs 
NAME                READYTOUSE   VOLUMEGROUPSNAPSHOTCLASS   VOLUMEGROUPSNAPSHOTCONTENT                              CREATIONTIME   AGE
my-group-snapshot   true         vgs-class                  groupsnapcontent-29f97dbc-d2a7-4204-b356-98a8c29870fb   11s            11s
% kubectl get vs  
NAME                                                                                           READYTOUSE   SOURCEPVC   SOURCESNAPSHOTCONTENT                                                                             RESTORESIZE   SNAPSHOTCLASS   SNAPSHOTCONTENT                                                                                   CREATIONTIME   AGE
snapshot-0a89a20f6f519bbb505bd29e6e8651e50319d3e527705239f9687d1fbce1a8a6-2023-10-13-8.50.54   true                     snapcontent-0a89a20f6f519bbb505bd29e6e8651e50319d3e527705239f9687d1fbce1a8a6-2023-10-13-8.50.54   1Gi                           snapcontent-0a89a20f6f519bbb505bd29e6e8651e50319d3e527705239f9687d1fbce1a8a6-2023-10-13-8.50.54   19s            19s
snapshot-4c6d03c90c122b50363288a808f0e784469b9730117fa028df007967a6b19359-2023-10-13-8.50.54   true                     snapcontent-4c6d03c90c122b50363288a808f0e784469b9730117fa028df007967a6b19359-2023-10-13-8.50.54   1Gi                           snapcontent-4c6d03c90c122b50363288a808f0e784469b9730117fa028df007967a6b19359-2023-10-13-8.50.54   19s            19s
% kubectl get vsc
NAME                                                                                              READYTOUSE   RESTORESIZE   DELETIONPOLICY   DRIVER                VOLUMESNAPSHOTCLASS   VOLUMESNAPSHOT                                                                                 VOLUMESNAPSHOTNAMESPACE   AGE
snapcontent-0a89a20f6f519bbb505bd29e6e8651e50319d3e527705239f9687d1fbce1a8a6-2023-10-13-8.50.54   true         1073741824    Delete           hostpath.csi.k8s.io                         snapshot-0a89a20f6f519bbb505bd29e6e8651e50319d3e527705239f9687d1fbce1a8a6-2023-10-13-8.50.54   default                   20s
snapcontent-4c6d03c90c122b50363288a808f0e784469b9730117fa028df007967a6b19359-2023-10-13-8.50.54   true         1073741824    Delete           hostpath.csi.k8s.io                         snapshot-4c6d03c90c122b50363288a808f0e784469b9730117fa028df007967a6b19359-2023-10-13-8.50.54   default                   20s

Delete the volume group snapshot and verify that objects are deleted from API server:

% kubectl delete -f volumegroupsnapshot-example.yaml 
volumegroupsnapshot.groupsnapshot.storage.k8s.io "my-group-snapshot" deleted
% kubectl get vgs
No resources found in default namespace.
% kubectl get vgsc
No resources found

Verify that VS and VSC still exist:

% kubectl get vs  
NAME                                                                                           READYTOUSE   SOURCEPVC   SOURCESNAPSHOTCONTENT                                                                             RESTORESIZE   SNAPSHOTCLASS   SNAPSHOTCONTENT                                                                                   CREATIONTIME   AGE
snapshot-0a89a20f6f519bbb505bd29e6e8651e50319d3e527705239f9687d1fbce1a8a6-2023-10-13-8.50.54   true                     snapcontent-0a89a20f6f519bbb505bd29e6e8651e50319d3e527705239f9687d1fbce1a8a6-2023-10-13-8.50.54   1Gi                           snapcontent-0a89a20f6f519bbb505bd29e6e8651e50319d3e527705239f9687d1fbce1a8a6-2023-10-13-8.50.54   19s            19s
snapshot-4c6d03c90c122b50363288a808f0e784469b9730117fa028df007967a6b19359-2023-10-13-8.50.54   true                     snapcontent-4c6d03c90c122b50363288a808f0e784469b9730117fa028df007967a6b19359-2023-10-13-8.50.54   1Gi                           snapcontent-4c6d03c90c122b50363288a808f0e784469b9730117fa028df007967a6b19359-2023-10-13-8.50.54   19s            19s
% kubectl get vsc
NAME                                                                                              READYTOUSE   RESTORESIZE   DELETIONPOLICY   DRIVER                VOLUMESNAPSHOTCLASS   VOLUMESNAPSHOT                                                                                 VOLUMESNAPSHOTNAMESPACE   AGE
snapcontent-0a89a20f6f519bbb505bd29e6e8651e50319d3e527705239f9687d1fbce1a8a6-2023-10-13-8.50.54   true         1073741824    Delete           hostpath.csi.k8s.io                         snapshot-0a89a20f6f519bbb505bd29e6e8651e50319d3e527705239f9687d1fbce1a8a6-2023-10-13-8.50.54   default                   20s
snapcontent-4c6d03c90c122b50363288a808f0e784469b9730117fa028df007967a6b19359-2023-10-13-8.50.54   true         1073741824    Delete           hostpath.csi.k8s.io                         snapshot-4c6d03c90c122b50363288a808f0e784469b9730117fa028df007967a6b19359-2023-10-13-8.50.54   default                   20s

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Introduce logic to delete volume group snapshots

linux-foundation-easycla · 2023-08-15T11:49:40Z

The committers listed above are authorized under a signed CLA.

✅ login: RaunakShah / name: Raunak Pradip Shah (facefba)

RaunakShah · 2023-10-13T09:16:14Z

/assign @xing-yang

pkg/sidecar-controller/csi_handler.go

pkg/utils/util.go

pkg/sidecar-controller/groupsnapshot_helper.go

xing-yang · 2023-10-14T21:17:48Z

pkg/common-controller/groupsnapshot_controller_helper.go


 	/*
-		TODO: Add finalizer to group snapshot
+		TODO: Add PVC finalizer


Why do we need PVC finalizer?

This is an open question I had - do we add any finalisers to PVCs, similar to the case for individual VolumeSnapshots?

Yes, we do need to add PVC finalizers when creating a snapshot from it.

So will the individual snapshot creation handle that, or does the group snapshot also need to add a finaliser?

Since the creation of the individual VolumeSnapshot API objects (that are part of a group snapshot) are handled by the group snapshot controller, the PVC finalizer should also be added by the group snapshot controller. Let me know if you see any problems.

In the first revision, let's try to separate the logic as much as possible to avoid introducing regression to the existing volume snapshot feature.

I'm going to merge this PR. Please address this comment in a followup PR.

pkg/common-controller/groupsnapshot_controller_helper.go

xing-yang · 2023-10-30T02:45:37Z

Can you add a release note?

xing-yang · 2023-11-03T01:54:52Z

/lgtm
/approve

k8s-ci-robot · 2023-11-03T01:55:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RaunakShah, 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~~ [xing-yang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

xing-yang · 2023-11-03T01:55:55Z

/hold

xing-yang · 2023-11-03T01:56:27Z

Actually can you please squash the commits? After that we can cancel the hold.

RaunakShah · 2023-11-03T07:47:21Z

Actually can you please squash the commits? After that we can cancel the hold.

Done

xing-yang · 2023-11-03T11:07:50Z

/hold cancel

xing-yang · 2023-11-03T14:16:33Z

/lgtm

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Aug 15, 2023

k8s-ci-robot requested review from pohly and yuxiangqian August 15, 2023 11:49

k8s-ci-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Aug 15, 2023

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 22, 2023

RaunakShah changed the title ~~[WIP][Do not review] DeleteVolumeGroupSnapshot API~~ DeleteVolumeGroupSnapshot API - part 1 Oct 13, 2023

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 13, 2023

k8s-ci-robot assigned xing-yang Oct 13, 2023

xing-yang reviewed Oct 14, 2023

View reviewed changes

RaunakShah force-pushed the deletegroupsnapshot-copy branch from 3edce23 to 49306f3 Compare October 25, 2023 07:19

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 1, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 3, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 3, 2023

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 3, 2023

add DeleteVolumeGroupSnapshot API to delete group snapshot

facefba

RaunakShah force-pushed the deletegroupsnapshot-copy branch from 5e44057 to facefba Compare November 3, 2023 07:46

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 3, 2023

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 3, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 3, 2023

k8s-ci-robot merged commit dad8b28 into kubernetes-csi:master Nov 3, 2023
8 checks passed

RaunakShah mentioned this pull request Nov 7, 2023

DeleteVolumeGroupSnapshot API - part 2 #952

Merged

xing-yang mentioned this pull request Nov 17, 2023

[WIP] add DeleteVolumeGroupSnapshot API to delete group snapshot #874

Closed

mpatlasov mentioned this pull request Feb 5, 2024

STOR-1700: Rebase external-snapshotter to v7.0.0 to get VolumeGroupSnapshot openshift/csi-external-snapshotter#139

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DeleteVolumeGroupSnapshot API - part 1 #882

DeleteVolumeGroupSnapshot API - part 1 #882

RaunakShah commented Aug 15, 2023 •

edited

Loading

linux-foundation-easycla bot commented Aug 15, 2023 •

edited

Loading

RaunakShah commented Oct 13, 2023

xing-yang Oct 14, 2023

RaunakShah Oct 25, 2023

xing-yang Oct 30, 2023

RaunakShah Nov 1, 2023

xing-yang Nov 2, 2023 •

edited

Loading

xing-yang Nov 2, 2023

xing-yang Nov 3, 2023

xing-yang commented Oct 30, 2023

xing-yang commented Nov 3, 2023

k8s-ci-robot commented Nov 3, 2023

xing-yang commented Nov 3, 2023

xing-yang commented Nov 3, 2023

RaunakShah commented Nov 3, 2023

xing-yang commented Nov 3, 2023

xing-yang commented Nov 3, 2023

DeleteVolumeGroupSnapshot API - part 1 #882

DeleteVolumeGroupSnapshot API - part 1 #882

Conversation

RaunakShah commented Aug 15, 2023 • edited Loading

linux-foundation-easycla bot commented Aug 15, 2023 • edited Loading

RaunakShah commented Oct 13, 2023

xing-yang Oct 14, 2023

Choose a reason for hiding this comment

RaunakShah Oct 25, 2023

Choose a reason for hiding this comment

xing-yang Oct 30, 2023

Choose a reason for hiding this comment

RaunakShah Nov 1, 2023

Choose a reason for hiding this comment

xing-yang Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

xing-yang Nov 2, 2023

Choose a reason for hiding this comment

xing-yang Nov 3, 2023

Choose a reason for hiding this comment

xing-yang commented Oct 30, 2023

xing-yang commented Nov 3, 2023

k8s-ci-robot commented Nov 3, 2023

xing-yang commented Nov 3, 2023

xing-yang commented Nov 3, 2023

RaunakShah commented Nov 3, 2023

xing-yang commented Nov 3, 2023

xing-yang commented Nov 3, 2023

RaunakShah commented Aug 15, 2023 •

edited

Loading

linux-foundation-easycla bot commented Aug 15, 2023 •

edited

Loading

xing-yang Nov 2, 2023 •

edited

Loading