Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for defining pod and container overrides via attribute #944

Merged
merged 8 commits into from
Oct 19, 2022

Conversation

amisevsk
Copy link
Collaborator

@amisevsk amisevsk commented Oct 4, 2022

What does this PR do?

This PR supercedes #868

Adds two new attributes:

  • pod-overrides can be used to define overrides for the workspace pod
  • container-overrides can be used to define overrides for individual workspace containers

The implementation of the override follows discussion in devfile/api#920. For pod-overrides, the attribute is used to override fields in a workspace deployment's podSpecTemplate in the same was as defined in #868. For container-overrides, the contents of the attribute are applied as a strategic merge patch on the container.

Container overrides can be specified to e.g. use extended Kubernetes resources:

components:
  - name: go
    attributes:
      container-overrides: {"resources":{"limits":{"nvidia.com/gpu":"1"}}}
    container:
      image: ...

For pod-overrides, since there are multiple places this attribute can be defined, all instances of the attribute are considered in the following order:

  1. Any pod-overrides attributes in DevWorkspace components, in the order they appear
  2. A pod-overrides attribute in the global attributes field .spec.template.attributes
    with later definitions overriding fields in previous ones. For example, the workspace
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: theia-next
spec:
  started: true
  template:
    attributes:
      pod-overrides:
        metadata:
          labels:
            shared-label: global-value
    components:
      - name: my-container-1
        attributes:
          pod-overrides:
            metadata:
              labels:
                shared-label: container1-value # Overridden by my-container-2 attribute, then global attribute
                container1-label: container1-value
                container2-label: container1-value # Overridden by my-container-2 attribute
        container:
          image: quay.io/wto/web-terminal-tooling:next
      - name: my-container-2
        attributes:
          pod-overrides:
            metadata:
              labels:
                shared-label: container2-value # Overridden by global attribute
                container2-label: container2-value
        container:
          image: quay.io/wto/web-terminal-tooling:next

will resolve to the overall override

metadata:
  labels:
    shared-label: global-value
    container1-label: container1-value
    container2-label: container2-value

Pod overrides can also be specified as json for compactness (without enclosing quotes):

kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: sample
spec:
  started: true
  template:
    attributes:
      pod-overrides: {"spec":{"runtimeClassName":"kata"}}
# ...

What issues does this PR fix or reference?

Closes #852
Closes #945

Is it tested? How?

Test cases included in PR. Functionality can be tested using samples/pod-overrides.yaml and samples/container-overrides.yaml. For the pod overrides sample, the workspace will likely fail to stat as it sets the scheduler to stork and the runtimeClassName to kata, which are likely not available. To verify things are working as expected, check that fields are overridden in the Deployment for that workspace.

PR Checklist

  • E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)
    • v8-devworkspace-operator-e2e: DevWorkspace e2e test
    • v8-che-happy-path: Happy path for verification integration with Che

@amisevsk amisevsk requested a review from ibuziuk as a code owner October 4, 2022 20:26
@openshift-ci openshift-ci bot added the approved label Oct 4, 2022
@amisevsk amisevsk changed the title Add support for defining deployment's podSpec overrides via attribute Add support for defining pod and container overrides via attirbute Oct 5, 2022
@amisevsk amisevsk changed the title Add support for defining pod and container overrides via attirbute Add support for defining pod and container overrides via attribute Oct 5, 2022
Copy link
Collaborator

@dkwon17 dkwon17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The samples are working, tried different combinations of pod and container overrides, and the code looks good to me 👍

Copy link
Collaborator

@AObuchow AObuchow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code & test cases looks good to me. Tested on OpenShift and the workspace created by the pod-overrides.yaml sample had the correct fields overridden:

kind: Deployment
apiVersion: apps/v1
metadata:
  annotations:
    deployment.kubernetes.io/revision: '1'
  resourceVersion: '35882'
  name: workspacea0e5f894af174c4a
  uid: b2f001a4-7394-44bd-ba83-f537da406563
  creationTimestamp: '2022-10-05T17:06:14Z'
  generation: 2
(..)
  labels:
    controller.devfile.io/creator: ''
    controller.devfile.io/devworkspace_id: workspacea0e5f894af174c4a
    controller.devfile.io/devworkspace_name: theia-next
spec:
  replicas: 0
  selector:
    matchLabels:
      controller.devfile.io/devworkspace_id: workspacea0e5f894af174c4a
  template:
    metadata:
      name: workspacea0e5f894af174c4a
      namespace: devworkspace-controller
      creationTimestamp: null
      labels:
        controller.devfile.io/creator: ''
        controller.devfile.io/devworkspace_id: workspacea0e5f894af174c4a
        controller.devfile.io/devworkspace_name: theia-next
      annotations:
+        io.kubernetes.cri-o.userns-mode: 'auto:size=65536;map-to-root=true'
+        io.openshift.userns: 'true'
+        openshift.io/scc: container-build
    spec:
      restartPolicy: Always
(..)
      serviceAccountName: workspacea0e5f894af174c4a-sa
      imagePullSecrets:
        - name: workspacea0e5f894af174c4a-sa-dockercfg-4sw5t
+      schedulerName: stork
      terminationGracePeriodSeconds: 10
+      runtimeClassName: kata

@amisevsk
Copy link
Collaborator Author

amisevsk commented Oct 7, 2022

/retest

Since we've decided not to add a field to the DevWorkspace API for
podSpecOverrides, instead use an attribute to support the same purpose.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Add support for overriding any field in the workspace's deployment
PodTemplateSpec via specifying an attribute. To make the attribute
easier to use, it can be defined multiple times in the DevWorkspace,
with each instance of the field being applied over the previous.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Add attribute 'container-overrides' that can be used to override fields
in containers used in a workspace.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Apply container-overrides as strategic merge patches on top of workspace
containers when defined.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@codecov
Copy link

codecov bot commented Oct 11, 2022

Codecov Report

Base: 48.50% // Head: 49.16% // Increases project coverage by +0.65% 🎉

Coverage data is based on head (8985cf0) compared to base (96f534e).
Patch coverage: 68.60% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #944      +/-   ##
==========================================
+ Coverage   48.50%   49.16%   +0.65%     
==========================================
  Files          36       38       +2     
  Lines        2307     2390      +83     
==========================================
+ Hits         1119     1175      +56     
- Misses       1076     1093      +17     
- Partials      112      122      +10     
Impacted Files Coverage Δ
pkg/library/container/container.go 48.38% <23.07%> (-23.05%) ⬇️
pkg/library/overrides/containers.go 67.85% <67.85%> (ø)
pkg/library/overrides/pods.go 82.22% <82.22%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@openshift-ci
Copy link

openshift-ci bot commented Oct 12, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, AObuchow, dkwon17, ibuziuk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants