Skip to content

PublishAllPorts: create port mappings for exposed ports#51586

Merged
akerouanton merged 1 commit into
moby:masterfrom
robmry:ep_options_exposed_ports
Nov 25, 2025
Merged

PublishAllPorts: create port mappings for exposed ports#51586
akerouanton merged 1 commit into
moby:masterfrom
robmry:ep_options_exposed_ports

Conversation

@robmry

@robmry robmry commented Nov 24, 2025

Copy link
Copy Markdown
Contributor

- What I did

The -P option to publish exposed ports doesn't work for Windows containers in 29.x.

Before 4239806, buildPortsRelatedCreateEndpointOptions created ports entries for each of c.Config.ExposedPorts. So, they were included in the endpoint options if c.HostConfig.PublishAllPorts.

Restore that behaviour.

- How I did it

- How to verify it

Run a Windows container with exposed ports and -P ... check that docker ps shows the published ports.

(Needs an integration test.)

- Human readable description for the release notes

- fix `--publish-all` / `-P` for Windows containers

Signed-off-by: Rob Murray <rob.murray@docker.com>
@robmry robmry added this to the 29.1.0 milestone Nov 24, 2025
@robmry robmry requested a review from akerouanton November 25, 2025 09:36
@robmry robmry marked this pull request as ready for review November 25, 2025 09:37
@robmry robmry requested a review from corhere November 25, 2025 12:20
@akerouanton akerouanton merged commit 03797ac into moby:master Nov 25, 2025
262 of 264 checks passed
@thaJeztah

Copy link
Copy Markdown
Member

Whoops! Curious; why did it only fail on Windows?

@robmry

robmry commented Nov 26, 2025

Copy link
Copy Markdown
Contributor Author

Whoops! Curious; why did it only fail on Windows?

Yeah - it's surprising! I think we'll need to come back to do some tidying.

The bridge driver uses Sandbox options to set up the port mappings, they're set up in buildSandboxOptions and it included the ExposedPorts.

But Windows uses endpoint options, and they're set up separately in this buildPortsRelatedCreateEndpointOptions.

I've not examined the differences, but it seems likely we're unnecessarily doing stuff twice/differently.

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.

5 participants