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

Check for swarm-mode network conflict during create network #24431

Merged
merged 2 commits into from
Jul 8, 2016

Conversation

mavenugo
Copy link
Contributor

@mavenugo mavenugo commented Jul 7, 2016

Reverts #24158 which causes service create to fail if it is attached to multiple networks. We need to find another better way to handle the case of failing network connect/disconnect for managed containers.

Fixes #23983

@GordonTheTurtle GordonTheTurtle added status/0-triage dco/no Automatically set by a bot when one of the commits lacks proper signature labels Jul 7, 2016
This reverts commit 0ce5158.

Signed-off-by: Madhu Venugopal <madhu@docker.com>
Signed-off-by: Madhu Venugopal <madhu@docker.com>
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jul 7, 2016
@mavenugo mavenugo changed the title Revert Check for swarm-mode network conflict during create network Jul 7, 2016
@tiborvass
Copy link
Contributor

@mavenugo can you clarify the situation? The issue this pr is supposed to fix, is already closed. Should we reopen it?

@mavenugo
Copy link
Contributor Author

mavenugo commented Jul 8, 2016

@tiborvass #24158 solves #23983. But along with the correct patch, I incorrectly added another fix which caused an issue wherein a service fails if it has multiple networks.

With this PR, am retaining just the fix that is required to solve #23983, but removing the incorrect patch that is causing the breakage.

@tiborvass
Copy link
Contributor

LGTM

@tiborvass tiborvass added this to the 1.12.0 milestone Jul 8, 2016
@thaJeztah
Copy link
Member

Should we have a test?

@mavenugo
Copy link
Contributor Author

mavenugo commented Jul 8, 2016

@thaJeztah since this involves swarm-mode & there is a bigger effort on introducing proper IT for swarm-mode, I was hoping to add it as part of that. Will that work ?
Yes. this particular case can be tested with a simple 1 node cluster. But so are many other swarm-mode tests.

@thaJeztah
Copy link
Member

alright, as long as we don't forget

LGTM

@thaJeztah thaJeztah merged commit e10c11e into moby:master Jul 8, 2016
@mavenugo
Copy link
Contributor Author

mavenugo commented Jul 8, 2016

@tiborvass just to confirm ... the Cherry-pick should only be done on the the commit : 6a4b21b . The other one is a revert and hence it can be safely ignored.

@tiborvass
Copy link
Contributor

@mavenugo it's automated so it will cherrypick both.

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

Successfully merging this pull request may close these issues.

None yet

4 participants