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

chore: remove unused parameter from util funcs; add unit test

Browse Source 
Added the unit tests from
https://github.com/kubernetes/kubernetes/blob/master/pkg/util/slice/slice_test.go
(minus modifier tests) so we can be sure to not change behaviour.
This commit is contained in:
Max Jonas Werner
2020-07-22 09:30:23 +02:00
parent 1f13709413
commit 04848c755a
5 changed files with 73 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1320,7 +1320,7 @@ func evaluateFinalizerTests(ctrl *csiSnapshotCommonController, reactor *snapshot
if funcName == "testAddPVCFinalizer" { if funcName == "testAddPVCFinalizer" {
for _, pvc := range reactor.claims { for _, pvc := range reactor.claims {
if test.initialClaims[0].Name == pvc.Name { if test.initialClaims[0].Name == pvc.Name {
if !utils.ContainsString(test.initialClaims[0].ObjectMeta.Finalizers, utils.PVCFinalizer, nil) && utils.ContainsString(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer, nil) { if !utils.ContainsString(test.initialClaims[0].ObjectMeta.Finalizers, utils.PVCFinalizer) && utils.ContainsString(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer) {
klog.V(4).Infof("test %q succeeded. PVCFinalizer is added to PVC %s", test.name, pvc.Name) klog.V(4).Infof("test %q succeeded. PVCFinalizer is added to PVC %s", test.name, pvc.Name)
bHasPVCFinalizer = true bHasPVCFinalizer = true
} }
@@ -1335,7 +1335,7 @@ func evaluateFinalizerTests(ctrl *csiSnapshotCommonController, reactor *snapshot
if funcName == "testRemovePVCFinalizer" { if funcName == "testRemovePVCFinalizer" {
for _, pvc := range reactor.claims { for _, pvc := range reactor.claims {
if test.initialClaims[0].Name == pvc.Name { if test.initialClaims[0].Name == pvc.Name {
if utils.ContainsString(test.initialClaims[0].ObjectMeta.Finalizers, utils.PVCFinalizer, nil) && !utils.ContainsString(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer, nil) { if utils.ContainsString(test.initialClaims[0].ObjectMeta.Finalizers, utils.PVCFinalizer) && !utils.ContainsString(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer) {
klog.V(4).Infof("test %q succeeded. PVCFinalizer is removed from PVC %s", test.name, pvc.Name) klog.V(4).Infof("test %q succeeded. PVCFinalizer is removed from PVC %s", test.name, pvc.Name)
bHasPVCFinalizer = false bHasPVCFinalizer = false
} }
@@ -1350,8 +1350,8 @@ 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, nil) && utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer, nil) && if !utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) && utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) &&
!utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer, nil) && utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer, nil) { !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
} }
@@ -1366,8 +1366,8 @@ func evaluateFinalizerTests(ctrl *csiSnapshotCommonController, reactor *snapshot
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, nil) && !utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer, nil) && if utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) && !utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) &&
utils.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer, nil) && !utils.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer, nil) { 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
} }

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

@@ -222,7 +222,7 @@ func (ctrl *csiSnapshotCommonController) isPVCwithFinalizerInUseByCurrentSnapsho
} }
// Check if there is a Finalizer on PVC. If not, return false // Check if there is a Finalizer on PVC. If not, return false
if !utils.ContainsString(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer, nil) { if !utils.ContainsString(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer) {
return false return false
} }
@@ -820,7 +820,7 @@ func (ctrl *csiSnapshotCommonController) ensurePVCFinalizer(snapshot *crdv1.Volu
} }
// If PVC is not being deleted and PVCFinalizer is not added yet, the PVCFinalizer should be added. // If PVC is not being deleted and PVCFinalizer is not added yet, the PVCFinalizer should be added.
if pvc.ObjectMeta.DeletionTimestamp == nil && !utils.ContainsString(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer, nil) { if pvc.ObjectMeta.DeletionTimestamp == nil && !utils.ContainsString(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer) {
// Add the finalizer // Add the finalizer
pvcClone := pvc.DeepCopy() pvcClone := pvc.DeepCopy()
pvcClone.ObjectMeta.Finalizers = append(pvcClone.ObjectMeta.Finalizers, utils.PVCFinalizer) pvcClone.ObjectMeta.Finalizers = append(pvcClone.ObjectMeta.Finalizers, utils.PVCFinalizer)
@@ -841,7 +841,7 @@ func (ctrl *csiSnapshotCommonController) removePVCFinalizer(pvc *v1.PersistentVo
// TODO(xyang): We get PVC from informer but it may be outdated // TODO(xyang): We get PVC from informer but it may be outdated
// Should get it from API server directly before removing finalizer // Should get it from API server directly before removing finalizer
pvcClone := pvc.DeepCopy() pvcClone := pvc.DeepCopy()
pvcClone.ObjectMeta.Finalizers = utils.RemoveString(pvcClone.ObjectMeta.Finalizers, utils.PVCFinalizer, nil) pvcClone.ObjectMeta.Finalizers = utils.RemoveString(pvcClone.ObjectMeta.Finalizers, utils.PVCFinalizer)
_, err := ctrl.client.CoreV1().PersistentVolumeClaims(pvcClone.Namespace).Update(context.TODO(), pvcClone, metav1.UpdateOptions{}) _, err := ctrl.client.CoreV1().PersistentVolumeClaims(pvcClone.Namespace).Update(context.TODO(), pvcClone, metav1.UpdateOptions{})
if err != nil { if err != nil {
@@ -897,7 +897,7 @@ func (ctrl *csiSnapshotCommonController) checkandRemovePVCFinalizer(snapshot *cr
klog.V(5).Infof("checkandRemovePVCFinalizer for snapshot [%s]: snapshot status [%#v]", snapshot.Name, snapshot.Status) klog.V(5).Infof("checkandRemovePVCFinalizer for snapshot [%s]: snapshot status [%#v]", snapshot.Name, snapshot.Status)
// Check if there is a Finalizer on PVC to be removed // Check if there is a Finalizer on PVC to be removed
if utils.ContainsString(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer, nil) { if utils.ContainsString(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer) {
// There is a Finalizer on PVC. Check if PVC is used // There is a Finalizer on PVC. Check if PVC is used
// and remove finalizer if it's not used. // and remove finalizer if it's not used.
inUse := ctrl.isPVCBeingUsed(pvc, snapshot) inUse := ctrl.isPVCBeingUsed(pvc, snapshot)
@@ -1294,10 +1294,10 @@ func (ctrl *csiSnapshotCommonController) removeSnapshotFinalizer(snapshot *crdv1
snapshotClone := snapshot.DeepCopy() snapshotClone := snapshot.DeepCopy()
if removeSourceFinalizer { if removeSourceFinalizer {
snapshotClone.ObjectMeta.Finalizers = utils.RemoveString(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer, nil) snapshotClone.ObjectMeta.Finalizers = utils.RemoveString(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer)
} }
if removeBoundFinalizer { if removeBoundFinalizer {
snapshotClone.ObjectMeta.Finalizers = utils.RemoveString(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer, nil) snapshotClone.ObjectMeta.Finalizers = utils.RemoveString(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer)
} }
_, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{}) _, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{})
if err != nil { if err != nil {

4
pkg/sidecar-controller/snapshot_controller.go
View File

@@ -505,12 +505,12 @@ func (ctrl *csiSnapshotSideCarController) GetCredentialsFromAnnotation(content *
// removeContentFinalizer removes the VolumeSnapshotContentFinalizer from a // removeContentFinalizer removes the VolumeSnapshotContentFinalizer from a
// content if there exists one. // content if there exists one.
func (ctrl csiSnapshotSideCarController) removeContentFinalizer(content *crdv1.VolumeSnapshotContent) error { func (ctrl csiSnapshotSideCarController) removeContentFinalizer(content *crdv1.VolumeSnapshotContent) error {
if !utils.ContainsString(content.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer, nil) { if !utils.ContainsString(content.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer) {
// the finalizer does not exit, return directly // the finalizer does not exit, return directly
return nil return nil
} }
contentClone := content.DeepCopy() contentClone := content.DeepCopy()
contentClone.ObjectMeta.Finalizers = utils.RemoveString(contentClone.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer, nil) contentClone.ObjectMeta.Finalizers = utils.RemoveString(contentClone.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer)
_, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) _, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{})
if err != nil { if err != nil {

21
pkg/utils/util.go
View File

@@ -109,30 +109,23 @@ var SnapshotterListSecretParams = secretParamsMap{
} }
// ContainsString checks if a given slice of strings contains the provided string. // ContainsString checks if a given slice of strings contains the provided string.
// If a modifier func is provided, it is called with the slice item before the comparation. func ContainsString(slice []string, s string) bool {
func ContainsString(slice []string, s string, modifier func(s string) string) bool {
for _, item := range slice { for _, item := range slice {
if item == s { if item == s {
return true return true
} }
if modifier != nil && modifier(item) == s {
return true
}
} }
return false return false
} }
// RemoveString returns a newly created []string that contains all items from slice that // RemoveString returns a newly created []string that contains all items from slice that
// are not equal to s and modifier(s) in case modifier func is provided. // are not equal to s.
func RemoveString(slice []string, s string, modifier func(s string) string) []string { func RemoveString(slice []string, s string) []string {
newSlice := make([]string, 0) newSlice := make([]string, 0)
for _, item := range slice { for _, item := range slice {
if item == s { if item == s {
continue continue
} }
if modifier != nil && modifier(item) == s {
continue
}
newSlice = append(newSlice, item) newSlice = append(newSlice, item)
} }
if len(newSlice) == 0 { if len(newSlice) == 0 {
@@ -371,23 +364,23 @@ func NoResyncPeriodFunc() time.Duration {
// NeedToAddContentFinalizer checks if a Finalizer needs to be added for the volume snapshot content. // NeedToAddContentFinalizer checks if a Finalizer needs to be added for the volume snapshot content.
func NeedToAddContentFinalizer(content *crdv1.VolumeSnapshotContent) bool { func NeedToAddContentFinalizer(content *crdv1.VolumeSnapshotContent) bool {
return content.ObjectMeta.DeletionTimestamp == nil && !ContainsString(content.ObjectMeta.Finalizers, VolumeSnapshotContentFinalizer, nil) return content.ObjectMeta.DeletionTimestamp == nil && !ContainsString(content.ObjectMeta.Finalizers, VolumeSnapshotContentFinalizer)
} }
// IsSnapshotDeletionCandidate checks if a volume snapshot deletionTimestamp // IsSnapshotDeletionCandidate checks if a volume snapshot deletionTimestamp
// is set and any finalizer is on the snapshot. // is set and any finalizer is on the snapshot.
func IsSnapshotDeletionCandidate(snapshot *crdv1.VolumeSnapshot) bool { func IsSnapshotDeletionCandidate(snapshot *crdv1.VolumeSnapshot) bool {
return snapshot.ObjectMeta.DeletionTimestamp != nil && (ContainsString(snapshot.ObjectMeta.Finalizers, VolumeSnapshotAsSourceFinalizer, nil) || ContainsString(snapshot.ObjectMeta.Finalizers, VolumeSnapshotBoundFinalizer, nil)) return snapshot.ObjectMeta.DeletionTimestamp != nil && (ContainsString(snapshot.ObjectMeta.Finalizers, VolumeSnapshotAsSourceFinalizer) || ContainsString(snapshot.ObjectMeta.Finalizers, VolumeSnapshotBoundFinalizer))
} }
// NeedToAddSnapshotAsSourceFinalizer checks if a Finalizer needs to be added for the volume snapshot as a source for PVC. // NeedToAddSnapshotAsSourceFinalizer checks if a Finalizer needs to be added for the volume snapshot as a source for PVC.
func NeedToAddSnapshotAsSourceFinalizer(snapshot *crdv1.VolumeSnapshot) bool { func NeedToAddSnapshotAsSourceFinalizer(snapshot *crdv1.VolumeSnapshot) bool {
return snapshot.ObjectMeta.DeletionTimestamp == nil && !ContainsString(snapshot.ObjectMeta.Finalizers, VolumeSnapshotAsSourceFinalizer, nil) return snapshot.ObjectMeta.DeletionTimestamp == nil && !ContainsString(snapshot.ObjectMeta.Finalizers, VolumeSnapshotAsSourceFinalizer)
} }
// NeedToAddSnapshotBoundFinalizer checks if a Finalizer needs to be added for the bound volume snapshot. // NeedToAddSnapshotBoundFinalizer checks if a Finalizer needs to be added for the bound volume snapshot.
func NeedToAddSnapshotBoundFinalizer(snapshot *crdv1.VolumeSnapshot) bool { func NeedToAddSnapshotBoundFinalizer(snapshot *crdv1.VolumeSnapshot) bool {
return snapshot.ObjectMeta.DeletionTimestamp == nil && !ContainsString(snapshot.ObjectMeta.Finalizers, VolumeSnapshotBoundFinalizer, nil) && IsBoundVolumeSnapshotContentNameSet(snapshot) return snapshot.ObjectMeta.DeletionTimestamp == nil && !ContainsString(snapshot.ObjectMeta.Finalizers, VolumeSnapshotBoundFinalizer) && IsBoundVolumeSnapshotContentNameSet(snapshot)
} }
func deprecationWarning(deprecatedParam, newParam, removalVersion string) string { func deprecationWarning(deprecatedParam, newParam, removalVersion string) string {

52
pkg/utils/util_test.go
View File

@@ -25,6 +25,58 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
) )
func TestContainsString(t *testing.T) {
src := []string{"aa", "bb", "cc"}
if !ContainsString(src, "bb") {
t.Errorf("ContainsString didn't find the string as expected")
}
}
func TestRemoveString(t *testing.T) {
tests := []struct {
testName string
input []string
remove string
want []string
}{
{
testName: "Nil input slice",
input: nil,
remove: "",
want: nil,
},
{
testName: "Slice doesn't contain the string",
input: []string{"a", "ab", "cdef"},
remove: "NotPresentInSlice",
want: []string{"a", "ab", "cdef"},
},
{
testName: "All strings removed, result is nil",
input: []string{"a"},
remove: "a",
want: nil,
},
{
testName: "One string removed",
input: []string{"a", "ab", "cdef"},
remove: "ab",
want: []string{"a", "cdef"},
},
{
testName: "All(three) strings removed",
input: []string{"ab", "a", "ab", "cdef", "ab"},
remove: "ab",
want: []string{"a", "cdef"},
},
}
for _, tt := range tests {
if got := RemoveString(tt.input, tt.remove); !reflect.DeepEqual(got, tt.want) {
t.Errorf("%v: RemoveString(%v, %q) = %v WANT %v", tt.testName, tt.input, tt.remove, got, tt.want)
}
}
}
func TestGetSecretReference(t *testing.T) { func TestGetSecretReference(t *testing.T) {
testcases := map[string]struct { testcases := map[string]struct {
secretParams secretParamsMap secretParams secretParamsMap