diff --git a/pkg/quota/evaluator/core/pods.go b/pkg/quota/evaluator/core/pods.go index da0b18a5e3c..27a6ce83c8c 100644 --- a/pkg/quota/evaluator/core/pods.go +++ b/pkg/quota/evaluator/core/pods.go @@ -23,12 +23,14 @@ import ( "k8s.io/kubernetes/pkg/admission" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/resource" + "k8s.io/kubernetes/pkg/api/validation" clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/kubelet/qos/util" "k8s.io/kubernetes/pkg/quota" "k8s.io/kubernetes/pkg/quota/generic" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util/sets" + "k8s.io/kubernetes/pkg/util/validation/field" ) // NewPodEvaluator returns an evaluator that can evaluate pods @@ -64,12 +66,26 @@ func NewPodEvaluator(kubeClient clientset.Interface) quota.Evaluator { } // PodConstraintsFunc verifies that all required resources are present on the pod +// In addition, it validates that the resources are valid (i.e. requests < limits) func PodConstraintsFunc(required []api.ResourceName, object runtime.Object) error { pod, ok := object.(*api.Pod) if !ok { return fmt.Errorf("Unexpected input object %v", object) } + // Pod level resources are often set during admission control + // As a consequence, we want to verify that resources are valid prior + // to ever charging quota prematurely in case they are not. + 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"))...) + } + if len(allErrs) > 0 { + return allErrs.ToAggregate() + } + // TODO: fix this when we have pod level cgroups // since we do not yet pod level requests/limits, we need to ensure each // container makes an explict request or limit for a quota tracked resource diff --git a/plugin/pkg/admission/resourcequota/admission_test.go b/plugin/pkg/admission/resourcequota/admission_test.go index 4c619805c67..4705e1487bc 100644 --- a/plugin/pkg/admission/resourcequota/admission_test.go +++ b/plugin/pkg/admission/resourcequota/admission_test.go @@ -314,11 +314,18 @@ func TestAdmitEnforceQuotaConstraints(t *testing.T) { evaluator: evaluator, } indexer.Add(resourceQuota) + // verify all values are specified as required on the quota newPod := validPod("not-allowed-pod", 1, getResourceRequirements(getResourceList("100m", "2Gi"), getResourceList("200m", ""))) err := handler.Admit(admission.NewAttributesRecord(newPod, api.Kind("Pod").WithVersion("version"), newPod.Namespace, newPod.Name, api.Resource("pods").WithVersion("version"), "", admission.Create, nil)) if err == nil { t.Errorf("Expected an error because the pod does not specify a memory limit") } + // verify the requests and limits are actually valid (in this case, we fail because the limits < requests) + newPod = validPod("not-allowed-pod", 1, getResourceRequirements(getResourceList("200m", "2Gi"), getResourceList("100m", "1Gi"))) + err = handler.Admit(admission.NewAttributesRecord(newPod, api.Kind("Pod").WithVersion("version"), newPod.Namespace, newPod.Name, api.Resource("pods").WithVersion("version"), "", admission.Create, nil)) + if err == nil { + t.Errorf("Expected an error because the pod does not specify a memory limit") + } } // TestAdmitPodInNamespaceWithoutQuota ensures that if a namespace has no quota, that a pod can get in