Implement support for vlan dynamic removal, update dhclient to remove…

… only if configured (#465)

google_guest_agent/network/manager/common.go

@@ -264,16 +264,6 @@ func readYamlFile(filepath string, ptr any) error {
 	return yaml.Unmarshal(bytes, ptr)
 }
 
-// fileExists returns true only if file exists and it can successfully run stat
-// on the path.
-func fileExists(filepath string) bool {
-	s, err := os.Stat(filepath)
-	if err != nil && !errors.Is(os.ErrNotExist, err) {
-		return false
-	}
-	return !s.IsDir()
-}
-
 // isUbuntu1804 checks if agent is running on Ubuntu 18.04. This is a helper
 // method to support some exceptions we have for 18.04.
 func isUbuntu1804() bool {

google_guest_agent/network/manager/common_test.go

@@ -16,8 +16,6 @@ package manager
 
 import (
 	"fmt"
-	"os"
-	"path/filepath"
 	"sort"
 	"testing"
 
@@ -138,42 +136,3 @@ func TestVlanInterfaceParentMap(t *testing.T) {
 		})
 	}
 }
-
-func TestFileExists(t *testing.T) {
-	dir := t.TempDir()
-	f, err := os.CreateTemp(dir, "file")
-	if err != nil {
-		t.Fatalf("os.CreateTemp(%s, file) failed unexpectedly with error: %v", dir, err)
-	}
-	defer f.Close()
-
-	tests := []struct {
-		name string
-		want bool
-		path string
-	}{
-		{
-			name: "existing_file",
-			want: true,
-			path: f.Name(),
-		},
-		{
-			name: "existing_dir",
-			want: false,
-			path: dir,
-		},
-		{
-			name: "non_existing_file",
-			want: false,
-			path: filepath.Join(t.TempDir(), "random"),
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			if got := fileExists(test.path); got != test.want {
-				t.Errorf("fileExists(%s) = %t, want = %t", test.path, got, test.want)
-			}
-		})
-	}
-}

google_guest_agent/network/manager/dhclient_linux.go

@@ -478,12 +478,57 @@ func dhclientProcessExists(_ context.Context, iface string, ipVersion ipVersion)
 	return false, nil
 }
 
+// anyDhclientProcessExists returns true if there's at-least one dhclient process
+// running for any of the known ethernet interfaces, regarless of Ipv4/Ipv6 stack.
+func anyDhclientProcessExists(nics *Interfaces) (bool, error) {
+	processes, err := ps.Find(".*dhclient.*")
+	if err != nil {
+		return false, fmt.Errorf("error finding dhclient process: %v", err)
+	}
+
+	if len(processes) == 0 {
+		return false, nil
+	}
+
+	interfaces, err := interfaceNames(nics.EthernetInterfaces)
+	if err != nil {
+		return false, fmt.Errorf("error getting interface names: %v", err)
+	}
+
+	for _, process := range processes {
+		commandLine := process.CommandLine
+		for _, iface := range interfaces {
+			if slices.Contains(commandLine, iface) {
+				return true, nil
+			}
+		}
+	}
+	return false, nil
+}
+
 // Rollback releases all leases from DHClient, effectively undoing the dhclient configurations.
 func (n *dhclient) Rollback(ctx context.Context, nics *Interfaces) error {
 	if err := n.RollbackNics(ctx, nics); err != nil {
 		return fmt.Errorf("failed to rollback ethernet interfaces: %w", err)
 	}
 
+	// This prevents incorrect rollback by dhclient where NICs are managed by netplan.
+	// VLAN interfaces does not have dhclient process running and IPs are assigned
+	// directly by running [ipAddressSet] command. Attempt to rollback any VLAN interfaces
+	// only if network stack is managed by dhclient (at-least one dhclient process for
+	// known ethernet interfaces). Simple dhclient existence does not prove this its managed by
+	// dhcliet as in case of Debian-12 we have dhclient but NICs are managed by netplan/networkd.
+
+	managed, err := anyDhclientProcessExists(nics)
+	if err != nil {
+		return fmt.Errorf("unable to detect if nics are dhclient managed: %w", err)
+	}
+
+	if !managed {
+		logger.Infof("No dhclient process found for any of the known ethernet interfaces, skipping vlan rollback.")
+		return nil
+	}
+
 	if err := n.removeVlanInterfaces(ctx, nil); err != nil {
 		return fmt.Errorf("failed to rollback vlan interfaces: %+v", err)
 	}

google_guest_agent/network/manager/dhclient_linux_test.go

@@ -19,6 +19,7 @@ package manager
 import (
 	"context"
 	"fmt"
+	"net"
 	"os/exec"
 	"slices"
 	"strings"
@@ -29,6 +30,7 @@ import (
 	"github.com/GoogleCloudPlatform/guest-agent/google_guest_agent/osinfo"
 	"github.com/GoogleCloudPlatform/guest-agent/google_guest_agent/ps"
 	"github.com/GoogleCloudPlatform/guest-agent/google_guest_agent/run"
+	"github.com/GoogleCloudPlatform/guest-agent/metadata"
 )
 
 // The test DHClient to use in the test.
@@ -513,3 +515,75 @@ func TestDhclientProcessExists(t *testing.T) {
 		})
 	}
 }
+
+func TestAnyDhclientProcessExists(t *testing.T) {
+	ifaces, err := net.Interfaces()
+	if err != nil {
+		t.Fatalf("net.Interfaces() failed unexpectedly with error: %v", err)
+	}
+
+	iface := ifaces[1]
+
+	nics := &Interfaces{EthernetInterfaces: []metadata.NetworkInterfaces{
+		{Mac: iface.HardwareAddr.String()},
+	}}
+
+	tests := []struct {
+		name     string
+		want     bool
+		exists   bool
+		wantErr  bool
+		throwErr bool
+		version  []ipVersion
+	}{
+		{
+			name:    "no_processes",
+			want:    false,
+			exists:  false,
+			wantErr: false,
+		},
+		{
+			name:    "ipv4_processes",
+			want:    true,
+			exists:  true,
+			wantErr: false,
+			version: []ipVersion{ipv4},
+		},
+		{
+			name:    "ipv4_ipv6_processes",
+			want:    true,
+			exists:  true,
+			wantErr: false,
+			version: []ipVersion{ipv4, ipv6},
+		},
+		{
+			name:     "fake_error",
+			want:     false,
+			wantErr:  true,
+			throwErr: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			opts := dhclientTestOpts{
+				processOpts: dhclientProcessOpts{
+					ifaces:      []string{iface.Name},
+					existFlags:  []bool{test.exists},
+					returnError: test.throwErr,
+					ipVersions:  test.version,
+				},
+			}
+			dhclientTestSetup(t, opts)
+			t.Cleanup(func() { dhclientTestTearDown(t) })
+
+			got, err := anyDhclientProcessExists(nics)
+			if (err != nil) != test.wantErr {
+				t.Errorf("anyDhclientProcessExists(%+v) = %v, want error: %t", nics, err, test.wantErr)
+			}
+			if got != test.want {
+				t.Errorf("anyDhclientProcessExists(%+v) = %t, want process exists: %t", nics, got, test.want)
+			}
+		})
+	}
+}