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

[Devfile] Support commands for Os/K8s tool #12479

Closed
skabashnyuk opened this issue Jan 21, 2019 · 8 comments
Closed

[Devfile] Support commands for Os/K8s tool #12479

skabashnyuk opened this issue Jan 21, 2019 · 8 comments
Assignees
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed. status/analyzing An issue has been proposed and it is currently being analyzed for effort and implementation approach

Comments

@skabashnyuk
Copy link
Contributor

Description

Now we support commands only for the ChePlugin or CheEditor tool.
We want to introduce an ability to define commands for OS/K8s tools
Like

specVersion: 0.0.1
name: che-in-che
projects:
  - name: che
    source:
      type: git
      location: 'https://github.com/eclipse/che.git'
tools:
  - name: theia-editor
    type: cheEditor
    id: org.eclipse.che.editor.theia:1.0.0
  - name: exec-plugin
    type: chePlugin
    id: che-machine-exec-plugin:0.0.1
  - name: che-dev
    type: openshift
    local: .che-dev.yaml
commands:
  - name: build
    actions:
      - type: exec
        tool: che-dev
        command: mvn clean install
        workdir: /projects/che
@skabashnyuk skabashnyuk added kind/task Internal things, technical debt, and to-do tasks to be performed. team/platform labels Jan 21, 2019
@skabashnyuk
Copy link
Contributor Author

One of the issues that exist is: what to do if OS/K8S recipe contains more then one container.
Should we specify some additional information in command definition?
@l0rd can you comment?

@l0rd
Copy link
Contributor

l0rd commented Jan 21, 2019

Option 1 Use an action#tool-selector that, associated with the action#tool-name, would select the right container.

Option 2 Use one of the filter that will be put in place by #12386

Option 3 Prompt the user from the editor when running the command to select one of the containers that match the filters/selectors

These options are not mutually exclusive. Ideally we should put in place all of them.

@sleshchenko
Copy link
Member

Option 1 Use an action#tool-selector that, associated with the action#tool-name, would select the right container.

@l0rd Could you clarify what you mean action#tool-selector? It should be a new field in action to specify selector for picking up the right container? How this selector is supposed to work since Kubernetes Pod has labels but not a container.

Option 2 Use one of the filter that will be put in place by #12386

Could you clarify what does it mean for Devfile format? It is introducing new field in action to specify whether this action is supposed to be run in recipe/tool sourced container?

@sleshchenko sleshchenko added the status/analyzing An issue has been proposed and it is currently being analyzed for effort and implementation approach label Jan 29, 2019
@sleshchenko sleshchenko self-assigned this Jan 29, 2019
@l0rd
Copy link
Contributor

l0rd commented Jan 30, 2019

@sleshchenko for action#tool-selector I was thinking about something like

    actions:
      - type: exec
        tool: 
           name: che
           selector:
               name: postgres
        command: psql

For Option 2 nothing in the Devfile format. I hoped that #12386 would introduce filters based on container labels like terminal-enabled: true. In that case we could have filter-out containers that had terminal-enabled: false. But #12386 uses a different filter mechanism that is not useful here. So let's not consider Option 2 for now.

@sleshchenko
Copy link
Member

@l0rd Thanks for clarifications.

    actions:
      - type: exec
        tool: 
           name: che
           selector:
               name: postgres
        command: psql

I expect from selector to be matched with objects labels. But container does not have own labels. If we would match name selector field with container name than it would not be so clear format since Pods in tool also have names.
Also there is only restriction from Kubernetes to be containers names unique whithing one Pod. And tool may contain multiple containers with the same names. Like Pod{name: db, containers{name: main}}, Pod{name: web-app, containers{name: main}}.

I have in mind the following way how to provide not bad UX for user with minimal afford.
As an example user uses the following Devfile:

specVersion: 0.0.1
name: che-in-che
projects:
   ...
tools:
  - name: theia-editor
    type: cheEditor
    id: org.eclipse.che.editor.theia:1.0.0
  - name: che-dev
    type: openshift
    local: che-dev.yaml
commands:
  - name: build
    actions:
      - type: exec
        tool: che-dev
        command: mvn clean install
        workdir: /projects/che
  - name: hello-theia
    actions:
      - type: exec
        tool: theia-editor
        command: echo Hello from Theia IDE
  1. The user run hello-theia command and it is automatically executed in first theia-plugin container (it contains the only one container). Such functionality is already implemented.

As for build command there can be two cases:
The tool contains the only one container:
2. The user run build command. Machine name is preconfigured and command is executed there without user confirmation.
Implementation notes: Che Server should be updated to set machine name if OpenShift, Kubernetes tool contains one container.

The tool contains the several containers:
2. The user run build command. Machine name is not preconfigured. Theia asks user to choose machine from recipe.

Implementation notes: #12386 should be merged. And Theia Tasks plugin should be reworked to propose user to choose machines from recipe sourced containers, choose plugin containers is possible as additional option. It may looks like the following:
screenshot_20190130_125624_____
After clicking on Show other contains containers list should be updated
screenshot_20190130_125624__
Maybe tool containers should be marked like the following
screenshot_20190130_125624__123
The label may be different like editor, plugin, etc.

Later Devfile format may be improved to provide a user an ability to pre-configure container from Kubernetes/OpenShift tool with multiple containers. But I think we should consider implementing the described workflow as initial and simpler one.
@skabashnyuk @l0rd WDYT?

@skabashnyuk
Copy link
Contributor Author

I would say

  1. If a tool has a single pod with one container - run inside of it.
  2. If a tool has multiple pods with one container inside of each - use selectors to match pod with a single container.
  3. all other cases - let Theia to ask concrete container + create an issue if necessary to improve this situation.

@sleshchenko
Copy link
Member

  1. If a tool has a single pod with one container - run inside of it.

It's what I mentioned - We can easily implement it now - let's do it.

  1. If a tool has multiple pods with one container inside of each - use selectors to match pod with a single container.
  2. all other cases - let Theia to ask concrete container + create an issue if necessary to improve this situation.

We can create another issue for Implement an ability to specify container for Kubernetes/OpenShift tool command where we can discuss and implement the following Devfile format changes

specVersion: 0.0.1
name: che-in-che
projects:
   ...
tools:
  - name: theia-editor
    type: cheEditor
    id: org.eclipse.che.editor.theia:1.0.0
  - name: che-dev
    type: openshift
    local: che-dev.yaml
commands:
  - name: build
    actions:
      - type: exec
        tool: 
          name: che-dev
          containerName: mysql
          podName: db
          selector:
            app.kubernetes.io/component: database
        command: mvn clean install
        workdir: /projects/che

Where new fields selector, containerName, podName can pick up needed containers/container.

@sleshchenko
Copy link
Member

A case when K8s/OS tool has only one container is already handled by the following PR #12589.
For handling a case when K8s/OS tool has several containers there new issue is registered #12606

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed. status/analyzing An issue has been proposed and it is currently being analyzed for effort and implementation approach
Projects
None yet
Development

No branches or pull requests

3 participants