-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Allow pods to use --net=none #9185
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon 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 |
test/e2e/pod_create_test.go
Outdated
session := podmanTest.Podman([]string{"run", "--pod", podName, ALPINE, "wget", "www.podman.io"}) | ||
session.WaitWithDefaultTimeout() | ||
Expect(session.ExitCode()).To(Equal(1)) | ||
Expect(session.ErrorToString()).To(ContainSubstring("wget: bad address 'www.podman.io'")) |
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 would recommend changing this test to ip -o -4 addr
This will print one network interface per line. In this case we should only have one, the loopback adapter. The current test could also fail because dns is not working.
would other namespaces have the same issue when shared with the host? |
@giuseppe What issue, specifically? Requiring a bool in the configuration to tell us to set them to Host? |
no nevermind, I thought the same issue could happen with other namespaces beside network, but that is not the case. LGTM |
/lgtm |
We need an extra field in the pod infra container config. We may want to reevaluate that struct at some point, as storing network modes as bools will rapidly become unsustainable, but that's a discussion for another time. Otherwise, straightforward plumbing. Fixes containers#9165 Signed-off-by: Matthew Heon <[email protected]>
/hold cancel |
Needs a fresh LGTM |
/lgtm |
We need an extra field in the pod infra container config. We may want to reevaluate that struct at some point, as storing network modes as bools will rapidly become unsustainable, but that's a discussion for another time. Otherwise, straightforward plumbing.
Fixes #9165