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

support overriding entrypoint on service create from the cli #29171

Closed
vikstrous opened this Issue Dec 6, 2016 · 25 comments

Comments

Projects
None yet
10 participants
@vikstrous
Contributor

vikstrous commented Dec 6, 2016

Just opening this as a separate issue here from #25303 so it can be tracked. There was as discussion about adding --entrypoint to service create. This would be extremely useful to have for 1.13 because right now a large number of docker images are unusable as services without modification.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah
Member

thaJeztah commented Dec 6, 2016

@vdemeester

This comment has been minimized.

Show comment
Hide comment
@vdemeester

vdemeester Dec 6, 2016

Member

/cc @stevvooe and @dnephin too, remember some discussion about it 👼

Member

vdemeester commented Dec 6, 2016

/cc @stevvooe and @dnephin too, remember some discussion about it 👼

@dnephin

This comment has been minimized.

Show comment
Hide comment
@dnephin

dnephin Dec 6, 2016

Member

Yes,we've had a lot of discussions about this. We had this implemented for 1.12, but we removed it.

I think the two questions are:

  • what is the flag called? Docker calls it entrypoint, but swarmkit calls it command, which is very confusing to anyone who is used to Docker
  • How are multi-word args parsed? Is the flag repeated for each arg, or do we use shlex?

I think the second question should be mostly resolved now that we use shlex for service "args", it seems natural to use them for the entrypoint as well.

Member

dnephin commented Dec 6, 2016

Yes,we've had a lot of discussions about this. We had this implemented for 1.12, but we removed it.

I think the two questions are:

  • what is the flag called? Docker calls it entrypoint, but swarmkit calls it command, which is very confusing to anyone who is used to Docker
  • How are multi-word args parsed? Is the flag repeated for each arg, or do we use shlex?

I think the second question should be mostly resolved now that we use shlex for service "args", it seems natural to use them for the entrypoint as well.

@justincormack

This comment has been minimized.

Show comment
Hide comment
@justincormack

justincormack Dec 6, 2016

Contributor

Previous discussion is largely in #24196

Contributor

justincormack commented Dec 6, 2016

Previous discussion is largely in #24196

@justincormack

This comment has been minimized.

Show comment
Hide comment
@justincormack

justincormack Dec 6, 2016

Contributor

Summarizing the previous discussion, you should not need this as the command in a service is the entire command, so just specify it in full and all is well. It is confusing vs docker though, and docs improvements are needed.

Contributor

justincormack commented Dec 6, 2016

Summarizing the previous discussion, you should not need this as the command in a service is the entire command, so just specify it in full and all is well. It is confusing vs docker though, and docs improvements are needed.

@vikstrous

This comment has been minimized.

Show comment
Hide comment
@vikstrous

vikstrous Dec 6, 2016

Contributor

@justincormack Makes sense. I think it's fine if what's currently passed as arguments is passed as the entrypoint instead. That's a backwards incompatible CLI change though :/

Contributor

vikstrous commented Dec 6, 2016

@justincormack Makes sense. I think it's fine if what's currently passed as arguments is passed as the entrypoint instead. That's a backwards incompatible CLI change though :/

@stevvooe stevvooe changed the title from support overwriding entrypoint on service create from the cli to support overriding entrypoint on service create from the cli Dec 6, 2016

@justincormack

This comment has been minimized.

Show comment
Hide comment
@justincormack

justincormack Dec 6, 2016

Contributor

@vikstrous hmm, yes.

Contributor

justincormack commented Dec 6, 2016

@vikstrous hmm, yes.

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Dec 6, 2016

Contributor

@vikstrous @justincormack @dnephin Please see the table in #24196 (comment). I'll even link you to the k8s docs, which get this right.

what is the flag called? Docker calls it entrypoint, but swarmkit calls it command, which is very confusing to anyone who is used to Docker

No, the concept of Entrypoint and Cmd made no sense in any universe. In fact, every other project uses the concept of Command and Args. An image has the concept of an Entrypoint, which is really the default command, but containers don't really have this. They have a list of arguments. We specified a base command in swarmkit to represent this, which can override the image's Entrypoint. Cmd was confusing because it is not the actual command. It is really argv, which we have called args or arguments for 30+ of Unix.

This is confusing to docker users who weren't previously familiar with shell usage. It's compounded by the fact that users who were previously familiar with certain terminology, such as entrypoint (see entrypoint for previous definition, which supports why this is relevant to images) and args, encountering very imprecise uses of this terminology. When you change the name of the field to Args, this confusion goes away, since it provides a separate word for a concept that we had but were naming incorrectly (Cmd).

Today, we pass the results of the command line into the Args field. The Args field, roughly equivalent to Cmd (not Command or command or command), is appended to the contents of Entrypoint. There is currently no way to override Entrypoint in the command line, which is being requested here. The Entrypoint can be overridden with the Command field on ContainerSpec.

I've created one more table so we don't get confused:

Image.Entrypoint Image.Cmd ContainerSpec.Command ContainerSpec.Args Actual Command
/bin/cmd - - - /bin/cmd
/bin/cmd subcommand - - /bin/cmd subcommand
/bin/cmd subcommand - override /bin/cmd override
/bin/cmd subcommand - override more /bin/cmd override more
/bin/cmd subcommand arg - - /bin/cmd subcommand arg
/bin/cmd subcommand arg - override doubled /bin/cmd override doubled
/bin/cmd - /bin/tool - /bin/tool
/bin/cmd args /bin/tool - /bin/tool
/bin/cmd args /bin/tool user args /bin/tool user args

In code, this is the following:

var command, args []string
if len(ContainerSpec.Command) > 0 {
  command = append(command, ContainerSpec.Command...)
  command = append(command, ContainerSpec.Args...)
} else {
  command = append(command, Image.Entrypoint...)
  if len(ContainerSpec.Args) > 0 {
    command = append(command, ContainerSpec.Args...)
  } else {
    command = append(command, Image.Cmd...)
  }
}

// now you have "command", which is the actual argument list for the container

With above, we can say the following:

  • Image.Entrypoint is the default command to run in the container.
  • Image.Args is the default arguments to append to the command in the container, if using the default entrypoint.
  • ContainerSpec.Command if specified, will override Image.Entrypoint.
  • ContainerSpec.Args will be appended to the container command, whether it is from Image.Entrypoint or ContainerSpec.Command as arguments.

In regards to this issue, the question is whether or not to allow an image's Entrypoint to be overridden on service. That is the role of the flag. How it does this with the API, whether it uses the Command field or a flock of pigeons doesn't matter.

The main issue was providing effect argument parsing on the cli under a regular flag.

Contributor

stevvooe commented Dec 6, 2016

@vikstrous @justincormack @dnephin Please see the table in #24196 (comment). I'll even link you to the k8s docs, which get this right.

what is the flag called? Docker calls it entrypoint, but swarmkit calls it command, which is very confusing to anyone who is used to Docker

No, the concept of Entrypoint and Cmd made no sense in any universe. In fact, every other project uses the concept of Command and Args. An image has the concept of an Entrypoint, which is really the default command, but containers don't really have this. They have a list of arguments. We specified a base command in swarmkit to represent this, which can override the image's Entrypoint. Cmd was confusing because it is not the actual command. It is really argv, which we have called args or arguments for 30+ of Unix.

This is confusing to docker users who weren't previously familiar with shell usage. It's compounded by the fact that users who were previously familiar with certain terminology, such as entrypoint (see entrypoint for previous definition, which supports why this is relevant to images) and args, encountering very imprecise uses of this terminology. When you change the name of the field to Args, this confusion goes away, since it provides a separate word for a concept that we had but were naming incorrectly (Cmd).

Today, we pass the results of the command line into the Args field. The Args field, roughly equivalent to Cmd (not Command or command or command), is appended to the contents of Entrypoint. There is currently no way to override Entrypoint in the command line, which is being requested here. The Entrypoint can be overridden with the Command field on ContainerSpec.

I've created one more table so we don't get confused:

Image.Entrypoint Image.Cmd ContainerSpec.Command ContainerSpec.Args Actual Command
/bin/cmd - - - /bin/cmd
/bin/cmd subcommand - - /bin/cmd subcommand
/bin/cmd subcommand - override /bin/cmd override
/bin/cmd subcommand - override more /bin/cmd override more
/bin/cmd subcommand arg - - /bin/cmd subcommand arg
/bin/cmd subcommand arg - override doubled /bin/cmd override doubled
/bin/cmd - /bin/tool - /bin/tool
/bin/cmd args /bin/tool - /bin/tool
/bin/cmd args /bin/tool user args /bin/tool user args

In code, this is the following:

var command, args []string
if len(ContainerSpec.Command) > 0 {
  command = append(command, ContainerSpec.Command...)
  command = append(command, ContainerSpec.Args...)
} else {
  command = append(command, Image.Entrypoint...)
  if len(ContainerSpec.Args) > 0 {
    command = append(command, ContainerSpec.Args...)
  } else {
    command = append(command, Image.Cmd...)
  }
}

// now you have "command", which is the actual argument list for the container

With above, we can say the following:

  • Image.Entrypoint is the default command to run in the container.
  • Image.Args is the default arguments to append to the command in the container, if using the default entrypoint.
  • ContainerSpec.Command if specified, will override Image.Entrypoint.
  • ContainerSpec.Args will be appended to the container command, whether it is from Image.Entrypoint or ContainerSpec.Command as arguments.

In regards to this issue, the question is whether or not to allow an image's Entrypoint to be overridden on service. That is the role of the flag. How it does this with the API, whether it uses the Command field or a flock of pigeons doesn't matter.

The main issue was providing effect argument parsing on the cli under a regular flag.

@dnephin

This comment has been minimized.

Show comment
Hide comment
@dnephin

dnephin Dec 6, 2016

Member

I've created one more table so we don't get confused:

I don't think there is any confusion about how this works in swarmkit. It is irrelevant how it works in swarmkit because (most) users do not interact with swarmkit directly, they interact with the Docker CLI, and the UX of the Docker CLI must be consistent. This issue is only about the CLI flags, nothing else.

The only thing that is relevant to this discussion about CLI flags is this:

  • docker run --entrypoint "<entrypoint>" command...
  • docker service create command...

It should be obvious from these two commands that in order to be consistent we need an --entrypoint flag that sets the entrypoint using ContainerSpec.Command.

Member

dnephin commented Dec 6, 2016

I've created one more table so we don't get confused:

I don't think there is any confusion about how this works in swarmkit. It is irrelevant how it works in swarmkit because (most) users do not interact with swarmkit directly, they interact with the Docker CLI, and the UX of the Docker CLI must be consistent. This issue is only about the CLI flags, nothing else.

The only thing that is relevant to this discussion about CLI flags is this:

  • docker run --entrypoint "<entrypoint>" command...
  • docker service create command...

It should be obvious from these two commands that in order to be consistent we need an --entrypoint flag that sets the entrypoint using ContainerSpec.Command.

@dnephin dnephin added the area/cli label Dec 6, 2016

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Dec 6, 2016

Contributor

@dnephin Let me highlight what I said in the post above:

The main issue was providing effect argument parsing on the cli under a regular flag.

That is why --entrypoint is not on docker service. There was no solid proposal for how to parse flag-based args in the same way as create/run.

It is not because of confusion around swarmkit, a lack of consistency or any other reason.

It should be obvious from these two commands that in order to be consistent we need an --entrypoint flag that sets the entrypoint using ContainerSpec.Command.

I agree.

How is the --entrypoint flag parsed today?

Contributor

stevvooe commented Dec 6, 2016

@dnephin Let me highlight what I said in the post above:

The main issue was providing effect argument parsing on the cli under a regular flag.

That is why --entrypoint is not on docker service. There was no solid proposal for how to parse flag-based args in the same way as create/run.

It is not because of confusion around swarmkit, a lack of consistency or any other reason.

It should be obvious from these two commands that in order to be consistent we need an --entrypoint flag that sets the entrypoint using ContainerSpec.Command.

I agree.

How is the --entrypoint flag parsed today?

@dnephin

This comment has been minimized.

Show comment
Hide comment
@dnephin

dnephin Dec 7, 2016

Member

docker run passes the quoted string to the API as-is, so it's parsed by the backend.

docker update --args uses shlex.

The API excepts a []string so I think --entrypoint should work the same as --args and use shlex

Member

dnephin commented Dec 7, 2016

docker run passes the quoted string to the API as-is, so it's parsed by the backend.

docker update --args uses shlex.

The API excepts a []string so I think --entrypoint should work the same as --args and use shlex

@dnephin

This comment has been minimized.

Show comment
Hide comment
@dnephin
Member

dnephin commented Dec 7, 2016

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Dec 8, 2016

Contributor

docker run passes the quoted string to the API as-is, so it's parsed by the backend.

Isn't this a legacy API?

docker update --args uses shlex.
The API excepts a []string so I think --entrypoint should work the same as --args and use shlex

So, we did end up with shlex...

I played around with --entrypoint on docker run and it is quite broken. Let's take this one:

$ docker run --entrypoint foo --entrypoint bar -it --rm ubuntu
docker: Error response from daemon: oci runtime error: container_linux.go:247: starting container process caused "exec: \"bar\": executable file not found in $PATH".

The two flags are accepted, but then the backend only gets bar. Broken.

Let's give it a space-separated argument, which should run foo with bar as the arg:

$ docker run --entrypoint 'foo bar' -it --rm ubuntu
docker: Error response from daemon: oci runtime error: container_linux.go:247: starting container process caused "exec: \"foo bar\": executable file not found in $PATH".

Broken.

I guess I am okay with the shlex version.

Contributor

stevvooe commented Dec 8, 2016

docker run passes the quoted string to the API as-is, so it's parsed by the backend.

Isn't this a legacy API?

docker update --args uses shlex.
The API excepts a []string so I think --entrypoint should work the same as --args and use shlex

So, we did end up with shlex...

I played around with --entrypoint on docker run and it is quite broken. Let's take this one:

$ docker run --entrypoint foo --entrypoint bar -it --rm ubuntu
docker: Error response from daemon: oci runtime error: container_linux.go:247: starting container process caused "exec: \"bar\": executable file not found in $PATH".

The two flags are accepted, but then the backend only gets bar. Broken.

Let's give it a space-separated argument, which should run foo with bar as the arg:

$ docker run --entrypoint 'foo bar' -it --rm ubuntu
docker: Error response from daemon: oci runtime error: container_linux.go:247: starting container process caused "exec: \"foo bar\": executable file not found in $PATH".

Broken.

I guess I am okay with the shlex version.

@justincormack

This comment has been minimized.

Show comment
Hide comment
@justincormack

justincormack Dec 8, 2016

Contributor
Contributor

justincormack commented Dec 8, 2016

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Dec 8, 2016

Member

Yes its always been impossible to reset the default docker entry point of
sh -c from the CLI. Very broken.

In 1.13 you can reset the entrypoint by specifying an empty string; #23718, or do you mean that it's always executed in the "shell" form?

The two flags are accepted, but then the backend only gets bar. Broken.

For flags that don't accept multiple values and are provided multiple times, the later value overwrites the former one. This has always been the case in the docker CLI, and I think it has come up in the past, but changing that behavior was considered too much of a breaking change at that point.

w.r.t.

docker run --entrypoint 'foo bar' -it --rm ubuntu

The way to do that in docker would be to set foo as entrypoint, and bar as cmd

docker run --entrypoint foo -it --rm ubuntu bar

(yup, it's confusing)

Member

thaJeztah commented Dec 8, 2016

Yes its always been impossible to reset the default docker entry point of
sh -c from the CLI. Very broken.

In 1.13 you can reset the entrypoint by specifying an empty string; #23718, or do you mean that it's always executed in the "shell" form?

The two flags are accepted, but then the backend only gets bar. Broken.

For flags that don't accept multiple values and are provided multiple times, the later value overwrites the former one. This has always been the case in the docker CLI, and I think it has come up in the past, but changing that behavior was considered too much of a breaking change at that point.

w.r.t.

docker run --entrypoint 'foo bar' -it --rm ubuntu

The way to do that in docker would be to set foo as entrypoint, and bar as cmd

docker run --entrypoint foo -it --rm ubuntu bar

(yup, it's confusing)

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Dec 8, 2016

Contributor

@thaJeztah The command should not accept two --entrypoint flags. That should be an error.

Also, if the argument for adding this is consistency, do we add this new flag as consistently broken?

Contributor

stevvooe commented Dec 8, 2016

@thaJeztah The command should not accept two --entrypoint flags. That should be an error.

Also, if the argument for adding this is consistency, do we add this new flag as consistently broken?

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Dec 8, 2016

Member

Also, if the argument for adding this is consistency, do we add this new flag as consistently broken?

I'm not saying we should repeat the wrong behavior on docker service create, but fixing it for docker run could be troublesome, given that it has been like that forever.

It will still result in people expecting the same behavior as docker run, so we should make sure there's a clear error message.

@dnephin do you know if Cobra can enable/disable that behavior on a per-command base?

Member

thaJeztah commented Dec 8, 2016

Also, if the argument for adding this is consistency, do we add this new flag as consistently broken?

I'm not saying we should repeat the wrong behavior on docker service create, but fixing it for docker run could be troublesome, given that it has been like that forever.

It will still result in people expecting the same behavior as docker run, so we should make sure there's a clear error message.

@dnephin do you know if Cobra can enable/disable that behavior on a per-command base?

@liyaka

This comment has been minimized.

Show comment
Hide comment
@liyaka

liyaka Dec 12, 2016

Bottom line - will it be or not --entrypoint/--command/--cmd/...flag for docker service create command that will replace the image CMD??
This is really an inconsistent behavior and missing functionality...

liyaka commented Dec 12, 2016

Bottom line - will it be or not --entrypoint/--command/--cmd/...flag for docker service create command that will replace the image CMD??
This is really an inconsistent behavior and missing functionality...

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Dec 12, 2016

Contributor

The current syntax is: docker service create [opts] image [entrypoint/command] [args]

I think the current behavior is the least confusing, albeit not consistent with docker run.
The only thing you can't do is add args w/o modifying the entrypoint.
... except you can via docker service update --args

Adding a --entrypoint will be even more confusing. The only really confusing part is that it's not the same as docker run, which itself has confused probably just about every single user to docker at some point.

Contributor

cpuguy83 commented Dec 12, 2016

The current syntax is: docker service create [opts] image [entrypoint/command] [args]

I think the current behavior is the least confusing, albeit not consistent with docker run.
The only thing you can't do is add args w/o modifying the entrypoint.
... except you can via docker service update --args

Adding a --entrypoint will be even more confusing. The only really confusing part is that it's not the same as docker run, which itself has confused probably just about every single user to docker at some point.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Dec 12, 2016

Member

@cpuguy83 the problem currently is that you cannot override the entrypoint, for example, running;

docker service create --name foo docker:1.13-rc sh -c "echo foo"

Results in sh -c "echo foo" being set as command, but the entrypoint is not reset;

"Cmd": [
    "sh",
    "-c",
    "echo foo"
],
"Image": "docker@sha256:986f74e5b0810a22e081afd66c96256dbfe1b105ce328da1de0a5c6d32814bdf",
"Volumes": null,
"WorkingDir": "",
"Entrypoint": [
    "docker-entrypoint.sh"
],
Member

thaJeztah commented Dec 12, 2016

@cpuguy83 the problem currently is that you cannot override the entrypoint, for example, running;

docker service create --name foo docker:1.13-rc sh -c "echo foo"

Results in sh -c "echo foo" being set as command, but the entrypoint is not reset;

"Cmd": [
    "sh",
    "-c",
    "echo foo"
],
"Image": "docker@sha256:986f74e5b0810a22e081afd66c96256dbfe1b105ce328da1de0a5c6d32814bdf",
"Volumes": null,
"WorkingDir": "",
"Entrypoint": [
    "docker-entrypoint.sh"
],
@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Dec 12, 2016

Contributor

Yep, I guess you're right, we always add entrypoint back.

Contributor

cpuguy83 commented Dec 12, 2016

Yep, I guess you're right, we always add entrypoint back.

@krasi-georgiev

This comment has been minimized.

Show comment
Hide comment
@krasi-georgiev

krasi-georgiev Jan 12, 2017

Contributor

entrypoint has always been confusing for me as well , command and args doesn't need explaining

docker service create image command arg = entrypoint.sh command arg

much cleaner
docker service create --no-entrypoint image command arg1 arg2

Contributor

krasi-georgiev commented Jan 12, 2017

entrypoint has always been confusing for me as well , command and args doesn't need explaining

docker service create image command arg = entrypoint.sh command arg

much cleaner
docker service create --no-entrypoint image command arg1 arg2

@mostolog

This comment has been minimized.

Show comment
Hide comment
@mostolog

mostolog Feb 22, 2017

to sum up: would docker services/stack support --entrypoint? Do they already?

mostolog commented Feb 22, 2017

to sum up: would docker services/stack support --entrypoint? Do they already?

@dnephin

This comment has been minimized.

Show comment
Hide comment
@dnephin

dnephin Feb 22, 2017

Member

It's already supported in stack deploy, and #29228 is open to add it to services

Member

dnephin commented Feb 22, 2017

It's already supported in stack deploy, and #29228 is open to add it to services

@thaJeztah thaJeztah added this to the 17.05.0 milestone Mar 29, 2017

@thaJeztah thaJeztah removed this from Backlog / undecided in 1.13-rcX Mar 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment