-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
🐛 capd: don't add host ports to exposed ports of containers #10657
🐛 capd: don't add host ports to exposed ports of containers #10657
Conversation
This fixes an issue when running capd with podmans docker compatibility mode. And it's also incorrect in general.
/area provider/infrastructure-docker |
Can you please add the error when using CAPD with podman in the description? This helps to verify in future to verify if the same issue occurs again. |
done |
/test help |
@chrischdi: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/test pull-cluster-api-e2e-main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
I think this PR makes sense as is. Thanks @schrej !
Looks like podman is a bit more correct on its api / docker a bit more lazy :-)
LGTM label has been added. Git tree hash: 8de191905bd15b8f244d03ed32c42bf27846ba14
|
Thx! Looks good to me and tests are green so /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This fixes an issue when running capd with podmans docker compatibility mode. And it's also incorrect in general.
What this PR does / why we need it:
CAPD currently adds the host ports that are part of a port mapping to the list of 'exposed ports' of the container. This is incorrect and leads to an error when using CAPD with podman, since capd sets the host port to 0 so an available port is automatically selected.
There is no specific documentation around this that I could find. The closest is this: moby/moby#3039
This took way to long 😄
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #