Skip to content

Commit f0fe2a3

Browse files
smarterclaytonsmarterclayton
committed
UPSTREAM: 29588: Properly apply quota for init containers
:100644 100644 9fa480b... 6c76d8d... M pkg/quota/evaluator/core/pods.go :000000 100644 0000000... 57cabf7... A pkg/quota/evaluator/core/pods_test.go :100644 100644 5462854... 13f0af3... M pkg/quota/resources.go :100644 100644 79a3184... 74e55c4... M pkg/quota/resources_test.go
1 parent 5c2adcf commit f0fe2a3

File tree

4 files changed

+338
-10
lines changed

4 files changed

+338
-10
lines changed

‎pkg/quota/evaluator/core/pods.go

+30-10
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,11 @@ func PodConstraintsFunc(required []api.ResourceName, object runtime.Object) erro
7979
allErrs := field.ErrorList{}
8080
fldPath := field.NewPath("spec").Child("containers")
8181
for i, ctr := range pod.Spec.Containers {
82-
idxPath := fldPath.Index(i)
83-
allErrs = append(allErrs, validation.ValidateResourceRequirements(&ctr.Resources, idxPath.Child("resources"))...)
82+
allErrs = append(allErrs, validation.ValidateResourceRequirements(&ctr.Resources, fldPath.Index(i).Child("resources"))...)
83+
}
84+
fldPath = field.NewPath("spec").Child("initContainers")
85+
for i, ctr := range pod.Spec.InitContainers {
86+
allErrs = append(allErrs, validation.ValidateResourceRequirements(&ctr.Resources, fldPath.Index(i).Child("resources"))...)
8487
}
8588
if len(allErrs) > 0 {
8689
return allErrs.ToAggregate()
@@ -92,21 +95,30 @@ func PodConstraintsFunc(required []api.ResourceName, object runtime.Object) erro
9295
requiredSet := quota.ToSet(required)
9396
missingSet := sets.NewString()
9497
for i := range pod.Spec.Containers {
95-
requests := pod.Spec.Containers[i].Resources.Requests
96-
limits := pod.Spec.Containers[i].Resources.Limits
97-
containerUsage := podUsageHelper(requests, limits)
98-
containerSet := quota.ToSet(quota.ResourceNames(containerUsage))
99-
if !containerSet.Equal(requiredSet) {
100-
difference := requiredSet.Difference(containerSet)
101-
missingSet.Insert(difference.List()...)
102-
}
98+
enforcePodContainerConstraints(&pod.Spec.Containers[i], requiredSet, missingSet)
99+
}
100+
for i := range pod.Spec.InitContainers {
101+
enforcePodContainerConstraints(&pod.Spec.InitContainers[i], requiredSet, missingSet)
103102
}
104103
if len(missingSet) == 0 {
105104
return nil
106105
}
107106
return fmt.Errorf("must specify %s", strings.Join(missingSet.List(), ","))
108107
}
109108

109+
// enforcePodContainerConstraints checks for required resources that are not set on this container and
110+
// adds them to missingSet.
111+
func enforcePodContainerConstraints(container *api.Container, requiredSet, missingSet sets.String) {
112+
requests := container.Resources.Requests
113+
limits := container.Resources.Limits
114+
containerUsage := podUsageHelper(requests, limits)
115+
containerSet := quota.ToSet(quota.ResourceNames(containerUsage))
116+
if !containerSet.Equal(requiredSet) {
117+
difference := requiredSet.Difference(containerSet)
118+
missingSet.Insert(difference.List()...)
119+
}
120+
}
121+
110122
// podUsageHelper can summarize the pod quota usage based on requests and limits
111123
func podUsageHelper(requests api.ResourceList, limits api.ResourceList) api.ResourceList {
112124
result := api.ResourceList{}
@@ -144,10 +156,18 @@ func PodUsageFunc(object runtime.Object) api.ResourceList {
144156
// when we have pod level cgroups, we can just read pod level requests/limits
145157
requests := api.ResourceList{}
146158
limits := api.ResourceList{}
159+
147160
for i := range pod.Spec.Containers {
148161
requests = quota.Add(requests, pod.Spec.Containers[i].Resources.Requests)
149162
limits = quota.Add(limits, pod.Spec.Containers[i].Resources.Limits)
150163
}
164+
// InitContainers are run sequentially before other containers start, so the highest
165+
// init container resource is compared against the sum of app containers to determine
166+
// the effective usage for both requests and limits.
167+
for i := range pod.Spec.InitContainers {
168+
requests = quota.Max(requests, pod.Spec.InitContainers[i].Resources.Requests)
169+
limits = quota.Max(limits, pod.Spec.InitContainers[i].Resources.Limits)
170+
}
151171

152172
return podUsageHelper(requests, limits)
153173
}

‎pkg/quota/evaluator/core/pods_test.go

+253
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
/*
2+
Copyright 2016 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package core
18+
19+
import (
20+
"testing"
21+
22+
"k8s.io/kubernetes/pkg/api"
23+
"k8s.io/kubernetes/pkg/api/resource"
24+
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
25+
"k8s.io/kubernetes/pkg/quota"
26+
)
27+
28+
func TestPodConstraintsFunc(t *testing.T) {
29+
testCases := map[string]struct {
30+
pod *api.Pod
31+
required []api.ResourceName
32+
err string
33+
}{
34+
"init container resource invalid": {
35+
pod: &api.Pod{
36+
Spec: api.PodSpec{
37+
InitContainers: []api.Container{{
38+
Resources: api.ResourceRequirements{
39+
Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")},
40+
Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")},
41+
},
42+
}},
43+
},
44+
},
45+
err: `spec.initContainers[0].resources.limits[cpu]: Invalid value: "1m": must be greater than or equal to request`,
46+
},
47+
"container resource invalid": {
48+
pod: &api.Pod{
49+
Spec: api.PodSpec{
50+
Containers: []api.Container{{
51+
Resources: api.ResourceRequirements{
52+
Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")},
53+
Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")},
54+
},
55+
}},
56+
},
57+
},
58+
err: `spec.containers[0].resources.limits[cpu]: Invalid value: "1m": must be greater than or equal to request`,
59+
},
60+
"init container resource missing": {
61+
pod: &api.Pod{
62+
Spec: api.PodSpec{
63+
InitContainers: []api.Container{{
64+
Resources: api.ResourceRequirements{
65+
Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")},
66+
Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")},
67+
},
68+
}},
69+
},
70+
},
71+
required: []api.ResourceName{api.ResourceMemory},
72+
err: `must specify memory`,
73+
},
74+
"container resource missing": {
75+
pod: &api.Pod{
76+
Spec: api.PodSpec{
77+
Containers: []api.Container{{
78+
Resources: api.ResourceRequirements{
79+
Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")},
80+
Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")},
81+
},
82+
}},
83+
},
84+
},
85+
required: []api.ResourceName{api.ResourceMemory},
86+
err: `must specify memory`,
87+
},
88+
}
89+
for testName, test := range testCases {
90+
err := PodConstraintsFunc(test.required, test.pod)
91+
switch {
92+
case err != nil && len(test.err) == 0,
93+
err == nil && len(test.err) != 0,
94+
err != nil && test.err != err.Error():
95+
t.Errorf("%s unexpected error: %v", testName, err)
96+
}
97+
}
98+
}
99+
100+
func TestPodEvaluatorUsage(t *testing.T) {
101+
kubeClient := fake.NewSimpleClientset()
102+
evaluator := NewPodEvaluator(kubeClient)
103+
testCases := map[string]struct {
104+
pod *api.Pod
105+
usage api.ResourceList
106+
}{
107+
"init container CPU": {
108+
pod: &api.Pod{
109+
Spec: api.PodSpec{
110+
InitContainers: []api.Container{{
111+
Resources: api.ResourceRequirements{
112+
Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")},
113+
Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")},
114+
},
115+
}},
116+
},
117+
},
118+
usage: api.ResourceList{
119+
api.ResourceRequestsCPU: resource.MustParse("1m"),
120+
api.ResourceLimitsCPU: resource.MustParse("2m"),
121+
api.ResourcePods: resource.MustParse("1"),
122+
api.ResourceCPU: resource.MustParse("1m"),
123+
},
124+
},
125+
"init container MEM": {
126+
pod: &api.Pod{
127+
Spec: api.PodSpec{
128+
InitContainers: []api.Container{{
129+
Resources: api.ResourceRequirements{
130+
Requests: api.ResourceList{api.ResourceMemory: resource.MustParse("1m")},
131+
Limits: api.ResourceList{api.ResourceMemory: resource.MustParse("2m")},
132+
},
133+
}},
134+
},
135+
},
136+
usage: api.ResourceList{
137+
api.ResourceRequestsMemory: resource.MustParse("1m"),
138+
api.ResourceLimitsMemory: resource.MustParse("2m"),
139+
api.ResourcePods: resource.MustParse("1"),
140+
api.ResourceMemory: resource.MustParse("1m"),
141+
},
142+
},
143+
"container CPU": {
144+
pod: &api.Pod{
145+
Spec: api.PodSpec{
146+
Containers: []api.Container{{
147+
Resources: api.ResourceRequirements{
148+
Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")},
149+
Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")},
150+
},
151+
}},
152+
},
153+
},
154+
usage: api.ResourceList{
155+
api.ResourceRequestsCPU: resource.MustParse("1m"),
156+
api.ResourceLimitsCPU: resource.MustParse("2m"),
157+
api.ResourcePods: resource.MustParse("1"),
158+
api.ResourceCPU: resource.MustParse("1m"),
159+
},
160+
},
161+
"container MEM": {
162+
pod: &api.Pod{
163+
Spec: api.PodSpec{
164+
Containers: []api.Container{{
165+
Resources: api.ResourceRequirements{
166+
Requests: api.ResourceList{api.ResourceMemory: resource.MustParse("1m")},
167+
Limits: api.ResourceList{api.ResourceMemory: resource.MustParse("2m")},
168+
},
169+
}},
170+
},
171+
},
172+
usage: api.ResourceList{
173+
api.ResourceRequestsMemory: resource.MustParse("1m"),
174+
api.ResourceLimitsMemory: resource.MustParse("2m"),
175+
api.ResourcePods: resource.MustParse("1"),
176+
api.ResourceMemory: resource.MustParse("1m"),
177+
},
178+
},
179+
"init container maximums override sum of containers": {
180+
pod: &api.Pod{
181+
Spec: api.PodSpec{
182+
InitContainers: []api.Container{
183+
{
184+
Resources: api.ResourceRequirements{
185+
Requests: api.ResourceList{
186+
api.ResourceCPU: resource.MustParse("4"),
187+
api.ResourceMemory: resource.MustParse("100M"),
188+
},
189+
Limits: api.ResourceList{
190+
api.ResourceCPU: resource.MustParse("8"),
191+
api.ResourceMemory: resource.MustParse("200M"),
192+
},
193+
},
194+
},
195+
{
196+
Resources: api.ResourceRequirements{
197+
Requests: api.ResourceList{
198+
api.ResourceCPU: resource.MustParse("1"),
199+
api.ResourceMemory: resource.MustParse("50M"),
200+
},
201+
Limits: api.ResourceList{
202+
api.ResourceCPU: resource.MustParse("2"),
203+
api.ResourceMemory: resource.MustParse("100M"),
204+
},
205+
},
206+
},
207+
},
208+
Containers: []api.Container{
209+
{
210+
Resources: api.ResourceRequirements{
211+
Requests: api.ResourceList{
212+
api.ResourceCPU: resource.MustParse("1"),
213+
api.ResourceMemory: resource.MustParse("50M"),
214+
},
215+
Limits: api.ResourceList{
216+
api.ResourceCPU: resource.MustParse("2"),
217+
api.ResourceMemory: resource.MustParse("100M"),
218+
},
219+
},
220+
},
221+
{
222+
Resources: api.ResourceRequirements{
223+
Requests: api.ResourceList{
224+
api.ResourceCPU: resource.MustParse("2"),
225+
api.ResourceMemory: resource.MustParse("25M"),
226+
},
227+
Limits: api.ResourceList{
228+
api.ResourceCPU: resource.MustParse("5"),
229+
api.ResourceMemory: resource.MustParse("50M"),
230+
},
231+
},
232+
},
233+
},
234+
},
235+
},
236+
usage: api.ResourceList{
237+
api.ResourceRequestsCPU: resource.MustParse("4"),
238+
api.ResourceRequestsMemory: resource.MustParse("100M"),
239+
api.ResourceLimitsCPU: resource.MustParse("8"),
240+
api.ResourceLimitsMemory: resource.MustParse("200M"),
241+
api.ResourcePods: resource.MustParse("1"),
242+
api.ResourceCPU: resource.MustParse("4"),
243+
api.ResourceMemory: resource.MustParse("100M"),
244+
},
245+
},
246+
}
247+
for testName, testCase := range testCases {
248+
actual := evaluator.Usage(testCase.pod)
249+
if !quota.Equals(testCase.usage, actual) {
250+
t.Errorf("%s expected: %v, actual: %v", testName, testCase.usage, actual)
251+
}
252+
}
253+
}

‎pkg/quota/resources.go

+20
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,26 @@ func LessThanOrEqual(a api.ResourceList, b api.ResourceList) (bool, []api.Resour
6161
return result, resourceNames
6262
}
6363

64+
// Max returns the result of Max(a, b) for each named resource
65+
func Max(a api.ResourceList, b api.ResourceList) api.ResourceList {
66+
result := api.ResourceList{}
67+
for key, value := range a {
68+
if other, found := b[key]; found {
69+
if value.Cmp(other) <= 0 {
70+
result[key] = *other.Copy()
71+
continue
72+
}
73+
}
74+
result[key] = *value.Copy()
75+
}
76+
for key, value := range b {
77+
if _, found := result[key]; !found {
78+
result[key] = *value.Copy()
79+
}
80+
}
81+
return result
82+
}
83+
6484
// Add returns the result of a + b for each named resource
6585
func Add(a api.ResourceList, b api.ResourceList) api.ResourceList {
6686
result := api.ResourceList{}

‎pkg/quota/resources_test.go

+35
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,41 @@ func TestEquals(t *testing.T) {
7676
}
7777
}
7878

79+
func TestMax(t *testing.T) {
80+
testCases := map[string]struct {
81+
a api.ResourceList
82+
b api.ResourceList
83+
expected api.ResourceList
84+
}{
85+
"noKeys": {
86+
a: api.ResourceList{},
87+
b: api.ResourceList{},
88+
expected: api.ResourceList{},
89+
},
90+
"toEmpty": {
91+
a: api.ResourceList{api.ResourceCPU: resource.MustParse("100m")},
92+
b: api.ResourceList{},
93+
expected: api.ResourceList{api.ResourceCPU: resource.MustParse("100m")},
94+
},
95+
"matching": {
96+
a: api.ResourceList{api.ResourceCPU: resource.MustParse("100m")},
97+
b: api.ResourceList{api.ResourceCPU: resource.MustParse("150m")},
98+
expected: api.ResourceList{api.ResourceCPU: resource.MustParse("150m")},
99+
},
100+
"matching(reverse)": {
101+
a: api.ResourceList{api.ResourceCPU: resource.MustParse("150m")},
102+
b: api.ResourceList{api.ResourceCPU: resource.MustParse("100m")},
103+
expected: api.ResourceList{api.ResourceCPU: resource.MustParse("150m")},
104+
},
105+
}
106+
for testName, testCase := range testCases {
107+
sum := Max(testCase.a, testCase.b)
108+
if result := Equals(testCase.expected, sum); !result {
109+
t.Errorf("%s expected: %v, actual: %v", testName, testCase.expected, sum)
110+
}
111+
}
112+
}
113+
79114
func TestAdd(t *testing.T) {
80115
testCases := map[string]struct {
81116
a api.ResourceList

0 commit comments

Comments
 (0)