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

Fix issues of multiple published ports mapping to the same target port #29787

Merged
merged 2 commits into from
Jan 10, 2017

Conversation

yongtang
Copy link
Member

@yongtang yongtang commented Dec 30, 2016

This fix tries to address the issue raised in #29730 where a service with multiple published ports mapping to the same target port (e.g., --publish 5000:80 --publish 5001:80) can't be allocated.

The reason for the issue is that, getPortConfigKey is used for both allocated ports and configured (may or may not be allocated) ports. However, getPortConfigKey will not take into consideration the PublishedPort field, which actually could be different for different allocated ports.

This fix saves a map of portKey:portNum:portState, instead of currently used portKey:portState so that multiple published ports could be processed.

An integration test has been added to address moby/swarmkit#1835 (comment)

See moby/swarmkit#1835 for changes in SwarmKit.

This fix fixes #29730

Signed-off-by: Yong Tang yong.tang.github@outlook.com

This fix tries to address the issue raised in docker/docker-29730
where a service with multiple published ports mapping to the same target
port (e.g., `--publish 5000:80 --publish 5001:80`) can't be allocated.

The reason for the issue is that, `getPortConfigKey` is used for both
allocated ports and configured (may or may not be allocated) ports.
However, `getPortConfigKey` will not take into consideration the
`PublishedPort` field, which actually could be different for different
allocated ports.

This fix saves a map of `portKey:portNum:portState`,  instead of currently
used `portKey:portState` so that multiple published ports could be processed.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
This commit updates swarmkit to c971468

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the 29730-multiple-published-port branch from 98edb4c to f3a274f Compare January 10, 2017 11:47
@yongtang
Copy link
Member Author

Updated the PR to vendor c971468.

/cc @aaronlehmann @thaJeztah @aboch Please take a look.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🦁

@vdemeester vdemeester merged commit 82dfa65 into moby:master Jan 10, 2017
@GordonTheTurtle GordonTheTurtle added this to the 1.14.0 milestone Jan 10, 2017
@yongtang yongtang deleted the 29730-multiple-published-port branch January 10, 2017 23:37
@thaJeztah thaJeztah modified the milestones: 1.13.1, 1.14.0 Jan 18, 2017
@thaJeztah thaJeztah modified the milestones: 17.03.0, 1.13.1 Feb 18, 2017
@thaJeztah
Copy link
Member

cherry-picked through #31143

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.

[1.13.x] same container port published twice prevent service to run
4 participants