Skip to content

Commit c10db73

Browse files
danielhelfandtekton-robot
danielhelfand
authored and
tekton-robot
committed
leave max container request values at original index
1 parent d6dd94e commit c10db73

File tree

4 files changed

+152
-19
lines changed

4 files changed

+152
-19
lines changed

‎pkg/pod/pod.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha
119119
initContainers = append(initContainers, entrypointInit)
120120
volumes = append(volumes, toolsVolume, downwardVolume)
121121

122-
// Zero out non-max resource requests, move max resource requests to the last step.
122+
// Zero out non-max resource requests.
123123
stepContainers = resolveResourceRequests(stepContainers)
124124

125125
// Add implicit env vars.

‎pkg/pod/pod_test.go

+11-5
Original file line numberDiff line numberDiff line change
@@ -436,10 +436,16 @@ func TestMakePod(t *testing.T) {
436436
"cmd",
437437
"--",
438438
},
439-
Env: implicitEnvVars,
440-
VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...),
441-
WorkingDir: pipeline.WorkspaceDir,
442-
Resources: corev1.ResourceRequirements{Requests: allZeroQty()},
439+
Env: implicitEnvVars,
440+
VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...),
441+
WorkingDir: pipeline.WorkspaceDir,
442+
Resources: corev1.ResourceRequirements{
443+
Requests: corev1.ResourceList{
444+
corev1.ResourceCPU: resource.MustParse("8"),
445+
corev1.ResourceMemory: zeroQty,
446+
corev1.ResourceEphemeralStorage: zeroQty,
447+
},
448+
},
443449
TerminationMessagePath: "/tekton/termination",
444450
}, {
445451
Name: "step-unnamed-1",
@@ -461,7 +467,7 @@ func TestMakePod(t *testing.T) {
461467
WorkingDir: pipeline.WorkspaceDir,
462468
Resources: corev1.ResourceRequirements{
463469
Requests: corev1.ResourceList{
464-
corev1.ResourceCPU: resource.MustParse("8"),
470+
corev1.ResourceCPU: zeroQty,
465471
corev1.ResourceMemory: resource.MustParse("100Gi"),
466472
corev1.ResourceEphemeralStorage: zeroQty,
467473
},

‎pkg/pod/resource_request.go

+23-7
Original file line numberDiff line numberDiff line change
@@ -33,20 +33,36 @@ func allZeroQty() corev1.ResourceList {
3333

3434
func resolveResourceRequests(containers []corev1.Container) []corev1.Container {
3535
max := allZeroQty()
36-
for _, c := range containers {
36+
resourceNames := []corev1.ResourceName{corev1.ResourceCPU, corev1.ResourceMemory, corev1.ResourceEphemeralStorage}
37+
maxIndicesByResource := make(map[corev1.ResourceName]int, len(resourceNames))
38+
for _, resourceName := range resourceNames {
39+
maxIndicesByResource[resourceName] = -1
40+
}
41+
42+
// Find max resource requests and associated list indices for
43+
// containers for CPU, memory, and ephemeral storage resources
44+
for i, c := range containers {
3745
for k, v := range c.Resources.Requests {
3846
if v.Cmp(max[k]) > 0 {
47+
maxIndicesByResource[k] = i
3948
max[k] = v
4049
}
4150
}
4251
}
4352

44-
// Set resource requests for all steps but the last container to
45-
// zero.
46-
for i := range containers[:len(containers)-1] {
47-
containers[i].Resources.Requests = allZeroQty()
53+
// Set all non max resource requests to 0. Leave max request at index
54+
// originally defined to account for limit of step.
55+
for i := range containers {
56+
if containers[i].Resources.Requests == nil {
57+
containers[i].Resources.Requests = allZeroQty()
58+
continue
59+
}
60+
for _, resourceName := range resourceNames {
61+
if maxIndicesByResource[resourceName] != i {
62+
containers[i].Resources.Requests[resourceName] = zeroQty
63+
}
64+
}
4865
}
49-
// Set the last container's request to the max of all resources.
50-
containers[len(containers)-1].Resources.Requests = max
66+
5167
return containers
5268
}

‎pkg/pod/resource_request_test.go

+117-6
Original file line numberDiff line numberDiff line change
@@ -70,18 +70,61 @@ func TestResolveResourceRequests(t *testing.T) {
7070
},
7171
}},
7272
want: []corev1.Container{{
73-
// All zeroed out.
74-
Resources: corev1.ResourceRequirements{Requests: allZeroQty()},
73+
// ResourceCPU max request
74+
Resources: corev1.ResourceRequirements{
75+
Requests: corev1.ResourceList{
76+
corev1.ResourceCPU: resource.MustParse("10"),
77+
corev1.ResourceMemory: zeroQty,
78+
corev1.ResourceEphemeralStorage: zeroQty,
79+
},
80+
},
7581
}, {
76-
// Requests zeroed out, limits remain.
82+
// ResourceMemory max request
7783
Resources: corev1.ResourceRequirements{
78-
Requests: allZeroQty(),
84+
Requests: corev1.ResourceList{
85+
corev1.ResourceCPU: zeroQty,
86+
corev1.ResourceMemory: resource.MustParse("10Gi"),
87+
corev1.ResourceEphemeralStorage: zeroQty,
88+
},
89+
Limits: corev1.ResourceList{
90+
corev1.ResourceMemory: resource.MustParse("11Gi"),
91+
},
92+
},
93+
}, {
94+
// ResourceEphemeralStorage max request
95+
Resources: corev1.ResourceRequirements{
96+
Requests: corev1.ResourceList{
97+
corev1.ResourceCPU: zeroQty,
98+
corev1.ResourceMemory: zeroQty,
99+
corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"),
100+
},
101+
Limits: corev1.ResourceList{
102+
corev1.ResourceMemory: resource.MustParse("100Gi"),
103+
},
104+
},
105+
}},
106+
}, {
107+
desc: "Max requests all with step2",
108+
in: []corev1.Container{{
109+
Resources: corev1.ResourceRequirements{
110+
Requests: corev1.ResourceList{
111+
corev1.ResourceCPU: resource.MustParse("10"),
112+
corev1.ResourceMemory: resource.MustParse("10Gi"),
113+
corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"),
114+
},
115+
},
116+
}, {
117+
Resources: corev1.ResourceRequirements{
118+
Requests: corev1.ResourceList{
119+
corev1.ResourceCPU: resource.MustParse("11"),
120+
corev1.ResourceMemory: resource.MustParse("11Gi"),
121+
corev1.ResourceEphemeralStorage: resource.MustParse("101Gi"),
122+
},
79123
Limits: corev1.ResourceList{
80124
corev1.ResourceMemory: resource.MustParse("11Gi"),
81125
},
82126
},
83127
}, {
84-
// Requests to the max, limits remain.
85128
Resources: corev1.ResourceRequirements{
86129
Requests: corev1.ResourceList{
87130
corev1.ResourceCPU: resource.MustParse("10"),
@@ -93,7 +136,75 @@ func TestResolveResourceRequests(t *testing.T) {
93136
},
94137
},
95138
}},
96-
}} {
139+
want: []corev1.Container{{
140+
// All zeroed out since step 2 has max requests
141+
Resources: corev1.ResourceRequirements{
142+
Requests: allZeroQty(),
143+
},
144+
}, {
145+
// All max requests
146+
Resources: corev1.ResourceRequirements{
147+
Requests: corev1.ResourceList{
148+
corev1.ResourceCPU: resource.MustParse("11"),
149+
corev1.ResourceMemory: resource.MustParse("11Gi"),
150+
corev1.ResourceEphemeralStorage: resource.MustParse("101Gi"),
151+
},
152+
Limits: corev1.ResourceList{
153+
corev1.ResourceMemory: resource.MustParse("11Gi"),
154+
},
155+
},
156+
}, {
157+
// All zeroed out since step 2 has max requests
158+
Resources: corev1.ResourceRequirements{
159+
Requests: allZeroQty(),
160+
Limits: corev1.ResourceList{
161+
corev1.ResourceMemory: resource.MustParse("100Gi"),
162+
},
163+
},
164+
}},
165+
},
166+
{
167+
desc: "Only one step container with only memory request value filled out",
168+
in: []corev1.Container{{
169+
Resources: corev1.ResourceRequirements{
170+
Requests: corev1.ResourceList{
171+
corev1.ResourceMemory: resource.MustParse("10Gi"),
172+
},
173+
},
174+
}},
175+
want: []corev1.Container{{
176+
// ResourceMemory max request set. zeroQty for non set resources.
177+
Resources: corev1.ResourceRequirements{
178+
Requests: corev1.ResourceList{
179+
corev1.ResourceCPU: zeroQty,
180+
corev1.ResourceMemory: resource.MustParse("10Gi"),
181+
corev1.ResourceEphemeralStorage: zeroQty,
182+
},
183+
},
184+
}},
185+
}, {
186+
desc: "Only one step container with all request values filled out",
187+
in: []corev1.Container{{
188+
Resources: corev1.ResourceRequirements{
189+
Requests: corev1.ResourceList{
190+
corev1.ResourceCPU: resource.MustParse("10"),
191+
corev1.ResourceMemory: resource.MustParse("10Gi"),
192+
corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"),
193+
},
194+
},
195+
}},
196+
want: []corev1.Container{{
197+
// All max values set
198+
Resources: corev1.ResourceRequirements{
199+
Requests: corev1.ResourceList{
200+
corev1.ResourceCPU: resource.MustParse("10"),
201+
corev1.ResourceMemory: resource.MustParse("10Gi"),
202+
corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"),
203+
},
204+
},
205+
}},
206+
},
207+
} {
97208
t.Run(c.desc, func(t *testing.T) {
98209
got := resolveResourceRequests(c.in)
99210
if d := cmp.Diff(c.want, got, resourceQuantityCmp); d != "" {

0 commit comments

Comments
 (0)