-
Notifications
You must be signed in to change notification settings - Fork 55
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
Conversation
c806116
to
78d3d47
Compare
pkg/library/overrides/testdata/container-overrides/container-defines-overrides.yaml
Outdated
Show resolved
Hide resolved
pkg/library/overrides/testdata/container-overrides/error_cannot-parse-override.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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 👍
There was a problem hiding this 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
/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>
d0b5535
to
8985cf0
Compare
Codecov ReportBase: 48.50% // Head: 49.16% // Increases project coverage by
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
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. |
[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 |
What does this PR do?
This PR supercedes #868
Adds two new attributes:
pod-overrides
can be used to define overrides for the workspace podcontainer-overrides
can be used to define overrides for individual workspace containersThe 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. Forcontainer-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:
For
pod-overrides
, since there are multiple places this attribute can be defined, all instances of the attribute are considered in the following order:pod-overrides
attributes in DevWorkspace components, in the order they appearpod-overrides
attribute in the global attributes field.spec.template.attributes
with later definitions overriding fields in previous ones. For example, the workspace
will resolve to the overall override
Pod overrides can also be specified as json for compactness (without enclosing quotes):
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
andsamples/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
/test v8-devworkspace-operator-e2e, v8-che-happy-path
to trigger)v8-devworkspace-operator-e2e
: DevWorkspace e2e testv8-che-happy-path
: Happy path for verification integration with Che