Configure NetworkManager while it is still starting

Two ways a NetworkManager host could end up with nothing written to disk,
both hit at once by an EL10 image whose only backend is NetworkManager.

Backend detection waits for the daemon to finish starting before it will
register the backend, and gave up on it when that wait ran out. The
Startup property stays true for as long as a device is still working
through its initial activation, so a VM whose DHCP request nothing
answers holds it true indefinitely -- the state a host is in precisely
when it is about to be given a static address. Detection then reported a
NetworkManager host as having no network configuration backend at all,
which on an image with no network-scripts, networkd, or netplan is fatal.
The wait still runs, since a daemon that has settled will not rewrite the
change as it finishes starting, but a daemon that owns its bus name is
configured either way.

The write paths then matched a profile to an interface only by
connection.interface-name. NetworkManager's own default wired connection
-- the one it creates for a device with no profile of its own, which is
exactly the case on an image whose baked profile does not match the VM's
hardware address -- names no interface at all, so every write skipped it
and silently persisted nothing. When no profile names the interface, the
profile the device is actually running and any profile pinned to its
hardware address are matched instead. Profiles bound by name are matched
first and alone, so a host configured the ordinary way is unaffected and
pays neither lookup.

networkManager.go

@@ -115,13 +115,11 @@ func (*networkManager) ParseConnection(settings gonetworkmanager.ConnectionSetti
 	}
 	conn.ID = id
 
-	// Get the interface name.
+	// Get the interface name. A profile need not name one: NetworkManager binds
-	name, ok := settings["connection"]["interface-name"].(string)
+	// its own default wired connections to a device by hardware address instead,
-	if !ok {
+	// and those are still connections this configures. Callers that need a name
-		err = fmt.Errorf("failed to get interface name")
+	// check for an empty one.
-		return
+	conn.Name, _ = settings["connection"]["interface-name"].(string)
-	}
-	conn.Name = name
 
 	// Get the addressing method of each family. These decide whether a DHCP
 	// client runs, independently of any static addresses parsed below.
@@ -297,6 +295,145 @@ func (*networkManager) ParseConnection(settings gonetworkmanager.ConnectionSetti
 	return
 }
 
+// nmTarget is a saved connection that configures an interface, paired with the
+// id nmcli is driven with.
+type nmTarget struct {
+	ID       string
+	Settings gonetworkmanager.ConnectionSettings
+}
+
+// nmSettingsMAC returns the hardware address a profile is pinned to, or an empty
+// string when it is pinned to none. D-Bus reports mac-address as a byte array,
+// but a profile read back from a keyfile can surface it already printed, so both
+// are accepted.
+func nmSettingsMAC(settings gonetworkmanager.ConnectionSettings) string {
+	for _, group := range []string{"802-3-ethernet", "802-11-wireless"} {
+		switch mac := settings[group]["mac-address"].(type) {
+		case string:
+			return mac
+		case []byte:
+			if len(mac) != 0 {
+				return net.HardwareAddr(mac).String()
+			}
+		}
+	}
+	return ""
+}
+
+// nmActiveConnectionUUID returns the uuid of the profile NetworkManager has
+// active on iface. An interface with nothing active on it, and a host whose
+// nmcli cannot answer, both return an empty string: this only ever adds
+// candidates to a match, so failing to resolve it costs nothing beyond what
+// the interface name alone would have found.
+func nmActiveConnectionUUID(ctx context.Context, iface string) string {
+	out, err := runCommand(ctx, "nmcli", "-g", "GENERAL.CON-UUID", "device", "show", iface)
+	if err != nil {
+		return ""
+	}
+	for _, line := range out {
+		line = strings.TrimSpace(line)
+		// nmcli prints "--" for a device that is not connected.
+		if line != "" && line != "--" {
+			return line
+		}
+	}
+	return ""
+}
+
+// nmDeviceMAC returns the hardware address of iface as the running system
+// reports it, or an empty string when there is no such device.
+func nmDeviceMAC(iface string) string {
+	link, err := netlink.LinkByName(iface)
+	if err != nil {
+		return ""
+	}
+	return link.Attrs().HardwareAddr.String()
+}
+
+// connectionsFor returns the saved connections that configure iface.
+//
+// A profile that names an interface is bound to it and is the whole answer when
+// one exists. NetworkManager also binds profiles to a device by other means:
+// the default wired connection it creates for a device with no profile of its
+// own names no interface at all, and a profile can be pinned to a hardware
+// address instead. Matching on the name alone left a host whose only profile
+// was one of those with nothing to write to -- the change applied to the
+// running system and disappeared on the next reboot -- so when nothing names
+// the interface, the profile the device is actually running and any profile
+// pinned to its hardware address are matched instead.
+//
+// The fallback is only reached when the name matches nothing, so a host whose
+// profiles are named in the ordinary way pays neither of its lookups.
+func (nm *networkManager) connectionsFor(ctx context.Context, iface string) ([]nmTarget, error) {
+	connections, err := nm.config.ListConnections()
+	if err != nil {
+		return nil, err
+	}
+
+	// Read every profile once; both passes below work from the same snapshot.
+	var all []nmTarget
+	for _, c := range connections {
+		settings, serr := c.GetSettings()
+		if serr != nil {
+			return nil, serr
+		}
+		id, ok := settings["connection"]["id"].(string)
+		if !ok || id == "" {
+			// Without an id there is nothing to point nmcli at.
+			continue
+		}
+		all = append(all, nmTarget{ID: id, Settings: settings})
+	}
+
+	// Profiles bound to the interface by name.
+	matched := matchByIfaceName(all, iface)
+	if len(matched) != 0 {
+		return matched, nil
+	}
+
+	// Nothing names it: resolve what the device is running and what it is, and
+	// match on those instead.
+	matched = matchByDevice(all, nmActiveConnectionUUID(ctx, iface), nmDeviceMAC(iface))
+	if len(matched) == 0 {
+		logger.Printf("NetworkManager has no connection profile for %s; nothing to persist to", iface)
+	}
+	return matched, nil
+}
+
+// matchByIfaceName returns the profiles bound to iface by name.
+func matchByIfaceName(all []nmTarget, iface string) []nmTarget {
+	var matched []nmTarget
+	for _, t := range all {
+		if name, _ := t.Settings["connection"]["interface-name"].(string); name == iface {
+			matched = append(matched, t)
+		}
+	}
+	return matched
+}
+
+// matchByDevice returns the profiles bound to a device by something other than
+// its name: the one it is currently running, and any pinned to its hardware
+// address. A profile that names an interface is bound there and is never a
+// candidate here, whichever interface that is. Either identifier may be empty
+// on a host where it could not be resolved, which matches nothing rather than
+// everything.
+func matchByDevice(all []nmTarget, activeUUID, mac string) []nmTarget {
+	var matched []nmTarget
+	for _, t := range all {
+		if name, _ := t.Settings["connection"]["interface-name"].(string); name != "" {
+			continue
+		}
+		if uuid, _ := t.Settings["connection"]["uuid"].(string); activeUUID != "" && uuid == activeUUID {
+			matched = append(matched, t)
+			continue
+		}
+		if pinned := nmSettingsMAC(t.Settings); mac != "" && pinned != "" && strings.EqualFold(pinned, mac) {
+			matched = append(matched, t)
+		}
+	}
+	return matched
+}
+
 // nmBusName is the well-known D-Bus name the NetworkManager daemon takes once
 // it is running. Until some process owns it, there is nothing on the bus to
 // answer a configuration call.
@@ -313,6 +450,18 @@ func nmBusNameOwned(conn *dbus.Conn) (bool, error) {
 	return owned, err
 }
 
+// nmOnBus reports whether the NetworkManager daemon owns its bus name, opening
+// the system bus to ask. It answers the narrower question networkManagerReady
+// asks first: whether there is a daemon to configure at all, regardless of how
+// far along its startup is.
+func nmOnBus() (bool, error) {
+	conn, err := dbus.SystemBus()
+	if err != nil {
+		return false, err
+	}
+	return nmBusNameOwned(conn)
+}
+
 // networkManagerReady reports whether NetworkManager is on the bus and has
 // finished starting up. The two are distinct: the daemon takes its bus name
 // early, then spends a while bringing up the connections it is configured to
@@ -323,11 +472,7 @@ func nmBusNameOwned(conn *dbus.Conn) (bool, error) {
 // because a host early enough in boot to beat NetworkManager can also be early
 // enough to beat dbus-daemon.
 func networkManagerReady() (bool, error) {
-	conn, err := dbus.SystemBus()
+	owned, err := nmOnBus()
-	if err != nil {
-		return false, err
-	}
-	owned, err := nmBusNameOwned(conn)
 	if err != nil || !owned {
 		return false, err
 	}
@@ -348,9 +493,27 @@ func networkManagerReady() (bool, error) {
 // configured through a running daemon, so a configurator built while it is
 // still starting would hold a settings handle that reports no connections and
 // accepts no changes.
+//
+// Running out of that wait is not by itself a reason to give up on the backend.
+// The Startup property stays true for as long as any device is still working
+// through its initial activation, and a device retrying a lease no DHCP server
+// will ever answer holds it true indefinitely -- a state a host is most likely
+// to be in precisely when someone is about to give it a static address. What
+// the wait buys is a daemon that will not rewrite the change as it finishes
+// starting; a daemon that is on the bus is still configurable without it, and
+// dropping the backend instead would report a NetworkManager host as having
+// nowhere to persist its network configuration at all.
 func newNetworkManager(ctx context.Context, readyTimeout time.Duration) (nm *networkManager, err error) {
-	if err = waitForServiceReady(ctx, "NetworkManager", readyTimeout, networkManagerReady); err != nil {
+	if waitErr := waitForServiceReady(ctx, "NetworkManager", readyTimeout, networkManagerReady); waitErr != nil {
-		return nil, err
+		// A cancelled caller is not waiting for an answer any more.
+		if ctx.Err() != nil {
+			return nil, waitErr
+		}
+		owned, busErr := nmOnBus()
+		if busErr != nil || !owned {
+			return nil, waitErr
+		}
+		logger.Printf("NetworkManager has not finished starting up; configuring it anyway: %v", waitErr)
 	}
 
 	config, err := gonetworkmanager.NewSettings()
@@ -403,6 +566,13 @@ func (nm *networkManager) GetInterfaces() (interfaces []*Interface, err error) {
 			continue
 		}
 
+		// A profile that names no interface cannot be reported as one: there is
+		// no device name to key the merge by. It is still written to, which
+		// connectionsFor resolves separately.
+		if connection.Name == "" {
+			continue
+		}
+
 		// Find the MAC address and link.
 		var mac net.HardwareAddr
 		var foundLink netlink.Link
@@ -468,38 +638,16 @@ func (nm *networkManager) SetIfaceAddresses(ctx context.Context, iface string, a
 		}
 	}
 
-	// Get connections from NM.
+	// Find the connections that configure the interface.
-	connections, err := nm.config.ListConnections()
+	targets, err := nm.connectionsFor(ctx, iface)
 	if err != nil {
 		return err
 	}
 
 	var errs []error
 
-	// Find the connections that has the interfaces.
+	for _, t := range targets {
-	for _, c := range connections {
+		id, settings := t.ID, t.Settings
-		// Get the settings of the connection.
-		var settings gonetworkmanager.ConnectionSettings
-		settings, err = c.GetSettings()
-		if err != nil {
-			return err
-		}
-
-		// Get the interface name.
-		name, ok := settings["connection"]["interface-name"].(string)
-		if !ok {
-			continue
-		}
-		if name != iface {
-			continue
-		}
-
-		// Get the interface id.
-		id, ok := settings["connection"]["id"].(string)
-		if !ok {
-			err = fmt.Errorf("failed to get interface id")
-			return
-		}
 
 		// Read the current addressing method of each family. NetworkManager
 		// serves static addresses alongside a lease when the method is "auto",
@@ -546,34 +694,16 @@ func (nm *networkManager) SetIfaceAddresses(ctx context.Context, iface string, a
 
 // Set the DHCP client state on an interface.
 func (nm *networkManager) SetIfaceDHCP(ctx context.Context, iface string, dhcp4, dhcp6 bool) (err error) {
-	// Get connections from NM.
+	// Find the connections that configure the interface.
-	connections, err := nm.config.ListConnections()
+	targets, err := nm.connectionsFor(ctx, iface)
 	if err != nil {
 		return err
 	}
 
 	var errs []error
 
-	// Find the connections that has the interfaces.
+	for _, t := range targets {
-	for _, c := range connections {
+		id, settings := t.ID, t.Settings
-		// Get the settings of the connection.
-		var settings gonetworkmanager.ConnectionSettings
-		settings, err = c.GetSettings()
-		if err != nil {
-			return err
-		}
-
-		// Get the interface name.
-		name, ok := settings["connection"]["interface-name"].(string)
-		if !ok || name != iface {
-			continue
-		}
-
-		// Get the interface id.
-		id, ok := settings["connection"]["id"].(string)
-		if !ok {
-			return fmt.Errorf("failed to get interface id")
-		}
 
 		// Parse the connection so turning a lease off can fall back to the
 		// method that still serves whatever static addresses it holds.
@@ -634,38 +764,16 @@ func (nm *networkManager) SetIfaceRoutes(ctx context.Context, iface string, rout
 		}
 	}
 
-	// Get connections from NM.
+	// Find the connections that configure the interface.
-	connections, err := nm.config.ListConnections()
+	targets, err := nm.connectionsFor(ctx, iface)
 	if err != nil {
 		return err
 	}
 
 	var errs []error
 
-	// Find the connections that has the interfaces.
+	for _, t := range targets {
-	for _, c := range connections {
+		id := t.ID
-		// Get the settings of the connection.
-		var settings gonetworkmanager.ConnectionSettings
-		settings, err = c.GetSettings()
-		if err != nil {
-			return err
-		}
-
-		// Get the interface name.
-		name, ok := settings["connection"]["interface-name"].(string)
-		if !ok {
-			continue
-		}
-		if name != iface {
-			continue
-		}
-
-		// Get the interface id.
-		id, ok := settings["connection"]["id"].(string)
-		if !ok {
-			err = fmt.Errorf("failed to get interface id")
-			return
-		}
 
 		// Update routes.
 		_, err = runCommand(ctx, "nmcli", "connection", "modify", id, "ipv4.routes", strings.Join(routes4, ","))
@@ -699,38 +807,16 @@ func (nm *networkManager) SetIfaceDNS(ctx context.Context, iface string, servers
 	}
 	search := strings.Join(searchDomains, ",")
 
-	// Get connections from NM.
+	// Find the connections that configure the interface.
-	connections, err := nm.config.ListConnections()
+	targets, err := nm.connectionsFor(ctx, iface)
 	if err != nil {
 		return err
 	}
 
 	var errs []error
 
-	// Find the connections that has the interfaces.
+	for _, t := range targets {
-	for _, c := range connections {
+		id := t.ID
-		// Get the settings of the connection.
-		var settings gonetworkmanager.ConnectionSettings
-		settings, err = c.GetSettings()
-		if err != nil {
-			return err
-		}
-
-		// Get the interface name.
-		name, ok := settings["connection"]["interface-name"].(string)
-		if !ok {
-			continue
-		}
-		if name != iface {
-			continue
-		}
-
-		// Get the interface id.
-		id, ok := settings["connection"]["id"].(string)
-		if !ok {
-			err = fmt.Errorf("failed to get interface id")
-			return
-		}
 
 		// Update DNS servers and search domains for each family. Also disable
 		// automatic DNS from DHCP/RA so the static servers are the only

networkManager_test.go

@@ -443,3 +443,87 @@ func TestNMDNSParsing(t *testing.T) {
 	assert.Emptyf(t, nmNameservers(map[string]any{}), "empty group dns, want empty")
 	assert.Emptyf(t, nmSearchDomains(map[string]any{}), "empty group dns-search, want empty")
 }
+
+// nmConn builds a profile for the matching tests. An empty interface name or
+// mac leaves that property off the profile entirely, the way NetworkManager
+// reports one that is not bound by it.
+func nmConn(id, uuid, iface, mac string) nmTarget {
+	conn := map[string]any{"id": id, "uuid": uuid}
+	if iface != "" {
+		conn["interface-name"] = iface
+	}
+	ethernet := map[string]any{}
+	if mac != "" {
+		ethernet["mac-address"] = []byte(parseMAC(mac))
+	}
+	return nmTarget{ID: id, Settings: gonetworkmanager.ConnectionSettings{
+		"connection":     conn,
+		"802-3-ethernet": ethernet,
+	}}
+}
+
+func parseMAC(s string) net.HardwareAddr {
+	mac, _ := net.ParseMAC(s)
+	return mac
+}
+
+// A profile that names the interface is bound to it, and is the whole answer
+// when one exists.
+func TestMatchByIfaceName(t *testing.T) {
+	all := []nmTarget{
+		nmConn("eth0", "uuid-eth0", "eth0", ""),
+		nmConn("eth1", "uuid-eth1", "eth1", ""),
+		nmConn("Wired connection 1", "uuid-wired", "", "52:54:00:34:7a:66"),
+	}
+	matched := matchByIfaceName(all, "eth0")
+	require.Len(t, matched, 1)
+	assert.Equal(t, "eth0", matched[0].ID)
+
+	assert.Empty(t, matchByIfaceName(all, "eth2"))
+}
+
+// The fallback covers the profiles a device runs without being named by one:
+// NetworkManager's default wired connection, and a profile pinned to the
+// device's hardware address.
+func TestMatchByDevice(t *testing.T) {
+	wired := nmConn("Wired connection 1", "uuid-wired", "", "")
+	pinned := nmConn("static", "uuid-static", "", "52:54:00:34:7A:66")
+	other := nmConn("eth1", "uuid-eth1", "eth1", "52:54:00:34:7a:66")
+	all := []nmTarget{wired, pinned, other}
+
+	// The active profile matches even though it names nothing.
+	matched := matchByDevice(all, "uuid-wired", "")
+	require.Len(t, matched, 1)
+	assert.Equal(t, "Wired connection 1", matched[0].ID)
+
+	// A profile pinned to the device's address matches, case-insensitively.
+	matched = matchByDevice(all, "", "52:54:00:34:7a:66")
+	require.Len(t, matched, 1)
+	assert.Equal(t, "static", matched[0].ID)
+
+	// Both identifiers together match each profile once.
+	matched = matchByDevice(all, "uuid-wired", "52:54:00:34:7a:66")
+	require.Len(t, matched, 2)
+
+	// A profile bound to another interface by name is never a candidate, and
+	// identifiers that could not be resolved match nothing rather than
+	// everything.
+	assert.Empty(t, matchByDevice(all, "", ""))
+}
+
+// The hardware address a profile is pinned to is read from either the byte
+// array D-Bus reports or the printed form a keyfile can surface.
+func TestNMSettingsMAC(t *testing.T) {
+	bytes := gonetworkmanager.ConnectionSettings{
+		"802-3-ethernet": {"mac-address": []byte(parseMAC("52:54:00:34:7a:66"))},
+	}
+	assert.Equal(t, "52:54:00:34:7a:66", nmSettingsMAC(bytes))
+
+	printed := gonetworkmanager.ConnectionSettings{
+		"802-3-ethernet": {"mac-address": "52:54:00:34:7a:66"},
+	}
+	assert.Equal(t, "52:54:00:34:7a:66", nmSettingsMAC(printed))
+
+	assert.Empty(t, nmSettingsMAC(gonetworkmanager.ConnectionSettings{"connection": {"id": "x"}}))
+	assert.Empty(t, nmSettingsMAC(gonetworkmanager.ConnectionSettings{"802-3-ethernet": {"mac-address": []byte{}}}))
+}