mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-23 19:56:01 +00:00
Init container quota is inaccurate
Usage charged should be max of greater of init container or all regular containers. Also, need to validate init container inputs
This commit is contained in:
Clayton Coleman
parent
ed3a29bd6a
commit
958d78cb10
@ -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)
|
||||
}
|
||||
|
||||
253
pkg/quota/evaluator/core/pods_test.go
Normal file
253
pkg/quota/evaluator/core/pods_test.go
Normal file
@ -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: Invalid value: "1m": must be greater than or equal to cpu 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: Invalid value: "1m": must be greater than or equal to cpu 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)
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -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{}
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user