[lxc-devel] [lxd/master] Network: Use Instance UUID for generating OVN logical switch port names

tomponline on Github lxc-bot at linuxcontainers.org
Thu Nov 19 16:32:11 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1711 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201119/469b82bc/attachment.bin>
-------------- next part --------------
From b730e82e3cba90255cc78987d5d615a6f0a1857e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 16:16:27 +0000
Subject: [PATCH 1/7] lxd/network/openvswitch/ovs: Adds
 InterfaceAssociatedOVNSwitchPort function

Allows the retrieval of the current external OVN logical switch port associated to an OVS interface.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/openvswitch/ovs.go | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lxd/network/openvswitch/ovs.go b/lxd/network/openvswitch/ovs.go
index 478d9325aa..2d66bc6eb8 100644
--- a/lxd/network/openvswitch/ovs.go
+++ b/lxd/network/openvswitch/ovs.go
@@ -150,6 +150,16 @@ func (o *OVS) InterfaceAssociateOVNSwitchPort(interfaceName string, ovnSwitchPor
 	return nil
 }
 
+// InterfaceAssociatedOVNSwitchPort returns the OVN switch port associated to the OVS interface.
+func (o *OVS) InterfaceAssociatedOVNSwitchPort(interfaceName string) (OVNSwitchPort, error) {
+	ovnSwitchPort, err := shared.RunCommand("ovs-vsctl", "get", "interface", interfaceName, "external_ids:iface-id")
+	if err != nil {
+		return "", err
+	}
+
+	return OVNSwitchPort(strings.TrimSpace(ovnSwitchPort)), nil
+}
+
 // ChassisID returns the local chassis ID.
 func (o *OVS) ChassisID() (string, error) {
 	// ovs-vsctl's get command doesn't support its --format flag, so we always get the output quoted.

From 0e05f658031a0e447a1f5467f7d9435e6fbce74d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 16:17:26 +0000
Subject: [PATCH 2/7] lxd/network/driver/ovn: Updates Instance port functions
 to use instance UUID rather than instance ID

Allows for clean removal of OVN ports after an instance has been `lxd import`ed while running (changing its instance ID).

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

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 950ac04be9..4f437fdb22 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -1949,8 +1949,8 @@ func (n *ovn) Update(newNetwork api.NetworkPut, targetNode string, clientType cl
 }
 
 // getInstanceDevicePortName returns the switch port name to use for an instance device.
-func (n *ovn) getInstanceDevicePortName(instanceID int, deviceName string) openvswitch.OVNSwitchPort {
-	return openvswitch.OVNSwitchPort(fmt.Sprintf("%s-%d-%s", n.getIntSwitchInstancePortPrefix(), instanceID, deviceName))
+func (n *ovn) getInstanceDevicePortName(instanceUUID string, deviceName string) openvswitch.OVNSwitchPort {
+	return openvswitch.OVNSwitchPort(fmt.Sprintf("%s-%s-%s", n.getIntSwitchInstancePortPrefix(), instanceUUID, deviceName))
 }
 
 // InstanceDevicePortValidateExternalRoutes validates the external routes for an OVN instance port.
@@ -2041,7 +2041,11 @@ func (n *ovn) InstanceDevicePortValidateExternalRoutes(deviceInstance instance.I
 }
 
 // InstanceDevicePortAdd adds an instance device port to the internal logical switch and returns the port name.
-func (n *ovn) InstanceDevicePortAdd(instanceID int, instanceName string, deviceName string, mac net.HardwareAddr, ips []net.IP, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) (openvswitch.OVNSwitchPort, error) {
+func (n *ovn) InstanceDevicePortAdd(instanceUUID string, instanceName string, deviceName string, mac net.HardwareAddr, ips []net.IP, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) (openvswitch.OVNSwitchPort, error) {
+	if instanceUUID == "" {
+		return "", fmt.Errorf("Instance UUID is required")
+	}
+
 	var dhcpV4ID, dhcpv6ID string
 
 	revert := revert.New()
@@ -2101,7 +2105,7 @@ func (n *ovn) InstanceDevicePortAdd(instanceID int, instanceName string, deviceN
 		}
 	}
 
-	instancePortName := n.getInstanceDevicePortName(instanceID, deviceName)
+	instancePortName := n.getInstanceDevicePortName(instanceUUID, deviceName)
 
 	// Add port with mayExist set to true, so that if instance port exists, we don't fail and continue below
 	// to configure the port as needed. This is required in case the OVN northbound database was unavailable
@@ -2232,8 +2236,12 @@ func (n *ovn) InstanceDevicePortAdd(instanceID int, instanceName string, deviceN
 }
 
 // InstanceDevicePortDynamicIPs returns the dynamically allocated IPs for a device port.
-func (n *ovn) InstanceDevicePortDynamicIPs(instanceID int, deviceName string) ([]net.IP, error) {
-	instancePortName := n.getInstanceDevicePortName(instanceID, deviceName)
+func (n *ovn) InstanceDevicePortDynamicIPs(instanceUUID string, deviceName string) ([]net.IP, error) {
+	if instanceUUID == "" {
+		return nil, fmt.Errorf("Instance UUID is required")
+	}
+
+	instancePortName := n.getInstanceDevicePortName(instanceUUID, deviceName)
 
 	client, err := n.getClient()
 	if err != nil {

From c4b940b413307a714b99f6e471739008125c98c3 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 16:18:39 +0000
Subject: [PATCH 3/7] lxd/network/driver/ovn: Updates InstanceDevicePortDelete
 to accept an instance UUID and a ovsExternalOVNPort hint

This allows OVN instance ports started using the old naming regime (using instance ID) to be properly cleaned up when they are stopped by utilising the current active OVN external switch port associated to the local OVS interface.

In cases where this is not available we fallback to using the latest regime of using the instance's UUID for generating the OVN switch port name.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 4f437fdb22..03dd522eb7 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -2252,8 +2252,20 @@ func (n *ovn) InstanceDevicePortDynamicIPs(instanceUUID string, deviceName strin
 }
 
 // InstanceDevicePortDelete deletes an instance device port from the internal logical switch.
-func (n *ovn) InstanceDevicePortDelete(instanceID int, deviceName string, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) error {
-	instancePortName := n.getInstanceDevicePortName(instanceID, deviceName)
+func (n *ovn) InstanceDevicePortDelete(instanceUUID string, deviceName string, ovsExternalOVNPort openvswitch.OVNSwitchPort, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) error {
+	// Decide whether to use OVS provided OVN port name or internally derived OVN port name.
+	instancePortName := ovsExternalOVNPort
+	source := "OVS"
+	if ovsExternalOVNPort == "" {
+		if instanceUUID == "" {
+			return fmt.Errorf("Instance UUID is required")
+		}
+
+		instancePortName = n.getInstanceDevicePortName(instanceUUID, deviceName)
+		source = "internal"
+	}
+
+	n.logger.Debug("Deleting instance port", log.Ctx{"port": instancePortName, "source": source})
 
 	client, err := n.getClient()
 	if err != nil {

From a82843c04e0b2e0733b31e954c576db6d53cbe41 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 16:20:34 +0000
Subject: [PATCH 4/7] lxd/device/nic/ovn: Update ovnNet interface to use
 instance UUIDs.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_ovn.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index ea3110b54a..b593dd7f5b 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -29,9 +29,9 @@ type ovnNet interface {
 	network.Network
 
 	InstanceDevicePortValidateExternalRoutes(deviceInstance instance.Instance, deviceName string, externalRoutes []*net.IPNet) error
-	InstanceDevicePortAdd(instanceID int, instanceName string, deviceName string, mac net.HardwareAddr, ips []net.IP, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) (openvswitch.OVNSwitchPort, error)
-	InstanceDevicePortDelete(instanceID int, deviceName string, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) error
-	InstanceDevicePortDynamicIPs(instanceID int, deviceName string) ([]net.IP, error)
+	InstanceDevicePortAdd(instanceUUID string, instanceName string, deviceName string, mac net.HardwareAddr, ips []net.IP, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) (openvswitch.OVNSwitchPort, error)
+	InstanceDevicePortDelete(instanceUUID string, deviceName string, ovsExternalOVNPort openvswitch.OVNSwitchPort, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) error
+	InstanceDevicePortDynamicIPs(instanceUUID string, deviceName string) ([]net.IP, error)
 }
 
 type nicOVN struct {

From 347475e2f37ff2a382f61aadd93aefccb4acea33 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 16:21:27 +0000
Subject: [PATCH 5/7] lxd/device/nic/ovn: Use volatile.uuid instance UUID
 rather than instance ID for OVN switch port name

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_ovn.go | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index b593dd7f5b..99f85f20cb 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -300,12 +300,13 @@ func (d *nicOVN) Start() (*deviceConfig.RunConfig, error) {
 	}
 
 	// Add new OVN logical switch port for instance.
-	logicalPortName, err := d.network.InstanceDevicePortAdd(d.inst.ID(), d.inst.Name(), d.name, mac, ips, internalRoutes, externalRoutes)
+	instanceUUID := d.inst.LocalConfig()["volatile.uuid"]
+	logicalPortName, err := d.network.InstanceDevicePortAdd(instanceUUID, d.inst.Name(), d.name, mac, ips, internalRoutes, externalRoutes)
 	if err != nil {
 		return nil, errors.Wrapf(err, "Failed adding OVN port")
 	}
 
-	revert.Add(func() { d.network.InstanceDevicePortDelete(d.inst.ID(), d.name, internalRoutes, externalRoutes) })
+	revert.Add(func() { d.network.InstanceDevicePortDelete(instanceUUID, d.name, "", internalRoutes, externalRoutes) })
 
 	// Attach host side veth interface to bridge.
 	integrationBridge, err := d.getIntegrationBridgeName()
@@ -512,7 +513,8 @@ func (d *nicOVN) State() (*api.InstanceStateNetwork, error) {
 
 	// OVN only supports dynamic IP allocation if neither IPv4 or IPv6 are statically set.
 	if d.config["ipv4.address"] == "" && d.config["ipv6.address"] == "" {
-		dynamicIPs, err := d.network.InstanceDevicePortDynamicIPs(d.inst.ID(), d.name)
+		instanceUUID := d.inst.LocalConfig()["volatile.uuid"]
+		dynamicIPs, err := d.network.InstanceDevicePortDynamicIPs(instanceUUID, d.name)
 		if err == nil {
 			for _, dynamicIP := range dynamicIPs {
 				family := "inet"

From 9cfdd8f33ca2a52a9956e3cdd891dc2b3e3185e0 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 16:21:54 +0000
Subject: [PATCH 6/7] lxd/device/nic/ovn: No need for intermediate v variable

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_ovn.go | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index 99f85f20cb..a603a8d9d2 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -369,10 +369,8 @@ func (d *nicOVN) Start() (*deviceConfig.RunConfig, error) {
 func (d *nicOVN) Update(oldDevices deviceConfig.Devices, isRunning bool) error {
 	oldConfig := oldDevices[d.name]
 
-	v := d.volatileGet()
-
 	// Populate device config with volatile fields if needed.
-	networkVethFillFromVolatile(d.config, v)
+	networkVethFillFromVolatile(d.config, d.volatileGet())
 
 	// If instance is running, apply host side limits and filters first before rebuilding
 	// dnsmasq config below so that existing config can be used as part of the filter removal.
@@ -452,9 +450,7 @@ func (d *nicOVN) postStop() error {
 		"host_name": "",
 	})
 
-	v := d.volatileGet()
-
-	networkVethFillFromVolatile(d.config, v)
+	networkVethFillFromVolatile(d.config, d.volatileGet())
 
 	if d.config["host_name"] != "" && shared.PathExists(fmt.Sprintf("/sys/class/net/%s", d.config["host_name"])) {
 		integrationBridge, err := d.getIntegrationBridgeName()
@@ -487,10 +483,8 @@ func (d *nicOVN) Remove() error {
 
 // State gets the state of an OVN NIC by querying the OVN Northbound logical switch port record.
 func (d *nicOVN) State() (*api.InstanceStateNetwork, error) {
-	v := d.volatileGet()
-
 	// Populate device config with volatile fields (hwaddr and host_name) if needed.
-	networkVethFillFromVolatile(d.config, v)
+	networkVethFillFromVolatile(d.config, d.volatileGet())
 
 	addresses := []api.InstanceStateNetworkAddress{}
 	netConfig := d.network.Config()

From 0e6bbdfca3717af559f7cefe8e381a0e341d0532 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 16:23:24 +0000
Subject: [PATCH 7/7] lxd/device/nic/ovn: Updates Stop to pass instance UUID
 and an OVS external OVN switch port hint to InstanceDevicePortDelete

Allows for instance ports created under an earlier naming regime to still be cleaned up when the device is stopped.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_ovn.go | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index a603a8d9d2..8669939e9a 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -435,7 +435,19 @@ func (d *nicOVN) Stop() (*deviceConfig.RunConfig, error) {
 		}
 	}
 
-	err = d.network.InstanceDevicePortDelete(d.inst.ID(), d.name, internalRoutes, externalRoutes)
+	// Try and retrieve the last associated OVN switch port for the instance interface in the local OVS DB.
+	// If we cannot get this, don't fail, as InstanceDevicePortDelete will then try and generate the likely
+	// port name using the same regime it does for new ports. This part is only here in order to allow
+	// instance ports generated under an older regime to be cleaned up properly.
+	networkVethFillFromVolatile(d.config, d.volatileGet())
+	ovs := openvswitch.NewOVS()
+	ovsExternalOVNPort, err := ovs.InterfaceAssociatedOVNSwitchPort(d.config["host_name"])
+	if err != nil {
+		d.logger.Warn("Could not find OVN Switch port associated to OVS interface", log.Ctx{"interface": d.config["host_name"]})
+	}
+
+	instanceUUID := d.inst.LocalConfig()["volatile.uuid"]
+	err = d.network.InstanceDevicePortDelete(instanceUUID, d.name, ovsExternalOVNPort, internalRoutes, externalRoutes)
 	if err != nil {
 		// Don't fail here as we still want the postStop hook to run to clean up the local veth pair.
 		d.logger.Error("Failed to remove OVN device port", log.Ctx{"err": err})


More information about the lxc-devel mailing list