Che: Add previewURL support in Devfile format

[l0rd]

Is your enhancement related to a problem? Please describe.

Extend current devfile spec to be allow users to specify a previewURL for a given command

Describe the solution you'd like

 commands:
   - name: run
     previewUrl:
       port: <a port>
       path: <a subpath>
     actions:
       - type: exec
         component: mysql
         command: mvn clean
         workdir: /projects/spring-petclinic

Implementation

• [x] MVP as beta feature. Server side implementation https://github.com/eclipse/che/issues/14893
• [x] Validate devfile with schema with matching apiVersion https://github.com/eclipse/che/issues/14824
• [x] Theia side for previewurl proposed solution https://github.com/eclipse/che/issues/14892
• [x] Theia shows two dialogues when previewUrl is defined in command https://github.com/eclipse/che/issues/14802
  
  
  
  • [x] Theia doesn't update command's previewUrl on 2nd workspace start https://github.com/eclipse/che/issues/14803
  
  
  • [x] Document preferences related to preview URL functionality https://github.com/eclipse/che/issues/14999
  
  
  • [x] Promote to stable https://github.com/eclipse/che/issues/15412
  
  

[skabashnyuk]

@l0rd Does the scope of this issue limited to add previewUrl only in devfile. Or it also means to add some information in workspace runtime. Like

    attributes
        previewUrl:https://host:port/path

[skabashnyuk]

@l0rd any comments?

[gorkem]

How would ports-plugin utilize this information?

[skabashnyuk]

The idea is to add a new attribute(look like an old Che6 commands attribute) in commands in runtime previewUrl

Then task-plugin can read it https://github.com/eclipse/che-theia/blob/master/plugins/task-plugin/src/task/task-protocol.ts#L15

[sparkoo]

@l0rd is command executed always on one of existing servers? Or can it start new one?

[sparkoo]

There's an issue how to find correct server with URL.

There are already exposed URL's in object Runtime>Machines>Server>Url. We can select machine by machineName attribute from command object. As @sleshchenko pointed out, it is not ensured that command will have machineName. Next issue is how to select Server. It should be filterable by port.

Another approach would be to create an extra route for previewUrl. I'm going to investigate this option.

[sparkoo]

@l0rd @skabashnyuk are both port and path mandatory? I could imagine we can use /as default path and 80 as default port. But do we want to do that?

[sleshchenko]

are both port and path mandatory? I could imagine we can use /as default path and 80 as default port. But do we want to do that?

I agree that path may be optional with / as default value.
But port - it's not a port that clients should use to access an application, but port of a container that should be exposed. For java default is 8080, for nodejs like 4040, I think it would be better to require this value to be set.

[skabashnyuk]

There are already exposed URL's in object Runtime>Machines>Server>Url. We can select machine by machineName attribute from command object. As @sleshchenko pointed out, it is not ensured that command will have machineName. Next issue is how to select Server. It should be filterable by port.

That is all internal logic of che server. That should not be our focus.

I think that port is mandatory. It can be already declared as an endpoint in devfile https://github.com/eclipse/che-devfile-registry/blob/master/devfiles/java-web-spring/devfile.yaml#L29 or editor https://github.com/eclipse/che-plugin-registry/blob/d9998222c284d40ae0d2e1661452c1bcd5e4fddb/v3/plugins/ws-skeleton/eclipseide/4.9.0/meta.yaml#L17 or plugin, or it can be a command that can start a new server.

[l0rd]

@sparkoo commands of type exec should be executed in one of the existing containers. We have plans for other type of commands (start, stop etc...) but do not support that yet. With the introduction of ephemeral containers we should definitely consider supporting those commands as well. For port and path I am +1 with @sleshchenko and @skabashnyuk comment.

[metlos]

Since we already have something that almost does the job - dockerimage endpoint (or rather the server in the old workspace config model), I'm struggling to see the real benefit of having a previewUrl placed on a command.

The intention seems to be that when the user invokes the command, Theia will know what URL to open to show the web ui of the application the command has supposedly started. Or at least that's how I understood it.

But this completely ignores the usecase of invoking the application using the terminal or some other means (like would we be smart enough if previewUrl was placed on a command specifying launch configuration? how would that even work if the referenced launch.json contained more configs?).

IMHO, we'd be much better served with adding endpoint support to other component types and adding the path (and maybe even protocol) to the endpoint configuration. In such arrangement, we'd support all invocation modes.

If we wanted to add the support for automatically opening URL for commands, I think we could do it using a command attribute referencing the endpoint, e.g.:

components:
- name: nodejs
  type: dockerimage
  ...
  endpoints:
  - name: web-app
    port: 4040
    path: /app
commands:
- name: start
  actions:
  - type: exec
    component: nodejs
    previewEndpoint: web-app

WDYT @skabashnyuk @sleshchenko @l0rd ?

(I updated the example to use the previewEndpoint wording @sleshchenko suggested. I like it better than my original opensEndpoint).

[sleshchenko]

I see it logical to stick previewURL to an endpoint.
Otherwise, I expect that the port described in previewURL is exposed even if there is no corresponding port in component. If there is no such - for k8s it's not clear which container should be exposed, then every?
Currently, Theia already displays endpoints in containers view and without path they are useless in some cases.

What if previewEndpoint instead of opensEndpoint? It's up to client to open endpoint URL or show somewhere on UI, like command output panel.

[l0rd]

@metlos makes sense. But there is something that bothers me. A preview URL is basically an endpoint + a path. We currently already have a preview URL that pops up when the a command is run and a service is started in a container (thanks to the port plugin) but this preview URL doesn't contain the path. There is no way to specify the path specific to that given command.

In other words I am not sure that we should add a path to the endpoint but we may probably want to specify a preview URL as an endpoint + a path:

 commands:
   - name: run
     previewUrl:
       endpoint: <a endpoint>
       path: <a subpath>
     actions:
       - type: exec
         component: mysql
         command: mvn clean
         workdir: /projects/spring-petclinic

[sparkoo]

@l0rd that would make things much easier as all exposed ports will be defined by endpoints and previewUrl will always have endpoint to match. So we won't get to the situation when we have previewUrl port that is not exposed and we would have to create new service/ingress/route for it.

+1 from me. @metlos @sleshchenko ?

[metlos]

In other words I am not sure that we should add a path to the endpoint but we may probably want to specify a preview URL as an endpoint + a path:

Well, my understanding of the endpoint is just "there is something running in this component on this port". We don't specify anything about what that something might be - it might be DB server communicating over binary protocol, it might be a webserver, it might be exposed, it might not.

We already document that endpoints do have a path attribute: https://github.com/eclipse/che-docs/blob/master/src/main/pages/che-7/end-user-guide/ref_devfile-reference.adoc#endpoints and the Che server translates the endpoint path attribute into the corresponding server declaration.

IMHO, the endpoints (as can be reviewed in the above doc link) offer a much richer set of capabilities that already cover all of what we want to achieve with previewUrl (like for example the integration with JWT proxy).

None of the above would solve the issue with the port plugin not offering the path, but IMHO the previewUrl on a command wouldn't either. The port plugin works even if you start your server from the terminal as far as I remember, which previewUrl would not be able to.

So IMHO, it would be best to just have previewEndpoint or just endpoint on the action (not the command as a whole, which would get us into trouble once we actually support multiple actions per command).

[sleshchenko]

Well, my understanding of the endpoint is just "there is something running in this component on this port".

+1
We already have My Runtime view that allows us to use endpoints without previewURLs.
We even have Django devfile that declares endpoint and if you open it - you get 404
Seems it's a case where endpoint path is useful since {endpoint}/admin shows some content but

So, I think it makes sense to add path for the endpoint.
And if we have path for endpoint - do we really need path for previewUrl? Should it override endpoints path or should be added there?

Endpoint name is unique in the scope of its component but not whole devfile.
From this point of view +1 to add previewEndpoint on action level but not command.

[l0rd]

I understand @metlos approach but endpoint is not a Che concept, it's a Kubernetes one. If we can define a path in a Kubernetes endpoint I am ok with that.

[sparkoo]

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.16/#serviceport-v1-core

[sleshchenko]

I understand @metlos approach but endpoint is not a Che concept, it's a Kubernetes one. If we can define a path in a Kubernetes endpoint I am ok with that.

Thanks for sharing it. Previously I never thought about devfile/plugins endpoints as about k8s endpoints.
Now I'm trying to understand how our devfile/plugins endpoints suites to k8s endpoint.
@l0rd You mean https://theithollow.com/2019/02/04/kubernetes-endpoints/ these, service related endpoints, right?

[sparkoo]

@l0rd we already have attributes in endpoint https://github.com/eclipse/che-devfile-registry/blob/master/devfiles/nodejs-mongo/devfile.yaml#L50

[sparkoo]

if I set this endpoint

endpoints:
      - name: 'nodejs'
        port: 3000
        attributes:
          path: '/hello'

and run the command with action's component: nodejs or click on endpoint in the right panel, the opened url indeed have the path correctly appended. I didn't know about this at all. I'm now wondering whether there is anything left to fix the purpose of the original issue AND whether all this effort https://github.com/eclipse/che/issues/13964 made sense.
@l0rd @sleshchenko @metlos

[sparkoo]

here's simple devfile you can try, that runs go http server on port 8081 and respond on /hello path

apiVersion: 1.0.0
metadata:
  generateName: dummy-go-server
projects:
  -
    name: dummy-http-server
    source:
      type: git
      location: "https://github.com/sparkoo/dummy-http-server.git"
components:
  -
    type: dockerimage
    image: quay.io/eclipse/che-golang-1.10:nightly
    alias: go-cli
    mountSources: true
    memoryLimit: 256Mi
    endpoints:
      - name: 'port1'
        port: 8081
        attributes:
          path: /hello
commands:
  -
    name: run on 8081 and /hello path
    actions:
      -
        type: exec
        component: go-cli
        command: "go run main.go 8081 /hello"
        workdir: ${CHE_PROJECTS_ROOT}/dummy-http-server

[l0rd]

@sleshchenko yes, when we had to decide the naming we had to find something that could be a service (not public) or a service+ingress (public) and we decided that in the kubernetes world that would be called endpoint.

@sparkoo the big problems I see are:

• it doesn't make sense to extend the devfile spec with endpoints for kubernetes and openshift components: these abstractions already exist and are services + ingress/routes
• for one endpoint there can be multiple paths: one endpoint (hostname + port) can serve multiple commands, each one with a different path

[sparkoo]

* it doesn't make sense to extend the devfile spec with endpoints for kubernetes and openshift components: these abstractions already exist and are services + ingress/routes

then I don't understand how this should work with kubernetes/openshift components, when they can't have defined endpoint in devfile?

     previewUrl:
       endpoint: <a endpoint>
       path: <a subpath>

[l0rd]

Good point @sparkoo. We could use the service selector to identify the endpoint or the name if there is no selector. But it's not a simple and straightforward solution. I don't know what's better. Feel free to decide whatever makes more sense to you. I am ok with both choices.

[sparkoo]

I guess we have to get route/ingress for previewUrl. Not the service.

Having following devfile

devfile

---
apiVersion: 1.0.0
metadata:
  generateName: dummy-go-server
projects:
  -
    name: dummy-http-server
    source:
      type: git
      location: "https://github.com/sparkoo/dummy-http-server.git"
components:
  -
    type: openshift
    alias: go-os
    mountSources: true
    referenceContent: |
          kind: List
          items:
          - kind: Pod
            apiVersion: v1
            metadata:
              name: go-runner
            spec:
              containers:
                - name: server
                  image: quay.io/eclipse/che-golang-1.10:nightly
                  ports:
                  - containerPort: 8082
                    protocol: TCP
                  resources:
                    limits:
                      memory: 256Mi
          - kind: Service
            apiVersion: v1
            metadata:
              name: go-dummy-service
            spec:
              ports:
                - name: web
                  port: 8082
                  targetPort: 8082
          - kind: Route
            apiVersion: v1
            metadata:
              name: go-dummy-route
            spec:
              to:
                kind: Service
                name: go-dummy-service
              port:
                targetPort: web
commands:
  -
    name: run on 8082 and /hello path on openshift
    actions:
      -
        type: exec
        component: go-os
        command: "go run main.go 8082 /hello"
        workdir: ${CHE_PROJECTS_ROOT}/dummy-http-server

The created Route looks like this:

route

[~/dev] λ oc get route routee5cr82j5 -o yaml
apiVersion: route.openshift.io/v1
kind: Route
metadata:
  annotations:
    openshift.io/host.generated: "true"
  creationTimestamp: 2019-10-01T23:18:55Z
  labels:
    che.original_name: go-dummy-route
    che.workspace_id: workspacenlpisd8qfg8tag9i
  name: routee5cr82j5
  namespace: eclipse-che
  resourceVersion: "75445"
  selfLink: /apis/route.openshift.io/v1/namespaces/eclipse-che/routes/routee5cr82j5
  uid: d6cb851f-e4a1-11e9-8470-52540035cac7
spec:
  host: routee5cr82j5-eclipse-che.192.168.42.10.nip.io
  port:
    targetPort: web
  to:
    kind: Service
    name: go-dummy-service
    weight: 100
  wildcardPolicy: None
status:
  ingress:
  - conditions:
    - lastTransitionTime: 2019-10-01T23:18:55Z
      status: "True"
      type: Admitted
    host: routee5cr82j5-eclipse-che.192.168.42.10.nip.io
    routerName: router
    wildcardPolicy: None

So if we have previewUrl like this:

commands:
   - name: run
     actions:
       - type: exec
         component: go-os
         command: "go run main.go 8082 /hello"
         workdir: ${CHE_PROJECTS_ROOT}/dummy-http-server
         previewUrl:
           endpoint: go-dummy-route
           path: /hello

we should be able to get correct route by it's labels

  labels:
    che.original_name: go-dummy-route
    che.workspace_id: workspacenlpisd8qfg8tag9i

[sparkoo]

After yesterday's meeting, we're basically back at the beginning.

Devfile

commands:
   - name: run
     previewUrl:
       port: <port> # mandatory
       path: <path> # optional, defaults to ‘/’
     actions:
       - type: exec
         component: go-os
         command: "go run main.go 8082 /hello"
         workdir: ${CHE_PROJECTS_ROOT}/dummy-http-server

Logic

At workspace start-up time:

• Get previewUrl.port and scan through existing services to find matching port
  
  
  
  • If service found, find ingress/route to that service
    
    
    
    
    
    • If ingress/route found, use it’s host + previewUrl.path to compose preview url
    
    
    
    • If not found, create ingress/route and compose preview url
    
    
    
  
  
  • If service not found, create service + ingress/route and compose preview url
  
  

PreviewUrl is set to runtime>command>attribute[“previewUrl”]

next

As this is mostly approach I was implementing, I will continue working on draft PR https://github.com/eclipse/che/pull/14713

thoughts? @l0rd @skabashnyuk @metlos @sleshchenko

[l0rd]

@sparkoo 👍 we will need to think about the integration with the port plugin. But as discussed yesterded this should be considered as a second step.

[sparkoo]

I've created 2 theia issues that affects overall preview url functionality and UX.

14802 and #14803

[sparkoo]

We've discovered new issues while implementing devfile PreviewUrl support. Important note is, that we've already decided to introduce preview url as a beta feature.

Description

Theia receive commands from server in runtime object. Task-plugin can read PreviewUrl as and additional attribute of the command with key previewUrl. Attribute is simple string. In case of previewUrl, task-plugin reads the value and opens browser window on that address.

So the command object could look like this:

"commands": [
  {
    "attributes": {
      "componentAlias": "go-k8s",
      "workingDir": "${CHE_PROJECTS_ROOT}/dummy-http-server",
      "previewUrl": "https://some-url-here"
    },
    "commandLine": "go run main.go 8080 /hello",
    "name": "run on 8080 and /hello",
    "type": "exec"
  }
]
 ```

We've tried to go with these 2 ways.

### Issues

 - Constructing full urls on the server and sending them in `previewUrl` command's attribute to the editor. Theia stores tasks configurations in `tasks.json` file. There is an issue when workspace is started 2nd (and following) time, it will have new urls, because new ingresses/routes are created at each workspace start. Theia's `tasks.json` is not updated with these new urls and so it will not work anymore in this workspace. Another issue with this approach is that `tasks.json` should not store dynamic runtime values. It is configuration file and it should not change with each workspace startup. This approach is ok for server side, but problematic for Theia.

 - Second approach is using variables that Theia understands and can evaluate. There are variables for server urls like ${server.<servername>}. This works ok for dockerimage components. However, issue here is that when we have k8s component with more containers in one pod. Then we don't know where command will run. In this case, user is asked to choose the container where he want to run it. So on the server, we can't decide what variable to use. This approach is ok for Theia, but is problematic for server.
 <details>
 <summary>example k8s devfile with multiple containers</summary>

 ```
 ---
apiVersion: 1.0.0
metadata:
  name: dummy-go-k8s
projects:
  -
    name: dummy-http-server
    source:
      type: git
      location: "https://github.com/sparkoo/dummy-http-server.git"
components:
  -
    type: kubernetes
    alias: go-k8s
    mountSources: true
    referenceContent: |
          kind: List
          items:
          - kind: Pod
            apiVersion: v1
            metadata:
              name: go-runner
            spec:
              containers:
                - name: go-110-server
                  image: quay.io/eclipse/che-golang-1.10:nightly
                - name: go-112-server
                  image: quay.io/eclipse/che-golang-1.12:nightly
commands:
  -
    name: run on 8080 and /hello
    previewUrl:
      port: 8080
      path: /hello
    actions:
      -
        type: exec
        component: go-k8s
        command: "go run main.go 8080 /hello"
        workdir: ${CHE_PROJECTS_ROOT}/dummy-http-server

 ```
 </details>


### Options

We have 2 options where to go now

  - Choose 1st or 2nd way and have known limitations. Preview url will be devfile's beta feature, we may address this later.

  - Find the solution

### Proposed solution

Possible solution to this will be to construct full preview urls on the server. This is possible, because we know port where command runs, and we can find/create matching service/ingress(route). Then send the urls to Theia in a map that might look like this:

{
"previewUrl1": "http://server123-previewurl8080.nip.io/hello",
"previewUrl2": "http://serverabc-previewurl8888.nip.io/api"
}

And in command object use key of previewurl as the variable:

"commands": [
{
"attributes": {
"componentAlias": "go-k8s",
"previewUrl": "${previewUrl1}",
"workingDir": "${CHE_PROJECTS_ROOT}/dummy-http-server"
},
"commandLine": "go run main.go 8080 /hello",
"name": "run on 8080 and /hello",
"type": "exec"
}
]
```
Theia then will create it's variables from the received map and will be able to evaluate the variables in the commands when needed.

There is already links object in the Workspace object that we're sending to Theia (https://github.com/eclipse/che/blob/master/wsmaster/che-core-api-workspace-shared/src/main/java/org/eclipse/che/api/workspace/shared/dto/WorkspaceDto.java#L65). We could include preview urls there with some special key prefix like previewurl.. Theia will read these keys and store them as variables.

current links object

{
  "links": {
    "environment/outputChannel": "ws://che-che.192.168.39.57.nip.io/api/websocket",
    "environment/statusChannel": "ws://che-che.192.168.39.57.nip.io/api/websocket",
    "ide": "http://che-che.192.168.39.57.nip.io/che/dummy-go-k8s",
    "self": "http://che-che.192.168.39.57.nip.io/api/workspace/workspace88z4ubiciehol3c2"
  }
}

I believe this solution will work and should not be much additional work (at least on server side, I don't know about Theia). It will for sure postpone Preview Url to 7.4.0, but that is probably going to happen anyway, unfortunately.

Questions

• Does the proposed solution make sense?
• Can we see any pitfall in the proposal?
• Can we use links object for preview urls? Or should we introduce a new object for preview urls?
• Any other ideas how to fix the issues?

cc: @l0rd @RomanNikitenko @skabashnyuk @sleshchenko @metlos

[sparkoo]

there is Theia issue that affects the previewUrl feature https://github.com/eclipse/che/issues/14876

[skabashnyuk]

As for the beta feature, I think @sparkos' s solution https://github.com/eclipse/che/issues/13945#issuecomment-541233853 - is ok-ish.
I think one of the trickiest questions before we promote this feature to stable would be - the algorithm of how we proceed if user editing both devfile and task.json at the same time in the same place. And this issue https://github.com/eclipse/che/issues/14876 is one of the sign
of the potential problem of having two sources of commands devfile and task.json.

A couple of question to @sparkos' s solution

• How this id ${previewUrl1} would be calculated. Does it uniquely identify command? CRC?
• @RomanNikitenko what do you do if you didn't find this id after workspace restart. The reason: the user just deletes it in devfile.

[sparkoo]

* How this id ${previewUrl1} would be calculated.  Does it uniquely identify command? CRC?

When #14876 is fixed, we don't have to do any sophisticated calculations. At workspace startup, we basically find URL for command (either it exists by defined endpoint or we create service+route), put it into the map under some key and put this key to command's attribute. Theia then receives this map and command's attributes and must properly update tasks.json with the key received from server. For consistency and human readability I would use something like name.replace(" ", "_")+random_noise() as a key (random noise to fix conflicting names, if someone has run command and run_command in the same devfile).

[sparkoo]

I've just realized that having random noise in key would mean that this key will change at each workspace startup, which is not ok for Theia. We should change tasks.json config only when we really have to. So yes, having some command.name + "-" + CRC(command.name), to have same key for the same name everytime should be enough.

[sparkoo]

we even don't need the tasks.json <> devfile sync to be fixed. Preview url key should be the same for the command name, so we're good here.

[skabashnyuk]

we even don't need the tasks.json <> devfile sync to be fixed. Preview url key should be the same for the command name, so we're good here.

Sounds ok.

[sparkoo]

to sum up the proposed solution:

server will create workspace object like this:

...
    "links": {
      "self": "http://che-eclipse-che.192.168.42.10.nip.io/api/workspace/workspace7grc58gn0jozwkmz",
      "ide": "http://che-eclipse-che.192.168.42.10.nip.io/admin/nodejs-mongo-ymkmt",
      "environment/statusChannel": "ws://che-eclipse-che.192.168.42.10.nip.io/api/websocket",
      "environment/outputChannel": "ws://che-eclipse-che.192.168.42.10.nip.io/api/websocket",
      "previewurl/runthewebapp_123": "http://ingress111aaa.192.168.42.10.nip.io/hello",
      "previewurl/createtestuser_456": "http://ingress222bbb.192.168.42.10.nip.io/some/path"
    },
...
      "commands": [
        {
          "commandLine": "npm install && npm run dev",
          "name": "run the web app",
          "attributes": {
            "componentAlias": "nodejs",
            "machineName": "nodejs",
            "workingDir": "${CHE_PROJECTS_ROOT}/nodejs-mongodb-app"
          },
          "type": "exec",
          "previewUrl": "${previewurl/runthewebapp_123}"
        },
        {
          "commandLine": "curl -X POST --data '{\"user\": {\"username\": \"test\", \"email\": \"[email protected]\", \"password\": \"password\"}}' -H \"Content-Type: application/json\" http://localhost:3000/api/users\n",
          "name": "create test user",
          "attributes": {
            "componentAlias": "nodejs",
            "machineName": "nodejs"
          },
          "type": "exec",
          "previewUrl": "${previewurl/createtestuser_456}"
        }
      ],
...

Theia's responsibility is then to resolve variables in previewUrl attributes, with values from links object.

Key previewurl/run_the_web_app_123 is constructed in this format previewurl/<command_name>_<crc>, where previewurl is hard prefix, <command_name> is command's name without spaces and <crc> is some simple crc hash function applied on commands name. Theia do care only about prefix to know that this is previewurl link and it should create a variable from it. The rest is simple string without any extra meaning for Theia.

[sparkoo]

cc: @l0rd @RomanNikitenko @skabashnyuk

[skabashnyuk]

https://github.com/eclipse/che/issues/13945#issuecomment-542082017

ok for me

[RomanNikitenko]

@sparkoo

After restarting workspace key previewurl/createtestuser_456 will be the same previewurl/createtestuser_456? I mean crc hash _456 is not changed, right?

[sparkoo]

@sparkoo

After restarting workspace key previewurl/createtestuser_456 will be the same previewurl/createtestuser_456? I mean crc hash _456 is not changed, right?

yes, it will be calculated from command name, so as long as the name won't change, crc will be the same.

[sparkoo]

I've created an issue for Theia support of previewurl by proposed solution #14892

cc: @RomanNikitenko @l0rd

[l0rd]

@sparkoo that's fine for me

[slemeur]

When are you planning to merge this work to stable?

[sparkoo]

@slemeur we can plan it for next sprint, or even include it in current sprint. It should be straightforward patch.

[sparkoo]

@slemeur I've created an issue for that https://github.com/eclipse/che/issues/15412

[sparkoo]

all issues in the scope of this epic are done. closing.