f-46: dhcpbackend: drive dnsmasq or the built-in server through one interface #46

internal/subnet et internal/vm ne parlent plus à dnsmasq en direct mais à une
interface Backend, choisie au démarrage par dhcp.backend. Le défaut restant
dnsmasq, un agent.yml de 0.1.0 se comporte à l'identique ; ValidBackend refuse
toute autre valeur que dnsmasq ou two.

Les entrées ip→mac sont écrites hors du backend : elles font autorité pour les
deux, internal/vm/data.go les lisant par GetMACForIP quel que soit le serveur.
D'où l'extraction de dhcp.Entries, que GenerateConfig mêlait à l'écriture du
fichier dnsmasq.

Reservation porte Index et DefaultRoute : dnsmasq en dérive ses tags et n'en
pose que sur les interfaces sans route par défaut, le backend two transmet le
drapeau tel quel. La notion de tag ne remonte plus jusqu'à internal/vm.

Two.ConfigureSubnet sonde get-state avant de pousser la configuration : le job
systemd d'une unit Type=simple est done dès le fork, avant que la socket
existe. TeardownSubnet arrête l'unit avant de supprimer le .state.

Le backend two est testé contre un vrai serveur sur une vraie socket. Onze
mutations, toutes détectées.

Signed-off-by: GnomeZworc <nicolas.boufidjeline@g3e.fr>

internal/vm/create.go

@@ -8,7 +8,7 @@ import (
 	"path/filepath"
 
 	configuration "git.g3e.fr/syonad/two/internal/config/agent"
-	"git.g3e.fr/syonad/two/internal/dhcp"
+	"git.g3e.fr/syonad/two/internal/dhcpbackend"
 	"git.g3e.fr/syonad/two/internal/iptables"
 	"git.g3e.fr/syonad/two/internal/metadata"
 	"git.g3e.fr/syonad/two/internal/netif"
@@ -34,6 +34,11 @@ func StartVM(db *badger.DB, name string, cfg *configuration.Config) error {
 	}
 	nic := d.primary()
 
+	backend, err := dhcpbackend.New(cfg)
+	if err != nil {
+		return err
+	}
+
 	for _, n := range d.nics {
 		if err := netif.CreateTap(n.tapID, n.bridge, n.vpcName); err != nil {
 			return fmt.Errorf("create tap of interface %d: %w", n.index, err)
@@ -54,7 +59,7 @@ func StartVM(db *badger.DB, name string, cfg *configuration.Config) error {
 		return fmt.Errorf("add metadata redirect: %w", err)
 	}
 
-	if err := writeDHCPFiles(d, name); err != nil {
+	if err := writeDHCPFiles(d, name, backend); err != nil {
 		return err
 	}
 
@@ -109,53 +114,37 @@ func StartVM(db *badger.DB, name string, cfg *configuration.Config) error {
 	return state.Set(db, "vm/"+name, state.Running)
 }
 
-// writeDHCPFiles écrit, pour chaque subnet touché par la VM, les réservations
-// de ses interfaces et les options qui suppriment la route par défaut sur les
-// interfaces non primaires. Le subnet de l'interface primaire ne reçoit aucune
-// option : les options non taggées du subnet portent déjà la route par défaut.
-func writeDHCPFiles(d vmData, name string) error {
-	type subnetFiles struct {
-		nic          nicData
-		reservations []dhcp.Reservation
-		tags         []string
-	}
-	bySubnet := make(map[string]*subnetFiles)
+func dhcpReservations(d vmData) map[string]*subnetReservations {
+	bySubnet := make(map[string]*subnetReservations)
 
 	for _, n := range d.nics {
-		confName := n.vpcName + "_" + n.bridge
-		if bySubnet[confName] == nil {
-			bySubnet[confName] = &subnetFiles{nic: n}
+		key := n.vpcName + "_" + n.bridge
+		if bySubnet[key] == nil {
+			bySubnet[key] = &subnetReservations{subnet: dhcpbackend.Subnet{
+				Name:        n.subnetName,
+				VPC:         n.vpcName,
+				Bridge:      n.bridge,
+				InterfaceIP: net.ParseIP(n.interfaceIP),
+				VPCRoute:    n.vpcCIDR,
+			}}
 		}
-		f := bySubnet[confName]
-		f.reservations = append(f.reservations, dhcp.Reservation{
-			MAC: n.mac, IP: n.ip, Tag: nicTag(name, n.index),
+		f := bySubnet[key]
+		f.reservations = append(f.reservations, dhcpbackend.Reservation{
+			Index: n.index, MAC: n.mac, IP: n.ip, DefaultRoute: n.primary,
 		})
-		if !n.primary {
-			f.tags = append(f.tags, nicTag(name, n.index))
-		}
 	}
+	return bySubnet
+}
 
-	for confName, f := range bySubnet {
-		if err := dhcp.WriteReservations(dhcp.DefaultConfDir, confName, name, f.reservations); err != nil {
-			return fmt.Errorf("write dhcp reservations on %s: %w", confName, err)
-		}
-		if err := dhcp.WriteVMOptions(dhcp.DefaultConfDir, confName, name, f.tags, dhcp.Config{
-			InterfaceIP: net.ParseIP(f.nic.interfaceIP),
-			VPCRoute:    f.nic.vpcCIDR,
-		}); err != nil {
-			return fmt.Errorf("write dhcp options on %s: %w", confName, err)
+func writeDHCPFiles(d vmData, name string, backend dhcpbackend.Backend) error {
+	for _, f := range dhcpReservations(d) {
+		if err := backend.SetVM(f.subnet, name, f.reservations); err != nil {
+			return err
 		}
 	}
 	return nil
 }
 
-// nicTag identifie une interface auprès de dnsmasq. Il est par interface et non
-// par VM : deux interfaces d'une même VM peuvent partager un subnet, et n'y
-// avoir pas le même rôle.
-func nicTag(vmName string, index int) string {
-	return fmt.Sprintf("%s-%d", vmName, index)
-}
-
 func copyFile(src, dst string) error {
 	if err := os.MkdirAll(filepath.Dir(dst), 0755); err != nil {
 		return err

internal/vm/delete.go

@@ -7,14 +7,13 @@ import (
 	"time"
 
 	configuration "git.g3e.fr/syonad/two/internal/config/agent"
-	"git.g3e.fr/syonad/two/internal/dhcp"
+	"git.g3e.fr/syonad/two/internal/dhcpbackend"
 	"git.g3e.fr/syonad/two/internal/iptables"
 	"git.g3e.fr/syonad/two/internal/metadata"
 	"git.g3e.fr/syonad/two/internal/netif"
 	"git.g3e.fr/syonad/two/internal/netns"
 	"git.g3e.fr/syonad/two/internal/qmp"
 	"git.g3e.fr/syonad/two/internal/state"
-	"git.g3e.fr/syonad/two/pkg/systemd"
 
 	"github.com/dgraph-io/badger/v4"
 )
@@ -34,6 +33,11 @@ func StopVM(db *badger.DB, name string, cfg *configuration.Config) error {
 	}
 	nic := d.primary()
 
+	backend, err := dhcpbackend.New(cfg)
+	if err != nil {
+		return err
+	}
+
 	socketPath := filepath.Join(cfg.QEMU.QMPDir, name+".sock")
 
 	if _, err := os.Stat(socketPath); err == nil {
@@ -69,7 +73,7 @@ func StopVM(db *badger.DB, name string, cfg *configuration.Config) error {
 		}
 	}
 
-	if err := removeDHCPFiles(d, name); err != nil {
+	if err := removeDHCPFiles(d, name, backend); err != nil {
 		return err
 	}
 
@@ -81,52 +85,15 @@ func StopVM(db *badger.DB, name string, cfg *configuration.Config) error {
 	return state.Set(db, "vm/"+name, state.Deleted)
 }
 
-// removeDHCPFiles retire les fichiers de la VM dans chaque subnet qu'elle
-// touche, puis redémarre les dnsmasq concernés : un fichier ajouté dans un
-// dhcp-hostsdir est relu à chaud, un fichier retiré ne l'est pas (vérifié sur
-// dnsmasq 2.90).
-func removeDHCPFiles(d vmData, name string) error {
-	seen := make(map[string]bool)
-	for _, n := range d.nics {
-		confName := n.vpcName + "_" + n.bridge
-		if seen[confName] {
-			continue
-		}
-		seen[confName] = true
-		if err := removeDHCPReservation(confName, name); err != nil {
+func removeDHCPFiles(d vmData, name string, backend dhcpbackend.Backend) error {
+	for _, f := range dhcpReservations(d) {
+		if err := backend.DelVM(f.subnet, name, f.reservations); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func removeDHCPReservation(confName, name string) error {
-	if err := dhcp.RemoveReservations(dhcp.DefaultConfDir, confName, name); err != nil {
-		return err
-	}
-
-	svc, err := systemd.New()
-	if err != nil {
-		return fmt.Errorf("connect to systemd: %w", err)
-	}
-	defer svc.Close()
-
-	unit := dhcp.UnitName(confName)
-	status, err := svc.Status(unit)
-	if err != nil || status.ActiveState != "active" {
-		return nil
-	}
-	if err := svc.Restart(unit); err != nil {
-		return fmt.Errorf("restart %s: %w", unit, err)
-	}
-	if status, err := svc.Status(unit); err != nil {
-		return fmt.Errorf("status %s after restart: %w", unit, err)
-	} else if status.ActiveState != "active" {
-		return fmt.Errorf("%s is %s after restart", unit, status.ActiveState)
-	}
-	return nil
-}
-
 func waitQMPDead(socketPath string, timeout, poll time.Duration) {
 	timer := time.After(timeout)
 	for {

internal/vm/dhcp.go

@@ -0,0 +1,10 @@
+package vm
+
+import (
+	"git.g3e.fr/syonad/two/internal/dhcpbackend"
+)
+
+type subnetReservations struct {
+	subnet       dhcpbackend.Subnet
+	reservations []dhcpbackend.Reservation
+}

internal/vm/dhcp_test.go

@@ -0,0 +1,122 @@
+package vm
+
+import (
+	"net"
+	"testing"
+)
+
+func nic(idx int, subnet, vpc, bridge, ip, mac string, primary bool) nicData {
+	return nicData{
+		index:       idx,
+		subnetName:  subnet,
+		vpcName:     vpc,
+		bridge:      bridge,
+		interfaceIP: "10.0.5.1",
+		ip:          ip,
+		mac:         mac,
+		primary:     primary,
+	}
+}
+
+func group(t *testing.T, groups map[string]*subnetReservations, key string) *subnetReservations {
+	t.Helper()
+	g, ok := groups[key]
+	if !ok || g == nil {
+		t.Fatalf("no group %q, got %v", key, keysOf(groups))
+	}
+	return g
+}
+
+func keysOf(groups map[string]*subnetReservations) []string {
+	keys := make([]string, 0, len(groups))
+	for k := range groups {
+		keys = append(keys, k)
+	}
+	return keys
+}
+
+func TestDHCPReservations_GroupsInterfacesBySubnet(t *testing.T) {
+	d := vmData{nics: []nicData{
+		nic(0, "sn-000001", "vp-admin", "br-000001", "10.0.5.10", "00:22:33:00:00:0a", true),
+		nic(1, "sn-000001", "vp-admin", "br-000001", "10.0.5.11", "00:22:33:00:00:0b", false),
+		nic(2, "sn-000002", "vp-admin", "br-000002", "10.0.6.10", "00:22:33:00:00:0c", false),
+	}}
+
+	got := dhcpReservations(d)
+	if len(got) != 2 {
+		t.Fatalf("groups = %d, want one per subnet", len(got))
+	}
+	if n := len(group(t, got, "vp-admin_br-000001").reservations); n != 2 {
+		t.Errorf("br-000001 carries %d reservations, want 2", n)
+	}
+	if n := len(group(t, got, "vp-admin_br-000002").reservations); n != 1 {
+		t.Errorf("br-000002 carries %d reservations, want 1", n)
+	}
+}
+
+func TestDHCPReservations_OnlyThePrimaryCarriesTheDefaultRoute(t *testing.T) {
+	d := vmData{nics: []nicData{
+		nic(0, "sn-000001", "vp-admin", "br-000001", "10.0.5.10", "00:22:33:00:00:0a", true),
+		nic(1, "sn-000001", "vp-admin", "br-000001", "10.0.5.11", "00:22:33:00:00:0b", false),
+	}}
+
+	res := group(t, dhcpReservations(d), "vp-admin_br-000001").reservations
+	byMAC := map[string]bool{}
+	for _, r := range res {
+		byMAC[r.MAC] = r.DefaultRoute
+	}
+	if !byMAC["00:22:33:00:00:0a"] {
+		t.Error("the primary interface must carry the default route")
+	}
+	if byMAC["00:22:33:00:00:0b"] {
+		t.Error("a secondary interface must not carry the default route")
+	}
+}
+
+func TestDHCPReservations_KeepsTheInterfaceIndex(t *testing.T) {
+	d := vmData{nics: []nicData{
+		nic(3, "sn-000001", "vp-admin", "br-000001", "10.0.5.10", "00:22:33:00:00:0a", true),
+	}}
+
+	res := group(t, dhcpReservations(d), "vp-admin_br-000001").reservations
+	if res[0].Index != 3 {
+		t.Errorf("index = %d, want 3: the dnsmasq tag is derived from it", res[0].Index)
+	}
+}
+
+func TestDHCPReservations_CarriesTheSubnetIdentity(t *testing.T) {
+	_, vpcCIDR, err := net.ParseCIDR("10.0.0.0/16")
+	if err != nil {
+		t.Fatalf("ParseCIDR: %v", err)
+	}
+	n := nic(0, "sn-000001", "vp-admin", "br-000001", "10.0.5.10", "00:22:33:00:00:0a", true)
+	n.vpcCIDR = vpcCIDR
+
+	g := group(t, dhcpReservations(vmData{nics: []nicData{n}}), "vp-admin_br-000001")
+	if g.subnet.Name != "sn-000001" {
+		t.Errorf("subnet name = %s, want sn-000001", g.subnet.Name)
+	}
+	if !g.subnet.InterfaceIP.Equal(net.ParseIP("10.0.5.1")) {
+		t.Errorf("interface ip = %s, want 10.0.5.1", g.subnet.InterfaceIP)
+	}
+	if g.subnet.VPCRoute == nil || g.subnet.VPCRoute.String() != "10.0.0.0/16" {
+		t.Errorf("vpc route = %v, want 10.0.0.0/16", g.subnet.VPCRoute)
+	}
+}
+
+func TestDHCPReservations_SameBridgeInTwoVPCsStaysSeparate(t *testing.T) {
+	d := vmData{nics: []nicData{
+		nic(0, "sn-000001", "vp-admin", "br-000001", "10.0.5.10", "00:22:33:00:00:0a", true),
+		nic(1, "sn-000009", "vp-other", "br-000001", "10.9.5.10", "00:22:33:00:00:0d", false),
+	}}
+
+	if got := len(dhcpReservations(d)); got != 2 {
+		t.Errorf("groups = %d, want 2: the vpc is part of the instance identity", got)
+	}
+}
+
+func TestDHCPReservations_NoNICYieldsNoGroup(t *testing.T) {
+	if got := len(dhcpReservations(vmData{})); got != 0 {
+		t.Errorf("groups = %d, want none", got)
+	}
+}