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

cluster: Only pass a join address when in the process of joining a cluster #33361

Merged
merged 1 commit into from Jun 14, 2017

Conversation

Projects
None yet
5 participants
@aaronlehmann
Contributor

aaronlehmann commented May 23, 2017

This code currently passes a random manager address when creating a new Node. This doesn't really make sense - we should only pass a join address on the initial join, or when retrying that join. An upcoming change to swarmkit will pay attention to JoinAddr when a node is already part of a cluster, so passing in the random value needs to be avoided.

Fixes #32980

cc @tonistiigi

cluster: Only pass a join address when in the process of joining a cl…
…uster

This code currently passes a random manager address when creating a new
Node. This doesn't really make sense - we should only pass a join
address on the initial join, or when retrying that join. An upcoming
change to swarmkit will pay attention to JoinAddr significant when a
node is already part of a cluster, so passing in the random value needs
to be avoided.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
@tonistiigi

This comment has been minimized.

Show comment
Hide comment
@tonistiigi

tonistiigi May 23, 2017

Member

LGTM

Member

tonistiigi commented May 23, 2017

LGTM

@mlaventure

LGTM

But I'd like for the test to be re-run given this is an old PR. I don't seem to be able to do so though. (cc @thaJeztah @tiborvass any idea why not?)

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 14, 2017

Member

No, no idea why it doesn't work, I think I've seen the same on some PR's. Let me try through Slack

Member

thaJeztah commented Jun 14, 2017

No, no idea why it doesn't work, I think I've seen the same on some PR's. Let me try through Slack

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 14, 2017

Member

Yup, works through slack. Odd

Member

thaJeztah commented Jun 14, 2017

Yup, works through slack. Odd

@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure

mlaventure Jun 14, 2017

Contributor

All 3 Janky errors seem unrelated:

15:44:58 ----------------------------------------------------------------------
15:44:58 FAIL: docker_cli_exec_test.go:226: DockerSuite.TestExecStopNotHanging
15:44:58 
15:44:58 docker_cli_exec_test.go:248:
15:44:58     c.Fatal("Container stop timed out")
15:44:58 ... Error: Container stop timed out
15:44:58 
15:44:59 
15:44:59 ----------------------------------------------------------------------

15:57:41 ----------------------------------------------------------------------
15:57:41 FAIL: docker_cli_start_test.go:94: DockerSuite.TestStartPausedContainer
15:57:41 
15:57:41 docker_cli_start_test.go:98:
15:57:41     runSleepingContainer(c, "-d", "--name", "testing")
15:57:41 /go/src/github.com/docker/docker/pkg/testutil/cmd/command.go:64:
15:57:41     t.Fatalf("at %s:%d - %s", filepath.Base(file), line, err.Error())
15:57:41 ... Error: at cli.go:48 - 
15:57:41 Command:  /usr/local/bin/docker run -d -d --name testing busybox top
15:57:41 ExitCode: 125
15:57:41 Error:    exit status 125
15:57:41 Stdout:   cdb603cce5eb040a3003afb27fdb892b359a1cf3e39f2f68301c34ab7f2a1842
15:57:41 
15:57:41 Stderr:   /usr/local/bin/docker: Error response from daemon: endpoint with name testing already exists in network bridge.
15:57:41 
15:57:41 
15:57:41 Failures:
15:57:41 ExitCode was 125 expected 0
15:57:41 Expected no error
15:57:41 
15:57:41 
15:57:43 
15:57:43 ----------------------------------------------------------------------

17:05:55 ----------------------------------------------------------------------
17:05:55 FAIL: docker_cli_external_volume_driver_unix_test.go:446: DockerExternalVolumeSuite.TestExternalVolumeDriverBindExternalVolume
17:05:55 
17:05:55 docker_cli_external_volume_driver_unix_test.go:448:
17:05:55     dockerCmd(c, "run", "-d", "--name", "testing", "-v", "foo:/bar", "busybox", "top")
17:05:55 /go/src/github.com/docker/docker/pkg/testutil/cmd/command.go:64:
17:05:55     t.Fatalf("at %s:%d - %s", filepath.Base(file), line, err.Error())
17:05:55 ... Error: at cli.go:48 - 
17:05:55 Command:  /usr/local/bin/docker run -d --name testing -v foo:/bar busybox top
17:05:55 ExitCode: 125
17:05:55 Error:    exit status 125
17:05:55 Stdout:   d906acd135e600278a4463e3b82d0039430be0652d6309752d6369e599f0397e
17:05:55 
17:05:55 Stderr:   /usr/local/bin/docker: Error response from daemon: endpoint with name testing already exists in network bridge.
17:05:55 
17:05:55 
17:05:55 Failures:
17:05:55 ExitCode was 125 expected 0
17:05:55 Expected no error
17:05:55 
17:05:55 
17:05:56 
17:05:56 ----------------------------------------------------------------------

going to restart it for good mesure.

Contributor

mlaventure commented Jun 14, 2017

All 3 Janky errors seem unrelated:

15:44:58 ----------------------------------------------------------------------
15:44:58 FAIL: docker_cli_exec_test.go:226: DockerSuite.TestExecStopNotHanging
15:44:58 
15:44:58 docker_cli_exec_test.go:248:
15:44:58     c.Fatal("Container stop timed out")
15:44:58 ... Error: Container stop timed out
15:44:58 
15:44:59 
15:44:59 ----------------------------------------------------------------------

15:57:41 ----------------------------------------------------------------------
15:57:41 FAIL: docker_cli_start_test.go:94: DockerSuite.TestStartPausedContainer
15:57:41 
15:57:41 docker_cli_start_test.go:98:
15:57:41     runSleepingContainer(c, "-d", "--name", "testing")
15:57:41 /go/src/github.com/docker/docker/pkg/testutil/cmd/command.go:64:
15:57:41     t.Fatalf("at %s:%d - %s", filepath.Base(file), line, err.Error())
15:57:41 ... Error: at cli.go:48 - 
15:57:41 Command:  /usr/local/bin/docker run -d -d --name testing busybox top
15:57:41 ExitCode: 125
15:57:41 Error:    exit status 125
15:57:41 Stdout:   cdb603cce5eb040a3003afb27fdb892b359a1cf3e39f2f68301c34ab7f2a1842
15:57:41 
15:57:41 Stderr:   /usr/local/bin/docker: Error response from daemon: endpoint with name testing already exists in network bridge.
15:57:41 
15:57:41 
15:57:41 Failures:
15:57:41 ExitCode was 125 expected 0
15:57:41 Expected no error
15:57:41 
15:57:41 
15:57:43 
15:57:43 ----------------------------------------------------------------------

17:05:55 ----------------------------------------------------------------------
17:05:55 FAIL: docker_cli_external_volume_driver_unix_test.go:446: DockerExternalVolumeSuite.TestExternalVolumeDriverBindExternalVolume
17:05:55 
17:05:55 docker_cli_external_volume_driver_unix_test.go:448:
17:05:55     dockerCmd(c, "run", "-d", "--name", "testing", "-v", "foo:/bar", "busybox", "top")
17:05:55 /go/src/github.com/docker/docker/pkg/testutil/cmd/command.go:64:
17:05:55     t.Fatalf("at %s:%d - %s", filepath.Base(file), line, err.Error())
17:05:55 ... Error: at cli.go:48 - 
17:05:55 Command:  /usr/local/bin/docker run -d --name testing -v foo:/bar busybox top
17:05:55 ExitCode: 125
17:05:55 Error:    exit status 125
17:05:55 Stdout:   d906acd135e600278a4463e3b82d0039430be0652d6309752d6369e599f0397e
17:05:55 
17:05:55 Stderr:   /usr/local/bin/docker: Error response from daemon: endpoint with name testing already exists in network bridge.
17:05:55 
17:05:55 
17:05:55 Failures:
17:05:55 ExitCode was 125 expected 0
17:05:55 Expected no error
17:05:55 
17:05:55 
17:05:56 
17:05:56 ----------------------------------------------------------------------

going to restart it for good mesure.

@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure

mlaventure Jun 14, 2017

Contributor

Alright, it passed, merging.

The other are flaky test. the TestStartPausedContainer seems to be an network issue though. I'll open an issue for it.

Contributor

mlaventure commented Jun 14, 2017

Alright, it passed, merging.

The other are flaky test. the TestStartPausedContainer seems to be an network issue though. I'll open an issue for it.

@mlaventure mlaventure merged commit c86323c into moby:master Jun 14, 2017

5 of 6 checks passed

windowsRS1 Jenkins build Docker-PRs-WoW-RS1 14934 has failed
Details
dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 35027 has succeeded
Details
janky Jenkins build Docker-PRs 43634 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 4005 has succeeded
Details
z Jenkins build Docker-PRs-s390x 3740 has succeeded
Details

@aaronlehmann aaronlehmann deleted the aaronlehmann:no-join-address branch Jul 12, 2017

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