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

Static IP and MAC addresses are allowed in rootless CNI #8198

Closed
wants to merge 1 commit into from
Closed

Static IP and MAC addresses are allowed in rootless CNI #8198

wants to merge 1 commit into from

Conversation

mheon
Copy link
Member

@mheon mheon commented Oct 30, 2020

Loosen some restrictions within specgen around rootlesscontainers - they can now set static IP and MAC addresses as long as they are in a CNI network.

Fixes #7842

@mheon
Static IP and MAC addresses are allowed in rootless CNI
c265403 
Loosen some restrictions within specgen around rootless
containers - they can now set static IP and MAC addresses as long
as they are in a CNI network.

Fixes containers#7842

Signed-off-by: Matthew Heon <[email protected]>
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 30, 2020
@mheon mheon mentioned this pull request Oct 30, 2020
Closed
giuseppe
giuseppe approved these changes Oct 30, 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

@rhatdan
Copy link
Member

rhatdan commented Oct 30, 2020

/lgtm
/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 30, 2020
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2020
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AkihiroSuda, giuseppe, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Luap99
Copy link
Member

Luap99 commented Oct 30, 2020

It doesn't work for me:

$ bin/podman network create --subnet 172.16.1.0/24 staticip
/home/paul/.config/cni/net.d/staticip.conflist
$ bin/podman run --network staticip --ip 172.16.1.5 alpine ip addr
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host 
       valid_lft forever preferred_lft forever
3: eth0@if6: <BROADCAST,MULTICAST,UP,LOWER_UP,M-DOWN> mtu 1500 qdisc noqueue state UP 
    link/ether 42:7a:c6:c6:7d:e1 brd ff:ff:ff:ff:ff:ff
    inet 172.16.1.2/24 brd 172.16.1.255 scope global eth0
       valid_lft forever preferred_lft forever
    inet6 fe80::407a:c6ff:fec6:7de1/64 scope link tentative 
       valid_lft forever preferred_lft forever
$ bin/podman version
Version:      2.2.0-dev
API Version:  2.0.0
Go Version:   go1.14.9
Git Commit:   c26540348f5681c0c3c550109f89ae8e8d2d9d82
Built:        Fri Oct 30 15:40:05 2020
OS/Arch:      linux/amd64

@mheon
Copy link
Member Author

mheon commented Oct 30, 2020

Hm. Confirmed - it looks like we are passing the static IP over, but CNI is not honoring it.

@mheon
Copy link
Member Author

mheon commented Oct 30, 2020

Actually, we may not be passing the IP at all. I don't see us generating a CNI config via getPodNetwork

@rhatdan rhatdan removed the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2020
@rhatdan
Copy link
Member

rhatdan commented Oct 30, 2020

Removed the LGTM flag, @mheon did you figure out what is wrong?

@mheon
Copy link
Member Author

mheon commented Oct 30, 2020

Still untangling - trying to figure out exactly how we're calling CNI such that we're not using the existing logic

@rhatdan
Copy link
Member

rhatdan commented Nov 23, 2020

@mheon What is going on with this PR?

@mheon
Copy link
Member Author

mheon commented Nov 23, 2020

I completely forgot it existed.

I'll take another look when I get back next week.

@zhangguanzhang
Copy link
Collaborator

I think must enable some test in files:
https://github.com/containers/podman/blob/master/test/e2e/create_staticip_test.go
https://github.com/containers/podman/blob/master/test/e2e/create_staticmac_test.go

@Luap99
Copy link
Member

Luap99 commented Nov 24, 2020

Replaced by #8469

@Luap99 Luap99 closed this Nov 24, 2020
@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

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@giuseppe giuseppe giuseppe approved these changes

Assignees

@rhatdan rhatdan

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"rootless containers and pods cannot be assigned static IP addresses" (podman-run, rootless, CNI)
7 participants
@mheon @rhatdan @openshift-ci-robot @Luap99 @zhangguanzhang @giuseppe @AkihiroSuda