Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

[17.09] Use non-detached mode as default for service commands #230

Closed

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Sep 18, 2017

Backport of docker/cli#525 for 17.09

Commit 330a003 added a --detach=false option
to various service-related commands, with the intent to make this the default in
a future version (17.09).

This patch changes the default to use "interactive" (non-detached), allowing
users to override this by setting the --detach option.

To prevent problems when connecting to older daemon versions (17.05 and below,
see commit db60f255617fd90cb093813dcdfe7eec840eeff8), the detach option is
ignored for those versions, and detach is always true.

Before this change, a warning was printed to announce the upcoming default:

$ docker service create nginx:alpine
saxiyn3pe559d753730zr0xer
Since --detach=false was not specified, tasks will be created in the background.
In a future release, --detach=false will become the default.

After this change, no warning is printed, but --detach is disabled;

$ docker service create nginx:alpine
y9jujwzozi0hwgj5yaadzliq6
overall progress: 1 out of 1 tasks
1/1: running   [==================================================>]
verify: Service converged

Setting the --detach flag makes the cli use the pre-17.06 behavior:

$ docker service create --detach nginx:alpine
280hjnzy0wzje5o56gr22a46n

Running against a 17.03 daemon, without specifying the --detach flag;

$ docker service create nginx:alpine
kqheg7ogj0kszoa34g4p73i8q

Signed-off-by: Sebastiaan van Stijn github@gone.nl
(cherry picked from commit 0c27355f7b07943425d360d1345c625f0e60b8cf)

Commit 330a003 added a `--detach=false` option
to various service-related commands, with the intent to make this the default in
a future version (17.09).

This patch changes the default to use "interactive" (non-detached), allowing
users to override this by setting the `--detach` option.

To prevent problems when connecting to older daemon versions (17.05 and below,
see commit db60f255617fd90cb093813dcdfe7eec840eeff8), the detach option is
ignored for those versions, and detach is always true.

Before this change, a warning was printed to announce the upcoming default:

    $ docker service create nginx:alpine
    saxiyn3pe559d753730zr0xer
    Since --detach=false was not specified, tasks will be created in the background.
    In a future release, --detach=false will become the default.

After this change, no warning is printed, but `--detach` is disabled;

    $ docker service create nginx:alpine
    y9jujwzozi0hwgj5yaadzliq6
    overall progress: 1 out of 1 tasks
    1/1: running   [==================================================>]
    verify: Service converged

Setting the `--detach` flag makes the cli use the pre-17.06 behavior:

    $ docker service create --detach nginx:alpine
    280hjnzy0wzje5o56gr22a46n

Running against a 17.03 daemon, without specifying the `--detach` flag;

    $ docker service create nginx:alpine
    kqheg7ogj0kszoa34g4p73i8q

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 0c27355f7b07943425d360d1345c625f0e60b8cf)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@andrewhsu andrewhsu added this to the 17.09.0 milestone Sep 20, 2017
@vieux
Copy link
Contributor

vieux commented Sep 21, 2017

LGTM

Signed-off-by: Victor Vieux <victorvieux@gmail.com>
@andrewhsu
Copy link
Contributor

Seeing failures in the ce-tests build log:

00:20:39.722 --- FAIL: Test (332.98s)
00:20:39.722 panic: DockerTrustedSwarmSuite.TestTrustedServiceUpdate test timed out after 5m0s [recovered]
00:20:39.722 	panic: DockerTrustedSwarmSuite.TestTrustedServiceUpdate test timed out after 5m0s

Talked with @vieux about this one...may need to either adjust tests in moby/moby or create new tests in docker/cli. The test commit 8ee7a3b to just modify the one failing test from before did not seem to catch all situations this change in behaviour affects.

@andrewhsu andrewhsu modified the milestones: 17.09.0, 17.09.1 Oct 5, 2017
@andrewhsu andrewhsu modified the milestone: 17.09.1 Nov 28, 2017
@andrewhsu
Copy link
Contributor

this one did not make the cut for 17.09.0, so not introducing it in 17.09.1 because of the behaviour change it would introduce

@andrewhsu andrewhsu closed this Nov 30, 2017
@thaJeztah thaJeztah deleted the backport-change-detach-default branch November 30, 2017 01:07
@andrewhsu andrewhsu removed this from the 17.09.1 milestone Nov 30, 2017
docker-jenkins pushed a commit that referenced this pull request Sep 20, 2018
[18.09] systemd/docker.service: fix PATH
Upstream-commit: 1b2edd1ffda28df88692bb7faa16ac30f879c5b7
Component: packaging
docker-jenkins pushed a commit that referenced this pull request Jun 18, 2019
[18.09 backport] Consider WINDOWS_BASE_IMAGE_TAG override when setting Windows base image for tests
Upstream-commit: bb54c5bf2a3ab1c698fda6b3d67773d25755ae45
Component: engine
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants