diff --git a/pkg/scheduler/framework/plugins/noderesources/fit_test.go b/pkg/scheduler/framework/plugins/noderesources/fit_test.go index 8a10cb3755a..19589435e4d 100644 --- a/pkg/scheduler/framework/plugins/noderesources/fit_test.go +++ b/pkg/scheduler/framework/plugins/noderesources/fit_test.go @@ -675,19 +675,29 @@ func TestRestartableInitContainers(t *testing.T) { }, } } - newPodWithRestartableInitContainers := func() *v1.Pod { + newPodWithRestartableInitContainers := func(request, sidecarRequest *v1.ResourceList) *v1.Pod { restartPolicyAlways := v1.ContainerRestartPolicyAlways + + container := v1.Container{Name: "regular"} + if request != nil { + container.Resources = v1.ResourceRequirements{ + Requests: *request, + } + } + + sidecarContainer := v1.Container{ + Name: "restartable-init", + RestartPolicy: &restartPolicyAlways, + } + if sidecarRequest != nil { + sidecarContainer.Resources = v1.ResourceRequirements{ + Requests: *sidecarRequest, + } + } return &v1.Pod{ Spec: v1.PodSpec{ - Containers: []v1.Container{ - {Name: "regular"}, - }, - InitContainers: []v1.Container{ - { - Name: "restartable-init", - RestartPolicy: &restartPolicyAlways, - }, - }, + Containers: []v1.Container{container}, + InitContainers: []v1.Container{sidecarContainer}, }, } } @@ -697,6 +707,7 @@ func TestRestartableInitContainers(t *testing.T) { pod *v1.Pod enableSidecarContainers bool wantPreFilterStatus *framework.Status + wantFilterStatus *framework.Status }{ { name: "allow pod without restartable init containers if sidecar containers is disabled", @@ -704,7 +715,7 @@ func TestRestartableInitContainers(t *testing.T) { }, { name: "not allow pod with restartable init containers if sidecar containers is disabled", - pod: newPodWithRestartableInitContainers(), + pod: newPodWithRestartableInitContainers(nil, nil), wantPreFilterStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, "Pod has a restartable init container and the SidecarContainers feature is disabled"), }, { @@ -715,7 +726,24 @@ func TestRestartableInitContainers(t *testing.T) { { name: "allow pod with restartable init containers if sidecar containers is enabled", enableSidecarContainers: true, - pod: newPodWithRestartableInitContainers(), + pod: newPodWithRestartableInitContainers(nil, nil), + }, + { + name: "allow pod if the total requested resources do not exceed the node's allocatable resources", + enableSidecarContainers: true, + pod: newPodWithRestartableInitContainers( + &v1.ResourceList{v1.ResourceCPU: *resource.NewMilliQuantity(1, resource.DecimalSI)}, + &v1.ResourceList{v1.ResourceCPU: *resource.NewMilliQuantity(1, resource.DecimalSI)}, + ), + }, + { + name: "not allow pod if the total requested resources do exceed the node's allocatable resources", + enableSidecarContainers: true, + pod: newPodWithRestartableInitContainers( + &v1.ResourceList{v1.ResourceCPU: *resource.NewMilliQuantity(1, resource.DecimalSI)}, + &v1.ResourceList{v1.ResourceCPU: *resource.NewMilliQuantity(2, resource.DecimalSI)}, + ), + wantFilterStatus: framework.NewStatus(framework.Unschedulable, "Insufficient cpu"), }, } @@ -724,7 +752,7 @@ func TestRestartableInitContainers(t *testing.T) { _, ctx := ktesting.NewTestContext(t) ctx, cancel := context.WithCancel(ctx) defer cancel() - node := v1.Node{Status: v1.NodeStatus{Capacity: v1.ResourceList{}, Allocatable: makeAllocatableResources(0, 0, 1, 0, 0, 0)}} + node := v1.Node{Status: v1.NodeStatus{Capacity: v1.ResourceList{}, Allocatable: makeAllocatableResources(2, 0, 1, 0, 0, 0)}} nodeInfo := framework.NewNodeInfo() nodeInfo.SetNode(&node) @@ -735,15 +763,15 @@ func TestRestartableInitContainers(t *testing.T) { cycleState := framework.NewCycleState() _, preFilterStatus := p.(framework.PreFilterPlugin).PreFilter(ctx, cycleState, test.pod) if diff := cmp.Diff(test.wantPreFilterStatus, preFilterStatus); diff != "" { - t.Error("status does not match (-expected +actual):\n", diff) + t.Error("prefilter status does not match (-expected +actual):\n", diff) } if !preFilterStatus.IsSuccess() { return } filterStatus := p.(framework.FilterPlugin).Filter(ctx, cycleState, test.pod, nodeInfo) - if !filterStatus.IsSuccess() { - t.Error("status does not match (-expected +actual):\n- Success\n +\n", filterStatus.Code()) + if diff := cmp.Diff(test.wantFilterStatus, filterStatus); diff != "" { + t.Error("filter status does not match (-expected +actual):\n", diff) } }) } @@ -932,6 +960,52 @@ func TestFitScore(t *testing.T) { }, runPreScore: false, }, + { + name: "test case for ScoringStrategy MostAllocated with sidecar container", + requestedPod: st.MakePod(). + Req(map[v1.ResourceName]string{"cpu": "1000", "memory": "2000"}). + Obj(), + nodes: []*v1.Node{ + st.MakeNode().Name("node1").Capacity(map[v1.ResourceName]string{"cpu": "4000", "memory": "10000"}).Obj(), + st.MakeNode().Name("node2").Capacity(map[v1.ResourceName]string{"cpu": "4000", "memory": "10000"}).Obj(), + }, + existingPods: []*v1.Pod{ + st.MakePod().Node("node1").Req(map[v1.ResourceName]string{"cpu": "1000", "memory": "2000"}). + SidecarReq(map[v1.ResourceName]string{"cpu": "1000", "memory": "2000"}).Obj(), + st.MakePod().Node("node2").Req(map[v1.ResourceName]string{"cpu": "1000", "memory": "2000"}).Obj(), + }, + expectedPriorities: []framework.NodeScore{{Name: "node1", Score: 67}, {Name: "node2", Score: 45}}, + nodeResourcesFitArgs: config.NodeResourcesFitArgs{ + ScoringStrategy: &config.ScoringStrategy{ + Type: config.MostAllocated, + Resources: defaultResources, + }, + }, + runPreScore: true, + }, + { + name: "test case for ScoringStrategy LeastAllocated with sidecar container", + requestedPod: st.MakePod(). + Req(map[v1.ResourceName]string{"cpu": "1000", "memory": "2000"}). + Obj(), + nodes: []*v1.Node{ + st.MakeNode().Name("node1").Capacity(map[v1.ResourceName]string{"cpu": "4000", "memory": "10000"}).Obj(), + st.MakeNode().Name("node2").Capacity(map[v1.ResourceName]string{"cpu": "4000", "memory": "10000"}).Obj(), + }, + existingPods: []*v1.Pod{ + st.MakePod().Node("node1").Req(map[v1.ResourceName]string{"cpu": "1000", "memory": "2000"}). + SidecarReq(map[v1.ResourceName]string{"cpu": "1000", "memory": "2000"}).Obj(), + st.MakePod().Node("node2").Req(map[v1.ResourceName]string{"cpu": "1000", "memory": "2000"}).Obj(), + }, + expectedPriorities: []framework.NodeScore{{Name: "node1", Score: 32}, {Name: "node2", Score: 55}}, + nodeResourcesFitArgs: config.NodeResourcesFitArgs{ + ScoringStrategy: &config.ScoringStrategy{ + Type: config.LeastAllocated, + Resources: defaultResources, + }, + }, + runPreScore: true, + }, } for _, test := range tests { diff --git a/pkg/scheduler/testing/wrappers.go b/pkg/scheduler/testing/wrappers.go index 15d331aaeb9..c0ab9b491ec 100644 --- a/pkg/scheduler/testing/wrappers.go +++ b/pkg/scheduler/testing/wrappers.go @@ -224,6 +224,12 @@ func (c *ContainerWrapper) ResourceLimits(limMap map[v1.ResourceName]string) *Co return c } +// RestartPolicy sets the container's restartPolicy to the given restartPolicy. +func (c *ContainerWrapper) RestartPolicy(restartPolicy v1.ContainerRestartPolicy) *ContainerWrapper { + c.Container.RestartPolicy = &restartPolicy + return c +} + // PodWrapper wraps a Pod inside. type PodWrapper struct{ v1.Pod } @@ -701,6 +707,17 @@ func (p *PodWrapper) InitReq(resMap map[v1.ResourceName]string) *PodWrapper { return p } +// SidecarReq adds a new sidecar container to the inner pod with given resource map. +func (p *PodWrapper) SidecarReq(resMap map[v1.ResourceName]string) *PodWrapper { + if len(resMap) == 0 { + return p + } + + name := fmt.Sprintf("sidecar-con%d", len(p.Spec.InitContainers)) + p.Spec.InitContainers = append(p.Spec.InitContainers, MakeContainer().Name(name).Image(imageutils.GetPauseImageName()).RestartPolicy(v1.ContainerRestartPolicyAlways).Resources(resMap).Obj()) + return p +} + // PreemptionPolicy sets the give preemption policy to the inner pod. func (p *PodWrapper) PreemptionPolicy(policy v1.PreemptionPolicy) *PodWrapper { p.Spec.PreemptionPolicy = &policy diff --git a/test/integration/scheduler/scoring/priorities_test.go b/test/integration/scheduler/scoring/priorities_test.go index cd273a5a62c..c4a92cf3fb8 100644 --- a/test/integration/scheduler/scoring/priorities_test.go +++ b/test/integration/scheduler/scoring/priorities_test.go @@ -108,20 +108,17 @@ func initTestSchedulerForPriorityTest(t *testing.T, preScorePluginName, scorePlu return testCtx } -func initTestSchedulerForNodeResourcesTest(t *testing.T) *testutils.TestContext { +func initTestSchedulerForNodeResourcesTest(t *testing.T, strategy configv1.ScoringStrategyType) *testutils.TestContext { cfg := configtesting.V1ToInternalWithDefaults(t, configv1.KubeSchedulerConfiguration{ Profiles: []configv1.KubeSchedulerProfile{ { SchedulerName: pointer.String(v1.DefaultSchedulerName), - }, - { - SchedulerName: pointer.String("gpu-binpacking-scheduler"), PluginConfig: []configv1.PluginConfig{ { Name: noderesources.Name, Args: runtime.RawExtension{Object: &configv1.NodeResourcesFitArgs{ ScoringStrategy: &configv1.ScoringStrategy{ - Type: configv1.MostAllocated, + Type: strategy, Resources: []configv1.ResourceSpec{ {Name: string(v1.ResourceCPU), Weight: 1}, {Name: string(v1.ResourceMemory), Weight: 1}, @@ -147,63 +144,220 @@ func initTestSchedulerForNodeResourcesTest(t *testing.T) *testutils.TestContext // TestNodeResourcesScoring verifies that scheduler's node resources priority function // works correctly. func TestNodeResourcesScoring(t *testing.T) { - testCtx := initTestSchedulerForNodeResourcesTest(t) - // Add a few nodes. - _, err := createAndWaitForNodesInCache(testCtx, "testnode", st.MakeNode().Capacity( - map[v1.ResourceName]string{ - v1.ResourceCPU: "8", - v1.ResourceMemory: "16G", - resourceGPU: "4", - }), 2) - if err != nil { - t.Fatal(err) - } - cpuBoundPod1, err := runPausePod(testCtx.ClientSet, st.MakePod().Namespace(testCtx.NS.Name).Name("cpubound1").Res( - map[v1.ResourceName]string{ - v1.ResourceCPU: "2", - v1.ResourceMemory: "4G", - resourceGPU: "1", + tests := []struct { + name string + pod func(testCtx *testutils.TestContext) *v1.Pod + existingPods func(testCtx *testutils.TestContext) []*v1.Pod + nodes []*v1.Node + strategy configv1.ScoringStrategyType + // expectedNodeName is the list of node names. The pod should be scheduled on either of them. + expectedNodeName []string + }{ + { + name: "with least allocated strategy, pod scheduled to node with more allocatable resources", + pod: func(testCtx *testutils.TestContext) *v1.Pod { + return st.MakePod().Namespace(testCtx.NS.Name).Name("pod"). + Res(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "4G", + resourceGPU: "1", + }).Obj() + }, + existingPods: func(testCtx *testutils.TestContext) []*v1.Pod { + return []*v1.Pod{ + st.MakePod().Namespace(testCtx.NS.Name).Name("existing-pod").Node("node-1"). + Res(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "4G", + resourceGPU: "1", + }).Obj(), + } + }, + nodes: []*v1.Node{ + st.MakeNode().Name("node-1").Capacity( + map[v1.ResourceName]string{ + v1.ResourceCPU: "8", + v1.ResourceMemory: "16G", + resourceGPU: "4", + }).Obj(), + st.MakeNode().Name("node-2").Capacity( + map[v1.ResourceName]string{ + v1.ResourceCPU: "8", + v1.ResourceMemory: "16G", + resourceGPU: "4", + }).Obj(), + }, + strategy: configv1.LeastAllocated, + expectedNodeName: []string{"node-2"}, }, - ).Obj()) - if err != nil { - t.Fatal(err) - } - gpuBoundPod1, err := runPausePod(testCtx.ClientSet, st.MakePod().Namespace(testCtx.NS.Name).Name("gpubound1").Res( - map[v1.ResourceName]string{ - v1.ResourceCPU: "1", - v1.ResourceMemory: "2G", - resourceGPU: "2", + { + name: "with most allocated strategy, pod scheduled to node with less allocatable resources", + pod: func(testCtx *testutils.TestContext) *v1.Pod { + return st.MakePod().Namespace(testCtx.NS.Name).Name("pod"). + Res(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "4G", + resourceGPU: "1", + }).Obj() + }, + existingPods: func(testCtx *testutils.TestContext) []*v1.Pod { + return []*v1.Pod{ + st.MakePod().Namespace(testCtx.NS.Name).Name("existing-pod").Node("node-1"). + Res(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "4G", + resourceGPU: "1", + }).Obj(), + } + }, + nodes: []*v1.Node{ + st.MakeNode().Name("node-1").Capacity( + map[v1.ResourceName]string{ + v1.ResourceCPU: "8", + v1.ResourceMemory: "16G", + resourceGPU: "4", + }).Obj(), + st.MakeNode().Name("node-2").Capacity( + map[v1.ResourceName]string{ + v1.ResourceCPU: "8", + v1.ResourceMemory: "16G", + resourceGPU: "4", + }).Obj(), + }, + strategy: configv1.MostAllocated, + expectedNodeName: []string{"node-1"}, + }, + { + name: "with least allocated strategy, take existing sidecars into consideration", + pod: func(testCtx *testutils.TestContext) *v1.Pod { + return st.MakePod().Namespace(testCtx.NS.Name).Name("pod"). + Res(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "4G", + resourceGPU: "1", + }).Obj() + }, + existingPods: func(testCtx *testutils.TestContext) []*v1.Pod { + return []*v1.Pod{ + st.MakePod().Namespace(testCtx.NS.Name).Name("existing-pod-1").Node("node-1"). + Res(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "4G", + resourceGPU: "1", + }). + SidecarReq(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "2G", + }). + Obj(), + st.MakePod().Namespace(testCtx.NS.Name).Name("existing-pod-2").Node("node-2"). + Res(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "4G", + resourceGPU: "1", + }).Obj(), + } + }, + nodes: []*v1.Node{ + st.MakeNode().Name("node-1").Capacity( + map[v1.ResourceName]string{ + v1.ResourceCPU: "8", + v1.ResourceMemory: "16G", + resourceGPU: "4", + }).Obj(), + st.MakeNode().Name("node-2").Capacity( + map[v1.ResourceName]string{ + v1.ResourceCPU: "8", + v1.ResourceMemory: "16G", + resourceGPU: "4", + }).Obj(), + }, + strategy: configv1.LeastAllocated, + expectedNodeName: []string{"node-2"}, + }, + { + name: "with most allocated strategy, take existing sidecars into consideration", + pod: func(testCtx *testutils.TestContext) *v1.Pod { + return st.MakePod().Namespace(testCtx.NS.Name).Name("pod"). + Res(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "4G", + resourceGPU: "1", + }).Obj() + }, + existingPods: func(testCtx *testutils.TestContext) []*v1.Pod { + return []*v1.Pod{ + st.MakePod().Namespace(testCtx.NS.Name).Name("existing-pod-1").Node("node-1"). + Res(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "4G", + resourceGPU: "1", + }). + SidecarReq(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "2G", + }). + Obj(), + st.MakePod().Namespace(testCtx.NS.Name).Name("existing-pod-2").Node("node-2"). + Res(map[v1.ResourceName]string{ + v1.ResourceCPU: "2", + v1.ResourceMemory: "4G", + resourceGPU: "1", + }).Obj(), + } + }, + nodes: []*v1.Node{ + st.MakeNode().Name("node-1").Capacity( + map[v1.ResourceName]string{ + v1.ResourceCPU: "8", + v1.ResourceMemory: "16G", + resourceGPU: "4", + }).Obj(), + st.MakeNode().Name("node-2").Capacity( + map[v1.ResourceName]string{ + v1.ResourceCPU: "8", + v1.ResourceMemory: "16G", + resourceGPU: "4", + }).Obj(), + }, + strategy: configv1.MostAllocated, + expectedNodeName: []string{"node-1"}, }, - ).Obj()) - if err != nil { - t.Fatal(err) - } - if cpuBoundPod1.Spec.NodeName == "" || gpuBoundPod1.Spec.NodeName == "" { - t.Fatalf("pods should have nodeName assigned, got %q and %q", - cpuBoundPod1.Spec.NodeName, gpuBoundPod1.Spec.NodeName) } - // Since both pods used the default scheduler, then they should land on two different - // nodes because the default configuration uses LeastAllocated. - if cpuBoundPod1.Spec.NodeName == gpuBoundPod1.Spec.NodeName { - t.Fatalf("pods should have landed on different nodes, both scheduled on %q", - cpuBoundPod1.Spec.NodeName) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SidecarContainers, true) + testCtx := initTestSchedulerForNodeResourcesTest(t, tt.strategy) - // The following pod is using the gpu-binpacking-scheduler profile, which gives a higher weight to - // GPU-based binpacking, and so it should land on the node with higher GPU utilization. - cpuBoundPod2, err := runPausePod(testCtx.ClientSet, st.MakePod().Namespace(testCtx.NS.Name).Name("cpubound2").SchedulerName("gpu-binpacking-scheduler").Res( - map[v1.ResourceName]string{ - v1.ResourceCPU: "2", - v1.ResourceMemory: "4G", - resourceGPU: "1", - }, - ).Obj()) - if err != nil { - t.Fatal(err) - } - if cpuBoundPod2.Spec.NodeName != gpuBoundPod1.Spec.NodeName { - t.Errorf("pods should have landed on the same node") + for _, n := range tt.nodes { + if _, err := createNode(testCtx.ClientSet, n); err != nil { + t.Fatalf("failed to create node: %v", err) + } + } + + if err := testutils.WaitForNodesInCache(testCtx.Ctx, testCtx.Scheduler, len(tt.nodes)); err != nil { + t.Fatalf("failed to wait for nodes in cache: %v", err) + } + + if tt.existingPods != nil { + for _, p := range tt.existingPods(testCtx) { + if _, err := runPausePod(testCtx.ClientSet, p); err != nil { + t.Fatalf("failed to create existing pod: %v", err) + } + } + } + + pod, err := runPausePod(testCtx.ClientSet, tt.pod(testCtx)) + if err != nil { + t.Fatalf("Error running pause pod: %v", err) + } + + err = wait.PollUntilContextTimeout(testCtx.Ctx, pollInterval, wait.ForeverTestTimeout, false, podScheduledIn(testCtx.ClientSet, pod.Namespace, pod.Name, tt.expectedNodeName)) + if err != nil { + t.Errorf("Error while trying to wait for a pod to be scheduled: %v", err) + } + }) } }