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.

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

Fix creation of macvlan interfaces #11663

Merged
merged 2 commits into from
Nov 2, 2016

Conversation

danwinship
Copy link
Contributor

@danwinship danwinship commented Oct 31, 2016

We need to create the macvlan in the default namespace (so it can refer to the default interface) and then move it.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1389717 (bug 1389717)

@dcbw / @openshift/networking PTAL

Copy link
Contributor

@knobunc knobunc left a comment

Choose a reason for hiding this comment

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

LGTM

@knobunc
Copy link
Contributor

knobunc commented Oct 31, 2016

[test]

@danwinship
Copy link
Contributor Author

pushed another commit; we still had macvlan-related code in the shell script even though it never got used any more

@pravisankar
Copy link

LGTM

@knobunc
Copy link
Contributor

knobunc commented Nov 1, 2016

[test] flake seems to be the same as #11685

@danwinship
Copy link
Contributor Author

flake #10336, [test]

LinkAttrs: netlink.LinkAttrs{
MTU: defIface.Attrs().MTU,
Name: "macvlan0",
ParentIndex: defIface.Attrs().Index,
Copy link
Contributor

Choose a reason for hiding this comment

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

You can give LinkAdd() the netns like so:

Namespace:   netlink.NsFd(int(ns.Fd())),

and it'll get put into the namespace after it's created. Then I'd do "ns.Do()" for the LinkByName and LinkSetUp.

The CNI plugin implements this internally, and was never passing an
8th arg to the script.
@danwinship
Copy link
Contributor Author

@dcbw: updated
@knobunc: this will need a new merge tag after dcbw reviews it, because I deleted your previous one

@dcbw
Copy link
Contributor

dcbw commented Nov 2, 2016

@danwinship needs a gofmt on pkg/sdn/plugin/pod_linux.go

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 620dae3

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10992/) (Base Commit: 6932bcc)

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS

@knobunc
Copy link
Contributor

knobunc commented Nov 2, 2016

[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 2, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (Image: devenv-rhel7_5305)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 620dae3

@openshift-bot openshift-bot merged commit d0fd557 into openshift:master Nov 2, 2016
@danwinship danwinship deleted the fix-macvlan branch November 3, 2016 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants