[lxc-devel] [lxd/master] container/lxc: Moves volatile host_name enrichment to fillNetworkDevice

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 422 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190522/b55f26bb/attachment.bin>
-------------- next part --------------
From b27eeadb7df294928441afaf00a41f14ef7aed78 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 22 May 2019 19:02:45 +0100
Subject: [PATCH] container/lxc: Moves volatile host_name enrichment into
 fillNetworkDevice

Also clears volatile host_name keys when container stops.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container_lxc.go | 51 +++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 93e255d162..305500f62d 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -3102,10 +3102,10 @@ func (c *containerLXC) OnStop(target string) error {
 		logger.Error("Failed to set container state", log.Ctx{"container": c.Name(), "err": err})
 	}
 
-	// Clean up networking routes
-	err = c.cleanupNetworkRoutes()
+	// Clean up networking veth devices
+	err = c.cleanupHostVethDevices()
 	if err != nil {
-		logger.Error("Failed to cleanup network routes: ", log.Ctx{"container": c.Name(), "err": err})
+		logger.Error("Failed to cleanup veth devices: ", log.Ctx{"container": c.Name(), "err": err})
 	}
 
 	go func(c *containerLXC, target string, op *lxcContainerOperation) {
@@ -3163,19 +3163,38 @@ func (c *containerLXC) OnStop(target string) error {
 	return nil
 }
 
-// cleanupNetworkRoutes removes any static routes added on the host for nic devices.
-func (c *containerLXC) cleanupNetworkRoutes() error {
+// cleanupHostVethDevices removes host side configuration for veth devices.
+func (c *containerLXC) cleanupHostVethDevices() error {
+	volatileNics := make([]string, 0)
+
 	for _, k := range c.expandedDevices.DeviceNames() {
 		m := c.expandedDevices[k]
 		if m["type"] != "nic" {
 			continue
 		}
 
-		// Remove any static veth routes
+		m, err := c.fillNetworkDevice(k, m)
+		if err != nil {
+			continue
+		}
+
+		// Remove any static host side veth routes
 		if shared.StringInSlice(m["nictype"], []string{"bridged", "p2p"}) {
 			c.removeNetworkRoutes(k, m)
+			volatileNics = append(volatileNics, k) // Record for volatile removal
 		}
+	}
+
+	// Clear host side config from volatile nics
+	volatile := make(map[string]string)
+	for _, deviceName := range volatileNics {
+		hostNameKey := fmt.Sprintf("volatile.%s.host_name", deviceName)
+		volatile[hostNameKey] = "" // Remove volatile host_name for device
+	}
 
+	err := c.VolatileSet(volatile)
+	if err != nil {
+		return err
 	}
 
 	return nil
@@ -3206,12 +3225,6 @@ func (c *containerLXC) OnNetworkUp(deviceName string, hostName string) error {
 
 // setupHostVethDevice configures a nic device's host side veth settings.
 func (c *containerLXC) setupHostVethDevice(deviceName string, device types.Device, oldDevice types.Device) error {
-	// If not populated already, check if volatile data contains the most recently added host_name.
-	if device["host_name"] == "" {
-		hostNameKey := fmt.Sprintf("volatile.%s.host_name", deviceName)
-		device["host_name"] = c.localConfig[hostNameKey]
-	}
-
 	// Check whether host device resolution succeeded.
 	if device["host_name"] == "" {
 		return fmt.Errorf("Failed to find host side veth name for device \"%s\"", deviceName)
@@ -5103,7 +5116,11 @@ func (c *containerLXC) Update(args db.ContainerArgs, userRequested bool) error {
 						return err
 					}
 
-					err = c.setupHostVethDevice(k, m, oldExpandedDevices[k])
+					// We're updating the same device, so copy enriched host_name
+					// into oldDevice config for veth host device setup.
+					oldDevice := oldExpandedDevices[k]
+					oldDevice["host_name"] = m["host_name"]
+					err = c.setupHostVethDevice(k, m, oldDevice)
 					if err != nil {
 						return err
 					}
@@ -8135,7 +8152,7 @@ func (c *containerLXC) fillNetworkDevice(name string, m types.Device) (types.Dev
 	}
 
 	// Fill in the host name (but don't generate a static one ourselves)
-	if m["host_name"] == "" && shared.StringInSlice(m["nictype"], []string{"sriov"}) {
+	if m["host_name"] == "" && shared.StringInSlice(m["nictype"], []string{"bridged", "p2p", "sriov"}) {
 		configKey := fmt.Sprintf("volatile.%s.host_name", name)
 		newDevice["host_name"] = c.localConfig[configKey]
 	}
@@ -8860,12 +8877,6 @@ func (c *containerLXC) setNetworkRoutes(deviceName string, m types.Device, oldDe
 // removeNetworkRoutes removes any routes created for this device on the host that were first added
 // with setNetworkRoutes(). Expects to be passed the device config from the oldExpandedDevices.
 func (c *containerLXC) removeNetworkRoutes(deviceName string, m types.Device) {
-	// If not populated already, check if volatile data contains the most recently added host_name.
-	if m["host_name"] == "" {
-		hostNameKey := fmt.Sprintf("volatile.%s.host_name", deviceName)
-		m["host_name"] = c.localConfig[hostNameKey]
-	}
-
 	// Decide whether the route should point to the veth parent or the bridge parent
 	routeDev := m["host_name"]
 	if m["nictype"] == "bridged" {