podman: add support for specifying MAC #4451

giuseppe · 2019-11-05T14:28:12Z

rebase of #4058

Signed-off-by: Jakub Filak [email protected]
Signed-off-by: Giuseppe Scrivano [email protected]

openshift-ci-robot · 2019-11-05T14:28:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [giuseppe]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

giuseppe · 2019-11-05T16:51:01Z

@rhatdan I've tried vendor-in-container that I copied from buildah but there is no difference

vrothberg · 2019-11-06T08:04:40Z

@rhatdan I've tried vendor-in-container that I copied from buildah but there is no difference

libpod did not migrate to using go 1.13 for the vendor checks yet. You can run the following command and commit the changes which should unblock this PR:

$ podman run --privileged --rm --env HOME=/root --env GOPROXY=https://proxy.golang.org -v `pwd`:/src -w /src golang:1.12 make vendor

giuseppe · 2019-11-06T10:21:30Z

$ podman run --privileged --rm --env HOME=/root --env GOPROXY=https://proxy.golang.org -v pwd:/src -w /src golang:1.12 make vendor

thanks for the black magic! It did the trick. I've updated the patch that adds vendor-in-container

Signed-off-by: Giuseppe Scrivano <[email protected]>

`go get github.com/cri-o/ocicni@deac903fd99b6c52d781c9f42b8db3af7dcfd00a` I had to fix compilation errors in libpod/networking_linux.go --- ocicni.Networks has changed from string to the structure NetAttachment with the member Name (the former string value) and the member Ifname (optional). I don't think we can make use of Ifname here, so I just map the array of structures to array of strings - e.g. dropping Ifname. --- The function GetPodNetworkStatus no longer returns Result but it returns the wrapper structure NetResult which contains the former Result plus NetAttachment (Network name and Interface name). Again, I don't think we can make use of that information here, so I just added `.Result` to fix the build. --- Issue: containers#1136 Signed-off-by: Jakub Filak <[email protected]>

I basically copied and adapted the statements for setting IP. Closes containers#1136 Signed-off-by: Jakub Filak <[email protected]>

Signed-off-by: Giuseppe Scrivano <[email protected]>

rhatdan · 2019-11-06T15:30:31Z

Makefile

 		$(GO) mod verify

+vendor-in-container:
+	podman run --privileged --rm --env HOME=/root -v `pwd`:/src -w /src docker.io/library/golang:1.12 make vendor


We want to move everyone to golang 1.13.

@vrothberg Correct?

we need first to update the CI image, if I use golang:1.13 the vendor check fails

The CI is currently using 1.12 already which means we can't use 1.13 on our dev machines. The change here is not needed.

giuseppe · 2019-11-06T17:45:52Z

tests are passing

giuseppe · 2019-11-07T08:36:01Z

for reviewers: I've changed this chunk: https://github.com/containers/libpod/pull/4451/files#diff-39e56de7ab505f865271f02541bd0857R840-R855

rhatdan · 2019-11-07T13:41:07Z

LGTM
Really need @mheon to review.
@filak-sap PTAL

mheon · 2019-11-07T14:47:17Z

test/e2e/checkpoint_test.go

 		IPBefore.WaitWithDefaultTimeout()
 		Expect(IPBefore.ExitCode()).To(Equal(0))

+		MACBefore := podmanTest.Podman([]string{"inspect", "-l", "--format={{.NetworkSettings.MacAddress}}"})


Can we get a non-checkpoint test? Set --mac-address ab:cd:ef:gh and verify that it's set in the container?

mheon · 2019-11-07T14:47:32Z

Code LGTM but I would like another test

Signed-off-by: Giuseppe Scrivano <[email protected]>

giuseppe · 2019-11-07T19:17:40Z

Code LGTM but I would like another test

test added

mheon · 2019-11-07T19:24:09Z

LGTM

mheon · 2019-11-07T19:24:15Z

/lgtm

rhatdan · 2019-11-07T22:21:03Z

Awesome job @filak-sap and @giuseppe
Thanks for getting this done.

filak-sap · 2019-11-08T06:51:32Z

Thank you @giuseppe ! BTW What did I do wrong?

openshift-ci-robot requested review from jwhonce and mrunalp November 5, 2019 14:28

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 5, 2019

giuseppe mentioned this pull request Nov 5, 2019

[wip] podman: add support for specifying MAC #4058

Closed

giuseppe force-pushed the set-mac branch 2 times, most recently from 7bc500c to 88b88d9 Compare November 5, 2019 15:27

openshift-ci-robot added size/XL and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 5, 2019

giuseppe force-pushed the set-mac branch from 88b88d9 to 5e841b1 Compare November 5, 2019 15:46

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL labels Nov 5, 2019

giuseppe force-pushed the set-mac branch 2 times, most recently from b6d6bdf to ed27f2f Compare November 5, 2019 16:50

giuseppe force-pushed the set-mac branch 2 times, most recently from 74236df to e5ddfe5 Compare November 5, 2019 18:29

openshift-ci-robot added size/XL and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 5, 2019

giuseppe force-pushed the set-mac branch from e5ddfe5 to 9065a52 Compare November 5, 2019 20:39

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL labels Nov 5, 2019

giuseppe force-pushed the set-mac branch from 9065a52 to 0865c51 Compare November 6, 2019 09:07

openshift-ci-robot added size/XL and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 6, 2019

giuseppe force-pushed the set-mac branch 2 times, most recently from cc6a756 to e49488f Compare November 6, 2019 11:50

giuseppe force-pushed the set-mac branch from e49488f to 65efb2c Compare November 6, 2019 11:55

giuseppe mentioned this pull request Nov 6, 2019

Add support for make vendor-in-container #4453

Merged

giuseppe and others added 4 commits November 6, 2019 16:22

Makefile: add vendor-in-container

16cb2b3

Signed-off-by: Giuseppe Scrivano <[email protected]>

podman: add support for specifying MAC

2497b6c

I basically copied and adapted the statements for setting IP. Closes containers#1136 Signed-off-by: Jakub Filak <[email protected]>

cni: enable tuning plugin

e379f7e

Signed-off-by: Giuseppe Scrivano <[email protected]>

giuseppe force-pushed the set-mac branch from 65efb2c to e379f7e Compare November 6, 2019 15:22

rhatdan reviewed Nov 6, 2019

View reviewed changes

mheon reviewed Nov 7, 2019

View reviewed changes

test: add tests for --mac-address

82e4116

Signed-off-by: Giuseppe Scrivano <[email protected]>

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL labels Nov 7, 2019

openshift-ci-robot assigned mheon Nov 7, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 7, 2019

openshift-merge-robot merged commit d919961 into containers:master Nov 7, 2019

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 26, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2023

podman: add support for specifying MAC #4451

podman: add support for specifying MAC #4451

Uh oh!

Conversation

giuseppe commented Nov 5, 2019

Uh oh!

openshift-ci-robot commented Nov 5, 2019

Uh oh!

giuseppe commented Nov 5, 2019

Uh oh!

vrothberg commented Nov 6, 2019

Uh oh!

giuseppe commented Nov 6, 2019

Uh oh!

rhatdan Nov 6, 2019

Choose a reason for hiding this comment

Uh oh!

giuseppe Nov 6, 2019

Choose a reason for hiding this comment

Uh oh!

vrothberg Nov 6, 2019

Choose a reason for hiding this comment

Uh oh!

giuseppe commented Nov 6, 2019

Uh oh!

giuseppe commented Nov 7, 2019

Uh oh!

rhatdan commented Nov 7, 2019

Uh oh!

mheon Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

mheon commented Nov 7, 2019

Uh oh!

giuseppe commented Nov 7, 2019

Uh oh!

mheon commented Nov 7, 2019

Uh oh!

mheon commented Nov 7, 2019

Uh oh!

rhatdan commented Nov 7, 2019

Uh oh!

filak-sap commented Nov 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants