From 70d7d33e5996d0ea9ad19803261572c25e3acad9 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Tue, 9 Dec 2025 17:31:49 +0100 Subject: [PATCH 1/8] Add container resource caps enforcement for workspace containers Implements ContainerResourceCaps configuration to enforce maximum resource limits and requests for workspace containers. When configured, container resource requirements that exceed the caps will be limited to the maximum values. This feature does not apply to initContainers or projectClone containers. Changes include: - New ContainerResourceCaps field in DevWorkspaceOperatorConfig API - Updated CRDs with the new field definition - Controller integration to pass resource caps to container handlers - Resource capping logic in container, flatten, merge, and projects packages - Tests for resource cap enforcement Assisted-by: Claude. Signed-off-by: Anatolii Bazko --- .../devworkspaceoperatorconfig_types.go | 5 + .../v1alpha1/zz_generated.deepcopy.go | 5 + .../workspace/devworkspace_controller.go | 3 + ...evfile.io_devworkspaceoperatorconfigs.yaml | 63 +++++++++++ deploy/deployment/kubernetes/combined.yaml | 63 +++++++++++ ...r.devfile.io.CustomResourceDefinition.yaml | 63 +++++++++++ deploy/deployment/openshift/combined.yaml | 63 +++++++++++ ...r.devfile.io.CustomResourceDefinition.yaml | 63 +++++++++++ ...evfile.io_devworkspaceoperatorconfigs.yaml | 63 +++++++++++ pkg/config/sync.go | 9 ++ pkg/library/container/container.go | 14 ++- pkg/library/container/container_test.go | 2 +- pkg/library/container/conversion.go | 9 +- pkg/library/flatten/flatten.go | 3 +- pkg/library/flatten/merge.go | 16 ++- pkg/library/projects/clone.go | 1 + pkg/library/resources/helper.go | 107 +++++++++++++++++- pkg/library/resources/helper_test.go | 99 ++++++++++++++++ 18 files changed, 636 insertions(+), 15 deletions(-) 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..9368362c6 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -264,6 +264,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request K8sClient: r.Client, HttpClient: httpClient, DefaultResourceRequirements: workspace.Config.Workspace.DefaultContainerResources, + ResourceCaps: workspace.Config.Workspace.ContainerResourceCaps, } if wsDefaults.NeedsDefaultTemplate(workspace) { @@ -335,6 +336,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, ) @@ -356,6 +358,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request Image: workspace.Config.Workspace.ProjectCloneConfig.Image, Env: env.GetEnvironmentVariablesForProjectClone(workspace), Resources: workspace.Config.Workspace.ProjectCloneConfig.Resources, + Caps: workspace.Config.Workspace.ContainerResourceCaps, } if workspace.Config.Workspace.ProjectCloneConfig.ImagePullPolicy != "" { projectCloneOptions.PullPolicy = config.Workspace.ProjectCloneConfig.ImagePullPolicy 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..673daa8ff 100644 --- a/pkg/library/container/container.go +++ b/pkg/library/container/container.go @@ -45,7 +45,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") } @@ -60,7 +68,7 @@ func GetKubeContainersFromDevfile(workspace *dw.DevWorkspaceTemplateSpec, securi if component.Container == nil { continue } - k8sContainer, err := convertContainerToK8s(component, securityContext, pullPolicy, defaultResources) + k8sContainer, err := convertContainerToK8s(component, securityContext, pullPolicy, defaultResources, resourceCaps) if err != nil { return nil, err } @@ -86,7 +94,7 @@ func GetKubeContainersFromDevfile(workspace *dw.DevWorkspaceTemplateSpec, securi } for _, initComponent := range initComponents { - k8sContainer, err := convertContainerToK8s(initComponent, securityContext, pullPolicy, defaultResources) + k8sContainer, err := convertContainerToK8s(initComponent, securityContext, pullPolicy, defaultResources, nil) if err != nil { return nil, err } 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/container/conversion.go b/pkg/library/container/conversion.go index c2bbd5dd7..5a46c1107 100644 --- a/pkg/library/container/conversion.go +++ b/pkg/library/container/conversion.go @@ -25,7 +25,13 @@ import ( corev1 "k8s.io/api/core/v1" ) -func convertContainerToK8s(devfileComponent dw.Component, securityContext *corev1.SecurityContext, pullPolicy string, defaultResources *corev1.ResourceRequirements) (*corev1.Container, error) { +func convertContainerToK8s( + devfileComponent dw.Component, + securityContext *corev1.SecurityContext, + pullPolicy string, + defaultResources *corev1.ResourceRequirements, + resourceCaps *corev1.ResourceRequirements, +) (*corev1.Container, error) { if devfileComponent.Container == nil { return nil, fmt.Errorf("cannot get k8s container from non-container component") } @@ -36,6 +42,7 @@ func convertContainerToK8s(devfileComponent dw.Component, securityContext *corev return nil, fmt.Errorf("failed to get resources for container %s: %s", devfileComponent.Name, err) } containerResources = dwResources.ApplyDefaults(containerResources, defaultResources) + containerResources = dwResources.ApplyCaps(containerResources, resourceCaps) if err := dwResources.ValidateResources(containerResources); err != nil { return nil, fmt.Errorf("container resources are invalid for component %s: %w", devfileComponent.Name, err) } diff --git a/pkg/library/flatten/flatten.go b/pkg/library/flatten/flatten.go index c30063a53..292001935 100644 --- a/pkg/library/flatten/flatten.go +++ b/pkg/library/flatten/flatten.go @@ -50,6 +50,7 @@ type ResolverTools struct { K8sClient client.Client HttpClient network.HTTPGetter DefaultResourceRequirements *corev1.ResourceRequirements + ResourceCaps *corev1.ResourceRequirements } // ResolveDevWorkspace takes a devworkspace and returns a "resolved" version of it -- i.e. one where all plugins and parents @@ -67,7 +68,7 @@ func ResolveDevWorkspace(workspace *dw.DevWorkspaceTemplateSpec, contributions [ } if needsMerge, err := needsContainerContributionMerge(resolvedDW); needsMerge { - if err := mergeContainerContributions(resolvedDW, tooling.DefaultResourceRequirements); err != nil { + if err := mergeContainerContributions(resolvedDW, tooling.DefaultResourceRequirements, tooling.ResourceCaps); err != nil { return nil, nil, err } } else if err != nil { diff --git a/pkg/library/flatten/merge.go b/pkg/library/flatten/merge.go index dad88b3ad..55171acf0 100644 --- a/pkg/library/flatten/merge.go +++ b/pkg/library/flatten/merge.go @@ -175,7 +175,11 @@ func needsContainerContributionMerge(flattenedSpec *dw.DevWorkspaceTemplateSpec) return hasContribution && hasTarget, nil } -func mergeContainerContributions(flattenedSpec *dw.DevWorkspaceTemplateSpec, defaultResources *corev1.ResourceRequirements) error { +func mergeContainerContributions( + flattenedSpec *dw.DevWorkspaceTemplateSpec, + defaultResources *corev1.ResourceRequirements, + resourceCaps *corev1.ResourceRequirements, +) error { var contributions []dw.Component contributionNameSet := map[string]bool{} for _, component := range flattenedSpec.Components { @@ -201,7 +205,7 @@ func mergeContainerContributions(flattenedSpec *dw.DevWorkspaceTemplateSpec, def // drop contributions from updated list as they will be merged continue } else if component.Name == targetComponentName && !mergeDone { - mergedComponent, err := mergeContributionsInto(&component, contributions, defaultResources) + mergedComponent, err := mergeContributionsInto(&component, contributions, defaultResources, resourceCaps) if err != nil { return fmt.Errorf("failed to merge container contributions: %w", err) } @@ -269,7 +273,12 @@ func findMergeTarget(flattenedSpec *dw.DevWorkspaceTemplateSpec) (mergeTargetCom return "", fmt.Errorf("couldn't find any merge contribution target component") } -func mergeContributionsInto(mergeInto *dw.Component, contributions []dw.Component, defaultResources *corev1.ResourceRequirements) (*dw.Component, error) { +func mergeContributionsInto( + mergeInto *dw.Component, + contributions []dw.Component, + defaultResources *corev1.ResourceRequirements, + resourceCaps *corev1.ResourceRequirements, +) (*dw.Component, error) { if mergeInto == nil || mergeInto.Container == nil { return nil, fmt.Errorf("attempting to merge container contributions into a non-container component") } @@ -303,6 +312,7 @@ func mergeContributionsInto(mergeInto *dw.Component, contributions []dw.Componen return nil, err } totalResources = dwResources.AddResourceRequirements(totalResources, componentResources) + totalResources = dwResources.ApplyCaps(totalResources, resourceCaps) component.Container.MemoryLimit = "" component.Container.MemoryRequest = "" diff --git a/pkg/library/projects/clone.go b/pkg/library/projects/clone.go index c1e4d9de1..34f29def3 100644 --- a/pkg/library/projects/clone.go +++ b/pkg/library/projects/clone.go @@ -38,6 +38,7 @@ type Options struct { Image string PullPolicy corev1.PullPolicy Resources *corev1.ResourceRequirements + Caps *corev1.ResourceRequirements Env []corev1.EnvVar } diff --git a/pkg/library/resources/helper.go b/pkg/library/resources/helper.go index c18135d8d..757e14444 100644 --- a/pkg/library/resources/helper.go +++ b/pkg/library/resources/helper.go @@ -166,21 +166,21 @@ func ApplyDefaults(resources, defaults *corev1.ResourceRequirements) *corev1.Res } // Set default limits if not present - for resource, quantity := range defaults.Limits { + for resourceName, quantity := range defaults.Limits { if result.Limits == nil { result.Limits = corev1.ResourceList{} } - if _, ok := result.Limits[resource]; !ok && !quantity.IsZero() { - result.Limits[resource] = quantity + if _, ok := result.Limits[resourceName]; !ok && !quantity.IsZero() { + result.Limits[resourceName] = quantity } } // Set default requests if not present - for resource, quantity := range defaults.Requests { + for resourceName, quantity := range defaults.Requests { if result.Requests == nil { result.Requests = corev1.ResourceList{} } - if _, ok := result.Requests[resource]; !ok && !quantity.IsZero() { - result.Requests[resource] = quantity + if _, ok := result.Requests[resourceName]; !ok && !quantity.IsZero() { + result.Requests[resourceName] = quantity } } @@ -220,6 +220,101 @@ 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) + } + + // Edge cases: after applying caps, we might create invalid resources (limit < request). + // We need to adjust to ensure the result is still valid. + memLimit := result.Limits[corev1.ResourceMemory] + memRequest := result.Requests[corev1.ResourceMemory] + if !memLimit.IsZero() && !memRequest.IsZero() && memLimit.Cmp(memRequest) < 0 { + capMemLimit := caps.Limits[corev1.ResourceMemory] + capMemRequest := caps.Requests[corev1.ResourceMemory] + switch { + case !capMemLimit.IsZero() && capMemRequest.IsZero(): + // Cap limit caused the issue, adjust request down to match limit + result.Requests[corev1.ResourceMemory] = capMemLimit + case capMemLimit.IsZero() && !capMemRequest.IsZero(): + // Cap request as maximum shouldn't cause limit < request, but adjust request down to match limit + result.Requests[corev1.ResourceMemory] = memLimit + default: + // Both caps or neither caps - use caps limit for both to ensure validity + if !capMemLimit.IsZero() { + result.Limits[corev1.ResourceMemory] = capMemLimit + result.Requests[corev1.ResourceMemory] = capMemLimit + } else { + result.Requests[corev1.ResourceMemory] = memLimit + } + } + } + + cpuLimit := result.Limits[corev1.ResourceCPU] + cpuRequest := result.Requests[corev1.ResourceCPU] + if !cpuLimit.IsZero() && !cpuRequest.IsZero() && cpuLimit.Cmp(cpuRequest) < 0 { + capCPULimit := caps.Limits[corev1.ResourceCPU] + capCPURequest := caps.Requests[corev1.ResourceCPU] + switch { + case !capCPULimit.IsZero() && capCPURequest.IsZero(): + // Cap limit caused the issue, adjust request down to match limit + result.Requests[corev1.ResourceCPU] = capCPULimit + case capCPULimit.IsZero() && !capCPURequest.IsZero(): + // Cap request as maximum shouldn't cause limit < request, but adjust request down to match limit + result.Requests[corev1.ResourceCPU] = cpuLimit + default: + // Both caps or neither caps - use caps limit for both to ensure validity + if !capCPULimit.IsZero() { + result.Limits[corev1.ResourceCPU] = capCPULimit + result.Requests[corev1.ResourceCPU] = capCPULimit + } else { + result.Requests[corev1.ResourceCPU] = cpuLimit + } + } + } + + 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..555816083 100644 --- a/pkg/library/resources/helper_test.go +++ b/pkg/library/resources/helper_test.go @@ -304,6 +304,105 @@ 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: "Caps maximum limits and maximum requests", + base: getResourceRequirements("3000Mi", "50Mi", "3000m", "50m"), + caps: getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"), + expected: getResourceRequirements("2000Mi", "50Mi", "2000m", "50m"), + }, + { + 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("2000Mi", "", "2000m", ""), + 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: "Caps maximum limit (existing higher than caps)", + base: getResourceRequirements("5000Mi", "100Mi", "5000m", "100m"), + caps: getResourceRequirements("2000Mi", "", "2000m", ""), + expected: getResourceRequirements("2000Mi", "100Mi", "2000m", "100m"), + }, + { + name: "Caps maximum request (existing higher than caps)", + base: getResourceRequirements("5000Mi", "500Mi", "5000m", "500m"), + caps: getResourceRequirements("", "200Mi", "", "200m"), + expected: getResourceRequirements("5000Mi", "200Mi", "5000m", "200m"), + }, + { + name: "Adjusts memory request when caps limit creates conflict", + base: getResourceRequirements("", "1000Mi", "", ""), + caps: getResourceRequirements("500Mi", "", "", ""), + expected: getResourceRequirements("500Mi", "500Mi", "", ""), + }, + { + name: "Adjusts memory request when caps request creates conflict", + base: getResourceRequirements("100Mi", "200Mi", "", ""), + caps: getResourceRequirements("", "500Mi", "", ""), + expected: getResourceRequirements("100Mi", "100Mi", "", ""), + }, + { + name: "Adjusts CPU request when caps limit creates conflict", + base: getResourceRequirements("", "", "", "1000m"), + caps: getResourceRequirements("", "", "500m", ""), + expected: getResourceRequirements("", "", "500m", "500m"), + }, + { + name: "Adjusts CPU request when caps request creates conflict", + base: getResourceRequirements("", "", "100m", "200m"), + caps: getResourceRequirements("", "", "", "500m"), + expected: getResourceRequirements("", "", "100m", "100m"), + }, + { + name: "Handles conflicting caps values (both limit and request capped, request > limit)", + base: getResourceRequirements("100Mi", "1000Mi", "100m", "1000m"), + caps: getResourceRequirements("500Mi", "1000Mi", "500m", "1000m"), + expected: getResourceRequirements("500Mi", "500Mi", "500m", "500m"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := ApplyCaps(tt.base, tt.caps) + if !assert.Equal(t, tt.expected, actual) { + // Print more useful diff since quantity is not a simple struct + expectedYaml, _ := yaml.Marshal(tt.expected) + actualYaml, _ := yaml.Marshal(actual) + t.Logf("\nExpected:\n%s\nActual:\n%s", expectedYaml, actualYaml) + } + }) + } +} + func TestValidate(t *testing.T) { tests := []struct { name string From bf8aa66636c54b5a965a7b1086178d57e81283e4 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 10 Dec 2025 10:02:26 +0100 Subject: [PATCH 2/8] fixup Signed-off-by: Anatolii Bazko --- pkg/library/resources/helper_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/library/resources/helper_test.go b/pkg/library/resources/helper_test.go index 555816083..5337cd969 100644 --- a/pkg/library/resources/helper_test.go +++ b/pkg/library/resources/helper_test.go @@ -393,12 +393,7 @@ func TestApplyCaps(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { actual := ApplyCaps(tt.base, tt.caps) - if !assert.Equal(t, tt.expected, actual) { - // Print more useful diff since quantity is not a simple struct - expectedYaml, _ := yaml.Marshal(tt.expected) - actualYaml, _ := yaml.Marshal(actual) - t.Logf("\nExpected:\n%s\nActual:\n%s", expectedYaml, actualYaml) - } + assert.Equal(t, tt.expected, actual) }) } } From 55a9d3a0984993c7c4c8428733a63333881eb820 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 10 Dec 2025 11:49:55 +0100 Subject: [PATCH 3/8] fixup Signed-off-by: Anatolii Bazko --- pkg/library/container/container.go | 10 ++++++++-- pkg/library/container/conversion.go | 2 -- pkg/library/flatten/merge.go | 1 - 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/library/container/container.go b/pkg/library/container/container.go index 673daa8ff..74177bb0d 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" ) @@ -68,7 +69,7 @@ func GetKubeContainersFromDevfile( if component.Container == nil { continue } - k8sContainer, err := convertContainerToK8s(component, securityContext, pullPolicy, defaultResources, resourceCaps) + k8sContainer, err := convertContainerToK8s(component, securityContext, pullPolicy, defaultResources) if err != nil { return nil, err } @@ -82,6 +83,11 @@ func GetKubeContainersFromDevfile( } k8sContainer = patchedContainer } + + // applying caps only after overrides + resources := dwResources.ApplyCaps(&k8sContainer.Resources, resourceCaps) + k8sContainer.Resources = *resources + podAdditions.Containers = append(podAdditions.Containers, *k8sContainer) } @@ -94,7 +100,7 @@ func GetKubeContainersFromDevfile( } for _, initComponent := range initComponents { - k8sContainer, err := convertContainerToK8s(initComponent, securityContext, pullPolicy, defaultResources, nil) + k8sContainer, err := convertContainerToK8s(initComponent, securityContext, pullPolicy, defaultResources) if err != nil { return nil, err } diff --git a/pkg/library/container/conversion.go b/pkg/library/container/conversion.go index 5a46c1107..e48d9f2e6 100644 --- a/pkg/library/container/conversion.go +++ b/pkg/library/container/conversion.go @@ -30,7 +30,6 @@ func convertContainerToK8s( securityContext *corev1.SecurityContext, pullPolicy string, defaultResources *corev1.ResourceRequirements, - resourceCaps *corev1.ResourceRequirements, ) (*corev1.Container, error) { if devfileComponent.Container == nil { return nil, fmt.Errorf("cannot get k8s container from non-container component") @@ -42,7 +41,6 @@ func convertContainerToK8s( return nil, fmt.Errorf("failed to get resources for container %s: %s", devfileComponent.Name, err) } containerResources = dwResources.ApplyDefaults(containerResources, defaultResources) - containerResources = dwResources.ApplyCaps(containerResources, resourceCaps) if err := dwResources.ValidateResources(containerResources); err != nil { return nil, fmt.Errorf("container resources are invalid for component %s: %w", devfileComponent.Name, err) } diff --git a/pkg/library/flatten/merge.go b/pkg/library/flatten/merge.go index 55171acf0..412139857 100644 --- a/pkg/library/flatten/merge.go +++ b/pkg/library/flatten/merge.go @@ -312,7 +312,6 @@ func mergeContributionsInto( return nil, err } totalResources = dwResources.AddResourceRequirements(totalResources, componentResources) - totalResources = dwResources.ApplyCaps(totalResources, resourceCaps) component.Container.MemoryLimit = "" component.Container.MemoryRequest = "" From ea9d22fa3d0c0a2a42b97321a120ed3ea879d2bf Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 10 Dec 2025 11:53:01 +0100 Subject: [PATCH 4/8] fixup Signed-off-by: Anatolii Bazko --- pkg/library/container/conversion.go | 7 +------ pkg/library/flatten/flatten.go | 3 +-- pkg/library/flatten/merge.go | 15 +++------------ pkg/library/projects/clone.go | 1 - 4 files changed, 5 insertions(+), 21 deletions(-) diff --git a/pkg/library/container/conversion.go b/pkg/library/container/conversion.go index e48d9f2e6..c2bbd5dd7 100644 --- a/pkg/library/container/conversion.go +++ b/pkg/library/container/conversion.go @@ -25,12 +25,7 @@ import ( corev1 "k8s.io/api/core/v1" ) -func convertContainerToK8s( - devfileComponent dw.Component, - securityContext *corev1.SecurityContext, - pullPolicy string, - defaultResources *corev1.ResourceRequirements, -) (*corev1.Container, error) { +func convertContainerToK8s(devfileComponent dw.Component, securityContext *corev1.SecurityContext, pullPolicy string, defaultResources *corev1.ResourceRequirements) (*corev1.Container, error) { if devfileComponent.Container == nil { return nil, fmt.Errorf("cannot get k8s container from non-container component") } diff --git a/pkg/library/flatten/flatten.go b/pkg/library/flatten/flatten.go index 292001935..c30063a53 100644 --- a/pkg/library/flatten/flatten.go +++ b/pkg/library/flatten/flatten.go @@ -50,7 +50,6 @@ type ResolverTools struct { K8sClient client.Client HttpClient network.HTTPGetter DefaultResourceRequirements *corev1.ResourceRequirements - ResourceCaps *corev1.ResourceRequirements } // ResolveDevWorkspace takes a devworkspace and returns a "resolved" version of it -- i.e. one where all plugins and parents @@ -68,7 +67,7 @@ func ResolveDevWorkspace(workspace *dw.DevWorkspaceTemplateSpec, contributions [ } if needsMerge, err := needsContainerContributionMerge(resolvedDW); needsMerge { - if err := mergeContainerContributions(resolvedDW, tooling.DefaultResourceRequirements, tooling.ResourceCaps); err != nil { + if err := mergeContainerContributions(resolvedDW, tooling.DefaultResourceRequirements); err != nil { return nil, nil, err } } else if err != nil { diff --git a/pkg/library/flatten/merge.go b/pkg/library/flatten/merge.go index 412139857..dad88b3ad 100644 --- a/pkg/library/flatten/merge.go +++ b/pkg/library/flatten/merge.go @@ -175,11 +175,7 @@ func needsContainerContributionMerge(flattenedSpec *dw.DevWorkspaceTemplateSpec) return hasContribution && hasTarget, nil } -func mergeContainerContributions( - flattenedSpec *dw.DevWorkspaceTemplateSpec, - defaultResources *corev1.ResourceRequirements, - resourceCaps *corev1.ResourceRequirements, -) error { +func mergeContainerContributions(flattenedSpec *dw.DevWorkspaceTemplateSpec, defaultResources *corev1.ResourceRequirements) error { var contributions []dw.Component contributionNameSet := map[string]bool{} for _, component := range flattenedSpec.Components { @@ -205,7 +201,7 @@ func mergeContainerContributions( // drop contributions from updated list as they will be merged continue } else if component.Name == targetComponentName && !mergeDone { - mergedComponent, err := mergeContributionsInto(&component, contributions, defaultResources, resourceCaps) + mergedComponent, err := mergeContributionsInto(&component, contributions, defaultResources) if err != nil { return fmt.Errorf("failed to merge container contributions: %w", err) } @@ -273,12 +269,7 @@ func findMergeTarget(flattenedSpec *dw.DevWorkspaceTemplateSpec) (mergeTargetCom return "", fmt.Errorf("couldn't find any merge contribution target component") } -func mergeContributionsInto( - mergeInto *dw.Component, - contributions []dw.Component, - defaultResources *corev1.ResourceRequirements, - resourceCaps *corev1.ResourceRequirements, -) (*dw.Component, error) { +func mergeContributionsInto(mergeInto *dw.Component, contributions []dw.Component, defaultResources *corev1.ResourceRequirements) (*dw.Component, error) { if mergeInto == nil || mergeInto.Container == nil { return nil, fmt.Errorf("attempting to merge container contributions into a non-container component") } diff --git a/pkg/library/projects/clone.go b/pkg/library/projects/clone.go index 34f29def3..c1e4d9de1 100644 --- a/pkg/library/projects/clone.go +++ b/pkg/library/projects/clone.go @@ -38,7 +38,6 @@ type Options struct { Image string PullPolicy corev1.PullPolicy Resources *corev1.ResourceRequirements - Caps *corev1.ResourceRequirements Env []corev1.EnvVar } From 5155e99d3575cd19926a21cc5e386b333e07999d Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 10 Dec 2025 11:55:59 +0100 Subject: [PATCH 5/8] fixup Signed-off-by: Anatolii Bazko --- controllers/workspace/devworkspace_controller.go | 2 -- pkg/library/resources/helper.go | 12 ++++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index 9368362c6..3b9ccb5df 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -264,7 +264,6 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request K8sClient: r.Client, HttpClient: httpClient, DefaultResourceRequirements: workspace.Config.Workspace.DefaultContainerResources, - ResourceCaps: workspace.Config.Workspace.ContainerResourceCaps, } if wsDefaults.NeedsDefaultTemplate(workspace) { @@ -358,7 +357,6 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request Image: workspace.Config.Workspace.ProjectCloneConfig.Image, Env: env.GetEnvironmentVariablesForProjectClone(workspace), Resources: workspace.Config.Workspace.ProjectCloneConfig.Resources, - Caps: workspace.Config.Workspace.ContainerResourceCaps, } if workspace.Config.Workspace.ProjectCloneConfig.ImagePullPolicy != "" { projectCloneOptions.PullPolicy = config.Workspace.ProjectCloneConfig.ImagePullPolicy diff --git a/pkg/library/resources/helper.go b/pkg/library/resources/helper.go index 757e14444..2fceb885d 100644 --- a/pkg/library/resources/helper.go +++ b/pkg/library/resources/helper.go @@ -166,21 +166,21 @@ func ApplyDefaults(resources, defaults *corev1.ResourceRequirements) *corev1.Res } // Set default limits if not present - for resourceName, quantity := range defaults.Limits { + for resource, quantity := range defaults.Limits { if result.Limits == nil { result.Limits = corev1.ResourceList{} } - if _, ok := result.Limits[resourceName]; !ok && !quantity.IsZero() { - result.Limits[resourceName] = quantity + if _, ok := result.Limits[resource]; !ok && !quantity.IsZero() { + result.Limits[resource] = quantity } } // Set default requests if not present - for resourceName, quantity := range defaults.Requests { + for resource, quantity := range defaults.Requests { if result.Requests == nil { result.Requests = corev1.ResourceList{} } - if _, ok := result.Requests[resourceName]; !ok && !quantity.IsZero() { - result.Requests[resourceName] = quantity + if _, ok := result.Requests[resource]; !ok && !quantity.IsZero() { + result.Requests[resource] = quantity } } From dca26b2c4d6f960805d2dc8ca09d9b1fc7b7f729 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 10 Dec 2025 15:34:03 +0100 Subject: [PATCH 6/8] fixup Signed-off-by: Anatolii Bazko --- pkg/library/resources/helper.go | 55 ++++++++++++++++++---------- pkg/library/resources/helper_test.go | 54 ++++++--------------------- 2 files changed, 47 insertions(+), 62 deletions(-) diff --git a/pkg/library/resources/helper.go b/pkg/library/resources/helper.go index 2fceb885d..b9f76a9ff 100644 --- a/pkg/library/resources/helper.go +++ b/pkg/library/resources/helper.go @@ -273,19 +273,26 @@ func ApplyCaps(resources, caps *corev1.ResourceRequirements) *corev1.ResourceReq capMemRequest := caps.Requests[corev1.ResourceMemory] switch { case !capMemLimit.IsZero() && capMemRequest.IsZero(): - // Cap limit caused the issue, adjust request down to match limit - result.Requests[corev1.ResourceMemory] = capMemLimit - case capMemLimit.IsZero() && !capMemRequest.IsZero(): - // Cap request as maximum shouldn't cause limit < request, but adjust request down to match limit - result.Requests[corev1.ResourceMemory] = memLimit - default: - // Both caps or neither caps - use caps limit for both to ensure validity - if !capMemLimit.IsZero() { - result.Limits[corev1.ResourceMemory] = capMemLimit + // Only a memory 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 memLimit.Equal(capMemLimit) { result.Requests[corev1.ResourceMemory] = capMemLimit } else { - result.Requests[corev1.ResourceMemory] = memLimit + // The invalid state (limit < request) existed in the original resources before caps were applied. } + case capMemLimit.IsZero() && !capMemRequest.IsZero(): + // Only a memory request cap was set, and it's higher than the existing limit. + // Adjust the limit up to match the capped request. + if memRequest.Equal(capMemRequest) { + result.Limits[corev1.ResourceMemory] = capMemRequest + } 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 } } @@ -296,19 +303,27 @@ func ApplyCaps(resources, caps *corev1.ResourceRequirements) *corev1.ResourceReq capCPURequest := caps.Requests[corev1.ResourceCPU] switch { case !capCPULimit.IsZero() && capCPURequest.IsZero(): - // Cap limit caused the issue, adjust request down to match limit - result.Requests[corev1.ResourceCPU] = capCPULimit - case capCPULimit.IsZero() && !capCPURequest.IsZero(): - // Cap request as maximum shouldn't cause limit < request, but adjust request down to match limit - result.Requests[corev1.ResourceCPU] = cpuLimit - default: - // Both caps or neither caps - use caps limit for both to ensure validity - if !capCPULimit.IsZero() { - result.Limits[corev1.ResourceCPU] = capCPULimit + // Only a CPU 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 cpuLimit.Equal(capCPULimit) { result.Requests[corev1.ResourceCPU] = capCPULimit } else { - result.Requests[corev1.ResourceCPU] = cpuLimit + // The invalid state (limit < request) existed in the original resources before caps were applied. + break } + case capCPULimit.IsZero() && !capCPURequest.IsZero(): + // Only a CPU request cap was set, and it's higher than the existing limit. + // Adjust the limit up to match the capped request. + if cpuRequest.Equal(capCPURequest) { + result.Limits[corev1.ResourceCPU] = capCPURequest + } 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 } } diff --git a/pkg/library/resources/helper_test.go b/pkg/library/resources/helper_test.go index 5337cd969..58b3ef5ad 100644 --- a/pkg/library/resources/helper_test.go +++ b/pkg/library/resources/helper_test.go @@ -318,10 +318,10 @@ func TestApplyCaps(t *testing.T) { expected: getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"), }, { - name: "Caps maximum limits and maximum requests", - base: getResourceRequirements("3000Mi", "50Mi", "3000m", "50m"), + name: "Applies all caps values", + base: getResourceRequirements("3000Mi", "500Mi", "3000m", "500m"), caps: getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"), - expected: getResourceRequirements("2000Mi", "50Mi", "2000m", "50m"), + expected: getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"), }, { name: "Keeps existing values when within caps bounds", @@ -332,7 +332,7 @@ func TestApplyCaps(t *testing.T) { { name: "Does not apply empty caps fields", base: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"), - caps: getResourceRequirements("2000Mi", "", "2000m", ""), + caps: getResourceRequirements("", "", "", ""), expected: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"), }, { @@ -348,46 +348,16 @@ func TestApplyCaps(t *testing.T) { expected: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"), }, { - name: "Caps maximum limit (existing higher than caps)", - base: getResourceRequirements("5000Mi", "100Mi", "5000m", "100m"), - caps: getResourceRequirements("2000Mi", "", "2000m", ""), - expected: getResourceRequirements("2000Mi", "100Mi", "2000m", "100m"), - }, - { - name: "Caps maximum request (existing higher than caps)", - base: getResourceRequirements("5000Mi", "500Mi", "5000m", "500m"), - caps: getResourceRequirements("", "200Mi", "", "200m"), - expected: getResourceRequirements("5000Mi", "200Mi", "5000m", "200m"), - }, - { - name: "Adjusts memory request when caps limit creates conflict", - base: getResourceRequirements("", "1000Mi", "", ""), - caps: getResourceRequirements("500Mi", "", "", ""), - expected: getResourceRequirements("500Mi", "500Mi", "", ""), - }, - { - name: "Adjusts memory request when caps request creates conflict", - base: getResourceRequirements("100Mi", "200Mi", "", ""), - caps: getResourceRequirements("", "500Mi", "", ""), - expected: getResourceRequirements("100Mi", "100Mi", "", ""), - }, - { - name: "Adjusts CPU request when caps limit creates conflict", - base: getResourceRequirements("", "", "", "1000m"), - caps: getResourceRequirements("", "", "500m", ""), - expected: getResourceRequirements("", "", "500m", "500m"), - }, - { - name: "Adjusts CPU request when caps request creates conflict", - base: getResourceRequirements("", "", "100m", "200m"), - caps: getResourceRequirements("", "", "", "500m"), - expected: getResourceRequirements("", "", "100m", "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: "Handles conflicting caps values (both limit and request capped, request > limit)", - base: getResourceRequirements("100Mi", "1000Mi", "100m", "1000m"), - caps: getResourceRequirements("500Mi", "1000Mi", "500m", "1000m"), - 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 { From 6f633cb5575b0b6c6f5ff4239cce9b2e4c93bf84 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 10 Dec 2025 15:49:34 +0100 Subject: [PATCH 7/8] fixup Signed-off-by: Anatolii Bazko --- pkg/library/resources/helper.go | 66 +++++++++++---------------------- 1 file changed, 22 insertions(+), 44 deletions(-) diff --git a/pkg/library/resources/helper.go b/pkg/library/resources/helper.go index b9f76a9ff..5b1ee5826 100644 --- a/pkg/library/resources/helper.go +++ b/pkg/library/resources/helper.go @@ -264,58 +264,36 @@ func ApplyCaps(resources, caps *corev1.ResourceRequirements) *corev1.ResourceReq // 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. - memLimit := result.Limits[corev1.ResourceMemory] - memRequest := result.Requests[corev1.ResourceMemory] - if !memLimit.IsZero() && !memRequest.IsZero() && memLimit.Cmp(memRequest) < 0 { - capMemLimit := caps.Limits[corev1.ResourceMemory] - capMemRequest := caps.Requests[corev1.ResourceMemory] - switch { - case !capMemLimit.IsZero() && capMemRequest.IsZero(): - // Only a memory 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 memLimit.Equal(capMemLimit) { - result.Requests[corev1.ResourceMemory] = capMemLimit - } else { - // The invalid state (limit < request) existed in the original resources before caps were applied. - } - case capMemLimit.IsZero() && !capMemRequest.IsZero(): - // Only a memory request cap was set, and it's higher than the existing limit. - // Adjust the limit up to match the capped request. - if memRequest.Equal(capMemRequest) { - result.Limits[corev1.ResourceMemory] = capMemRequest - } 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 - } - } - - cpuLimit := result.Limits[corev1.ResourceCPU] - cpuRequest := result.Requests[corev1.ResourceCPU] - if !cpuLimit.IsZero() && !cpuRequest.IsZero() && cpuLimit.Cmp(cpuRequest) < 0 { - capCPULimit := caps.Limits[corev1.ResourceCPU] - capCPURequest := caps.Requests[corev1.ResourceCPU] + 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 !capCPULimit.IsZero() && capCPURequest.IsZero(): - // Only a CPU limit cap was set, and it caused the limit to be lower than the request. + 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 cpuLimit.Equal(capCPULimit) { - result.Requests[corev1.ResourceCPU] = capCPULimit + if resLimit.Equal(capResLimit) { + result.Requests[resourceName] = capResLimit } else { // The invalid state (limit < request) existed in the original resources before caps were applied. - break } - case capCPULimit.IsZero() && !capCPURequest.IsZero(): - // Only a CPU request cap was set, and it's higher than the existing limit. + 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 cpuRequest.Equal(capCPURequest) { - result.Limits[corev1.ResourceCPU] = capCPURequest + if resRequest.Equal(capResRequest) { + result.Limits[resourceName] = capResRequest } else { // The invalid state (limit < request) existed in the original resources before caps were applied. break From 34713b8748e187f20fac70e0c16da392a603e348 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 10 Dec 2025 16:12:30 +0100 Subject: [PATCH 8/8] fixup Signed-off-by: Anatolii Bazko --- pkg/library/container/container.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/library/container/container.go b/pkg/library/container/container.go index 74177bb0d..88bfd9854 100644 --- a/pkg/library/container/container.go +++ b/pkg/library/container/container.go @@ -86,6 +86,9 @@ func GetKubeContainersFromDevfile( // 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)