configurable default Workspace Spec Template #867

AObuchow · 2022-06-13T14:45:27Z

This PR adds a new CRD configuration option defaultTemplate to DWO. The configuration option takes in a DevWorkspaceTemplateSpecContent which gets applied to the workspace's devfile if the devfile does not define any components. Currently, the DevWorkspaceTemplateSpecContent will overwrite the devfile's existing template spec but keep the existing projects (if they were defined).

What issues does this PR fix or reference?

Fix #853

Is it tested? How?

Create a minimal devfile, such as the following:

kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: minimal
spec:
  started: true

Start up DWO
Edit the DWO config custom resource kubectl edit dwoc -n $NAMESPACE
Add the defaultTemplate to the workspace attribute:

    defaultTemplate:
      commands:
      - exec:
          commandLine: echo "Hello from $(pwd)"
          component: theia-ide
          workingDir: ${PROJECTS_ROOT}/project/app
        id: say-hello
      components:
      - name: theia
        plugin:
          components:
          - container:
              env:
              - name: THEIA_HOST
                value: 0.0.0.0
            name: theia-ide
          uri: https://che-plugin-registry-main.surge.sh/v3/plugins/eclipse/che-theia/next/devfile.yaml
      projects:
      - git:
          remotes:
            origin: https://github.com/che-samples/web-nodejs-sample.git
        name: web-nodejs-sample

The resulting DWO configuration should ressemble something like the following:

apiVersion: controller.devfile.io/v1alpha1
config:
  routing:
    clusterHostSuffix: 192.168.39.217.nip.io
    defaultRoutingClass: basic
  workspace:
    defaultTemplate:
      commands:
      - exec:
          commandLine: echo "Hello from $(pwd)"
          component: theia-ide
          workingDir: ${PROJECTS_ROOT}/project/app
        id: say-hello
      components:
      - name: theia
        plugin:
          components:
          - container:
              env:
              - name: THEIA_HOST
                value: 0.0.0.0
            name: theia-ide
          uri: https://che-plugin-registry-main.surge.sh/v3/plugins/eclipse/che-theia/next/devfile.yaml
      projects:
      - git:
          remotes:
            origin: https://github.com/che-samples/web-nodejs-sample.git
        name: web-nodejs-sample
    imagePullPolicy: Always
kind: DevWorkspaceOperatorConfig
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"controller.devfile.io/v1alpha1","config":{"routing":{"clusterHostSuffix":"192.168.39.217.nip.io","defaultRoutingClass":"basic"},"workspace":{"imagePu\
llPolicy":"Always"}},"kind":"DevWorkspaceOperatorConfig","metadata":{"annotations":{},"name":"devworkspace-operator-config","namespace":"devworkspace-controller"}}
  creationTimestamp: "2022-06-10T20:28:38Z"
  generation: 4
  name: devworkspace-operator-config
  namespace: devworkspace-controller
  resourceVersion: "42646"
  uid: 4f5198b5-1d0d-48d6-9f10-6d0955363934

Apply the minimal devfile from step 1: kubectl apply -f minimal_devfile.yaml -n $NAMESPACE
Wait for the workspace to startup and open it:

$: kubectl get devworkspace -n $NAMESPACE 
NAME      DEVWORKSPACE ID             PHASE     INFO
minimal   workspace7dab320bcbd64672   Running   https://workspace7dab320bcbd64672-theia-3100.192.168.39.217.nip.io/

Once opened, the workspace should contain the web-nodejs-sample project

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

controllers/workspace/devworkspace_controller.go

AObuchow · 2022-06-13T15:23:55Z

To test out that existing projects (from the minimal devfile) are not being overwritten, try the following devfile:

kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: minimal
spec:
  started: true
  template:
    projects:
      - name: go-starter
        git:
          remotes:
            origin: https://github.com/devfile-samples/devfile-stack-go.git

After applying the devfile and opening the workspace, the node and go project should both be present:

AObuchow · 2022-06-13T18:07:46Z

Currently, sync_test'sTestMergeConfigLooksAtAllFieldsis failing when fuzzing the expected config. It seems to have trouble fuzzing the DefaultTemplate, specifically for the Components.

controllers/workspace/devworkspace_controller.go

pkg/config/defaults.go

AObuchow · 2022-06-13T18:24:56Z

Currently, sync_test'sTestMergeConfigLooksAtAllFieldsis failing when fuzzing the expected config. It seems to have trouble fuzzing the DefaultTemplate, specifically for the Components.

The fuzzer seems to be panicking on Object from runtime.RawExtension which is reached from the DefaultTemplate:
DefaultTemplate *dw.DevWorkspaceTemplateSpecContent -> Components []Component -> ComponentUnion -> Custom *CustomComponent -> EmbeddedResource runtime.RawExtension -> Object Object

AObuchow · 2022-06-13T21:38:31Z

pkg/library/workspace/helper.go

+// limitations under the License.
+//
+
+package workspace


I'm not really sure about this package name or file name...
We already have /pkg/controllers/workspace and there's also another file called helper.go for the flatten package.

Why not name the package e.g. defaults or workspacedefaults

I went with defaults as it seems more generic/could potentially be reused more easily in the future. However, I'm not yet sure about the filename helper.go, maybe it should just be called workspace.go?

AObuchow · 2022-06-13T22:03:12Z

Currently, sync_test'sTestMergeConfigLooksAtAllFieldsis failing when fuzzing the expected config. It seems to have trouble fuzzing the DefaultTemplate, specifically for the Components.

The fuzzer seems to be panicking on Object from runtime.RawExtension which is reached from the DefaultTemplate: DefaultTemplate *dw.DevWorkspaceTemplateSpecContent -> Components []Component -> ComponentUnion -> Custom *CustomComponent -> EmbeddedResource runtime.RawExtension -> Object Object

This was fixed with f.SkipFieldsWithPattern(regexp.MustCompile("^Object$")) but there might be a better way to fix this?

pkg/config/defaults.go

controllers/workspace/devworkspace_controller.go

pkg/config/sync_test.go

amisevsk · 2022-06-14T20:03:41Z

pkg/library/workspace/helper.go

+// limitations under the License.
+//
+
+package workspace


Why not name the package e.g. defaults or workspacedefaults

pkg/library/workspace/helper.go

amisevsk

LGTM, still need to test it (tomorrow)

AObuchow · 2022-06-16T14:15:31Z

LGTM, still need to test it (tomorrow)

Sounds good :) I just rebased the PR

amisevsk

Tested on OpenShift and it works like a charm 👍

Well done!

amisevsk · 2022-06-17T00:49:31Z

Don't forget to squash the commits down to something tidy and make sure all commits have signoff -- tests will need to be re-run once this is done.

/ok-to-test

Fix devfile#853 Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

Part of devfile#853 Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

ibuziuk · 2022-06-21T15:24:01Z

deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml

@@ -80,6 +80,1278 @@ spec:
                        pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
                        x-kubernetes-int-or-string: true
                    type: object
+                  defaultTemplate:


@AObuchow could you please clarify if the spec for defaultTemplate was created from scratch?

It was automatically created with make generate manifests fmt generate_default_deployment generate_olm_bundle_yaml. The reason it is so large is because of all the fields contained within the DevWorkspaceTemplateSpecContent struct

ibuziuk

great job 👍

openshift-ci · 2022-06-23T13:49:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, AObuchow, 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:

~~OWNERS~~ [amisevsk]

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

AObuchow requested review from amisevsk and ibuziuk as code owners June 13, 2022 14:45

openshift-ci bot added the do-not-merge/work-in-progress label Jun 13, 2022

AObuchow commented Jun 13, 2022

View reviewed changes

controllers/workspace/devworkspace_controller.go Outdated Show resolved Hide resolved

AObuchow commented Jun 13, 2022

View reviewed changes

controllers/workspace/devworkspace_controller.go Outdated Show resolved Hide resolved

amisevsk reviewed Jun 13, 2022

View reviewed changes

controllers/workspace/devworkspace_controller.go Outdated Show resolved Hide resolved

controllers/workspace/devworkspace_controller.go Outdated Show resolved Hide resolved

pkg/config/defaults.go Outdated Show resolved Hide resolved

AObuchow commented Jun 13, 2022

View reviewed changes

AObuchow force-pushed the AObuchow/issue853 branch from c42f8e5 to d6f87b6 Compare June 13, 2022 22:05

AObuchow commented Jun 13, 2022

View reviewed changes

pkg/config/defaults.go Outdated Show resolved Hide resolved

AObuchow requested a review from amisevsk June 13, 2022 22:05

amisevsk reviewed Jun 14, 2022

View reviewed changes

AObuchow force-pushed the AObuchow/issue853 branch from d6f87b6 to 2439266 Compare June 15, 2022 04:59

amisevsk approved these changes Jun 15, 2022

View reviewed changes

openshift-ci bot assigned amisevsk Jun 15, 2022

openshift-ci bot added lgtm approved labels Jun 15, 2022

AObuchow changed the title ~~WIP: configurable default Workspace Spec Template~~ configurable default Workspace Spec Template Jun 16, 2022

openshift-ci bot removed the do-not-merge/work-in-progress label Jun 16, 2022

AObuchow force-pushed the AObuchow/issue853 branch from 2439266 to 630ef81 Compare June 16, 2022 14:14

openshift-ci bot removed the lgtm label Jun 16, 2022

amisevsk approved these changes Jun 16, 2022

View reviewed changes

openshift-ci bot added the lgtm label Jun 16, 2022

openshift-ci bot added the ok-to-test label Jun 17, 2022

feat: configurable default Workspace Spec Template

b1ac05e

Fix devfile#853 Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

Generate config option for DefaultTemplate spec

41c8db7

Part of devfile#853 Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

AObuchow force-pushed the AObuchow/issue853 branch from 630ef81 to 41c8db7 Compare June 17, 2022 14:08

openshift-ci bot removed the lgtm label Jun 17, 2022

ibuziuk reviewed Jun 21, 2022

View reviewed changes

ibuziuk approved these changes Jun 23, 2022

View reviewed changes

openshift-ci bot assigned ibuziuk Jun 23, 2022

openshift-ci bot added the lgtm label Jun 23, 2022

AObuchow merged commit bc018d1 into devfile:main Jun 23, 2022

AObuchow deleted the AObuchow/issue853 branch June 23, 2022 14:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

configurable default Workspace Spec Template #867

configurable default Workspace Spec Template #867

AObuchow commented Jun 13, 2022

AObuchow commented Jun 13, 2022

AObuchow commented Jun 13, 2022

AObuchow commented Jun 13, 2022

AObuchow Jun 13, 2022

amisevsk Jun 14, 2022

AObuchow Jun 15, 2022

AObuchow commented Jun 13, 2022

amisevsk Jun 14, 2022

amisevsk left a comment

AObuchow commented Jun 16, 2022

amisevsk left a comment

amisevsk commented Jun 17, 2022

ibuziuk Jun 21, 2022

AObuchow Jun 22, 2022

ibuziuk left a comment

openshift-ci bot commented Jun 23, 2022

configurable default Workspace Spec Template #867

configurable default Workspace Spec Template #867

Conversation

AObuchow commented Jun 13, 2022

What issues does this PR fix or reference?

Is it tested? How?

PR Checklist

AObuchow commented Jun 13, 2022

AObuchow commented Jun 13, 2022

AObuchow commented Jun 13, 2022

AObuchow Jun 13, 2022

Choose a reason for hiding this comment

amisevsk Jun 14, 2022

Choose a reason for hiding this comment

AObuchow Jun 15, 2022

Choose a reason for hiding this comment

AObuchow commented Jun 13, 2022

amisevsk Jun 14, 2022

Choose a reason for hiding this comment

amisevsk left a comment

Choose a reason for hiding this comment

AObuchow commented Jun 16, 2022

amisevsk left a comment

Choose a reason for hiding this comment

amisevsk commented Jun 17, 2022

ibuziuk Jun 21, 2022

Choose a reason for hiding this comment

AObuchow Jun 22, 2022

Choose a reason for hiding this comment

ibuziuk left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jun 23, 2022