daemon: Add wildcard support to --devices ("eth+") #15697

joamaki · 2021-04-14T17:18:51Z

This adds support for device name wildcards to the "--devices"
option, e.g. if user specifies "--devices=eth+,en+" the agent
will match devices eth0, eth1, en0, etc.

Fixes: #15693

joamaki · 2021-04-14T17:41:15Z

test-me-please

brb · 2021-04-19T09:30:58Z

daemon/cmd/kube_proxy_replacement_test.go

-	c.Assert(detectDevices(true, true), IsNil)
-	c.Assert(option.Config.Devices, checker.DeepEquals, []string{"dummy1"})
-	c.Assert(option.Config.DirectRoutingDevice, Equals, "dummy1")
+	act()


Is it leftover?

Which one? act()? This function abstracted the namespace setup and act is the actual test function invoked in the new fresh namespace. Perhaps it could use a better name?

Where is it defined?

The diff looks a bit funny as I moved a chunk of code around. Perhaps have a look at the whole file? I added withFreshNetNS function that takes a function as argument and it takes care of setting up and tearing down the namespace around the act() function call.

Aha, I see it now. Maybe s/act/test/?

joamaki · 2021-04-20T13:03:24Z

test-me-please

daemon/cmd/kube_proxy_replacement.go

brb

The approach looks good!

A few things:

My comment re duplicates ^^.
This needs a doc change https://github.com/cilium/cilium/blob/master/Documentation/gettingstarted/kubeproxy-free.rst (rendered https://docs.cilium.io/en/v1.9/gettingstarted/kubeproxy-free/).

brb · 2021-04-22T09:47:47Z

daemon/cmd/kube_proxy_replacement.go

+	if err != nil {
+		log.WithError(err).Fatal("Cannot list network devices via netlink")
+	}
+	var expandedDevices []string


I'd use map (e.g. map[string]struct{}) instead to avoid duplicates.

Added the deduplication and tests for it.

brb · 2021-04-22T09:48:46Z

daemon/cmd/kube_proxy_replacement_test.go

-	c.Assert(detectDevices(true, true), IsNil)
-	c.Assert(option.Config.Devices, checker.DeepEquals, []string{"dummy1"})
-	c.Assert(option.Config.DirectRoutingDevice, Equals, "dummy1")
+	act()


Where is it defined?

brb · 2021-04-22T13:03:57Z

Documentation/gettingstarted/kubeproxy-free.rst

+listed device has to be named the same on all Cilium managed nodes. Alternatively
+if the devices do not match across different nodes, the wildcard option can be 
+used, e.g. ``devices=eth+``, which would match any device starting with prefix
+``eth``.


Maybe add that if no device can be matched, then cilium-agent will try to perform the auto detection.

joamaki · 2021-04-26T09:36:41Z

test-me-please

joamaki · 2021-04-27T07:17:02Z

test-net-next

brb

🚀

This adds support for device name wildcards to the "--devices" option, e.g. if user specifies "--devices=eth+,en+" the agent will match devices eth0, eth1, en0, etc. Fixes: cilium#15693 Signed-off-by: Jussi Maki <jussi@isovalent.com>

aanm · 2021-04-27T17:49:52Z

test-gke

aanm · 2021-04-27T17:49:57Z

test-runtime

aanm · 2021-04-27T17:50:07Z

test-1.20-4.19

aanm · 2021-04-27T17:50:13Z

test-1.21-4.9

joamaki · 2021-04-28T10:47:55Z

test-net-next

joamaki · 2021-04-28T13:42:17Z

test-gke

joamaki · 2021-04-28T14:14:11Z

test-runtime

joamaki · 2021-04-28T17:26:23Z

test-1.21-4.9

joamaki · 2021-04-28T17:26:31Z

test-1.20-4.19

joestringer

LGTM. An interesting followup would be to allow this to dynamically apply to new devices as they are attached (common in some cloud environments like EKS), but that's more of a "future wishlist" item. In the mean time if users are concerned about matching dynamic devices, they can restart the agent.

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 14, 2021

maintainer-s-little-helper bot added this to In progress in 1.10.0 Apr 14, 2021

joamaki requested a review from brb April 14, 2021 17:18

maintainer-s-little-helper bot assigned brb Apr 14, 2021

joamaki added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Apr 14, 2021

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 14, 2021

brb reviewed Apr 19, 2021

View reviewed changes

joamaki force-pushed the pr/joamaki/devices-wildcard branch from 3d09b54 to 7e2bb90 Compare April 20, 2021 08:54

joamaki marked this pull request as ready for review April 20, 2021 13:03

joamaki requested a review from a team April 20, 2021 13:03

aanm reviewed Apr 20, 2021

View reviewed changes

daemon/cmd/kube_proxy_replacement.go Show resolved Hide resolved

joamaki force-pushed the pr/joamaki/devices-wildcard branch from 7e2bb90 to ff3261e Compare April 21, 2021 08:08

aanm approved these changes Apr 21, 2021

View reviewed changes

joamaki force-pushed the pr/joamaki/devices-wildcard branch from ff3261e to a2fbd38 Compare April 22, 2021 08:25

brb requested changes Apr 22, 2021

View reviewed changes

joamaki force-pushed the pr/joamaki/devices-wildcard branch 2 times, most recently from 0b6b6c6 to 676ed2d Compare April 22, 2021 12:04

joamaki requested a review from a team as a code owner April 22, 2021 12:04

joamaki requested review from a team and qmonnet April 22, 2021 12:04

maintainer-s-little-helper bot assigned qmonnet Apr 22, 2021

joamaki force-pushed the pr/joamaki/devices-wildcard branch from 676ed2d to 7238631 Compare April 22, 2021 13:00

brb reviewed Apr 22, 2021

View reviewed changes

qmonnet approved these changes Apr 22, 2021

View reviewed changes

qmonnet removed their assignment Apr 22, 2021

joamaki force-pushed the pr/joamaki/devices-wildcard branch from 7238631 to 868a007 Compare April 26, 2021 08:14

joamaki force-pushed the pr/joamaki/devices-wildcard branch from 868a007 to 524ed64 Compare April 27, 2021 07:02

brb approved these changes Apr 27, 2021

View reviewed changes

daemon: Add wildcard support to --devices ("eth+")

db613b4

This adds support for device name wildcards to the "--devices" option, e.g. if user specifies "--devices=eth+,en+" the agent will match devices eth0, eth1, en0, etc. Fixes: cilium#15693 Signed-off-by: Jussi Maki <jussi@isovalent.com>

aanm added the needs-backport/1.10 label Apr 27, 2021

joamaki force-pushed the pr/joamaki/devices-wildcard branch from 524ed64 to db613b4 Compare April 28, 2021 08:34

brb removed their assignment Apr 28, 2021

joestringer approved these changes Apr 28, 2021

View reviewed changes

joestringer merged commit efb3a72 into cilium:master Apr 28, 2021

1.10.0 automation moved this from In progress to Done Apr 28, 2021

christarazi mentioned this pull request Apr 28, 2021

v1.10 backports 2021-04-28 #15919

Merged

christarazi added backport-pending/1.10 and removed needs-backport/1.10 labels Apr 28, 2021

maintainer-s-little-helper bot added this to Backport pending to v1.10 in 1.10.0-rc2 Apr 28, 2021

joestringer added backport-done/1.10 and removed backport-pending/1.10 labels May 6, 2021

maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.0-rc2 May 6, 2021

aanm mentioned this pull request May 17, 2021

Prepare for release v1.10.0-rc2 #16167

Merged

brb mentioned this pull request Aug 9, 2021

Cilium does not recognize alternative network interface names #17066

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

daemon: Add wildcard support to --devices ("eth+") #15697

daemon: Add wildcard support to --devices ("eth+") #15697

joamaki commented Apr 14, 2021

joamaki commented Apr 14, 2021

brb Apr 19, 2021

joamaki Apr 19, 2021

brb Apr 22, 2021

joamaki Apr 22, 2021

brb Apr 22, 2021

joamaki commented Apr 20, 2021

brb left a comment

brb Apr 22, 2021

joamaki Apr 22, 2021

brb Apr 22, 2021

brb Apr 22, 2021

joamaki Apr 26, 2021

joamaki commented Apr 26, 2021

joamaki commented Apr 27, 2021

brb left a comment

aanm commented Apr 27, 2021

aanm commented Apr 27, 2021

aanm commented Apr 27, 2021

aanm commented Apr 27, 2021

joamaki commented Apr 28, 2021

joamaki commented Apr 28, 2021

joamaki commented Apr 28, 2021

joamaki commented Apr 28, 2021

joamaki commented Apr 28, 2021

joestringer left a comment

daemon: Add wildcard support to --devices ("eth+") #15697

daemon: Add wildcard support to --devices ("eth+") #15697

Conversation

joamaki commented Apr 14, 2021

joamaki commented Apr 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joamaki commented Apr 20, 2021

brb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joamaki commented Apr 26, 2021

joamaki commented Apr 27, 2021

brb left a comment

Choose a reason for hiding this comment

aanm commented Apr 27, 2021

aanm commented Apr 27, 2021

aanm commented Apr 27, 2021

aanm commented Apr 27, 2021

joamaki commented Apr 28, 2021

joamaki commented Apr 28, 2021

joamaki commented Apr 28, 2021

joamaki commented Apr 28, 2021

joamaki commented Apr 28, 2021

joestringer left a comment

Choose a reason for hiding this comment