Skip to content

Commit

Permalink
Use patch instead of update for GroupSnapshots and VolumeSnapshots
Browse files Browse the repository at this point in the history
Signed-off-by: Tiger Kaovilai <[email protected]>
  • Loading branch information
kaovilai committed Feb 22, 2024
1 parent 10c1032 commit a877f42
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 21 deletions.
24 changes: 11 additions & 13 deletions pkg/common-controller/groupsnapshot_controller_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -1023,11 +1022,11 @@ func (ctrl *csiSnapshotCommonController) addGroupSnapshotFinalizer(groupSnapshot
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())
patchedObj, err := utils.PatchAddFinalizers(groupSnapshotClone, ctrl.clientset, utils.VolumeGroupSnapshotBoundFinalizer)
if err != nil {
return newControllerUpdateError(utils.GroupSnapshotKey(groupSnapshot), err.Error())
}
updatedGroupSnapshot = patchedObj.(*crdv1alpha1.VolumeGroupSnapshot)
}
} else {
// Otherwise, perform a patch
Expand Down Expand Up @@ -1114,7 +1113,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
Expand Down Expand Up @@ -1161,7 +1160,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)
Expand Down Expand Up @@ -1224,8 +1223,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())
}
Expand Down
100 changes: 100 additions & 0 deletions pkg/common-controller/groupsnapshot_controller_helper_test.go
Original file line number Diff line number Diff line change
@@ -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))
}

})
}
}
19 changes: 11 additions & 8 deletions pkg/common-controller/snapshot_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1503,13 +1503,15 @@ func (ctrl *csiSnapshotCommonController) addSnapshotFinalizer(snapshot *crdv1.Vo
// Unable to find a patch that correctly updated the finalizers if none currently exist.
if len(snapshot.ObjectMeta.Finalizers) == 0 {
snapshotClone := snapshot.DeepCopy()
finalizersToAdd := []string{}
if addSourceFinalizer {
snapshotClone.ObjectMeta.Finalizers = append(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer)
finalizersToAdd = append(finalizersToAdd, utils.VolumeSnapshotAsSourceFinalizer)
}
if addBoundFinalizer {
snapshotClone.ObjectMeta.Finalizers = append(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer)
finalizersToAdd = append(finalizersToAdd, utils.VolumeSnapshotBoundFinalizer)
}
updatedSnapshot, err = ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{})
newObj, err := utils.PatchAddFinalizers(snapshotClone, ctrl.clientset, )
updatedSnapshot = newObj.(*crdv1.VolumeSnapshot)
if err != nil {
return newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error())
}
Expand Down Expand Up @@ -1570,21 +1572,22 @@ 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{})
newObj, err := utils.PatchRemoveFinalizers(snapshotClone, ctrl.clientset, stringsToRemove...)
if err != nil {
return newControllerUpdateError(snapshot.Name, err.Error())
}
newSnapshot := newObj.(*crdv1.VolumeSnapshot)

_, err = ctrl.storeSnapshotUpdate(newSnapshot)
if err != nil {
Expand Down
88 changes: 88 additions & 0 deletions pkg/utils/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ 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"
Expand Down Expand Up @@ -97,3 +101,87 @@ func PatchVolumeGroupSnapshotContent(

return newGroupSnapshotContent, nil
}

// Remove one or more finalizers from an object
// if finalizers is empty, all finalizers will be removed
// 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) {
patches := []PatchOp{}
if len(finalizers) == 0 {
patches = []PatchOp{
{
Op: "remove",
Path: "/metadata/finalizers",
},
}
} else {
// 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),
})
}
}

data, err := json.Marshal(patches)
if nil != err {
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 PatchAddFinalizers(object metav1.Object, client clientset.Interface, finalizers ...string) (metav1.Object, error) {
patches := []PatchOp{}
for _, finalizer := range finalizers {
patches = append(patches, PatchOp{
Op: "add",
Path: "/metadata/finalizers/-",
Value: finalizer,
})
}

data, err := json.Marshal(patches)
if nil != err {
return nil, err
}
switch object.(type) {
case *crdv1.VolumeSnapshot:
return client.SnapshotV1().VolumeSnapshots(object.GetNamespace()).Patch(context.TODO(), object.GetName(), types.JSONPatchType, data, metav1.PatchOptions{})
case *crdv1alpha1.VolumeGroupSnapshot:
return client.GroupsnapshotV1alpha1().VolumeGroupSnapshots(object.GetNamespace()).Patch(context.TODO(), object.GetName(), types.JSONPatchType, data, metav1.PatchOptions{})
default:
return nil, errors.New("PatchAddFinalizers: unsupported object type")
}
}

0 comments on commit a877f42

Please sign in to comment.