fix: allow network device selector to match multiple links

Fixes #7673

Signed-off-by: Andrey Smirnov <[email protected]>

hack/release.toml

@@ -39,6 +39,14 @@ Talos is built with Go 1.21.
 The command `images` deprecated in Talos 1.5 was removed, please use `talosctl images default` instead.
 """
 
+    [notes.device-selectors]
+        title = "Network Device Selectors"
+        description = """\
+Previously, [network device selectors](https://www.talos.dev/v1.6/talos-guides/network/device-selector/) only matched the first link, now the configuration is applied to all matching links.
+"""
+
+
+
 [make_deps]
 
     [make_deps.tools]

internal/app/machined/pkg/controllers/network/device_config.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 
 	"github.com/cosi-project/runtime/pkg/controller"
-	"github.com/cosi-project/runtime/pkg/resource"
 	"github.com/cosi-project/runtime/pkg/safe"
 	"github.com/cosi-project/runtime/pkg/state"
 	glob "github.com/ryanuber/go-glob"
@@ -100,28 +99,21 @@ func (ctrl *DeviceConfigController) Run(ctx context.Context, r controller.Runtim
 		r.StartTrackingOutputs()
 
 		if cfgProvider != nil && cfgProvider.Machine() != nil {
-			selectedInterfaces := map[string]struct{}{}
-
 			for index, device := range cfgProvider.Machine().Network().Devices() {
+				out := []talosconfig.Device{device}
+
 				if device.Selector() != nil {
-					dev := device.(*v1alpha1.Device).DeepCopy()
-					device = dev
+					var matched []*v1alpha1.Device
 
-					err = ctrl.getDeviceBySelector(dev, links)
+					matched, err = ctrl.getDevicesBySelector(device, links)
 					if err != nil {
 						logger.Warn("failed to select an interface for a device", zap.Error(err))
 
 						continue
 					}
 
-					if _, ok := selectedInterfaces[device.Interface()]; ok {
-						return fmt.Errorf("the device %s is already configured by a selector", device.Interface())
-					}
-
-					selectedInterfaces[device.Interface()] = struct{}{}
-				}
-
-				if device.Bond() != nil && len(device.Bond().Selectors()) > 0 {
+					out = slices.Map(matched, func(device *v1alpha1.Device) talosconfig.Device { return device })
+				} else if device.Bond() != nil && len(device.Bond().Selectors()) > 0 {
 					dev := device.(*v1alpha1.Device).DeepCopy()
 					device = dev
 
@@ -131,22 +123,29 @@ func (ctrl *DeviceConfigController) Run(ctx context.Context, r controller.Runtim
 
 						continue
 					}
-				}
 
-				id := fmt.Sprintf("%s/%03d", device.Interface(), index)
+					out = []talosconfig.Device{device}
+				}
 
-				config := network.NewDeviceConfig(id, device)
+				for j, outDevice := range out {
+					id := fmt.Sprintf("%s/%03d", outDevice.Interface(), index)
 
-				if err = r.Modify(
-					ctx,
-					config,
-					func(r resource.Resource) error {
-						r.(*network.DeviceConfigSpec).TypedSpec().Device = device
+					if len(out) > 1 {
+						id = fmt.Sprintf("%s/%03d", id, j)
+					}
 
-						return nil
-					},
-				); err != nil {
-					return err
+					if err = safe.WriterModify(
+						ctx,
+						r,
+						network.NewDeviceConfig(id, outDevice),
+						func(r *network.DeviceConfigSpec) error {
+							r.TypedSpec().Device = outDevice
+
+							return nil
+						},
+					); err != nil {
+						return err
+					}
 				}
 			}
 		}
@@ -157,19 +156,22 @@ func (ctrl *DeviceConfigController) Run(ctx context.Context, r controller.Runtim
 	}
 }
 
-func (ctrl *DeviceConfigController) getDeviceBySelector(device *v1alpha1.Device, links safe.List[*network.LinkStatus]) error {
+func (ctrl *DeviceConfigController) getDevicesBySelector(device talosconfig.Device, links safe.List[*network.LinkStatus]) ([]*v1alpha1.Device, error) {
 	selector := device.Selector()
 
 	matches := ctrl.selectDevices(selector, links)
 	if len(matches) == 0 {
-		return fmt.Errorf("no matching network device for defined selector: %+v", selector)
+		return nil, fmt.Errorf("no matching network device for defined selector: %+v", selector)
 	}
 
-	link := matches[0]
+	out := make([]*v1alpha1.Device, len(matches))
 
-	device.DeviceInterface = link.Metadata().ID()
+	for i, link := range matches {
+		out[i] = device.(*v1alpha1.Device).DeepCopy()
+		out[i].DeviceInterface = link.Metadata().ID()
+	}
 
-	return nil
+	return out, nil
 }
 
 func (ctrl *DeviceConfigController) expandBondSelector(device *v1alpha1.Device, links safe.List[*network.LinkStatus]) error {

internal/app/machined/pkg/controllers/network/device_config_test.go

@@ -81,13 +81,33 @@ func (suite *DeviceConfigSpecSuite) TestSelectors() {
 		MachineConfig: &v1alpha1.MachineConfig{
 			MachineNetwork: &v1alpha1.NetworkConfig{
 				NetworkInterfaces: []*v1alpha1.Device{
+					// device selector selecing a single interface
 					{
 						DeviceSelector: &v1alpha1.NetworkDeviceSelector{
 							NetworkDeviceKernelDriver: kernelDriver,
 						},
 						DeviceAddresses: []string{"192.168.2.0/24"},
 						DeviceMTU:       1500,
 					},
+					// no device selector (explicit name)
+					{
+						DeviceInterface: "eth0",
+						DeviceAddresses: []string{"192.168.3.0/24"},
+					},
+					// device selector which doesn't match anything
+					{
+						DeviceSelector: &v1alpha1.NetworkDeviceSelector{
+							NetworkDeviceKernelDriver: "no-match",
+						},
+						DeviceAddresses: []string{"192.168.4.0/24"},
+					},
+					// device selector which matches multiple interfaces
+					{
+						DeviceSelector: &v1alpha1.NetworkDeviceSelector{
+							NetworkDeviceBus: "0000:01*",
+						},
+						DeviceAddresses: []string{"192.168.5.0/24"},
+					},
 				},
 			},
 		},
@@ -98,9 +118,11 @@ func (suite *DeviceConfigSpecSuite) TestSelectors() {
 
 	status := network.NewLinkStatus(network.NamespaceName, "eth0")
 	status.TypedSpec().Driver = kernelDriver
+	status.TypedSpec().BusPath = "0000:01:00.0"
 	suite.Require().NoError(suite.State().Create(suite.Ctx(), status))
 
 	status = network.NewLinkStatus(network.NamespaceName, "eth1")
+	status.TypedSpec().BusPath = "0000:01:01.0"
 	suite.Require().NoError(suite.State().Create(suite.Ctx(), status))
 
 	rtestutils.AssertResources(suite.Ctx(), suite.T(), suite.State(), []string{"eth0/000"},
@@ -109,6 +131,18 @@ func (suite *DeviceConfigSpecSuite) TestSelectors() {
 			assert.Equal([]string{"192.168.2.0/24"}, r.TypedSpec().Device.Addresses())
 		},
 	)
+
+	rtestutils.AssertResources(suite.Ctx(), suite.T(), suite.State(), []string{"eth0/001"},
+		func(r *network.DeviceConfigSpec, assert *assert.Assertions) {
+			assert.Equal([]string{"192.168.3.0/24"}, r.TypedSpec().Device.Addresses())
+		},
+	)
+
+	rtestutils.AssertResources(suite.Ctx(), suite.T(), suite.State(), []string{"eth0/003/000", "eth1/003/001"},
+		func(r *network.DeviceConfigSpec, assert *assert.Assertions) {
+			assert.Equal([]string{"192.168.5.0/24"}, r.TypedSpec().Device.Addresses())
+		},
+	)
 }
 
 func (suite *DeviceConfigSpecSuite) TestBondSelectors() {

website/content/v1.6/talos-guides/network/device-selector.md

@@ -25,7 +25,7 @@ Selector has the following traits:
 - qualifiers match a device by reading the hardware information in `/sys/class/net/...`
 - qualifiers are applied using logical `AND`
 - `machine.network.interfaces.deviceConfig` option is mutually exclusive with `machine.network.interfaces.interface`
-- the selector is invalid when it matches multiple devices, the controller will fail and won't create any devices for the malformed selector
+- if the selector matches multiple devices, the controller will apply config to all of them
 
 The available hardware information used in the selector can be observed in the `LinkStatus` resource (works in maintenance mode):
 
@@ -57,10 +57,3 @@ machine:
 ```
 
 In this example, the `bond0` interface will be created and bonded using two devices with the specified hardware addresses.
-
-## Use Case
-
-`machine.network.interfaces.interface` name is generated by the Linux kernel and can be changed after a reboot.
-Device names can change when the system has several interfaces of the same kind, e.g: `eth0`, `eth1`.
-
-In that case pinning it to `hardwareAddress` will make Talos reliably configure the device even when interface name changes.