diff --git a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go index 9ffff832f..e4ce7bd72 100644 --- a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go +++ b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go @@ -183,6 +183,11 @@ type WorkspaceConfig struct { // the value "0" should be used. By default, the memory limit is 128Mi and the memory request is 64Mi. // No CPU limit or request is added by default. DefaultContainerResources *corev1.ResourceRequirements `json:"defaultContainerResources,omitempty"` + // ContainerResourceCaps defines the maximum resource requirements enforced for all + // workspace containers. If a container specifies higher limits or requests, they + // will be capped at these maximum values. This is not applied to initContainers or + // the projectClone container. + ContainerResourceCaps *corev1.ResourceRequirements `json:"containerResourceCaps,omitempty"` // PodAnnotations defines the metadata.annotations for DevWorkspace pods created by the DevWorkspace Operator. PodAnnotations map[string]string `json:"podAnnotations,omitempty"` // RuntimeClassName defines the spec.runtimeClassName for DevWorkspace pods created by the DevWorkspace Operator. diff --git a/apis/controller/v1alpha1/zz_generated.deepcopy.go b/apis/controller/v1alpha1/zz_generated.deepcopy.go index f31eb7604..44e2ed806 100644 --- a/apis/controller/v1alpha1/zz_generated.deepcopy.go +++ b/apis/controller/v1alpha1/zz_generated.deepcopy.go @@ -771,6 +771,11 @@ func (in *WorkspaceConfig) DeepCopyInto(out *WorkspaceConfig) { *out = new(v1.ResourceRequirements) (*in).DeepCopyInto(*out) } + if in.ContainerResourceCaps != nil { + in, out := &in.ContainerResourceCaps, &out.ContainerResourceCaps + *out = new(v1.ResourceRequirements) + (*in).DeepCopyInto(*out) + } if in.PodAnnotations != nil { in, out := &in.PodAnnotations, &out.PodAnnotations *out = make(map[string]string, len(*in)) diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index e158e69ad..3b9ccb5df 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -335,6 +335,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request workspace.Config.Workspace.ContainerSecurityContext, workspace.Config.Workspace.ImagePullPolicy, workspace.Config.Workspace.DefaultContainerResources, + workspace.Config.Workspace.ContainerResourceCaps, workspace.Config.Workspace.PostStartTimeout, postStartDebugTrapSleepDuration, ) diff --git a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 471d552c8..7b96f969f 100644 --- a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -217,6 +217,69 @@ spec: .spec.started = false. If set to false, resources will be scaled down (e.g. deployments but the objects will be left on the cluster). The default value is false. type: boolean + containerResourceCaps: + description: |- + ContainerResourceCaps defines the maximum resource requirements enforced for all + workspace containers. If a container specifies higher limits or requests, they + will be capped at these maximum values. This is not applied to initContainers or + the projectClone container. + properties: + claims: + description: |- + Claims lists the names of resources, defined in spec.resourceClaims, + that are used by this container. + + This field depends on the + DynamicResourceAllocation feature gate. + + This field is immutable. It can only be set for containers. + items: + description: ResourceClaim references one entry in PodSpec.ResourceClaims. + properties: + name: + description: |- + Name must match the name of one entry in pod.spec.resourceClaims of + the Pod where this field is used. It makes that resource available + inside a container. + type: string + request: + description: |- + Request is the name chosen for a request in the referenced claim. + If empty, everything from the claim is made available, otherwise + only the result of this request. + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object containerSecurityContext: description: |- ContainerSecurityContext overrides the default ContainerSecurityContext used for all diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index 34953ec1e..99ef4b0b1 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -220,6 +220,69 @@ spec: .spec.started = false. If set to false, resources will be scaled down (e.g. deployments but the objects will be left on the cluster). The default value is false. type: boolean + containerResourceCaps: + description: |- + ContainerResourceCaps defines the maximum resource requirements enforced for all + workspace containers. If a container specifies higher limits or requests, they + will be capped at these maximum values. This is not applied to initContainers or + the projectClone container. + properties: + claims: + description: |- + Claims lists the names of resources, defined in spec.resourceClaims, + that are used by this container. + + This field depends on the + DynamicResourceAllocation feature gate. + + This field is immutable. It can only be set for containers. + items: + description: ResourceClaim references one entry in PodSpec.ResourceClaims. + properties: + name: + description: |- + Name must match the name of one entry in pod.spec.resourceClaims of + the Pod where this field is used. It makes that resource available + inside a container. + type: string + request: + description: |- + Request is the name chosen for a request in the referenced claim. + If empty, everything from the claim is made available, otherwise + only the result of this request. + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object containerSecurityContext: description: |- ContainerSecurityContext overrides the default ContainerSecurityContext used for all diff --git a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml index 910c7f6b5..ee498c6dc 100644 --- a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -220,6 +220,69 @@ spec: .spec.started = false. If set to false, resources will be scaled down (e.g. deployments but the objects will be left on the cluster). The default value is false. type: boolean + containerResourceCaps: + description: |- + ContainerResourceCaps defines the maximum resource requirements enforced for all + workspace containers. If a container specifies higher limits or requests, they + will be capped at these maximum values. This is not applied to initContainers or + the projectClone container. + properties: + claims: + description: |- + Claims lists the names of resources, defined in spec.resourceClaims, + that are used by this container. + + This field depends on the + DynamicResourceAllocation feature gate. + + This field is immutable. It can only be set for containers. + items: + description: ResourceClaim references one entry in PodSpec.ResourceClaims. + properties: + name: + description: |- + Name must match the name of one entry in pod.spec.resourceClaims of + the Pod where this field is used. It makes that resource available + inside a container. + type: string + request: + description: |- + Request is the name chosen for a request in the referenced claim. + If empty, everything from the claim is made available, otherwise + only the result of this request. + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object containerSecurityContext: description: |- ContainerSecurityContext overrides the default ContainerSecurityContext used for all diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 31bb95d7f..843f258d9 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -220,6 +220,69 @@ spec: .spec.started = false. If set to false, resources will be scaled down (e.g. deployments but the objects will be left on the cluster). The default value is false. type: boolean + containerResourceCaps: + description: |- + ContainerResourceCaps defines the maximum resource requirements enforced for all + workspace containers. If a container specifies higher limits or requests, they + will be capped at these maximum values. This is not applied to initContainers or + the projectClone container. + properties: + claims: + description: |- + Claims lists the names of resources, defined in spec.resourceClaims, + that are used by this container. + + This field depends on the + DynamicResourceAllocation feature gate. + + This field is immutable. It can only be set for containers. + items: + description: ResourceClaim references one entry in PodSpec.ResourceClaims. + properties: + name: + description: |- + Name must match the name of one entry in pod.spec.resourceClaims of + the Pod where this field is used. It makes that resource available + inside a container. + type: string + request: + description: |- + Request is the name chosen for a request in the referenced claim. + If empty, everything from the claim is made available, otherwise + only the result of this request. + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object containerSecurityContext: description: |- ContainerSecurityContext overrides the default ContainerSecurityContext used for all diff --git a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml index 910c7f6b5..ee498c6dc 100644 --- a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -220,6 +220,69 @@ spec: .spec.started = false. If set to false, resources will be scaled down (e.g. deployments but the objects will be left on the cluster). The default value is false. type: boolean + containerResourceCaps: + description: |- + ContainerResourceCaps defines the maximum resource requirements enforced for all + workspace containers. If a container specifies higher limits or requests, they + will be capped at these maximum values. This is not applied to initContainers or + the projectClone container. + properties: + claims: + description: |- + Claims lists the names of resources, defined in spec.resourceClaims, + that are used by this container. + + This field depends on the + DynamicResourceAllocation feature gate. + + This field is immutable. It can only be set for containers. + items: + description: ResourceClaim references one entry in PodSpec.ResourceClaims. + properties: + name: + description: |- + Name must match the name of one entry in pod.spec.resourceClaims of + the Pod where this field is used. It makes that resource available + inside a container. + type: string + request: + description: |- + Request is the name chosen for a request in the referenced claim. + If empty, everything from the claim is made available, otherwise + only the result of this request. + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object containerSecurityContext: description: |- ContainerSecurityContext overrides the default ContainerSecurityContext used for all diff --git a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml index d03cd57bd..e1a4ae6ec 100644 --- a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -218,6 +218,69 @@ spec: .spec.started = false. If set to false, resources will be scaled down (e.g. deployments but the objects will be left on the cluster). The default value is false. type: boolean + containerResourceCaps: + description: |- + ContainerResourceCaps defines the maximum resource requirements enforced for all + workspace containers. If a container specifies higher limits or requests, they + will be capped at these maximum values. This is not applied to initContainers or + the projectClone container. + properties: + claims: + description: |- + Claims lists the names of resources, defined in spec.resourceClaims, + that are used by this container. + + This field depends on the + DynamicResourceAllocation feature gate. + + This field is immutable. It can only be set for containers. + items: + description: ResourceClaim references one entry in PodSpec.ResourceClaims. + properties: + name: + description: |- + Name must match the name of one entry in pod.spec.resourceClaims of + the Pod where this field is used. It makes that resource available + inside a container. + type: string + request: + description: |- + Request is the name chosen for a request in the referenced claim. + If empty, everything from the claim is made available, otherwise + only the result of this request. + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object containerSecurityContext: description: |- ContainerSecurityContext overrides the default ContainerSecurityContext used for all diff --git a/pkg/config/sync.go b/pkg/config/sync.go index 76ffd89a5..5e451d602 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -404,6 +404,12 @@ func mergeConfig(from, to *controller.OperatorConfiguration) { } to.Workspace.DefaultContainerResources = mergeResources(from.Workspace.DefaultContainerResources, to.Workspace.DefaultContainerResources) } + if from.Workspace.ContainerResourceCaps != nil { + if to.Workspace.ContainerResourceCaps == nil { + to.Workspace.ContainerResourceCaps = &corev1.ResourceRequirements{} + } + to.Workspace.ContainerResourceCaps = mergeResources(from.Workspace.ContainerResourceCaps, to.Workspace.ContainerResourceCaps) + } if from.Workspace.PodAnnotations != nil { if to.Workspace.PodAnnotations == nil { @@ -667,6 +673,9 @@ func GetCurrentConfigString(currConfig *controller.OperatorConfiguration) string if !reflect.DeepEqual(workspace.DefaultContainerResources, defaultConfig.Workspace.DefaultContainerResources) { config = append(config, "workspace.defaultContainerResources is set") } + if workspace.ContainerResourceCaps != nil { + config = append(config, "workspace.containerResourceCaps is set") + } if !reflect.DeepEqual(workspace.PodAnnotations, defaultConfig.Workspace.PodAnnotations) { config = append(config, "workspace.podAnnotations is set") } diff --git a/pkg/library/container/container.go b/pkg/library/container/container.go index 52dc04290..88bfd9854 100644 --- a/pkg/library/container/container.go +++ b/pkg/library/container/container.go @@ -32,6 +32,7 @@ import ( "github.com/devfile/devworkspace-operator/pkg/library/flatten" "github.com/devfile/devworkspace-operator/pkg/library/lifecycle" "github.com/devfile/devworkspace-operator/pkg/library/overrides" + dwResources "github.com/devfile/devworkspace-operator/pkg/library/resources" corev1 "k8s.io/api/core/v1" ) @@ -45,7 +46,15 @@ import ( // rewritten as Volumes are added to PodAdditions, in order to support e.g. using one PVC to hold all volumes // // Note: Requires DevWorkspace to be flattened (i.e. the DevWorkspace contains no Parent or Components of type Plugin) -func GetKubeContainersFromDevfile(workspace *dw.DevWorkspaceTemplateSpec, securityContext *corev1.SecurityContext, pullPolicy string, defaultResources *corev1.ResourceRequirements, postStartTimeout string, postStartDebugTrapSleepDuration string) (*v1alpha1.PodAdditions, error) { +func GetKubeContainersFromDevfile( + workspace *dw.DevWorkspaceTemplateSpec, + securityContext *corev1.SecurityContext, + pullPolicy string, + defaultResources *corev1.ResourceRequirements, + resourceCaps *corev1.ResourceRequirements, + postStartTimeout string, + postStartDebugTrapSleepDuration string, +) (*v1alpha1.PodAdditions, error) { if !flatten.DevWorkspaceIsFlattened(workspace, nil) { return nil, fmt.Errorf("devfile is not flattened") } @@ -74,6 +83,14 @@ func GetKubeContainersFromDevfile(workspace *dw.DevWorkspaceTemplateSpec, securi } k8sContainer = patchedContainer } + + // applying caps only after overrides + resources := dwResources.ApplyCaps(&k8sContainer.Resources, resourceCaps) + if err := dwResources.ValidateResources(resources); err != nil { + return nil, fmt.Errorf("container resources are invalid for component %s: %w", component.Name, err) + } + k8sContainer.Resources = *resources + podAdditions.Containers = append(podAdditions.Containers, *k8sContainer) } diff --git a/pkg/library/container/container_test.go b/pkg/library/container/container_test.go index 6b98184f2..9f1d56e62 100644 --- a/pkg/library/container/container_test.go +++ b/pkg/library/container/container_test.go @@ -87,7 +87,7 @@ func TestGetKubeContainersFromDevfile(t *testing.T) { t.Run(tt.Name, func(t *testing.T) { // sanity check that file is read correctly. assert.True(t, len(tt.Input.Components) > 0, "Input defines no components") - gotPodAdditions, err := GetKubeContainersFromDevfile(tt.Input, nil, testImagePullPolicy, defaultResources, "", "") + gotPodAdditions, err := GetKubeContainersFromDevfile(tt.Input, nil, testImagePullPolicy, defaultResources, nil, "", "") if tt.Output.ErrRegexp != nil && assert.Error(t, err) { assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") } else { diff --git a/pkg/library/resources/helper.go b/pkg/library/resources/helper.go index c18135d8d..5b1ee5826 100644 --- a/pkg/library/resources/helper.go +++ b/pkg/library/resources/helper.go @@ -220,6 +220,94 @@ func ApplyDefaults(resources, defaults *corev1.ResourceRequirements) *corev1.Res return result } +func ApplyCaps(resources, caps *corev1.ResourceRequirements) *corev1.ResourceRequirements { + result := resources.DeepCopy() + if caps == nil { + return result + } + + // Apply caps limits as maximum values (use the smaller of existing and caps) + for resourceName, capLimit := range caps.Limits { + if capLimit.IsZero() { + continue + } + if result.Limits == nil { + result.Limits = corev1.ResourceList{} + } + existingLimit, hasExisting := result.Limits[resourceName] + if !hasExisting || existingLimit.IsZero() { + // No existing limit, use caps as maximum + result.Limits[resourceName] = capLimit + } else if existingLimit.Cmp(capLimit) > 0 { + // Existing limit is higher than caps, apply caps maximum + result.Limits[resourceName] = capLimit + } + // Otherwise, keep existing limit (it's already lower than or equal to caps) + } + + // Apply caps requests as maximum values (use the smaller of existing and caps) + for resourceName, capRequest := range caps.Requests { + if capRequest.IsZero() { + continue + } + if result.Requests == nil { + result.Requests = corev1.ResourceList{} + } + existingRequest, hasExisting := result.Requests[resourceName] + if !hasExisting || existingRequest.IsZero() { + // No existing request, use caps as maximum + result.Requests[resourceName] = capRequest + } else if existingRequest.Cmp(capRequest) > 0 { + // Existing request is higher than caps, apply caps maximum + result.Requests[resourceName] = capRequest + } + // Otherwise, keep existing request (it's already lower than or equal to caps) + } + + result = handleCapsEdgeCase(result, caps, corev1.ResourceMemory) + result = handleCapsEdgeCase(result, caps, corev1.ResourceCPU) + + return result +} + +func handleCapsEdgeCase(resources, caps *corev1.ResourceRequirements, resourceName corev1.ResourceName) *corev1.ResourceRequirements { + result := resources.DeepCopy() + + // Edge cases: after applying caps, we might create invalid resources (limit < request). + // We need to adjust to ensure the result is still valid. + resLimit := resources.Limits[resourceName] + resRequest := resources.Requests[resourceName] + if !resLimit.IsZero() && !resRequest.IsZero() && resLimit.Cmp(resRequest) < 0 { + capResLimit := caps.Limits[resourceName] + capResRequest := caps.Requests[resourceName] + switch { + case !capResLimit.IsZero() && capResRequest.IsZero(): + // Only a resource limit cap was set, and it caused the limit to be lower than the request. + // Adjust the request down to match the capped limit. + if resLimit.Equal(capResLimit) { + result.Requests[resourceName] = capResLimit + } else { + // The invalid state (limit < request) existed in the original resources before caps were applied. + } + case capResLimit.IsZero() && !capResRequest.IsZero(): + // Only a resource request cap was set, and it's higher than the existing limit. + // Adjust the limit up to match the capped request. + if resRequest.Equal(capResRequest) { + result.Limits[resourceName] = capResRequest + } else { + // The invalid state (limit < request) existed in the original resources before caps were applied. + break + } + default: + // Both limit and request caps were set (or neither was set), so the invalid state was present + // in the original resources. + break + } + } + + return result +} + // ValidateResources validates that a corev1.ResourceRequirements is valid, i.e. that (if specified), limits are greater than or equal to requests func ValidateResources(resources *corev1.ResourceRequirements) error { memLimit, hasMemLimit := resources.Limits[corev1.ResourceMemory] diff --git a/pkg/library/resources/helper_test.go b/pkg/library/resources/helper_test.go index 7ef862739..58b3ef5ad 100644 --- a/pkg/library/resources/helper_test.go +++ b/pkg/library/resources/helper_test.go @@ -304,6 +304,70 @@ func TestApplyDefaults(t *testing.T) { } } +func TestApplyCaps(t *testing.T) { + tests := []struct { + name string + base *corev1.ResourceRequirements + caps *corev1.ResourceRequirements + expected *corev1.ResourceRequirements + }{ + { + name: "Applies all caps values to empty resources", + base: &corev1.ResourceRequirements{}, + caps: getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"), + expected: getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"), + }, + { + name: "Applies all caps values", + base: getResourceRequirements("3000Mi", "500Mi", "3000m", "500m"), + caps: getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"), + expected: getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"), + }, + { + name: "Keeps existing values when within caps bounds", + base: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"), + caps: getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"), + expected: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"), + }, + { + name: "Does not apply empty caps fields", + base: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"), + caps: getResourceRequirements("", "", "", ""), + expected: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"), + }, + { + name: "Handles nil caps", + base: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"), + caps: nil, + expected: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"), + }, + { + name: "Ignores '0' fields in caps", + base: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"), + caps: getResourceRequirements("0", "0", "0", "0"), + expected: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"), + }, + { + name: "Adjusts request when caps limit creates conflict", + base: getResourceRequirements("2000Mi", "1000Mi", "", "1000m"), + caps: getResourceRequirements("500Mi", "", "500m", ""), + expected: getResourceRequirements("500Mi", "500Mi", "500m", "500m"), + }, + { + name: "Adjusts limit when caps request creates conflict", + base: getResourceRequirements("500Mi", "", "500m", ""), + caps: getResourceRequirements("", "1000Mi", "", "1000m"), + expected: getResourceRequirements("1000Mi", "1000Mi", "1000m", "1000m"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := ApplyCaps(tt.base, tt.caps) + assert.Equal(t, tt.expected, actual) + }) + } +} + func TestValidate(t *testing.T) { tests := []struct { name string