diff --git a/deploy/kubernetes/snapshot-controller/rbac-snapshot-controller.yaml b/deploy/kubernetes/snapshot-controller/rbac-snapshot-controller.yaml index a6b8c7a9..9fb7e9f3 100644 --- a/deploy/kubernetes/snapshot-controller/rbac-snapshot-controller.yaml +++ b/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 diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index 48786f84..4a18a57d 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/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,24 +1405,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 + + // 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) + } + 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 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(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 } diff --git a/pkg/sidecar-controller/snapshot_controller.go b/pkg/sidecar-controller/snapshot_controller.go index ec0c6deb..1f930472 100644 --- a/pkg/sidecar-controller/snapshot_controller.go +++ b/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) diff --git a/pkg/utils/patch.go b/pkg/utils/patch.go new file mode 100644 index 00000000..abd77a3d --- /dev/null +++ b/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 +}