From ba33f9a1ce2a43ad19909c6ef7efe6c6823b8db8 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Sun, 31 Jul 2016 22:51:02 -0400 Subject: [PATCH] UPSTREAM: 29588: Properly apply quota for init containers --- .../pkg/quota/evaluator/core/pods.go | 40 ++- .../pkg/quota/evaluator/core/pods_test.go | 253 ++++++++++++++++++ .../k8s.io/kubernetes/pkg/quota/resources.go | 20 ++ .../kubernetes/pkg/quota/resources_test.go | 35 +++ 4 files changed, 338 insertions(+), 10 deletions(-) create mode 100644 vendor/k8s.io/kubernetes/pkg/quota/evaluator/core/pods_test.go diff --git a/vendor/k8s.io/kubernetes/pkg/quota/evaluator/core/pods.go b/vendor/k8s.io/kubernetes/pkg/quota/evaluator/core/pods.go index 9fa480b797a4..6c76d8d6fd19 100644 --- a/vendor/k8s.io/kubernetes/pkg/quota/evaluator/core/pods.go +++ b/vendor/k8s.io/kubernetes/pkg/quota/evaluator/core/pods.go @@ -79,8 +79,11 @@ func PodConstraintsFunc(required []api.ResourceName, object runtime.Object) erro allErrs := field.ErrorList{} fldPath := field.NewPath("spec").Child("containers") for i, ctr := range pod.Spec.Containers { - idxPath := fldPath.Index(i) - allErrs = append(allErrs, validation.ValidateResourceRequirements(&ctr.Resources, idxPath.Child("resources"))...) + allErrs = append(allErrs, validation.ValidateResourceRequirements(&ctr.Resources, fldPath.Index(i).Child("resources"))...) + } + fldPath = field.NewPath("spec").Child("initContainers") + for i, ctr := range pod.Spec.InitContainers { + allErrs = append(allErrs, validation.ValidateResourceRequirements(&ctr.Resources, fldPath.Index(i).Child("resources"))...) } if len(allErrs) > 0 { return allErrs.ToAggregate() @@ -92,14 +95,10 @@ func PodConstraintsFunc(required []api.ResourceName, object runtime.Object) erro requiredSet := quota.ToSet(required) missingSet := sets.NewString() for i := range pod.Spec.Containers { - requests := pod.Spec.Containers[i].Resources.Requests - limits := pod.Spec.Containers[i].Resources.Limits - containerUsage := podUsageHelper(requests, limits) - containerSet := quota.ToSet(quota.ResourceNames(containerUsage)) - if !containerSet.Equal(requiredSet) { - difference := requiredSet.Difference(containerSet) - missingSet.Insert(difference.List()...) - } + enforcePodContainerConstraints(&pod.Spec.Containers[i], requiredSet, missingSet) + } + for i := range pod.Spec.InitContainers { + enforcePodContainerConstraints(&pod.Spec.InitContainers[i], requiredSet, missingSet) } if len(missingSet) == 0 { return nil @@ -107,6 +106,19 @@ func PodConstraintsFunc(required []api.ResourceName, object runtime.Object) erro return fmt.Errorf("must specify %s", strings.Join(missingSet.List(), ",")) } +// enforcePodContainerConstraints checks for required resources that are not set on this container and +// adds them to missingSet. +func enforcePodContainerConstraints(container *api.Container, requiredSet, missingSet sets.String) { + requests := container.Resources.Requests + limits := container.Resources.Limits + containerUsage := podUsageHelper(requests, limits) + containerSet := quota.ToSet(quota.ResourceNames(containerUsage)) + if !containerSet.Equal(requiredSet) { + difference := requiredSet.Difference(containerSet) + missingSet.Insert(difference.List()...) + } +} + // podUsageHelper can summarize the pod quota usage based on requests and limits func podUsageHelper(requests api.ResourceList, limits api.ResourceList) api.ResourceList { result := api.ResourceList{} @@ -144,10 +156,18 @@ func PodUsageFunc(object runtime.Object) api.ResourceList { // when we have pod level cgroups, we can just read pod level requests/limits requests := api.ResourceList{} limits := api.ResourceList{} + for i := range pod.Spec.Containers { requests = quota.Add(requests, pod.Spec.Containers[i].Resources.Requests) limits = quota.Add(limits, pod.Spec.Containers[i].Resources.Limits) } + // InitContainers are run sequentially before other containers start, so the highest + // init container resource is compared against the sum of app containers to determine + // the effective usage for both requests and limits. + for i := range pod.Spec.InitContainers { + requests = quota.Max(requests, pod.Spec.InitContainers[i].Resources.Requests) + limits = quota.Max(limits, pod.Spec.InitContainers[i].Resources.Limits) + } return podUsageHelper(requests, limits) } diff --git a/vendor/k8s.io/kubernetes/pkg/quota/evaluator/core/pods_test.go b/vendor/k8s.io/kubernetes/pkg/quota/evaluator/core/pods_test.go new file mode 100644 index 000000000000..57cabf7afa61 --- /dev/null +++ b/vendor/k8s.io/kubernetes/pkg/quota/evaluator/core/pods_test.go @@ -0,0 +1,253 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package core + +import ( + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/resource" + "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" + "k8s.io/kubernetes/pkg/quota" +) + +func TestPodConstraintsFunc(t *testing.T) { + testCases := map[string]struct { + pod *api.Pod + required []api.ResourceName + err string + }{ + "init container resource invalid": { + pod: &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{{ + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, + Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, + }, + }}, + }, + }, + err: `spec.initContainers[0].resources.limits[cpu]: Invalid value: "1m": must be greater than or equal to request`, + }, + "container resource invalid": { + pod: &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{{ + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, + Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, + }, + }}, + }, + }, + err: `spec.containers[0].resources.limits[cpu]: Invalid value: "1m": must be greater than or equal to request`, + }, + "init container resource missing": { + pod: &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{{ + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, + Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, + }, + }}, + }, + }, + required: []api.ResourceName{api.ResourceMemory}, + err: `must specify memory`, + }, + "container resource missing": { + pod: &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{{ + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, + Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, + }, + }}, + }, + }, + required: []api.ResourceName{api.ResourceMemory}, + err: `must specify memory`, + }, + } + for testName, test := range testCases { + err := PodConstraintsFunc(test.required, test.pod) + switch { + case err != nil && len(test.err) == 0, + err == nil && len(test.err) != 0, + err != nil && test.err != err.Error(): + t.Errorf("%s unexpected error: %v", testName, err) + } + } +} + +func TestPodEvaluatorUsage(t *testing.T) { + kubeClient := fake.NewSimpleClientset() + evaluator := NewPodEvaluator(kubeClient) + testCases := map[string]struct { + pod *api.Pod + usage api.ResourceList + }{ + "init container CPU": { + pod: &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{{ + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, + Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, + }, + }}, + }, + }, + usage: api.ResourceList{ + api.ResourceRequestsCPU: resource.MustParse("1m"), + api.ResourceLimitsCPU: resource.MustParse("2m"), + api.ResourcePods: resource.MustParse("1"), + api.ResourceCPU: resource.MustParse("1m"), + }, + }, + "init container MEM": { + pod: &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{{ + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceMemory: resource.MustParse("1m")}, + Limits: api.ResourceList{api.ResourceMemory: resource.MustParse("2m")}, + }, + }}, + }, + }, + usage: api.ResourceList{ + api.ResourceRequestsMemory: resource.MustParse("1m"), + api.ResourceLimitsMemory: resource.MustParse("2m"), + api.ResourcePods: resource.MustParse("1"), + api.ResourceMemory: resource.MustParse("1m"), + }, + }, + "container CPU": { + pod: &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{{ + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, + Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, + }, + }}, + }, + }, + usage: api.ResourceList{ + api.ResourceRequestsCPU: resource.MustParse("1m"), + api.ResourceLimitsCPU: resource.MustParse("2m"), + api.ResourcePods: resource.MustParse("1"), + api.ResourceCPU: resource.MustParse("1m"), + }, + }, + "container MEM": { + pod: &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{{ + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceMemory: resource.MustParse("1m")}, + Limits: api.ResourceList{api.ResourceMemory: resource.MustParse("2m")}, + }, + }}, + }, + }, + usage: api.ResourceList{ + api.ResourceRequestsMemory: resource.MustParse("1m"), + api.ResourceLimitsMemory: resource.MustParse("2m"), + api.ResourcePods: resource.MustParse("1"), + api.ResourceMemory: resource.MustParse("1m"), + }, + }, + "init container maximums override sum of containers": { + pod: &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceCPU: resource.MustParse("4"), + api.ResourceMemory: resource.MustParse("100M"), + }, + Limits: api.ResourceList{ + api.ResourceCPU: resource.MustParse("8"), + api.ResourceMemory: resource.MustParse("200M"), + }, + }, + }, + { + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceCPU: resource.MustParse("1"), + api.ResourceMemory: resource.MustParse("50M"), + }, + Limits: api.ResourceList{ + api.ResourceCPU: resource.MustParse("2"), + api.ResourceMemory: resource.MustParse("100M"), + }, + }, + }, + }, + Containers: []api.Container{ + { + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceCPU: resource.MustParse("1"), + api.ResourceMemory: resource.MustParse("50M"), + }, + Limits: api.ResourceList{ + api.ResourceCPU: resource.MustParse("2"), + api.ResourceMemory: resource.MustParse("100M"), + }, + }, + }, + { + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceCPU: resource.MustParse("2"), + api.ResourceMemory: resource.MustParse("25M"), + }, + Limits: api.ResourceList{ + api.ResourceCPU: resource.MustParse("5"), + api.ResourceMemory: resource.MustParse("50M"), + }, + }, + }, + }, + }, + }, + usage: api.ResourceList{ + api.ResourceRequestsCPU: resource.MustParse("4"), + api.ResourceRequestsMemory: resource.MustParse("100M"), + api.ResourceLimitsCPU: resource.MustParse("8"), + api.ResourceLimitsMemory: resource.MustParse("200M"), + api.ResourcePods: resource.MustParse("1"), + api.ResourceCPU: resource.MustParse("4"), + api.ResourceMemory: resource.MustParse("100M"), + }, + }, + } + for testName, testCase := range testCases { + actual := evaluator.Usage(testCase.pod) + if !quota.Equals(testCase.usage, actual) { + t.Errorf("%s expected: %v, actual: %v", testName, testCase.usage, actual) + } + } +} diff --git a/vendor/k8s.io/kubernetes/pkg/quota/resources.go b/vendor/k8s.io/kubernetes/pkg/quota/resources.go index 5462854e1bb1..13f0af33ff07 100644 --- a/vendor/k8s.io/kubernetes/pkg/quota/resources.go +++ b/vendor/k8s.io/kubernetes/pkg/quota/resources.go @@ -61,6 +61,26 @@ func LessThanOrEqual(a api.ResourceList, b api.ResourceList) (bool, []api.Resour return result, resourceNames } +// Max returns the result of Max(a, b) for each named resource +func Max(a api.ResourceList, b api.ResourceList) api.ResourceList { + result := api.ResourceList{} + for key, value := range a { + if other, found := b[key]; found { + if value.Cmp(other) <= 0 { + result[key] = *other.Copy() + continue + } + } + result[key] = *value.Copy() + } + for key, value := range b { + if _, found := result[key]; !found { + result[key] = *value.Copy() + } + } + return result +} + // Add returns the result of a + b for each named resource func Add(a api.ResourceList, b api.ResourceList) api.ResourceList { result := api.ResourceList{} diff --git a/vendor/k8s.io/kubernetes/pkg/quota/resources_test.go b/vendor/k8s.io/kubernetes/pkg/quota/resources_test.go index 79a3184f05c2..74e55c44164d 100644 --- a/vendor/k8s.io/kubernetes/pkg/quota/resources_test.go +++ b/vendor/k8s.io/kubernetes/pkg/quota/resources_test.go @@ -76,6 +76,41 @@ func TestEquals(t *testing.T) { } } +func TestMax(t *testing.T) { + testCases := map[string]struct { + a api.ResourceList + b api.ResourceList + expected api.ResourceList + }{ + "noKeys": { + a: api.ResourceList{}, + b: api.ResourceList{}, + expected: api.ResourceList{}, + }, + "toEmpty": { + a: api.ResourceList{api.ResourceCPU: resource.MustParse("100m")}, + b: api.ResourceList{}, + expected: api.ResourceList{api.ResourceCPU: resource.MustParse("100m")}, + }, + "matching": { + a: api.ResourceList{api.ResourceCPU: resource.MustParse("100m")}, + b: api.ResourceList{api.ResourceCPU: resource.MustParse("150m")}, + expected: api.ResourceList{api.ResourceCPU: resource.MustParse("150m")}, + }, + "matching(reverse)": { + a: api.ResourceList{api.ResourceCPU: resource.MustParse("150m")}, + b: api.ResourceList{api.ResourceCPU: resource.MustParse("100m")}, + expected: api.ResourceList{api.ResourceCPU: resource.MustParse("150m")}, + }, + } + for testName, testCase := range testCases { + sum := Max(testCase.a, testCase.b) + if result := Equals(testCase.expected, sum); !result { + t.Errorf("%s expected: %v, actual: %v", testName, testCase.expected, sum) + } + } +} + func TestAdd(t *testing.T) { testCases := map[string]struct { a api.ResourceList