-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Connect container run to kind network as bridge #3407
Connect container run to kind network as bridge #3407
Conversation
… make podman cry, due to the docker network connect call Signed-off-by: Matthias Wessendorf <[email protected]>
Hi @matzew. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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/test-infra repository. |
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
unclear why the error is happening but the flag change / step 4 removal seems OK to me.
/ok-to-test |
@@ -6,7 +6,7 @@ reg_name='kind-registry' | |||
reg_port='5001' | |||
if [ "$(docker inspect -f '{{.State.Running}}' "${reg_name}" 2>/dev/null || true)" != 'true' ]; then | |||
docker run \ | |||
-d --restart=always -p "127.0.0.1:${reg_port}:5000" --name "${reg_name}" \ | |||
-d --restart=always -p "127.0.0.1:${reg_port}:5000" --net=kind --name "${reg_name}" \ |
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.
this doesn't work reliably because the network doesn't exist yet if you've never created any clusters, which is why we use "network connect"
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.
We could, perhaps run this container in bridge
mode too? Like the kind-control-plane
container?
I think the podman team is not allowing to "mix" network modes, see containers/podman#13683 (comment)
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.
How would we do that? Creating another throwaway network for the registry?
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.
Specifying --network bridge
is probably OK, I think docker always has this network but it's not clear if podman does or we'd have to create it.
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.
I think the bridge
network will be there. It's well documented
https://github.com/containers/podman/blob/main/docs/tutorials/basic_networking.md#bridge
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.
I updated the PR to this
@@ -43,13 +43,7 @@ for node in $(kind get nodes); do | |||
EOF | |||
done | |||
|
|||
# 4. Connect the registry to the cluster network if not already connected | |||
# This allows kind to bootstrap the network but ensures they're on the same network |
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.
(this comment explains why the script works this way)
we can expand this script to support podman but the current approach will break lots of people running this in clean CI environments
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.
with --network bridge
as stated above, this section is not being removed
We usually use Ideally we'd auto detect and match but that's a chicken and egg. We could check |
Signed-off-by: Matthias Wessendorf <[email protected]>
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
/approve
thanks!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder, matzew 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 |
I am coming to this b/c I ran into the Error: "slirp4netns" is not supported: invalid network mode problem.
Than I saw the comment here
Which made me think to update the example script and do the
run
with a--net
. this seems to also make thedocker network connect
not needed (at least forpodman
this is fine).UPDATE:
After discussing on the PR I updated the script and we
run
the regisrty by default asbridge
network mode, which is supported by both:docker
andpodman