Use patch for snapshot-controller when there are no finalizers

- Also address PR feedback re: avoid a deepCopy for annotations patch

Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>

pkg/common-controller/framework_test.go

@@ -354,12 +354,6 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
 
 			storedVer, _ := strconv.Atoi(storedSnapshot.ResourceVersion)
 			storedSnapshot.ResourceVersion = strconv.Itoa(storedVer + 1)
-
-			// // If we were updating annotations and the new annotations are nil, leave as empty.
-			// // This seems to be the behavior for merge-patching nil & empty annotations
-			// if !reflect.DeepEqual(storedSnapshotContent.Annotations, content.Annotations) && content.Annotations == nil {
-			// 	content.Annotations = make(map[string]string)
-			// }
 		} else {
 			return true, nil, fmt.Errorf("cannot update snapshot %s: snapshot not found", action.GetName())
 		}

pkg/common-controller/snapshot_controller.go

@@ -1421,23 +1421,16 @@ func (ctrl *csiSnapshotCommonController) addSnapshotFinalizer(snapshot *crdv1.Vo
 	var updatedSnapshot *crdv1.VolumeSnapshot
 	var err error
 
-	// Must perform an update if no finalizers exist
+	var patches []utils.PatchOp
 	if len(snapshot.ObjectMeta.Finalizers) == 0 {
-		snapshotClone := snapshot.DeepCopy()
-		if addSourceFinalizer {
-			snapshotClone.ObjectMeta.Finalizers = append(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer)
-		}
-		if addBoundFinalizer {
-			snapshotClone.ObjectMeta.Finalizers = append(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer)
-		}
-		updatedSnapshot, err = ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{})
-		if err != nil {
-			return newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error())
-		}
+		// Replace finalizers with new array if there are no other finalizers
+		patches = append(patches, utils.PatchOp{
+			Op:    "add",
+			Path:  "/metadata/finalizers",
+			Value: []string{utils.VolumeSnapshotContentFinalizer},
+		})
 	} else {
 		// Otherwise, perform a patch
-		var patches []utils.PatchOp
-
 		if addSourceFinalizer {
 			patches = append(patches, utils.PatchOp{
 				Op:    "add",
@@ -1452,11 +1445,10 @@ func (ctrl *csiSnapshotCommonController) addSnapshotFinalizer(snapshot *crdv1.Vo
 				Value: utils.VolumeSnapshotBoundFinalizer,
 			})
 		}
-
-		updatedSnapshot, err = utils.PatchVolumeSnapshot(snapshot, patches, ctrl.clientset)
-		if err != nil {
-			return newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error())
-		}
+	}
+	updatedSnapshot, err = utils.PatchVolumeSnapshot(snapshot, patches, ctrl.clientset)
+	if err != nil {
+		return newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error())
 	}
 
 	_, err = ctrl.storeSnapshotUpdate(updatedSnapshot)

pkg/sidecar-controller/snapshot_controller.go

@@ -583,18 +583,23 @@ func (ctrl *csiSnapshotSideCarController) setAnnVolumeSnapshotBeingCreated(conte
 	}
 
 	// Set AnnVolumeSnapshotBeingCreated
+	// Combine existing annotations with the new annotations.
+	// If there are no existing annotations, we create a new map.
 	klog.V(5).Infof("setAnnVolumeSnapshotBeingCreated: set annotation [%s:yes] on content [%s].", utils.AnnVolumeSnapshotBeingCreated, content.Name)
-	contentClone := content.DeepCopy()
-	metav1.SetMetaDataAnnotation(&contentClone.ObjectMeta, utils.AnnVolumeSnapshotBeingCreated, "yes")
+	patchedAnnotations := make(map[string]string)
+	for k, v := range content.GetAnnotations() {
+		patchedAnnotations[k] = v
+	}
+	patchedAnnotations[utils.AnnVolumeSnapshotBeingCreated] = "yes"
 
 	var patches []utils.PatchOp
 	patches = append(patches, utils.PatchOp{
 		Op:    "replace",
 		Path:  "/metadata/annotations",
-		Value: contentClone.ObjectMeta.GetAnnotations(),
+		Value: patchedAnnotations,
 	})
 
-	patchedContent, err := utils.PatchVolumeSnapshotContent(contentClone, patches, ctrl.clientset)
+	patchedContent, err := utils.PatchVolumeSnapshotContent(content, patches, ctrl.clientset)
 	if err != nil {
 		return content, newControllerUpdateError(content.Name, err.Error())
 	}