From 875662b365e60621299e0ffb5cd4cab37039fb29 Mon Sep 17 00:00:00 2001 From: Raunak Pradip Shah Date: Wed, 6 Dec 2023 16:16:30 -0800 Subject: [PATCH] Add finalizer to indicate volume snapshot is part of a group --- .../groupsnapshot_controller_helper.go | 3 ++ pkg/common-controller/snapshot_controller.go | 41 +++++++++++-------- .../groupsnapshot_helper.go | 7 ++-- pkg/utils/util.go | 2 + 4 files changed, 32 insertions(+), 21 deletions(-) diff --git a/pkg/common-controller/groupsnapshot_controller_helper.go b/pkg/common-controller/groupsnapshot_controller_helper.go index 543638eb..92f377dc 100644 --- a/pkg/common-controller/groupsnapshot_controller_helper.go +++ b/pkg/common-controller/groupsnapshot_controller_helper.go @@ -1114,6 +1114,9 @@ func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimesta for _, snapshotRef := range groupSnapshot.Status.VolumeSnapshotRefList { snapshot, err := ctrl.snapshotLister.VolumeSnapshots(snapshotRef.Namespace).Get(snapshotRef.Name) if err != nil { + if apierrs.IsNotFound(err) { + continue + } return err } if ctrl.isVolumeBeingCreatedFromSnapshot(snapshot) { diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index d509fb29..be7d96bb 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/pkg/common-controller/snapshot_controller.go @@ -286,21 +286,6 @@ func (ctrl *csiSnapshotCommonController) processSnapshotWithDeletionTimestamp(sn content = nil } - // Block deletion if this snapshot belongs to a group snapshot. - if snapshot.Status != nil && snapshot.Status.VolumeGroupSnapshotName != nil { - groupSnapshot, err := ctrl.groupSnapshotLister.VolumeGroupSnapshots(snapshot.Namespace).Get(*snapshot.Status.VolumeGroupSnapshotName) - if err == nil { - msg := fmt.Sprintf("deletion of the individual volume snapshot %s is not allowed as it belongs to group snapshot %s. Deleting the group snapshot will trigger the deletion of all the individual volume snapshots that are part of the group.", utils.SnapshotKey(snapshot), utils.GroupSnapshotKey(groupSnapshot)) - klog.Error(msg) - ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotDeleteError", msg) - return fmt.Errorf(msg) - } - if !apierrs.IsNotFound(err) { - klog.Errorf("failed to delete snapshot %s: %v", utils.SnapshotKey(snapshot), err) - return err - } - } - klog.V(5).Infof("processSnapshotWithDeletionTimestamp[%s]: delete snapshot content and remove finalizer from snapshot if needed", utils.SnapshotKey(snapshot)) return ctrl.checkandRemoveSnapshotFinalizersAndCheckandDeleteContent(snapshot, content, deleteContent) @@ -323,6 +308,23 @@ func (ctrl *csiSnapshotCommonController) checkandRemoveSnapshotFinalizersAndChec return nil } + removeGroupFinalizer := false + // Block deletion if this snapshot belongs to a group snapshot. + if snapshot.Status != nil && snapshot.Status.VolumeGroupSnapshotName != nil { + removeGroupFinalizer = true + groupSnapshot, err := ctrl.groupSnapshotLister.VolumeGroupSnapshots(snapshot.Namespace).Get(*snapshot.Status.VolumeGroupSnapshotName) + if err == nil { + msg := fmt.Sprintf("deletion of the individual volume snapshot %s is not allowed as it belongs to group snapshot %s. Deleting the group snapshot will trigger the deletion of all the individual volume snapshots that are part of the group.", utils.SnapshotKey(snapshot), utils.GroupSnapshotKey(groupSnapshot)) + klog.Error(msg) + ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotDeletePending", msg) + return fmt.Errorf(msg) + } + if !apierrs.IsNotFound(err) { + klog.Errorf("failed to delete snapshot %s: %v", utils.SnapshotKey(snapshot), err) + return err + } + } + // regardless of the deletion policy, set the VolumeSnapshotBeingDeleted on // content object, this is to allow snapshotter sidecar controller to conduct // a delete operation whenever the content has deletion timestamp set. @@ -360,7 +362,7 @@ func (ctrl *csiSnapshotCommonController) checkandRemoveSnapshotFinalizersAndChec // c. If deletion will not cascade to the content, remove the finalizer on // the snapshot such that it can be removed from API server. removeBoundFinalizer := !(content != nil && deleteContent) - return ctrl.removeSnapshotFinalizer(snapshot, true, removeBoundFinalizer) + return ctrl.removeSnapshotFinalizer(snapshot, true, removeBoundFinalizer, removeGroupFinalizer) } // checkandAddSnapshotFinalizers checks and adds snapshot finailzers when needed @@ -1542,8 +1544,8 @@ func (ctrl *csiSnapshotCommonController) addSnapshotFinalizer(snapshot *crdv1.Vo } // removeSnapshotFinalizer removes a Finalizer for VolumeSnapshot. -func (ctrl *csiSnapshotCommonController) removeSnapshotFinalizer(snapshot *crdv1.VolumeSnapshot, removeSourceFinalizer bool, removeBoundFinalizer bool) error { - if !removeSourceFinalizer && !removeBoundFinalizer { +func (ctrl *csiSnapshotCommonController) removeSnapshotFinalizer(snapshot *crdv1.VolumeSnapshot, removeSourceFinalizer bool, removeBoundFinalizer bool, removeGroupFinalizer bool) error { + if !removeSourceFinalizer && !removeBoundFinalizer && !removeGroupFinalizer { return nil } @@ -1571,6 +1573,9 @@ func (ctrl *csiSnapshotCommonController) removeSnapshotFinalizer(snapshot *crdv1 if removeBoundFinalizer { snapshotClone.ObjectMeta.Finalizers = utils.RemoveString(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) } + if removeGroupFinalizer { + snapshotClone.ObjectMeta.Finalizers = utils.RemoveString(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotInGroupFinalizer) + } newSnapshot, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{}) if err != nil { return newControllerUpdateError(snapshot.Name, err.Error()) diff --git a/pkg/sidecar-controller/groupsnapshot_helper.go b/pkg/sidecar-controller/groupsnapshot_helper.go index f4a0cc4e..286eb96a 100644 --- a/pkg/sidecar-controller/groupsnapshot_helper.go +++ b/pkg/sidecar-controller/groupsnapshot_helper.go @@ -461,9 +461,10 @@ func (ctrl *csiSnapshotSideCarController) createGroupSnapshotWrapper(groupSnapsh label["volumeGroupSnapshotName"] = groupSnapshotContent.Spec.VolumeGroupSnapshotRef.Name volumeSnapshot := &crdv1.VolumeSnapshot{ ObjectMeta: metav1.ObjectMeta{ - Name: volumeSnapshotName, - Namespace: volumeSnapshotNamespace, - Labels: label, + Name: volumeSnapshotName, + Namespace: volumeSnapshotNamespace, + Labels: label, + Finalizers: []string{utils.VolumeSnapshotInGroupFinalizer}, }, Spec: crdv1.VolumeSnapshotSpec{ Source: crdv1.VolumeSnapshotSource{ diff --git a/pkg/utils/util.go b/pkg/utils/util.go index 4a3dcd88..ec654571 100644 --- a/pkg/utils/util.go +++ b/pkg/utils/util.go @@ -74,6 +74,8 @@ const ( VolumeSnapshotBoundFinalizer = "snapshot.storage.kubernetes.io/volumesnapshot-bound-protection" // Name of finalizer on VolumeSnapshot that is used as a source to create a PVC VolumeSnapshotAsSourceFinalizer = "snapshot.storage.kubernetes.io/volumesnapshot-as-source-protection" + // Name of finalizer on VolumeSnapshot that is a part of a VolumeGroupSnapshot + VolumeSnapshotInGroupFinalizer = "snapshot.storage.kubernetes.io/volumesnapshot-in-group-protection" // Name of finalizer on PVCs that is being used as a source to create VolumeSnapshots PVCFinalizer = "snapshot.storage.kubernetes.io/pvc-as-source-protection" // Name of finalizer on VolumeGroupSnapshotContents that are bound by VolumeGroupSnapshots