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

Implement docker update command #15078

Merged
merged 1 commit into from Dec 28, 2015

Conversation

Projects
None yet
@hqhq
Contributor

hqhq commented Jul 28, 2015

fixes #6323

It's used for change properties of one or more containers, we only
support resource configs for now. It can be extended in the future.

Signed-off-by: Qiang Huang h.huangqiang@huawei.com


The original PR #11442, compared to the former one, changelog:

  • support set for both running and stopped containers
  • support more cgroup configs as the new ones were merged
  • move some container config change logic to container daemon/container_xxx.go
  • addressed all issues pointed by @duglin

We have strong needs for this feature for a long time, see #3285. It's really useful feature and necessary for most product usage.

@icecrime @crosbymichael as we discussed on DockerCon, we should reconsider this feature, I have pinged you guys several times hope we can reopen the original one but got no reply, so I opened this new PR, hope it's ok. If the old one can be reopened, I'm ok to close this and continue the original discussion.

@hqhq hqhq changed the title from Implemet docker set command to Implement docker set command Jul 28, 2015

@calavera

This comment has been minimized.

Show comment
Hide comment
@calavera

calavera Jul 29, 2015

Contributor

So, my take on this new command is that it doesn't really address any of the issues described there, per-se:

Unfortunately, docker doesn't currently seem to support this. Since you can't re-run a stopped container with docker run, you would need to commit the container to an image and then re-run it.

Unfortunately, docker commit clears the container's configuration, so you need to explicitly set a startup configuration using the -run commandline argument, unless you don't mind manually specifying the entrypoint/cmd et cetera each time you run this new image with docker run (which I don't think is very user friendly). As a workaround, you can inspect the container, copy the Config, and paste the config into the -run parameter of docker commit, but this isn't really user-friendly either.

All those issues will still be present if we add this new command, but people will have to learn how to use docker set to work around them.

From my point of view, the main pain points are caused by bad usability in other commands. I'd personally prefer to have concrete solutions to those issues before adding a new command, but I can be convinced that we need a new command if there is no other way to solve those issues.

Contributor

calavera commented Jul 29, 2015

So, my take on this new command is that it doesn't really address any of the issues described there, per-se:

Unfortunately, docker doesn't currently seem to support this. Since you can't re-run a stopped container with docker run, you would need to commit the container to an image and then re-run it.

Unfortunately, docker commit clears the container's configuration, so you need to explicitly set a startup configuration using the -run commandline argument, unless you don't mind manually specifying the entrypoint/cmd et cetera each time you run this new image with docker run (which I don't think is very user friendly). As a workaround, you can inspect the container, copy the Config, and paste the config into the -run parameter of docker commit, but this isn't really user-friendly either.

All those issues will still be present if we add this new command, but people will have to learn how to use docker set to work around them.

From my point of view, the main pain points are caused by bad usability in other commands. I'd personally prefer to have concrete solutions to those issues before adding a new command, but I can be convinced that we need a new command if there is no other way to solve those issues.

@tiborvass

This comment has been minimized.

Show comment
Hide comment
@tiborvass

tiborvass Jul 30, 2015

Collaborator

I have a weak +1 for dynamically changing resource configs on both running and stopped containers, however I am not exactly sure if we would want (or even able to) change other settings (e.g. environment variables). Therefore, I don't know if set is the right keyword. Maybe it is.

Collaborator

tiborvass commented Jul 30, 2015

I have a weak +1 for dynamically changing resource configs on both running and stopped containers, however I am not exactly sure if we would want (or even able to) change other settings (e.g. environment variables). Therefore, I don't know if set is the right keyword. Maybe it is.

@hqhq

This comment has been minimized.

Show comment
Hide comment
@hqhq

hqhq Jul 30, 2015

Contributor

@calavera

Unfortunately, docker doesn't currently seem to support this. Since you can't re-run a stopped container with docker run, you would need to commit the container to an image and then re-run it.

We don't need to commit the container to an image and re-run it, we just do:

$ docker stop xxx
$ docker set xxx
$ docker start xxx

Unfortunately, docker commit clears the container's configuration, so you need to explicitly set a startup configuration using the -run commandline argument, unless you don't mind manually specifying the entrypoint/cmd et cetera each time you run this new image with docker run (which I don't think is very user friendly). As a workaround, you can inspect the container, copy the Config, and paste the config into the -run parameter of docker commit, but this isn't really user-friendly either.

As above, this is not the common use case, for people who want to dynamically change properties, they don't want to delete or commit current container and re-run it.

Contributor

hqhq commented Jul 30, 2015

@calavera

Unfortunately, docker doesn't currently seem to support this. Since you can't re-run a stopped container with docker run, you would need to commit the container to an image and then re-run it.

We don't need to commit the container to an image and re-run it, we just do:

$ docker stop xxx
$ docker set xxx
$ docker start xxx

Unfortunately, docker commit clears the container's configuration, so you need to explicitly set a startup configuration using the -run commandline argument, unless you don't mind manually specifying the entrypoint/cmd et cetera each time you run this new image with docker run (which I don't think is very user friendly). As a workaround, you can inspect the container, copy the Config, and paste the config into the -run parameter of docker commit, but this isn't really user-friendly either.

As above, this is not the common use case, for people who want to dynamically change properties, they don't want to delete or commit current container and re-run it.

@hqhq

This comment has been minimized.

Show comment
Hide comment
@hqhq

hqhq Jul 30, 2015

Contributor

@tiborvass I'm open with the command name 😄

Contributor

hqhq commented Jul 30, 2015

@tiborvass I'm open with the command name 😄

@selvik

This comment has been minimized.

Show comment
Hide comment
@selvik

selvik Aug 17, 2015

@hqhq I am trying to understand your related previous PR here: #11442 and the feedback received.

Do you agree with the suggestion that using --cgroup-parent enables us to change resource configs of a running container? I don't yet see how this could be done.

Since only docker run accepts the --cgroup-parent flag, the only way I can actually change the config. of a container that is already started is to directly write to the cgroup filesystem on the docker host.

Would you have an example of how --cgroup-parent can be used to change config of a running container? I have asked the same question in that PR too, but wasn't sure if a closed PR intimates those involved.

Thanks for any hints!

selvik commented Aug 17, 2015

@hqhq I am trying to understand your related previous PR here: #11442 and the feedback received.

Do you agree with the suggestion that using --cgroup-parent enables us to change resource configs of a running container? I don't yet see how this could be done.

Since only docker run accepts the --cgroup-parent flag, the only way I can actually change the config. of a container that is already started is to directly write to the cgroup filesystem on the docker host.

Would you have an example of how --cgroup-parent can be used to change config of a running container? I have asked the same question in that PR too, but wasn't sure if a closed PR intimates those involved.

Thanks for any hints!

@calavera

This comment has been minimized.

Show comment
Hide comment
@calavera

calavera Aug 17, 2015

Contributor

We don't need to commit the container to an image and re-run it, we just do:

but if we allow to run stopped containers with docker run we would not need to add a new command, right?

Contributor

calavera commented Aug 17, 2015

We don't need to commit the container to an image and re-run it, we just do:

but if we allow to run stopped containers with docker run we would not need to add a new command, right?

@hqhq

This comment has been minimized.

Show comment
Hide comment
@hqhq

hqhq Aug 19, 2015

Contributor

@selvik You can always change resource configs of running containers by changing their cgroup files, that has nothing to do with Docker, but usually the problem is it relayed on users understand cgroup details, know the cgroup path of their containers, and know which files to be changed, --parent-cgroup option just make user know what path is the container in.

So I didn't mean --cgroup-parent can resolve the problem, it just did very litter help, so we still need a more friendly docker command so users don't need to know there cgroup details.

Contributor

hqhq commented Aug 19, 2015

@selvik You can always change resource configs of running containers by changing their cgroup files, that has nothing to do with Docker, but usually the problem is it relayed on users understand cgroup details, know the cgroup path of their containers, and know which files to be changed, --parent-cgroup option just make user know what path is the container in.

So I didn't mean --cgroup-parent can resolve the problem, it just did very litter help, so we still need a more friendly docker command so users don't need to know there cgroup details.

@hqhq

This comment has been minimized.

Show comment
Hide comment
@hqhq

hqhq Aug 19, 2015

Contributor

@calavera Sort of. But it only resolve the problem that change configs for stopped containers, not for running containers, and I think that'll make it more complicated, because not all options of docker run can be changed. So I don't think that's a good idea to make docker run can start a stopped container.

Contributor

hqhq commented Aug 19, 2015

@calavera Sort of. But it only resolve the problem that change configs for stopped containers, not for running containers, and I think that'll make it more complicated, because not all options of docker run can be changed. So I don't think that's a good idea to make docker run can start a stopped container.

@selvik

This comment has been minimized.

Show comment
Hide comment
@selvik

selvik Aug 19, 2015

@hqhq I agree, we're on the same page then. docker set would be very useful for us. Looking forward to this PR being merged! Thanks for working on it!

selvik commented Aug 19, 2015

@hqhq I agree, we're on the same page then. docker set would be very useful for us. Looking forward to this PR being merged! Thanks for working on it!

@cnaize

This comment has been minimized.

Show comment
Hide comment
@cnaize

cnaize Aug 31, 2015

@hqhq

We don't need to commit the container to an image and re-run it, we just do:
$ docker stop xxx
$ docker set xxx
$ docker start xxx

Is it mean that I have to stop container if I want to change memory limit, for example?

I need to change limits on running container, how to do it? Only by cgroup?
Could you please provide an example?

If I can help you to implement changing limits on the fly, let me know.

cnaize commented Aug 31, 2015

@hqhq

We don't need to commit the container to an image and re-run it, we just do:
$ docker stop xxx
$ docker set xxx
$ docker start xxx

Is it mean that I have to stop container if I want to change memory limit, for example?

I need to change limits on running container, how to do it? Only by cgroup?
Could you please provide an example?

If I can help you to implement changing limits on the fly, let me know.

@hqhq

This comment has been minimized.

Show comment
Hide comment
@hqhq

hqhq Sep 1, 2015

Contributor

@cnaize No you don't need to stop container, you can just use docker set -m 300M my_container if you want to change memory limit for a running container.

My approach support changing configs for both running and stopped containers. You don't need to know if a container is running or stopped, for stopped container, the changed configs will be effective when it started again.

Currently, the configs I mentioned only mean resource configs, but it can be expended in the future.

Contributor

hqhq commented Sep 1, 2015

@cnaize No you don't need to stop container, you can just use docker set -m 300M my_container if you want to change memory limit for a running container.

My approach support changing configs for both running and stopped containers. You don't need to know if a container is running or stopped, for stopped container, the changed configs will be effective when it started again.

Currently, the configs I mentioned only mean resource configs, but it can be expended in the future.

@vdemeester

This comment has been minimized.

Show comment
Hide comment
@vdemeester

vdemeester Sep 30, 2015

Member

How are we on that ?

  • I think using flags (e.g. docker set --cpu-period … …) is not that good ; and I would tend to prefer something like docker set cpu-period … … but it really is a nit I guess. It would allow to not have to declare all the thing that can be change into the client code, but on the other hand it would probably expose too much stuff.
  • The command name set feels way too generic I think, it should probably be set-config or something like that. WDYT ?

@hqhq needs a rebase 😓

/cc @cpuguy83 @icecrime @runcom

Member

vdemeester commented Sep 30, 2015

How are we on that ?

  • I think using flags (e.g. docker set --cpu-period … …) is not that good ; and I would tend to prefer something like docker set cpu-period … … but it really is a nit I guess. It would allow to not have to declare all the thing that can be change into the client code, but on the other hand it would probably expose too much stuff.
  • The command name set feels way too generic I think, it should probably be set-config or something like that. WDYT ?

@hqhq needs a rebase 😓

/cc @cpuguy83 @icecrime @runcom

@tiborvass

This comment has been minimized.

Show comment
Hide comment
@tiborvass

tiborvass Oct 1, 2015

Collaborator

Collective review

There are valid usecases detailed below. The docker set solution would be a BIG change that would need @shykes's input before we can move on.

For the sake of the discussion, we added more questions to explore different solutions for the usecases.

Some valid usecases:

  • change ports dynamically:
    • I start developing in a container, and later I realize I want to expose a port.
    • add a debug port and then remove it
    • could docker network address this usecase? docker network handles networks and not ports. Is that an issue?
  • change cgroups:
    • one solution for this is to link from documentation to external tools that can change actual cgroups for docker containers
  • change volumes/label dynamically?
    • We need an answer to the following question: when is it appropriate to change a container property vs requiring to spawn a new container with different properties?
    • for labels specifically, should there be a label-specific UI if we were to also include changing daemon labels and not just container labels?

One solution would be to add a new docker set command but restrict it to whatever properties we agree on.

Collaborator

tiborvass commented Oct 1, 2015

Collective review

There are valid usecases detailed below. The docker set solution would be a BIG change that would need @shykes's input before we can move on.

For the sake of the discussion, we added more questions to explore different solutions for the usecases.

Some valid usecases:

  • change ports dynamically:
    • I start developing in a container, and later I realize I want to expose a port.
    • add a debug port and then remove it
    • could docker network address this usecase? docker network handles networks and not ports. Is that an issue?
  • change cgroups:
    • one solution for this is to link from documentation to external tools that can change actual cgroups for docker containers
  • change volumes/label dynamically?
    • We need an answer to the following question: when is it appropriate to change a container property vs requiring to spawn a new container with different properties?
    • for labels specifically, should there be a label-specific UI if we were to also include changing daemon labels and not just container labels?

One solution would be to add a new docker set command but restrict it to whatever properties we agree on.

@tiborvass

This comment has been minimized.

Show comment
Hide comment
@tiborvass

tiborvass Oct 1, 2015

Collaborator

Forgot to mention environment variables: I'm not even sure how you can do that without restarting the process at which point you might as well respawn the container.

Collaborator

tiborvass commented Oct 1, 2015

Forgot to mention environment variables: I'm not even sure how you can do that without restarting the process at which point you might as well respawn the container.

@duglin

This comment has been minimized.

Show comment
Hide comment
@duglin

duglin Oct 2, 2015

Contributor

yup - some config changes may not take effect until there's a restart of the container, but I can see that for some scenarios that's better than having to start from scratch with a new container.

Contributor

duglin commented Oct 2, 2015

yup - some config changes may not take effect until there's a restart of the container, but I can see that for some scenarios that's better than having to start from scratch with a new container.

@hqhq

This comment has been minimized.

Show comment
Hide comment
@hqhq

hqhq Oct 3, 2015

Contributor

Yes docker set has to be restricted to whatever properties we agreed on, not all properties docker run supported can be changed dynamically. And some of them need a restart to make them effect, I think we can give users some messages for these configs when they using them for a running container.
@vdemeester Seems this PR is still in design review stage, I'd like to rebase when it's in code review stage, is that OK?

Contributor

hqhq commented Oct 3, 2015

Yes docker set has to be restricted to whatever properties we agreed on, not all properties docker run supported can be changed dynamically. And some of them need a restart to make them effect, I think we can give users some messages for these configs when they using them for a running container.
@vdemeester Seems this PR is still in design review stage, I'd like to rebase when it's in code review stage, is that OK?

@rutsky

This comment has been minimized.

Show comment
Hide comment
@rutsky

rutsky Oct 6, 2015

Contributor

Where should I address request to implement docker set for specific configuration options?
E.g. I would like to have ability to change RestartPolicy, and change of this property shouldn't break anything (in comparison with for example changing environment variables).

Contributor

rutsky commented Oct 6, 2015

Where should I address request to implement docker set for specific configuration options?
E.g. I would like to have ability to change RestartPolicy, and change of this property shouldn't break anything (in comparison with for example changing environment variables).

@wdx900

This comment has been minimized.

Show comment
Hide comment
@wdx900

wdx900 Oct 21, 2015

Hi,我现在希望在container running的情况下去修改容器的端口映射,我看到docker set可以实现CPU和MEM的动态修改,但是如何去动态修改他的端口映射,你有什么建议吗,谢谢

wdx900 commented Oct 21, 2015

Hi,我现在希望在container running的情况下去修改容器的端口映射,我看到docker set可以实现CPU和MEM的动态修改,但是如何去动态修改他的端口映射,你有什么建议吗,谢谢

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Dec 28, 2015

Member

docs LGTM! I think we're there, and it's ready to merge! 👯

Thank you so much @hqhq, very happy to see this ready. Thanks for being so patient with us!

I restarted the Windows CI for good measure, because it timed out

Member

thaJeztah commented Dec 28, 2015

docs LGTM! I think we're there, and it's ready to merge! 👯

Thank you so much @hqhq, very happy to see this ready. Thanks for being so patient with us!

I restarted the Windows CI for good measure, because it timed out

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Dec 28, 2015

Member

sigh, flaky tests I think;

FAIL: docker_cli_events_test.go:415: DockerSuite.TestEventsStreaming

docker_cli_events_test.go:452:
    c.Fatal(observer.TimeoutError(containerID, "create"))
... Error: failed to observe event `create` for f0ad173280a3e918743be743bd661513b3bf9448d0eff85fd85f07e02a782528
2015-12-28T14:31:40.073268700Z 4a87a239201ddd2feb3843634fcd88435fb290f822f1d179458ee0093f6e9020: (from busybox) die
2015-12-28T14:31:40.276026700Z 4a87a239201ddd2feb3843634fcd88435fb290f822f1d179458ee0093f6e9020: (from busybox) stop
2015-12-28T14:31:40.513889100Z 4a87a239201ddd2feb3843634fcd88435fb290f822f1d179458ee0093f6e9020: (from busybox) destroy

and

FAIL: docker_cli_build_test.go:1873: DockerSuite.TestBuildCancellationKillsSleep

docker_cli_build_test.go:1935:
    c.Fatal(observer.TimeoutError(buildID, "start"))
... Error: failed to observe event `start` for 17521280d8c9
2015-12-28T14:17:33.251400000Z e60e841a06bd0f33fa0f5a19be09346d7f0fd5edf7965a50f5a95bab80ccfb3d: (from sha256:aa5d5f5a81c90b8683085c027bae215d2738ff25f9781f969c48b5b0c8a3bb97) create
2015-12-28T14:17:33.420929200Z e60e841a06bd0f33fa0f5a19be09346d7f0fd5edf7965a50f5a95bab80ccfb3d: (from sha256:aa5d5f5a81c90b8683085c027bae215d2738ff25f9781f969c48b5b0c8a3bb97) commit
2015-12-28T14:17:33.440901000Z e60e841a06bd0f33fa0f5a19be09346d7f0fd5edf7965a50f5a95bab80ccfb3d: (from sha256:aa5d5f5a81c90b8683085c027bae215d2738ff25f9781f969c48b5b0c8a3bb97) destroy
2015-12-28T14:17:33.441202900Z testbuildrootsource:latest: tag
2015-12-28T14:17:33.728062200Z sha256:9b4408f3c59a68065198827361560a8e49032a188fe043d5fec9e10303b86dd7: untag
2015-12-28T14:17:33.729825400Z sha256:9b4408f3c59a68065198827361560a8e49032a188fe043d5fec9e10303b86dd7: delete
2015-12-28T14:17:33.733190000Z sha256:6c0cef5b5c2aa992258a712d55a1c215cdf12d9055c5446f5bc548dcce724f3a: delete

Second run:

FAIL: docker_cli_events_test.go:216: DockerSuite.TestEventsImageImport

docker_cli_events_test.go:247:
    c.Fatal(observer.TimeoutError(imageRef, "import"))
... Error: failed to observe event `import` for sha256:18fc30c9f3cb1f8a7123af54ae0309d7afdcdb26e32dd37a49befa0f509c582b
2015-12-28T15:21:00.124964000Z 003dc22eb38ecf7b58b1df0019f3b02ccf07e47cd4d70a3febee961035b4d450: (from busybox) create
2015-12-28T15:21:00.127764900Z 003dc22eb38ecf7b58b1df0019f3b02ccf07e47cd4d70a3febee961035b4d450: (from busybox) attach
2015-12-28T15:21:00.164300600Z 003dc22eb38ecf7b58b1df0019f3b02ccf07e47cd4d70a3febee961035b4d450: (from busybox) start
2015-12-28T15:21:00.327801300Z 003dc22eb38ecf7b58b1df0019f3b02ccf07e47cd4d70a3febee961035b4d450: (from busybox) die
2015-12-28T15:21:00.446943600Z 003dc22eb38ecf7b58b1df0019f3b02ccf07e47cd4d70a3febee961035b4d450: (from busybox) destroy
Member

thaJeztah commented Dec 28, 2015

sigh, flaky tests I think;

FAIL: docker_cli_events_test.go:415: DockerSuite.TestEventsStreaming

docker_cli_events_test.go:452:
    c.Fatal(observer.TimeoutError(containerID, "create"))
... Error: failed to observe event `create` for f0ad173280a3e918743be743bd661513b3bf9448d0eff85fd85f07e02a782528
2015-12-28T14:31:40.073268700Z 4a87a239201ddd2feb3843634fcd88435fb290f822f1d179458ee0093f6e9020: (from busybox) die
2015-12-28T14:31:40.276026700Z 4a87a239201ddd2feb3843634fcd88435fb290f822f1d179458ee0093f6e9020: (from busybox) stop
2015-12-28T14:31:40.513889100Z 4a87a239201ddd2feb3843634fcd88435fb290f822f1d179458ee0093f6e9020: (from busybox) destroy

and

FAIL: docker_cli_build_test.go:1873: DockerSuite.TestBuildCancellationKillsSleep

docker_cli_build_test.go:1935:
    c.Fatal(observer.TimeoutError(buildID, "start"))
... Error: failed to observe event `start` for 17521280d8c9
2015-12-28T14:17:33.251400000Z e60e841a06bd0f33fa0f5a19be09346d7f0fd5edf7965a50f5a95bab80ccfb3d: (from sha256:aa5d5f5a81c90b8683085c027bae215d2738ff25f9781f969c48b5b0c8a3bb97) create
2015-12-28T14:17:33.420929200Z e60e841a06bd0f33fa0f5a19be09346d7f0fd5edf7965a50f5a95bab80ccfb3d: (from sha256:aa5d5f5a81c90b8683085c027bae215d2738ff25f9781f969c48b5b0c8a3bb97) commit
2015-12-28T14:17:33.440901000Z e60e841a06bd0f33fa0f5a19be09346d7f0fd5edf7965a50f5a95bab80ccfb3d: (from sha256:aa5d5f5a81c90b8683085c027bae215d2738ff25f9781f969c48b5b0c8a3bb97) destroy
2015-12-28T14:17:33.441202900Z testbuildrootsource:latest: tag
2015-12-28T14:17:33.728062200Z sha256:9b4408f3c59a68065198827361560a8e49032a188fe043d5fec9e10303b86dd7: untag
2015-12-28T14:17:33.729825400Z sha256:9b4408f3c59a68065198827361560a8e49032a188fe043d5fec9e10303b86dd7: delete
2015-12-28T14:17:33.733190000Z sha256:6c0cef5b5c2aa992258a712d55a1c215cdf12d9055c5446f5bc548dcce724f3a: delete

Second run:

FAIL: docker_cli_events_test.go:216: DockerSuite.TestEventsImageImport

docker_cli_events_test.go:247:
    c.Fatal(observer.TimeoutError(imageRef, "import"))
... Error: failed to observe event `import` for sha256:18fc30c9f3cb1f8a7123af54ae0309d7afdcdb26e32dd37a49befa0f509c582b
2015-12-28T15:21:00.124964000Z 003dc22eb38ecf7b58b1df0019f3b02ccf07e47cd4d70a3febee961035b4d450: (from busybox) create
2015-12-28T15:21:00.127764900Z 003dc22eb38ecf7b58b1df0019f3b02ccf07e47cd4d70a3febee961035b4d450: (from busybox) attach
2015-12-28T15:21:00.164300600Z 003dc22eb38ecf7b58b1df0019f3b02ccf07e47cd4d70a3febee961035b4d450: (from busybox) start
2015-12-28T15:21:00.327801300Z 003dc22eb38ecf7b58b1df0019f3b02ccf07e47cd4d70a3febee961035b4d450: (from busybox) die
2015-12-28T15:21:00.446943600Z 003dc22eb38ecf7b58b1df0019f3b02ccf07e47cd4d70a3febee961035b4d450: (from busybox) destroy

calavera added a commit that referenced this pull request Dec 28, 2015

Merge pull request #15078 from hqhq/hq_add_set_api_v2
Implement docker update command

@calavera calavera merged commit 8669ea0 into moby:master Dec 28, 2015

5 of 6 checks passed

windows Jenkins build Windows-PRs 19295 has failed
Details
docker/dco-signed All commits signed
Details
documentation success 2 tests run, 0 skipped, 0 failed.
Details
experimental Jenkins build Docker-PRs-experimental 12718 has succeeded
Details
janky Jenkins build Docker-PRs 21508 has succeeded
Details
userns Jenkins build Docker-PRs-userns 3963 has succeeded
Details
@moxiegirl

This comment has been minimized.

Show comment
Hide comment
@moxiegirl

moxiegirl Dec 28, 2015

Contributor

@hqhq Thank you for the work.

Contributor

moxiegirl commented Dec 28, 2015

@hqhq Thank you for the work.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Dec 28, 2015

Member

@moxiegirl I noticed some tiny nits in the docs, but will create a follow up PR to address those. I didn't think it was worth blocking the merge on those :)

Member

thaJeztah commented Dec 28, 2015

@moxiegirl I noticed some tiny nits in the docs, but will create a follow up PR to address those. I didn't think it was worth blocking the merge on those :)

@moxiegirl

This comment has been minimized.

Show comment
Hide comment
@moxiegirl

moxiegirl Dec 28, 2015

Contributor

@thaJeztah Thanks. You notice I had not actually done an LGTM on this one...it was in a hurry I believe to I won't complain but I will point out. :-P

Contributor

moxiegirl commented Dec 28, 2015

@thaJeztah Thanks. You notice I had not actually done an LGTM on this one...it was in a hurry I believe to I won't complain but I will point out. :-P

@hqhq hqhq deleted the hqhq:hq_add_set_api_v2 branch Dec 29, 2015

hqhq added a commit to hqhq/moby that referenced this pull request Dec 29, 2015

Remove redundant error messages
For operations on multi containers, we printed error for each
failed container, then printed an extra message for container
names, it seems redundant.

Addresses comments:
moby#15078 (comment)

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>

@thaJeztah thaJeztah added this to the 1.10 milestone Jan 3, 2016

@aanand aanand referenced this pull request Jan 6, 2016

Closed

Proposal : Container alias #18699

dave-tucker pushed a commit to dave-tucker/docker that referenced this pull request Jan 11, 2016

Remove redundant error messages
For operations on multi containers, we printed error for each
failed container, then printed an extra message for container
names, it seems redundant.

Addresses comments:
moby#15078 (comment)

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
@frakky

This comment has been minimized.

Show comment
Hide comment
@frakky

frakky Jan 12, 2016

Hey,
as this command will also handle changing ports on the fly in the future, how would I approach the implementation of that?
I would like to help creating the port mapping feature but I don't really know where to start.

frakky commented Jan 12, 2016

Hey,
as this command will also handle changing ports on the fly in the future, how would I approach the implementation of that?
I would like to help creating the port mapping feature but I don't really know where to start.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jan 12, 2016

Member

@frakky I think there's already an issue open for that feature; #2045. If you need help implementing, you can ask for assistance in the #docker-dev IRC channel. Keep in mind that it's currently very busy for the maintainers, because code-freeze for 1.10 is this week.

Member

thaJeztah commented Jan 12, 2016

@frakky I think there's already an issue open for that feature; #2045. If you need help implementing, you can ask for assistance in the #docker-dev IRC channel. Keep in mind that it's currently very busy for the maintainers, because code-freeze for 1.10 is this week.

lami02 added a commit to lami02/docker that referenced this pull request Jan 19, 2016

Remove redundant error messages
For operations on multi containers, we printed error for each
failed container, then printed an extra message for container
names, it seems redundant.

Addresses comments:
moby#15078 (comment)

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>

aditirajagopal pushed a commit to aditirajagopal/docker that referenced this pull request Feb 8, 2016

Remove redundant error messages
For operations on multi containers, we printed error for each
failed container, then printed an extra message for container
names, it seems redundant.

Addresses comments:
moby#15078 (comment)

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>

jheiss added a commit to jheiss/docker that referenced this pull request Mar 10, 2016

Remove redundant error messages
For operations on multi containers, we printed error for each
failed container, then printed an extra message for container
names, it seems redundant.

Addresses comments:
moby#15078 (comment)

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment