Skip to content
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.

Sign up for GitHub

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

Jump to bottom

rootless: support podman network create (CNI-in-slirp4netns) #7460

Merged
merged 2 commits into from
Sep 10, 2020
Merged

rootless: support podman network create (CNI-in-slirp4netns) #7460

merged 2 commits into from
Sep 10, 2020

Conversation

AkihiroSuda
Copy link
Collaborator

@AkihiroSuda AkihiroSuda commented Aug 26, 2020 •
edited
Loading

Fix #6863 , #1733

This PR allows using CNI networks for rootless, by running CNI plugins inside a sandbox container that is running with slirp4netns.

See https://github.com/AkihiroSuda/podman-network-create-for-rootless-podman for the design.

Usage is same as Rootful Podman:

$ podman network create foo
/home/user/.config/cni/net.d/foo.conflist
$ podman run -d --name web --hostname web --network foo nginx:alpine
$ podman run --rm --network foo alpine wget -O - http://web.dns.podman
Connecting to web.dns.podman (10.88.4.6:80)
...
<h1>Welcome to nginx!</h1>
...

Still lacks:

  • podman run -p (EDIT: now implemented)
  • deallocating unused netns (EDIT: now implemented)

TODO: move ghcr.io/akihirosuda/podman-rootless-cni-infra to official quay

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 26, 2020
This was referenced Aug 26, 2020
Closed
Closed
@AkihiroSuda AkihiroSuda force-pushed the allow-rootless-cni branch 3 times, most recently from 70004d5 to 1715184 Compare September 2, 2020 03:05
@AkihiroSuda AkihiroSuda mentioned this pull request Sep 2, 2020
Closed
@AkihiroSuda AkihiroSuda force-pushed the allow-rootless-cni branch 6 times, most recently from 59128c5 to b459f6f Compare September 7, 2020 11:50
@AkihiroSuda AkihiroSuda marked this pull request as ready for review September 7, 2020 11:50
@AkihiroSuda AkihiroSuda changed the title [WIP] rootless: support podman network create (CNI-in-slirp4netns) rootless: support podman network create (CNI-in-slirp4netns) Sep 7, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 7, 2020
@AkihiroSuda AkihiroSuda added CNI Bug with CNI networking for root containers kind/feature Categorizes issue or PR as related to a new feature. rootless slirp4netns Bug is in slirp4netns labels Sep 7, 2020
@AkihiroSuda
Copy link
Collaborator Author

Ready for review.

@giuseppe @rhatdan @mheon PTAL.

@AkihiroSuda AkihiroSuda force-pushed the allow-rootless-cni branch 5 times, most recently from f942857 to 2335d46 Compare September 8, 2020 06:40
@AkihiroSuda AkihiroSuda force-pushed the allow-rootless-cni branch 5 times, most recently from 7b98f87 to 01ba1a5 Compare September 8, 2020 10:14
@AkihiroSuda
Copy link
Collaborator Author

CI green

@AkihiroSuda
add contrib/rootless-cni-infra
d34868a 
Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda
rootless: support podman network create (CNI-in-slirp4netns)
f82abc7 
Usage:
```
$ podman network create foo
$ podman run -d --name web --hostname web --network foo nginx:alpine
$ podman run --rm --network foo alpine wget -O - http://web.dns.podman
Connecting to web.dns.podman (10.88.4.6:80)
...
<h1>Welcome to nginx!</h1>
...
```

See contrib/rootless-cni-infra for the design.

Signed-off-by: Akihiro Suda <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Sep 9, 2020

/approve
LGTM
Thanks @AkihiroSuda This looks very good.

I would like @giuseppe and @mheon to do final review.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 9, 2020
giuseppe
giuseppe approved these changes Sep 10, 2020
View reviewed changes
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AkihiroSuda, giuseppe, rhatdan

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mheon
mheon reviewed Sep 10, 2020
View reviewed changes
libpod/rootless_cni_linux.go
var rootlessCNIInfraImage = map[string]string{
// Built from ../contrib/rootless-cni-infra
// TODO: move to Podman's official quay
"amd64": "ghcr.io/akihirosuda/podman-rootless-cni-infra:gd34868a13-amd64",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomSweeneyRedHat Do you own the Quay? I want to make sure we have an action item to get this moved once it merges

mheon
mheon reviewed Sep 10, 2020
View reviewed changes
libpod/runtime_ctr.go

// GetContainersWithoutLock is same as GetContainers but without lock
func (r *Runtime) GetContainersWithoutLock(filters ...ContainerFilter) ([]*Container, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: The runtime lock is actually not really useful anymore - we can probably just remove it. No action needed in this PR, I'll make a fresh one for it.

@mheon
Copy link
Member

mheon commented Sep 10, 2020

Very nice work @AkihiroSuda - this should make rootless a lot easier to work with!

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 10, 2020
@openshift-merge-robot openshift-merge-robot merged commit 2f0e803 into containers:master Sep 10, 2020
@AkihiroSuda AkihiroSuda mentioned this pull request Sep 12, 2020
Closed
@burhan
Copy link

burhan commented Sep 15, 2020

Is this going to fix the issue where a rootless container cannot ping a server running on the host? The host is running postgresql, the rootless container wants to talk to it, but cannot "Connection Refused". Just want to know if this will address that, or do I need to raise a separate issue.

@AkihiroSuda
Copy link
Collaborator Author

Unrelated, please feel free to open an issue (with detailed reproducer)

@burhan
Copy link

burhan commented Sep 16, 2020

Thanks @AkihiroSuda, raised #7649 for the specific case I am seeing.

@muayyad-alsadi muayyad-alsadi mentioned this pull request Nov 5, 2020
Open
@simonkrenger simonkrenger mentioned this pull request Nov 18, 2020
Closed
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@mheon mheon mheon left review comments

@giuseppe giuseppe giuseppe approved these changes

@rhatdan rhatdan Awaiting requested review from rhatdan

Assignees

@mheon mheon

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. CNI Bug with CNI networking for root containers kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. rootless slirp4netns Bug is in slirp4netns
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

userspace network between containers
8 participants
@AkihiroSuda @rhatdan @openshift-ci-robot @mheon @burhan @giuseppe @TomSweeneyRedHat @openshift-merge-robot