From 84915d162390ccedbef54176a9097c02832adfd1 Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Fri, 15 May 2020 16:05:04 +0800 Subject: [PATCH 1/2] Expose the issue that max threshold haven't considered container size Signed-off-by: Dave Chen --- .../imagelocality/image_locality_test.go | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/pkg/scheduler/framework/plugins/imagelocality/image_locality_test.go b/pkg/scheduler/framework/plugins/imagelocality/image_locality_test.go index a6818a3dd5b..c7c92cd82d9 100644 --- a/pkg/scheduler/framework/plugins/imagelocality/image_locality_test.go +++ b/pkg/scheduler/framework/plugins/imagelocality/image_locality_test.go @@ -64,6 +64,20 @@ func TestImageLocalityPriority(t *testing.T) { }, } + test300600900 := v1.PodSpec{ + Containers: []v1.Container{ + { + Image: "gcr.io/300", + }, + { + Image: "gcr.io/600", + }, + { + Image: "gcr.io/900", + }, + }, + } + node403002000 := v1.NodeStatus{ Images: []v1.ContainerImage{ { @@ -108,6 +122,52 @@ func TestImageLocalityPriority(t *testing.T) { }, } + node60040900 := v1.NodeStatus{ + Images: []v1.ContainerImage{ + { + Names: []string{ + "gcr.io/600:" + parsers.DefaultImageTag, + }, + SizeBytes: int64(600 * mb), + }, + { + Names: []string{ + "gcr.io/40:" + parsers.DefaultImageTag, + }, + SizeBytes: int64(40 * mb), + }, + { + Names: []string{ + "gcr.io/900:" + parsers.DefaultImageTag, + }, + SizeBytes: int64(900 * mb), + }, + }, + } + + node300600900 := v1.NodeStatus{ + Images: []v1.ContainerImage{ + { + Names: []string{ + "gcr.io/300:" + parsers.DefaultImageTag, + }, + SizeBytes: int64(300 * mb), + }, + { + Names: []string{ + "gcr.io/600:" + parsers.DefaultImageTag, + }, + SizeBytes: int64(600 * mb), + }, + { + Names: []string{ + "gcr.io/900:" + parsers.DefaultImageTag, + }, + SizeBytes: int64(900 * mb), + }, + }, + } + nodeWithNoImages := v1.NodeStatus{} tests := []struct { @@ -181,6 +241,25 @@ func TestImageLocalityPriority(t *testing.T) { expectedList: []framework.NodeScore{{Name: "machine1", Score: 65}, {Name: "machine2", Score: 0}, {Name: "machine3", Score: 0}}, name: "if exceed limit, use limit (with node which has no images present)", }, + { + // Pod: gcr.io/300 gcr.io/600 gcr.io/900 + + // Node1 + // Image: gcr.io/600:latest 600MB, gcr.io/900:latest 900MB + // Score: 100 (600M * 2/3 + 900M * 2/3 = 1000M >= 1000M, max-threshold) + + // Node2 + // Image: gcr.io/300:latest 300MB, gcr.io/600:latest 600MB, gcr.io/900:latest 900MB + // Score: 100 (300M * 1/3 + 600M * 2/3 + 900M * 2/3) >= 1000M, max-threshold) + + // Node3 + // Image: + // Score: 0 + pod: &v1.Pod{Spec: test300600900}, + nodes: []*v1.Node{makeImageNode("machine1", node60040900), makeImageNode("machine2", node300600900), makeImageNode("machine3", nodeWithNoImages)}, + expectedList: []framework.NodeScore{{Name: "machine1", Score: 100}, {Name: "machine2", Score: 100}, {Name: "machine3", Score: 0}}, + name: "max threshold haven't considered the size of containers to be run for the pod", + }, } for _, test := range tests { From 42fbb1d72f64362887dbc210aefe0fb081979335 Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Fri, 15 May 2020 18:30:57 +0800 Subject: [PATCH 2/2] Define the thresholds per the size of container images Given the assumption that 90% of images on dockerhub drops into this range (23~1000)MB, this assumption is based on the container images instead of the pod. pod might hold multiple container images, it's better to multiply the assumption by the size of container images. Signed-off-by: Dave Chen --- .../plugins/imagelocality/image_locality.go | 11 +- .../imagelocality/image_locality_test.go | 116 ++++++++++++++---- 2 files changed, 97 insertions(+), 30 deletions(-) diff --git a/pkg/scheduler/framework/plugins/imagelocality/image_locality.go b/pkg/scheduler/framework/plugins/imagelocality/image_locality.go index 3b2990c602f..4eb61c7ad1b 100644 --- a/pkg/scheduler/framework/plugins/imagelocality/image_locality.go +++ b/pkg/scheduler/framework/plugins/imagelocality/image_locality.go @@ -29,9 +29,9 @@ import ( // The two thresholds are used as bounds for the image score range. They correspond to a reasonable size range for // container images compressed and stored in registries; 90%ile of images on dockerhub drops into this range. const ( - mb int64 = 1024 * 1024 - minThreshold int64 = 23 * mb - maxThreshold int64 = 1000 * mb + mb int64 = 1024 * 1024 + minThreshold int64 = 23 * mb + maxContainerThreshold int64 = 1000 * mb ) // ImageLocality is a score plugin that favors nodes that already have requested pod container's images. @@ -62,7 +62,7 @@ func (pl *ImageLocality) Score(ctx context.Context, state *framework.CycleState, } totalNumNodes := len(nodeInfos) - score := calculatePriority(sumImageScores(nodeInfo, pod.Spec.Containers, totalNumNodes)) + score := calculatePriority(sumImageScores(nodeInfo, pod.Spec.Containers, totalNumNodes), len(pod.Spec.Containers)) return score, nil } @@ -79,7 +79,8 @@ func New(_ runtime.Object, h framework.FrameworkHandle) (framework.Plugin, error // calculatePriority returns the priority of a node. Given the sumScores of requested images on the node, the node's // priority is obtained by scaling the maximum priority value with a ratio proportional to the sumScores. -func calculatePriority(sumScores int64) int64 { +func calculatePriority(sumScores int64, numContainers int) int64 { + maxThreshold := maxContainerThreshold * int64(numContainers) if sumScores < minThreshold { sumScores = minThreshold } else if sumScores > maxThreshold { diff --git a/pkg/scheduler/framework/plugins/imagelocality/image_locality_test.go b/pkg/scheduler/framework/plugins/imagelocality/image_locality_test.go index c7c92cd82d9..04c59ab6b41 100644 --- a/pkg/scheduler/framework/plugins/imagelocality/image_locality_test.go +++ b/pkg/scheduler/framework/plugins/imagelocality/image_locality_test.go @@ -59,7 +59,7 @@ func TestImageLocalityPriority(t *testing.T) { Image: "gcr.io/10", }, { - Image: "gcr.io/2000", + Image: "gcr.io/4000", }, }, } @@ -78,6 +78,17 @@ func TestImageLocalityPriority(t *testing.T) { }, } + test3040 := v1.PodSpec{ + Containers: []v1.Container{ + { + Image: "gcr.io/30", + }, + { + Image: "gcr.io/40", + }, + }, + } + node403002000 := v1.NodeStatus{ Images: []v1.ContainerImage{ { @@ -126,19 +137,19 @@ func TestImageLocalityPriority(t *testing.T) { Images: []v1.ContainerImage{ { Names: []string{ - "gcr.io/600:" + parsers.DefaultImageTag, + "gcr.io/600:latest", }, SizeBytes: int64(600 * mb), }, { Names: []string{ - "gcr.io/40:" + parsers.DefaultImageTag, + "gcr.io/40:latest", }, SizeBytes: int64(40 * mb), }, { Names: []string{ - "gcr.io/900:" + parsers.DefaultImageTag, + "gcr.io/900:latest", }, SizeBytes: int64(900 * mb), }, @@ -149,25 +160,65 @@ func TestImageLocalityPriority(t *testing.T) { Images: []v1.ContainerImage{ { Names: []string{ - "gcr.io/300:" + parsers.DefaultImageTag, + "gcr.io/300:latest", }, SizeBytes: int64(300 * mb), }, { Names: []string{ - "gcr.io/600:" + parsers.DefaultImageTag, + "gcr.io/600:latest", }, SizeBytes: int64(600 * mb), }, { Names: []string{ - "gcr.io/900:" + parsers.DefaultImageTag, + "gcr.io/900:latest", }, SizeBytes: int64(900 * mb), }, }, } + node400030 := v1.NodeStatus{ + Images: []v1.ContainerImage{ + { + Names: []string{ + "gcr.io/4000:latest", + }, + SizeBytes: int64(4000 * mb), + }, + { + Names: []string{ + "gcr.io/30:latest", + }, + SizeBytes: int64(30 * mb), + }, + }, + } + + node203040 := v1.NodeStatus{ + Images: []v1.ContainerImage{ + { + Names: []string{ + "gcr.io/20:latest", + }, + SizeBytes: int64(20 * mb), + }, + { + Names: []string{ + "gcr.io/30:latest", + }, + SizeBytes: int64(30 * mb), + }, + { + Names: []string{ + "gcr.io/40:latest", + }, + SizeBytes: int64(40 * mb), + }, + }, + } + nodeWithNoImages := v1.NodeStatus{} tests := []struct { @@ -186,10 +237,10 @@ func TestImageLocalityPriority(t *testing.T) { // Node2 // Image: gcr.io/250:latest 250MB - // Score: 100 * (250M/2 - 23M)/(1000M - 23M) = 100 + // Score: 100 * (250M/2 - 23M)/(1000M * 2 - 23M) = 5 pod: &v1.Pod{Spec: test40250}, nodes: []*v1.Node{makeImageNode("machine1", node403002000), makeImageNode("machine2", node25010)}, - expectedList: []framework.NodeScore{{Name: "machine1", Score: 0}, {Name: "machine2", Score: 10}}, + expectedList: []framework.NodeScore{{Name: "machine1", Score: 0}, {Name: "machine2", Score: 5}}, name: "two images spread on two nodes, prefer the larger image one", }, { @@ -197,48 +248,48 @@ func TestImageLocalityPriority(t *testing.T) { // Node1 // Image: gcr.io/40:latest 40MB, gcr.io/300:latest 300MB - // Score: 100 * ((40M + 300M)/2 - 23M)/(1000M - 23M) = 15 + // Score: 100 * ((40M + 300M)/2 - 23M)/(1000M * 2 - 23M) = 7 // Node2 // Image: not present // Score: 0 pod: &v1.Pod{Spec: test40300}, nodes: []*v1.Node{makeImageNode("machine1", node403002000), makeImageNode("machine2", node25010)}, - expectedList: []framework.NodeScore{{Name: "machine1", Score: 15}, {Name: "machine2", Score: 0}}, + expectedList: []framework.NodeScore{{Name: "machine1", Score: 7}, {Name: "machine2", Score: 0}}, name: "two images on one node, prefer this node", }, { - // Pod: gcr.io/2000 gcr.io/10 + // Pod: gcr.io/4000 gcr.io/10 // Node1 - // Image: gcr.io/2000:latest 2000MB - // Score: 100 (2000M/2 >= 1000M, max-threshold) + // Image: gcr.io/4000:latest 2000MB + // Score: 100 (4000 * 1/2 >= 1000M * 2, max-threshold) // Node2 // Image: gcr.io/10:latest 10MB // Score: 0 (10M/2 < 23M, min-threshold) pod: &v1.Pod{Spec: testMinMax}, - nodes: []*v1.Node{makeImageNode("machine1", node403002000), makeImageNode("machine2", node25010)}, + nodes: []*v1.Node{makeImageNode("machine1", node400030), makeImageNode("machine2", node25010)}, expectedList: []framework.NodeScore{{Name: "machine1", Score: framework.MaxNodeScore}, {Name: "machine2", Score: 0}}, name: "if exceed limit, use limit", }, { - // Pod: gcr.io/2000 gcr.io/10 + // Pod: gcr.io/4000 gcr.io/10 // Node1 - // Image: gcr.io/2000:latest 2000MB - // Score: 100 * (2000M/3 - 23M)/(1000M - 23M) = 65 + // Image: gcr.io/4000:latest 4000MB + // Score: 100 * (4000M/3 - 23M)/(1000M * 2 - 23M) = 66 // Node2 // Image: gcr.io/10:latest 10MB - // Score: 0 (10M/2 < 23M, min-threshold) + // Score: 0 (10M*1/3 < 23M, min-threshold) // Node3 // Image: // Score: 0 pod: &v1.Pod{Spec: testMinMax}, - nodes: []*v1.Node{makeImageNode("machine1", node403002000), makeImageNode("machine2", node25010), makeImageNode("machine3", nodeWithNoImages)}, - expectedList: []framework.NodeScore{{Name: "machine1", Score: 65}, {Name: "machine2", Score: 0}, {Name: "machine3", Score: 0}}, + nodes: []*v1.Node{makeImageNode("machine1", node400030), makeImageNode("machine2", node25010), makeImageNode("machine3", nodeWithNoImages)}, + expectedList: []framework.NodeScore{{Name: "machine1", Score: 66}, {Name: "machine2", Score: 0}, {Name: "machine3", Score: 0}}, name: "if exceed limit, use limit (with node which has no images present)", }, { @@ -246,19 +297,34 @@ func TestImageLocalityPriority(t *testing.T) { // Node1 // Image: gcr.io/600:latest 600MB, gcr.io/900:latest 900MB - // Score: 100 (600M * 2/3 + 900M * 2/3 = 1000M >= 1000M, max-threshold) + // Score: 100 * (600M * 2/3 + 900M * 2/3 - 23M) / (1000M * 3 - 23M) = 32 // Node2 // Image: gcr.io/300:latest 300MB, gcr.io/600:latest 600MB, gcr.io/900:latest 900MB - // Score: 100 (300M * 1/3 + 600M * 2/3 + 900M * 2/3) >= 1000M, max-threshold) + // Score: 100 * (300M * 1/3 + 600M * 2/3 + 900M * 2/3 - 23M) / (1000M *3 - 23M) = 36 // Node3 // Image: // Score: 0 pod: &v1.Pod{Spec: test300600900}, nodes: []*v1.Node{makeImageNode("machine1", node60040900), makeImageNode("machine2", node300600900), makeImageNode("machine3", nodeWithNoImages)}, - expectedList: []framework.NodeScore{{Name: "machine1", Score: 100}, {Name: "machine2", Score: 100}, {Name: "machine3", Score: 0}}, - name: "max threshold haven't considered the size of containers to be run for the pod", + expectedList: []framework.NodeScore{{Name: "machine1", Score: 32}, {Name: "machine2", Score: 36}, {Name: "machine3", Score: 0}}, + name: "pod with multiple large images, machine2 is preferred", + }, + { + // Pod: gcr.io/30 gcr.io/40 + + // Node1 + // Image: gcr.io/20:latest 20MB, gcr.io/30:latest 30MB gcr.io/40:latest 40MB + // Score: 100 * (30M + 40M * 1/2 - 23M) / (1000M * 2 - 23M) = 1 + + // Node2 + // Image: 100 * (30M - 23M) / (1000M * 2 - 23M) = 0 + // Score: 0 + pod: &v1.Pod{Spec: test3040}, + nodes: []*v1.Node{makeImageNode("machine1", node203040), makeImageNode("machine2", node400030)}, + expectedList: []framework.NodeScore{{Name: "machine1", Score: 1}, {Name: "machine2", Score: 0}}, + name: "pod with multiple small images", }, }