check for limitrange minimum for taskrun container requests

config/200-clusterrole.yaml

@@ -4,7 +4,7 @@ metadata:
   name: tekton-pipelines-admin
 rules:
   - apiGroups: [""]
-    resources: ["pods", "pods/log", "namespaces", "secrets", "events", "serviceaccounts", "configmaps", "persistentvolumeclaims"]
+    resources: ["pods", "pods/log", "namespaces", "secrets", "events", "serviceaccounts", "configmaps", "persistentvolumeclaims", "limitranges"]
     verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
   - apiGroups: ["apps"]
     resources: ["deployments"]

go.mod

@@ -4,6 +4,7 @@ go 1.13
 
 require (
 	cloud.google.com/go v0.47.0 // indirect
+	cloud.google.com/go/storage v1.0.0
 	contrib.go.opencensus.io/exporter/prometheus v0.1.0 // indirect
 	contrib.go.opencensus.io/exporter/stackdriver v0.12.8 // indirect
 	github.com/Azure/go-autorest/autorest/validation v0.2.0 // indirect
@@ -44,7 +45,7 @@ require (
 	golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e // indirect
 	golang.org/x/sys v0.0.0-20191210023423-ac6580df4449 // indirect
 	golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
-	google.golang.org/api v0.10.0 // indirect
+	google.golang.org/api v0.10.0
 	google.golang.org/appengine v1.6.5 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.2.5 // indirect

pkg/pod/pod.go

@@ -119,8 +119,23 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha
 	initContainers = append(initContainers, entrypointInit)
 	volumes = append(volumes, toolsVolume, downwardVolume)
 
+	// Get LimitRange by name if present in ConfigMap tekton-limitrange-config.
+	// Otherwise, pass empty request to resolveResourceRequests.
+	limitRangeName := ""
+	limitRangeConfig, err := kubeclient.CoreV1().ConfigMaps(taskRun.Namespace).Get("tekton-limitrange-config", metav1.GetOptions{})
+	if err == nil {
+		limitRangeName = limitRangeConfig.Data["default-limitrange-name"]
+	}
+	limitRange := &corev1.LimitRange{}
+	if limitRangeName != "" {
+		limitRange, err = kubeclient.CoreV1().LimitRanges(taskRun.Namespace).Get(limitRangeName, metav1.GetOptions{})
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	// Zero out non-max resource requests.
-	stepContainers = resolveResourceRequests(stepContainers)
+	stepContainers = resolveResourceRequests(stepContainers, limitRange)
 
 	// Add implicit env vars.
 	// They're prepended to the list, so that if the user specified any

pkg/pod/resource_request.go

@@ -21,6 +21,8 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 )
 
+var emptyLimitRange = &corev1.LimitRange{}
+var emptyResourceQuantity = resource.Quantity{}
 var zeroQty = resource.MustParse("0")
 
 func allZeroQty() corev1.ResourceList {
@@ -31,7 +33,7 @@ func allZeroQty() corev1.ResourceList {
 	}
 }
 
-func resolveResourceRequests(containers []corev1.Container) []corev1.Container {
+func resolveResourceRequests(containers []corev1.Container, limitRange *corev1.LimitRange) []corev1.Container {
 	max := allZeroQty()
 	resourceNames := []corev1.ResourceName{corev1.ResourceCPU, corev1.ResourceMemory, corev1.ResourceEphemeralStorage}
 	maxIndicesByResource := make(map[corev1.ResourceName]int, len(resourceNames))
@@ -50,16 +52,42 @@ func resolveResourceRequests(containers []corev1.Container) []corev1.Container {
 		}
 	}
 
+	// Get limitrange minimum for container requests so they won't
+	// be zeroed out if minimum is specified in namespace
+	limitRangeItems := limitRange.Spec.Limits
+	min := allZeroQty()
+	if limitRange != emptyLimitRange {
+		for _, limitRangeItem := range limitRangeItems {
+			if limitRangeItem.Type == "Container" {
+				if limitRangeItem.Min != nil {
+					min = limitRangeItem.Min
+				}
+				break
+			}
+		}
+	}
+
+	// Use zeroQty if request value is not set for min
+	if min[corev1.ResourceCPU] == emptyResourceQuantity {
+		min[corev1.ResourceCPU] = zeroQty
+	}
+	if min[corev1.ResourceMemory] == emptyResourceQuantity {
+		min[corev1.ResourceMemory] = zeroQty
+	}
+	if min[corev1.ResourceEphemeralStorage] == emptyResourceQuantity {
+		min[corev1.ResourceEphemeralStorage] = zeroQty
+	}
+
 	// 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()
+			containers[i].Resources.Requests = min
 			continue
 		}
 		for _, resourceName := range resourceNames {
 			if maxIndicesByResource[resourceName] != i {
-				containers[i].Resources.Requests[resourceName] = zeroQty
+				containers[i].Resources.Requests[resourceName] = min[resourceName]
 			}
 		}
 	}

pkg/pod/resource_request_test.go

@@ -28,7 +28,7 @@ var resourceQuantityCmp = cmp.Comparer(func(x, y resource.Quantity) bool {
 	return x.Cmp(y) == 0
 })
 
-func TestResolveResourceRequests(t *testing.T) {
+func TestResolveResourceRequests_No_LimitRange(t *testing.T) {
 	for _, c := range []struct {
 		desc     string
 		in, want []corev1.Container
@@ -206,7 +206,146 @@ func TestResolveResourceRequests(t *testing.T) {
 		},
 	} {
 		t.Run(c.desc, func(t *testing.T) {
-			got := resolveResourceRequests(c.in)
+			got := resolveResourceRequests(c.in, &corev1.LimitRange{})
+			if d := cmp.Diff(c.want, got, resourceQuantityCmp); d != "" {
+				t.Errorf("Diff(-want, +got): %s", d)
+			}
+		})
+	}
+}
+
+func TestResolveResourceRequests_LimitRange(t *testing.T) {
+	for _, c := range []struct {
+		desc     string
+		in, want []corev1.Container
+	}{{
+		desc: "three steps, no requests, apply minimum to all",
+		in:   []corev1.Container{{}, {}, {}},
+		want: []corev1.Container{{
+			Resources: corev1.ResourceRequirements{
+				Requests: corev1.ResourceList{
+					corev1.ResourceCPU:              resource.MustParse("100m"),
+					corev1.ResourceMemory:           resource.MustParse("99Mi"),
+					corev1.ResourceEphemeralStorage: resource.MustParse("100m"),
+				},
+			},
+		}, {
+			Resources: corev1.ResourceRequirements{
+				Requests: corev1.ResourceList{
+					corev1.ResourceCPU:              resource.MustParse("100m"),
+					corev1.ResourceMemory:           resource.MustParse("99Mi"),
+					corev1.ResourceEphemeralStorage: resource.MustParse("100m"),
+				},
+			},
+		}, {
+			Resources: corev1.ResourceRequirements{
+				Requests: corev1.ResourceList{
+					corev1.ResourceCPU:              resource.MustParse("100m"),
+					corev1.ResourceMemory:           resource.MustParse("99Mi"),
+					corev1.ResourceEphemeralStorage: resource.MustParse("100m"),
+				},
+			},
+		}},
+	}, {
+		desc: "three steps, no requests, apply minimum values when not max values",
+		in: []corev1.Container{{
+			Resources: corev1.ResourceRequirements{
+				Requests: corev1.ResourceList{
+					corev1.ResourceCPU: resource.MustParse("10"),
+				},
+			},
+		}, {
+			Resources: corev1.ResourceRequirements{
+				Requests: corev1.ResourceList{
+					corev1.ResourceMemory: resource.MustParse("10Gi"),
+				},
+				Limits: corev1.ResourceList{
+					corev1.ResourceMemory: resource.MustParse("11Gi"),
+				},
+			},
+		}, {
+			Resources: corev1.ResourceRequirements{
+				Requests: corev1.ResourceList{
+					corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"),
+				},
+				Limits: corev1.ResourceList{
+					corev1.ResourceMemory: resource.MustParse("100Gi"),
+				},
+			},
+		}},
+		want: []corev1.Container{{
+			// ResourceCPU max request
+			Resources: corev1.ResourceRequirements{
+				Requests: corev1.ResourceList{
+					corev1.ResourceCPU:              resource.MustParse("10"),
+					corev1.ResourceMemory:           resource.MustParse("99Mi"),
+					corev1.ResourceEphemeralStorage: resource.MustParse("100m"),
+				},
+			},
+		}, {
+			// ResourceMemory max request
+			Resources: corev1.ResourceRequirements{
+				Requests: corev1.ResourceList{
+					corev1.ResourceCPU:              resource.MustParse("100m"),
+					corev1.ResourceMemory:           resource.MustParse("10Gi"),
+					corev1.ResourceEphemeralStorage: resource.MustParse("100m"),
+				},
+				Limits: corev1.ResourceList{
+					corev1.ResourceMemory: resource.MustParse("11Gi"),
+				},
+			},
+		}, {
+			// ResourceEphemeralStorage max request
+			Resources: corev1.ResourceRequirements{
+				Requests: corev1.ResourceList{
+					corev1.ResourceCPU:              resource.MustParse("100m"),
+					corev1.ResourceMemory:           resource.MustParse("99Mi"),
+					corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"),
+				},
+				Limits: corev1.ResourceList{
+					corev1.ResourceMemory: resource.MustParse("100Gi"),
+				},
+			},
+		}},
+	}, {
+		desc: "Only one step container with all request values filled out, no min values",
+		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,
+				&corev1.LimitRange{
+					Spec: corev1.LimitRangeSpec{
+						Limits: []corev1.LimitRangeItem{
+							{
+								Type: "Container",
+								Min: corev1.ResourceList{
+									corev1.ResourceCPU:              resource.MustParse("100m"),
+									corev1.ResourceMemory:           resource.MustParse("99Mi"),
+									corev1.ResourceEphemeralStorage: resource.MustParse("100m"),
+								},
+							},
+						},
+					},
+				})
 			if d := cmp.Diff(c.want, got, resourceQuantityCmp); d != "" {
 				t.Errorf("Diff(-want, +got): %s", d)
 			}