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 per-user storage-class option #893

Merged
merged 3 commits into from
Aug 8, 2022

Conversation

AObuchow
Copy link
Collaborator

The 'per-user' storage type is an alternate storage-type name for the 'common' storage class.
In other words, it is an alias for the 'common' storage-type and behaves in the same manner.

Note that in the PR's current state, the 'per-user' storage-type replaces the 'common' storage-type, and the 'common' storage-type is treated as an alias of the 'per-user' storage-type. It may be better to reverse things, i.e. document that the 'per-user' storage-type is an alias of the 'common' storage-type

What issues does this PR fix or reference?

Part of eclipse-che/che#21405

Is it tested? How?

There are two parts of this PR to test.
Ensuring that the 'per-user' storage type works the same as the 'common' storage type:

  1. Start up DWO
  2. Create and apply a devfile that uses the 'per-user' storage type, e.g.
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: theia-next
spec:
  started: true
  template:
    attributes:
        controller.devfile.io/storage-type: per-user
    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
  1. Ensure that the workspace starts up and that the claim-devworkspace PVC is created.
  2. Delete the workspace and ensure that the claim-devworkspace PVC is deleted.

Ensuring that you can switch between the 'per-user' and 'common' storage-type:

  1. Start up DWO
  2. Create and apply 2 devfiles, one with the 'per-user' storage-type and the other with the 'common' storage-type
  3. After the workspaces are started up, do a kubectl edit or oc edit and change their storage-type from per-user -> common and common -> per-user respectively
  4. Ensure the edit is allowed by the webhook

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

Copy link
Contributor

@ibuziuk ibuziuk left a comment

Choose a reason for hiding this comment

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

LGTM, but could you please add some tests

@openshift-ci
Copy link

openshift-ci bot commented Jul 21, 2022

[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:

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

@openshift-ci openshift-ci bot removed the lgtm label Aug 5, 2022
@openshift-ci
Copy link

openshift-ci bot commented Aug 5, 2022

New changes are detected. LGTM label has been removed.

@AObuchow
Copy link
Collaborator Author

AObuchow commented Aug 5, 2022

LGTM, but could you please add some tests

I added a test to ensure that the CommonStorageProvisioner is used given a devfile that uses the per-user storage class. I haven't been able to think of a better way to test this feature so far.

The 'per-user' storage-type is an alternate storage-type name for the 'common' storage-type.
In other words, it is an alias for the 'common' storage-type and behaves in the same manner
as the 'common' storage-type.

Part of eclipse-che/che#21405

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
@AObuchow AObuchow force-pushed the per_user_pvc_strategy_alias branch from 6035284 to a437d4e Compare August 8, 2022 14:34
Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
@AObuchow AObuchow force-pushed the per_user_pvc_strategy_alias branch from a437d4e to b3e96ac Compare August 8, 2022 14:34
@AObuchow AObuchow merged commit d20bf94 into devfile:main Aug 8, 2022
@AObuchow AObuchow deleted the per_user_pvc_strategy_alias branch August 9, 2022 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants