feat: Allow external DWOC to be merged with internal DWOC on a per-workspace basis

[AObuchow]

What does this PR do?

This PR aims to make 2 changes to DWO:

1. Refactor (almost) all uses of the DWOC, so that, internally, each workspace instance has its own configuration. Currently, this per-workspace devworkspace-operator configuration is not visible to users, and was added only to support changed n.2
2. Allow for specifying an external DWOC (that exists on the cluster), which gets merged with the workspace's internal DWOC

The external DWOC's name and namespace are specified with the controller.devfile.io/devworkspace-config DevWorkspace attribute, which has the following structure:

attributes:
  controller.devfile.io/devworkspace-config:
    name: <string>
    namespace: <string>

What issues does this PR fix or reference?

Part of eclipse-che/che#21405

Is it tested? How?

I added 2 automated tests.

For manual testing, perform the following steps:

1. Deploy DWO
2. Create a devfile with the controller.devfile.io/devworkspace-config attribute set, e.g.

kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: theia-next-external-dwoc
spec:
  started: true
  template:
    attributes:
        controller.devfile.io/devworkspace-config:
          name: external-dwoc-test
          namespace: default
        controller.devfile.io/storage-type: per-workspace
    projects:
      - name: web-nodejs-sample
        git:
          remotes:
            origin: "https://github.com/che-samples/web-nodejs-sample.git"
    components:
      - name: theia
        plugin:
          uri: https://che-plugin-registry-main.surge.sh/v3/plugins/eclipse/che-theia/next/devfile.yaml
          components:
            - name: theia-ide
              container:
                env:
                  - name: THEIA_HOST
                    value: 0.0.0.0
    commands:
      - id: say-hello
        exec:
          component: theia-ide
          commandLine: echo "Hello from $(pwd)"
          workingDir: ${PROJECTS_ROOT}/project/app

3. Create a DWOC file, for reference, we'll call it test-external-dwoc.yaml. Also, ensure its name field is set to the same value as the controller.devfile.io/devworkspace-config-name attribute in the devfile from step 2:

apiVersion: controller.devfile.io/v1alpha1
config:
  enableExperimentalFeatures: true
  routing:
    clusterHostSuffix: 192.168.49.2.nip.io
    defaultRoutingClass: basic
  workspace:
    defaultStorageSize:
      common: 20Gi
      perWorkspace: 29Gi
    imagePullPolicy: Always
kind: DevWorkspaceOperatorConfig
metadata:
  name: external-dwoc-test
  namespace: default

4. Apply the (external) DWOC file you just created, ensuring it's in the namespace you specified in the controller.devfile.io/devworkspace-config-namespace (in my example, the default namespace is used): kubectl apply -f test-external-dwoc.yaml
5. Apply the devfile created in step 2.
6. Verify that the internal DWOC has been modified. I recommend checking the size of the per-workspace PVC that is created and ensuring it is 29Gi instead of the default 10Gi
7. Afterwards, you can ensure that workspace's that do not have the controller.devfile.io/devworkspace-config attribute set are not affected by the external DWOC, and instead, they use the global/internal DWOC. For instance, start up the per-workspace example and ensure the PVC created for it is 10Gi big: kubectl apply -f per-workspace-storage.yaml -n $NAMESPACE.

Some notes and limitations:

1. The HTTP client / Proxy settings cannot be affected by the externally applied DWOC, as the HTTP client is created and configured on DWO startup rather than workspace startup.
2. There is a handler that checks if the common PVC was deleted. Currently, it filters for PVC's related to DWO by checking the PVC name against the global configs PVC name. In the future, this limitation can be worked around by applying a DWO-specific annotation to all PVC's created by DWO, and filtering by that annotation.
3. This change could have weird implications for anything configurable that is shared across workspaces. Most notably, the common PVC. For example, if the common PVC does not yet exist on the cluster, and an external DWOC that has the common PVC size set to 20Gi is applied for a specific workspace, then other (future) workspaces that use the common storage strategy will also be using a PVC of 20Gi, even if they aren't using an external DWOC.

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

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: AObuchow
Once this PR has been reviewed and has the lgtm label, please assign amisevsk for approval by writing /assign @amisevsk in a comment. For more information see:The Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[AObuchow]

This PR is based off of option 2 of @amisevsk's configuring-DWO-from-che proposal

[AObuchow]

@amisevsk Though you are free to take an early look (e.g. checking out the TODO comments), this PR is unfortunately still in progress.

I still need to add some tests, document some stuff and try and make the commit history a bit cleaner.

[AObuchow]

The git history is unfortunately still very messy, so it might be better to review this PR as a whole (as difficult as that may be, due to the number of files modified in this PR :/ )

[AObuchow]

@amisevsk Should I make note of the new attribute in the additional configuration documentation? Or do we want to keep this as an internal/undocumented feature for the time being, seeing as it was intended for a specific use case (che)?

[amisevsk]

First pass at reviewing. Looking okay so far but I'm still not 100% on the approach we're taking -- there's some mess and inconsistency on whether we pass the extended struct (WorkspaceWithConfig) or whether we pass the config separately (e.g. in library packages) and I'm not sure which approach is better.

[AObuchow]

It's not really necessary, but I decided to call checkForStartTimeout() with the clusterWorkspace.Config rather than workspace.Config

[amisevsk]

I'm still having some issues with how we can do this cleanly in the code. Currently, we have a split between functions that take DevWorkspaceWithConfig and those that take DevWorkspace alone, and it leads to a really confusing interaction:

• There are two types of functions -- those that take DevWorkspaceWithConfig (DWWC functions) and those that take DevWorkspace alone (DW functions).
• A DW function cannot call a DWWC function
  • In other words -- for any DWWC function, all functions up the call chain to Reconcile must also be DWWC functions
• If a DWWC function calls a DW function, it must unpack the DevWorkspaceWithConfig into just a DevWorkspace

This leads to strange blocks, like

  switch newPhase {
	case dw.DevWorkspaceStatusRunning:
		metrics.WorkspaceRunning(workspace, logger) // Use DWWC here
	case dw.DevWorkspaceStatusFailed:
		metrics.WorkspaceFailed(&workspace.DevWorkspace, logger) // Have to unpack the DWWC here

it also removes the convenience of having a "common" config that can be read where needed -- if a function decides it wants its behavior to depend on configuration, it and all functions that call it recursively have to be updated to take DWWC instead of DW.

I'm thinking about approaches:

1. We can continue down this path, but consider updating our internal APIs to always take DevWorkspaceWithConfig and only use DevWorkspaces when necessary (i.e. in k8s API calls). This would look something like amisevsk@e5465c0 though note I haven't tested it thoroughly.

   • Diff against main (including existing PR changes): main...amisevsk:devworkspace-operator:use-dwwc

   • This makes the API less convenient to use -- there's generally more friction and we're passing around a struct that may just never be used, and it always has to be present.

   • On the other hand, the changes in the PR could be summarized to

     • Update all functions to take DWWC instead of DW (i.e. don't use plain DevWorkspaces anywhere)
     • Update all k8s API calls to unpack DWWC to DW

     rather than the current issue which is some functions are half-updated

2. We can take a step back and pass the config separately where it is needed. This has the same problems as above, but at least makes where the config is necessary explicit

3. We could also completely abandon the idea of an anywhere-accessible config for the DevWorkspace and instead propagate specific values as required, but this is a significant change and I have no idea how it would play out in the repo.

[AObuchow]

This PR has been reworked and moved to #918