alexlebens/external-snapshotter
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert patch for empty finalizers

Browse Source 
Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>
This commit is contained in:
Grant Griffiths
2021-09-29 19:22:43 -07:00
parent d14e2eea8f
commit ad7dfe6045
3 changed files with 46 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
pkg/common-controller/framework_test.go
View File

@@ -326,9 +326,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, snapshot, nil return true, snapshot, nil
case action.Matches("patch", "volumesnapshots"): case action.Matches("patch", "volumesnapshots"):
snapshot := &crdv1.VolumeSnapshot{}
action := action.(core.PatchAction) action := action.(core.PatchAction)
// Check and bump object version // Check and bump object version
storedSnapshot, found := r.snapshots[action.GetName()] storedSnapshot, found := r.snapshots[action.GetName()]
if found { if found {
@@ -359,9 +357,10 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
} }
// Store the updated object to appropriate places. // Store the updated object to appropriate places.
r.snapshots[snapshot.Name] = storedSnapshot r.snapshots[storedSnapshot.Name] = storedSnapshot
r.changedObjects = append(r.changedObjects, storedSnapshot) r.changedObjects = append(r.changedObjects, storedSnapshot)
r.changedSinceLastSync++ r.changedSinceLastSync++
klog.V(4).Infof("saved updated snapshot %s", storedSnapshot.Name) klog.V(4).Infof("saved updated snapshot %s", storedSnapshot.Name)
return true, storedSnapshot, nil return true, storedSnapshot, nil
@@ -1253,6 +1252,10 @@ func testAddSnapshotFinalizer(ctrl *csiSnapshotCommonController, reactor *snapsh
return ctrl.addSnapshotFinalizer(test.initialSnapshots[0], true, true) 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 { func testRemoveSnapshotFinalizer(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error {
return ctrl.removeSnapshotFinalizer(test.initialSnapshots[0], true, true) return ctrl.removeSnapshotFinalizer(test.initialSnapshots[0], true, true)
} }
@@ -1510,8 +1513,10 @@ func evaluateFinalizerTests(ctrl *csiSnapshotCommonController, reactor *snapshot
if funcName == "testAddSnapshotFinalizer" { if funcName == "testAddSnapshotFinalizer" {
for _, snapshot := range reactor.snapshots { for _, snapshot := range reactor.snapshots {
if test.initialSnapshots[0].Name == snapshot.Name { if test.initialSnapshots[0].Name == snapshot.Name {
if !utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) && utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) && if !utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) &&
!utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) && utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) { 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) klog.V(4).Infof("test %q succeeded. Finalizers are added to snapshot %s", test.name, snapshot.Name)
bHasSnapshotFinalizer = true bHasSnapshotFinalizer = true
} }
@@ -1519,15 +1524,18 @@ func evaluateFinalizerTests(ctrl *csiSnapshotCommonController, reactor *snapshot
} }
} }
if test.expectSuccess && !bHasSnapshotFinalizer { 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 bHasSnapshotFinalizer = true
if funcName == "testRemoveSnapshotFinalizer" { if funcName == "testRemoveSnapshotFinalizer" {
for _, snapshot := range reactor.snapshots { for _, snapshot := range reactor.snapshots {
if test.initialSnapshots[0].Name == snapshot.Name { if test.initialSnapshots[0].Name == snapshot.Name {
if utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) && !utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) && if utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) &&
utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) && !utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer) { !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) klog.V(4).Infof("test %q succeeded. SnapshotFinalizer is removed from Snapshot %s", test.name, snapshot.Name)
bHasSnapshotFinalizer = false bHasSnapshotFinalizer = false
} }

26
pkg/common-controller/snapshot_controller.go
View File

@@ -1421,16 +1421,25 @@ func (ctrl *csiSnapshotCommonController) addSnapshotFinalizer(snapshot *crdv1.Vo
var updatedSnapshot *crdv1.VolumeSnapshot var updatedSnapshot *crdv1.VolumeSnapshot
var err error var err error
var patches []utils.PatchOp // 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 { if len(snapshot.ObjectMeta.Finalizers) == 0 {
// Replace finalizers with new array if there are no other finalizers snapshotClone := snapshot.DeepCopy()
patches = append(patches, utils.PatchOp{ if addSourceFinalizer {
Op: "add", snapshotClone.ObjectMeta.Finalizers = append(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer)
Path: "/metadata/finalizers", }
Value: []string{utils.VolumeSnapshotContentFinalizer}, 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 { } else {
// Otherwise, perform a patch // Otherwise, perform a patch
var patches []utils.PatchOp
// If finalizers exist already, add new ones to the end of the array
if addSourceFinalizer { if addSourceFinalizer {
patches = append(patches, utils.PatchOp{ patches = append(patches, utils.PatchOp{
Op: "add", Op: "add",
@@ -1445,11 +1454,12 @@ func (ctrl *csiSnapshotCommonController) addSnapshotFinalizer(snapshot *crdv1.Vo
Value: utils.VolumeSnapshotBoundFinalizer, Value: utils.VolumeSnapshotBoundFinalizer,
}) })
} }
}
updatedSnapshot, err = utils.PatchVolumeSnapshot(snapshot, patches, ctrl.clientset) updatedSnapshot, err = utils.PatchVolumeSnapshot(snapshot, patches, ctrl.clientset)
if err != nil { if err != nil {
return newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error()) return newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error())
} }
}
_, err = ctrl.storeSnapshotUpdate(updatedSnapshot) _, err = ctrl.storeSnapshotUpdate(updatedSnapshot)
if err != nil { if err != nil {

10
pkg/common-controller/snapshot_finalizer_test.go
View File

@@ -19,6 +19,7 @@ package common_controller
import ( import (
"testing" "testing"
"github.com/kubernetes-csi/external-snapshotter/v4/pkg/utils"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
) )
@@ -70,7 +71,14 @@ func TestSnapshotFinalizer(t *testing.T) {
expectSuccess: true, 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), 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), initialClaims: newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty),
test: testRemoveSnapshotFinalizer, test: testRemoveSnapshotFinalizer,