From f2008152f4382add2bbc617e46390f9ea91084e9 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Fri, 8 Apr 2016 11:20:33 -0400 Subject: [PATCH] Update limit ranging to handle init containers --- plugin/pkg/admission/limitranger/admission.go | 99 ++++++++++++------- .../admission/limitranger/admission_test.go | 88 ++++++++++++++++- 2 files changed, 150 insertions(+), 37 deletions(-) diff --git a/plugin/pkg/admission/limitranger/admission.go b/plugin/pkg/admission/limitranger/admission.go index c8f3acd0cd0..6b6d8331fe4 100644 --- a/plugin/pkg/admission/limitranger/admission.go +++ b/plugin/pkg/admission/limitranger/admission.go @@ -208,45 +208,54 @@ func defaultContainerResourceRequirements(limitRange *api.LimitRange) api.Resour return requirements } +// mergeContainerResources handles defaulting all of the resources on a container. +func mergeContainerResources(container *api.Container, defaultRequirements *api.ResourceRequirements, annotationPrefix string, annotations []string) []string { + setRequests := []string{} + setLimits := []string{} + if container.Resources.Limits == nil { + container.Resources.Limits = api.ResourceList{} + } + if container.Resources.Requests == nil { + container.Resources.Requests = api.ResourceList{} + } + for k, v := range defaultRequirements.Limits { + _, found := container.Resources.Limits[k] + if !found { + container.Resources.Limits[k] = *v.Copy() + setLimits = append(setLimits, string(k)) + } + } + for k, v := range defaultRequirements.Requests { + _, found := container.Resources.Requests[k] + if !found { + container.Resources.Requests[k] = *v.Copy() + setRequests = append(setRequests, string(k)) + } + } + if len(setRequests) > 0 { + sort.Strings(setRequests) + a := strings.Join(setRequests, ", ") + fmt.Sprintf(" request for %s %s", annotationPrefix, container.Name) + annotations = append(annotations, a) + } + if len(setLimits) > 0 { + sort.Strings(setLimits) + a := strings.Join(setLimits, ", ") + fmt.Sprintf(" limit for %s %s", annotationPrefix, container.Name) + annotations = append(annotations, a) + } + return annotations +} + // mergePodResourceRequirements merges enumerated requirements with default requirements // it annotates the pod with information about what requirements were modified func mergePodResourceRequirements(pod *api.Pod, defaultRequirements *api.ResourceRequirements) { annotations := []string{} for i := range pod.Spec.Containers { - container := &pod.Spec.Containers[i] - setRequests := []string{} - setLimits := []string{} - if container.Resources.Limits == nil { - container.Resources.Limits = api.ResourceList{} - } - if container.Resources.Requests == nil { - container.Resources.Requests = api.ResourceList{} - } - for k, v := range defaultRequirements.Limits { - _, found := container.Resources.Limits[k] - if !found { - container.Resources.Limits[k] = *v.Copy() - setLimits = append(setLimits, string(k)) - } - } - for k, v := range defaultRequirements.Requests { - _, found := container.Resources.Requests[k] - if !found { - container.Resources.Requests[k] = *v.Copy() - setRequests = append(setRequests, string(k)) - } - } - if len(setRequests) > 0 { - sort.Strings(setRequests) - a := strings.Join(setRequests, ", ") + " request for container " + container.Name - annotations = append(annotations, a) - } - if len(setLimits) > 0 { - sort.Strings(setLimits) - a := strings.Join(setLimits, ", ") + " limit for container " + container.Name - annotations = append(annotations, a) - } + annotations = mergeContainerResources(&pod.Spec.Containers[i], defaultRequirements, "container", annotations) + } + + for i := range pod.Spec.InitContainers { + annotations = mergeContainerResources(&pod.Spec.InitContainers[i], defaultRequirements, "init container", annotations) } if len(annotations) > 0 { @@ -441,7 +450,7 @@ func PodLimitFunc(limitRange *api.LimitRange, pod *api.Pod) error { } } - // enforce pod limits + // enforce pod limits on init containers if limitType == api.LimitTypePod { containerRequests, containerLimits := []api.ResourceList{}, []api.ResourceList{} for j := range pod.Spec.Containers { @@ -451,6 +460,28 @@ func PodLimitFunc(limitRange *api.LimitRange, pod *api.Pod) error { } podRequests := sum(containerRequests) podLimits := sum(containerLimits) + for j := range pod.Spec.InitContainers { + container := &pod.Spec.InitContainers[j] + // take max(sum_containers, any_init_container) + for k, v := range container.Resources.Requests { + if v2, ok := podRequests[k]; ok { + if v.Cmp(v2) > 0 { + podRequests[k] = v + } + } else { + podRequests[k] = v + } + } + for k, v := range container.Resources.Limits { + if v2, ok := podLimits[k]; ok { + if v.Cmp(v2) > 0 { + podLimits[k] = v + } + } else { + podLimits[k] = v + } + } + } for k, v := range limit.Min { if err := minConstraint(limitType, k, v, podRequests, podLimits); err != nil { errs = append(errs, err) diff --git a/plugin/pkg/admission/limitranger/admission_test.go b/plugin/pkg/admission/limitranger/admission_test.go index 1b36f7342a1..c83edf9d701 100644 --- a/plugin/pkg/admission/limitranger/admission_test.go +++ b/plugin/pkg/admission/limitranger/admission_test.go @@ -134,6 +134,17 @@ func validPod(name string, numContainers int, resources api.ResourceRequirements return pod } +func validPodInit(pod api.Pod, resources ...api.ResourceRequirements) api.Pod { + for i := 0; i < len(resources); i++ { + pod.Spec.InitContainers = append(pod.Spec.InitContainers, api.Container{ + Image: "foo:V" + strconv.Itoa(i), + Resources: resources[i], + Name: "foo-" + strconv.Itoa(i), + }) + } + return pod +} + func TestDefaultContainerResourceRequirements(t *testing.T) { limitRange := validLimitRange() expected := api.ResourceRequirements{ @@ -183,7 +194,7 @@ func TestMergePodResourceRequirements(t *testing.T) { // pod with some resources enumerated should only merge empty input := getResourceRequirements(getResourceList("", "512Mi"), getResourceList("", "")) - pod = validPod("limit-memory", 1, input) + pod = validPodInit(validPod("limit-memory", 1, input), input) expected = api.ResourceRequirements{ Requests: api.ResourceList{ api.ResourceCPU: defaultRequirements.Requests[api.ResourceCPU], @@ -198,11 +209,18 @@ func TestMergePodResourceRequirements(t *testing.T) { t.Errorf("pod %v, expected != actual; %v != %v", pod.Name, expected, actual) } } + for i := range pod.Spec.InitContainers { + actual := pod.Spec.InitContainers[i].Resources + if !api.Semantic.DeepEqual(expected, actual) { + t.Errorf("pod %v, expected != actual; %v != %v", pod.Name, expected, actual) + } + } verifyAnnotation(t, &pod, "LimitRanger plugin set: cpu request for container foo-0; cpu, memory limit for container foo-0") // pod with all resources enumerated should not merge anything input = getResourceRequirements(getResourceList("100m", "512Mi"), getResourceList("200m", "1G")) - pod = validPod("limit-memory", 1, input) + initInputs := []api.ResourceRequirements{getResourceRequirements(getResourceList("200m", "1G"), getResourceList("400m", "2G"))} + pod = validPodInit(validPod("limit-memory", 1, input), initInputs...) expected = input mergePodResourceRequirements(&pod, &defaultRequirements) for i := range pod.Spec.Containers { @@ -211,6 +229,12 @@ func TestMergePodResourceRequirements(t *testing.T) { t.Errorf("pod %v, expected != actual; %v != %v", pod.Name, expected, actual) } } + for i := range pod.Spec.InitContainers { + actual := pod.Spec.InitContainers[i].Resources + if !api.Semantic.DeepEqual(initInputs[i], actual) { + t.Errorf("pod %v, expected != actual; %v != %v", pod.Name, initInputs[i], actual) + } + } expectNoAnnotation(t, &pod) } @@ -273,6 +297,20 @@ func TestPodLimitFunc(t *testing.T) { 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{}, api.ResourceList{}), }, + { + pod: validPodInit( + validPod("pod-init-min-memory-request", 2, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", ""))), + getResourceRequirements(getResourceList("", "100Mi"), getResourceList("", "")), + ), + limitRange: createLimitRange(api.LimitTypePod, getResourceList("", "100Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPodInit( + validPod("pod-init-min-memory-request-limit", 2, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", "100Mi"))), + getResourceRequirements(getResourceList("", "80Mi"), getResourceList("", "100Mi")), + ), + limitRange: createLimitRange(api.LimitTypePod, getResourceList("", "100Mi"), api.ResourceList{}, 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{}, api.ResourceList{}), @@ -281,6 +319,22 @@ func TestPodLimitFunc(t *testing.T) { pod: validPod("pod-max-cpu-limit", 2, getResourceRequirements(getResourceList("", ""), getResourceList("1", ""))), limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), }, + { + pod: validPodInit( + validPod("pod-init-max-cpu-request-limit", 2, getResourceRequirements(getResourceList("500m", ""), getResourceList("1", ""))), + getResourceRequirements(getResourceList("1", ""), getResourceList("2", "")), + getResourceRequirements(getResourceList("1", ""), getResourceList("1", "")), + ), + limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPodInit( + validPod("pod-init-max-cpu-limit", 2, getResourceRequirements(getResourceList("", ""), getResourceList("1", ""))), + getResourceRequirements(getResourceList("", ""), getResourceList("2", "")), + getResourceRequirements(getResourceList("", ""), getResourceList("2", "")), + ), + limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, 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{}, api.ResourceList{}), @@ -387,6 +441,13 @@ func TestPodLimitFunc(t *testing.T) { pod: validPod("pod-max-mem-limit", 3, getResourceRequirements(getResourceList("", ""), getResourceList("", "500Mi"))), limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), }, + { + pod: validPodInit( + validPod("pod-init-max-mem-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", "500Mi"))), + getResourceRequirements(getResourceList("", ""), getResourceList("", "1.5Gi")), + ), + limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, { pod: validPod("pod-max-mem-ratio", 3, getResourceRequirements(getResourceList("", "250Mi"), getResourceList("", "500Mi"))), limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("", "2Gi"), api.ResourceList{}, api.ResourceList{}, getResourceList("", "1.5")), @@ -403,7 +464,7 @@ func TestPodLimitFunc(t *testing.T) { func TestPodLimitFuncApplyDefault(t *testing.T) { limitRange := validLimitRange() - testPod := validPod("foo", 1, getResourceRequirements(api.ResourceList{}, api.ResourceList{})) + testPod := validPodInit(validPod("foo", 1, getResourceRequirements(api.ResourceList{}, api.ResourceList{})), getResourceRequirements(api.ResourceList{}, api.ResourceList{})) err := PodLimitFunc(&limitRange, &testPod) if err != nil { t.Errorf("Unexpected error for valid pod: %v, %v", testPod.Name, err) @@ -429,6 +490,27 @@ func TestPodLimitFuncApplyDefault(t *testing.T) { t.Errorf("Unexpected cpu value %s", requestCpu) } } + + for i := range testPod.Spec.InitContainers { + container := testPod.Spec.InitContainers[i] + limitMemory := container.Resources.Limits.Memory().String() + limitCpu := container.Resources.Limits.Cpu().String() + requestMemory := container.Resources.Requests.Memory().String() + requestCpu := container.Resources.Requests.Cpu().String() + + if limitMemory != "10Mi" { + t.Errorf("Unexpected memory value %s", limitMemory) + } + if limitCpu != "75m" { + t.Errorf("Unexpected cpu value %s", limitCpu) + } + if requestMemory != "5Mi" { + t.Errorf("Unexpected memory value %s", requestMemory) + } + if requestCpu != "50m" { + t.Errorf("Unexpected cpu value %s", requestCpu) + } + } } func TestLimitRangerIgnoresSubresource(t *testing.T) {