diff --git a/pkg/quota/v1/evaluator/core/pods.go b/pkg/quota/v1/evaluator/core/pods.go index daf15c48a3f..a0aab81842d 100644 --- a/pkg/quota/v1/evaluator/core/pods.go +++ b/pkg/quota/v1/evaluator/core/pods.go @@ -125,17 +125,25 @@ func (p *podEvaluator) Constraints(required []corev1.ResourceName, item runtime. // validation with resource counting, but we did this before QoS was even defined. // let's not make that mistake again with other resources now that QoS is defined. requiredSet := quota.ToSet(required).Intersection(validationSet) - missingSet := sets.NewString() + missingSetResourceToContainerNames := make(map[string]sets.String) for i := range pod.Spec.Containers { - enforcePodContainerConstraints(&pod.Spec.Containers[i], requiredSet, missingSet) + enforcePodContainerConstraints(&pod.Spec.Containers[i], requiredSet, missingSetResourceToContainerNames) } for i := range pod.Spec.InitContainers { - enforcePodContainerConstraints(&pod.Spec.InitContainers[i], requiredSet, missingSet) + enforcePodContainerConstraints(&pod.Spec.InitContainers[i], requiredSet, missingSetResourceToContainerNames) } - if len(missingSet) == 0 { + if len(missingSetResourceToContainerNames) == 0 { return nil } - return fmt.Errorf("must specify %s", strings.Join(missingSet.List(), ",")) + var resources = sets.NewString() + for resource := range missingSetResourceToContainerNames { + resources.Insert(resource) + } + var errorMessages = make([]string, 0, len(missingSetResourceToContainerNames)) + for _, resource := range resources.List() { + errorMessages = append(errorMessages, fmt.Sprintf("%s for: %s", resource, strings.Join(missingSetResourceToContainerNames[resource].List(), ","))) + } + return fmt.Errorf("must specify %s", strings.Join(errorMessages, "; ")) } // GroupResource that this evaluator tracks @@ -225,14 +233,21 @@ var _ quota.Evaluator = &podEvaluator{} // enforcePodContainerConstraints checks for required resources that are not set on this container and // adds them to missingSet. -func enforcePodContainerConstraints(container *corev1.Container, requiredSet, missingSet sets.String) { +func enforcePodContainerConstraints(container *corev1.Container, requiredSet sets.String, missingSetResourceToContainerNames map[string]sets.String) { requests := container.Resources.Requests limits := container.Resources.Limits containerUsage := podComputeUsageHelper(requests, limits) containerSet := quota.ToSet(quota.ResourceNames(containerUsage)) if !containerSet.Equal(requiredSet) { - difference := requiredSet.Difference(containerSet) - missingSet.Insert(difference.List()...) + if difference := requiredSet.Difference(containerSet); difference.Len() != 0 { + for _, diff := range difference.List() { + if _, ok := missingSetResourceToContainerNames[diff]; !ok { + missingSetResourceToContainerNames[diff] = sets.NewString(container.Name) + } else { + missingSetResourceToContainerNames[diff].Insert(container.Name) + } + } + } } } diff --git a/pkg/quota/v1/evaluator/core/pods_test.go b/pkg/quota/v1/evaluator/core/pods_test.go index afe65e2c18b..5ee3f70a19d 100644 --- a/pkg/quota/v1/evaluator/core/pods_test.go +++ b/pkg/quota/v1/evaluator/core/pods_test.go @@ -21,7 +21,6 @@ import ( "time" "github.com/google/go-cmp/cmp" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -47,6 +46,7 @@ func TestPodConstraintsFunc(t *testing.T) { pod: &api.Pod{ Spec: api.PodSpec{ InitContainers: []api.Container{{ + Name: "dummy", Resources: api.ResourceRequirements{ Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, @@ -55,12 +55,34 @@ func TestPodConstraintsFunc(t *testing.T) { }, }, required: []corev1.ResourceName{corev1.ResourceMemory}, - err: `must specify memory`, + err: `must specify memory for: dummy`, + }, + "multiple init container resource missing": { + pod: &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{{ + Name: "foo", + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, + Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, + }, + }, { + Name: "bar", + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, + Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, + }, + }}, + }, + }, + required: []corev1.ResourceName{corev1.ResourceMemory}, + err: `must specify memory for: bar,foo`, }, "container resource missing": { pod: &api.Pod{ Spec: api.PodSpec{ Containers: []api.Container{{ + Name: "dummy", Resources: api.ResourceRequirements{ Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, @@ -69,7 +91,43 @@ func TestPodConstraintsFunc(t *testing.T) { }, }, required: []corev1.ResourceName{corev1.ResourceMemory}, - err: `must specify memory`, + err: `must specify memory for: dummy`, + }, + "multiple container resource missing": { + pod: &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{{ + Name: "foo", + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, + Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, + }, + }, { + Name: "bar", + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, + Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, + }, + }}, + }, + }, + required: []corev1.ResourceName{corev1.ResourceMemory}, + err: `must specify memory for: bar,foo`, + }, + "container resource missing multiple": { + pod: &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{{ + Name: "foo", + Resources: api.ResourceRequirements{}, + }, { + Name: "bar", + Resources: api.ResourceRequirements{}, + }}, + }, + }, + required: []corev1.ResourceName{corev1.ResourceMemory, corev1.ResourceCPU}, + err: `must specify cpu for: bar,foo; memory for: bar,foo`, }, } evaluator := NewPodEvaluator(nil, clock.RealClock{}) @@ -79,7 +137,7 @@ func TestPodConstraintsFunc(t *testing.T) { 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) + t.Errorf("%s want: %v,got: %v", testName, test.err, err) } } }