feat(run): add support for --expose and --publish-all#5036
feat(run): add support for --expose and --publish-all#5036Mujib-Ahasan wants to merge 2 commits into
--expose and --publish-all#5036Conversation
| // publish is defined as StringSlice, not StringArray, to allow specifying "--publish=80:80,443:443" (compatible with Podman) | ||
| cmd.Flags().StringSliceP("publish", "p", nil, "Publish a container's port(s) to the host") | ||
| cmd.Flags().StringSlice("expose", nil, "Expose a port or a range of ports") | ||
| cmd.Flags().BoolP("publish-all", "P", false, "Publish all exposed ports to random ports") |
There was a problem hiding this comment.
Please update the docs too
nerdctl/docs/command-reference.md
Lines 457 to 460 in 4608753
There was a problem hiding this comment.
@AkihiroSuda Since nerdctl doesnot implement Docker content trust, would you accept --disable-content-trust as a compatibility (no operation) similar to Podman: https://docs.podman.io/en/latest/search.html?q=--disable-content-trust.
There was a problem hiding this comment.
@AkihiroSuda Since nerdctl doesnot implement Docker content trust, would you accept
--disable-content-trustas a compatibility (no operation) similar to Podman: https://docs.podman.io/en/latest/search.html?q=--disable-content-trust.
Doesn't seem relevant to --expose and --publish-all. Please open a new issue.
We have to consider what it means with cosign.
| UTSNamespace string | ||
| // PortMappings specifies a list of ports to publish from the container to the host | ||
| PortMappings []cni.PortMapping | ||
| ExposedPorts []string |
There was a problem hiding this comment.
Needs a code comment. How does it relate to PortMappings?
| // PortMappings specifies a list of ports to publish from the container to the host | ||
| PortMappings []cni.PortMapping | ||
| ExposedPorts []string | ||
| PublishAll bool |
Doesn't seem covered in this PR? |
will add this shortly! |
Mujib-Ahasan
force-pushed
the
expose-publish-all-flag
branch
2 times, most recently
from
2212a8e to
e38f368
Compare
| } | ||
|
|
||
| func TestRunExposeOnly(t *testing.T) { | ||
| base := testutil.NewBase(t) |
There was a problem hiding this comment.
NewBase is deprecated
Signed-off-by: Mujib Ahasan <ahasanmujib8@gmail.com>
Signed-off-by: Mujib Ahasan <ahasanmujib8@gmail.com>
Mujib-Ahasan
force-pushed
the
expose-publish-all-flag
branch
from
e38f368 to
6c2a114
Compare
2 participants
Description
This PR adds support for the Docker compatible
--exposeand--publish-all(-P) flags tonerdctl runandnerdctl create.Previously, nerdctl supported explicit port publishing through
--publish(-p), but did not support declaring additional exposed ports with--exposeor automatically publishing all exposed ports using-P. With this change, users can expose additional container ports via the CLI, and-Pwill publish both image defined (EXPOSE) and cli defined (--expose) ports to automatically allocated host ports by reusing the existing port mapping implementation. This improves Docker CLI compatibility while preserving the existing behaviors.Fixes: #4689