Make virt-dhcp persistant across restarts

Store the dhcp host file in a shared host directory.
In order to prevent virt-dhcp and its client getting out of sync this patch
also, exposes a method allowing the client to set its known hosts.
The method will wipe all the previously known hosts and will set the newly
provided ones.

cmd/virt-dhcp/virt-dhcp.go

@@ -20,9 +20,10 @@
 package main
 
 import (
+	"flag"
+
 	"kubevirt.io/kubevirt/pkg/log"
 	dhcp "kubevirt.io/kubevirt/pkg/virt-dhcp"
-	"flag"
 )
 
 const defaultDhcpSocket = "/var/run/kubevirt/virt_dhcp_sock"

pkg/virt-dhcp/client.go

@@ -44,3 +44,14 @@ func (dhcp *VirtDHCPClient) RemoveIP(mac string, ip string) error {
 	}
 	return nil
 }
+
+func (dhcp *VirtDHCPClient) SetIPs(hosts *[]AddArgs, reply *DhcpReply) error {
+	addReply := &DhcpReply{}
+
+	dhcp.client.Call("DHCP.SetIPs", hosts, addReply)
+	if addReply.Success != true {
+		msg := fmt.Sprintf("failed to set known hosts to dhcp: %s", addReply.Message)
+		return fmt.Errorf(msg)
+	}
+	return nil
+}

pkg/virt-dhcp/dnsmasq.go

@@ -17,12 +17,13 @@ import (
 )
 
 const (
-	DNSmasqExec   = "/usr/sbin/dnsmasq"
-	DHCPLeaseFile = "/tmp/dhcp.leases"
-	DHCPPidFile   = "/tmp/dhcp.pid"
-	DHCPHostsFile = "/tmp/dhcp.hosts"
+	DNSmasqExec = "/usr/sbin/dnsmasq"
+	DHCPPidFile = "/tmp/dhcp.pid"
 )
 
+var DHCPLeaseFile = "/var/run/kubevirt/dhcp.leases"
+var DHCPHostsFile = "/var/run/kubevirt/dhcp.hosts"
+
 type hostsDetails struct {
 	Mac   string
 	Lease int
@@ -78,14 +79,10 @@ type OSOps interface {
 	checkProcessExist(pid int) (bool, *os.Process)
 	killProcessIfExist(pid int) error
 	RecreateHostsFile() (*os.File, error)
-	executeCommand(string, []string) *exec.Cmd
 	readFromFile(file string) ([]byte, error)
-	IsDNSMasqPidRunning(pid int) bool
 }
 
-type OSHandler struct {
-	dnsmasq *DNSmasqInstance
-}
+type OSHandler struct{}
 
 var OS OSOps
 
@@ -101,22 +98,28 @@ func (o *OSHandler) IsFileExist(path string) (bool, error) {
 	return exists, err
 }
 
-func (o *OSHandler) getPidFromFile(file string) (int, error) {
+func (o *OSHandler) readFromFile(file string) ([]byte, error) {
 	content, err := ioutil.ReadFile(file)
 	if err != nil {
-		log.Log.Reason(err).Errorf("failed to open dnsmasq pid file: %s", file)
+		log.Log.Reason(err).Errorf("failed to open file: %s", file)
+		return nil, err
+	}
+	return content, nil
+}
+
+func (o *OSHandler) getPidFromFile(file string) (int, error) {
+	content, err := OS.readFromFile(file)
+	if err != nil {
 		return -1, err
 	}
 	lines := strings.Split(string(content), "\n")
 	pid, _ := strconv.Atoi(lines[0])
-	log.Log.Debugf("found dnsmasq pid: %s", pid)
 	return pid, nil
 }
 
 func (o *OSHandler) killProcessIfExist(pid int) error {
 	procExist, proc := OS.checkProcessExist(pid)
 	if procExist {
-		log.Log.Debugf("killing process %d", proc.Pid)
 		killErr := proc.Kill()
 		if killErr != nil {
 			log.Log.Warningf("failed to kill process %d", proc.Pid)
@@ -150,27 +153,6 @@ func (o *OSHandler) RecreateHostsFile() (*os.File, error) {
 	return f, nil
 }
 
-func (o *OSHandler) IsDNSMasqPidRunning(pid int) bool {
-	path := fmt.Sprintf("/proc/%d/cmdline", pid)
-	fileExist, err := OS.IsFileExist(path)
-	if err != nil {
-		panic(err)
-	}
-
-	if fileExist {
-		content, err := ioutil.ReadFile(path)
-		if err != nil {
-			return false
-		}
-		return strings.Contains(string(content), "dnsmasq")
-	}
-	return false
-}
-
-func (o *OSHandler) executeCommand(path string, startArgs []string) *exec.Cmd {
-	return exec.Command(path, startArgs...)
-}
-
 func (dnsmasq *DNSmasqInstance) formatDhcpRange() string {
 	return fmt.Sprintf("--dhcp-range=%s,%s", net.IP(dnsmasq.ipRange[0]).String(), net.IP(dnsmasq.ipRange[1]).String())
 }
@@ -188,15 +170,9 @@ func (dnsmasq *DNSmasqInstance) stop() error {
 	if dnsmasq.monitor.IsRunning() {
 		dnsmasq.monitor.Stop()
 	} else {
-		fileExist, err := OS.IsFileExist(DHCPPidFile)
-		if err != nil {
-			return err
-		}
-		if fileExist {
-			pid, err1 := OS.getPidFromFile(DHCPPidFile)
-			if err1 == nil {
-				OS.killProcessIfExist(pid)
-			}
+		pid, err1 := OS.getPidFromFile(DHCPPidFile)
+		if err1 == nil {
+			OS.killProcessIfExist(pid)
 		}
 	}
 	return nil
@@ -219,7 +195,6 @@ func (dnsmasq *DNSmasqInstance) Restart() error {
 	if len(dnsmasq.hosts) != 0 {
 		return dnsmasq.Start()
 	}
-
 	return nil
 }
 
@@ -234,6 +209,17 @@ func (dnsmasq *DNSmasqInstance) RemoveHost(ipaddr string) error {
 	return dnsmasq.Restart()
 }
 
+func (dnsmasq *DNSmasqInstance) SetKnownHosts(hostsList *[]AddArgs) error {
+	//Empty the known hosts store
+	dnsmasq.hosts = make(map[string]hostsDetails)
+
+	//Set the provided known hosts
+	for _, host := range *hostsList {
+		dnsmasq.hosts[host.IP] = hostsDetails{Mac: host.Mac, Lease: host.Lease}
+	}
+	return dnsmasq.Restart()
+}
+
 func (dnsmasq *DNSmasqInstance) writeDHCPHostsFile(file *os.File, dhcpHosts []string) error {
 	if len(dhcpHosts) != 0 {
 		t := template.Must(template.New("dhcpshosts").Parse(`{{range $k:=.}}{{printf "%s\n" $k }}{{end}}
@@ -277,17 +263,14 @@ func (dnsmasq *DNSmasqInstance) updateRange() {
 }
 
 func (dnsmasq *DNSmasqInstance) loadHostsFile() error {
-	fileExist, err := OS.IsFileExist(DHCPPidFile)
-	if err != nil {
-		return err
-	}
-	if !fileExist {
-		return nil
-	}
 	content, err := OS.readFromFile(DHCPHostsFile)
 	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
 		return err
 	}
+
 	lines := strings.Split(string(content), "\n")
 	for _, line := range lines {
 		data := strings.Split(line, ",")

pkg/virt-dhcp/dnsmasq_test.go

@@ -2,6 +2,7 @@ package virtdhcp
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
 	"strconv"
 	"strings"
@@ -57,7 +58,16 @@ func (o *MockOSHandler) killProcessIfExist(pid int) error {
 
 var _ = Describe("DHCP", func() {
 
+	var tempDir string
+
 	BeforeEach(func() {
+		tempDir, err := ioutil.TempDir("", "dhcptest")
+		Expect(err).NotTo(HaveOccurred())
+		DHCPLeaseFile = fmt.Sprintf("%s/dhcp.leases", tempDir)
+		DHCPHostsFile = fmt.Sprintf("%s/dhcp.hosts", tempDir)
+		cmdArgsPrefix = fmt.Sprintf("-k -d --strict-order --bind-dynamic --no-resolv --dhcp-no-override --dhcp-authoritative "+
+			"--except-interface=lo --dhcp-hostsfile=%s --dhcp-leasefile=%s --pid-file=/tmp/dhcp.pid",
+			DHCPHostsFile, DHCPLeaseFile)
 		testdnsmasq = NewDNSmasq()
 		OS = &MockOSHandler{}
 		testdnsmasq.monitor = FakeMonitor()
@@ -126,5 +136,17 @@ var _ = Describe("DHCP", func() {
 			Expect(len(testdnsmasq.hosts)).To(Equal(0))
 			Expect(cmdArgs).To(Equal([]string{}))
 		})
+		It("should set known hosts", func() {
+			hostsList := []AddArgs{{Mac: "12:d4:4f:99:7d:3f", IP: "10.32.0.15", Lease: 86400},
+				{Mac: "af:21:ac:bd:c3:ba", IP: "10.32.0.12", Lease: 86400}}
+			err := testdnsmasq.SetKnownHosts(&hostsList)
+
+			Expect(err).NotTo(HaveOccurred())
+			Expect(len(testdnsmasq.hosts)).To(Equal(2))
+		})
+
+		AfterEach(func() {
+			os.RemoveAll(tempDir)
+		})
 	})
 })

pkg/virt-dhcp/monitor.go

@@ -19,18 +19,18 @@ type MonitorRuntime struct {
 }
 
 type MonitorCmd struct {
-	Stop    chan bool
+	Stop           chan bool
 	processStopped chan bool
-	ProcCmd *exec.Cmd
-	cmdArgs []string
-	cmdPath string
-	retries int
+	ProcCmd        *exec.Cmd
+	cmdArgs        []string
+	cmdPath        string
+	retries        int
 }
 
 func NewMonitor() Monitor {
 	mon := MonitorRuntime{
-		stopChan:  make(chan bool),
-		pid:       0,
+		stopChan: make(chan bool),
+		pid:      0,
 	}
 	return &mon
 }
@@ -50,7 +50,7 @@ func (runtime *MonitorRuntime) IsRunning() bool {
 }
 
 func (mon *MonitorCmd) restart(pid *int) {
-	defer func () {mon.processStopped <- true}()
+	defer func() { mon.processStopped <- true }()
 	mon.ProcCmd = exec.Command(mon.cmdPath, mon.cmdArgs...)
 	err := mon.ProcCmd.Start()
 	mon.retries += 1
@@ -90,4 +90,4 @@ func RunMonitor(cmd string, args []string, stopChan chan bool, pid *int) error {
 		}
 	}
 	return nil
-}
+}

pkg/virt-dhcp/monitor_test.go

@@ -25,10 +25,11 @@ import (
 
 	"time"
 
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
 	"fmt"
 	"io/ioutil"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
 )
 
 func IsDNSMasqPidRunning(pid int) bool {

pkg/virt-dhcp/virt-dhcp.go

@@ -60,6 +60,19 @@ func (s *DHCP) RemoveIP(args *RemoveArgs, reply *DhcpReply) error {
 	return nil
 }
 
+func (s *DHCP) SetIPs(args *[]AddArgs, reply *DhcpReply) error {
+	reply.Success = true
+	s.mutex.Lock()
+	defer s.mutex.Unlock()
+	err := s.dnsmasq.SetKnownHosts(args)
+	if err != nil {
+		log.Log.Reason(err).Errorf("failed to set known hosts")
+		reply.Success = false
+		reply.Message = err.Error()
+	}
+	return nil
+}
+
 func createSocket(socketPath string) (net.Listener, error) {
 
 	os.RemoveAll(socketPath)
@@ -77,10 +90,12 @@ func Run(socket string) error {
 	var mutex = &sync.Mutex{}
 
 	rpcServer := rpc.NewServer()
+	dnsmasq := NewDNSmasq()
 	server := &DHCP{name: "virt-dhcp",
-		dnsmasq: NewDNSmasq(),
+		dnsmasq: dnsmasq,
 		mutex:   mutex}
 	rpcServer.Register(server)
+	dnsmasq.loadHostsFile()
 	sock, err := createSocket(DhcpSocket)
 	if err != nil {
 		return err