diff --git a/pkg/controller/volume/scheduling/scheduler_binder_test.go b/pkg/controller/volume/scheduling/scheduler_binder_test.go index 457a5cd0c80..7ca5082e92c 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder_test.go +++ b/pkg/controller/volume/scheduling/scheduler_binder_test.go @@ -113,6 +113,10 @@ var ( zone1Labels = map[string]string{v1.LabelZoneFailureDomain: "us-east-1", v1.LabelZoneRegion: "us-east-1a"} ) +func init() { + klog.InitFlags(nil) +} + type testEnv struct { client clientset.Interface reactor *pvtesting.VolumeReactor @@ -353,18 +357,18 @@ func (env *testEnv) deleteClaims(pvcs []*v1.PersistentVolumeClaim) { } } -func (env *testEnv) assumeVolumes(t *testing.T, name, node string, pod *v1.Pod, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) { +func (env *testEnv) assumeVolumes(t *testing.T, node string, pod *v1.Pod, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) { pvCache := env.internalBinder.pvCache for _, binding := range bindings { if err := pvCache.Assume(binding.pv); err != nil { - t.Fatalf("Failed to setup test %q: error: %v", name, err) + t.Fatalf("error: %v", err) } } pvcCache := env.internalBinder.pvcCache for _, pvc := range provisionings { if err := pvcCache.Assume(pvc); err != nil { - t.Fatalf("Failed to setup test %q: error: %v", name, err) + t.Fatalf("error: %v", err) } } @@ -376,72 +380,72 @@ func (env *testEnv) initPodCache(pod *v1.Pod, node string, bindings []*bindingIn cache.UpdateBindings(pod, node, bindings, provisionings) } -func (env *testEnv) validatePodCache(t *testing.T, name, node string, pod *v1.Pod, expectedBindings []*bindingInfo, expectedProvisionings []*v1.PersistentVolumeClaim) { +func (env *testEnv) validatePodCache(t *testing.T, node string, pod *v1.Pod, expectedBindings []*bindingInfo, expectedProvisionings []*v1.PersistentVolumeClaim) { cache := env.internalBinder.podBindingCache bindings := cache.GetBindings(pod, node) if aLen, eLen := len(bindings), len(expectedBindings); aLen != eLen { - t.Errorf("Test %q failed. expected %v bindings, got %v", name, eLen, aLen) + t.Errorf("expected %v bindings, got %v", eLen, aLen) } else if expectedBindings == nil && bindings != nil { // nil and empty are different - t.Errorf("Test %q failed. expected nil bindings, got empty", name) + t.Error("expected nil bindings, got empty") } else if expectedBindings != nil && bindings == nil { // nil and empty are different - t.Errorf("Test %q failed. expected empty bindings, got nil", name) + t.Error("expected empty bindings, got nil") } else { for i := 0; i < aLen; i++ { // Validate PV if !reflect.DeepEqual(expectedBindings[i].pv, bindings[i].pv) { - t.Errorf("Test %q failed. binding.pv doesn't match [A-expected, B-got]: %s", name, diff.ObjectDiff(expectedBindings[i].pv, bindings[i].pv)) + t.Errorf("binding.pv doesn't match [A-expected, B-got]: %s", diff.ObjectDiff(expectedBindings[i].pv, bindings[i].pv)) } // Validate PVC if !reflect.DeepEqual(expectedBindings[i].pvc, bindings[i].pvc) { - t.Errorf("Test %q failed. binding.pvc doesn't match [A-expected, B-got]: %s", name, diff.ObjectDiff(expectedBindings[i].pvc, bindings[i].pvc)) + t.Errorf("binding.pvc doesn't match [A-expected, B-got]: %s", diff.ObjectDiff(expectedBindings[i].pvc, bindings[i].pvc)) } } } provisionedClaims := cache.GetProvisionedPVCs(pod, node) if aLen, eLen := len(provisionedClaims), len(expectedProvisionings); aLen != eLen { - t.Errorf("Test %q failed. expected %v provisioned claims, got %v", name, eLen, aLen) + t.Errorf("expected %v provisioned claims, got %v", eLen, aLen) } else if expectedProvisionings == nil && provisionedClaims != nil { // nil and empty are different - t.Errorf("Test %q failed. expected nil provisionings, got empty", name) + t.Error("expected nil provisionings, got empty") } else if expectedProvisionings != nil && provisionedClaims == nil { // nil and empty are different - t.Errorf("Test %q failed. expected empty provisionings, got nil", name) + t.Error("expected empty provisionings, got nil") } else { for i := 0; i < aLen; i++ { if !reflect.DeepEqual(expectedProvisionings[i], provisionedClaims[i]) { - t.Errorf("Test %q failed. provisioned claims doesn't match [A-expected, B-got]: %s", name, diff.ObjectDiff(expectedProvisionings[i], provisionedClaims[i])) + t.Errorf("provisioned claims doesn't match [A-expected, B-got]: %s", diff.ObjectDiff(expectedProvisionings[i], provisionedClaims[i])) } } } } -func (env *testEnv) getPodBindings(t *testing.T, name, node string, pod *v1.Pod) []*bindingInfo { +func (env *testEnv) getPodBindings(t *testing.T, node string, pod *v1.Pod) []*bindingInfo { cache := env.internalBinder.podBindingCache return cache.GetBindings(pod, node) } -func (env *testEnv) validateAssume(t *testing.T, name string, pod *v1.Pod, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) { +func (env *testEnv) validateAssume(t *testing.T, pod *v1.Pod, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) { // Check pv cache pvCache := env.internalBinder.pvCache for _, b := range bindings { pv, err := pvCache.GetPV(b.pv.Name) if err != nil { - t.Errorf("Test %q failed: GetPV %q returned error: %v", name, b.pv.Name, err) + t.Errorf("GetPV %q returned error: %v", b.pv.Name, err) continue } if pv.Spec.ClaimRef == nil { - t.Errorf("Test %q failed: PV %q ClaimRef is nil", name, b.pv.Name) + t.Errorf("PV %q ClaimRef is nil", b.pv.Name) continue } if pv.Spec.ClaimRef.Name != b.pvc.Name { - t.Errorf("Test %q failed: expected PV.ClaimRef.Name %q, got %q", name, b.pvc.Name, pv.Spec.ClaimRef.Name) + t.Errorf("expected PV.ClaimRef.Name %q, got %q", b.pvc.Name, pv.Spec.ClaimRef.Name) } if pv.Spec.ClaimRef.Namespace != b.pvc.Namespace { - t.Errorf("Test %q failed: expected PV.ClaimRef.Namespace %q, got %q", name, b.pvc.Namespace, pv.Spec.ClaimRef.Namespace) + t.Errorf("expected PV.ClaimRef.Namespace %q, got %q", b.pvc.Namespace, pv.Spec.ClaimRef.Namespace) } } @@ -451,23 +455,23 @@ func (env *testEnv) validateAssume(t *testing.T, name string, pod *v1.Pod, bindi pvcKey := getPVCName(p) pvc, err := pvcCache.GetPVC(pvcKey) if err != nil { - t.Errorf("Test %q failed: GetPVC %q returned error: %v", name, pvcKey, err) + t.Errorf("GetPVC %q returned error: %v", pvcKey, err) continue } if pvc.Annotations[pvutil.AnnSelectedNode] != nodeLabelValue { - t.Errorf("Test %q failed: expected pvutil.AnnSelectedNode of pvc %q to be %q, but got %q", name, pvcKey, nodeLabelValue, pvc.Annotations[pvutil.AnnSelectedNode]) + t.Errorf("expected pvutil.AnnSelectedNode of pvc %q to be %q, but got %q", pvcKey, nodeLabelValue, pvc.Annotations[pvutil.AnnSelectedNode]) } } } -func (env *testEnv) validateFailedAssume(t *testing.T, name string, pod *v1.Pod, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) { +func (env *testEnv) validateFailedAssume(t *testing.T, pod *v1.Pod, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) { // All PVs have been unmodified in cache pvCache := env.internalBinder.pvCache for _, b := range bindings { pv, _ := pvCache.GetPV(b.pv.Name) // PV could be nil if it's missing from cache if pv != nil && pv != b.pv { - t.Errorf("Test %q failed: PV %q was modified in cache", name, b.pv.Name) + t.Errorf("PV %q was modified in cache", b.pv.Name) } } @@ -477,18 +481,17 @@ func (env *testEnv) validateFailedAssume(t *testing.T, name string, pod *v1.Pod, pvcKey := getPVCName(p) pvc, err := pvcCache.GetPVC(pvcKey) if err != nil { - t.Errorf("Test %q failed: GetPVC %q returned error: %v", name, pvcKey, err) + t.Errorf("GetPVC %q returned error: %v", pvcKey, err) continue } if pvc.Annotations[pvutil.AnnSelectedNode] != "" { - t.Errorf("Test %q failed: expected pvutil.AnnSelectedNode of pvc %q empty, but got %q", name, pvcKey, pvc.Annotations[pvutil.AnnSelectedNode]) + t.Errorf("expected pvutil.AnnSelectedNode of pvc %q empty, but got %q", pvcKey, pvc.Annotations[pvutil.AnnSelectedNode]) } } } func (env *testEnv) validateBind( t *testing.T, - name string, pod *v1.Pod, expectedPVs []*v1.PersistentVolume, expectedAPIPVs []*v1.PersistentVolume) { @@ -498,25 +501,24 @@ func (env *testEnv) validateBind( for _, pv := range expectedPVs { cachedPV, err := pvCache.GetPV(pv.Name) if err != nil { - t.Errorf("Test %q failed: GetPV %q returned error: %v", name, pv.Name, err) + t.Errorf("GetPV %q returned error: %v", pv.Name, err) } // Cache may be overridden by API object with higher version, compare but ignore resource version. newCachedPV := cachedPV.DeepCopy() newCachedPV.ResourceVersion = pv.ResourceVersion if !reflect.DeepEqual(newCachedPV, pv) { - t.Errorf("Test %q failed: cached PV check failed [A-expected, B-got]:\n%s", name, diff.ObjectDiff(pv, cachedPV)) + t.Errorf("cached PV check failed [A-expected, B-got]:\n%s", diff.ObjectDiff(pv, cachedPV)) } } // Check reactor for API updates if err := env.reactor.CheckVolumes(expectedAPIPVs); err != nil { - t.Errorf("Test %q failed: API reactor validation failed: %v", name, err) + t.Errorf("API reactor validation failed: %v", err) } } func (env *testEnv) validateProvision( t *testing.T, - name string, pod *v1.Pod, expectedPVCs []*v1.PersistentVolumeClaim, expectedAPIPVCs []*v1.PersistentVolumeClaim) { @@ -526,19 +528,19 @@ func (env *testEnv) validateProvision( for _, pvc := range expectedPVCs { cachedPVC, err := pvcCache.GetPVC(getPVCName(pvc)) if err != nil { - t.Errorf("Test %q failed: GetPVC %q returned error: %v", name, getPVCName(pvc), err) + t.Errorf("GetPVC %q returned error: %v", getPVCName(pvc), err) } // Cache may be overridden by API object with higher version, compare but ignore resource version. newCachedPVC := cachedPVC.DeepCopy() newCachedPVC.ResourceVersion = pvc.ResourceVersion if !reflect.DeepEqual(newCachedPVC, pvc) { - t.Errorf("Test %q failed: cached PVC check failed [A-expected, B-got]:\n%s", name, diff.ObjectDiff(pvc, cachedPVC)) + t.Errorf("cached PVC check failed [A-expected, B-got]:\n%s", diff.ObjectDiff(pvc, cachedPVC)) } } // Check reactor for API updates if err := env.reactor.CheckClaims(expectedAPIPVCs); err != nil { - t.Errorf("Test %q failed: API reactor validation failed: %v", name, err) + t.Errorf("API reactor validation failed: %v", err) } } @@ -734,7 +736,7 @@ func addProvisionAnn(pvc *v1.PersistentVolumeClaim) *v1.PersistentVolumeClaim { } func TestFindPodVolumesWithoutProvisioning(t *testing.T) { - scenarios := map[string]struct { + type scenarioType struct { // Inputs pvs []*v1.PersistentVolume podPVCs []*v1.PersistentVolumeClaim @@ -750,7 +752,8 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { expectedUnbound bool expectedBound bool shouldFail bool - }{ + } + scenarios := map[string]scenarioType{ "no-volumes": { pod: makePod(nil), expectedUnbound: true, @@ -879,11 +882,9 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { }, } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - for name, scenario := range scenarios { - klog.V(5).Infof("Running test case %q", name) + run := func(t *testing.T, scenario scenarioType) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() // Setup testEnv := newTestBinder(t, ctx.Done()) @@ -905,23 +906,27 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { // Validate if !scenario.shouldFail && err != nil { - t.Errorf("Test %q failed: returned error: %v", name, err) + t.Errorf("returned error: %v", err) } if scenario.shouldFail && err == nil { - t.Errorf("Test %q failed: returned success but expected error", name) + t.Error("returned success but expected error") } if boundSatisfied != scenario.expectedBound { - t.Errorf("Test %q failed: expected boundSatsified %v, got %v", name, scenario.expectedBound, boundSatisfied) + t.Errorf("expected boundSatsified %v, got %v", scenario.expectedBound, boundSatisfied) } if unboundSatisfied != scenario.expectedUnbound { - t.Errorf("Test %q failed: expected unboundSatsified %v, got %v", name, scenario.expectedUnbound, unboundSatisfied) + t.Errorf("expected unboundSatsified %v, got %v", scenario.expectedUnbound, unboundSatisfied) } - testEnv.validatePodCache(t, name, testNode.Name, scenario.pod, scenario.expectedBindings, nil) + testEnv.validatePodCache(t, testNode.Name, scenario.pod, scenario.expectedBindings, nil) + } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { run(t, scenario) }) } } func TestFindPodVolumesWithProvisioning(t *testing.T) { - scenarios := map[string]struct { + type scenarioType struct { // Inputs pvs []*v1.PersistentVolume podPVCs []*v1.PersistentVolumeClaim @@ -938,7 +943,8 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { expectedUnbound bool expectedBound bool shouldFail bool - }{ + } + scenarios := map[string]scenarioType{ "one-provisioned": { podPVCs: []*v1.PersistentVolumeClaim{provisionedPVC}, expectedProvisions: []*v1.PersistentVolumeClaim{provisionedPVC}, @@ -1001,10 +1007,10 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { }, } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + run := func(t *testing.T, scenario scenarioType) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() - for name, scenario := range scenarios { // Setup testEnv := newTestBinder(t, ctx.Done()) testEnv.initVolumes(scenario.pvs, scenario.pvs) @@ -1025,25 +1031,29 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { // Validate if !scenario.shouldFail && err != nil { - t.Errorf("Test %q failed: returned error: %v", name, err) + t.Errorf("returned error: %v", err) } if scenario.shouldFail && err == nil { - t.Errorf("Test %q failed: returned success but expected error", name) + t.Error("returned success but expected error") } if boundSatisfied != scenario.expectedBound { - t.Errorf("Test %q failed: expected boundSatsified %v, got %v", name, scenario.expectedBound, boundSatisfied) + t.Errorf("expected boundSatsified %v, got %v", scenario.expectedBound, boundSatisfied) } if unboundSatisfied != scenario.expectedUnbound { - t.Errorf("Test %q failed: expected unboundSatsified %v, got %v", name, scenario.expectedUnbound, unboundSatisfied) + t.Errorf("expected unboundSatsified %v, got %v", scenario.expectedUnbound, unboundSatisfied) } - testEnv.validatePodCache(t, name, testNode.Name, scenario.pod, scenario.expectedBindings, scenario.expectedProvisions) + testEnv.validatePodCache(t, testNode.Name, scenario.pod, scenario.expectedBindings, scenario.expectedProvisions) + } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { run(t, scenario) }) } } // TestFindPodVolumesWithCSIMigration aims to test the node affinity check procedure that's // done in FindPodVolumes. In order to reach this code path, the given PVCs must be bound to a PV. func TestFindPodVolumesWithCSIMigration(t *testing.T) { - scenarios := map[string]struct { + type scenarioType struct { // Inputs pvs []*v1.PersistentVolume podPVCs []*v1.PersistentVolumeClaim @@ -1060,7 +1070,8 @@ func TestFindPodVolumesWithCSIMigration(t *testing.T) { expectedUnbound bool expectedBound bool shouldFail bool - }{ + } + scenarios := map[string]scenarioType{ "pvc-bound": { podPVCs: []*v1.PersistentVolumeClaim{boundMigrationPVC}, pvs: []*v1.PersistentVolume{migrationPVBound}, @@ -1094,14 +1105,12 @@ func TestFindPodVolumesWithCSIMigration(t *testing.T) { }, } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + run := func(t *testing.T, scenario scenarioType) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, true)() - - for name, scenario := range scenarios { - klog.V(5).Infof("Running test case %q", name) + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, true)() // Setup testEnv := newTestBinder(t, ctx.Done()) @@ -1135,22 +1144,26 @@ func TestFindPodVolumesWithCSIMigration(t *testing.T) { // Validate if !scenario.shouldFail && err != nil { - t.Errorf("Test %q failed: returned error: %v", name, err) + t.Errorf("returned error: %v", err) } if scenario.shouldFail && err == nil { - t.Errorf("Test %q failed: returned success but expected error", name) + t.Error("returned success but expected error") } if boundSatisfied != scenario.expectedBound { - t.Errorf("Test %q failed: expected boundSatsified %v, got %v", name, scenario.expectedBound, boundSatisfied) + t.Errorf("expected boundSatsified %v, got %v", scenario.expectedBound, boundSatisfied) } if unboundSatisfied != scenario.expectedUnbound { - t.Errorf("Test %q failed: expected unboundSatsified %v, got %v", name, scenario.expectedUnbound, unboundSatisfied) + t.Errorf("expected unboundSatsified %v, got %v", scenario.expectedUnbound, unboundSatisfied) } } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { run(t, scenario) }) + } } func TestAssumePodVolumes(t *testing.T) { - scenarios := map[string]struct { + type scenarioType struct { // Inputs podPVCs []*v1.PersistentVolumeClaim pvs []*v1.PersistentVolume @@ -1163,7 +1176,8 @@ func TestAssumePodVolumes(t *testing.T) { expectedBindings []*bindingInfo expectedProvisionings []*v1.PersistentVolumeClaim - }{ + } + scenarios := map[string]scenarioType{ "all-bound": { podPVCs: []*v1.PersistentVolumeClaim{boundPVC}, pvs: []*v1.PersistentVolume{pvBound}, @@ -1213,11 +1227,9 @@ func TestAssumePodVolumes(t *testing.T) { }, } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - for name, scenario := range scenarios { - klog.V(5).Infof("Running test case %q", name) + run := func(t *testing.T, scenario scenarioType) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() // Setup testEnv := newTestBinder(t, ctx.Done()) @@ -1231,13 +1243,13 @@ func TestAssumePodVolumes(t *testing.T) { // Validate if !scenario.shouldFail && err != nil { - t.Errorf("Test %q failed: returned error: %v", name, err) + t.Errorf("returned error: %v", err) } if scenario.shouldFail && err == nil { - t.Errorf("Test %q failed: returned success but expected error", name) + t.Error("returned success but expected error") } if scenario.expectedAllBound != allBound { - t.Errorf("Test %q failed: returned unexpected allBound: %v", name, allBound) + t.Errorf("returned unexpected allBound: %v", allBound) } if scenario.expectedBindings == nil { scenario.expectedBindings = scenario.bindings @@ -1246,16 +1258,20 @@ func TestAssumePodVolumes(t *testing.T) { scenario.expectedProvisionings = scenario.provisionedPVCs } if scenario.shouldFail { - testEnv.validateFailedAssume(t, name, pod, scenario.expectedBindings, scenario.expectedProvisionings) + testEnv.validateFailedAssume(t, pod, scenario.expectedBindings, scenario.expectedProvisionings) } else { - testEnv.validateAssume(t, name, pod, scenario.expectedBindings, scenario.expectedProvisionings) + testEnv.validateAssume(t, pod, scenario.expectedBindings, scenario.expectedProvisionings) } - testEnv.validatePodCache(t, name, pod.Spec.NodeName, pod, scenario.expectedBindings, scenario.expectedProvisionings) + testEnv.validatePodCache(t, pod.Spec.NodeName, pod, scenario.expectedBindings, scenario.expectedProvisionings) + } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { run(t, scenario) }) } } func TestBindAPIUpdate(t *testing.T) { - scenarios := map[string]struct { + type scenarioType struct { // Inputs bindings []*bindingInfo cachedPVs []*v1.PersistentVolume @@ -1276,7 +1292,8 @@ func TestBindAPIUpdate(t *testing.T) { expectedPVCs []*v1.PersistentVolumeClaim // if nil, use expectedPVCs expectedAPIPVCs []*v1.PersistentVolumeClaim - }{ + } + scenarios := map[string]scenarioType{ "nothing-to-bind-nil": { shouldFail: true, }, @@ -1347,11 +1364,9 @@ func TestBindAPIUpdate(t *testing.T) { }, } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - for name, scenario := range scenarios { - klog.V(4).Infof("Running test case %q", name) + run := func(t *testing.T, scenario scenarioType) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() // Setup testEnv := newTestBinder(t, ctx.Done()) @@ -1364,17 +1379,17 @@ func TestBindAPIUpdate(t *testing.T) { } testEnv.initVolumes(scenario.cachedPVs, scenario.apiPVs) testEnv.initClaims(scenario.cachedPVCs, scenario.apiPVCs) - testEnv.assumeVolumes(t, name, "node1", pod, scenario.bindings, scenario.provisionedPVCs) + testEnv.assumeVolumes(t, "node1", pod, scenario.bindings, scenario.provisionedPVCs) // Execute err := testEnv.internalBinder.bindAPIUpdate(pod.Name, scenario.bindings, scenario.provisionedPVCs) // Validate if !scenario.shouldFail && err != nil { - t.Errorf("Test %q failed: returned error: %v", name, err) + t.Errorf("returned error: %v", err) } if scenario.shouldFail && err == nil { - t.Errorf("Test %q failed: returned success but expected error", name) + t.Error("returned success but expected error") } if scenario.expectedAPIPVs == nil { scenario.expectedAPIPVs = scenario.expectedPVs @@ -1382,13 +1397,17 @@ func TestBindAPIUpdate(t *testing.T) { if scenario.expectedAPIPVCs == nil { scenario.expectedAPIPVCs = scenario.expectedPVCs } - testEnv.validateBind(t, name, pod, scenario.expectedPVs, scenario.expectedAPIPVs) - testEnv.validateProvision(t, name, pod, scenario.expectedPVCs, scenario.expectedAPIPVCs) + testEnv.validateBind(t, pod, scenario.expectedPVs, scenario.expectedAPIPVs) + testEnv.validateProvision(t, pod, scenario.expectedPVCs, scenario.expectedAPIPVCs) + } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { run(t, scenario) }) } } func TestCheckBindings(t *testing.T) { - scenarios := map[string]struct { + type scenarioType struct { // Inputs initPVs []*v1.PersistentVolume initPVCs []*v1.PersistentVolumeClaim @@ -1407,7 +1426,8 @@ func TestCheckBindings(t *testing.T) { // Expected return values shouldFail bool expectedBound bool - }{ + } + scenarios := map[string]scenarioType{ "nothing-to-bind-nil": { shouldFail: true, }, @@ -1538,11 +1558,9 @@ func TestCheckBindings(t *testing.T) { }, } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - for name, scenario := range scenarios { - klog.V(4).Infof("Running test case %q", name) + run := func(t *testing.T, scenario scenarioType) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() // Setup pod := makePod(nil) @@ -1550,7 +1568,7 @@ func TestCheckBindings(t *testing.T) { testEnv.initNodes([]*v1.Node{node1}) testEnv.initVolumes(scenario.initPVs, nil) testEnv.initClaims(scenario.initPVCs, nil) - testEnv.assumeVolumes(t, name, "node1", pod, scenario.bindings, scenario.provisionedPVCs) + testEnv.assumeVolumes(t, "node1", pod, scenario.bindings, scenario.provisionedPVCs) // Before execute if scenario.deletePVs { @@ -1569,19 +1587,23 @@ func TestCheckBindings(t *testing.T) { // Validate if !scenario.shouldFail && err != nil { - t.Errorf("Test %q failed: returned error: %v", name, err) + t.Errorf("returned error: %v", err) } if scenario.shouldFail && err == nil { - t.Errorf("Test %q failed: returned success but expected error", name) + t.Error("returned success but expected error") } if scenario.expectedBound != allBound { - t.Errorf("Test %q failed: returned bound %v", name, allBound) + t.Errorf("returned bound %v", allBound) } } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { run(t, scenario) }) + } } func TestCheckBindingsWithCSIMigration(t *testing.T) { - scenarios := map[string]struct { + type scenarioType struct { // Inputs initPVs []*v1.PersistentVolume initPVCs []*v1.PersistentVolumeClaim @@ -1599,7 +1621,8 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { shouldFail bool expectedBound bool migrationEnabled bool - }{ + } + scenarios := map[string]scenarioType{ "provisioning-pvc-bound": { bindings: []*bindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provMigrationPVCBound)}, @@ -1658,41 +1681,43 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { }, } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + run := func(t *testing.T, scenario scenarioType) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, scenario.migrationEnabled)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, scenario.migrationEnabled)() + + // Setup + pod := makePod(nil) + testEnv := newTestBinder(t, ctx.Done()) + testEnv.initNodes(scenario.initNodes) + testEnv.initCSINodes(scenario.initCSINodes) + testEnv.initVolumes(scenario.initPVs, nil) + testEnv.initClaims(scenario.initPVCs, nil) + testEnv.assumeVolumes(t, "node1", pod, scenario.bindings, scenario.provisionedPVCs) + + // Before execute + testEnv.updateVolumes(t, scenario.apiPVs, true) + testEnv.updateClaims(t, scenario.apiPVCs, true) + + // Execute + allBound, err := testEnv.internalBinder.checkBindings(pod, scenario.bindings, scenario.provisionedPVCs) + + // Validate + if !scenario.shouldFail && err != nil { + t.Errorf("returned error: %v", err) + } + if scenario.shouldFail && err == nil { + t.Error("returned success but expected error") + } + if scenario.expectedBound != allBound { + t.Errorf("returned bound %v", allBound) + } + } for name, scenario := range scenarios { - t.Run(name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, scenario.migrationEnabled)() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, scenario.migrationEnabled)() - - // Setup - pod := makePod(nil) - testEnv := newTestBinder(t, ctx.Done()) - testEnv.initNodes(scenario.initNodes) - testEnv.initCSINodes(scenario.initCSINodes) - testEnv.initVolumes(scenario.initPVs, nil) - testEnv.initClaims(scenario.initPVCs, nil) - testEnv.assumeVolumes(t, name, "node1", pod, scenario.bindings, scenario.provisionedPVCs) - - // Before execute - testEnv.updateVolumes(t, scenario.apiPVs, true) - testEnv.updateClaims(t, scenario.apiPVCs, true) - - // Execute - allBound, err := testEnv.internalBinder.checkBindings(pod, scenario.bindings, scenario.provisionedPVCs) - - // Validate - if !scenario.shouldFail && err != nil { - t.Errorf("Test %q failed: returned error: %v", name, err) - } - if scenario.shouldFail && err == nil { - t.Errorf("Test %q failed: returned success but expected error", name) - } - if scenario.expectedBound != allBound { - t.Errorf("Test %q failed: returned bound %v", name, allBound) - } - }) + t.Run(name, func(t *testing.T) { run(t, scenario) }) } } @@ -1721,7 +1746,6 @@ func TestBindPodVolumes(t *testing.T) { // Expected return values shouldFail bool } - scenarios := map[string]scenarioType{ "nothing-to-bind-nil": { bindingsNil: true, @@ -1853,11 +1877,9 @@ func TestBindPodVolumes(t *testing.T) { }, } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - for name, scenario := range scenarios { - klog.V(4).Infof("Running test case %q", name) + run := func(t *testing.T, scenario scenarioType) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() // Setup pod := makePod(nil) @@ -1877,20 +1899,20 @@ func TestBindPodVolumes(t *testing.T) { testEnv.initNodes(scenario.nodes) testEnv.initVolumes(scenario.initPVs, scenario.initPVs) testEnv.initClaims(scenario.initPVCs, scenario.initPVCs) - testEnv.assumeVolumes(t, name, "node1", pod, bindings, claimsToProvision) + testEnv.assumeVolumes(t, "node1", pod, bindings, claimsToProvision) } // Before Execute if scenario.apiPV != nil { _, err := testEnv.client.CoreV1().PersistentVolumes().Update(scenario.apiPV) if err != nil { - t.Fatalf("Test %q failed: failed to update PV %q", name, scenario.apiPV.Name) + t.Fatalf("failed to update PV %q", scenario.apiPV.Name) } } if scenario.apiPVC != nil { _, err := testEnv.client.CoreV1().PersistentVolumeClaims(scenario.apiPVC.Namespace).Update(scenario.apiPVC) if err != nil { - t.Fatalf("Test %q failed: failed to update PVC %q", name, getPVCName(scenario.apiPVC)) + t.Fatalf("failed to update PVC %q", getPVCName(scenario.apiPVC)) } } @@ -1908,12 +1930,16 @@ func TestBindPodVolumes(t *testing.T) { // Validate if !scenario.shouldFail && err != nil { - t.Errorf("Test %q failed: returned error: %v", name, err) + t.Errorf("returned error: %v", err) } if scenario.shouldFail && err == nil { - t.Errorf("Test %q failed: returned success but expected error", name) + t.Error("returned success but expected error") } } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { run(t, scenario) }) + } } func TestFindAssumeVolumes(t *testing.T) { @@ -1947,7 +1973,7 @@ func TestFindAssumeVolumes(t *testing.T) { if !unboundSatisfied { t.Errorf("Test failed: couldn't find PVs for all PVCs") } - expectedBindings := testEnv.getPodBindings(t, "before-assume", testNode.Name, pod) + expectedBindings := testEnv.getPodBindings(t, testNode.Name, pod) // 2. Assume matches allBound, err := testEnv.binder.AssumePodVolumes(pod, testNode.Name) @@ -1957,15 +1983,14 @@ func TestFindAssumeVolumes(t *testing.T) { if allBound { t.Errorf("Test failed: detected unbound volumes as bound") } - testEnv.validateAssume(t, "assume", pod, expectedBindings, nil) + testEnv.validateAssume(t, pod, expectedBindings, nil) // After assume, claimref should be set on pv - expectedBindings = testEnv.getPodBindings(t, "after-assume", testNode.Name, pod) + expectedBindings = testEnv.getPodBindings(t, testNode.Name, pod) // 3. Find matching PVs again // This should always return the original chosen pv // Run this many times in case sorting returns different orders for the two PVs. - t.Logf("Testing FindPodVolumes after Assume") for i := 0; i < 50; i++ { unboundSatisfied, _, err := testEnv.binder.FindPodVolumes(pod, testNode) if err != nil { @@ -1974,6 +1999,6 @@ func TestFindAssumeVolumes(t *testing.T) { if !unboundSatisfied { t.Errorf("Test failed: couldn't find PVs for all PVCs") } - testEnv.validatePodCache(t, "after-assume", testNode.Name, pod, expectedBindings, nil) + testEnv.validatePodCache(t, testNode.Name, pod, expectedBindings, nil) } }