cloud-config: write network-config

In order to configure multiple IP addresses for different network
devices, we need to tell cloud-init to enable DHCP on for those devices.

This is easier said than done, as the different distributions we use:
* Use netplan, NetworkManager and ifcfg scripts
* Use different network device names
* Have inconsistent behaviour when handling "dhcp=false" vs the default
  (which it claims is also "false")

The only consistent behaviour between all these different distributions is:
* If no network-config is found, cloud-init will sequentially configure the
  devices  to use DHCP, meaning we configure our "access" network first
* If all attached networks run DHCP, we can tell cloud-init to configure
  all devices through dhcp.

All other cases (such as non-dhcp-enabled networks) result in some distribution
or another to time out waiting in the "network-online.target" phase of system
start up. These issues can be fixed on a per-distribution basis, but I could
not find a cloud-init solution that works for _all_ distributions at the
same time.

README.md

@@ -97,6 +97,8 @@ If you require DNS resolution from your VMs to return correct FQDNs, add the
 
 By default, Virter uses the libvirt network named `default`.
 
+Check out [`doc/networks.md`](./doc/networks.md) for more details on VM networking.
+
 ### DHCP Leases
 
 Libvirt produces some weird behavior when MAC or IP addresses are reused while

doc/networks.md

@@ -0,0 +1,62 @@
+# Virter Networking
+
+Using Virter requires at least one virtual network. All VMs started by Virter will be
+added to this network, and all interactions with the guest (`vm ssh`/`vm cp`/`vm exec`/...)
+happen through this network.
+
+In almost all cases, a suitable network called `default` is already configured in libvirt.
+
+## Defining additional networks
+
+In addition to the access network, you can use Virter to create new virtual networks. These can
+then be used to run VMs with multiple network interfaces or VMs running in multiple networks.
+
+You can list the currently defined networks using the following command
+```
+$ virter network ls
+Name                      Forward-Type  IP-Range          Domain  DHCP                           Bridge
+default (virter default)  nat           192.168.122.1/24  test    192.168.122.2-192.168.122.254  virbr0
+```
+
+To add a second network named `net1`, configured to assign IP addresses in the range `10.255.0.0/24`, run:
+```
+$ virter network add net1 --dhcp --network-cidr 10.255.0.1/24
+$ virter network ls
+Name                      Forward-Type  IP-Range          Domain  DHCP                           Bridge
+net1                                    10.255.0.1/24             10.255.0.2-10.255.0.254        virbr1
+default (virter default)  nat           192.168.122.1/24  test    192.168.122.2-192.168.122.254  virbr0
+```
+
+You can also remove networks again, using `virter network rm <name>`.
+
+## Defining VMs attached to multiple networks
+
+You can specify additional network devices that should be added to a VM. For example, to attach a VM to the
+network `net1`, run:
+
+```
+$ virter vm run alma-8 --id 8 --nic type=network,source=net1
+...
+$ virter network list-attached default
+VM        MAC                IP             Hostname  Host Device
+alma-8-8  52:54:00:00:00:08  192.168.122.8  alma-8-8  vnet1
+$ virter network list-attached net1
+VM        MAC                IP            Hostname  Host Device
+alma-8-8  52:54:00:b3:72:d7  10.255.0.207  alma-8-8  vnet2
+```
+
+## Running VMs attached to multiple networks
+
+Ideally, VMs started with multiple network interfaces should have all those interfaces configured as best as possible.
+That means:
+* All interfaces should be up.
+* All interfaces running in a network with DHCP should be running a DHCP client.
+
+Due to inconsistent behaviour of cloud-init between platforms, this is sadly not generally possible for Virter.
+The limited configuration we can achieve is:
+* If _all_ networks run DHCP, all those networks will be configured to use DHCP.
+* If at least one network does not DHCP, cloud-init falls back to the default behaviour:
+  * The first network device is configured for DHCP, which will always be the Virter access network device.
+  * All other network devices are left alone. Exact behaviour depends on the guest OS. There is no guarantee
+    that DHCP is configured for all networks where it is available. There is also no guarantee that the network
+    interfaces are up.

internal/virter/cloudconfig.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/kdomanski/iso9660"
 	"github.com/kr/text"
+	lx "github.com/libvirt/libvirt-go-xml"
 
 	"github.com/LINBIT/virter/pkg/sshkeys"
 )
@@ -14,6 +15,14 @@ const templateMetaData = `instance-id: {{ .VMName }}
 local-hostname: {{ .VMName }}
 `
 
+const templateNetworkConfig = `version: 2
+ethernets:
+{{- range . }}
+  {{ . }}:
+    dhcp4: true
+{{- end }}
+`
+
 const templateUserData = `#cloud-config
 disable_root: False
 # Ideally, we would set this to "unchanged". However, this causes cloud-init on centos-6
@@ -51,6 +60,57 @@ func (v *Virter) metaData(vmName string) (string, error) {
 	return renderTemplate("meta-data", templateMetaData, templateData)
 }
 
+// NetworkConfig returns cloud-init configuration, initializing all networks with DHCP if possible.
+//
+// See the end of ./doc/networks.md for limitations.
+func (v *Virter) NetworkConfig(nics []NIC) (string, error) {
+	if len(nics) == 0 {
+		// We know the default configuration works, so no need to make it worse in case some old cloud-init version
+		// doesn't work with this type of network configuration...
+		return "", nil
+	}
+
+	configuredNics := make([]string, 0, len(nics)+2)
+	configuredNics = append(configuredNics, "eth0", "enp1s0")
+
+	allNets, err := v.NetworkList()
+	if err != nil {
+		return "", err
+	}
+
+	for i, nic := range nics {
+		if nic.GetType() != NICTypeNetwork {
+			// Extra NIC without configured dhcp support, can't enable more than the default NIC.
+			return "", nil
+		}
+
+		var net *lx.Network
+		for i := range allNets {
+			if allNets[i].Name == nic.GetSource() {
+				net = &allNets[i]
+			}
+		}
+
+		if net == nil {
+			return "", fmt.Errorf("NIC assigned to unknown network '%s'", nic.GetSource())
+		}
+
+		if len(net.IPs) < 1 {
+			// No IPs configure -> no DHCP configured, can't enable more than the default NIC
+			return "", nil
+		}
+
+		if net.IPs[0].DHCP == nil {
+			// No DHCP configured, can't enable more than the default NIC
+			return "", nil
+		}
+
+		configuredNics = append(configuredNics, fmt.Sprintf("eth%d", i+1), fmt.Sprintf("enp%ds0", i+2))
+	}
+
+	return renderTemplate("network-config", templateNetworkConfig, configuredNics)
+}
+
 func (v *Virter) userData(vmName string, sshPublicKeys []string, hostkey sshkeys.HostKey, mounts []string) (string, error) {
 	privateKey := text.Indent(hostkey.PrivateKey(), "    ")
 	publicKey := text.Indent(hostkey.PublicKey(), "    ")
@@ -81,6 +141,11 @@ func (v *Virter) createCIData(vmConfig VMConfig, hostkey sshkeys.HostKey) (*RawL
 		return nil, err
 	}
 
+	networkConfig, err := v.NetworkConfig(vmConfig.ExtraNics)
+	if err != nil {
+		return nil, err
+	}
+
 	mounts := make([]string, len(vmConfig.Mounts))
 	for i, m := range vmConfig.Mounts {
 		mounts[i] = m.GetVMPath()
@@ -96,6 +161,12 @@ func (v *Virter) createCIData(vmConfig VMConfig, hostkey sshkeys.HostKey) (*RawL
 		"user-data": []byte(userData),
 	}
 
+	// Only explicitly add network config if we have something to configure.
+	// Otherwise, cloud-init might not configure the network at all.
+	if networkConfig != "" {
+		files["network-config"] = []byte(networkConfig)
+	}
+
 	ciData, err := GenerateISO(files)
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate ISO: %w", err)

internal/virter/cloudconfig_test.go

@@ -0,0 +1,103 @@
+package virter_test
+
+import (
+	"testing"
+
+	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/LINBIT/virter/internal/virter"
+)
+
+type fakeNetworkNic string
+
+func (f fakeNetworkNic) GetType() string {
+	return "network"
+}
+
+func (f fakeNetworkNic) GetSource() string {
+	return string(f)
+}
+
+func (f fakeNetworkNic) GetModel() string {
+	return "virtio"
+}
+
+func (f fakeNetworkNic) GetMAC() string {
+	return "fake"
+}
+
+var testNetworks = map[string][]libvirtxml.NetworkIP{
+	"dhcp1":  {{DHCP: &libvirtxml.NetworkDHCP{}}},
+	"dhcp2":  {{DHCP: &libvirtxml.NetworkDHCP{}}},
+	"nodhcp": {{}},
+	"noip":   nil,
+}
+
+func TestVirter_NetworkConfig(t *testing.T) {
+	l := newFakeLibvirtConnection()
+	v := virter.New(l, poolName, networkName, newMockKeystore())
+
+	for name, ips := range testNetworks {
+		err := v.NetworkAdd(libvirtxml.Network{Name: name, IPs: ips})
+		assert.NoError(t, err)
+	}
+
+	testcases := []struct {
+		name     string
+		nics     []virter.NIC
+		expected string
+	}{
+		{
+			name:     "default-no-config",
+			expected: "",
+		},
+		{
+			name: "all-dhcp-config",
+			nics: []virter.NIC{
+				fakeNetworkNic("dhcp1"),
+				fakeNetworkNic("dhcp2"),
+			},
+			expected: `version: 2
+ethernets:
+  eth0:
+    dhcp4: true
+  enp1s0:
+    dhcp4: true
+  eth1:
+    dhcp4: true
+  enp2s0:
+    dhcp4: true
+  eth2:
+    dhcp4: true
+  enp3s0:
+    dhcp4: true
+`,
+		},
+		{
+			name: "some-without-dhcp-no-config",
+			nics: []virter.NIC{
+				fakeNetworkNic("dhcp1"),
+				fakeNetworkNic("nodhcp"),
+			},
+			expected: "",
+		},
+		{
+			name: "some-without-ip-no-config",
+			nics: []virter.NIC{
+				fakeNetworkNic("dhcp1"),
+				fakeNetworkNic("noip"),
+			},
+			expected: "",
+		},
+	}
+
+	for i := range testcases {
+		tcase := &testcases[i]
+		t.Run(tcase.name, func(t *testing.T) {
+			actual, err := v.NetworkConfig(tcase.nics)
+			assert.NoError(t, err)
+			assert.Equal(t, tcase.expected, actual)
+		})
+	}
+}

tests/test-run.toml

@@ -39,15 +39,15 @@ done
 # Checks that shell steps work as expected
 [[steps]]
 [steps.shell]
-script = """
+script = '''
 set -ex
 
 test -f /virter/example.txt
 cd /virter
 sha256sum -c example.txt.sha256sum
 
 # CentOS 6 doesn't use systemd, Upstart is a mess, so don't even bother...
-[ command -v systemctl ] || exit 0
+command -v systemctl || exit 0
 
 while true; do
 	running=$(systemctl is-system-running || true)
@@ -57,4 +57,29 @@ while true; do
 	systemctl list-units --failed 1>&2
 	exit 1
 done
-"""
+
+NR_ADDRS=0
+while read IDX NAME ; do
+    case "$NAME" in
+    lo)
+      # Skip loopback
+      # Default interface, must have IP and should be up
+      ip -oneline -4 addr show dev $NAME | grep -q "inet" || exit 1
+      ip -oneline link show dev $NAME | grep -q "state UP" || exit 1
+      ;;
+    eth*|enp*s0)
+      # Should have IP address, and be online
+      ip -oneline -4 addr show dev $NAME | grep -q "inet" || exit 1
+      ip -oneline link show dev $NAME | grep -q "state UP" || exit 1
+      NR_ADDRS=$(($NR_ADDRS + 1)
+      ;;
+    *)
+      echo Unexpected network interface $NAME 1>&2
+      exit 1
+      ;;
+    esac
+done < <(ip -oneline link show | grep -oP "^\d+:\s\w+")
+
+# Should have two networks configured
+test $NR_ADDRS -eq 2
+'''

tests/tests.toml

@@ -10,3 +10,5 @@ artifacts = [
 [tests.smoke]
 vms = [1]
 needallplatforms = true
+[[tests.smoke.networks]]
+dhcp = true