Merge pull request #526 from ggriffiths/updatecontentstatuserror_patch

Use JSON patch for many VolumeSnapshot and VolumeSnapshotContent updates

deploy/kubernetes/csi-snapshotter/rbac-csi-snapshotter.yaml

@@ -35,10 +35,10 @@ rules:
     verbs: ["get", "list", "watch"]
   - apiGroups: ["snapshot.storage.k8s.io"]
     resources: ["volumesnapshotcontents"]
-    verbs: ["create", "get", "list", "watch", "update", "delete"]
+    verbs: ["create", "get", "list", "watch", "update", "delete", "patch"]
   - apiGroups: ["snapshot.storage.k8s.io"]
     resources: ["volumesnapshotcontents/status"]
-    verbs: ["update"]
+    verbs: ["update", "patch"]
 
 ---
 kind: ClusterRoleBinding

deploy/kubernetes/snapshot-controller/rbac-snapshot-controller.yaml

@@ -34,13 +34,16 @@ rules:
     verbs: ["get", "list", "watch"]
   - apiGroups: ["snapshot.storage.k8s.io"]
     resources: ["volumesnapshotcontents"]
-    verbs: ["create", "get", "list", "watch", "update", "delete"]
+    verbs: ["create", "get", "list", "watch", "update", "delete", "patch"]
+  - apiGroups: ["snapshot.storage.k8s.io"]
+    resources: ["volumesnapshotcontents/status"]
+    verbs: ["patch"]
   - apiGroups: ["snapshot.storage.k8s.io"]
     resources: ["volumesnapshots"]
-    verbs: ["get", "list", "watch", "update"]
+    verbs: ["get", "list", "watch", "update", "patch"]
   - apiGroups: ["snapshot.storage.k8s.io"]
     resources: ["volumesnapshots/status"]
-    verbs: ["update"]
+    verbs: ["update", "patch"]
 
 ---
 kind: ClusterRoleBinding

go.mod

@@ -4,6 +4,7 @@ go 1.16
 
 require (
 	github.com/container-storage-interface/spec v1.5.0
+	github.com/evanphx/json-patch v4.11.0+incompatible
 	github.com/fsnotify/fsnotify v1.4.9
 	github.com/golang/mock v1.4.4
 	github.com/golang/protobuf v1.5.2

pkg/common-controller/framework_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package common_controller
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"net/http"
@@ -29,8 +30,7 @@ import (
 	"testing"
 	"time"
 
-	"k8s.io/client-go/util/workqueue"
-
+	jsonpatch "github.com/evanphx/json-patch"
 	crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
 	clientset "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned"
 	"github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned/fake"
@@ -55,6 +55,7 @@ import (
 	core "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/client-go/util/workqueue"
 	klog "k8s.io/klog/v2"
 )
 
@@ -258,6 +259,46 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
 		klog.V(4).Infof("saved updated content %s", content.Name)
 		return true, content, nil
 
+	case action.Matches("patch", "volumesnapshotcontents"):
+		content := &crdv1.VolumeSnapshotContent{}
+		action := action.(core.PatchAction)
+
+		// Check and bump object version
+		storedSnapshotContent, found := r.contents[action.GetName()]
+		if found {
+			// Apply patch
+			storedSnapshotBytes, err := json.Marshal(storedSnapshotContent)
+			if err != nil {
+				return true, nil, err
+			}
+			contentPatch, err := jsonpatch.DecodePatch(action.GetPatch())
+			if err != nil {
+				return true, nil, err
+			}
+
+			modified, err := contentPatch.Apply(storedSnapshotBytes)
+			if err != nil {
+				return true, nil, err
+			}
+
+			err = json.Unmarshal(modified, content)
+			if err != nil {
+				return true, nil, err
+			}
+
+			storedVer, _ := strconv.Atoi(content.ResourceVersion)
+			content.ResourceVersion = strconv.Itoa(storedVer + 1)
+		} else {
+			return true, nil, fmt.Errorf("cannot update snapshot content %s: snapshot content not found", action.GetName())
+		}
+
+		// Store the updated object to appropriate places.
+		r.contents[content.Name] = content
+		r.changedObjects = append(r.changedObjects, content)
+		r.changedSinceLastSync++
+		klog.V(4).Infof("saved updated content %s", content.Name)
+		return true, content, nil
+
 	case action.Matches("update", "volumesnapshots"):
 		obj := action.(core.UpdateAction).GetObject()
 		snapshot := obj.(*crdv1.VolumeSnapshot)
@@ -284,6 +325,45 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
 		klog.V(4).Infof("saved updated snapshot %s", snapshot.Name)
 		return true, snapshot, nil
 
+	case action.Matches("patch", "volumesnapshots"):
+		action := action.(core.PatchAction)
+		// Check and bump object version
+		storedSnapshot, found := r.snapshots[action.GetName()]
+		if found {
+			// Apply patch
+			storedSnapshotBytes, err := json.Marshal(storedSnapshot)
+			if err != nil {
+				return true, nil, err
+			}
+			snapPatch, err := jsonpatch.DecodePatch(action.GetPatch())
+			if err != nil {
+				return true, nil, err
+			}
+
+			modified, err := snapPatch.Apply(storedSnapshotBytes)
+			if err != nil {
+				return true, nil, err
+			}
+
+			err = json.Unmarshal(modified, storedSnapshot)
+			if err != nil {
+				return true, nil, err
+			}
+
+			storedVer, _ := strconv.Atoi(storedSnapshot.ResourceVersion)
+			storedSnapshot.ResourceVersion = strconv.Itoa(storedVer + 1)
+		} else {
+			return true, nil, fmt.Errorf("cannot update snapshot %s: snapshot not found", action.GetName())
+		}
+
+		// Store the updated object to appropriate places.
+		r.snapshots[storedSnapshot.Name] = storedSnapshot
+		r.changedObjects = append(r.changedObjects, storedSnapshot)
+		r.changedSinceLastSync++
+
+		klog.V(4).Infof("saved updated snapshot %s", storedSnapshot.Name)
+		return true, storedSnapshot, nil
+
 	case action.Matches("get", "volumesnapshotcontents"):
 		name := action.(core.GetAction).GetName()
 		content, found := r.contents[name]
@@ -437,6 +517,7 @@ func (r *snapshotReactor) checkContents(expectedContents []*crdv1.VolumeSnapshot
 		}
 		gotMap[v.Name] = v
 	}
+
 	if !reflect.DeepEqual(expectedMap, gotMap) {
 		// Print ugly but useful diff of expected and received objects for
 		// easier debugging.
@@ -714,6 +795,8 @@ func newSnapshotReactor(kubeClient *kubefake.Clientset, client *fake.Clientset,
 	client.AddReactor("create", "volumesnapshotcontents", reactor.React)
 	client.AddReactor("update", "volumesnapshotcontents", reactor.React)
 	client.AddReactor("update", "volumesnapshots", reactor.React)
+	client.AddReactor("patch", "volumesnapshotcontents", reactor.React)
+	client.AddReactor("patch", "volumesnapshots", reactor.React)
 	client.AddReactor("update", "volumesnapshotclasses", reactor.React)
 	client.AddReactor("get", "volumesnapshotcontents", reactor.React)
 	client.AddReactor("get", "volumesnapshots", reactor.React)
@@ -1169,6 +1252,10 @@ func testAddSnapshotFinalizer(ctrl *csiSnapshotCommonController, reactor *snapsh
 	return ctrl.addSnapshotFinalizer(test.initialSnapshots[0], true, true)
 }
 
+func testAddSingleSnapshotFinalizer(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error {
+	return ctrl.addSnapshotFinalizer(test.initialSnapshots[0], false, true)
+}
+
 func testRemoveSnapshotFinalizer(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error {
 	return ctrl.removeSnapshotFinalizer(test.initialSnapshots[0], true, true)
 }
@@ -1426,8 +1513,10 @@ func evaluateFinalizerTests(ctrl *csiSnapshotCommonController, reactor *snapshot
 		if funcName == "testAddSnapshotFinalizer" {
 			for _, snapshot := range reactor.snapshots {
 				if test.initialSnapshots[0].Name == snapshot.Name {
-					if !utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) && utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) &&
-						!utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) && utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) {
+					if !utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) &&
+						utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) &&
+						!utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) &&
+						utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) {
 						klog.V(4).Infof("test %q succeeded. Finalizers are added to snapshot %s", test.name, snapshot.Name)
 						bHasSnapshotFinalizer = true
 					}
@@ -1435,15 +1524,18 @@ func evaluateFinalizerTests(ctrl *csiSnapshotCommonController, reactor *snapshot
 				}
 			}
 			if test.expectSuccess && !bHasSnapshotFinalizer {
-				t.Errorf("Test %q: failed to add finalizer to Snapshot %s", test.name, test.initialSnapshots[0].Name)
+				t.Errorf("Test %q: failed to add finalizer to Snapshot %s. Finalizers: %s", test.name, test.initialSnapshots[0].Name, test.initialSnapshots[0].GetFinalizers())
 			}
 		}
 		bHasSnapshotFinalizer = true
 		if funcName == "testRemoveSnapshotFinalizer" {
 			for _, snapshot := range reactor.snapshots {
 				if test.initialSnapshots[0].Name == snapshot.Name {
-					if utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) && !utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) &&
-						utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) && !utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) {
+					if utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) &&
+						!utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) &&
+						utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) &&
+						!utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) {
+
 						klog.V(4).Infof("test %q succeeded. SnapshotFinalizer is removed from Snapshot %s", test.name, snapshot.Name)
 						bHasSnapshotFinalizer = false
 					}

pkg/common-controller/snapshot_controller.go

@@ -809,10 +809,25 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotErrorStatusWithEvent(snap
 
 // addContentFinalizer adds a Finalizer for VolumeSnapshotContent.
 func (ctrl *csiSnapshotCommonController) addContentFinalizer(content *crdv1.VolumeSnapshotContent) error {
-	contentClone := content.DeepCopy()
-	contentClone.ObjectMeta.Finalizers = append(contentClone.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer)
 
-	newContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{})
+	var patches []utils.PatchOp
+	if len(content.Finalizers) > 0 {
+		// Add to the end of the finalizers if we have any other finalizers
+		patches = append(patches, utils.PatchOp{
+			Op:    "add",
+			Path:  "/metadata/finalizers/-",
+			Value: utils.VolumeSnapshotContentFinalizer,
+		})
+
+	} else {
+		// 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},
+		})
+	}
+	newContent, err := utils.PatchVolumeSnapshotContent(content, patches, ctrl.clientset)
 	if err != nil {
 		return newControllerUpdateError(content.Name, err.Error())
 	}
@@ -983,15 +998,26 @@ func (ctrl *csiSnapshotCommonController) checkandBindSnapshotContent(snapshot *c
 	} else if content.Spec.VolumeSnapshotRef.UID != "" && content.Spec.VolumeSnapshotClassName != nil {
 		return content, nil
 	}
-	contentClone := content.DeepCopy()
-	contentClone.Spec.VolumeSnapshotRef.UID = snapshot.UID
+
+	patches := []utils.PatchOp{
+		{
+			Op:    "replace",
+			Path:  "/spec/volumeSnapshotRef/uid",
+			Value: string(snapshot.UID),
+		},
+	}
 	if snapshot.Spec.VolumeSnapshotClassName != nil {
 		className := *(snapshot.Spec.VolumeSnapshotClassName)
-		contentClone.Spec.VolumeSnapshotClassName = &className
+		patches = append(patches, utils.PatchOp{
+			Op:    "replace",
+			Path:  "/spec/volumeSnapshotClassName",
+			Value: className,
+		})
 	}
-	newContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{})
+
+	newContent, err := utils.PatchVolumeSnapshotContent(content, patches, ctrl.clientset)
 	if err != nil {
-		klog.V(4).Infof("updating VolumeSnapshotContent[%s] error status failed %v", contentClone.Name, err)
+		klog.V(4).Infof("updating VolumeSnapshotContent[%s] error status failed %v", content.Name, err)
 		return content, err
 	}
 
@@ -1392,24 +1418,55 @@ func isControllerUpdateFailError(err *crdv1.VolumeSnapshotError) bool {
 
 // addSnapshotFinalizer adds a Finalizer for VolumeSnapshot.
 func (ctrl *csiSnapshotCommonController) addSnapshotFinalizer(snapshot *crdv1.VolumeSnapshot, addSourceFinalizer bool, addBoundFinalizer bool) error {
-	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)
-	}
-	newSnapshot, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{})
-	if err != nil {
-		return newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error())
+	var updatedSnapshot *crdv1.VolumeSnapshot
+	var err error
+
+	// NOTE(ggriffiths): Must perform an update if no finalizers exist.
+	// Unable to find a patch that correctly updated the finalizers if none currently exist.
+	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())
+		}
+	} else {
+		// Otherwise, perform a patch
+		var patches []utils.PatchOp
+
+		// If finalizers exist already, add new ones to the end of the array
+		if addSourceFinalizer {
+			patches = append(patches, utils.PatchOp{
+				Op:    "add",
+				Path:  "/metadata/finalizers/-",
+				Value: utils.VolumeSnapshotAsSourceFinalizer,
+			})
+		}
+		if addBoundFinalizer {
+			patches = append(patches, utils.PatchOp{
+				Op:    "add",
+				Path:  "/metadata/finalizers/-",
+				Value: utils.VolumeSnapshotBoundFinalizer,
+			})
+		}
+
+		updatedSnapshot, err = utils.PatchVolumeSnapshot(snapshot, patches, ctrl.clientset)
+		if err != nil {
+			return newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error())
+		}
 	}
 
-	_, err = ctrl.storeSnapshotUpdate(newSnapshot)
+	_, err = ctrl.storeSnapshotUpdate(updatedSnapshot)
 	if err != nil {
 		klog.Errorf("failed to update snapshot store %v", err)
 	}
 
-	klog.V(5).Infof("Added protection finalizer to volume snapshot %s", utils.SnapshotKey(newSnapshot))
+	klog.V(5).Infof("Added protection finalizer to volume snapshot %s", utils.SnapshotKey(updatedSnapshot))
 	return nil
 }
 
@@ -1489,14 +1546,21 @@ func (ctrl *csiSnapshotCommonController) setAnnVolumeSnapshotBeingDeleted(conten
 	// Set AnnVolumeSnapshotBeingDeleted if it is not set yet
 	if !metav1.HasAnnotation(content.ObjectMeta, utils.AnnVolumeSnapshotBeingDeleted) {
 		klog.V(5).Infof("setAnnVolumeSnapshotBeingDeleted: set annotation [%s] on content [%s].", utils.AnnVolumeSnapshotBeingDeleted, content.Name)
+		var patches []utils.PatchOp
 		metav1.SetMetaDataAnnotation(&content.ObjectMeta, utils.AnnVolumeSnapshotBeingDeleted, "yes")
+		patches = append(patches, utils.PatchOp{
+			Op:    "replace",
+			Path:  "/metadata/annotations",
+			Value: content.ObjectMeta.GetAnnotations(),
+		})
 
-		updateContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), content, metav1.UpdateOptions{})
+		patchedContent, err := utils.PatchVolumeSnapshotContent(content, patches, ctrl.clientset)
 		if err != nil {
 			return content, newControllerUpdateError(content.Name, err.Error())
 		}
+
 		// update content if update is successful
-		content = updateContent
+		content = patchedContent
 
 		_, err = ctrl.storeContentUpdate(content)
 		if err != nil {

pkg/common-controller/snapshot_finalizer_test.go

@@ -19,6 +19,7 @@ package common_controller
 import (
 	"testing"
 
+	"github.com/kubernetes-csi/external-snapshotter/v4/pkg/utils"
 	v1 "k8s.io/api/core/v1"
 )
 
@@ -70,7 +71,14 @@ func TestSnapshotFinalizer(t *testing.T) {
 			expectSuccess:    true,
 		},
 		{
-			name:             "2-1 - successful remove Snapshot finalizer",
+			name:             "2-2 - successful add single Snapshot finalizer with patch",
+			initialSnapshots: withSnapshotFinalizers(newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", classSilver, "", &False, nil, nil, nil, false, false, nil), utils.VolumeSnapshotBoundFinalizer),
+			initialClaims:    newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty),
+			test:             testAddSingleSnapshotFinalizer,
+			expectSuccess:    true,
+		},
+		{
+			name:             "2-3 - successful remove Snapshot finalizer",
 			initialSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", classSilver, "", &False, nil, nil, nil, false, true, nil),
 			initialClaims:    newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty),
 			test:             testRemoveSnapshotFinalizer,

pkg/common-controller/snapshot_update_test.go

@@ -162,7 +162,7 @@ func TestSync(t *testing.T) {
 			expectedSnapshots: newSnapshotArray("snap2-12", "snapuid2-12", "", "content2-12", validSecretClass, "content2-12", &False, nil, nil, newVolumeError("Snapshot failed to bind VolumeSnapshotContent, mock update error"), false, true, nil),
 			errors: []reactorError{
 				// Inject error to the forth client.VolumesnapshotV1().VolumeSnapshots().Update call.
-				{"update", "volumesnapshotcontents", errors.New("mock update error")},
+				{"patch", "volumesnapshotcontents", errors.New("mock update error")},
 			},
 			test: testSyncSnapshot,
 		},
@@ -312,7 +312,7 @@ func TestSync(t *testing.T) {
 			initialSecrets:   []*v1.Secret{secret()},
 			errors: []reactorError{
 				// Inject error to the forth client.VolumesnapshotV1().VolumeSnapshots().Update call.
-				{"update", "volumesnapshotcontents", errors.New("mock update error")},
+				{"patch", "volumesnapshotcontents", errors.New("mock update error")},
 			},
 			expectSuccess: false,
 			test:          testSyncContentError,

pkg/sidecar-controller/content_create_test.go

@@ -85,7 +85,7 @@ func TestSyncContent(t *testing.T) {
 		{
 			name: "1-3: Basic sync content create snapshot with non-existent secret",
 			initialContents: withContentAnnotations(withContentStatus(newContentArray("content1-3", "snapuid1-3", "snap1-3", "sid1-3", invalidSecretClass, "", "volume-handle-1-3", retainPolicy, nil, &defaultSize, true),
-				nil), map[string]string{
+				&crdv1.VolumeSnapshotContentStatus{}), map[string]string{
 				utils.AnnDeletionSecretRefName:      "",
 				utils.AnnDeletionSecretRefNamespace: "",
 			}),
@@ -94,12 +94,12 @@ func TestSyncContent(t *testing.T) {
 					SnapshotHandle: nil,
 					RestoreSize:    nil,
 					ReadyToUse:     &False,
-					Error:          newSnapshotError("Failed to create snapshot: failed to get input parameters to create snapshot for content content1-3: \"cannot retrieve secrets for snapshot content \\\"content1-3\\\", err: secret name or namespace not specified\""),
+					Error:          newSnapshotError("Failed to check and update snapshot content: failed to get input parameters to create snapshot for content content1-3: \"cannot retrieve secrets for snapshot content \\\"content1-3\\\", err: secret name or namespace not specified\""),
 				}), map[string]string{
 				utils.AnnDeletionSecretRefName:      "",
 				utils.AnnDeletionSecretRefNamespace: "",
 			}), initialSecrets: []*v1.Secret{}, // no initial secret created
-			expectedEvents: []string{"Warning SnapshotCreationFailed"},
+			expectedEvents: []string{"Warning SnapshotContentCheckandUpdateFailed"},
 			errors:         noerrors,
 			test:           testSyncContent,
 		},
@@ -149,7 +149,7 @@ func TestSyncContent(t *testing.T) {
 		{
 			name: "1-5: Basic sync content create snapshot with failed secret call",
 			initialContents: withContentAnnotations(withContentStatus(newContentArray("content1-5", "snapuid1-5", "snap1-5", "sid1-5", invalidSecretClass, "", "volume-handle-1-5", retainPolicy, nil, &defaultSize, true),
-				nil), map[string]string{
+				&crdv1.VolumeSnapshotContentStatus{}), map[string]string{
 				utils.AnnDeletionSecretRefName:      "secret",
 				utils.AnnDeletionSecretRefNamespace: "default",
 			}),
@@ -158,12 +158,12 @@ func TestSyncContent(t *testing.T) {
 					SnapshotHandle: nil,
 					RestoreSize:    nil,
 					ReadyToUse:     &False,
-					Error:          newSnapshotError("Failed to create snapshot: failed to get input parameters to create snapshot for content content1-5: \"cannot get credentials for snapshot content \\\"content1-5\\\"\""),
+					Error:          newSnapshotError(`Failed to check and update snapshot content: failed to get input parameters to create snapshot for content content1-5: "cannot get credentials for snapshot content \"content1-5\""`),
 				}), map[string]string{
 				utils.AnnDeletionSecretRefName:      "secret",
 				utils.AnnDeletionSecretRefNamespace: "default",
 			}), initialSecrets: []*v1.Secret{}, // no initial secret created
-			expectedEvents: []string{"Warning SnapshotCreationFailed"},
+			expectedEvents: []string{"Warning SnapshotContentCheckandUpdateFailed"},
 			errors: []reactorError{
 				// Inject error to the first client.VolumesnapshotV1().VolumeSnapshots().Update call.
 				// All other calls will succeed.

pkg/sidecar-controller/framework_test.go

@@ -15,6 +15,7 @@ package sidecar_controller
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"reflect"
@@ -25,6 +26,7 @@ import (
 	"testing"
 	"time"
 
+	jsonpatch "github.com/evanphx/json-patch"
 	crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
 	clientset "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned"
 	"github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned/fake"
@@ -215,6 +217,46 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
 		klog.V(4).Infof("saved updated content %s", content.Name)
 		return true, content, nil
 
+	case action.Matches("patch", "volumesnapshotcontents"):
+		content := &crdv1.VolumeSnapshotContent{}
+		action := action.(core.PatchAction)
+
+		// Check and bump object version
+		storedSnapshotContent, found := r.contents[action.GetName()]
+		if found {
+			// Apply patch
+			storedSnapshotBytes, err := json.Marshal(storedSnapshotContent)
+			if err != nil {
+				return true, nil, err
+			}
+			contentPatch, err := jsonpatch.DecodePatch(action.GetPatch())
+			if err != nil {
+				return true, nil, err
+			}
+
+			modified, err := contentPatch.Apply(storedSnapshotBytes)
+			if err != nil {
+				return true, nil, err
+			}
+
+			err = json.Unmarshal(modified, content)
+			if err != nil {
+				return true, nil, err
+			}
+
+			storedVer, _ := strconv.Atoi(content.ResourceVersion)
+			content.ResourceVersion = strconv.Itoa(storedVer + 1)
+		} else {
+			return true, nil, fmt.Errorf("cannot update snapshot content %s: snapshot content not found", action.GetName())
+		}
+
+		// Store the updated object to appropriate places.
+		r.contents[content.Name] = content
+		r.changedObjects = append(r.changedObjects, content)
+		r.changedSinceLastSync++
+		klog.V(4).Infof("saved updated content %s", content.Name)
+		return true, content, nil
+
 	case action.Matches("get", "volumesnapshotcontents"):
 		name := action.(core.GetAction).GetName()
 		content, found := r.contents[name]
@@ -488,6 +530,7 @@ func newSnapshotReactor(kubeClient *kubefake.Clientset, client *fake.Clientset,
 
 	client.AddReactor("create", "volumesnapshotcontents", reactor.React)
 	client.AddReactor("update", "volumesnapshotcontents", reactor.React)
+	client.AddReactor("patch", "volumesnapshotcontents", reactor.React)
 	client.AddReactor("get", "volumesnapshotcontents", reactor.React)
 	client.AddReactor("delete", "volumesnapshotcontents", reactor.React)
 	kubeClient.AddReactor("get", "secrets", reactor.React)

pkg/sidecar-controller/snapshot_controller.go

@@ -146,19 +146,26 @@ func (ctrl *csiSnapshotSideCarController) updateContentErrorStatusWithEvent(cont
 		klog.V(4).Infof("updateContentStatusWithEvent[%s]: the same error %v is already set", content.Name, content.Status.Error)
 		return nil
 	}
-	contentClone := content.DeepCopy()
-	if contentClone.Status == nil {
-		contentClone.Status = &crdv1.VolumeSnapshotContentStatus{}
-	}
-	contentClone.Status.Error = &crdv1.VolumeSnapshotError{
-		Time: &metav1.Time{
-			Time: time.Now(),
-		},
-		Message: &message,
-	}
+
 	ready := false
-	contentClone.Status.ReadyToUse = &ready
-	newContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().UpdateStatus(context.TODO(), contentClone, metav1.UpdateOptions{})
+	patch := []utils.PatchOp{
+		{
+			Op:   "replace",
+			Path: "/status/error",
+			Value: &crdv1.VolumeSnapshotError{
+				Time: &metav1.Time{
+					Time: time.Now(),
+				},
+				Message: &message,
+			},
+		},
+		{
+			Op:    "replace",
+			Path:  "/status/readyToUse",
+			Value: &ready,
+		},
+	}
+	newContent, err := utils.PatchVolumeSnapshotContent(content, patch, ctrl.clientset, "status")
 
 	// Emit the event even if the status update fails so that user can see the error
 	ctrl.eventRecorder.Event(newContent, eventtype, reason, message)
@@ -576,16 +583,28 @@ 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"
 
-	updatedContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{})
+	var patches []utils.PatchOp
+	patches = append(patches, utils.PatchOp{
+		Op:    "replace",
+		Path:  "/metadata/annotations",
+		Value: patchedAnnotations,
+	})
+
+	patchedContent, err := utils.PatchVolumeSnapshotContent(content, patches, ctrl.clientset)
 	if err != nil {
 		return content, newControllerUpdateError(content.Name, err.Error())
 	}
 	// update content if update is successful
-	content = updatedContent
+	content = patchedContent
 
 	_, err = ctrl.storeContentUpdate(content)
 	if err != nil {

pkg/utils/patch.go

@@ -0,0 +1,58 @@
+package utils
+
+import (
+	"context"
+	"encoding/json"
+
+	crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
+	clientset "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+)
+
+// PatchOp represents a json patch operation
+type PatchOp struct {
+	Op    string      `json:"op"`
+	Path  string      `json:"path"`
+	Value interface{} `json:"value,omitempty"`
+}
+
+// PatchVolumeSnapshotContent patches a volume snapshot content object
+func PatchVolumeSnapshotContent(
+	existingSnapshotContent *crdv1.VolumeSnapshotContent,
+	patch []PatchOp,
+	client clientset.Interface,
+	subresources ...string,
+) (*crdv1.VolumeSnapshotContent, error) {
+	data, err := json.Marshal(patch)
+	if nil != err {
+		return existingSnapshotContent, err
+	}
+
+	newSnapshotContent, err := client.SnapshotV1().VolumeSnapshotContents().Patch(context.TODO(), existingSnapshotContent.Name, types.JSONPatchType, data, metav1.PatchOptions{}, subresources...)
+	if err != nil {
+		return existingSnapshotContent, err
+	}
+
+	return newSnapshotContent, nil
+}
+
+// PatchVolumeSnapshot patches a volume snapshot object
+func PatchVolumeSnapshot(
+	existingSnapshot *crdv1.VolumeSnapshot,
+	patch []PatchOp,
+	client clientset.Interface,
+	subresources ...string,
+) (*crdv1.VolumeSnapshot, error) {
+	data, err := json.Marshal(patch)
+	if nil != err {
+		return existingSnapshot, err
+	}
+
+	newSnapshot, err := client.SnapshotV1().VolumeSnapshots(existingSnapshot.Namespace).Patch(context.TODO(), existingSnapshot.Name, types.JSONPatchType, data, metav1.PatchOptions{}, subresources...)
+	if err != nil {
+		return existingSnapshot, err
+	}
+
+	return newSnapshot, nil
+}

vendor/modules.txt

@@ -10,6 +10,7 @@ github.com/container-storage-interface/spec/lib/go/csi
 # github.com/davecgh/go-spew v1.1.1
 github.com/davecgh/go-spew/spew
 # github.com/evanphx/json-patch v4.11.0+incompatible
+## explicit
 github.com/evanphx/json-patch
 # github.com/fsnotify/fsnotify v1.4.9
 ## explicit