The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8170
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
- Switches new OVN instance port names to use the instance's UUID setting rather than the instance's ID property.
- This is because the instance's ID property can change if the instance is recovered using `lxd import` while it is still running.
- This has the knock on effect that when the running instance is eventually stopped, its OVN logical switch port is not cleaned up as the ID has changed (and it is left in the OVN NB database).
- Using the `volatile.uuid` setting ensures that it will remain the same between recoveries.
However this introduces a related problem. What about instances that are already started and have an OVN port name containing the instance ID from the old regime. Once this change is applied, any running instance that is stopped will then also leave its OVN port orphaned.
To account for this, we lookup the external OVN port name stored in the local OVS integration bridge (that maintains the link between local veth interface and OVN logical port) at device stop time. If there is a value available we use that over the current naming regime (as it represents the actual logical switch port the device was started with) to clear up the port.
If for some reason the external OVN port name is not available in the local OVS bridge (perhaps it was removed manually before shutdown) then we fallback to using the current naming regime to try and clear up the OVN port.
From b730e82e3cba90255cc78987d5d615a6f0a1857e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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})
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
