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 an ability to specify entrypoint for dockerimage tool in Devfile #12668

Closed
sleshchenko opened this issue Feb 14, 2019 · 9 comments
Closed
Labels
kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system. status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it.

Comments

@sleshchenko
Copy link
Member

Description

There are cases when a user wants to use some official docker image in dockerimage tool but default entrypoint is not suitable for his needs. In this case, it is needed to have an ability to specify entrypoint and arguments for dockerimage tool in Devfile.
The proposal is to add two optional fields command and arguments which type is string array. It's the same as Kubernetes Container model has https://docs.openshift.com/container-platform/3.5/rest_api/kubernetes_v1.html#v1-container.
It would look like the following

  - name: nodejs
    type: dockerimage
    image: node:0.10.40
    command: ['/bin/sh', '-c']
    args: ['tail -f /dev/null']
@sleshchenko sleshchenko added kind/enhancement A feature request - must adhere to the feature request template. status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it. team/platform labels Feb 14, 2019
@sleshchenko
Copy link
Member Author

@l0rd Do you think the proposed format is good, or it would be better to have something like current devfile draft has entryPoint: sleep infinity https://github.com/redhat-developer/devfile/blob/master/Devfile.yaml#L13 ?

@l0rd
Copy link
Contributor

l0rd commented Feb 14, 2019

@sleshchenko command and args is perfect.

@sleshchenko
Copy link
Member Author

Here is a technical issue in implementing it. We convert dockerimage tool to an environment with dockerimage recipe. For example:

  - name: che-dev
    type: dockerimage
    image: eclipse/che-dev:nightly
    env:
      - name: CHE_LOGS_DIR
        value: /home/user/che-logs
    endpoints:
      - name: tomcat
        port: 8080
        attributes:
          public: 'true'
    volumes:
      - name: maven
        containerPath: /home/user/.m2
    memoryLimit: 1.5G
    mountSources: true

Will be converted to

{
    "che-dev": {
        "machines": {
            "che-dev": {
                "attributes": {
                    "memoryLimitBytes": "1500000000"
                },
                "servers": {
                    "tomcat": {
                        "attributes": {
                            "public": "true"
                        },
                        "port": "8080",
                        "protocol": "http"
                    }
                },
                "volumes": {
                    "projects": {
                        "path": "/projects"
                    },
                    "maven": {
                        "path": "/home/user/.m2"
                    }
                },
                "installers": [],
                "env": {
                    "CHE_LOGS_DIR": "/home/user/che-logs"
                }
            }
        },
        "recipe": {
            "type": "dockerimage",
            "content": "eclipse/che-dev:nightly"
        }
    }
}

And the issue that currently there is no an ability to specify entrypoint for dockerimage machine.
Possible solution would be add an ability to define entrypoint via machine configuration attributes, then we would be able to specify something like that:

{
    "che-dev": {
        "machines": {
            "che-dev": {
                "attributes": {
                    "memoryLimitBytes": "1500000000",
                    "command": "['/bin/sh', '-c']",
                    "args": "['tail -f /dev/null']"
                },
                ...
            }
        },
        "recipe": {
            "type": "dockerimage",
            "content": "eclipse/che-dev:nightly"
        }
    }
}

Attributes names can be more self-explaining, like containerCommand, containerArgs.
@garagatyi @skabashnyuk WDYT? Which attributes names do you like more?

@garagatyi
Copy link

👍 to use attributes until we remove workspace config and start using devfile directly.
I'm OK with the proposed names

@l0rd
Copy link
Contributor

l0rd commented Feb 19, 2019

👍 for containerCommand and containerArgs

@skabashnyuk
Copy link
Contributor

ok for me.

@metlos
Copy link
Contributor

metlos commented Mar 5, 2019

The fool of me forgot about the second PR :)

@skabashnyuk skabashnyuk added the severity/P1 Has a major impact to usage or development of the system. label Mar 6, 2019
@metlos
Copy link
Contributor

metlos commented Mar 11, 2019

Both PRs are merged, so I'm closing this issue. Yay!

@metlos metlos closed this as completed Mar 11, 2019
@l0rd
Copy link
Contributor

l0rd commented Mar 11, 2019

@metlos 🎉 🎉 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system. status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it.
Projects
None yet
Development

No branches or pull requests

5 participants