diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go index 120ce9b5..673946c8 100644 --- a/pkg/common-controller/framework_test.go +++ b/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()) } diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index e2f56e91..db346b4e 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/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) diff --git a/pkg/sidecar-controller/snapshot_controller.go b/pkg/sidecar-controller/snapshot_controller.go index 0cb8d5f9..313f173d 100644 --- a/pkg/sidecar-controller/snapshot_controller.go +++ b/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()) }