diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 00e45500695..15511a872bf 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -119,7 +119,7 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha initContainers = append(initContainers, entrypointInit) volumes = append(volumes, toolsVolume, downwardVolume) - // Zero out non-max resource requests, move max resource requests to the last step. + // Zero out non-max resource requests. stepContainers = resolveResourceRequests(stepContainers) // Add implicit env vars. diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index a30b34d75b9..50a6e043778 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -436,10 +436,16 @@ func TestMakePod(t *testing.T) { "cmd", "--", }, - Env: implicitEnvVars, - VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...), - WorkingDir: pipeline.WorkspaceDir, - Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, + Env: implicitEnvVars, + VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...), + WorkingDir: pipeline.WorkspaceDir, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("8"), + corev1.ResourceMemory: zeroQty, + corev1.ResourceEphemeralStorage: zeroQty, + }, + }, TerminationMessagePath: "/tekton/termination", }, { Name: "step-unnamed-1", @@ -461,7 +467,7 @@ func TestMakePod(t *testing.T) { WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("8"), + corev1.ResourceCPU: zeroQty, corev1.ResourceMemory: resource.MustParse("100Gi"), corev1.ResourceEphemeralStorage: zeroQty, }, diff --git a/pkg/pod/resource_request.go b/pkg/pod/resource_request.go index cc599c2d7ec..5cbbc2080c5 100644 --- a/pkg/pod/resource_request.go +++ b/pkg/pod/resource_request.go @@ -33,20 +33,36 @@ func allZeroQty() corev1.ResourceList { func resolveResourceRequests(containers []corev1.Container) []corev1.Container { max := allZeroQty() - for _, c := range containers { + resourceNames := []corev1.ResourceName{corev1.ResourceCPU, corev1.ResourceMemory, corev1.ResourceEphemeralStorage} + maxIndicesByResource := make(map[corev1.ResourceName]int, len(resourceNames)) + for _, resourceName := range resourceNames { + maxIndicesByResource[resourceName] = -1 + } + + // Find max resource requests and associated list indices for + // containers for CPU, memory, and ephemeral storage resources + for i, c := range containers { for k, v := range c.Resources.Requests { if v.Cmp(max[k]) > 0 { + maxIndicesByResource[k] = i max[k] = v } } } - // Set resource requests for all steps but the last container to - // zero. - for i := range containers[:len(containers)-1] { - containers[i].Resources.Requests = allZeroQty() + // Set all non max resource requests to 0. Leave max request at index + // originally defined to account for limit of step. + for i := range containers { + if containers[i].Resources.Requests == nil { + containers[i].Resources.Requests = allZeroQty() + continue + } + for _, resourceName := range resourceNames { + if maxIndicesByResource[resourceName] != i { + containers[i].Resources.Requests[resourceName] = zeroQty + } + } } - // Set the last container's request to the max of all resources. - containers[len(containers)-1].Resources.Requests = max + return containers } diff --git a/pkg/pod/resource_request_test.go b/pkg/pod/resource_request_test.go index 76bf6ae77b9..b034cb00c99 100644 --- a/pkg/pod/resource_request_test.go +++ b/pkg/pod/resource_request_test.go @@ -70,18 +70,61 @@ func TestResolveResourceRequests(t *testing.T) { }, }}, want: []corev1.Container{{ - // All zeroed out. - Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, + // ResourceCPU max request + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10"), + corev1.ResourceMemory: zeroQty, + corev1.ResourceEphemeralStorage: zeroQty, + }, + }, }, { - // Requests zeroed out, limits remain. + // ResourceMemory max request Resources: corev1.ResourceRequirements{ - Requests: allZeroQty(), + Requests: corev1.ResourceList{ + corev1.ResourceCPU: zeroQty, + corev1.ResourceMemory: resource.MustParse("10Gi"), + corev1.ResourceEphemeralStorage: zeroQty, + }, + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("11Gi"), + }, + }, + }, { + // ResourceEphemeralStorage max request + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: zeroQty, + corev1.ResourceMemory: zeroQty, + corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("100Gi"), + }, + }, + }}, + }, { + desc: "Max requests all with step2", + in: []corev1.Container{{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10"), + corev1.ResourceMemory: resource.MustParse("10Gi"), + corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"), + }, + }, + }, { + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("11"), + corev1.ResourceMemory: resource.MustParse("11Gi"), + corev1.ResourceEphemeralStorage: resource.MustParse("101Gi"), + }, Limits: corev1.ResourceList{ corev1.ResourceMemory: resource.MustParse("11Gi"), }, }, }, { - // Requests to the max, limits remain. Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("10"), @@ -93,7 +136,75 @@ func TestResolveResourceRequests(t *testing.T) { }, }, }}, - }} { + want: []corev1.Container{{ + // All zeroed out since step 2 has max requests + Resources: corev1.ResourceRequirements{ + Requests: allZeroQty(), + }, + }, { + // All max requests + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("11"), + corev1.ResourceMemory: resource.MustParse("11Gi"), + corev1.ResourceEphemeralStorage: resource.MustParse("101Gi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("11Gi"), + }, + }, + }, { + // All zeroed out since step 2 has max requests + Resources: corev1.ResourceRequirements{ + Requests: allZeroQty(), + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("100Gi"), + }, + }, + }}, + }, + { + desc: "Only one step container with only memory request value filled out", + in: []corev1.Container{{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("10Gi"), + }, + }, + }}, + want: []corev1.Container{{ + // ResourceMemory max request set. zeroQty for non set resources. + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: zeroQty, + corev1.ResourceMemory: resource.MustParse("10Gi"), + corev1.ResourceEphemeralStorage: zeroQty, + }, + }, + }}, + }, { + desc: "Only one step container with all request values filled out", + in: []corev1.Container{{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10"), + corev1.ResourceMemory: resource.MustParse("10Gi"), + corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"), + }, + }, + }}, + want: []corev1.Container{{ + // All max values set + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10"), + corev1.ResourceMemory: resource.MustParse("10Gi"), + corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"), + }, + }, + }}, + }, + } { t.Run(c.desc, func(t *testing.T) { got := resolveResourceRequests(c.in) if d := cmp.Diff(c.want, got, resourceQuantityCmp); d != "" {