From 6eb0b034ac9695adc79aaedfcfe5e5a96b633124 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Fri, 14 Feb 2020 13:40:29 +0100 Subject: [PATCH 1/3] volume scheduler: move reason strings into volume code The scheduler doesn't really need to know in detail which reasons rendered a node unusable for a node. All it needs from the volume binder is a list of reasons that it then can present to the user. This seems a bit cleaner. But the main reason for the change is that it simplifies the checking of CSI inline volumes and perhaps later capacity checking. Both will lead to new failure reasons, which then can be added without changing the interface. --- .../volume/scheduling/scheduler_binder.go | 52 ++-- .../scheduling/scheduler_binder_fake.go | 15 +- .../scheduling/scheduler_binder_test.go | 241 ++++++++---------- pkg/scheduler/core/generic_scheduler_test.go | 6 +- .../plugins/volumebinding/volume_binding.go | 18 +- .../volumebinding/volume_binding_test.go | 28 +- pkg/scheduler/scheduler_test.go | 30 +-- 7 files changed, 174 insertions(+), 216 deletions(-) diff --git a/pkg/controller/volume/scheduling/scheduler_binder.go b/pkg/controller/volume/scheduling/scheduler_binder.go index 564e26eef37..0fb249813e0 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder.go +++ b/pkg/controller/volume/scheduling/scheduler_binder.go @@ -45,6 +45,13 @@ import ( volumeutil "k8s.io/kubernetes/pkg/volume/util" ) +const ( + // ErrReasonBindConflict is used for VolumeBindingNoMatch predicate error. + ErrReasonBindConflict = "node(s) didn't find available persistent volumes to bind" + // ErrReasonNodeConflict is used for VolumeNodeAffinityConflict predicate error. + ErrReasonNodeConflict = "node(s) had volume node affinity conflict" +) + // InTreeToCSITranslator contains methods required to check migratable status // and perform translations from InTree PV's to CSI type InTreeToCSITranslator interface { @@ -83,11 +90,11 @@ type SchedulerVolumeBinder interface { // If a PVC is bound, it checks if the PV's NodeAffinity matches the Node. // Otherwise, it tries to find an available PV to bind to the PVC. // - // It returns true if all of the Pod's PVCs have matching PVs or can be dynamic provisioned, - // and returns true if bound volumes satisfy the PV NodeAffinity. + // It returns an error when something went wrong or a list of reasons why the node is + // (currently) not usable for the pod. // // This function is called by the volume binding scheduler predicate and can be called in parallel - FindPodVolumes(pod *v1.Pod, node *v1.Node) (unboundVolumesSatisified, boundVolumesSatisfied bool, err error) + FindPodVolumes(pod *v1.Pod, node *v1.Node) (reasons []string, err error) // AssumePodVolumes will: // 1. Take the PV matches for unbound PVCs and update the PV cache assuming @@ -166,15 +173,29 @@ func (b *volumeBinder) GetBindingsCache() PodBindingCache { // FindPodVolumes caches the matching PVs and PVCs to provision per node in podBindingCache. // This method intentionally takes in a *v1.Node object instead of using volumebinder.nodeInformer. // That's necessary because some operations will need to pass in to the predicate fake node objects. -func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (unboundVolumesSatisfied, boundVolumesSatisfied bool, err error) { +func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (reasons []string, err error) { podName := getPodName(pod) // Warning: Below log needs high verbosity as it can be printed several times (#60933). klog.V(5).Infof("FindPodVolumes for pod %q, node %q", podName, node.Name) - // Initialize to true for pods that don't have volumes - unboundVolumesSatisfied = true - boundVolumesSatisfied = true + // Initialize to true for pods that don't have volumes. These + // booleans get translated into reason strings when the function + // returns without an error. + unboundVolumesSatisfied := true + boundVolumesSatisfied := true + defer func() { + if err != nil { + return + } + if !boundVolumesSatisfied { + reasons = append(reasons, ErrReasonNodeConflict) + } + if !unboundVolumesSatisfied { + reasons = append(reasons, ErrReasonBindConflict) + } + }() + start := time.Now() defer func() { metrics.VolumeSchedulingStageLatency.WithLabelValues("predicate").Observe(time.Since(start).Seconds()) @@ -210,19 +231,19 @@ func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (unboundVolume // volumes can get bound/provisioned in between calls. boundClaims, claimsToBind, unboundClaimsImmediate, err := b.getPodVolumes(pod) if err != nil { - return false, false, err + return nil, err } // Immediate claims should be bound if len(unboundClaimsImmediate) > 0 { - return false, false, fmt.Errorf("pod has unbound immediate PersistentVolumeClaims") + return nil, fmt.Errorf("pod has unbound immediate PersistentVolumeClaims") } // Check PV node affinity on bound volumes if len(boundClaims) > 0 { boundVolumesSatisfied, err = b.checkBoundClaims(boundClaims, node, podName) if err != nil { - return false, false, err + return nil, err } } @@ -237,8 +258,9 @@ func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (unboundVolume for _, claim := range claimsToBind { if selectedNode, ok := claim.Annotations[pvutil.AnnSelectedNode]; ok { if selectedNode != node.Name { - // Fast path, skip unmatched node - return false, boundVolumesSatisfied, nil + // Fast path, skip unmatched node. + unboundVolumesSatisfied = false + return } claimsToProvision = append(claimsToProvision, claim) } else { @@ -251,7 +273,7 @@ func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (unboundVolume var unboundClaims []*v1.PersistentVolumeClaim unboundVolumesSatisfied, matchedBindings, unboundClaims, err = b.findMatchingVolumes(pod, claimsToFindMatching, node) if err != nil { - return false, false, err + return nil, err } claimsToProvision = append(claimsToProvision, unboundClaims...) } @@ -260,12 +282,12 @@ func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (unboundVolume if len(claimsToProvision) > 0 { unboundVolumesSatisfied, provisionedClaims, err = b.checkVolumeProvisions(pod, claimsToProvision, node) if err != nil { - return false, false, err + return nil, err } } } - return unboundVolumesSatisfied, boundVolumesSatisfied, nil + return } // AssumePodVolumes will take the cached matching PVs and PVCs to provision diff --git a/pkg/controller/volume/scheduling/scheduler_binder_fake.go b/pkg/controller/volume/scheduling/scheduler_binder_fake.go index bb38c0d6ba2..fa64d53c8ad 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder_fake.go +++ b/pkg/controller/volume/scheduling/scheduler_binder_fake.go @@ -20,12 +20,11 @@ import "k8s.io/api/core/v1" // FakeVolumeBinderConfig holds configurations for fake volume binder. type FakeVolumeBinderConfig struct { - AllBound bool - FindUnboundSatsified bool - FindBoundSatsified bool - FindErr error - AssumeErr error - BindErr error + AllBound bool + FindReasons []string + FindErr error + AssumeErr error + BindErr error } // NewFakeVolumeBinder sets up all the caches needed for the scheduler to make @@ -44,8 +43,8 @@ type FakeVolumeBinder struct { } // FindPodVolumes implements SchedulerVolumeBinder.FindPodVolumes. -func (b *FakeVolumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (unboundVolumesSatisfied, boundVolumesSatsified bool, err error) { - return b.config.FindUnboundSatsified, b.config.FindBoundSatsified, b.config.FindErr +func (b *FakeVolumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (reasons []string, err error) { + return b.config.FindReasons, b.config.FindErr } // AssumePodVolumes implements SchedulerVolumeBinder.AssumePodVolumes. diff --git a/pkg/controller/volume/scheduling/scheduler_binder_test.go b/pkg/controller/volume/scheduling/scheduler_binder_test.go index 777fae8e333..b502079027c 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder_test.go +++ b/pkg/controller/volume/scheduling/scheduler_binder_test.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "reflect" + "sort" "testing" "time" @@ -735,6 +736,41 @@ func addProvisionAnn(pvc *v1.PersistentVolumeClaim) *v1.PersistentVolumeClaim { return res } +// reasonNames pretty-prints a list of reasons with variable names in +// case of a test failure because that is easier to read than the full +// strings. +func reasonNames(reasons []string) string { + var varNames []string + for _, reason := range reasons { + switch reason { + case ErrReasonBindConflict: + varNames = append(varNames, "ErrReasonBindConflict") + case ErrReasonNodeConflict: + varNames = append(varNames, "ErrReasonNodeConflict") + default: + varNames = append(varNames, reason) + } + } + return fmt.Sprintf("%v", varNames) +} + +func checkReasons(t *testing.T, actual, expected []string) { + equal := len(actual) == len(expected) + sort.Strings(actual) + sort.Strings(expected) + if equal { + for i, reason := range actual { + if reason != expected[i] { + equal = false + break + } + } + } + if !equal { + t.Errorf("expected failure reasons %s, got %s", reasonNames(expected), reasonNames(actual)) + } +} + func TestFindPodVolumesWithoutProvisioning(t *testing.T) { type scenarioType struct { // Inputs @@ -749,39 +785,28 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { expectedBindings []*bindingInfo // Expected return values - expectedUnbound bool - expectedBound bool - shouldFail bool + reasons []string + shouldFail bool } scenarios := map[string]scenarioType{ "no-volumes": { - pod: makePod(nil), - expectedUnbound: true, - expectedBound: true, + pod: makePod(nil), }, "no-pvcs": { - pod: makePodWithoutPVC(), - expectedUnbound: true, - expectedBound: true, + pod: makePodWithoutPVC(), }, "pvc-not-found": { - cachePVCs: []*v1.PersistentVolumeClaim{}, - podPVCs: []*v1.PersistentVolumeClaim{boundPVC}, - expectedUnbound: false, - expectedBound: false, - shouldFail: true, + cachePVCs: []*v1.PersistentVolumeClaim{}, + podPVCs: []*v1.PersistentVolumeClaim{boundPVC}, + shouldFail: true, }, "bound-pvc": { - podPVCs: []*v1.PersistentVolumeClaim{boundPVC}, - pvs: []*v1.PersistentVolume{pvBound}, - expectedUnbound: true, - expectedBound: true, + podPVCs: []*v1.PersistentVolumeClaim{boundPVC}, + pvs: []*v1.PersistentVolume{pvBound}, }, "bound-pvc,pv-not-exists": { - podPVCs: []*v1.PersistentVolumeClaim{boundPVC}, - expectedUnbound: false, - expectedBound: false, - shouldFail: true, + podPVCs: []*v1.PersistentVolumeClaim{boundPVC}, + shouldFail: true, }, "prebound-pvc": { podPVCs: []*v1.PersistentVolumeClaim{preboundPVC}, @@ -792,48 +817,37 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, pvs: []*v1.PersistentVolume{pvNode2, pvNode1a, pvNode1b}, expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, - expectedUnbound: true, - expectedBound: true, }, "unbound-pvc,pv-different-node": { - podPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, - pvs: []*v1.PersistentVolume{pvNode2}, - expectedUnbound: false, - expectedBound: true, + podPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, + pvs: []*v1.PersistentVolume{pvNode2}, + reasons: []string{ErrReasonBindConflict}, }, "two-unbound-pvcs": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, unboundPVC2}, pvs: []*v1.PersistentVolume{pvNode1a, pvNode1b}, expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(unboundPVC2, pvNode1b)}, - expectedUnbound: true, - expectedBound: true, }, "two-unbound-pvcs,order-by-size": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC2, unboundPVC}, pvs: []*v1.PersistentVolume{pvNode1a, pvNode1b}, expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(unboundPVC2, pvNode1b)}, - expectedUnbound: true, - expectedBound: true, }, "two-unbound-pvcs,partial-match": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, unboundPVC2}, pvs: []*v1.PersistentVolume{pvNode1a}, expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, - expectedUnbound: false, - expectedBound: true, + reasons: []string{ErrReasonBindConflict}, }, "one-bound,one-unbound": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, boundPVC}, pvs: []*v1.PersistentVolume{pvBound, pvNode1a}, expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, - expectedUnbound: true, - expectedBound: true, }, "one-bound,one-unbound,no-match": { - podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, boundPVC}, - pvs: []*v1.PersistentVolume{pvBound, pvNode2}, - expectedUnbound: false, - expectedBound: true, + podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, boundPVC}, + pvs: []*v1.PersistentVolume{pvBound, pvNode2}, + reasons: []string{ErrReasonBindConflict}, }, "one-prebound,one-unbound": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, preboundPVC}, @@ -841,35 +855,26 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { shouldFail: true, }, "immediate-bound-pvc": { - podPVCs: []*v1.PersistentVolumeClaim{immediateBoundPVC}, - pvs: []*v1.PersistentVolume{pvBoundImmediate}, - expectedUnbound: true, - expectedBound: true, + podPVCs: []*v1.PersistentVolumeClaim{immediateBoundPVC}, + pvs: []*v1.PersistentVolume{pvBoundImmediate}, }, "immediate-bound-pvc-wrong-node": { - podPVCs: []*v1.PersistentVolumeClaim{immediateBoundPVC}, - pvs: []*v1.PersistentVolume{pvBoundImmediateNode2}, - expectedUnbound: true, - expectedBound: false, + podPVCs: []*v1.PersistentVolumeClaim{immediateBoundPVC}, + pvs: []*v1.PersistentVolume{pvBoundImmediateNode2}, + reasons: []string{ErrReasonNodeConflict}, }, "immediate-unbound-pvc": { - podPVCs: []*v1.PersistentVolumeClaim{immediateUnboundPVC}, - expectedUnbound: false, - expectedBound: false, - shouldFail: true, + podPVCs: []*v1.PersistentVolumeClaim{immediateUnboundPVC}, + shouldFail: true, }, "immediate-unbound-pvc,delayed-mode-bound": { - podPVCs: []*v1.PersistentVolumeClaim{immediateUnboundPVC, boundPVC}, - pvs: []*v1.PersistentVolume{pvBound}, - expectedUnbound: false, - expectedBound: false, - shouldFail: true, + podPVCs: []*v1.PersistentVolumeClaim{immediateUnboundPVC, boundPVC}, + pvs: []*v1.PersistentVolume{pvBound}, + shouldFail: true, }, "immediate-unbound-pvc,delayed-mode-unbound": { - podPVCs: []*v1.PersistentVolumeClaim{immediateUnboundPVC, unboundPVC}, - expectedUnbound: false, - expectedBound: false, - shouldFail: true, + podPVCs: []*v1.PersistentVolumeClaim{immediateUnboundPVC, unboundPVC}, + shouldFail: true, }, } @@ -902,7 +907,7 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { } // Execute - unboundSatisfied, boundSatisfied, err := testEnv.binder.FindPodVolumes(scenario.pod, testNode) + reasons, err := testEnv.binder.FindPodVolumes(scenario.pod, testNode) // Validate if !scenario.shouldFail && err != nil { @@ -911,12 +916,7 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { if scenario.shouldFail && err == nil { t.Error("returned success but expected error") } - if boundSatisfied != scenario.expectedBound { - t.Errorf("expected boundSatisfied %v, got %v", scenario.expectedBound, boundSatisfied) - } - if unboundSatisfied != scenario.expectedUnbound { - t.Errorf("expected unboundSatisfied %v, got %v", scenario.expectedUnbound, unboundSatisfied) - } + checkReasons(t, reasons, scenario.reasons) testEnv.validatePodCache(t, testNode.Name, scenario.pod, scenario.expectedBindings, nil) } @@ -940,61 +940,46 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { expectedProvisions []*v1.PersistentVolumeClaim // Expected return values - expectedUnbound bool - expectedBound bool - shouldFail bool + reasons []string + shouldFail bool } scenarios := map[string]scenarioType{ "one-provisioned": { podPVCs: []*v1.PersistentVolumeClaim{provisionedPVC}, expectedProvisions: []*v1.PersistentVolumeClaim{provisionedPVC}, - expectedUnbound: true, - expectedBound: true, }, "two-unbound-pvcs,one-matched,one-provisioned": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, provisionedPVC}, pvs: []*v1.PersistentVolume{pvNode1a}, expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, expectedProvisions: []*v1.PersistentVolumeClaim{provisionedPVC}, - expectedUnbound: true, - expectedBound: true, }, "one-bound,one-provisioned": { podPVCs: []*v1.PersistentVolumeClaim{boundPVC, provisionedPVC}, pvs: []*v1.PersistentVolume{pvBound}, expectedProvisions: []*v1.PersistentVolumeClaim{provisionedPVC}, - expectedUnbound: true, - expectedBound: true, }, "one-binding,one-selected-node": { podPVCs: []*v1.PersistentVolumeClaim{boundPVC, selectedNodePVC}, pvs: []*v1.PersistentVolume{pvBound}, expectedProvisions: []*v1.PersistentVolumeClaim{selectedNodePVC}, - expectedUnbound: true, - expectedBound: true, }, "immediate-unbound-pvc": { - podPVCs: []*v1.PersistentVolumeClaim{immediateUnboundPVC}, - expectedUnbound: false, - expectedBound: false, - shouldFail: true, + podPVCs: []*v1.PersistentVolumeClaim{immediateUnboundPVC}, + shouldFail: true, }, "one-immediate-bound,one-provisioned": { podPVCs: []*v1.PersistentVolumeClaim{immediateBoundPVC, provisionedPVC}, pvs: []*v1.PersistentVolume{pvBoundImmediate}, expectedProvisions: []*v1.PersistentVolumeClaim{provisionedPVC}, - expectedUnbound: true, - expectedBound: true, }, "invalid-provisioner": { - podPVCs: []*v1.PersistentVolumeClaim{noProvisionerPVC}, - expectedUnbound: false, - expectedBound: true, + podPVCs: []*v1.PersistentVolumeClaim{noProvisionerPVC}, + reasons: []string{ErrReasonBindConflict}, }, "volume-topology-unsatisfied": { - podPVCs: []*v1.PersistentVolumeClaim{topoMismatchPVC}, - expectedUnbound: false, - expectedBound: true, + podPVCs: []*v1.PersistentVolumeClaim{topoMismatchPVC}, + reasons: []string{ErrReasonBindConflict}, }, } @@ -1027,7 +1012,7 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { } // Execute - unboundSatisfied, boundSatisfied, err := testEnv.binder.FindPodVolumes(scenario.pod, testNode) + reasons, err := testEnv.binder.FindPodVolumes(scenario.pod, testNode) // Validate if !scenario.shouldFail && err != nil { @@ -1036,12 +1021,7 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { if scenario.shouldFail && err == nil { t.Error("returned success but expected error") } - if boundSatisfied != scenario.expectedBound { - t.Errorf("expected boundSatisfied %v, got %v", scenario.expectedBound, boundSatisfied) - } - if unboundSatisfied != scenario.expectedUnbound { - t.Errorf("expected unboundSatisfied %v, got %v", scenario.expectedUnbound, unboundSatisfied) - } + checkReasons(t, reasons, scenario.reasons) testEnv.validatePodCache(t, testNode.Name, scenario.pod, scenario.expectedBindings, scenario.expectedProvisions) } @@ -1067,41 +1047,33 @@ func TestFindPodVolumesWithCSIMigration(t *testing.T) { initCSINodes []*storagev1.CSINode // Expected return values - expectedUnbound bool - expectedBound bool - shouldFail bool + reasons []string + shouldFail bool } scenarios := map[string]scenarioType{ "pvc-bound": { - podPVCs: []*v1.PersistentVolumeClaim{boundMigrationPVC}, - pvs: []*v1.PersistentVolume{migrationPVBound}, - initNodes: []*v1.Node{node1Zone1}, - initCSINodes: []*storagev1.CSINode{csiNode1Migrated}, - expectedBound: true, - expectedUnbound: true, + podPVCs: []*v1.PersistentVolumeClaim{boundMigrationPVC}, + pvs: []*v1.PersistentVolume{migrationPVBound}, + initNodes: []*v1.Node{node1Zone1}, + initCSINodes: []*storagev1.CSINode{csiNode1Migrated}, }, "pvc-bound,csinode-not-migrated": { - podPVCs: []*v1.PersistentVolumeClaim{boundMigrationPVC}, - pvs: []*v1.PersistentVolume{migrationPVBound}, - initNodes: []*v1.Node{node1Zone1}, - initCSINodes: []*storagev1.CSINode{csiNode1NotMigrated}, - expectedBound: true, - expectedUnbound: true, + podPVCs: []*v1.PersistentVolumeClaim{boundMigrationPVC}, + pvs: []*v1.PersistentVolume{migrationPVBound}, + initNodes: []*v1.Node{node1Zone1}, + initCSINodes: []*storagev1.CSINode{csiNode1NotMigrated}, }, "pvc-bound,missing-csinode": { - podPVCs: []*v1.PersistentVolumeClaim{boundMigrationPVC}, - pvs: []*v1.PersistentVolume{migrationPVBound}, - initNodes: []*v1.Node{node1Zone1}, - expectedBound: true, - expectedUnbound: true, + podPVCs: []*v1.PersistentVolumeClaim{boundMigrationPVC}, + pvs: []*v1.PersistentVolume{migrationPVBound}, + initNodes: []*v1.Node{node1Zone1}, }, "pvc-bound,node-different-zone": { - podPVCs: []*v1.PersistentVolumeClaim{boundMigrationPVC}, - pvs: []*v1.PersistentVolume{migrationPVBound}, - initNodes: []*v1.Node{node1Zone2}, - initCSINodes: []*storagev1.CSINode{csiNode1Migrated}, - expectedBound: false, - expectedUnbound: true, + podPVCs: []*v1.PersistentVolumeClaim{boundMigrationPVC}, + pvs: []*v1.PersistentVolume{migrationPVBound}, + initNodes: []*v1.Node{node1Zone2}, + initCSINodes: []*storagev1.CSINode{csiNode1Migrated}, + reasons: []string{ErrReasonNodeConflict}, }, } @@ -1140,7 +1112,7 @@ func TestFindPodVolumesWithCSIMigration(t *testing.T) { } // Execute - unboundSatisfied, boundSatisfied, err := testEnv.binder.FindPodVolumes(scenario.pod, node) + reasons, err := testEnv.binder.FindPodVolumes(scenario.pod, node) // Validate if !scenario.shouldFail && err != nil { @@ -1149,12 +1121,7 @@ func TestFindPodVolumesWithCSIMigration(t *testing.T) { if scenario.shouldFail && err == nil { t.Error("returned success but expected error") } - if boundSatisfied != scenario.expectedBound { - t.Errorf("expected boundSatisfied %v, got %v", scenario.expectedBound, boundSatisfied) - } - if unboundSatisfied != scenario.expectedUnbound { - t.Errorf("expected unboundSatisfied %v, got %v", scenario.expectedUnbound, unboundSatisfied) - } + checkReasons(t, reasons, scenario.reasons) } for name, scenario := range scenarios { @@ -1966,12 +1933,12 @@ func TestFindAssumeVolumes(t *testing.T) { // Execute // 1. Find matching PVs - unboundSatisfied, _, err := testEnv.binder.FindPodVolumes(pod, testNode) + reasons, err := testEnv.binder.FindPodVolumes(pod, testNode) if err != nil { t.Errorf("Test failed: FindPodVolumes returned error: %v", err) } - if !unboundSatisfied { - t.Errorf("Test failed: couldn't find PVs for all PVCs") + if len(reasons) > 0 { + t.Errorf("Test failed: couldn't find PVs for all PVCs: %v", reasons) } expectedBindings := testEnv.getPodBindings(t, testNode.Name, pod) @@ -1992,12 +1959,12 @@ func TestFindAssumeVolumes(t *testing.T) { // This should always return the original chosen pv // Run this many times in case sorting returns different orders for the two PVs. for i := 0; i < 50; i++ { - unboundSatisfied, _, err := testEnv.binder.FindPodVolumes(pod, testNode) + reasons, err := testEnv.binder.FindPodVolumes(pod, testNode) if err != nil { t.Errorf("Test failed: FindPodVolumes returned error: %v", err) } - if !unboundSatisfied { - t.Errorf("Test failed: couldn't find PVs for all PVCs") + if len(reasons) > 0 { + t.Errorf("Test failed: couldn't find PVs for all PVCs: %v", reasons) } testEnv.validatePodCache(t, testNode.Name, pod, expectedBindings, nil) } diff --git a/pkg/scheduler/core/generic_scheduler_test.go b/pkg/scheduler/core/generic_scheduler_test.go index a91766efd87..b418c312bca 100644 --- a/pkg/scheduler/core/generic_scheduler_test.go +++ b/pkg/scheduler/core/generic_scheduler_test.go @@ -38,6 +38,7 @@ import ( "k8s.io/client-go/informers" clientsetfake "k8s.io/client-go/kubernetes/fake" extenderv1 "k8s.io/kube-scheduler/extender/v1" + volumescheduling "k8s.io/kubernetes/pkg/controller/volume/scheduling" schedulerapi "k8s.io/kubernetes/pkg/scheduler/apis/config" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/defaultbinder" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/defaultpodtopologyspread" @@ -50,7 +51,6 @@ import ( "k8s.io/kubernetes/pkg/scheduler/framework/plugins/podtopologyspread" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/queuesort" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/tainttoleration" - "k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumebinding" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumerestrictions" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumezone" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" @@ -1999,8 +1999,8 @@ func TestNodesWherePreemptionMightHelp(t *testing.T) { name: "ErrVolume... errors should not be tried as it indicates that the pod is unschedulable due to no matching volumes for pod on node", nodesStatuses: framework.NodeToStatusMap{ "machine1": framework.NewStatus(framework.UnschedulableAndUnresolvable, volumezone.ErrReasonConflict), - "machine2": framework.NewStatus(framework.UnschedulableAndUnresolvable, volumebinding.ErrReasonNodeConflict), - "machine3": framework.NewStatus(framework.UnschedulableAndUnresolvable, volumebinding.ErrReasonBindConflict), + "machine2": framework.NewStatus(framework.UnschedulableAndUnresolvable, volumescheduling.ErrReasonNodeConflict), + "machine3": framework.NewStatus(framework.UnschedulableAndUnresolvable, volumescheduling.ErrReasonBindConflict), }, expected: map[string]bool{"machine4": true}, }, diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go index 772dad6f220..2beb0287b00 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go @@ -36,13 +36,6 @@ var _ framework.FilterPlugin = &VolumeBinding{} // Name is the name of the plugin used in Registry and configurations. const Name = "VolumeBinding" -const ( - // ErrReasonBindConflict is used for VolumeBindingNoMatch predicate error. - ErrReasonBindConflict = "node(s) didn't find available persistent volumes to bind" - // ErrReasonNodeConflict is used for VolumeNodeAffinityConflict predicate error. - ErrReasonNodeConflict = "node(s) had volume node affinity conflict" -) - // Name returns name of the plugin. It is used in logs, etc. func (pl *VolumeBinding) Name() string { return Name @@ -79,19 +72,16 @@ func (pl *VolumeBinding) Filter(ctx context.Context, cs *framework.CycleState, p return nil } - unboundSatisfied, boundSatisfied, err := pl.binder.Binder.FindPodVolumes(pod, node) + reasons, err := pl.binder.Binder.FindPodVolumes(pod, node) if err != nil { return framework.NewStatus(framework.Error, err.Error()) } - if !boundSatisfied || !unboundSatisfied { + if len(reasons) > 0 { status := framework.NewStatus(framework.UnschedulableAndUnresolvable) - if !boundSatisfied { - status.AppendReason(ErrReasonNodeConflict) - } - if !unboundSatisfied { - status.AppendReason(ErrReasonBindConflict) + for _, reason := range reasons { + status.AppendReason(reason) } return status } diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go index 390e0c58392..5128e4eb564 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go @@ -58,9 +58,7 @@ func TestVolumeBinding(t *testing.T) { pod: &v1.Pod{Spec: volState}, node: &v1.Node{}, volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - AllBound: true, - FindUnboundSatsified: true, - FindBoundSatsified: true, + AllBound: true, }, wantStatus: nil, }, @@ -69,31 +67,25 @@ func TestVolumeBinding(t *testing.T) { pod: &v1.Pod{Spec: volState}, node: &v1.Node{}, volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindUnboundSatsified: false, - FindBoundSatsified: true, + FindReasons: []string{volumescheduling.ErrReasonBindConflict}, }, - wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonBindConflict), + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, volumescheduling.ErrReasonBindConflict), }, { name: "bound and unbound unsatisfied", pod: &v1.Pod{Spec: volState}, node: &v1.Node{}, volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindUnboundSatsified: false, - FindBoundSatsified: false, + FindReasons: []string{volumescheduling.ErrReasonBindConflict, volumescheduling.ErrReasonNodeConflict}, }, - wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonNodeConflict, - ErrReasonBindConflict), + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, volumescheduling.ErrReasonBindConflict, volumescheduling.ErrReasonNodeConflict), }, { - name: "unbound/found matches/bind succeeds", - pod: &v1.Pod{Spec: volState}, - node: &v1.Node{}, - volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindUnboundSatsified: true, - FindBoundSatsified: true, - }, - wantStatus: nil, + name: "unbound/found matches/bind succeeds", + pod: &v1.Pod{Spec: volState}, + node: &v1.Node{}, + volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{}, + wantStatus: nil, }, { name: "predicate error", diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index 761c35dc3db..ec39968faae 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -901,9 +901,7 @@ func TestSchedulerWithVolumeBinding(t *testing.T) { { name: "all bound", volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - AllBound: true, - FindUnboundSatsified: true, - FindBoundSatsified: true, + AllBound: true, }, expectAssumeCalled: true, expectPodBind: &v1.Binding{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "foo-ns", UID: types.UID("foo")}, Target: v1.ObjectReference{Kind: "Node", Name: "machine1"}}, @@ -912,9 +910,8 @@ func TestSchedulerWithVolumeBinding(t *testing.T) { { name: "bound/invalid pv affinity", volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - AllBound: true, - FindUnboundSatsified: true, - FindBoundSatsified: false, + AllBound: true, + FindReasons: []string{volumescheduling.ErrReasonNodeConflict}, }, eventReason: "FailedScheduling", expectError: makePredicateError("1 node(s) had volume node affinity conflict"), @@ -922,8 +919,7 @@ func TestSchedulerWithVolumeBinding(t *testing.T) { { name: "unbound/no matches", volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindUnboundSatsified: false, - FindBoundSatsified: true, + FindReasons: []string{volumescheduling.ErrReasonBindConflict}, }, eventReason: "FailedScheduling", expectError: makePredicateError("1 node(s) didn't find available persistent volumes to bind"), @@ -931,18 +927,14 @@ func TestSchedulerWithVolumeBinding(t *testing.T) { { name: "bound and unbound unsatisfied", volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindUnboundSatsified: false, - FindBoundSatsified: false, + FindReasons: []string{volumescheduling.ErrReasonBindConflict, volumescheduling.ErrReasonNodeConflict}, }, eventReason: "FailedScheduling", expectError: makePredicateError("1 node(s) didn't find available persistent volumes to bind, 1 node(s) had volume node affinity conflict"), }, { - name: "unbound/found matches/bind succeeds", - volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindUnboundSatsified: true, - FindBoundSatsified: true, - }, + name: "unbound/found matches/bind succeeds", + volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{}, expectAssumeCalled: true, expectBindCalled: true, expectPodBind: &v1.Binding{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "foo-ns", UID: types.UID("foo")}, Target: v1.ObjectReference{Kind: "Node", Name: "machine1"}}, @@ -959,9 +951,7 @@ func TestSchedulerWithVolumeBinding(t *testing.T) { { name: "assume error", volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindUnboundSatsified: true, - FindBoundSatsified: true, - AssumeErr: assumeErr, + AssumeErr: assumeErr, }, expectAssumeCalled: true, eventReason: "FailedScheduling", @@ -970,9 +960,7 @@ func TestSchedulerWithVolumeBinding(t *testing.T) { { name: "bind error", volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindUnboundSatsified: true, - FindBoundSatsified: true, - BindErr: bindErr, + BindErr: bindErr, }, expectAssumeCalled: true, expectBindCalled: true, From 6329b17d2f6fbf834d58dbddd3ff95776c55fd30 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 25 Feb 2020 11:10:27 +0100 Subject: [PATCH 2/3] volume scheduler: introduce special string type This makes it possible to search for the special strings more easily (https://github.com/kubernetes/kubernetes/pull/88230#discussion_r382367043). --- .../volume/scheduling/scheduler_binder.go | 19 +++++++++--- .../scheduling/scheduler_binder_fake.go | 4 +-- .../scheduling/scheduler_binder_test.go | 30 +++++++++---------- pkg/scheduler/core/generic_scheduler_test.go | 4 +-- .../plugins/volumebinding/volume_binding.go | 2 +- .../volumebinding/volume_binding_test.go | 8 ++--- pkg/scheduler/scheduler_test.go | 6 ++-- 7 files changed, 42 insertions(+), 31 deletions(-) diff --git a/pkg/controller/volume/scheduling/scheduler_binder.go b/pkg/controller/volume/scheduling/scheduler_binder.go index 0fb249813e0..352816764a5 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder.go +++ b/pkg/controller/volume/scheduling/scheduler_binder.go @@ -45,11 +45,22 @@ import ( volumeutil "k8s.io/kubernetes/pkg/volume/util" ) +// ConflictReason is used for the special strings which explain why +// volume binding is impossible for a node. +type ConflictReason string + +// ConflictReasons contains all reasons that explain why volume binding is impossible for a node. +type ConflictReasons []ConflictReason + +func (reasons ConflictReasons) Len() int { return len(reasons) } +func (reasons ConflictReasons) Less(i, j int) bool { return reasons[i] < reasons[j] } +func (reasons ConflictReasons) Swap(i, j int) { reasons[i], reasons[j] = reasons[j], reasons[i] } + const ( // ErrReasonBindConflict is used for VolumeBindingNoMatch predicate error. - ErrReasonBindConflict = "node(s) didn't find available persistent volumes to bind" + ErrReasonBindConflict ConflictReason = "node(s) didn't find available persistent volumes to bind" // ErrReasonNodeConflict is used for VolumeNodeAffinityConflict predicate error. - ErrReasonNodeConflict = "node(s) had volume node affinity conflict" + ErrReasonNodeConflict ConflictReason = "node(s) had volume node affinity conflict" ) // InTreeToCSITranslator contains methods required to check migratable status @@ -94,7 +105,7 @@ type SchedulerVolumeBinder interface { // (currently) not usable for the pod. // // This function is called by the volume binding scheduler predicate and can be called in parallel - FindPodVolumes(pod *v1.Pod, node *v1.Node) (reasons []string, err error) + FindPodVolumes(pod *v1.Pod, node *v1.Node) (reasons ConflictReasons, err error) // AssumePodVolumes will: // 1. Take the PV matches for unbound PVCs and update the PV cache assuming @@ -173,7 +184,7 @@ func (b *volumeBinder) GetBindingsCache() PodBindingCache { // FindPodVolumes caches the matching PVs and PVCs to provision per node in podBindingCache. // This method intentionally takes in a *v1.Node object instead of using volumebinder.nodeInformer. // That's necessary because some operations will need to pass in to the predicate fake node objects. -func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (reasons []string, err error) { +func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (reasons ConflictReasons, err error) { podName := getPodName(pod) // Warning: Below log needs high verbosity as it can be printed several times (#60933). diff --git a/pkg/controller/volume/scheduling/scheduler_binder_fake.go b/pkg/controller/volume/scheduling/scheduler_binder_fake.go index fa64d53c8ad..45d66969401 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder_fake.go +++ b/pkg/controller/volume/scheduling/scheduler_binder_fake.go @@ -21,7 +21,7 @@ import "k8s.io/api/core/v1" // FakeVolumeBinderConfig holds configurations for fake volume binder. type FakeVolumeBinderConfig struct { AllBound bool - FindReasons []string + FindReasons ConflictReasons FindErr error AssumeErr error BindErr error @@ -43,7 +43,7 @@ type FakeVolumeBinder struct { } // FindPodVolumes implements SchedulerVolumeBinder.FindPodVolumes. -func (b *FakeVolumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (reasons []string, err error) { +func (b *FakeVolumeBinder) FindPodVolumes(pod *v1.Pod, node *v1.Node) (reasons ConflictReasons, err error) { return b.config.FindReasons, b.config.FindErr } diff --git a/pkg/controller/volume/scheduling/scheduler_binder_test.go b/pkg/controller/volume/scheduling/scheduler_binder_test.go index b502079027c..bbf6f033a72 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder_test.go +++ b/pkg/controller/volume/scheduling/scheduler_binder_test.go @@ -739,7 +739,7 @@ func addProvisionAnn(pvc *v1.PersistentVolumeClaim) *v1.PersistentVolumeClaim { // reasonNames pretty-prints a list of reasons with variable names in // case of a test failure because that is easier to read than the full // strings. -func reasonNames(reasons []string) string { +func reasonNames(reasons ConflictReasons) string { var varNames []string for _, reason := range reasons { switch reason { @@ -748,16 +748,16 @@ func reasonNames(reasons []string) string { case ErrReasonNodeConflict: varNames = append(varNames, "ErrReasonNodeConflict") default: - varNames = append(varNames, reason) + varNames = append(varNames, string(reason)) } } return fmt.Sprintf("%v", varNames) } -func checkReasons(t *testing.T, actual, expected []string) { +func checkReasons(t *testing.T, actual, expected ConflictReasons) { equal := len(actual) == len(expected) - sort.Strings(actual) - sort.Strings(expected) + sort.Sort(actual) + sort.Sort(expected) if equal { for i, reason := range actual { if reason != expected[i] { @@ -785,7 +785,7 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { expectedBindings []*bindingInfo // Expected return values - reasons []string + reasons ConflictReasons shouldFail bool } scenarios := map[string]scenarioType{ @@ -821,7 +821,7 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { "unbound-pvc,pv-different-node": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, pvs: []*v1.PersistentVolume{pvNode2}, - reasons: []string{ErrReasonBindConflict}, + reasons: ConflictReasons{ErrReasonBindConflict}, }, "two-unbound-pvcs": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, unboundPVC2}, @@ -837,7 +837,7 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, unboundPVC2}, pvs: []*v1.PersistentVolume{pvNode1a}, expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, - reasons: []string{ErrReasonBindConflict}, + reasons: ConflictReasons{ErrReasonBindConflict}, }, "one-bound,one-unbound": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, boundPVC}, @@ -847,7 +847,7 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { "one-bound,one-unbound,no-match": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, boundPVC}, pvs: []*v1.PersistentVolume{pvBound, pvNode2}, - reasons: []string{ErrReasonBindConflict}, + reasons: ConflictReasons{ErrReasonBindConflict}, }, "one-prebound,one-unbound": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, preboundPVC}, @@ -861,7 +861,7 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { "immediate-bound-pvc-wrong-node": { podPVCs: []*v1.PersistentVolumeClaim{immediateBoundPVC}, pvs: []*v1.PersistentVolume{pvBoundImmediateNode2}, - reasons: []string{ErrReasonNodeConflict}, + reasons: ConflictReasons{ErrReasonNodeConflict}, }, "immediate-unbound-pvc": { podPVCs: []*v1.PersistentVolumeClaim{immediateUnboundPVC}, @@ -940,7 +940,7 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { expectedProvisions []*v1.PersistentVolumeClaim // Expected return values - reasons []string + reasons ConflictReasons shouldFail bool } scenarios := map[string]scenarioType{ @@ -975,11 +975,11 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { }, "invalid-provisioner": { podPVCs: []*v1.PersistentVolumeClaim{noProvisionerPVC}, - reasons: []string{ErrReasonBindConflict}, + reasons: ConflictReasons{ErrReasonBindConflict}, }, "volume-topology-unsatisfied": { podPVCs: []*v1.PersistentVolumeClaim{topoMismatchPVC}, - reasons: []string{ErrReasonBindConflict}, + reasons: ConflictReasons{ErrReasonBindConflict}, }, } @@ -1047,7 +1047,7 @@ func TestFindPodVolumesWithCSIMigration(t *testing.T) { initCSINodes []*storagev1.CSINode // Expected return values - reasons []string + reasons ConflictReasons shouldFail bool } scenarios := map[string]scenarioType{ @@ -1073,7 +1073,7 @@ func TestFindPodVolumesWithCSIMigration(t *testing.T) { pvs: []*v1.PersistentVolume{migrationPVBound}, initNodes: []*v1.Node{node1Zone2}, initCSINodes: []*storagev1.CSINode{csiNode1Migrated}, - reasons: []string{ErrReasonNodeConflict}, + reasons: ConflictReasons{ErrReasonNodeConflict}, }, } diff --git a/pkg/scheduler/core/generic_scheduler_test.go b/pkg/scheduler/core/generic_scheduler_test.go index b418c312bca..c8bc97051c2 100644 --- a/pkg/scheduler/core/generic_scheduler_test.go +++ b/pkg/scheduler/core/generic_scheduler_test.go @@ -1999,8 +1999,8 @@ func TestNodesWherePreemptionMightHelp(t *testing.T) { name: "ErrVolume... errors should not be tried as it indicates that the pod is unschedulable due to no matching volumes for pod on node", nodesStatuses: framework.NodeToStatusMap{ "machine1": framework.NewStatus(framework.UnschedulableAndUnresolvable, volumezone.ErrReasonConflict), - "machine2": framework.NewStatus(framework.UnschedulableAndUnresolvable, volumescheduling.ErrReasonNodeConflict), - "machine3": framework.NewStatus(framework.UnschedulableAndUnresolvable, volumescheduling.ErrReasonBindConflict), + "machine2": framework.NewStatus(framework.UnschedulableAndUnresolvable, string(volumescheduling.ErrReasonNodeConflict)), + "machine3": framework.NewStatus(framework.UnschedulableAndUnresolvable, string(volumescheduling.ErrReasonBindConflict)), }, expected: map[string]bool{"machine4": true}, }, diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go index 2beb0287b00..1f8e254b94b 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go @@ -81,7 +81,7 @@ func (pl *VolumeBinding) Filter(ctx context.Context, cs *framework.CycleState, p if len(reasons) > 0 { status := framework.NewStatus(framework.UnschedulableAndUnresolvable) for _, reason := range reasons { - status.AppendReason(reason) + status.AppendReason(string(reason)) } return status } diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go index 5128e4eb564..c4f1631c794 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go @@ -67,18 +67,18 @@ func TestVolumeBinding(t *testing.T) { pod: &v1.Pod{Spec: volState}, node: &v1.Node{}, volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindReasons: []string{volumescheduling.ErrReasonBindConflict}, + FindReasons: []volumescheduling.ConflictReason{volumescheduling.ErrReasonBindConflict}, }, - wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, volumescheduling.ErrReasonBindConflict), + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, string(volumescheduling.ErrReasonBindConflict)), }, { name: "bound and unbound unsatisfied", pod: &v1.Pod{Spec: volState}, node: &v1.Node{}, volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindReasons: []string{volumescheduling.ErrReasonBindConflict, volumescheduling.ErrReasonNodeConflict}, + FindReasons: []volumescheduling.ConflictReason{volumescheduling.ErrReasonBindConflict, volumescheduling.ErrReasonNodeConflict}, }, - wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, volumescheduling.ErrReasonBindConflict, volumescheduling.ErrReasonNodeConflict), + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, string(volumescheduling.ErrReasonBindConflict), string(volumescheduling.ErrReasonNodeConflict)), }, { name: "unbound/found matches/bind succeeds", diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index ec39968faae..b21edc8c1ed 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -911,7 +911,7 @@ func TestSchedulerWithVolumeBinding(t *testing.T) { name: "bound/invalid pv affinity", volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ AllBound: true, - FindReasons: []string{volumescheduling.ErrReasonNodeConflict}, + FindReasons: volumescheduling.ConflictReasons{volumescheduling.ErrReasonNodeConflict}, }, eventReason: "FailedScheduling", expectError: makePredicateError("1 node(s) had volume node affinity conflict"), @@ -919,7 +919,7 @@ func TestSchedulerWithVolumeBinding(t *testing.T) { { name: "unbound/no matches", volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindReasons: []string{volumescheduling.ErrReasonBindConflict}, + FindReasons: volumescheduling.ConflictReasons{volumescheduling.ErrReasonBindConflict}, }, eventReason: "FailedScheduling", expectError: makePredicateError("1 node(s) didn't find available persistent volumes to bind"), @@ -927,7 +927,7 @@ func TestSchedulerWithVolumeBinding(t *testing.T) { { name: "bound and unbound unsatisfied", volumeBinderConfig: &volumescheduling.FakeVolumeBinderConfig{ - FindReasons: []string{volumescheduling.ErrReasonBindConflict, volumescheduling.ErrReasonNodeConflict}, + FindReasons: volumescheduling.ConflictReasons{volumescheduling.ErrReasonBindConflict, volumescheduling.ErrReasonNodeConflict}, }, eventReason: "FailedScheduling", expectError: makePredicateError("1 node(s) didn't find available persistent volumes to bind, 1 node(s) had volume node affinity conflict"), From 2e7ce8cea01890dc78e1e362093063f87d386520 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 25 Feb 2020 11:13:06 +0100 Subject: [PATCH 3/3] bazel update --- pkg/scheduler/core/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/scheduler/core/BUILD b/pkg/scheduler/core/BUILD index 51e543bdc29..baa45505b3f 100644 --- a/pkg/scheduler/core/BUILD +++ b/pkg/scheduler/core/BUILD @@ -44,6 +44,7 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/api/v1/pod:go_default_library", + "//pkg/controller/volume/scheduling:go_default_library", "//pkg/scheduler/apis/config:go_default_library", "//pkg/scheduler/framework/plugins/defaultbinder:go_default_library", "//pkg/scheduler/framework/plugins/defaultpodtopologyspread:go_default_library", @@ -56,7 +57,6 @@ go_test( "//pkg/scheduler/framework/plugins/podtopologyspread:go_default_library", "//pkg/scheduler/framework/plugins/queuesort:go_default_library", "//pkg/scheduler/framework/plugins/tainttoleration:go_default_library", - "//pkg/scheduler/framework/plugins/volumebinding:go_default_library", "//pkg/scheduler/framework/plugins/volumerestrictions:go_default_library", "//pkg/scheduler/framework/plugins/volumezone:go_default_library", "//pkg/scheduler/framework/v1alpha1:go_default_library",