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

[UD] Support factory create policies for devWorkspaces #19711

Closed
sleshchenko opened this issue Apr 29, 2021 · 4 comments · Fixed by eclipse-che/che-dashboard#297
Closed

[UD] Support factory create policies for devWorkspaces #19711

sleshchenko opened this issue Apr 29, 2021 · 4 comments · Fixed by eclipse-che/che-dashboard#297
Assignees
Labels
area/dashboard kind/task Internal things, technical debt, and to-do tasks to be performed. severity/P1 Has a major impact to usage or development of the system. sprint/current
Milestone

Comments

@sleshchenko
Copy link
Member

Is your task related to a problem? Please describe.

Che Workspaces supports two create policies: perClick, perUser.

But DevWorkspaces does not support them which cause to:

  • by default factory accepting fails if devworkspace with the devfile's name already exists (name should be generated since perClick is the default policy);
  • OpenShift DevConsole will work incorrectly if repository contains devfile v2, since it uses perUser policy;

To store used policy DevWorkspace CR labels/annotations should be used.

@sleshchenko sleshchenko added kind/task Internal things, technical debt, and to-do tasks to be performed. sprint/next severity/P1 Has a major impact to usage or development of the system. area/dashboard labels Apr 29, 2021
@sleshchenko sleshchenko added this to the 7.33 milestone Jun 15, 2021
@olexii4 olexii4 self-assigned this Jul 1, 2021
@sleshchenko
Copy link
Member Author

label value format is pretty limited, so annotation is our way.

Currently, we have

apiVersion: workspace.devfile.io/v1alpha2
kind: DevWorkspace
metadata:
  annotations:
    che.eclipse.org/devfile-source: |
      scm:
        repo: 'https://github.com/che-samples/java-spring-petclinic.git'
        revision: devfilev2
        fileName: devfile.yaml
apiVersion: workspace.devfile.io/v1alpha2
kind: DevWorkspace
metadata:
  annotations:
    che.eclipse.org/devfile-source: |
      url:
        location: >-
          https://raw.githubusercontent.com/che-samples/java-spring-petclinic/devfilev2/devfile.yaml

But it does not seem to be appropriate to store policyCreate, so instead of repurposing existing annotation I propose to use

apiVersion: workspace.devfile.io/v1alpha2
kind: DevWorkspace
metadata:
  annotations:
    che.eclipse.org/factory-parameters: url=https://raw.githubusercontent.com/che-samples/java-spring-petclinic/devfilev2/devfile.yaml&policies.create=peruser

^ So, dashboard is able to find a proper workspace for peruser, even without requesting Che Server to analyze devfile source.

@benoitf
Copy link
Contributor

benoitf commented Jul 2, 2021

I would prefer if we have structured content and smaller names
also why you would need the URL again?

something like

metadata:
  annotations:
    che.eclipse.org/factory: |
      createPolicy: peruser

@sleshchenko
Copy link
Member Author

I'm OK with factory as well.

also why you would need the URL again?

well, devfile source does not have an original URL but like in scm case has analyzed and probably modified content, like 'https://github.com/che-samples/java-spring-petclinic become https://github.com/che-samples/java-spring-petclinic.git
We also are able to include factory field into devfile-origin if it makes sense, like:

apiVersion: workspace.devfile.io/v1alpha2
kind: DevWorkspace
metadata:
  annotations:
    che.eclipse.org/devfile-source: |
      factory:
        params: url=https://raw.githubusercontent.com/che-samples/java-spring-petclinic/devfilev2/devfile.yaml&policies.create=peruser&devfile.override=metadata.name=your-first-dw
      scm:
        repo: 'https://github.com/che-samples/java-spring-petclinic.git'
        revision: devfilev2
        fileName: devfile.yaml

^ devfile.override is just here for an example, we haven't agreed yet if we need it for devworkspace.

@benoitf
Copy link
Contributor

benoitf commented Jul 2, 2021

I'm not sure I understand the use case where we need to override url from an url factory
I mean, you'll give URL after # and then you'll say that you want to use another URL so why not directly give the right URL ?

maybe we would need to rename devfile-source to factory then

Also why do we need factory params in the DevWorkspace ?

AFAIK we would only need in the devfile (like user want to define a perUser policy)
and in the other case, only dashboard will read that information so I'm not sure it's interesting to propagate that information ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dashboard kind/task Internal things, technical debt, and to-do tasks to be performed. severity/P1 Has a major impact to usage or development of the system. sprint/current
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants