diff --git a/api/swagger-spec/v1.json b/api/swagger-spec/v1.json index a6f18664eb9..0eb42575432 100644 --- a/api/swagger-spec/v1.json +++ b/api/swagger-spec/v1.json @@ -11895,7 +11895,7 @@ }, "requests": { "type": "any", - "description": "Minimum amount of resources requested; requests are honored only for persistent volumes as of now; see http://releases.k8s.io/HEAD/docs/design/resources.md#resource-specifications" + "description": "Minimum amount of resources requested; if Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value; see http://releases.k8s.io/HEAD/docs/design/resources.md#resource-specifications" } } }, diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go index 347d1bd477d..7531bfeb58a 100644 --- a/pkg/api/testing/fuzzer.go +++ b/pkg/api/testing/fuzzer.go @@ -159,6 +159,22 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer { //q.Amount.SetScale(inf.Scale(-c.Intn(12))) q.Amount.SetUnscaled(c.Int63n(1000)) }, + func(q *api.ResourceRequirements, c fuzz.Continue) { + randomQuantity := func() resource.Quantity { + return *resource.NewQuantity(c.Int63n(1000), resource.DecimalExponent) + } + q.Limits = make(api.ResourceList) + q.Requests = make(api.ResourceList) + cpuLimit := randomQuantity() + q.Limits[api.ResourceCPU] = *cpuLimit.Copy() + q.Requests[api.ResourceCPU] = *cpuLimit.Copy() + memoryLimit := randomQuantity() + q.Limits[api.ResourceMemory] = *memoryLimit.Copy() + q.Requests[api.ResourceMemory] = *memoryLimit.Copy() + storageLimit := randomQuantity() + q.Limits[api.ResourceStorage] = *storageLimit.Copy() + q.Requests[api.ResourceStorage] = *storageLimit.Copy() + }, 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 910ce2b35ad..0cfa4c21e00 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -682,12 +682,11 @@ type Capabilities struct { // ResourceRequirements describes the compute resource requirements. type ResourceRequirements struct { - // Limits describes the maximum amount of compute resources required. + // Limits describes the maximum amount of compute resources allowed. Limits ResourceList `json:"limits,omitempty"` // Requests describes the minimum amount of compute resources required. - // Note: 'Requests' are honored only for Persistent Volumes as of now. - // TODO: Update the scheduler to use 'Requests' in addition to 'Limits'. If Request is omitted for a container, - // it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value + // If Request is omitted for a container, it defaults to Limits if that is explicitly specified, + // otherwise to an implementation-defined value Requests ResourceList `json:"requests,omitempty"` } diff --git a/pkg/api/v1/defaults.go b/pkg/api/v1/defaults.go index b89461bf457..dbd0b2581b6 100644 --- a/pkg/api/v1/defaults.go +++ b/pkg/api/v1/defaults.go @@ -156,6 +156,19 @@ func addDefaultingFuncs() { obj.APIVersion = "v1" } }, + func(obj *ResourceRequirements) { + // Set requests to limits if requests are not specified (but limits are). + if obj.Limits != nil { + if obj.Requests == nil { + obj.Requests = make(ResourceList) + } + for key, value := range obj.Limits { + if _, exists := obj.Requests[key]; !exists { + obj.Requests[key] = *(value.Copy()) + } + } + } + }, ) } diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index c51e8a2f89d..0b247e84ac4 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -656,13 +656,12 @@ type Capabilities struct { // ResourceRequirements describes the compute resource requirements. type ResourceRequirements struct { - // Limits describes the maximum amount of compute resources required. + // Limits describes the maximum amount of compute resources allowed. Limits ResourceList `json:"limits,omitempty" description:"Maximum amount of compute resources allowed; see http://releases.k8s.io/HEAD/docs/design/resources.md#resource-specifications"` // Requests describes the minimum amount of compute resources required. - // Note: 'Requests' are honored only for Persistent Volumes as of now. - // TODO: Update the scheduler to use 'Requests' in addition to 'Limits'. If Request is omitted for a container, - // it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value - Requests ResourceList `json:"requests,omitempty" description:"Minimum amount of resources requested; requests are honored only for persistent volumes as of now; see http://releases.k8s.io/HEAD/docs/design/resources.md#resource-specifications"` + // If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + // otherwise to an implementation-defined value. + Requests ResourceList `json:"requests,omitempty" description:"Minimum amount of resources requested; if Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value; see http://releases.k8s.io/HEAD/docs/design/resources.md#resource-specifications"` } const ( diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index e61ed13cb7c..7e1bfabe442 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -1490,19 +1490,32 @@ func ValidateResourceRequirements(requirements *api.ResourceRequirements) errs.V allErrs := errs.ValidationErrorList{} for resourceName, quantity := range requirements.Limits { // Validate resource name. - errs := validateResourceName(resourceName.String(), fmt.Sprintf("resources.limits[%s]", resourceName)) + allErrs = append(allErrs, validateResourceName(resourceName.String(), fmt.Sprintf("resources.limits[%s]", resourceName))...) if api.IsStandardResourceName(resourceName.String()) { - errs = append(errs, validateBasicResource(quantity).Prefix(fmt.Sprintf("Resource %s: ", resourceName))...) + allErrs = append(allErrs, validateBasicResource(quantity).Prefix(fmt.Sprintf("Resource %s: ", resourceName))...) + } + // Check that request <= limit. + requestQuantity, exists := requirements.Requests[resourceName] + if exists { + var requestValue, limitValue int64 + requestValue = requestQuantity.Value() + limitValue = quantity.Value() + // Do a more precise comparison if possible (if the value won't overflow). + if requestValue <= resource.MaxMilliValue && limitValue <= resource.MaxMilliValue { + requestValue = requestQuantity.MilliValue() + limitValue = quantity.MilliValue() + } + if limitValue < requestValue { + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("resources.limits[%s]", resourceName), quantity.String(), "limit cannot be smaller than request")) + } } - allErrs = append(allErrs, errs...) } for resourceName, quantity := range requirements.Requests { // Validate resource name. - errs := validateResourceName(resourceName.String(), fmt.Sprintf("resources.requests[%s]", resourceName)) + allErrs = append(allErrs, validateResourceName(resourceName.String(), fmt.Sprintf("resources.requests[%s]", resourceName))...) if api.IsStandardResourceName(resourceName.String()) { - errs = append(errs, validateBasicResource(quantity).Prefix(fmt.Sprintf("Resource %s: ", resourceName))...) + allErrs = append(allErrs, validateBasicResource(quantity).Prefix(fmt.Sprintf("Resource %s: ", resourceName))...) } - allErrs = append(allErrs, errs...) } return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index add2fbb3db0..77d1953b1f3 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -844,6 +844,62 @@ func TestValidateContainers(t *testing.T) { }, ImagePullPolicy: "IfNotPresent", }, + { + Name: "resources-request-limit-simple", + Image: "image", + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceCPU): resource.MustParse("8"), + }, + Limits: api.ResourceList{ + api.ResourceName(api.ResourceCPU): resource.MustParse("10"), + }, + }, + ImagePullPolicy: "IfNotPresent", + }, + { + Name: "resources-request-limit-edge", + Image: "image", + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceCPU): resource.MustParse("10"), + api.ResourceName(api.ResourceMemory): resource.MustParse("10G"), + api.ResourceName("my.org/resource"): resource.MustParse("10m"), + }, + Limits: api.ResourceList{ + api.ResourceName(api.ResourceCPU): resource.MustParse("10"), + api.ResourceName(api.ResourceMemory): resource.MustParse("10G"), + api.ResourceName("my.org/resource"): resource.MustParse("10m"), + }, + }, + ImagePullPolicy: "IfNotPresent", + }, + { + Name: "resources-request-limit-partials", + Image: "image", + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceCPU): resource.MustParse("9.5"), + api.ResourceName(api.ResourceMemory): resource.MustParse("10G"), + }, + Limits: api.ResourceList{ + api.ResourceName(api.ResourceCPU): resource.MustParse("10"), + api.ResourceName("my.org/resource"): resource.MustParse("10m"), + }, + }, + ImagePullPolicy: "IfNotPresent", + }, + { + Name: "resources-request", + Image: "image", + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceCPU): resource.MustParse("9.5"), + api.ResourceName(api.ResourceMemory): resource.MustParse("10G"), + }, + }, + ImagePullPolicy: "IfNotPresent", + }, { Name: "same-host-port-different-protocol", Image: "image", @@ -1011,6 +1067,28 @@ func TestValidateContainers(t *testing.T) { ImagePullPolicy: "IfNotPresent", }, }, + "Request limit simple invalid": { + { + Name: "abc-123", + Image: "image", + Resources: api.ResourceRequirements{ + Limits: getResourceLimits("5", "3"), + Requests: getResourceLimits("6", "3"), + }, + ImagePullPolicy: "IfNotPresent", + }, + }, + "Request limit multiple invalid": { + { + Name: "abc-123", + Image: "image", + Resources: api.ResourceRequirements{ + Limits: getResourceLimits("5", "3"), + Requests: getResourceLimits("6", "4"), + }, + ImagePullPolicy: "IfNotPresent", + }, + }, } for k, v := range errorCases { if errs := validateContainers(v, volumes); len(errs) == 0 { diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 609345537cb..6c05f9cb920 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -579,7 +579,19 @@ func (dm *DockerManager) runContainer( } } memoryLimit := container.Resources.Limits.Memory().Value() - cpuShares := milliCPUToShares(container.Resources.Limits.Cpu().MilliValue()) + cpuRequest := container.Resources.Requests.Cpu() + cpuLimit := container.Resources.Limits.Cpu() + var cpuShares int64 + // If request is not specified, but limit is, we want request to default to limit. + // API server does this for new containers, but we repeat this logic in Kubelet + // for containers running on existing Kubernetes clusters. + if cpuRequest.Amount == nil && cpuLimit.Amount != nil { + cpuShares = milliCPUToShares(cpuLimit.MilliValue()) + } else { + // if cpuRequest.Amount is nil, then milliCPUToShares will return the minimal number + // of CPU shares. + cpuShares = milliCPUToShares(cpuRequest.MilliValue()) + } dockerOpts := docker.CreateContainerOptions{ Name: BuildDockerName(dockerName, container), Config: &docker.Config{ diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index e586bc954ee..e3ba56219c7 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -2100,7 +2100,7 @@ func TestHandleMemExceeded(t *testing.T) { testKubelet.fakeCadvisor.On("RootFsInfo").Return(cadvisorApiv2.FsInfo{}, nil) spec := api.PodSpec{Containers: []api.Container{{Resources: api.ResourceRequirements{ - Limits: api.ResourceList{ + Requests: api.ResourceList{ "memory": resource.MustParse("90"), }, }}}} diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates.go b/plugin/pkg/scheduler/algorithm/predicates/predicates.go index 63bd46f6e55..2d51f0166c8 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates.go @@ -107,10 +107,10 @@ type resourceRequest struct { func getResourceRequest(pod *api.Pod) resourceRequest { result := resourceRequest{} - for ix := range pod.Spec.Containers { - limits := pod.Spec.Containers[ix].Resources.Limits - result.memory += limits.Memory().Value() - result.milliCPU += limits.Cpu().MilliValue() + for _, container := range pod.Spec.Containers { + requests := container.Resources.Requests + result.memory += requests.Memory().Value() + result.milliCPU += requests.Cpu().MilliValue() } return result } diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go index 6c3a63b2179..6b6bde14053 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -59,7 +59,7 @@ func newResourcePod(usage ...resourceRequest) *api.Pod { for _, req := range usage { containers = append(containers, api.Container{ Resources: api.ResourceRequirements{ - Limits: api.ResourceList{ + Requests: api.ResourceList{ api.ResourceCPU: *resource.NewMilliQuantity(req.milliCPU, resource.DecimalSI), api.ResourceMemory: *resource.NewQuantity(req.memory, resource.BinarySI), },