WIP

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

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

pkg/common-controller/snapshot_controller.go

@@ -809,6 +809,19 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotErrorStatusWithEvent(snap
 
 // addContentFinalizer adds a Finalizer for VolumeSnapshotContent.
 func (ctrl *csiSnapshotCommonController) addContentFinalizer(content *crdv1.VolumeSnapshotContent) error {
+	// patches := []utils.PatchOp{
+	// 	{
+	// 		Op:    "add",
+	// 		Path:  "/metadata/finalizers/-",
+	// 		Value: utils.VolumeSnapshotContentFinalizer,
+	// 	},
+	// }
+
+	// newContent, err := utils.PatchVolumeSnapshotContent(content, patches, ctrl.clientset)
+	// if err != nil {
+	// 	return newControllerUpdateError(content.Name, err.Error())
+	// }
+
 	contentClone := content.DeepCopy()
 	contentClone.ObjectMeta.Finalizers = append(contentClone.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer)
 
@@ -1392,6 +1405,11 @@ func isControllerUpdateFailError(err *crdv1.VolumeSnapshotError) bool {
 
 // addSnapshotFinalizer adds a Finalizer for VolumeSnapshot.
 func (ctrl *csiSnapshotCommonController) addSnapshotFinalizer(snapshot *crdv1.VolumeSnapshot, addSourceFinalizer bool, addBoundFinalizer bool) error {
+	var updatedSnapshot *crdv1.VolumeSnapshot
+	var err error
+
+	// Must perform an update if no finalizers exist
+	if len(snapshot.ObjectMeta.Finalizers) == 0 {
 		snapshotClone := snapshot.DeepCopy()
 		if addSourceFinalizer {
 			snapshotClone.ObjectMeta.Finalizers = append(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer)
@@ -1399,17 +1417,43 @@ func (ctrl *csiSnapshotCommonController) addSnapshotFinalizer(snapshot *crdv1.Vo
 		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{})
+		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
 
-	_, err = ctrl.storeSnapshotUpdate(newSnapshot)
+		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,
+			})
+		}
+
+		klog.Infof("GGCSI - ADD SNAPSHOT FINALIZER - snapshot: %v", snapshot)
+		klog.Infof("GGCSI - ADD SNAPSHOT FINALIZER - patches: %v", patches)
+		updatedSnapshot, err = utils.PatchVolumeSnapshot(snapshot, patches, ctrl.clientset)
+		if err != nil {
+			return newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error())
+		}
+	}
+
+	_, 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
 }
 

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 {
+	ready := false
-		contentClone.Status = &crdv1.VolumeSnapshotContentStatus{}
+	patch := []utils.PatchOp{
-	}
+		{
-	contentClone.Status.Error = &crdv1.VolumeSnapshotError{
+			Op:   "replace",
+			Path: "/status/error",
+			Value: &crdv1.VolumeSnapshotError{
 				Time: &metav1.Time{
 					Time: time.Now(),
 				},
 				Message: &message,
+			},
+		},
+		{
+			Op:    "replace",
+			Path:  "/status/readyToUse",
+			Value: &ready,
+		},
 	}
-	ready := false
+	newContent, err := utils.PatchVolumeSnapshotContent(content, patch, ctrl.clientset, "status")
-	contentClone.Status.ReadyToUse = &ready
-	newContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().UpdateStatus(context.TODO(), contentClone, metav1.UpdateOptions{})
 
 	// Emit the event even if the status update fails so that user can see the error
 	ctrl.eventRecorder.Event(newContent, eventtype, reason, message)

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
+}