diff --git a/api/swagger-spec/v1.json b/api/swagger-spec/v1.json index 90a7e1234e..8240c9f9fa 100644 --- a/api/swagger-spec/v1.json +++ b/api/swagger-spec/v1.json @@ -11575,7 +11575,15 @@ }, "default": { "type": "any", - "description": "Default usage constraints on this kind by resource name. Default values on this kind by resource name if omitted." + "description": "Default resource requirement limit value by resource name if resource limit is omitted." + }, + "defaultRequest": { + "type": "any", + "description": "DefaultRequest is the default resource requirement request value by resource name if resource request is omitted." + }, + "maxLimitRequestRatio": { + "type": "any", + "description": "MaxLimitRequestRatio if specified, the named resource must have a request and limit that are both non-zero where limit divided by request is less than or equal to the enumerated value; this represents the max burst for the named resource." } } }, diff --git a/cluster/saltbase/salt/kube-admission-controls/limit-range/limit-range.yaml b/cluster/saltbase/salt/kube-admission-controls/limit-range/limit-range.yaml index de2bb13a9a..aff57ae9b6 100644 --- a/cluster/saltbase/salt/kube-admission-controls/limit-range/limit-range.yaml +++ b/cluster/saltbase/salt/kube-admission-controls/limit-range/limit-range.yaml @@ -6,5 +6,5 @@ metadata: spec: limits: - type: "Container" - default: + defaultRequest: cpu: "100m" diff --git a/docs/design/admission_control_limit_range.md b/docs/design/admission_control_limit_range.md index 621fd56491..e7c706efcf 100644 --- a/docs/design/admission_control_limit_range.md +++ b/docs/design/admission_control_limit_range.md @@ -53,7 +53,7 @@ The **LimitRange** resource is scoped to a **Namespace**. ### Type ```go -// A type of object that is limited +// LimitType is a type of object that is limited type LimitType string const ( @@ -63,44 +63,50 @@ const ( LimitTypeContainer LimitType = "Container" ) -// LimitRangeItem defines a min/max usage limit for any resource that matches on kind +// LimitRangeItem defines a min/max usage limit for any resource that matches on kind. type LimitRangeItem struct { - // Type of resource that this limit applies to - Type LimitType `json:"type,omitempty" description:"type of resource that this limit applies to"` - // Max usage constraints on this kind by resource name - Max ResourceList `json:"max,omitempty" description:"max usage constraints on this kind by resource name"` - // Min usage constraints on this kind by resource name - Min ResourceList `json:"min,omitempty" description:"min usage constraints on this kind by resource name"` - // Default resource limits on this kind by resource name - Default ResourceList `json:"default,omitempty" description:"default resource limits values on this kind by resource name if omitted"` - // DefaultRequests resource requests on this kind by resource name - DefaultRequests ResourceList `json:"defaultRequests,omitempty" description:"default resource requests values on this kind by resource name if omitted"` - // LimitRequestRatio is the ratio of limit over request that is the maximum allowed burst for the named resource - LimitRequestRatio ResourceList `json:"limitRequestRatio,omitempty" description:"the ratio of limit over request that is the maximum allowed burst for the named resource. if specified, the named resource must have a request and limit that are both non-zero where limit divided by request is less than or equal to the enumerated value"` + // Type of resource that this limit applies to. + Type LimitType `json:"type,omitempty"` + // Max usage constraints on this kind by resource name. + Max ResourceList `json:"max,omitempty"` + // Min usage constraints on this kind by resource name. + Min ResourceList `json:"min,omitempty"` + // Default resource requirement limit value by resource name if resource limit is omitted. + Default ResourceList `json:"default,omitempty"` + // DefaultRequest is the default resource requirement request value by resource name if resource request is omitted. + DefaultRequest ResourceList `json:"defaultRequest,omitempty"` + // MaxLimitRequestRatio if specified, the named resource must have a request and limit that are both non-zero where limit divided by request is less than or equal to the enumerated value; this represents the max burst for the named resource. + MaxLimitRequestRatio ResourceList `json:"maxLimitRequestRatio,omitempty"` } -// LimitRangeSpec defines a min/max usage limit for resources that match on kind +// LimitRangeSpec defines a min/max usage limit for resources that match on kind. type LimitRangeSpec struct { - // Limits is the list of LimitRangeItem objects that are enforced - Limits []LimitRangeItem `json:"limits" description:"limits is the list of LimitRangeItem objects that are enforced"` + // Limits is the list of LimitRangeItem objects that are enforced. + Limits []LimitRangeItem `json:"limits"` } -// LimitRange sets resource usage limits for each kind of resource in a Namespace +// LimitRange sets resource usage limits for each kind of resource in a Namespace. type LimitRange struct { - TypeMeta `json:",inline"` - ObjectMeta `json:"metadata,omitempty" description:"standard object metadata; see http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#metadata"` + TypeMeta `json:",inline"` + // Standard object's metadata. + // More info: http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#metadata + ObjectMeta `json:"metadata,omitempty"` - // Spec defines the limits enforced - Spec LimitRangeSpec `json:"spec,omitempty" description:"spec defines the limits enforced; http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#spec-and-status"` + // Spec defines the limits enforced. + // More info: http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#spec-and-status + Spec LimitRangeSpec `json:"spec,omitempty"` } // LimitRangeList is a list of LimitRange items. type LimitRangeList struct { TypeMeta `json:",inline"` - ListMeta `json:"metadata,omitempty" description:"standard list metadata; see http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#metadata"` + // Standard list metadata. + // More info: http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#types-kinds + ListMeta `json:"metadata,omitempty"` - // Items is a list of LimitRange objects - Items []LimitRange `json:"items" description:"items is a list of LimitRange objects; see http://releases.k8s.io/HEAD/docs/design/admission_control_limit_range.md"` + // Items is a list of LimitRange objects. + // More info: http://releases.k8s.io/HEAD/docs/design/admission_control_limit_range.md + Items []LimitRange `json:"items"` } ``` @@ -108,7 +114,7 @@ type LimitRangeList struct { Validation of a **LimitRange** enforces that for a given named resource the following rules apply: -Min (if specified) <= DefaultRequests (if specified) <= Default (if specified) <= Max (if specified) +Min (if specified) <= DefaultRequest (if specified) <= Default (if specified) <= Max (if specified) ### Default Value Behavior @@ -121,11 +127,11 @@ if LimitRangeItem.Default[resourceName] is undefined ``` ``` -if LimitRangeItem.DefaultRequests[resourceName] is undefined +if LimitRangeItem.DefaultRequest[resourceName] is undefined if LimitRangeItem.Default[resourceName] is defined - LimitRangeItem.DefaultRequests[resourceName] = LimitRangeItem.Default[resourceName] + LimitRangeItem.DefaultRequest[resourceName] = LimitRangeItem.Default[resourceName] else if LimitRangeItem.Min[resourceName] is defined - LimitRangeItem.DefaultRequests[resourceName] = LimitRangeItem.Min[resourceName] + LimitRangeItem.DefaultRequest[resourceName] = LimitRangeItem.Min[resourceName] ``` ## AdmissionControl plugin: LimitRanger diff --git a/docs/devel/api_changes.md b/docs/devel/api_changes.md index 72c38b7ff5..709f8c2c08 100644 --- a/docs/devel/api_changes.md +++ b/docs/devel/api_changes.md @@ -345,6 +345,22 @@ generator to create it from scratch. Unsurprisingly, adding manually written conversion also requires you to add tests to `pkg/api//conversion_test.go`. +## Edit deep copy files + +At this point you have both the versioned API changes and the internal +structure changes done. You now need to generate code to handle deep copy +of your versioned api objects. + +The deep copy code resides with each versioned API: + - `pkg/api//deep_copy_generated.go` containing auto-generated copy functions + +To regenerate them: + - run + +```sh +hack/update-generated-deep-copies.sh +``` + ## Update the fuzzer Part of our testing regimen for APIs is to "fuzz" (fill with random values) API diff --git a/pkg/api/deep_copy_generated.go b/pkg/api/deep_copy_generated.go index 7969dccffd..5bb46937ee 100644 --- a/pkg/api/deep_copy_generated.go +++ b/pkg/api/deep_copy_generated.go @@ -626,6 +626,30 @@ func deepCopy_api_LimitRangeItem(in LimitRangeItem, out *LimitRangeItem, c *conv } else { out.Default = nil } + if in.DefaultRequest != nil { + out.DefaultRequest = make(ResourceList) + for key, val := range in.DefaultRequest { + newVal := new(resource.Quantity) + if err := deepCopy_resource_Quantity(val, newVal, c); err != nil { + return err + } + out.DefaultRequest[key] = *newVal + } + } else { + out.DefaultRequest = nil + } + if in.MaxLimitRequestRatio != nil { + out.MaxLimitRequestRatio = make(ResourceList) + for key, val := range in.MaxLimitRequestRatio { + newVal := new(resource.Quantity) + if err := deepCopy_resource_Quantity(val, newVal, c); err != nil { + return err + } + out.MaxLimitRequestRatio[key] = *newVal + } + } else { + out.MaxLimitRequestRatio = nil + } return nil } diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go index 5bf8fd6d4e..44aef44e58 100644 --- a/pkg/api/testing/fuzzer.go +++ b/pkg/api/testing/fuzzer.go @@ -184,6 +184,28 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer { q.Limits[api.ResourceStorage] = *storageLimit.Copy() q.Requests[api.ResourceStorage] = *storageLimit.Copy() }, + func(q *api.LimitRangeItem, c fuzz.Continue) { + randomQuantity := func() resource.Quantity { + return *resource.NewQuantity(c.Int63n(1000), resource.DecimalExponent) + } + cpuLimit := randomQuantity() + + q.Type = api.LimitTypeContainer + q.Default = make(api.ResourceList) + q.Default[api.ResourceCPU] = *(cpuLimit.Copy()) + + q.DefaultRequest = make(api.ResourceList) + q.DefaultRequest[api.ResourceCPU] = *(cpuLimit.Copy()) + + q.Max = make(api.ResourceList) + q.Max[api.ResourceCPU] = *(cpuLimit.Copy()) + + q.Min = make(api.ResourceList) + q.Min[api.ResourceCPU] = *(cpuLimit.Copy()) + + q.MaxLimitRequestRatio = make(api.ResourceList) + q.MaxLimitRequestRatio[api.ResourceCPU] = resource.MustParse("10") + }, func(p *api.PullPolicy, c fuzz.Continue) { policies := []api.PullPolicy{api.PullAlways, api.PullNever, api.PullIfNotPresent} *p = policies[c.Rand.Intn(len(policies))] diff --git a/pkg/api/types.go b/pkg/api/types.go index 4b0afe6723..c88f0c7ebb 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1894,8 +1894,12 @@ type LimitRangeItem struct { Max ResourceList `json:"max,omitempty"` // Min usage constraints on this kind by resource name Min ResourceList `json:"min,omitempty"` - // Default usage constraints on this kind by resource name + // Default resource requirement limit value by resource name. Default ResourceList `json:"default,omitempty"` + // DefaultRequest resource requirement request value by resource name. + DefaultRequest ResourceList `json:"defaultRequest,omitempty"` + // MaxLimitRequestRatio represents the max burst value for the named resource + MaxLimitRequestRatio ResourceList `json:"maxLimitRequestRatio,omitempty"` } // LimitRangeSpec defines a min/max usage limit for resources that match on kind diff --git a/pkg/api/v1/conversion_generated.go b/pkg/api/v1/conversion_generated.go index b5b49dda40..6a75a1a278 100644 --- a/pkg/api/v1/conversion_generated.go +++ b/pkg/api/v1/conversion_generated.go @@ -730,6 +730,30 @@ func convert_api_LimitRangeItem_To_v1_LimitRangeItem(in *api.LimitRangeItem, out } else { out.Default = nil } + if in.DefaultRequest != nil { + out.DefaultRequest = make(ResourceList) + for key, val := range in.DefaultRequest { + newVal := resource.Quantity{} + if err := s.Convert(&val, &newVal, 0); err != nil { + return err + } + out.DefaultRequest[ResourceName(key)] = newVal + } + } else { + out.DefaultRequest = nil + } + if in.MaxLimitRequestRatio != nil { + out.MaxLimitRequestRatio = make(ResourceList) + for key, val := range in.MaxLimitRequestRatio { + newVal := resource.Quantity{} + if err := s.Convert(&val, &newVal, 0); err != nil { + return err + } + out.MaxLimitRequestRatio[ResourceName(key)] = newVal + } + } else { + out.MaxLimitRequestRatio = nil + } return nil } @@ -3020,6 +3044,30 @@ func convert_v1_LimitRangeItem_To_api_LimitRangeItem(in *LimitRangeItem, out *ap } else { out.Default = nil } + if in.DefaultRequest != nil { + out.DefaultRequest = make(api.ResourceList) + for key, val := range in.DefaultRequest { + newVal := resource.Quantity{} + if err := s.Convert(&val, &newVal, 0); err != nil { + return err + } + out.DefaultRequest[api.ResourceName(key)] = newVal + } + } else { + out.DefaultRequest = nil + } + if in.MaxLimitRequestRatio != nil { + out.MaxLimitRequestRatio = make(api.ResourceList) + for key, val := range in.MaxLimitRequestRatio { + newVal := resource.Quantity{} + if err := s.Convert(&val, &newVal, 0); err != nil { + return err + } + out.MaxLimitRequestRatio[api.ResourceName(key)] = newVal + } + } else { + out.MaxLimitRequestRatio = nil + } return nil } diff --git a/pkg/api/v1/deep_copy_generated.go b/pkg/api/v1/deep_copy_generated.go index eb57bd36fb..e806c6654a 100644 --- a/pkg/api/v1/deep_copy_generated.go +++ b/pkg/api/v1/deep_copy_generated.go @@ -641,6 +641,30 @@ func deepCopy_v1_LimitRangeItem(in LimitRangeItem, out *LimitRangeItem, c *conve } else { out.Default = nil } + if in.DefaultRequest != nil { + out.DefaultRequest = make(ResourceList) + for key, val := range in.DefaultRequest { + newVal := new(resource.Quantity) + if err := deepCopy_resource_Quantity(val, newVal, c); err != nil { + return err + } + out.DefaultRequest[key] = *newVal + } + } else { + out.DefaultRequest = nil + } + if in.MaxLimitRequestRatio != nil { + out.MaxLimitRequestRatio = make(ResourceList) + for key, val := range in.MaxLimitRequestRatio { + newVal := new(resource.Quantity) + if err := deepCopy_resource_Quantity(val, newVal, c); err != nil { + return err + } + out.MaxLimitRequestRatio[key] = *newVal + } + } else { + out.MaxLimitRequestRatio = nil + } return nil } diff --git a/pkg/api/v1/defaults.go b/pkg/api/v1/defaults.go index 3148b83ff0..5abf1ab2cd 100644 --- a/pkg/api/v1/defaults.go +++ b/pkg/api/v1/defaults.go @@ -173,6 +173,37 @@ func addDefaultingFuncs() { } } }, + func(obj *LimitRangeItem) { + // for container limits, we apply default values + if obj.Type == LimitTypeContainer { + + if obj.Default == nil { + obj.Default = make(ResourceList) + } + if obj.DefaultRequest == nil { + obj.DefaultRequest = make(ResourceList) + } + + // If a default limit is unspecified, but the max is specified, default the limit to the max + for key, value := range obj.Max { + if _, exists := obj.Default[key]; !exists { + obj.Default[key] = *(value.Copy()) + } + } + // If a default limit is specified, but the default request is not, default request to limit + for key, value := range obj.Default { + if _, exists := obj.DefaultRequest[key]; !exists { + obj.DefaultRequest[key] = *(value.Copy()) + } + } + // If a default request is not specified, but the min is provided, default request to the min + for key, value := range obj.Min { + if _, exists := obj.DefaultRequest[key]; !exists { + obj.DefaultRequest[key] = *(value.Copy()) + } + } + } + }, ) } diff --git a/pkg/api/v1/defaults_test.go b/pkg/api/v1/defaults_test.go index 008ca2c2ef..7a60866fb3 100644 --- a/pkg/api/v1/defaults_test.go +++ b/pkg/api/v1/defaults_test.go @@ -21,6 +21,7 @@ import ( "testing" "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/resource" versioned "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util" @@ -431,3 +432,45 @@ func TestSetDefaultObjectFieldSelectorAPIVersion(t *testing.T) { t.Errorf("Expected default APIVersion v1, got: %v", apiVersion) } } + +func TestSetDefaultLimitRangeItem(t *testing.T) { + limitRange := &versioned.LimitRange{ + ObjectMeta: versioned.ObjectMeta{ + Name: "test-defaults", + }, + Spec: versioned.LimitRangeSpec{ + Limits: []versioned.LimitRangeItem{{ + Type: versioned.LimitTypeContainer, + Max: versioned.ResourceList{ + versioned.ResourceCPU: resource.MustParse("100m"), + }, + Min: versioned.ResourceList{ + versioned.ResourceMemory: resource.MustParse("100Mi"), + }, + Default: versioned.ResourceList{}, + DefaultRequest: versioned.ResourceList{}, + }}, + }, + } + + output := roundTrip(t, runtime.Object(limitRange)) + limitRange2 := output.(*versioned.LimitRange) + defaultLimit := limitRange2.Spec.Limits[0].Default + defaultRequest := limitRange2.Spec.Limits[0].DefaultRequest + + // verify that default cpu was set to the max + defaultValue := defaultLimit[versioned.ResourceCPU] + if defaultValue.String() != "100m" { + t.Errorf("Expected default cpu: %s, got: %s", "100m", defaultValue.String()) + } + // verify that default request was set to the limit + requestValue := defaultRequest[versioned.ResourceCPU] + if requestValue.String() != "100m" { + t.Errorf("Expected request cpu: %s, got: %s", "100m", requestValue.String()) + } + // verify that if a min is provided, it will be the default if no limit is specified + requestMinValue := defaultRequest[versioned.ResourceMemory] + if requestMinValue.String() != "100Mi" { + t.Errorf("Expected request memory: %s, got: %s", "100Mi", requestMinValue.String()) + } +} diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index c78a5da964..68ac8cc32d 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -2266,9 +2266,12 @@ type LimitRangeItem struct { Max ResourceList `json:"max,omitempty"` // Min usage constraints on this kind by resource name. Min ResourceList `json:"min,omitempty"` - // Default usage constraints on this kind by resource name. - // Default values on this kind by resource name if omitted. + // Default resource requirement limit value by resource name if resource limit is omitted. Default ResourceList `json:"default,omitempty"` + // DefaultRequest is the default resource requirement request value by resource name if resource request is omitted. + DefaultRequest ResourceList `json:"defaultRequest,omitempty"` + // MaxLimitRequestRatio if specified, the named resource must have a request and limit that are both non-zero where limit divided by request is less than or equal to the enumerated value; this represents the max burst for the named resource. + MaxLimitRequestRatio ResourceList `json:"maxLimitRequestRatio,omitempty"` } // LimitRangeSpec defines a min/max usage limit for resources that match on kind. diff --git a/pkg/api/v1/types_swagger_doc_generated.go b/pkg/api/v1/types_swagger_doc_generated.go index 67f4c6c67b..555c3cc52a 100644 --- a/pkg/api/v1/types_swagger_doc_generated.go +++ b/pkg/api/v1/types_swagger_doc_generated.go @@ -432,11 +432,13 @@ func (LimitRange) SwaggerDoc() map[string]string { } var map_LimitRangeItem = map[string]string{ - "": "LimitRangeItem defines a min/max usage limit for any resource that matches on kind.", - "type": "Type of resource that this limit applies to.", - "max": "Max usage constraints on this kind by resource name.", - "min": "Min usage constraints on this kind by resource name.", - "default": "Default usage constraints on this kind by resource name. Default values on this kind by resource name if omitted.", + "": "LimitRangeItem defines a min/max usage limit for any resource that matches on kind.", + "type": "Type of resource that this limit applies to.", + "max": "Max usage constraints on this kind by resource name.", + "min": "Min usage constraints on this kind by resource name.", + "default": "Default resource requirement limit value by resource name if resource limit is omitted.", + "defaultRequest": "DefaultRequest is the default resource requirement request value by resource name if resource request is omitted.", + "maxLimitRequestRatio": "MaxLimitRequestRatio if specified, the named resource must have a request and limit that are both non-zero where limit divided by request is less than or equal to the enumerated value; this represents the max burst for the named resource.", } func (LimitRangeItem) SwaggerDoc() map[string]string { diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 44d06a0697..a04b827ff9 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -1352,6 +1352,7 @@ func ValidateLimitRange(limitRange *api.LimitRange) errs.ValidationErrorList { min := map[string]int64{} max := map[string]int64{} defaults := map[string]int64{} + defaultRequests := map[string]int64{} for k := range limit.Max { allErrs = append(allErrs, validateResourceName(string(k), fmt.Sprintf("spec.limits[%d].max[%s]", i, k))...) @@ -1371,28 +1372,56 @@ func ValidateLimitRange(limitRange *api.LimitRange) errs.ValidationErrorList { q := limit.Default[k] defaults[string(k)] = q.Value() } + for k := range limit.DefaultRequest { + allErrs = append(allErrs, validateResourceName(string(k), fmt.Sprintf("spec.limits[%d].defaultRequest[%s]", i, k))...) + keys.Insert(string(k)) + q := limit.DefaultRequest[k] + defaultRequests[string(k)] = q.Value() + } + for k := range limit.MaxLimitRequestRatio { + allErrs = append(allErrs, validateResourceName(string(k), fmt.Sprintf("spec.limits[%d].maxLimitRequestRatio[%s]", i, k))...) + } for k := range keys { minValue, minValueFound := min[k] maxValue, maxValueFound := max[k] defaultValue, defaultValueFound := defaults[k] + defaultRequestValue, defaultRequestValueFound := defaultRequests[k] if minValueFound && maxValueFound && minValue > maxValue { minQuantity := limit.Min[api.ResourceName(k)] maxQuantity := limit.Max[api.ResourceName(k)] - allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].max[%s]", i, k), minValue, fmt.Sprintf("min value %s is greater than max value %s", minQuantity.String(), maxQuantity.String()))) + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].min[%s]", i, k), minValue, fmt.Sprintf("min value %s is greater than max value %s", minQuantity.String(), maxQuantity.String()))) + } + + if defaultRequestValueFound && minValueFound && minValue > defaultRequestValue { + minQuantity := limit.Min[api.ResourceName(k)] + defaultRequestQuantity := limit.DefaultRequest[api.ResourceName(k)] + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].defaultRequest[%s]", i, k), defaultRequestValue, fmt.Sprintf("min value %s is greater than default request value %s", minQuantity.String(), defaultRequestQuantity.String()))) + } + + if defaultRequestValueFound && maxValueFound && defaultRequestValue > maxValue { + maxQuantity := limit.Max[api.ResourceName(k)] + defaultRequestQuantity := limit.DefaultRequest[api.ResourceName(k)] + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].defaultRequest[%s]", i, k), defaultRequestValue, fmt.Sprintf("default request value %s is greater than max value %s", defaultRequestQuantity.String(), maxQuantity.String()))) + } + + if defaultRequestValueFound && defaultValueFound && defaultRequestValue > defaultValue { + defaultQuantity := limit.Default[api.ResourceName(k)] + defaultRequestQuantity := limit.DefaultRequest[api.ResourceName(k)] + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].defaultRequest[%s]", i, k), defaultRequestValue, fmt.Sprintf("default request value %s is greater than default limit value %s", defaultRequestQuantity.String(), defaultQuantity.String()))) } if defaultValueFound && minValueFound && minValue > defaultValue { minQuantity := limit.Min[api.ResourceName(k)] defaultQuantity := limit.Default[api.ResourceName(k)] - allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].max[%s]", i, k), minValue, fmt.Sprintf("min value %s is greater than default value %s", minQuantity.String(), defaultQuantity.String()))) + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].default[%s]", i, k), minValue, fmt.Sprintf("min value %s is greater than default value %s", minQuantity.String(), defaultQuantity.String()))) } if defaultValueFound && maxValueFound && defaultValue > maxValue { maxQuantity := limit.Max[api.ResourceName(k)] defaultQuantity := limit.Default[api.ResourceName(k)] - allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].max[%s]", i, k), minValue, fmt.Sprintf("default value %s is greater than max value %s", defaultQuantity.String(), maxQuantity.String()))) + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].default[%s]", i, k), maxValue, fmt.Sprintf("default value %s is greater than max value %s", defaultQuantity.String(), maxQuantity.String()))) } } } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 5d518cba3e..b694f2a458 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -2814,13 +2814,20 @@ func TestValidateLimitRange(t *testing.T) { api.ResourceMemory: resource.MustParse("10000"), }, Min: api.ResourceList{ - api.ResourceCPU: resource.MustParse("0"), + api.ResourceCPU: resource.MustParse("5"), api.ResourceMemory: resource.MustParse("100"), }, Default: api.ResourceList{ api.ResourceCPU: resource.MustParse("50"), api.ResourceMemory: resource.MustParse("500"), }, + DefaultRequest: api.ResourceList{ + api.ResourceCPU: resource.MustParse("10"), + api.ResourceMemory: resource.MustParse("200"), + }, + MaxLimitRequestRatio: api.ResourceList{ + api.ResourceCPU: resource.MustParse("20"), + }, }, }, } @@ -2879,6 +2886,43 @@ func TestValidateLimitRange(t *testing.T) { }, } + invalidSpecRangeDefaultRequestOutsideRange := api.LimitRangeSpec{ + Limits: []api.LimitRangeItem{ + { + Type: api.LimitTypePod, + Max: api.ResourceList{ + api.ResourceCPU: resource.MustParse("1000"), + }, + Min: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + }, + DefaultRequest: api.ResourceList{ + api.ResourceCPU: resource.MustParse("2000"), + }, + }, + }, + } + + invalidSpecRangeRequestMoreThanDefaultRange := api.LimitRangeSpec{ + Limits: []api.LimitRangeItem{ + { + Type: api.LimitTypePod, + Max: api.ResourceList{ + api.ResourceCPU: resource.MustParse("1000"), + }, + Min: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + }, + Default: api.ResourceList{ + api.ResourceCPU: resource.MustParse("500"), + }, + DefaultRequest: api.ResourceList{ + api.ResourceCPU: resource.MustParse("800"), + }, + }, + }, + } + successCases := []api.LimitRange{ { ObjectMeta: api.ObjectMeta{ @@ -2927,6 +2971,14 @@ func TestValidateLimitRange(t *testing.T) { api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidSpecRangeDefaultOutsideRange}, "default value 2k is greater than max value 1k", }, + "invalid spec defaultrequest outside range": { + api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidSpecRangeDefaultRequestOutsideRange}, + "default request value 2k is greater than max value 1k", + }, + "invalid spec defaultrequest more than default": { + api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidSpecRangeRequestMoreThanDefaultRange}, + "default request value 800 is greater than default limit value 500", + }, } for k, v := range errorCases { errs := ValidateLimitRange(&v.R) diff --git a/pkg/kubectl/describe.go b/pkg/kubectl/describe.go index 55c7964714..608dfcd16a 100644 --- a/pkg/kubectl/describe.go +++ b/pkg/kubectl/describe.go @@ -182,14 +182,16 @@ func DescribeLimitRanges(limitRanges *api.LimitRangeList, w io.Writer) { fmt.Fprint(w, "No resource limits.\n") return } - fmt.Fprintf(w, "Resource Limits\n Type\tResource\tMin\tMax\tDefault\n") - fmt.Fprintf(w, " ----\t--------\t---\t---\t---\n") + fmt.Fprintf(w, "Resource Limits\n Type\tResource\tMin\tMax\tRequest\tLimit\tLimit/Request\n") + fmt.Fprintf(w, " ----\t--------\t---\t---\t-------\t-----\t-------------\n") for _, limitRange := range limitRanges.Items { for i := range limitRange.Spec.Limits { item := limitRange.Spec.Limits[i] maxResources := item.Max minResources := item.Min - defaultResources := item.Default + defaultLimitResources := item.Default + defaultRequestResources := item.DefaultRequest + ratio := item.MaxLimitRequestRatio set := map[api.ResourceName]bool{} for k := range maxResources { @@ -198,7 +200,13 @@ func DescribeLimitRanges(limitRanges *api.LimitRangeList, w io.Writer) { for k := range minResources { set[k] = true } - for k := range defaultResources { + for k := range defaultLimitResources { + set[k] = true + } + for k := range defaultRequestResources { + set[k] = true + } + for k := range ratio { set[k] = true } @@ -206,7 +214,9 @@ func DescribeLimitRanges(limitRanges *api.LimitRangeList, w io.Writer) { // if no value is set, we output - maxValue := "-" minValue := "-" - defaultValue := "-" + defaultLimitValue := "-" + defaultRequestValue := "-" + ratioValue := "-" maxQuantity, maxQuantityFound := maxResources[k] if maxQuantityFound { @@ -218,13 +228,23 @@ func DescribeLimitRanges(limitRanges *api.LimitRangeList, w io.Writer) { minValue = minQuantity.String() } - defaultQuantity, defaultQuantityFound := defaultResources[k] - if defaultQuantityFound { - defaultValue = defaultQuantity.String() + defaultLimitQuantity, defaultLimitQuantityFound := defaultLimitResources[k] + if defaultLimitQuantityFound { + defaultLimitValue = defaultLimitQuantity.String() } - msg := " %v\t%v\t%v\t%v\t%v\n" - fmt.Fprintf(w, msg, item.Type, k, minValue, maxValue, defaultValue) + defaultRequestQuantity, defaultRequestQuantityFound := defaultRequestResources[k] + if defaultRequestQuantityFound { + defaultRequestValue = defaultRequestQuantity.String() + } + + ratioQuantity, ratioQuantityFound := ratio[k] + if ratioQuantityFound { + ratioValue = ratioQuantity.String() + } + + msg := " %s\t%v\t%v\t%v\t%v\t%v\t%v\n" + fmt.Fprintf(w, msg, item.Type, k, minValue, maxValue, defaultRequestValue, defaultLimitValue, ratioValue) } } } @@ -287,13 +307,15 @@ func describeLimitRange(limitRange *api.LimitRange) (string, error) { return tabbedString(func(out io.Writer) error { fmt.Fprintf(out, "Name:\t%s\n", limitRange.Name) fmt.Fprintf(out, "Namespace:\t%s\n", limitRange.Namespace) - fmt.Fprintf(out, "Type\tResource\tMin\tMax\tDefault\n") - fmt.Fprintf(out, "----\t--------\t---\t---\t---\n") + fmt.Fprintf(out, "Type\tResource\tMin\tMax\tRequest\tLimit\tLimit/Request\n") + fmt.Fprintf(out, "----\t--------\t---\t---\t-------\t-----\t-------------\n") for i := range limitRange.Spec.Limits { item := limitRange.Spec.Limits[i] maxResources := item.Max minResources := item.Min - defaultResources := item.Default + defaultLimitResources := item.Default + defaultRequestResources := item.DefaultRequest + ratio := item.MaxLimitRequestRatio set := map[api.ResourceName]bool{} for k := range maxResources { @@ -302,7 +324,13 @@ func describeLimitRange(limitRange *api.LimitRange) (string, error) { for k := range minResources { set[k] = true } - for k := range defaultResources { + for k := range defaultLimitResources { + set[k] = true + } + for k := range defaultRequestResources { + set[k] = true + } + for k := range ratio { set[k] = true } @@ -310,7 +338,9 @@ func describeLimitRange(limitRange *api.LimitRange) (string, error) { // if no value is set, we output - maxValue := "-" minValue := "-" - defaultValue := "-" + defaultLimitValue := "-" + defaultRequestValue := "-" + ratioValue := "-" maxQuantity, maxQuantityFound := maxResources[k] if maxQuantityFound { @@ -322,13 +352,23 @@ func describeLimitRange(limitRange *api.LimitRange) (string, error) { minValue = minQuantity.String() } - defaultQuantity, defaultQuantityFound := defaultResources[k] - if defaultQuantityFound { - defaultValue = defaultQuantity.String() + defaultLimitQuantity, defaultLimitQuantityFound := defaultLimitResources[k] + if defaultLimitQuantityFound { + defaultLimitValue = defaultLimitQuantity.String() } - msg := "%v\t%v\t%v\t%v\t%v\n" - fmt.Fprintf(out, msg, item.Type, k, minValue, maxValue, defaultValue) + defaultRequestQuantity, defaultRequestQuantityFound := defaultRequestResources[k] + if defaultRequestQuantityFound { + defaultRequestValue = defaultRequestQuantity.String() + } + + ratioQuantity, ratioQuantityFound := ratio[k] + if ratioQuantityFound { + ratioValue = ratioQuantity.String() + } + + msg := "%v\t%v\t%v\t%v\t%v\t%v\t%v\n" + fmt.Fprintf(out, msg, item.Type, k, minValue, maxValue, defaultRequestValue, defaultLimitValue, ratioValue) } } return nil diff --git a/plugin/pkg/admission/limitranger/admission.go b/plugin/pkg/admission/limitranger/admission.go index ff02dd6a60..bc544c3863 100644 --- a/plugin/pkg/admission/limitranger/admission.go +++ b/plugin/pkg/admission/limitranger/admission.go @@ -138,15 +138,19 @@ func Limit(limitRange *api.LimitRange, resourceName string, obj runtime.Object) // defaultContainerResourceRequirements returns the default requirements for a container // the requirement.Limits are taken from the LimitRange defaults (if specified) -// the requirement.Requests are taken from the LimitRange min (if specified) +// the requirement.Requests are taken from the LimitRange default request (if specified) func defaultContainerResourceRequirements(limitRange *api.LimitRange) api.ResourceRequirements { requirements := api.ResourceRequirements{} - requirements.Limits = api.ResourceList{} requirements.Requests = api.ResourceList{} + requirements.Limits = api.ResourceList{} for i := range limitRange.Spec.Limits { limit := limitRange.Spec.Limits[i] if limit.Type == api.LimitTypeContainer { + for k, v := range limit.DefaultRequest { + value := v.Copy() + requirements.Requests[k] = *value + } for k, v := range limit.Default { value := v.Copy() requirements.Limits[k] = *value @@ -181,91 +185,176 @@ func mergePodResourceRequirements(pod *api.Pod, defaultRequirements *api.Resourc } } +// requestLimitEnforcedValues returns the specified values at a common precision to support comparability +func requestLimitEnforcedValues(requestQuantity, limitQuantity, enforcedQuantity resource.Quantity) (request, limit, enforced int64) { + request = requestQuantity.Value() + limit = limitQuantity.Value() + enforced = enforcedQuantity.Value() + // do a more precise comparison if possible (if the value won't overflow) + if request <= resource.MaxMilliValue && limit <= resource.MaxMilliValue && enforced <= resource.MaxMilliValue { + request = requestQuantity.MilliValue() + limit = limitQuantity.MilliValue() + enforced = enforcedQuantity.MilliValue() + } + return +} + +// minConstraint enforces the min constraint over the specified resource +func minConstraint(limitType api.LimitType, resourceName api.ResourceName, enforced resource.Quantity, request api.ResourceList, limit api.ResourceList) error { + req, reqExists := request[resourceName] + lim, limExists := limit[resourceName] + observedReqValue, observedLimValue, enforcedValue := requestLimitEnforcedValues(req, lim, enforced) + + if !reqExists { + return fmt.Errorf("Minimum %s usage per %s is %s. No request is specified.", resourceName, limitType, enforced.String()) + } + if observedReqValue < enforcedValue { + return fmt.Errorf("Minimum %s usage per %s is %s, but request is %s.", resourceName, limitType, enforced.String(), req.String()) + } + if limExists && (observedLimValue < enforcedValue) { + return fmt.Errorf("Minimum %s usage per %s is %s, but limit is %s.", resourceName, limitType, enforced.String(), lim.String()) + } + return nil +} + +// maxConstraint enforces the max constraint over the specified resource +func maxConstraint(limitType api.LimitType, resourceName api.ResourceName, enforced resource.Quantity, request api.ResourceList, limit api.ResourceList) error { + req, reqExists := request[resourceName] + lim, limExists := limit[resourceName] + observedReqValue, observedLimValue, enforcedValue := requestLimitEnforcedValues(req, lim, enforced) + + if !limExists { + return fmt.Errorf("Maximum %s usage per %s is %s. No limit is specified.", resourceName, limitType, enforced.String()) + } + if observedLimValue > enforcedValue { + return fmt.Errorf("Maximum %s usage per %s is %s, but limit is %s.", resourceName, limitType, enforced.String(), lim.String()) + } + if reqExists && (observedReqValue > enforcedValue) { + return fmt.Errorf("Maximum %s usage per %s is %s, but request is %s.", resourceName, limitType, enforced.String(), req.String()) + } + return nil +} + +// limitRequestRatioConstraint enforces the limit to request ratio over the specified resource +func limitRequestRatioConstraint(limitType api.LimitType, resourceName api.ResourceName, enforced resource.Quantity, request api.ResourceList, limit api.ResourceList) error { + req, reqExists := request[resourceName] + lim, limExists := limit[resourceName] + observedReqValue, observedLimValue, enforcedValue := requestLimitEnforcedValues(req, lim, enforced) + + if !reqExists || (observedReqValue == int64(0)) { + return fmt.Errorf("%s max limit to request ratio per %s is %s, but no request is specified or request is 0.", resourceName, limitType, enforced.String()) + } + if !limExists || (observedLimValue == int64(0)) { + return fmt.Errorf("%s max limit to request ratio per %s is %s, but no limit is specified or limit is 0.", resourceName, limitType, enforced.String()) + } + + observedValue := observedLimValue / observedReqValue + + if observedValue > enforcedValue { + return fmt.Errorf("%s max limit to request ratio per %s is %s, but provided ratio is %d.", resourceName, limitType, enforced.String(), observedValue) + } + + return nil +} + +// sum takes the total of each named resource across all inputs +// if a key is not in each input, then the output resource list will omit the key +func sum(inputs []api.ResourceList) api.ResourceList { + result := api.ResourceList{} + keys := []api.ResourceName{} + for i := range inputs { + for k := range inputs[i] { + keys = append(keys, k) + } + } + for _, key := range keys { + total, isSet := int64(0), true + + for i := range inputs { + input := inputs[i] + v, exists := input[key] + if exists { + if key == api.ResourceCPU { + total = total + v.MilliValue() + } else { + total = total + v.Value() + } + } else { + isSet = false + } + } + + if isSet { + if key == api.ResourceCPU { + result[key] = *(resource.NewMilliQuantity(total, resource.DecimalSI)) + } else { + result[key] = *(resource.NewQuantity(total, resource.DecimalSI)) + } + + } + } + return result +} + // PodLimitFunc enforces resource requirements enumerated by the pod against // the specified LimitRange. The pod may be modified to apply default resource // requirements if not specified, and enumerated on the LimitRange func PodLimitFunc(limitRange *api.LimitRange, pod *api.Pod) error { - defaultResources := defaultContainerResourceRequirements(limitRange) mergePodResourceRequirements(pod, &defaultResources) - podCPU := int64(0) - podMem := int64(0) - - minContainerCPU := int64(0) - minContainerMem := int64(0) - maxContainerCPU := int64(0) - maxContainerMem := int64(0) - - for i := range pod.Spec.Containers { - container := &pod.Spec.Containers[i] - containerCPU := container.Resources.Limits.Cpu().MilliValue() - containerMem := container.Resources.Limits.Memory().Value() - - if i == 0 { - minContainerCPU = containerCPU - minContainerMem = containerMem - maxContainerCPU = containerCPU - maxContainerMem = containerMem - } - - podCPU = podCPU + container.Resources.Limits.Cpu().MilliValue() - podMem = podMem + container.Resources.Limits.Memory().Value() - - minContainerCPU = Min(containerCPU, minContainerCPU) - minContainerMem = Min(containerMem, minContainerMem) - maxContainerCPU = Max(containerCPU, maxContainerCPU) - maxContainerMem = Max(containerMem, maxContainerMem) - } - for i := range limitRange.Spec.Limits { limit := limitRange.Spec.Limits[i] - for _, minOrMax := range []string{"Min", "Max"} { - var rl api.ResourceList - switch minOrMax { - case "Min": - rl = limit.Min - case "Max": - rl = limit.Max - } - for k, v := range rl { - observed := int64(0) - enforced := int64(0) - var err error - switch k { - case api.ResourceMemory: - enforced = v.Value() - switch limit.Type { - case api.LimitTypePod: - observed = podMem - err = fmt.Errorf("%simum memory usage per pod is %s", minOrMax, v.String()) - case api.LimitTypeContainer: - observed = maxContainerMem - err = fmt.Errorf("%simum memory usage per container is %s", minOrMax, v.String()) - } - case api.ResourceCPU: - enforced = v.MilliValue() - switch limit.Type { - case api.LimitTypePod: - observed = podCPU - err = fmt.Errorf("%simum CPU usage per pod is %s, but requested %s", minOrMax, v.String(), resource.NewMilliQuantity(observed, resource.DecimalSI)) - case api.LimitTypeContainer: - observed = maxContainerCPU - err = fmt.Errorf("%simum CPU usage per container is %s", minOrMax, v.String()) + limitType := limit.Type + + // enforce container limits + if limitType == api.LimitTypeContainer { + for j := range pod.Spec.Containers { + container := &pod.Spec.Containers[j] + for k, v := range limit.Min { + if err := minConstraint(limitType, k, v, container.Resources.Requests, container.Resources.Limits); err != nil { + return err } } - switch minOrMax { - case "Min": - if observed < enforced { + for k, v := range limit.Max { + if err := maxConstraint(limitType, k, v, container.Resources.Requests, container.Resources.Limits); err != nil { return err } - case "Max": - if observed > enforced { + } + for k, v := range limit.MaxLimitRequestRatio { + if err := limitRequestRatioConstraint(limitType, k, v, container.Resources.Requests, container.Resources.Limits); err != nil { return err } } } } + + // enforce pod limits + if limitType == api.LimitTypePod { + containerRequests, containerLimits := []api.ResourceList{}, []api.ResourceList{} + for j := range pod.Spec.Containers { + container := &pod.Spec.Containers[j] + containerRequests = append(containerRequests, container.Resources.Requests) + containerLimits = append(containerLimits, container.Resources.Limits) + } + podRequests := sum(containerRequests) + podLimits := sum(containerLimits) + for k, v := range limit.Min { + if err := minConstraint(limitType, k, v, podRequests, podLimits); err != nil { + return err + } + } + for k, v := range limit.Max { + if err := maxConstraint(limitType, k, v, podRequests, podLimits); err != nil { + return err + } + } + for k, v := range limit.MaxLimitRequestRatio { + if err := limitRequestRatioConstraint(limitType, k, v, podRequests, podLimits); err != nil { + return err + } + } + } } return nil } diff --git a/plugin/pkg/admission/limitranger/admission_test.go b/plugin/pkg/admission/limitranger/admission_test.go index dba8f56dda..2635c4bfdd 100644 --- a/plugin/pkg/admission/limitranger/admission_test.go +++ b/plugin/pkg/admission/limitranger/admission_test.go @@ -38,13 +38,34 @@ func getResourceList(cpu, memory string) api.ResourceList { return res } -func getResourceRequirements(limits, requests api.ResourceList) api.ResourceRequirements { +func getResourceRequirements(requests, limits api.ResourceList) api.ResourceRequirements { res := api.ResourceRequirements{} - res.Limits = limits res.Requests = requests + res.Limits = limits return res } +// createLimitRange creates a limit range with the specified data +func createLimitRange(limitType api.LimitType, min, max, defaultLimit, defaultRequest api.ResourceList) api.LimitRange { + return api.LimitRange{ + ObjectMeta: api.ObjectMeta{ + Name: "abc", + Namespace: "test", + }, + Spec: api.LimitRangeSpec{ + Limits: []api.LimitRangeItem{ + { + Type: limitType, + Min: min, + Max: max, + Default: defaultLimit, + DefaultRequest: defaultRequest, + }, + }, + }, + } +} + func validLimitRange() api.LimitRange { return api.LimitRange{ ObjectMeta: api.ObjectMeta{ @@ -59,10 +80,11 @@ func validLimitRange() api.LimitRange { Min: getResourceList("50m", "2Mi"), }, { - Type: api.LimitTypeContainer, - Max: getResourceList("100m", "2Gi"), - Min: getResourceList("25m", "1Mi"), - Default: getResourceList("50m", "5Mi"), + Type: api.LimitTypeContainer, + Max: getResourceList("100m", "2Gi"), + Min: getResourceList("25m", "1Mi"), + Default: getResourceList("75m", "10Mi"), + DefaultRequest: getResourceList("50m", "5Mi"), }, }, }, @@ -110,8 +132,8 @@ func validPod(name string, numContainers int, resources api.ResourceRequirements func TestDefaultContainerResourceRequirements(t *testing.T) { limitRange := validLimitRange() expected := api.ResourceRequirements{ - Limits: getResourceList("50m", "5Mi"), - Requests: api.ResourceList{}, + Requests: getResourceList("50m", "5Mi"), + Limits: getResourceList("75m", "10Mi"), } actual := defaultContainerResourceRequirements(&limitRange) @@ -125,7 +147,7 @@ func TestDefaultContainerResourceRequirements(t *testing.T) { func TestMergePodResourceRequirements(t *testing.T) { limitRange := validLimitRange() - // pod with no resources enumerated should get each resource from default + // pod with no resources enumerated should get each resource from default request expected := getResourceRequirements(getResourceList("", ""), getResourceList("", "")) pod := validPod("empty-resources", 1, expected) defaultRequirements := defaultContainerResourceRequirements(&limitRange) @@ -141,11 +163,11 @@ func TestMergePodResourceRequirements(t *testing.T) { input := getResourceRequirements(getResourceList("", "512Mi"), getResourceList("", "")) pod = validPod("limit-memory", 1, input) expected = api.ResourceRequirements{ - Limits: api.ResourceList{ - api.ResourceCPU: defaultRequirements.Limits[api.ResourceCPU], + Requests: api.ResourceList{ + api.ResourceCPU: defaultRequirements.Requests[api.ResourceCPU], api.ResourceMemory: resource.MustParse("512Mi"), }, - Requests: api.ResourceList{}, + Limits: defaultRequirements.Limits, } mergePodResourceRequirements(&pod, &defaultRequirements) for i := range pod.Spec.Containers { @@ -157,34 +179,172 @@ func TestMergePodResourceRequirements(t *testing.T) { } func TestPodLimitFunc(t *testing.T) { - limitRange := validLimitRange() - successCases := []api.Pod{ - validPod("foo", 2, getResourceRequirements(getResourceList("100m", "2Gi"), getResourceList("", ""))), - validPod("bar", 1, getResourceRequirements(getResourceList("100m", "2Gi"), getResourceList("", ""))), + type testCase struct { + pod api.Pod + limitRange api.LimitRange } - errorCases := map[string]api.Pod{ - "min-container-cpu": validPod("foo", 1, getResourceRequirements(getResourceList("25m", "2Gi"), getResourceList("", ""))), - "max-container-cpu": validPod("foo", 1, getResourceRequirements(getResourceList("110m", "1Gi"), getResourceList("", ""))), - "min-container-mem": validPod("foo", 1, getResourceRequirements(getResourceList("30m", "0"), getResourceList("", ""))), - "max-container-mem": validPod("foo", 1, getResourceRequirements(getResourceList("30m", "3Gi"), getResourceList("", ""))), - "min-pod-cpu": validPod("foo", 1, getResourceRequirements(getResourceList("40m", "2Gi"), getResourceList("", ""))), - "max-pod-cpu": validPod("foo", 4, getResourceRequirements(getResourceList("60m", "1Mi"), getResourceList("", ""))), - "max-pod-memory": validPod("foo", 3, getResourceRequirements(getResourceList("60m", "2Gi"), getResourceList("", ""))), - "min-pod-memory": validPod("foo", 3, getResourceRequirements(getResourceList("60m", "0"), getResourceList("", ""))), + successCases := []testCase{ + { + pod: validPod("ctr-min-cpu-request", 1, getResourceRequirements(getResourceList("100m", ""), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("50m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-min-cpu-request-limit", 1, getResourceRequirements(getResourceList("100m", ""), getResourceList("200m", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("50m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-min-memory-request", 1, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("", "50Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-min-memory-request-limit", 1, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", "100Mi"))), + limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("", "50Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-max-cpu-request-limit", 1, getResourceRequirements(getResourceList("500m", ""), getResourceList("1", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-max-cpu-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("1", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-max-mem-request-limit", 1, getResourceRequirements(getResourceList("", "250Mi"), getResourceList("", "500Mi"))), + limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-max-mem-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", "500Mi"))), + limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-min-cpu-request", 2, getResourceRequirements(getResourceList("75m", ""), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypePod, getResourceList("100m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-min-cpu-request-limit", 2, getResourceRequirements(getResourceList("75m", ""), getResourceList("200m", ""))), + limitRange: createLimitRange(api.LimitTypePod, getResourceList("100m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-min-memory-request", 2, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypePod, getResourceList("", "100Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-min-memory-request-limit", 2, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", "100Mi"))), + limitRange: createLimitRange(api.LimitTypePod, getResourceList("", "100Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-max-cpu-request-limit", 2, getResourceRequirements(getResourceList("500m", ""), getResourceList("1", ""))), + limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-max-cpu-limit", 2, getResourceRequirements(getResourceList("", ""), getResourceList("1", ""))), + limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-max-mem-request-limit", 2, getResourceRequirements(getResourceList("", "250Mi"), getResourceList("", "500Mi"))), + limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-max-mem-limit", 2, getResourceRequirements(getResourceList("", ""), getResourceList("", "500Mi"))), + limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}), + }, } - for i := range successCases { - err := PodLimitFunc(&limitRange, &successCases[i]) + test := successCases[i] + err := PodLimitFunc(&test.limitRange, &test.pod) if err != nil { - t.Errorf("Unexpected error for valid pod: %v, %v", successCases[i].Name, err) + t.Errorf("Unexpected error for pod: %s, %v", test.pod.Name, err) } } - for k, v := range errorCases { - err := PodLimitFunc(&limitRange, &v) + errorCases := []testCase{ + { + pod: validPod("ctr-min-cpu-request", 1, getResourceRequirements(getResourceList("40m", ""), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("50m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-min-cpu-request-limit", 1, getResourceRequirements(getResourceList("40m", ""), getResourceList("200m", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("50m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-min-cpu-no-request-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("50m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-min-memory-request", 1, getResourceRequirements(getResourceList("", "40Mi"), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("", "50Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-min-memory-request-limit", 1, getResourceRequirements(getResourceList("", "40Mi"), getResourceList("", "100Mi"))), + limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("", "50Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-min-memory-no-request-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, getResourceList("", "50Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-max-cpu-request-limit", 1, getResourceRequirements(getResourceList("500m", ""), getResourceList("2500m", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-max-cpu-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("2500m", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-max-cpu-no-request-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-max-mem-request-limit", 1, getResourceRequirements(getResourceList("", "250Mi"), getResourceList("", "2Gi"))), + limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-max-mem-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", "2Gi"))), + limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("ctr-max-mem-no-request-limit", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypeContainer, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-min-cpu-request", 1, getResourceRequirements(getResourceList("75m", ""), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypePod, getResourceList("100m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-min-cpu-request-limit", 1, getResourceRequirements(getResourceList("75m", ""), getResourceList("200m", ""))), + limitRange: createLimitRange(api.LimitTypePod, getResourceList("100m", ""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-min-memory-request", 1, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", ""))), + limitRange: createLimitRange(api.LimitTypePod, getResourceList("", "100Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-min-memory-request-limit", 1, getResourceRequirements(getResourceList("", "60Mi"), getResourceList("", "100Mi"))), + limitRange: createLimitRange(api.LimitTypePod, getResourceList("", "100Mi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-max-cpu-request-limit", 3, getResourceRequirements(getResourceList("500m", ""), getResourceList("1", ""))), + limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-max-cpu-limit", 3, getResourceRequirements(getResourceList("", ""), getResourceList("1", ""))), + limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("2", ""), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-max-mem-request-limit", 3, getResourceRequirements(getResourceList("", "250Mi"), getResourceList("", "500Mi"))), + limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}), + }, + { + pod: validPod("pod-max-mem-limit", 3, getResourceRequirements(getResourceList("", ""), getResourceList("", "500Mi"))), + limitRange: createLimitRange(api.LimitTypePod, api.ResourceList{}, getResourceList("", "1Gi"), api.ResourceList{}, api.ResourceList{}), + }, + } + for i := range errorCases { + test := errorCases[i] + err := PodLimitFunc(&test.limitRange, &test.pod) if err == nil { - t.Errorf("Expected error for %s", k) + t.Errorf("Expected error for pod: %s", test.pod.Name) } } } @@ -199,23 +359,22 @@ func TestPodLimitFuncApplyDefault(t *testing.T) { for i := range testPod.Spec.Containers { container := testPod.Spec.Containers[i] - memory := testPod.Spec.Containers[i].Resources.Limits.Memory().String() - cpu := testPod.Spec.Containers[i].Resources.Limits.Cpu().String() - switch container.Image { - case "boo:V1": - if memory != "100Mi" { - t.Errorf("Unexpected memory value %s", memory) - } - if cpu != "50m" { - t.Errorf("Unexpected cpu value %s", cpu) - } - case "foo:V1": - if memory != "2Gi" { - t.Errorf("Unexpected memory value %s", memory) - } - if cpu != "100m" { - t.Errorf("Unexpected cpu value %s", cpu) - } + limitMemory := container.Resources.Limits.Memory().String() + limitCpu := container.Resources.Limits.Cpu().String() + requestMemory := container.Resources.Requests.Memory().String() + requestCpu := container.Resources.Requests.Cpu().String() + + if limitMemory != "10Mi" { + t.Errorf("Unexpected memory value %s", limitMemory) + } + if limitCpu != "75m" { + t.Errorf("Unexpected cpu value %s", limitCpu) + } + if requestMemory != "5Mi" { + t.Errorf("Unexpected memory value %s", requestMemory) + } + if requestCpu != "50m" { + t.Errorf("Unexpected cpu value %s", requestCpu) } } }