Network subsystem remove os rules (#396)

* netplan: add support for default MTU value from mds

MDS will provide us with the user's defined MTU make sure we honor
it on the netplan dropin.

* network: remove OS rules and introduce netplan dhcp overrides

We are removing the backward compatibility OS rules and assuming the
detected available backend. This change also removes the notion of
fallback as it was before. Additionally the rollback logic changed
to "rollback all backends except the active one", the "except the
active one" rule is important to prevent restarting network when
no configuration change's happened.

In the set we are also adding the netplan dhcp overrides directives
that was resulting into misconfigured dhcp routes.

google_guest_agent/network/manager/common.go

@@ -76,6 +76,26 @@ func interfaceListsIpv4Ipv6(nics []metadata.NetworkInterfaces) ([]string, []stri
 	return googleInterfaces, googleIpv6Interfaces
 }
 
+// interfacesMTUMap returns a map indexes by the interface's name with the MTU value
+// provided by the metadata descriptor.
+func interfacesMTUMap(nics []metadata.NetworkInterfaces) (map[string]int, error) {
+	res := make(map[string]int)
+
+	for _, ni := range nics {
+		iface, err := GetInterfaceByMAC(ni.Mac)
+		if err != nil {
+			if _, found := badMAC[ni.Mac]; !found {
+				logger.Errorf("error getting interface: %s", err)
+				badMAC[ni.Mac] = iface
+			}
+			continue
+		}
+		res[iface.Name] = ni.MTU
+	}
+
+	return res, nil
+}
+
 // GetInterfaceByMAC gets the interface given the mac string.
 func GetInterfaceByMAC(mac string) (net.Interface, error) {
 	hwaddr, err := net.ParseMAC(mac)

google_guest_agent/network/manager/dhclient_linux.go

@@ -156,12 +156,6 @@ type ipVersion struct {
 // dhclient implements the manager.Service interface for dhclient use cases.
 type dhclient struct{}
 
-// init adds this network manager service to the list of known network managers.
-// DHClient will be the default fallback.
-func init() {
-	registerManager(&dhclient{}, true)
-}
-
 // Name returns the name of the network manager service.
 func (n dhclient) Name() string {
 	return "dhclient"
@@ -431,12 +425,7 @@ func partitionInterfaces(ctx context.Context, interfaces, ipv6Interfaces []strin
 	var obtainIpv6Interfaces []string
 	var releaseIpv6Interfaces []string
 
-	for i, iface := range interfaces {
-		if !shouldManageInterface(i == 0) {
-			// Do not setup anything for this interface to avoid duplicate processes.
-			continue
-		}
-
+	for _, iface := range interfaces {
 		// Check for IPv4 interfaces for which to obtain a lease.
 		processExists, err := dhclientProcessExists(ctx, iface, ipv4)
 		if err != nil {

google_guest_agent/network/manager/manager.go

@@ -19,7 +19,6 @@ package manager
 import (
 	"context"
 	"fmt"
-	"slices"
 
 	"github.com/GoogleCloudPlatform/guest-agent/google_guest_agent/cfg"
 	"github.com/GoogleCloudPlatform/guest-agent/google_guest_agent/osinfo"
@@ -108,136 +107,28 @@ var (
 	// network interface.
 	knownNetworkManagers []Service
 
-	// fallbackNetworkManager is the network manager service to be assumed if
-	// none of the known network managers returned true from IsManaging()
-	fallbackNetworkManager Service
-
-	// defaultOSRules lists the rules for applying interface configurations for primary
-	// and secondary interfaces.
-	defaultOSRules = []osConfigRule{
-		// Debian rules.
-		{
-			osNames: []string{"debian"},
-			majorVersions: map[int]bool{
-				10: true,
-				11: true,
-				12: true,
-			},
-			action: osConfigAction{
-				ignorePrimary:   true,
-				ignoreSecondary: true,
-			},
-		},
-		// RHEL rules.
-		{
-			osNames: []string{"rhel", "centos", "rocky"},
-			majorVersions: map[int]bool{
-				7: true,
-				8: true,
-				9: true,
-			},
-			action: osConfigAction{
-				ignorePrimary: true,
-			},
-		},
-		// Ubuntu rules
-		{
-			osNames: []string{"ubuntu"},
-			majorVersions: map[int]bool{
-				18: true,
-				20: true,
-				22: true,
-				23: true,
-			},
-			action: osConfigAction{
-				ignorePrimary: true,
-			},
-		},
-	}
-
 	// osinfoGet points to the function to use for getting osInfo.
 	// Primarily used for testing.
 	osinfoGet = osinfo.Get
-
-	// osRules points to the rules to use for finding relevant ignore rules.
-	// Primarily used for testing.
-	osRules = defaultOSRules
 )
 
-// registerManager registers the provided network manager service to the list of known
-// network manager services. Fallback specifies whether the provided service should be
-// marked as a fallback service.
-func registerManager(s Service, fallback bool) {
-	if !fallback {
-		knownNetworkManagers = append(knownNetworkManagers, s)
-	} else {
-		if fallbackNetworkManager != nil {
-			panic("trying to register second fallback network manager")
-		} else {
-			fallbackNetworkManager = s
-		}
-	}
-}
-
 // detectNetworkManager detects the network manager managing the primary network interface.
 // This network manager will be used to set up primary and secondary network interfaces.
-func detectNetworkManager(ctx context.Context, iface string) ([]serviceStatus, error) {
+func detectNetworkManager(ctx context.Context, iface string) (*serviceStatus, error) {
 	logger.Infof("Detecting network manager...")
 
-	networkManagers := knownNetworkManagers
-	if fallbackNetworkManager != nil {
-		networkManagers = append(networkManagers, fallbackNetworkManager)
-	}
-
-	var res []serviceStatus
-	for _, curr := range networkManagers {
+	for _, curr := range knownNetworkManagers {
 		active, err := curr.IsManaging(ctx, iface)
 		if err != nil {
 			return nil, err
 		}
 
 		if active {
-			res = append(res, serviceStatus{manager: curr, active: active})
-		}
-	}
-
-	if len(res) == 0 {
-		if fallbackNetworkManager != nil {
-			return append(res, serviceStatus{manager: fallbackNetworkManager, active: true}), nil
-		}
-		return nil, fmt.Errorf("no network manager impl found for %s", iface)
-	}
-
-	return res, nil
-}
-
-// findOSRule finds the osConfigRule that applies to the current system.
-func findOSRule() *osConfigRule {
-	osInfo := osinfoGet()
-	for _, curr := range osRules {
-		if !slices.Contains(curr.osNames, osInfo.OS) {
-			continue
-		}
-
-		if curr.majorVersions[osInfo.Version.Major] {
-			return &curr
+			return &serviceStatus{manager: curr, active: active}, nil
 		}
 	}
-	return nil
-}
 
-// shouldManageInterface returns whether the guest agent should manage an interface
-// provided whether the interface of interest is the primary interface or not.
-func shouldManageInterface(isPrimary bool) bool {
-	rule := findOSRule()
-	if rule != nil {
-		if isPrimary {
-			return !rule.action.ignorePrimary
-		}
-		return !rule.action.ignoreSecondary
-	}
-	// Assume setup for anything not specified.
-	return true
+	return nil, fmt.Errorf("no network manager impl found for %s", iface)
 }
 
 // SetupInterfaces sets up all the network interfaces on the system, applying rules described
@@ -268,40 +159,39 @@ func SetupInterfaces(ctx context.Context, config *cfg.Sections, mds *metadata.De
 	primaryInterface := interfaces[0]
 
 	// Get the network manager.
-	services, err := detectNetworkManager(ctx, primaryInterface)
+	activeService, err := detectNetworkManager(ctx, primaryInterface)
 	if err != nil {
 		return fmt.Errorf("error detecting network manager service: %v", err)
 	}
 
 	// Attempt to rollback any left over configuration of non active network managers.
-	for _, svc := range services {
-		logger.Infof("Rolling back %s", svc.manager.Name())
-		if err = svc.manager.Rollback(ctx, nics); err != nil {
-			logger.Errorf("Failed to roll back config for %s: %v", svc.manager.Name(), err)
+	for _, svc := range knownNetworkManagers {
+		if svc == activeService.manager {
+			continue
+		}
+
+		logger.Infof("Rolling back %s", svc.Name())
+		if err = svc.Rollback(ctx, nics); err != nil {
+			logger.Errorf("Failed to roll back config for %s: %v", svc.Name(), err)
 		}
 	}
 
 	// Attempt to configure all present/active network managers.
-	for _, svc := range services {
-		if !svc.active {
-			continue
-		}
 
-		svc.manager.Configure(ctx, config)
+	activeService.manager.Configure(ctx, config)
 
-		logger.Infof("Setting up %s", svc.manager.Name())
-		if err = svc.manager.SetupEthernetInterface(ctx, config, nics); err != nil {
-			return fmt.Errorf("manager(%s): error setting up ethernet interfaces: %v", svc.manager.Name(), err)
-		}
+	logger.Infof("Setting up %s", activeService.manager.Name())
+	if err = activeService.manager.SetupEthernetInterface(ctx, config, nics); err != nil {
+		return fmt.Errorf("manager(%s): error setting up ethernet interfaces: %v", activeService.manager.Name(), err)
+	}
 
-		if config.Unstable.VlanSetupEnabled {
-			if err = svc.manager.SetupVlanInterface(ctx, config, nics); err != nil {
-				return fmt.Errorf("manager(%s): error setting up vlan interfaces: %v", svc.manager.Name(), err)
-			}
+	if config.Unstable.VlanSetupEnabled {
+		if err = activeService.manager.SetupVlanInterface(ctx, config, nics); err != nil {
+			return fmt.Errorf("manager(%s): error setting up vlan interfaces: %v", activeService.manager.Name(), err)
 		}
-
-		logger.Infof("Finished setting up %s", svc.manager.Name())
 	}
 
+	logger.Infof("Finished setting up %s", activeService.manager.Name())
+
 	return nil
 }

google_guest_agent/network/manager/manager_linux.go

@@ -0,0 +1,40 @@
+//  Copyright 2024 Google LLC
+//
+//  Licensed under the Apache License, Version 2.0 (the "License");
+//  you may not use this file except in compliance with the License.
+//  You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+//  Unless required by applicable law or agreed to in writing, software
+//  distributed under the License is distributed on an "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//  See the License for the specific language governing permissions and
+//  limitations under the License.
+
+package manager
+
+func init() {
+	// knownNetworkManagers is a list of supported/available network managers.
+	knownNetworkManagers = []Service{
+		&netplan{
+			netplanConfigDir:  "/etc/netplan/",
+			networkdDropinDir: "/etc/systemd/network/",
+			priority:          20,
+		},
+		&wicked{
+			configDir: defaultWickedConfigDir,
+		},
+		&networkManager{
+			configDir:         defaultNetworkManagerConfigDir,
+			networkScriptsDir: defaultNetworkScriptsDir,
+		},
+		&systemdNetworkd{
+			configDir:          "/usr/lib/systemd/network",
+			networkCtlKeys:     []string{"AdministrativeState", "SetupState"},
+			priority:           defaultSystemdNetworkdPriority,
+			deprecatedPriority: deprecatedPriority,
+		},
+		&dhclient{},
+	}
+}

google_guest_agent/network/manager/manager_test.go

@@ -19,7 +19,6 @@ package manager
 import (
 	"context"
 	"fmt"
-	"slices"
 	"testing"
 
 	"github.com/GoogleCloudPlatform/guest-agent/google_guest_agent/cfg"
@@ -83,7 +82,6 @@ func (n mockService) Rollback(ctx context.Context, nics *Interfaces) error {
 func managerTestSetup() {
 	// Clear the known network managers and fallbacks.
 	knownNetworkManagers = []Service{}
-	fallbackNetworkManager = nil
 
 	// Create our own osinfo function for testing.
 	osinfoGet = func() osinfo.OSInfo {
@@ -202,14 +200,21 @@ func TestDetectNetworkManager(t *testing.T) {
 		},
 	}
 
+	prevKnownNetworkManager := knownNetworkManagers
+	t.Cleanup(func() {
+		knownNetworkManagers = prevKnownNetworkManager
+	})
+
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			managerTestSetup()
+
+			knownNetworkManagers = nil
 			for _, service := range test.services {
-				registerManager(service, service.isFallback)
+				knownNetworkManagers = append(knownNetworkManagers, service)
 			}
 
-			services, err := detectNetworkManager(context.Background(), "iface")
+			activeService, err := detectNetworkManager(context.Background(), "iface")
 
 			if err != nil {
 				if !test.expectErr {
@@ -226,12 +231,8 @@ func TestDetectNetworkManager(t *testing.T) {
 				t.Fatalf("no error returned when error expected, expected error: %s", test.expectedErrorMessage)
 			}
 
-			contains := slices.ContainsFunc(services, func(svce serviceStatus) bool {
-				return svce.manager == test.expectedManager && svce.active
-			})
-
-			if !contains {
-				t.Fatalf("did not get expected network manager. Expected: %v, Actual: %v", test.expectedManager, services)
+			if activeService.manager != test.expectedManager {
+				t.Fatalf("did not get expected network manager. Expected: %v, Actual: %v", test.expectedManager, activeService)
 			}
 		})
 	}
@@ -286,18 +287,6 @@ func TestFindOSRule(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			managerTestSetup()
-
-			osRules = test.rules
-			osRule := findOSRule()
-
-			if osRule == nil && !test.expectedNil {
-				t.Errorf("findOSRule() returned nil when non-nil expected")
-			}
-			if osRule != nil && test.expectedNil {
-				t.Errorf("findOSRule() returned non-nil when nil expected: %+v", osRule)
-			}
-
-			osRules = defaultOSRules
 		})
 	}
 }