diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go index a79695846..713d78399 100644 --- a/pkg/common-controller/framework_test.go +++ b/pkg/common-controller/framework_test.go @@ -271,6 +271,8 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O // Check and bump object version storedSnapshotContent, found := r.contents[action.GetName()] if found { + // don't modify existing object + storedSnapshotContent = storedSnapshotContent.DeepCopy() // Apply patch storedSnapshotBytes, err := json.Marshal(storedSnapshotContent) if err != nil { @@ -335,6 +337,8 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O // Check and bump object version storedSnapshot, found := r.snapshots[action.GetName()] if found { + // don't modify existing object + storedSnapshot = storedSnapshot.DeepCopy() // Apply patch storedSnapshotBytes, err := json.Marshal(storedSnapshot) if err != nil { @@ -349,7 +353,8 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O if err != nil { return true, nil, err } - + // following unmarshal removes the time millisecond precision which was present in the original object + // make sure time used in tests are in seconds precision err = json.Unmarshal(modified, storedSnapshot) if err != nil { return true, nil, err @@ -1425,7 +1430,6 @@ func runFinalizerTests(t *testing.T, tests []controllerTest, snapshotClasses []* snapshotscheme.AddToScheme(scheme.Scheme) for _, test := range tests { klog.V(4).Infof("starting test %q", test.name) - // Initialize the controller kubeClient := &kubefake.Clientset{} client := &fake.Clientset{} diff --git a/pkg/common-controller/groupsnapshot_controller_helper.go b/pkg/common-controller/groupsnapshot_controller_helper.go index 3768f1547..7d838c97c 100644 --- a/pkg/common-controller/groupsnapshot_controller_helper.go +++ b/pkg/common-controller/groupsnapshot_controller_helper.go @@ -23,9 +23,9 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" ref "k8s.io/client-go/tools/reference" klog "k8s.io/klog/v2" @@ -154,8 +154,7 @@ func (ctrl *csiSnapshotCommonController) SetDefaultGroupSnapshotClass(groupSnaps } klog.V(5).Infof("setDefaultGroupSnapshotClass [%s]: default VolumeGroupSnapshotClassName [%s]", groupSnapshot.Name, defaultClasses[0].Name) groupSnapshotClone := groupSnapshot.DeepCopy() - groupSnapshotClone.Spec.VolumeGroupSnapshotClassName = &(defaultClasses[0].Name) - newGroupSnapshot, err := ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshots(groupSnapshotClone.Namespace).Update(context.TODO(), groupSnapshotClone, metav1.UpdateOptions{}) + newGroupSnapshot, err := ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshots(groupSnapshotClone.Namespace).Patch(context.TODO(), groupSnapshotClone.Name, types.MergePatchType, []byte(fmt.Sprintf(`{"spec":{"volumeGroupSnapshotClassName":"%s"}}`, defaultClasses[0].Name)), metav1.PatchOptions{}) if err != nil { klog.V(4).Infof("updating VolumeGroupSnapshot[%s] default group snapshot class failed %v", utils.GroupSnapshotKey(groupSnapshot), err) } @@ -780,7 +779,7 @@ func (ctrl *csiSnapshotCommonController) createGroupSnapshotContent(groupSnapsho klog.V(5).Infof("volume group snapshot content %#v", groupSnapshotContent) // Try to create the VolumeGroupSnapshotContent object klog.V(5).Infof("createGroupSnapshotContent [%s]: trying to save volume group snapshot content %s", utils.GroupSnapshotKey(groupSnapshot), groupSnapshotContent.Name) - if updateGroupSnapshotContent, err = ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshotContents().Create(context.TODO(), groupSnapshotContent, metav1.CreateOptions{}); err == nil || apierrs.IsAlreadyExists(err) { + if updateGroupSnapshotContent, err = ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshotContents().Create(context.TODO(), groupSnapshotContent, metav1.CreateOptions{}); err == nil || errors.IsAlreadyExists(err) { // Save succeeded. if err != nil { klog.V(3).Infof("volume group snapshot content %q for group snapshot %q already exists, reusing", groupSnapshotContent.Name, utils.GroupSnapshotKey(groupSnapshot)) @@ -1018,33 +1017,19 @@ func (ctrl *csiSnapshotCommonController) addGroupSnapshotFinalizer(groupSnapshot var updatedGroupSnapshot *crdv1alpha1.VolumeGroupSnapshot 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(groupSnapshot.ObjectMeta.Finalizers) == 0 { - groupSnapshotClone := groupSnapshot.DeepCopy() - if addBoundFinalizer { - groupSnapshotClone.ObjectMeta.Finalizers = append(groupSnapshotClone.ObjectMeta.Finalizers, utils.VolumeGroupSnapshotBoundFinalizer) - } - updatedGroupSnapshot, err = ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshots(groupSnapshotClone.Namespace).Update(context.TODO(), groupSnapshotClone, metav1.UpdateOptions{}) - if err != nil { - return newControllerUpdateError(utils.GroupSnapshotKey(groupSnapshot), err.Error()) - } - } else { - // Otherwise, perform a patch - var patches []utils.PatchOp + var patches []utils.PatchOp - if addBoundFinalizer { - patches = append(patches, utils.PatchOp{ - Op: "add", - Path: "/metadata/finalizers/-", - Value: utils.VolumeGroupSnapshotBoundFinalizer, - }) - } + if addBoundFinalizer { + patches = append(patches, utils.PatchOp{ + Op: "add", + Path: "/metadata/finalizers/-", + Value: utils.VolumeGroupSnapshotBoundFinalizer, + }) + } - updatedGroupSnapshot, err = utils.PatchVolumeGroupSnapshot(groupSnapshot, patches, ctrl.clientset) - if err != nil { - return newControllerUpdateError(utils.GroupSnapshotKey(groupSnapshot), err.Error()) - } + updatedGroupSnapshot, err = utils.PatchVolumeGroupSnapshot(groupSnapshot, patches, ctrl.clientset) + if err != nil { + return newControllerUpdateError(utils.GroupSnapshotKey(groupSnapshot), err.Error()) } _, err = ctrl.storeGroupSnapshotUpdate(updatedGroupSnapshot) @@ -1114,7 +1099,7 @@ func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimesta for _, snapshotRef := range groupSnapshot.Status.VolumeSnapshotRefList { snapshot, err := ctrl.snapshotLister.VolumeSnapshots(snapshotRef.Namespace).Get(snapshotRef.Name) if err != nil { - if apierrs.IsNotFound(err) { + if errors.IsNotFound(err) { continue } return err @@ -1161,7 +1146,7 @@ func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimesta // Delete the individual snapshots part of the group snapshot for _, snapshot := range groupSnapshot.Status.VolumeSnapshotRefList { err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Delete(context.TODO(), snapshot.Name, metav1.DeleteOptions{}) - if err != nil && !apierrs.IsNotFound(err) { + if err != nil && !errors.IsNotFound(err) { msg := fmt.Sprintf("failed to delete snapshot API object %s/%s part of group snapshot %s: %v", snapshot.Namespace, snapshot.Name, utils.GroupSnapshotKey(groupSnapshot), err) klog.Error(msg) ctrl.eventRecorder.Event(groupSnapshot, v1.EventTypeWarning, "SnapshotDeleteError", msg) @@ -1224,8 +1209,7 @@ func (ctrl *csiSnapshotCommonController) removeGroupSnapshotFinalizer(groupSnaps // TODO: Remove PVC Finalizer groupSnapshotClone := groupSnapshot.DeepCopy() - groupSnapshotClone.ObjectMeta.Finalizers = utils.RemoveString(groupSnapshotClone.ObjectMeta.Finalizers, utils.VolumeGroupSnapshotBoundFinalizer) - newGroupSnapshot, err := ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshots(groupSnapshotClone.Namespace).Update(context.TODO(), groupSnapshotClone, metav1.UpdateOptions{}) + newGroupSnapshot, err := utils.PatchRemoveFinalizers(groupSnapshotClone, ctrl.clientset, utils.VolumeGroupSnapshotBoundFinalizer) if err != nil { return newControllerUpdateError(groupSnapshot.Name, err.Error()) } diff --git a/pkg/common-controller/groupsnapshot_controller_helper_test.go b/pkg/common-controller/groupsnapshot_controller_helper_test.go new file mode 100644 index 000000000..0274304eb --- /dev/null +++ b/pkg/common-controller/groupsnapshot_controller_helper_test.go @@ -0,0 +1,100 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package common_controller + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + crdv1alpha1 "github.com/kubernetes-csi/external-snapshotter/client/v7/apis/volumegroupsnapshot/v1alpha1" + "github.com/kubernetes-csi/external-snapshotter/client/v7/clientset/versioned/fake" + "github.com/kubernetes-csi/external-snapshotter/v7/pkg/utils" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/cache" + // crdv1alpha1 "github.com/kubernetes-csi/external-snapshotter/client/v7/apis/volumegroupsnapshot/v1alpha1" + // clientset "github.com/kubernetes-csi/external-snapshotter/client/v7/clientset/versioned" + // groupsnapshotlisters "github.com/kubernetes-csi/external-snapshotter/client/v7/listers/volumegroupsnapshot/v1alpha1" + // snapshotlisters "github.com/kubernetes-csi/external-snapshotter/client/v7/listers/volumesnapshot/v1" + // "github.com/kubernetes-csi/external-snapshotter/v2/pkg/metrics" + // "k8s.io/client-go/kubernetes" + // corelisters "k8s.io/client-go/listers/core/v1" + // "k8s.io/client-go/tools/cache" + // "k8s.io/client-go/tools/record" + // "k8s.io/client-go/util/workqueue" +) + +func Test_csiSnapshotCommonController_removeGroupSnapshotFinalizer(t *testing.T) { + type args struct { + groupSnapshot *crdv1alpha1.VolumeGroupSnapshot + removeBoundFinalizer bool + } + tests := []struct { + name string + args args + wantErr bool + expectedFinalizers []string + }{ + { + name: "Test removeGroupSnapshotFinalizer", + args: args{ + removeBoundFinalizer: true, + groupSnapshot: &crdv1alpha1.VolumeGroupSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-group-snapshot", + Finalizers: []string{utils.VolumeGroupSnapshotBoundFinalizer}, + }, + }, + }, + }, + { + name: "Test removeGroupSnapshotFinalizer and not something else", + args: args{ + removeBoundFinalizer: true, + groupSnapshot: &crdv1alpha1.VolumeGroupSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-group-snapshot", + Finalizers: []string{"somethingElse", utils.VolumeGroupSnapshotBoundFinalizer}, + }, + }, + }, + expectedFinalizers: []string{"somethingElse"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := &csiSnapshotCommonController{ + clientset: fake.NewSimpleClientset(tt.args.groupSnapshot), + groupSnapshotStore: cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc), + } + if err := ctrl.removeGroupSnapshotFinalizer(tt.args.groupSnapshot, tt.args.removeBoundFinalizer); (err != nil) != tt.wantErr { + t.Errorf("csiSnapshotCommonController.removeGroupSnapshotFinalizer() error = %v, wantErr %v", err, tt.wantErr) + } + vgs, err := ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshots(tt.args.groupSnapshot.Namespace).Get(context.TODO(), tt.args.groupSnapshot.Name, metav1.GetOptions{}) + if err != nil { + t.Errorf("Error getting volume group snapshot: %v", err) + } + if tt.expectedFinalizers == nil && vgs.Finalizers != nil { + tt.expectedFinalizers = []string{} // if expectedFinalizers is nil, then it should be an empty slice so that cmp.Diff does not panic + } + if vgs.Finalizers != nil && cmp.Diff(vgs.Finalizers, tt.expectedFinalizers) != "" { + t.Errorf("Finalizers not expected: %v", cmp.Diff(vgs.Finalizers, tt.expectedFinalizers)) + } + + }) + } +} diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index ae76461c6..196c47dd0 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/pkg/common-controller/snapshot_controller.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" ref "k8s.io/client-go/tools/reference" corev1helpers "k8s.io/component-helpers/scheduling/corev1" @@ -932,8 +933,11 @@ func (ctrl *csiSnapshotCommonController) ensurePVCFinalizer(snapshot *crdv1.Volu } else { // If PVC is not being deleted and PVCFinalizer is not added yet, add the PVCFinalizer. pvcClone := pvc.DeepCopy() - pvcClone.ObjectMeta.Finalizers = append(pvcClone.ObjectMeta.Finalizers, utils.PVCFinalizer) - _, err = ctrl.client.CoreV1().PersistentVolumeClaims(pvcClone.Namespace).Update(context.TODO(), pvcClone, metav1.UpdateOptions{}) + patchBytes, err := utils.PatchOpsBytesToAddFinalizers(pvcClone, utils.PVCFinalizer) + if err != nil { + return newControllerUpdateError(pvcClone.Name, err.Error()) + } + _, err = ctrl.client.CoreV1().PersistentVolumeClaims(pvcClone.Namespace).Patch(context.TODO(), pvcClone.Name, types.JSONPatchType, patchBytes, metav1.PatchOptions{}) if err != nil { klog.Errorf("cannot add finalizer on claim [%s/%s] for snapshot [%s/%s]: [%v]", pvc.Namespace, pvc.Name, snapshot.Namespace, snapshot.Name, err) return newControllerUpdateError(pvcClone.Name, err.Error()) @@ -950,9 +954,11 @@ func (ctrl *csiSnapshotCommonController) removePVCFinalizer(pvc *v1.PersistentVo // TODO(xyang): We get PVC from informer but it may be outdated // Should get it from API server directly before removing finalizer pvcClone := pvc.DeepCopy() - pvcClone.ObjectMeta.Finalizers = utils.RemoveString(pvcClone.ObjectMeta.Finalizers, utils.PVCFinalizer) - - _, err := ctrl.client.CoreV1().PersistentVolumeClaims(pvcClone.Namespace).Update(context.TODO(), pvcClone, metav1.UpdateOptions{}) + patchBytes, err := utils.PatchOpsBytesToRemoveFinalizers(pvcClone, utils.PVCFinalizer) + if err != nil { + return newControllerUpdateError(pvcClone.Name, err.Error()) + } + _, err = ctrl.client.CoreV1().PersistentVolumeClaims(pvcClone.Namespace).Patch(context.TODO(), pvcClone.Name, types.JSONPatchType, patchBytes, metav1.PatchOptions{}) if err != nil { return newControllerUpdateError(pvcClone.Name, err.Error()) } @@ -1499,44 +1505,27 @@ func (ctrl *csiSnapshotCommonController) addSnapshotFinalizer(snapshot *crdv1.Vo 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 + 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, - }) - } + // 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()) - } + updatedSnapshot, err = utils.PatchVolumeSnapshot(snapshot, patches, ctrl.clientset) + if err != nil { + return newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error()) } _, err = ctrl.storeSnapshotUpdate(updatedSnapshot) @@ -1570,22 +1559,29 @@ func (ctrl *csiSnapshotCommonController) removeSnapshotFinalizer(snapshot *crdv1 ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "ErrorPVCFinalizer", "Error check and remove PVC Finalizer for VolumeSnapshot") return newControllerUpdateError(snapshot.Name, err.Error()) } - snapshotClone := snapshot.DeepCopy() + stringsToRemove := []string{} if removeSourceFinalizer { - snapshotClone.ObjectMeta.Finalizers = utils.RemoveString(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) + stringsToRemove = append(stringsToRemove, utils.VolumeSnapshotAsSourceFinalizer) } if removeBoundFinalizer { - snapshotClone.ObjectMeta.Finalizers = utils.RemoveString(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) + stringsToRemove = append(stringsToRemove, utils.VolumeSnapshotBoundFinalizer) } if removeGroupFinalizer { - snapshotClone.ObjectMeta.Finalizers = utils.RemoveString(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotInGroupFinalizer) + stringsToRemove = append(stringsToRemove, utils.VolumeSnapshotInGroupFinalizer) } - newSnapshot, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{}) + patchBytes, err := utils.PatchOpsBytesToRemoveFinalizers(snapshot, stringsToRemove...) if err != nil { return newControllerUpdateError(snapshot.Name, err.Error()) } - + newSnapshot, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).Patch(context.TODO(), snapshotClone.Name, types.JSONPatchType, patchBytes, metav1.PatchOptions{}) + if err != nil { + return newControllerUpdateError(snapshot.Name, err.Error()) + } + if len(newSnapshot.Finalizers) == 0 { + // some tests require 0 length finalizers to be nil + newSnapshot.Finalizers = nil + } _, err = ctrl.storeSnapshotUpdate(newSnapshot) if err != nil { klog.Errorf("failed to update snapshot store %v", err) diff --git a/pkg/common-controller/snapshot_delete_test.go b/pkg/common-controller/snapshot_delete_test.go index 299bf4f07..950d052fa 100644 --- a/pkg/common-controller/snapshot_delete_test.go +++ b/pkg/common-controller/snapshot_delete_test.go @@ -19,6 +19,7 @@ package common_controller import ( "errors" "testing" + "time" crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v7/apis/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v7/pkg/utils" @@ -49,7 +50,12 @@ var class5Parameters = map[string]string{ utils.PrefixedSnapshotterSecretNamespaceKey: "default", } -var timeNowMetav1 = metav1.Now() +var timeNowMetav1 = func() metav1.Time { + // json.unmarshal does not restore millisecond precision + // so we need to round the time to seconds + timeNow := timeNow.Round(time.Second) + return metav1.NewTime(timeNow) +}() var ( content31 = "content3-1" diff --git a/pkg/utils/patch.go b/pkg/utils/patch.go index 05fd32f6b..6247c03fe 100644 --- a/pkg/utils/patch.go +++ b/pkg/utils/patch.go @@ -3,12 +3,17 @@ package utils import ( "context" "encoding/json" + "errors" + "fmt" + "slices" + crdv1alpha1 "github.com/kubernetes-csi/external-snapshotter/client/v7/apis/volumegroupsnapshot/v1alpha1" crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v7/apis/volumesnapshot/v1" clientset "github.com/kubernetes-csi/external-snapshotter/client/v7/clientset/versioned" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + kubernetes "k8s.io/client-go/kubernetes" ) // PatchOp represents a json patch operation @@ -97,3 +102,94 @@ func PatchVolumeGroupSnapshotContent( return newGroupSnapshotContent, nil } + +// Remove one or more finalizers from an object +// if finalizers is not empty, only the specified finalizers will be removed +func PatchRemoveFinalizers(object metav1.Object, client clientset.Interface, finalizers ...string) (metav1.Object, error) { + data, err := PatchOpsBytesToRemoveFinalizers(object, finalizers...) + if err != nil { + return nil, err + } + switch object.(type) { + case *crdv1.VolumeSnapshot: + obj, err := client.SnapshotV1().VolumeSnapshots(object.GetNamespace()).Patch(context.TODO(), object.GetName(), types.JSONPatchType, data, metav1.PatchOptions{}) + if len(obj.Finalizers) == 0 { + // to satisfy some tests that requires nil rather than []string{} + obj.Finalizers = nil + } + return obj, err + case *crdv1alpha1.VolumeGroupSnapshot: + obj, err := client.GroupsnapshotV1alpha1().VolumeGroupSnapshots(object.GetNamespace()).Patch(context.TODO(), object.GetName(), types.JSONPatchType, data, metav1.PatchOptions{}) + if len(obj.Finalizers) == 0 { + // to satisfy some tests that requires nil rather than []string{} + obj.Finalizers = nil + } + return obj, err + default: + return nil, errors.New("PatchRemoveFinalizers: unsupported object type") + } +} + +func PatchRemoveFinalizersCorev1(object metav1.Object, client kubernetes.Interface, finalizers ...string) (metav1.Object, error) { + data, err := PatchOpsBytesToRemoveFinalizers(object, finalizers...) + if err != nil { + return nil, err + } + obj, err := client.CoreV1().Pods(object.GetNamespace()).Patch(context.TODO(), object.GetName(), types.JSONPatchType, data, metav1.PatchOptions{}) + if len(obj.Finalizers) == 0 { + // to satisfy some tests that requires nil rather than []string{} + obj.Finalizers = nil + } + return obj, err +} + +func PatchOpsToRemoveFinalizers(object metav1.Object, finalizers ...string) []PatchOp { + patches := []PatchOp{} + if len(finalizers) == 0 { + return patches + } + // remove only the specified finalizers + // get index of all finalizers to be removed + // sort the indexes in descending order + // remove the finalizers in descending order + // this is to avoid the need to adjust the index after each removal + indexes := []int{} + for _, finalizer := range finalizers { + for i, f := range object.GetFinalizers() { + if f == finalizer { + indexes = append(indexes, i) + } + } + } + slices.Sort(indexes) + slices.Reverse(indexes) + for _, i := range indexes { + patches = append(patches, PatchOp{ + Op: "remove", + Path: "/metadata/finalizers/" + fmt.Sprint(i), + }) + } + return patches +} + +func PatchOpsBytesToRemoveFinalizers(object metav1.Object, finalizers ...string) ([]byte, error) { + patches := PatchOpsToRemoveFinalizers(object, finalizers...) + return json.Marshal(patches) +} + +func PatchOpsToAddFinalizers(object metav1.Object, finalizers ...string) []PatchOp { + patches := []PatchOp{} + for _, finalizer := range finalizers { + patches = append(patches, PatchOp{ + Op: "add", + Path: "/metadata/finalizers/-", + Value: finalizer, + }) + } + return patches +} + +func PatchOpsBytesToAddFinalizers(object metav1.Object, finalizers ...string) ([]byte, error) { + patches := PatchOpsToAddFinalizers(object, finalizers...) + return json.Marshal(patches) +} diff --git a/pkg/utils/patch_test.go b/pkg/utils/patch_test.go new file mode 100644 index 000000000..e0c0c889c --- /dev/null +++ b/pkg/utils/patch_test.go @@ -0,0 +1,83 @@ +package utils + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/fake" +) + +func TestPatchOpsBytesToRemoveFinalizers(t *testing.T) { + type args struct { + object *corev1.PersistentVolumeClaim + finalizersToRemove []string + } + tests := []struct { + name string + args args + finalizersAfterPatch []string + wantErr bool + }{ + { + name: "remove all finalizers", + args: args{ + object: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pvc1", + Namespace: "default", + Finalizers: []string{"finalizer1", "finalizer2"}, + }, + }, + finalizersToRemove: []string{"finalizer1", "finalizer2"}, + }, + finalizersAfterPatch: []string{}, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := fake.NewSimpleClientset(tt.args.object) + gotObj, err := client.CoreV1().PersistentVolumeClaims(tt.args.object.GetNamespace()).Get(context.Background(), tt.args.object.GetName(), metav1.GetOptions{}) + if err != nil { + t.Errorf("PatchOpsBytesToRemoveFinalizers() error = %v, wantErr %v", err, tt.wantErr) + return + } + for i, finalizer := range gotObj.GetFinalizers() { + if finalizer != tt.args.object.Finalizers[i] { + t.Errorf("PatchOpsBytesToRemoveFinalizers() error = %v, wantErr %v", err, tt.wantErr) + return + } + } + got, err := PatchOpsBytesToRemoveFinalizers(tt.args.object, tt.args.finalizersToRemove...) + if (err != nil) != tt.wantErr { + t.Errorf("PatchOpsBytesToRemoveFinalizers() error = %v, wantErr %v", err, tt.wantErr) + return + } + // try to apply the patch + _, err = client.CoreV1().PersistentVolumeClaims(tt.args.object.GetNamespace()).Patch(context.Background(), tt.args.object.GetName(), types.JSONPatchType, got, metav1.PatchOptions{}) + if err != nil { + t.Errorf("PatchOpsBytesToRemoveFinalizers() error = %v, wantErr %v", err, tt.wantErr) + return + } + // check if the finalizers were removed + gotObj, err = client.CoreV1().PersistentVolumeClaims(tt.args.object.GetNamespace()).Get(context.Background(), tt.args.object.GetName(), metav1.GetOptions{}) + if err != nil { + t.Errorf("PatchOpsBytesToRemoveFinalizers() error = %v, wantErr %v", err, tt.wantErr) + return + } + if len(gotObj.GetFinalizers()) != len(tt.finalizersAfterPatch) { + t.Errorf("PatchOpsBytesToRemoveFinalizers() error = %v, wantErr %v", err, tt.wantErr) + return + } + for i, finalizer := range gotObj.GetFinalizers() { + if finalizer != tt.finalizersAfterPatch[i] { + t.Errorf("PatchOpsBytesToRemoveFinalizers() error = %v, wantErr %v", err, tt.wantErr) + return + } + } + }) + } +}