Update admission control logic for LimitRange

plugin/pkg/admission/limitranger/admission.go

@@ -138,15 +138,19 @@ func Limit(limitRange *api.LimitRange, resourceName string, obj runtime.Object)
 
 // defaultContainerResourceRequirements returns the default requirements for a container
 // the requirement.Limits are taken from the LimitRange defaults (if specified)
-// the requirement.Requests are taken from the LimitRange min (if specified)
+// the requirement.Requests are taken from the LimitRange default request (if specified)
 func defaultContainerResourceRequirements(limitRange *api.LimitRange) api.ResourceRequirements {
 	requirements := api.ResourceRequirements{}
-	requirements.Limits = api.ResourceList{}
 	requirements.Requests = api.ResourceList{}
+	requirements.Limits = api.ResourceList{}
 
 	for i := range limitRange.Spec.Limits {
 		limit := limitRange.Spec.Limits[i]
 		if limit.Type == api.LimitTypeContainer {
+			for k, v := range limit.DefaultRequest {
+				value := v.Copy()
+				requirements.Requests[k] = *value
+			}
 			for k, v := range limit.Default {
 				value := v.Copy()
 				requirements.Limits[k] = *value
@@ -181,89 +185,174 @@ func mergePodResourceRequirements(pod *api.Pod, defaultRequirements *api.Resourc
 	}
 }
 
+// requestLimitEnforcedValues returns the specified values at a common precision to support comparability
+func requestLimitEnforcedValues(requestQuantity, limitQuantity, enforcedQuantity resource.Quantity) (request, limit, enforced int64) {
+	request = requestQuantity.Value()
+	limit = limitQuantity.Value()
+	enforced = enforcedQuantity.Value()
+	// do a more precise comparison if possible (if the value won't overflow)
+	if request <= resource.MaxMilliValue && limit <= resource.MaxMilliValue && enforced <= resource.MaxMilliValue {
+		request = requestQuantity.MilliValue()
+		limit = limitQuantity.MilliValue()
+		enforced = enforcedQuantity.MilliValue()
+	}
+	return
+}
+
+// minConstraint enforces the min constraint over the specified resource
+func minConstraint(limitType api.LimitType, resourceName api.ResourceName, enforced resource.Quantity, request api.ResourceList, limit api.ResourceList) error {
+	req, reqExists := request[resourceName]
+	lim, limExists := limit[resourceName]
+	observedReqValue, observedLimValue, enforcedValue := requestLimitEnforcedValues(req, lim, enforced)
+
+	if !reqExists {
+		return fmt.Errorf("Minimum %s usage per %s is %s.  No request is specified.", resourceName, limitType, enforced.String())
+	}
+	if observedReqValue < enforcedValue {
+		return fmt.Errorf("Minimum %s usage per %s is %s, but request is %s.", resourceName, limitType, enforced.String(), req.String())
+	}
+	if limExists && (observedLimValue < enforcedValue) {
+		return fmt.Errorf("Minimum %s usage per %s is %s, but limit is %s.", resourceName, limitType, enforced.String(), lim.String())
+	}
+	return nil
+}
+
+// maxConstraint enforces the max constraint over the specified resource
+func maxConstraint(limitType api.LimitType, resourceName api.ResourceName, enforced resource.Quantity, request api.ResourceList, limit api.ResourceList) error {
+	req, reqExists := request[resourceName]
+	lim, limExists := limit[resourceName]
+	observedReqValue, observedLimValue, enforcedValue := requestLimitEnforcedValues(req, lim, enforced)
+
+	if !limExists {
+		return fmt.Errorf("Maximum %s usage per %s is %s.  No limit is specified.", resourceName, limitType, enforced.String())
+	}
+	if observedLimValue > enforcedValue {
+		return fmt.Errorf("Maximum %s usage per %s is %s, but limit is %s.", resourceName, limitType, enforced.String(), lim.String())
+	}
+	if reqExists && (observedReqValue > enforcedValue) {
+		return fmt.Errorf("Maximum %s usage per %s is %s, but request is %s.", resourceName, limitType, enforced.String(), req.String())
+	}
+	return nil
+}
+
+// limitRequestRatioConstraint enforces the limit to request ratio over the specified resource
+func limitRequestRatioConstraint(limitType api.LimitType, resourceName api.ResourceName, enforced resource.Quantity, request api.ResourceList, limit api.ResourceList) error {
+	req, reqExists := request[resourceName]
+	lim, limExists := limit[resourceName]
+	observedReqValue, observedLimValue, enforcedValue := requestLimitEnforcedValues(req, lim, enforced)
+
+	if !reqExists || (observedReqValue == int64(0)) {
+		return fmt.Errorf("%s max limit to request ratio per %s is %s, but no request is specified or request is 0.", resourceName, limitType, enforced.String())
+	}
+	if !limExists || (observedLimValue == int64(0)) {
+		return fmt.Errorf("%s max limit to request ratio per %s is %s, but no limit is specified or limit is 0.", resourceName, limitType, enforced.String())
+	}
+
+	observedValue := observedLimValue / observedReqValue
+
+	if observedValue > enforcedValue {
+		return fmt.Errorf("%s max limit to request ratio per %s is %s, but provided ratio is %d.", resourceName, limitType, enforced.String(), observedValue)
+	}
+
+	return nil
+}
+
+// sum takes the total of each named resource across all inputs
+// if a key is not in each input, then the output resource list will omit the key
+func sum(inputs []api.ResourceList) api.ResourceList {
+	result := api.ResourceList{}
+	keys := []api.ResourceName{}
+	for i := range inputs {
+		for k := range inputs[i] {
+			keys = append(keys, k)
+		}
+	}
+	for _, key := range keys {
+		total, isSet := int64(0), true
+
+		for i := range inputs {
+			input := inputs[i]
+			v, exists := input[key]
+			if exists {
+				if key == api.ResourceCPU {
+					total = total + v.MilliValue()
+				} else {
+					total = total + v.Value()
+				}
+			} else {
+				isSet = false
+			}
+		}
+
+		if isSet {
+			if key == api.ResourceCPU {
+				result[key] = *(resource.NewMilliQuantity(total, resource.DecimalSI))
+			} else {
+				result[key] = *(resource.NewQuantity(total, resource.DecimalSI))
+			}
+
+		}
+	}
+	return result
+}
+
 // PodLimitFunc enforces resource requirements enumerated by the pod against
 // the specified LimitRange.  The pod may be modified to apply default resource
 // requirements if not specified, and enumerated on the LimitRange
 func PodLimitFunc(limitRange *api.LimitRange, pod *api.Pod) error {
-
 	defaultResources := defaultContainerResourceRequirements(limitRange)
 	mergePodResourceRequirements(pod, &defaultResources)
 
-	podCPU := int64(0)
-	podMem := int64(0)
-
-	minContainerCPU := int64(0)
-	minContainerMem := int64(0)
-	maxContainerCPU := int64(0)
-	maxContainerMem := int64(0)
-
-	for i := range pod.Spec.Containers {
-		container := &pod.Spec.Containers[i]
-		containerCPU := container.Resources.Limits.Cpu().MilliValue()
-		containerMem := container.Resources.Limits.Memory().Value()
-
-		if i == 0 {
-			minContainerCPU = containerCPU
-			minContainerMem = containerMem
-			maxContainerCPU = containerCPU
-			maxContainerMem = containerMem
-		}
-
-		podCPU = podCPU + container.Resources.Limits.Cpu().MilliValue()
-		podMem = podMem + container.Resources.Limits.Memory().Value()
-
-		minContainerCPU = Min(containerCPU, minContainerCPU)
-		minContainerMem = Min(containerMem, minContainerMem)
-		maxContainerCPU = Max(containerCPU, maxContainerCPU)
-		maxContainerMem = Max(containerMem, maxContainerMem)
-	}
-
 	for i := range limitRange.Spec.Limits {
 		limit := limitRange.Spec.Limits[i]
-		for _, minOrMax := range []string{"Min", "Max"} {
+		limitType := limit.Type
-			var rl api.ResourceList
+
-			switch minOrMax {
+		// enforce container limits
-			case "Min":
+		if limitType == api.LimitTypeContainer {
-				rl = limit.Min
+			for j := range pod.Spec.Containers {
-			case "Max":
+				container := &pod.Spec.Containers[j]
-				rl = limit.Max
+				for k, v := range limit.Min {
-			}
+					if err := minConstraint(limitType, k, v, container.Resources.Requests, container.Resources.Limits); err != nil {
-			for k, v := range rl {
-				observed := int64(0)
-				enforced := int64(0)
-				var err error
-				switch k {
-				case api.ResourceMemory:
-					enforced = v.Value()
-					switch limit.Type {
-					case api.LimitTypePod:
-						observed = podMem
-						err = fmt.Errorf("%simum memory usage per pod is %s", minOrMax, v.String())
-					case api.LimitTypeContainer:
-						observed = maxContainerMem
-						err = fmt.Errorf("%simum memory usage per container is %s", minOrMax, v.String())
-					}
-				case api.ResourceCPU:
-					enforced = v.MilliValue()
-					switch limit.Type {
-					case api.LimitTypePod:
-						observed = podCPU
-						err = fmt.Errorf("%simum CPU usage per pod is %s, but requested %s", minOrMax, v.String(), resource.NewMilliQuantity(observed, resource.DecimalSI))
-					case api.LimitTypeContainer:
-						observed = maxContainerCPU
-						err = fmt.Errorf("%simum CPU usage per container is %s", minOrMax, v.String())
-					}
-				}
-				switch minOrMax {
-				case "Min":
-					if observed < enforced {
 						return err
 					}
-				case "Max":
+				}
-					if observed > enforced {
+				for k, v := range limit.Max {
+					if err := maxConstraint(limitType, k, v, container.Resources.Requests, container.Resources.Limits); err != nil {
 						return err
 					}
 				}
+				for k, v := range limit.MaxLimitRequestRatio {
+					if err := limitRequestRatioConstraint(limitType, k, v, container.Resources.Requests, container.Resources.Limits); err != nil {
+						return err
+					}
+				}
+			}
+		}
+
+		// enforce pod limits
+		if limitType == api.LimitTypePod {
+			containerRequests, containerLimits := []api.ResourceList{}, []api.ResourceList{}
+			for j := range pod.Spec.Containers {
+				container := &pod.Spec.Containers[j]
+				containerRequests = append(containerRequests, container.Resources.Requests)
+				containerLimits = append(containerLimits, container.Resources.Limits)
+			}
+			podRequests := sum(containerRequests)
+			podLimits := sum(containerLimits)
+			for k, v := range limit.Min {
+				if err := minConstraint(limitType, k, v, podRequests, podLimits); err != nil {
+					return err
+				}
+			}
+			for k, v := range limit.Max {
+				if err := maxConstraint(limitType, k, v, podRequests, podLimits); err != nil {
+					return err
+				}
+			}
+			for k, v := range limit.MaxLimitRequestRatio {
+				if err := limitRequestRatioConstraint(limitType, k, v, podRequests, podLimits); err != nil {
+					return err
+				}
 			}
 		}
 	}

plugin/pkg/admission/limitranger/admission_test.go

@@ -38,13 +38,34 @@ func getResourceList(cpu, memory string) api.ResourceList {
 	return res
 }
 
-func getResourceRequirements(limits, requests api.ResourceList) api.ResourceRequirements {
+func getResourceRequirements(requests, limits api.ResourceList) api.ResourceRequirements {
 	res := api.ResourceRequirements{}
-	res.Limits = limits
 	res.Requests = requests
+	res.Limits = limits
 	return res
 }
 
+// createLimitRange creates a limit range with the specified data
+func createLimitRange(limitType api.LimitType, min, max, defaultLimit, defaultRequest api.ResourceList) api.LimitRange {
+	return api.LimitRange{
+		ObjectMeta: api.ObjectMeta{
+			Name:      "abc",
+			Namespace: "test",
+		},
+		Spec: api.LimitRangeSpec{
+			Limits: []api.LimitRangeItem{
+				{
+					Type:           limitType,
+					Min:            min,
+					Max:            max,
+					Default:        defaultLimit,
+					DefaultRequest: defaultRequest,
+				},
+			},
+		},
+	}
+}
+
 func validLimitRange() api.LimitRange {
 	return api.LimitRange{
 		ObjectMeta: api.ObjectMeta{
@@ -62,7 +83,8 @@ func validLimitRange() api.LimitRange {
 					Type:           api.LimitTypeContainer,
 					Max:            getResourceList("100m", "2Gi"),
 					Min:            getResourceList("25m", "1Mi"),
-					Default: getResourceList("50m", "5Mi"),
+					Default:        getResourceList("75m", "10Mi"),
+					DefaultRequest: getResourceList("50m", "5Mi"),
 				},
 			},
 		},
@@ -110,8 +132,8 @@ func validPod(name string, numContainers int, resources api.ResourceRequirements
 func TestDefaultContainerResourceRequirements(t *testing.T) {
 	limitRange := validLimitRange()
 	expected := api.ResourceRequirements{
-		Limits:   getResourceList("50m", "5Mi"),
+		Requests: getResourceList("50m", "5Mi"),
-		Requests: api.ResourceList{},
+		Limits:   getResourceList("75m", "10Mi"),
 	}
 
 	actual := defaultContainerResourceRequirements(&limitRange)
@@ -125,7 +147,7 @@ func TestDefaultContainerResourceRequirements(t *testing.T) {
 func TestMergePodResourceRequirements(t *testing.T) {
 	limitRange := validLimitRange()
 
-	// pod with no resources enumerated should get each resource from default
+	// pod with no resources enumerated should get each resource from default request
 	expected := getResourceRequirements(getResourceList("", ""), getResourceList("", ""))
 	pod := validPod("empty-resources", 1, expected)
 	defaultRequirements := defaultContainerResourceRequirements(&limitRange)
@@ -141,11 +163,11 @@ func TestMergePodResourceRequirements(t *testing.T) {
 	input := getResourceRequirements(getResourceList("", "512Mi"), getResourceList("", ""))
 	pod = validPod("limit-memory", 1, input)
 	expected = api.ResourceRequirements{
-		Limits: api.ResourceList{
+		Requests: api.ResourceList{
-			api.ResourceCPU:    defaultRequirements.Limits[api.ResourceCPU],
+			api.ResourceCPU:    defaultRequirements.Requests[api.ResourceCPU],
 			api.ResourceMemory: resource.MustParse("512Mi"),
 		},
-		Requests: api.ResourceList{},
+		Limits: defaultRequirements.Limits,
 	}
 	mergePodResourceRequirements(&pod, &defaultRequirements)
 	for i := range pod.Spec.Containers {
@@ -157,34 +179,172 @@ func TestMergePodResourceRequirements(t *testing.T) {
 }
 
 func TestPodLimitFunc(t *testing.T) {
-	limitRange := validLimitRange()
+	type testCase struct {
-	successCases := []api.Pod{
+		pod        api.Pod
-		validPod("foo", 2, getResourceRequirements(getResourceList("100m", "2Gi"), getResourceList("", ""))),
+		limitRange api.LimitRange
-		validPod("bar", 1, getResourceRequirements(getResourceList("100m", "2Gi"), getResourceList("", ""))),
 	}
 
-	errorCases := map[string]api.Pod{
+	successCases := []testCase{
-		"min-container-cpu": validPod("foo", 1, getResourceRequirements(getResourceList("25m", "2Gi"), getResourceList("", ""))),
+		{
-		"max-container-cpu": validPod("foo", 1, getResourceRequirements(getResourceList("110m", "1Gi"), getResourceList("", ""))),
+			pod:        validPod("ctr-min-cpu-request", 1, getResourceRequirements(getResourceList("100m", ""), getResourceList("", ""))),
-		"min-container-mem": validPod("foo", 1, getResourceRequirements(getResourceList("30m", "0"), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("50m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
-		"max-container-mem": validPod("foo", 1, getResourceRequirements(getResourceList("30m", "3Gi"), getResourceList("", ""))),
+		},
-		"min-pod-cpu":       validPod("foo", 1, getResourceRequirements(getResourceList("40m", "2Gi"), getResourceList("", ""))),
+		{
-		"max-pod-cpu":       validPod("foo", 4, getResourceRequirements(getResourceList("60m", "1Mi"), getResourceList("", ""))),
+			pod:        validPod("ctr-min-cpu-request-limit", 1, getResourceRequirements(getResourceList("100m", ""), getResourceList("200m", ""))),
-		"max-pod-memory":    validPod("foo", 3, getResourceRequirements(getResourceList("60m", "2Gi"), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("50m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
-		"min-pod-memory":    validPod("foo", 3, getResourceRequirements(getResourceList("60m", "0"), getResourceList("", ""))),
+		},
+		{
+			pod:        validPod("ctr-min-memory-request", 1, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("", "50Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-min-memory-request-limit", 1, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", "100Mi"))),
+			limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("", "50Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-max-cpu-request-limit", 1, getResourceRequirements(getResourceList("500m", ""), getResourceList("1", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-max-cpu-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("1", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-max-mem-request-limit", 1, getResourceRequirements(getResourceList("", "250Mi"), getResourceList("", "500Mi"))),
+			limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-max-mem-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", "500Mi"))),
+			limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-min-cpu-request", 2, getResourceRequirements(getResourceList("75m", ""), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypePod, getResourceList("100m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-min-cpu-request-limit", 2, getResourceRequirements(getResourceList("75m", ""), getResourceList("200m", ""))),
+			limitRange: createLimitRange(api.LimitTypePod, getResourceList("100m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-min-memory-request", 2, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypePod, getResourceList("", "100Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-min-memory-request-limit", 2, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", "100Mi"))),
+			limitRange: createLimitRange(api.LimitTypePod, getResourceList("", "100Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-max-cpu-request-limit", 2, getResourceRequirements(getResourceList("500m", ""), getResourceList("1", ""))),
+			limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-max-cpu-limit", 2, getResourceRequirements(getResourceList("", ""), getResourceList("1", ""))),
+			limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-max-mem-request-limit", 2, getResourceRequirements(getResourceList("", "250Mi"), getResourceList("", "500Mi"))),
+			limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-max-mem-limit", 2, getResourceRequirements(getResourceList("", ""), getResourceList("", "500Mi"))),
+			limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}),
+		},
 	}
-
 	for i := range successCases {
-		err := PodLimitFunc(&limitRange, &successCases[i])
+		test := successCases[i]
+		err := PodLimitFunc(&test.limitRange, &test.pod)
 		if err != nil {
-			t.Errorf("Unexpected error for valid pod: %v, %v", successCases[i].Name, err)
+			t.Errorf("Unexpected error for pod: %s, %v", test.pod.Name, err)
 		}
 	}
 
-	for k, v := range errorCases {
+	errorCases := []testCase{
-		err := PodLimitFunc(&limitRange, &v)
+		{
+			pod:        validPod("ctr-min-cpu-request", 1, getResourceRequirements(getResourceList("40m", ""), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("50m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-min-cpu-request-limit", 1, getResourceRequirements(getResourceList("40m", ""), getResourceList("200m", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("50m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-min-cpu-no-request-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("50m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-min-memory-request", 1, getResourceRequirements(getResourceList("", "40Mi"), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("", "50Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-min-memory-request-limit", 1, getResourceRequirements(getResourceList("", "40Mi"), getResourceList("", "100Mi"))),
+			limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("", "50Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-min-memory-no-request-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("", "50Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-max-cpu-request-limit", 1, getResourceRequirements(getResourceList("500m", ""), getResourceList("2500m", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-max-cpu-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("2500m", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-max-cpu-no-request-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-max-mem-request-limit", 1, getResourceRequirements(getResourceList("", "250Mi"), getResourceList("", "2Gi"))),
+			limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-max-mem-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", "2Gi"))),
+			limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("ctr-max-mem-no-request-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-min-cpu-request", 1, getResourceRequirements(getResourceList("75m", ""), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypePod, getResourceList("100m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-min-cpu-request-limit", 1, getResourceRequirements(getResourceList("75m", ""), getResourceList("200m", ""))),
+			limitRange: createLimitRange(api.LimitTypePod, getResourceList("100m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-min-memory-request", 1, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", ""))),
+			limitRange: createLimitRange(api.LimitTypePod, getResourceList("", "100Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-min-memory-request-limit", 1, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", "100Mi"))),
+			limitRange: createLimitRange(api.LimitTypePod, getResourceList("", "100Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-max-cpu-request-limit", 3, getResourceRequirements(getResourceList("500m", ""), getResourceList("1", ""))),
+			limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-max-cpu-limit", 3, getResourceRequirements(getResourceList("", ""), getResourceList("1", ""))),
+			limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-max-mem-request-limit", 3, getResourceRequirements(getResourceList("", "250Mi"), getResourceList("", "500Mi"))),
+			limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}),
+		},
+		{
+			pod:        validPod("pod-max-mem-limit", 3, getResourceRequirements(getResourceList("", ""), getResourceList("", "500Mi"))),
+			limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}),
+		},
+	}
+	for i := range errorCases {
+		test := errorCases[i]
+		err := PodLimitFunc(&test.limitRange, &test.pod)
 		if err == nil {
-			t.Errorf("Expected error for %s", k)
+			t.Errorf("Expected error for pod: %s", test.pod.Name)
 		}
 	}
 }
@@ -199,23 +359,22 @@ func TestPodLimitFuncApplyDefault(t *testing.T) {
 
 	for i := range testPod.Spec.Containers {
 		container := testPod.Spec.Containers[i]
-		memory := testPod.Spec.Containers[i].Resources.Limits.Memory().String()
+		limitMemory := container.Resources.Limits.Memory().String()
-		cpu := testPod.Spec.Containers[i].Resources.Limits.Cpu().String()
+		limitCpu := container.Resources.Limits.Cpu().String()
-		switch container.Image {
+		requestMemory := container.Resources.Requests.Memory().String()
-		case "boo:V1":
+		requestCpu := container.Resources.Requests.Cpu().String()
-			if memory != "100Mi" {
+
-				t.Errorf("Unexpected memory value %s", memory)
+		if limitMemory != "10Mi" {
+			t.Errorf("Unexpected memory value %s", limitMemory)
 		}
-			if cpu != "50m" {
+		if limitCpu != "75m" {
-				t.Errorf("Unexpected cpu value %s", cpu)
+			t.Errorf("Unexpected cpu value %s", limitCpu)
 		}
-		case "foo:V1":
+		if requestMemory != "5Mi" {
-			if memory != "2Gi" {
+			t.Errorf("Unexpected memory value %s", requestMemory)
-				t.Errorf("Unexpected memory value %s", memory)
-			}
-			if cpu != "100m" {
-				t.Errorf("Unexpected cpu value %s", cpu)
 		}
+		if requestCpu != "50m" {
+			t.Errorf("Unexpected cpu value %s", requestCpu)
 		}
 	}
 }