From 07313a730defbd88be4e5dcacd152f53fcb26db3 Mon Sep 17 00:00:00 2001 From: vadasambar Date: Wed, 17 May 2023 15:57:50 +0530 Subject: [PATCH] test: add extra test cases for predicate checker - where the predicate checker uses custom scheduler config Signed-off-by: vadasambar --- .../filter_out_schedulable_test.go | 4 +- cluster-autoscaler/core/test/common.go | 2 +- .../estimator/binpacking_estimator_test.go | 2 +- .../mixed_nodeinfos_processor_test.go | 6 +- cluster-autoscaler/simulator/cluster_test.go | 2 +- .../predicatechecker/schedulerbased_test.go | 274 +++++++++++++++--- .../simulator/predicatechecker/testchecker.go | 17 +- .../scheduling/hinting_simulator_test.go | 4 +- 8 files changed, 250 insertions(+), 61 deletions(-) diff --git a/cluster-autoscaler/core/podlistprocessor/filter_out_schedulable_test.go b/cluster-autoscaler/core/podlistprocessor/filter_out_schedulable_test.go index 9252d2d25bf5..a5c98e93cbb5 100644 --- a/cluster-autoscaler/core/podlistprocessor/filter_out_schedulable_test.go +++ b/cluster-autoscaler/core/podlistprocessor/filter_out_schedulable_test.go @@ -156,7 +156,7 @@ func TestFilterOutSchedulable(t *testing.T) { for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { clusterSnapshot := clustersnapshot.NewBasicClusterSnapshot() - predicateChecker, err := predicatechecker.NewTestPredicateChecker() + predicateChecker, err := predicatechecker.NewTestPredicateChecker(nil) assert.NoError(t, err) var allExpectedScheduledPods []*apiv1.Pod @@ -262,7 +262,7 @@ func BenchmarkFilterOutSchedulable(b *testing.B) { } } - predicateChecker, err := predicatechecker.NewTestPredicateChecker() + predicateChecker, err := predicatechecker.NewTestPredicateChecker(nil) assert.NoError(b, err) clusterSnapshot := snapshotFactory() diff --git a/cluster-autoscaler/core/test/common.go b/cluster-autoscaler/core/test/common.go index 6a673d21a4a6..724579ff6ebc 100644 --- a/cluster-autoscaler/core/test/common.go +++ b/cluster-autoscaler/core/test/common.go @@ -170,7 +170,7 @@ func NewScaleTestAutoscalingContext( // Ignoring error here is safe - if a test doesn't specify valid estimatorName, // it either doesn't need one, or should fail when it turns out to be nil. estimatorBuilder, _ := estimator.NewEstimatorBuilder(options.EstimatorName, estimator.NewThresholdBasedEstimationLimiter(0, 0)) - predicateChecker, err := predicatechecker.NewTestPredicateChecker() + predicateChecker, err := predicatechecker.NewTestPredicateChecker(nil) if err != nil { return context.AutoscalingContext{}, err } diff --git a/cluster-autoscaler/estimator/binpacking_estimator_test.go b/cluster-autoscaler/estimator/binpacking_estimator_test.go index 3a6a961e6b50..d14e999de2e6 100644 --- a/cluster-autoscaler/estimator/binpacking_estimator_test.go +++ b/cluster-autoscaler/estimator/binpacking_estimator_test.go @@ -171,7 +171,7 @@ func TestBinpackingEstimate(t *testing.T) { // Add one node in different zone to trigger topology spread constraints clusterSnapshot.AddNode(makeNode(100, 100, "oldnode", "zone-jupiter")) - predicateChecker, err := predicatechecker.NewTestPredicateChecker() + predicateChecker, err := predicatechecker.NewTestPredicateChecker(nil) assert.NoError(t, err) limiter := NewThresholdBasedEstimationLimiter(tc.maxNodes, time.Duration(0)) estimator := NewBinpackingNodeEstimator(predicateChecker, clusterSnapshot, limiter) diff --git a/cluster-autoscaler/processors/nodeinfosprovider/mixed_nodeinfos_processor_test.go b/cluster-autoscaler/processors/nodeinfosprovider/mixed_nodeinfos_processor_test.go index a201480ecb6f..b05b73b353ca 100644 --- a/cluster-autoscaler/processors/nodeinfosprovider/mixed_nodeinfos_processor_test.go +++ b/cluster-autoscaler/processors/nodeinfosprovider/mixed_nodeinfos_processor_test.go @@ -76,7 +76,7 @@ func TestGetNodeInfosForGroups(t *testing.T) { podLister := kube_util.NewTestPodLister([]*apiv1.Pod{}) registry := kube_util.NewListerRegistry(nil, nil, podLister, nil, nil, nil, nil, nil, nil, nil) - predicateChecker, err := predicatechecker.NewTestPredicateChecker() + predicateChecker, err := predicatechecker.NewTestPredicateChecker(nil) assert.NoError(t, err) ctx := context.AutoscalingContext{ @@ -161,7 +161,7 @@ func TestGetNodeInfosForGroupsCache(t *testing.T) { podLister := kube_util.NewTestPodLister([]*apiv1.Pod{}) registry := kube_util.NewListerRegistry(nil, nil, podLister, nil, nil, nil, nil, nil, nil, nil) - predicateChecker, err := predicatechecker.NewTestPredicateChecker() + predicateChecker, err := predicatechecker.NewTestPredicateChecker(nil) assert.NoError(t, err) // Fill cache @@ -250,7 +250,7 @@ func TestGetNodeInfosCacheExpired(t *testing.T) { provider := testprovider.NewTestAutoprovisioningCloudProvider(nil, nil, nil, nil, nil, nil) podLister := kube_util.NewTestPodLister([]*apiv1.Pod{}) registry := kube_util.NewListerRegistry(nil, nil, podLister, nil, nil, nil, nil, nil, nil, nil) - predicateChecker, err := predicatechecker.NewTestPredicateChecker() + predicateChecker, err := predicatechecker.NewTestPredicateChecker(nil) assert.NoError(t, err) ctx := context.AutoscalingContext{ diff --git a/cluster-autoscaler/simulator/cluster_test.go b/cluster-autoscaler/simulator/cluster_test.go index 74cf2ff09f06..10b5a4ff0d2a 100644 --- a/cluster-autoscaler/simulator/cluster_test.go +++ b/cluster-autoscaler/simulator/cluster_test.go @@ -147,7 +147,7 @@ func TestFindNodesToRemove(t *testing.T) { } clusterSnapshot := clustersnapshot.NewBasicClusterSnapshot() - predicateChecker, err := predicatechecker.NewTestPredicateChecker() + predicateChecker, err := predicatechecker.NewTestPredicateChecker(nil) assert.NoError(t, err) tracker := NewUsageTracker() diff --git a/cluster-autoscaler/simulator/predicatechecker/schedulerbased_test.go b/cluster-autoscaler/simulator/predicatechecker/schedulerbased_test.go index 4405490880b8..b1b4914e70f6 100644 --- a/cluster-autoscaler/simulator/predicatechecker/schedulerbased_test.go +++ b/cluster-autoscaler/simulator/predicatechecker/schedulerbased_test.go @@ -17,10 +17,13 @@ limitations under the License. package predicatechecker import ( + "os" + "path/filepath" "testing" "time" "k8s.io/autoscaler/cluster-autoscaler/simulator/clustersnapshot" + scheduler "k8s.io/autoscaler/cluster-autoscaler/utils/scheduler" . "k8s.io/autoscaler/cluster-autoscaler/utils/test" "github.com/stretchr/testify/assert" @@ -36,52 +39,122 @@ func TestCheckPredicate(t *testing.T) { n1000 := BuildTestNode("n1000", 1000, 2000000) SetNodeReadyState(n1000, true, time.Time{}) + n1000Unschedulable := BuildTestNode("n1000", 1000, 2000000) + SetNodeReadyState(n1000Unschedulable, true, time.Time{}) + + defaultPredicateChecker, err := NewTestPredicateChecker(nil) + assert.NoError(t, err) + + // temp dir + tmpDir, err := os.MkdirTemp("", "scheduler-configs") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + customConfigFile := filepath.Join(tmpDir, "custom_config.yaml") + if err := os.WriteFile(customConfigFile, []byte(` +apiVersion: kubescheduler.config.k8s.io/v1 +kind: KubeSchedulerConfiguration +profiles: +- pluginConfig: + plugins: + multiPoint: + disabled: + - name: NodeResourcesFit + weight: 1 + schedulerName: custom-scheduler`), os.FileMode(0600)); err != nil { + t.Fatal(err) + } + + customConfig, err := scheduler.GetSchedulerConfig(customConfigFile) + assert.NoError(t, err) + customPredicateChecker, err := NewTestPredicateChecker(customConfig) + assert.NoError(t, err) tests := []struct { - name string - node *apiv1.Node - scheduledPods []*apiv1.Pod - testPod *apiv1.Pod - expectError bool + name string + node *apiv1.Node + scheduledPods []*apiv1.Pod + testPod *apiv1.Pod + predicateChecker PredicateChecker + expectError bool }{ + // default predicate checker test cases + { + name: "default - other pod - insuficient cpu", + node: n1000, + scheduledPods: []*apiv1.Pod{p450}, + testPod: p600, + expectError: true, + predicateChecker: defaultPredicateChecker, + }, + { + name: "default - other pod - ok", + node: n1000, + scheduledPods: []*apiv1.Pod{p450}, + testPod: p500, + expectError: false, + predicateChecker: defaultPredicateChecker, + }, + { + name: "default - empty - insuficient cpu", + node: n1000, + scheduledPods: []*apiv1.Pod{}, + testPod: p8000, + expectError: true, + predicateChecker: defaultPredicateChecker, + }, { - name: "other pod - insuficient cpu", - node: n1000, - scheduledPods: []*apiv1.Pod{p450}, - testPod: p600, - expectError: true, + name: "default - empty - ok", + node: n1000, + scheduledPods: []*apiv1.Pod{}, + testPod: p600, + expectError: false, + predicateChecker: defaultPredicateChecker, }, + // custom predicate checker test cases { - name: "other pod - ok", - node: n1000, - scheduledPods: []*apiv1.Pod{p450}, - testPod: p500, - expectError: false, + name: "custom - other pod - ok", + node: n1000, + scheduledPods: []*apiv1.Pod{p450}, + testPod: p600, + expectError: false, + predicateChecker: customPredicateChecker, }, { - name: "empty - insuficient cpu", - node: n1000, - scheduledPods: []*apiv1.Pod{}, - testPod: p8000, - expectError: true, + name: "custom -other pod - ok", + node: n1000, + scheduledPods: []*apiv1.Pod{p450}, + testPod: p500, + expectError: false, + predicateChecker: customPredicateChecker, }, { - name: "empty - ok", - node: n1000, - scheduledPods: []*apiv1.Pod{}, - testPod: p600, - expectError: false, + name: "custom -empty - ok", + node: n1000, + scheduledPods: []*apiv1.Pod{}, + testPod: p8000, + expectError: false, + predicateChecker: customPredicateChecker, + }, + { + name: "custom -empty - ok", + node: n1000, + scheduledPods: []*apiv1.Pod{}, + testPod: p600, + expectError: false, + predicateChecker: customPredicateChecker, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var err error - predicateChecker, err := NewTestPredicateChecker() clusterSnapshot := clustersnapshot.NewBasicClusterSnapshot() err = clusterSnapshot.AddNodeWithPods(tt.node, tt.scheduledPods) assert.NoError(t, err) - predicateError := predicateChecker.CheckPredicates(clusterSnapshot, tt.testPod, tt.node.Name) + predicateError := tt.predicateChecker.CheckPredicates(clusterSnapshot, tt.testPod, tt.node.Name) if tt.expectError { assert.NotNil(t, predicateError) assert.Equal(t, NotSchedulablePredicateError, predicateError.ErrorType()) @@ -102,27 +175,107 @@ func TestFitsAnyNode(t *testing.T) { n1000 := BuildTestNode("n1000", 1000, 2000000) n2000 := BuildTestNode("n2000", 2000, 2000000) - var err error + defaultPredicateChecker, err := NewTestPredicateChecker(nil) + assert.NoError(t, err) - clusterSnapshot := clustersnapshot.NewBasicClusterSnapshot() - err = clusterSnapshot.AddNode(n1000) + // temp dir + tmpDir, err := os.MkdirTemp("", "scheduler-configs") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + customConfigFile := filepath.Join(tmpDir, "custom_config.yaml") + if err := os.WriteFile(customConfigFile, []byte(` +apiVersion: kubescheduler.config.k8s.io/v1 +kind: KubeSchedulerConfiguration +profiles: +- pluginConfig: + plugins: + multiPoint: + disabled: + - name: NodeResourcesFit + weight: 1 + schedulerName: custom-scheduler`), os.FileMode(0600)); err != nil { + t.Fatal(err) + } + + customConfig, err := scheduler.GetSchedulerConfig(customConfigFile) assert.NoError(t, err) - err = clusterSnapshot.AddNode(n2000) + customPredicateChecker, err := NewTestPredicateChecker(customConfig) assert.NoError(t, err) - predicateChecker, err := NewTestPredicateChecker() - assert.NoError(t, err) + testCases := []struct { + name string + predicateChecker PredicateChecker + pod *apiv1.Pod + expectedNodes []string + expectError bool + }{ + // default predicate checker test cases + { + name: "default - small pod - no error", + predicateChecker: defaultPredicateChecker, + pod: p900, + expectedNodes: []string{"n1000", "n2000"}, + expectError: false, + }, + { + name: "default - medium pod - no error", + predicateChecker: defaultPredicateChecker, + pod: p1900, + expectedNodes: []string{"n2000"}, + expectError: false, + }, + { + name: "default - large pod - insufficient cpu", + predicateChecker: defaultPredicateChecker, + pod: p2100, + expectError: true, + }, - nodeName, err := predicateChecker.FitsAnyNode(clusterSnapshot, p900) - assert.NoError(t, err) - assert.True(t, nodeName == "n1000" || nodeName == "n2000") + // custom predicate checker test cases + { + name: "custom - small pod - no error", + predicateChecker: customPredicateChecker, + pod: p900, + expectedNodes: []string{"n1000", "n2000"}, + expectError: false, + }, + { + name: "custom - medium pod - no error", + predicateChecker: customPredicateChecker, + pod: p1900, + expectedNodes: []string{"n1000", "n2000"}, + expectError: false, + }, + { + name: "custom - large pod - insufficient cpu", + predicateChecker: customPredicateChecker, + pod: p2100, + expectedNodes: []string{"n1000", "n2000"}, + expectError: false, + }, + } - nodeName, err = predicateChecker.FitsAnyNode(clusterSnapshot, p1900) + clusterSnapshot := clustersnapshot.NewBasicClusterSnapshot() + err = clusterSnapshot.AddNode(n1000) assert.NoError(t, err) - assert.Equal(t, "n2000", nodeName) + err = clusterSnapshot.AddNode(n2000) + assert.NoError(t, err) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + nodeName, err := tc.predicateChecker.FitsAnyNode(clusterSnapshot, tc.pod) + if tc.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Contains(t, tc.expectedNodes, nodeName) + } + }) + } - nodeName, err = predicateChecker.FitsAnyNode(clusterSnapshot, p2100) - assert.Error(t, err) } func TestDebugInfo(t *testing.T) { @@ -142,16 +295,47 @@ func TestDebugInfo(t *testing.T) { } SetNodeReadyState(node1, true, time.Time{}) - predicateChecker, err := NewTestPredicateChecker() - assert.NoError(t, err) - clusterSnapshot := clustersnapshot.NewBasicClusterSnapshot() - err = clusterSnapshot.AddNode(node1) + err := clusterSnapshot.AddNode(node1) assert.NoError(t, err) - predicateErr := predicateChecker.CheckPredicates(clusterSnapshot, p1, "n1") + // with default predicate checker + defaultPredicateChecker, err := NewTestPredicateChecker(nil) + assert.NoError(t, err) + predicateErr := defaultPredicateChecker.CheckPredicates(clusterSnapshot, p1, "n1") assert.NotNil(t, predicateErr) assert.Equal(t, "node(s) had untolerated taint {SomeTaint: WhyNot?}", predicateErr.Message()) assert.Contains(t, predicateErr.VerboseMessage(), "RandomTaint") + + // with custom predicate checker + + // temp dir + tmpDir, err := os.MkdirTemp("", "scheduler-configs") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + customConfigFile := filepath.Join(tmpDir, "custom_config.yaml") + if err := os.WriteFile(customConfigFile, []byte(` +apiVersion: kubescheduler.config.k8s.io/v1 +kind: KubeSchedulerConfiguration +profiles: +- pluginConfig: + plugins: + multiPoint: + disabled: + - name: TaintToleration + weight: 1 + schedulerName: custom-scheduler`), os.FileMode(0600)); err != nil { + t.Fatal(err) + } + + customConfig, err := scheduler.GetSchedulerConfig(customConfigFile) + assert.NoError(t, err) + customPredicateChecker, err := NewTestPredicateChecker(customConfig) + assert.NoError(t, err) + predicateErr = customPredicateChecker.CheckPredicates(clusterSnapshot, p1, "n1") + assert.Nil(t, predicateErr) } diff --git a/cluster-autoscaler/simulator/predicatechecker/testchecker.go b/cluster-autoscaler/simulator/predicatechecker/testchecker.go index 399dd80c2e10..a559f6c0c3dc 100644 --- a/cluster-autoscaler/simulator/predicatechecker/testchecker.go +++ b/cluster-autoscaler/simulator/predicatechecker/testchecker.go @@ -18,15 +18,20 @@ package predicatechecker import ( clientsetfake "k8s.io/client-go/kubernetes/fake" - scheduler_config "k8s.io/kubernetes/pkg/scheduler/apis/config/latest" + "k8s.io/kubernetes/pkg/scheduler/apis/config" + scheduler_config_latest "k8s.io/kubernetes/pkg/scheduler/apis/config/latest" ) // NewTestPredicateChecker builds test version of PredicateChecker. -func NewTestPredicateChecker() (PredicateChecker, error) { - cfg, err := scheduler_config.Default() - if err != nil { - return nil, err +func NewTestPredicateChecker(schedConfig *config.KubeSchedulerConfiguration) (PredicateChecker, error) { + if schedConfig == nil { + var err error + schedConfig, err = scheduler_config_latest.Default() + if err != nil { + return nil, err + } } + // just call out to NewSchedulerBasedPredicateChecker but use fake kubeClient - return NewSchedulerBasedPredicateChecker(clientsetfake.NewSimpleClientset(), cfg, make(chan struct{})) + return NewSchedulerBasedPredicateChecker(clientsetfake.NewSimpleClientset(), schedConfig, make(chan struct{})) } diff --git a/cluster-autoscaler/simulator/scheduling/hinting_simulator_test.go b/cluster-autoscaler/simulator/scheduling/hinting_simulator_test.go index 3c5ad37bc73a..f3e8c504424e 100644 --- a/cluster-autoscaler/simulator/scheduling/hinting_simulator_test.go +++ b/cluster-autoscaler/simulator/scheduling/hinting_simulator_test.go @@ -134,7 +134,7 @@ func TestTrySchedulePods(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { t.Parallel() clusterSnapshot := clustersnapshot.NewBasicClusterSnapshot() - predicateChecker, err := predicatechecker.NewTestPredicateChecker() + predicateChecker, err := predicatechecker.NewTestPredicateChecker(nil) assert.NoError(t, err) clustersnapshot.InitializeClusterSnapshotOrDie(t, clusterSnapshot, tc.nodes, tc.pods) s := NewHintingSimulator(predicateChecker) @@ -211,7 +211,7 @@ func TestPodSchedulesOnHintedNode(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { t.Parallel() clusterSnapshot := clustersnapshot.NewBasicClusterSnapshot() - predicateChecker, err := predicatechecker.NewTestPredicateChecker() + predicateChecker, err := predicatechecker.NewTestPredicateChecker(nil) assert.NoError(t, err) nodes := make([]*apiv1.Node, 0, len(tc.nodeNames)) for _, n := range tc.nodeNames {