Skip to content

(windows) Pick a random host port if the user does not specify a host port. - #2368

Merged
mavenugo merged 1 commit into
moby:masterfrom
pradipd:WindowsPortMappingFix
Apr 23, 2019
Merged

(windows) Pick a random host port if the user does not specify a host port. #2368
mavenugo merged 1 commit into
moby:masterfrom
pradipd:WindowsPortMappingFix

Conversation

@pradipd

@pradipdpradipd commented Apr 19, 2019

Copy link
Copy Markdown
Contributor

For overlay, l2bridge, and l2tunnel, if the user does not specify a host port, windows driver will select a random port for them. This matches linux behavior.
For ics and nat networks the windows OS will choose the port.

Signed-off-by: Pradip Dhara pradipd@microsoft.com

@pradipd

pradipd commented Apr 19, 2019

Copy link
Copy Markdown
ContributorAuthor

@mavenugo PTAL.

@pradipd
pradipdforce-pushed the WindowsPortMappingFix branch 2 times, most recently from 7e6982e to 43a5636CompareApril 19, 2019 21:47
mavenugo
mavenugo reviewed Apr 21, 2019
View reviewed changes
Comment threadportmapper/mapper.go Outdated
mavenugo
mavenugo approved these changes Apr 21, 2019
View reviewed changes

@mavenugomavenugo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with a minor comment.

@pradipd can you pls address the minor comment ?

selansen
selansen reviewed Apr 21, 2019
View reviewed changes
Comment threadportallocator/portallocator_windows.go Outdated
@pradipd
Pick a random host port if the user does not specify a host port.
e6b4a7a
For overlay, l2bridge, and l2tunnel, if the user does not specify a host port, windows driver will select a random port for them. This matches linux behavior.
For ics and nat networks the windows OS will choose the port.
Signed-off-by: Pradip Dhara <pradipd@microsoft.com>
@pradipd
pradipdforce-pushed the WindowsPortMappingFix branch from 43a5636 to e6b4a7aCompareApril 22, 2019 17:43
@andrewhsu

andrewhsu commented Apr 22, 2019

Copy link
Copy Markdown
Contributor

Looks like unit tests still failing.

Looking to get this backported to bump_18.09 when this PR is ready.

@pradipd

pradipd commented Apr 23, 2019

Copy link
Copy Markdown
ContributorAuthor

I can't get a local repro.
I'm stuck on:
Could not get ipvs family information from the kernel. It is possible that ipvs is not enabled in your kernel. Native loadbalancing will not work until this
--- FAIL: TestService (0.00s)

I'm running on Ubuntu 16.04. What do I need to do to move past this?

@thaJeztah

thaJeztah commented Apr 23, 2019

Copy link
Copy Markdown
Member

Could someone restart CI on this one, or could this be an actual issue?

Makefile:123: recipe for target 'unit-tests' failed
make: *** [unit-tests] Terminated
Too long with no output (exceeded 10m0s)

If this is due to flakiness / slow runs, there's a configuration option to extend the timeout (see docker/cli#1750, where I raised it for docker/cli)

@mavenugo

mavenugo commented Apr 23, 2019

Copy link
Copy Markdown
Contributor

I restarted the CI again. The flakiness doesnt seem to be related. But let me give it another go.

@thaJeztah

thaJeztah commented Apr 23, 2019

Copy link
Copy Markdown
Member

CI is green now 👍

thaJeztah
thaJeztah reviewed Apr 23, 2019
View reviewed changes
Comment threadportallocator/portallocator_windows.go

const (
StartPortRange = 60000
EndPortRange = 65000

@thaJeztahthaJeztahApr 23, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like these don't have to be exported

Also; given that constants are assigned for these, I assume there's no configurable ephemeral port range on Windows (as there's on Linux)?

@thaJeztahthaJeztahApr 23, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like there is a configurable range; should that be used? https://support.microsoft.com/en-gb/help/929851/the-default-dynamic-port-range-for-tcp-ip-has-changed-in-windows-vista

@mavenugo
mavenugo merged commit 9ff9b57 into moby:masterApr 23, 2019
@thaJeztahthaJeztah mentioned this pull request Apr 23, 2019
Merged
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Apr 23, 2019
@thaJeztah
bump libnetwork 872f0a83c98add6cae255c8859e29532febc0039 (18.09 branch)
353796f
full diff: moby/libnetwork@c902989...872f0a8
- moby/libnetwork#2354 [18.09 backport] Cleanup the cluster provider when the agent is closed
- backport of moby/libnetwork#2307 Fix for problem where agent is stopped and does not restart
- fixesdocker/for-linux#495 Docker swarm overlay networking not working after --force-new-cluster
- moby/libnetwork#2369 [18.09 BACKPORT] Pick a random host port if the user does not specify a host port
- backport of moby/libnetwork#2368 (windows) Pick a random host port if the user does not specify a host port
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztahthaJeztah mentioned this pull request Apr 23, 2019
Closed
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Apr 23, 2019
@thaJeztah
bump libnetwork 9ff9b57c344df5cd47443ad9e65702ec85c5aeb0
7f2c564
full diff: moby/libnetwork@48f8463...9ff9b57
- moby/libnetwork#2368 (windows) Pick a random host port if the user does not specify a host port
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztahthaJeztah mentioned this pull request Apr 23, 2019
Merged
@andrewhsuandrewhsu mentioned this pull request Apr 23, 2019
Merged
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Apr 23, 2019
@thaJeztah@andrewhsu
bump libnetwork 872f0a83c98add6cae255c8859e29532febc0039 (18.09 branch)
bccb447
full diff: moby/libnetwork@c902989...872f0a8
- moby/libnetwork#2354 [18.09 backport] Cleanup the cluster provider when the agent is closed
- backport of moby/libnetwork#2307 Fix for problem where agent is stopped and does not restart
- fixesdocker/for-linux#495 Docker swarm overlay networking not working after --force-new-cluster
- moby/libnetwork#2369 [18.09 BACKPORT] Pick a random host port if the user does not specify a host port
- backport of moby/libnetwork#2368 (windows) Pick a random host port if the user does not specify a host port
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 5354408039681020f9ad6afe4bf696fc90f9ce69
Component: engine
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Apr 24, 2019
@thaJeztah
bump libnetwork 9ff9b57c344df5cd47443ad9e65702ec85c5aeb0
cf31da2
full diff: moby/libnetwork@48f8463...9ff9b57
- moby/libnetwork#2368 (windows) Pick a random host port if the user does not specify a host port
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 7f2c564b23377e1c69bc06e40fef1a459d6f51b5
Component: engine
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztahthaJeztahthaJeztah left review comments
+2 more reviewers
@selansenselansenselansen left review comments
@mavenugomavenugomavenugo approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pradipd@andrewhsu@thaJeztah@mavenugo@selansen