Make generated pods only request the maximum necessary resources

Before this change, if memory or CPU requests were set in a Task's
Steps, (which are Containers) the generated Pod would require the sum
of all of the Steps requests to be scheduled on a Node. However,
because Tekton overwrites Container entrypoints in Tasks to
effectively make the Containers execute one at a time, we want to
make Pods generated by the TaskRun only request the maximum resources
that will be necessary for any single Container rather than the sum
of all resource requests.

To make this happen, when generating a Pod for a Task, we find the
Step with largest cpu/memory requests among all Steps, and set
the cpu/memory requests for all other steps to 0, respectively. If
no Step has an explicit cpu/memory request, all requests are set to
0. If we unset resource requests instead of setting them to 0
explicitly, then the limits would be used for the requests, which
would defeat the purpose of unsetting the requested values (and
could end up making the Pod request more memory than it did in the
first place).

Fixes tektoncd#598

pkg/reconciler/v1alpha1/taskrun/resources/pod.go

@@ -28,6 +28,7 @@ import (
 	"path/filepath"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
@@ -73,6 +74,8 @@ var (
 		VolumeSource: emptyVolumeSource,
 	}}
 
+	zeroQty = resource.MustParse("0")
+
 	// Random byte reader used for pod name generation.
 	// var for testing.
 	randReader = rand.Reader
@@ -174,6 +177,9 @@ func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient k
 	initContainers := []corev1.Container{*cred}
 	podContainers := []corev1.Container{}
 
+	maxCpuIdx := findMaxResourceRequest(taskSpec.Steps, corev1.ResourceCPU)
+	maxMemIdx := findMaxResourceRequest(taskSpec.Steps, corev1.ResourceMemory)
+
 	for i, step := range taskSpec.Steps {
 		step.Env = append(implicitEnvVars, step.Env...)
 		// TODO(mattmoor): Check that volumeMounts match volumes.
@@ -202,6 +208,7 @@ func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient k
 		if step.Name == names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%v%v", containerPrefix, entrypoint.InitContainerName)) {
 			initContainers = append(initContainers, step)
 		} else {
+			zeroNonMaxResourceRequests(&step, i == maxCpuIdx, i == maxMemIdx)
 			podContainers = append(podContainers, step)
 		}
 	}
@@ -261,3 +268,36 @@ func makeLabels(s *v1alpha1.TaskRun) map[string]string {
 	labels[pipeline.GroupName+pipeline.TaskRunLabelKey] = s.Name
 	return labels
 }
+
+// zeroNonMaxResourceRequests zeroes out the container's cpu or memory resource 
+// requests if the container does not have the largest request out of all
+// containers in the pod. This is done because Tekton overwrites each 
+// container's entrypoint to make containers effectively execute one at a time,
+// so we want pods to only request the maximum resources needed at any single point in time.
+// If no contianer has an explicit resource request, all requests are set to 0.
+func zeroNonMaxResourceRequests(container *corev1.Container, isMaxCpu, isMaxMem bool) {
+	if container.Resources.Requests == nil {
+		container.Resources.Requests = corev1.ResourceList{}
+	}
+	if !isMaxCpu {
+		container.Resources.Requests[corev1.ResourceCPU] = zeroQty
+	}
+	if !isMaxMem {
+		container.Resources.Requests[corev1.ResourceMemory] = zeroQty
+	}
+}
+
+// findMaxResourceRequest returns the index of the container with the maximum 
+// request for the given resource from among the given set of containers.
+func findMaxResourceRequest(containers []corev1.Container, resourceName corev1.ResourceName) int {
+	maxIdx := -1
+	maxReq := zeroQty
+	for i, c := range containers {
+		req, exists := c.Resources.Requests[resourceName]
+		if exists && req.Cmp(maxReq) > 0 {
+			maxIdx = i
+			maxReq = req
+		}
+	}
+	return maxIdx
+}

pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go

@@ -22,7 +22,6 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -33,8 +32,10 @@ import (
 )
 
 var (
-	ignorePrivateResourceFields = cmpopts.IgnoreUnexported(resource.Quantity{})
-	nopContainer                = corev1.Container{
+	resourceQuantityCmp = cmp.Comparer(func(x, y resource.Quantity) bool {
+		return x.Cmp(y) == 0
+	})
+	nopContainer = corev1.Container{
 		Name:    "nop",
 		Image:   *nopImage,
 		Command: []string{"/ko-app/nop"},
@@ -104,6 +105,12 @@ func TestMakePod(t *testing.T) {
 				Env:          implicitEnvVars,
 				VolumeMounts: implicitVolumeMounts,
 				WorkingDir:   workspaceDir,
+				Resources: corev1.ResourceRequirements{
+					Requests: corev1.ResourceList{
+						corev1.ResourceCPU: resource.MustParse("0"),
+						corev1.ResourceMemory: resource.MustParse("0"),
+					},
+				},
 			},
 				nopContainer,
 			},
@@ -143,6 +150,12 @@ func TestMakePod(t *testing.T) {
 				Env:          implicitEnvVars,
 				VolumeMounts: implicitVolumeMounts,
 				WorkingDir:   workspaceDir,
+				Resources: corev1.ResourceRequirements{
+					Requests: corev1.ResourceList{
+						corev1.ResourceCPU: resource.MustParse("0"),
+						corev1.ResourceMemory: resource.MustParse("0"),
+					},
+				},
 			},
 				nopContainer,
 			},
@@ -176,6 +189,12 @@ func TestMakePod(t *testing.T) {
 				Env:          implicitEnvVars,
 				VolumeMounts: implicitVolumeMounts,
 				WorkingDir:   workspaceDir,
+				Resources: corev1.ResourceRequirements{
+					Requests: corev1.ResourceList{
+						corev1.ResourceCPU: resource.MustParse("0"),
+						corev1.ResourceMemory: resource.MustParse("0"),
+					},
+				},
 			},
 				nopContainer,
 			},
@@ -209,6 +228,12 @@ func TestMakePod(t *testing.T) {
 				Env:          implicitEnvVars,
 				VolumeMounts: implicitVolumeMounts,
 				WorkingDir:   workspaceDir,
+				Resources: corev1.ResourceRequirements{
+					Requests: corev1.ResourceList{
+						corev1.ResourceCPU: resource.MustParse("0"),
+						corev1.ResourceMemory: resource.MustParse("0"),
+					},
+				},
 			},
 				nopContainer,
 			},
@@ -257,7 +282,7 @@ func TestMakePod(t *testing.T) {
 				t.Errorf("Pod name got %q, want %q", got.Name, wantName)
 			}
 
-			if d := cmp.Diff(&got.Spec, c.want, ignorePrivateResourceFields); d != "" {
+			if d := cmp.Diff(&got.Spec, c.want, resourceQuantityCmp); d != "" {
 				t.Errorf("Diff spec:\n%s", d)
 			}