[lxc-devel] [lxd/master] Network: Reverts ovn.name and ovn.ovs_bridge settings and defers network start until after cluster pre-join phase

tomponline on Github lxc-bot at linuxcontainers.org
Mon Sep 14 11:01:20 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 3464 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200914/6c84433d/attachment-0001.bin>
-------------- next part --------------
From 5742c8a094003fc32129c99badedf5c2da5500e8 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Sep 2020 10:41:19 +0100
Subject: [PATCH 01/10] Revert "api: Adds network_ovn_name API extension"

This reverts commit 71898bd2654e8e0e977f5ba9c319c5e85b5c69fd.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 doc/api-extensions.md | 4 ----
 shared/version/api.go | 1 -
 2 files changed, 5 deletions(-)

diff --git a/doc/api-extensions.md b/doc/api-extensions.md
index 14d2316208..fa749cb371 100644
--- a/doc/api-extensions.md
+++ b/doc/api-extensions.md
@@ -1158,7 +1158,3 @@ Adds the `ovn.ovs_bridge` setting to `bridge` networks to allow the `ovn` networ
 
 If missing, the first `ovn` network to specify a `bridge` network as its parent `network` will cause the
 setting to be populated with a random interface name prefixed with "ovn".
-
-## network\_ovn\_name
-Adds the `ovn.name` setting to `ovn` networks to allow nodes joining cluster to access the logical OVN network
-name during the pre-join stage before the node is connected to the cluster database.
diff --git a/shared/version/api.go b/shared/version/api.go
index ac07e18827..4771c18f21 100644
--- a/shared/version/api.go
+++ b/shared/version/api.go
@@ -225,7 +225,6 @@ var APIExtensions = []string{
 	"container_syscall_intercept_bpf_devices",
 	"network_type_ovn",
 	"network_bridge_ovn_bridge",
-	"network_ovn_name",
 }
 
 // APIExtensionsCount returns the number of available API extensions.

From 8f7a59c11ada33c5f53890355e92e6aafc096415 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Sep 2020 10:41:43 +0100
Subject: [PATCH 02/10] Revert "doc/networks: Adds ovn.name to OVN network doc"

This reverts commit 1da05127ce526532196400a51fae93bfd751ea37.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 doc/networks.md | 1 -
 1 file changed, 1 deletion(-)

diff --git a/doc/networks.md b/doc/networks.md
index d2509a5400..63af6893b9 100644
--- a/doc/networks.md
+++ b/doc/networks.md
@@ -298,4 +298,3 @@ dns.search                      | string    | -                     | -
 ipv4.address                    | string    | standard mode         | random unused subnet      | IPv4 address for the bridge (CIDR notation). Use "none" to turn off IPv4 or "auto" to generate a new one
 ipv6.address                    | string    | standard mode         | random unused subnet      | IPv6 address for the bridge (CIDR notation). Use "none" to turn off IPv6 or "auto" to generate a new one
 network                         | string    | -                     | -                         | Parent network to use for outbound external network access
-ovn.name                        | string    | -                     | -                         | Name of OVN logical network (populated automatically at network setup time)

From da2cd398906410dbf43fbb7a4d66fa1d3c7c3d20 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Sep 2020 10:42:00 +0100
Subject: [PATCH 03/10] Revert "lxd/network/driver/ovn: Adds ovn.name setting
 to store OVN logical network name"

This reverts commit a3ce774924f0ce526e44b9173b5a934081d65650.

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

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index b22642d5c0..2ec0c2576f 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -38,10 +38,6 @@ const ovnVolatileParentIPv6 = "volatile.network.ipv6.address"
 // associated veth interfaces when using the parent network as an OVN uplink.
 const ovnParentOVSBridge = "ovn.ovs_bridge"
 
-// ovnName setting on the OVN network config indicating the name of the logical OVN network. Allows the consistent
-// use of the OVN logical network name in cluster node pre-join phase before node joins clustered database.
-const ovnName = "ovn.name"
-
 // ovnParentVars OVN object variables derived from parent network.
 type ovnParentVars struct {
 	// Router.
@@ -99,8 +95,7 @@ func (n *ovn) Validate(config map[string]string) error {
 		"dns.domain": validate.IsAny,
 		"dns.search": validate.IsAny,
 
-		// Populated automatically at network setup time.
-		ovnName:               validate.IsAny,
+		// Volatile keys populated automatically as needed.
 		ovnVolatileParentIPv4: validate.Optional(validate.IsNetworkAddressV4),
 		ovnVolatileParentIPv6: validate.Optional(validate.IsNetworkAddressV6),
 	}
@@ -146,9 +141,9 @@ func (n *ovn) getNetworkPrefix() string {
 	return fmt.Sprintf("lxd-net%d", n.id)
 }
 
-// getChassisGroup returns OVN chassis group name to use based on ovn.name setting in config.
+// getChassisGroup returns OVN chassis group name to use.
 func (n *ovn) getChassisGroupName() openvswitch.OVNChassisGroup {
-	return openvswitch.OVNChassisGroup(n.config[ovnName])
+	return openvswitch.OVNChassisGroup(n.getNetworkPrefix())
 }
 
 // getRouterName returns OVN logical router name to use.
@@ -269,29 +264,23 @@ func (n *ovn) getIntSwitchInstancePortPrefix() string {
 
 // setupParentPort initialises the parent uplink connection. Returns the derived ovnParentVars settings used
 // during the initial creation of the logical network.
-func (n *ovn) setupParentPort(tx *db.ClusterTx, parentNet Network, routerMAC net.HardwareAddr) (*ovnParentVars, error) {
-	// Store the OVN OVS bridge name in the parent network config if not set.
-	parentNetConf := parentNet.Config()
-	if parentNetConf[ovnParentOVSBridge] == "" {
-		parentNetConf[ovnParentOVSBridge] = fmt.Sprintf("lxdovn%d", parentNet.ID())
-		err := tx.UpdateNetwork(parentNet.ID(), parentNet.Description(), parentNetConf)
-		if err != nil {
-			return nil, errors.Wrapf(err, "Failed saving parent network OVN OVS bridge name")
-		}
+func (n *ovn) setupParentPort(routerMAC net.HardwareAddr) (*ovnParentVars, error) {
+	parentNet, err := LoadByName(n.state, n.config["network"])
+	if err != nil {
+		return nil, errors.Wrapf(err, "Failed loading parent network")
 	}
 
 	switch parentNet.Type() {
 	case "bridge":
-		return n.setupParentPortBridge(tx, parentNet, routerMAC)
+		return n.setupParentPortBridge(parentNet, routerMAC)
 	}
 
 	return nil, fmt.Errorf("Network type %q unsupported as OVN parent", parentNet.Type())
 }
 
-// setupParentPortBridge allocates external uplink IPs from the parent bridge and stores them in the OVN network
-// config. Also generates the OVS uplink bridge name and stores in the parent network's config.
+// setupParentPortBridge allocates external IPs on the parent bridge.
 // Returns the derived ovnParentVars settings.
-func (n *ovn) setupParentPortBridge(tx *db.ClusterTx, parentNet Network, routerMAC net.HardwareAddr) (*ovnParentVars, error) {
+func (n *ovn) setupParentPortBridge(parentNet Network, routerMAC net.HardwareAddr) (*ovnParentVars, error) {
 	v := &ovnParentVars{}
 
 	bridgeNet, ok := parentNet.(*bridge)
@@ -328,57 +317,68 @@ func (n *ovn) setupParentPortBridge(tx *db.ClusterTx, parentNet Network, routerM
 
 	// Decide whether we need to allocate new IP(s) and go to the expense of retrieving all allocated IPs.
 	if (parentIPv4Net != nil && routerExtPortIPv4 == nil) || (parentIPv6Net != nil && routerExtPortIPv6 == nil) {
-		allAllocatedIPv4, allAllocatedIPv6, err := n.parentAllAllocatedIPs(tx, parentNet.Name())
-		if err != nil {
-			return nil, errors.Wrapf(err, "Failed to get all allocated IPs for parent")
-		}
-
-		if parentIPv4Net != nil && routerExtPortIPv4 == nil {
-			if parentNetConf["ipv4.ovn.ranges"] == "" {
-				return nil, fmt.Errorf(`Missing required "ipv4.ovn.ranges" config key on parent network`)
-			}
-
-			ipRanges, err := parseIPRanges(parentNetConf["ipv4.ovn.ranges"], parentNet.DHCPv4Subnet())
-			if err != nil {
-				return nil, errors.Wrapf(err, "Failed to parse parent IPv4 OVN ranges")
-			}
-
-			routerExtPortIPv4, err = n.parentAllocateIP(ipRanges, allAllocatedIPv4)
+		err := n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
+			allAllocatedIPv4, allAllocatedIPv6, err := n.parentAllAllocatedIPs(tx, parentNet.Name())
 			if err != nil {
-				return nil, errors.Wrapf(err, "Failed to allocate parent IPv4 address")
+				return errors.Wrapf(err, "Failed to get all allocated IPs for parent")
 			}
 
-			n.config[ovnVolatileParentIPv4] = routerExtPortIPv4.String()
-		}
+			if parentIPv4Net != nil && routerExtPortIPv4 == nil {
+				if parentNetConf["ipv4.ovn.ranges"] == "" {
+					return fmt.Errorf(`Missing required "ipv4.ovn.ranges" config key on parent network`)
+				}
 
-		if parentIPv6Net != nil && routerExtPortIPv6 == nil {
-			// If IPv6 OVN ranges are specified by the parent, allocate from them.
-			if parentNetConf["ipv6.ovn.ranges"] != "" {
-				ipRanges, err := parseIPRanges(parentNetConf["ipv6.ovn.ranges"], parentNet.DHCPv6Subnet())
+				ipRanges, err := parseIPRanges(parentNetConf["ipv4.ovn.ranges"], parentNet.DHCPv4Subnet())
 				if err != nil {
-					return nil, errors.Wrapf(err, "Failed to parse parent IPv6 OVN ranges")
+					return errors.Wrapf(err, "Failed to parse parent IPv4 OVN ranges")
 				}
 
-				routerExtPortIPv6, err = n.parentAllocateIP(ipRanges, allAllocatedIPv6)
+				routerExtPortIPv4, err = n.parentAllocateIP(ipRanges, allAllocatedIPv4)
 				if err != nil {
-					return nil, errors.Wrapf(err, "Failed to allocate parent IPv6 address")
+					return errors.Wrapf(err, "Failed to allocate parent IPv4 address")
 				}
 
-			} else {
-				// Otherwise use EUI64 derived from MAC address.
-				routerExtPortIPv6, err = eui64.ParseMAC(parentIPv6Net.IP, routerMAC)
-				if err != nil {
-					return nil, err
+				n.config[ovnVolatileParentIPv4] = routerExtPortIPv4.String()
+			}
+
+			if parentIPv6Net != nil && routerExtPortIPv6 == nil {
+				// If IPv6 OVN ranges are specified by the parent, allocate from them.
+				if parentNetConf["ipv6.ovn.ranges"] != "" {
+					ipRanges, err := parseIPRanges(parentNetConf["ipv6.ovn.ranges"], parentNet.DHCPv6Subnet())
+					if err != nil {
+						return errors.Wrapf(err, "Failed to parse parent IPv6 OVN ranges")
+					}
+
+					routerExtPortIPv6, err = n.parentAllocateIP(ipRanges, allAllocatedIPv6)
+					if err != nil {
+						return errors.Wrapf(err, "Failed to allocate parent IPv6 address")
+					}
+
+				} else {
+					// Otherwise use EUI64 derived from MAC address.
+					routerExtPortIPv6, err = eui64.ParseMAC(parentIPv6Net.IP, routerMAC)
+					if err != nil {
+						return err
+					}
 				}
+
+				n.config[ovnVolatileParentIPv6] = routerExtPortIPv6.String()
 			}
 
-			n.config[ovnVolatileParentIPv6] = routerExtPortIPv6.String()
-		}
+			networkID, err := tx.GetNetworkID(n.name)
+			if err != nil {
+				return errors.Wrapf(err, "Failed to get network ID for network %q", n.name)
+			}
+
+			err = tx.UpdateNetwork(networkID, n.description, n.config)
+			if err != nil {
+				return errors.Wrapf(err, "Failed saving allocated parent network IPs")
+			}
 
-		// Store allocated IPs on parent network in network config.
-		err = tx.UpdateNetwork(n.id, n.description, n.config)
+			return nil
+		})
 		if err != nil {
-			return nil, errors.Wrapf(err, "Failed saving allocated parent network IPs")
+			return nil, err
 		}
 	}
 
@@ -510,7 +510,21 @@ func (n *ovn) parentOperationLockName(parentNet Network) string {
 func (n *ovn) parentPortBridgeVars(parentNet Network) (*ovnParentPortBridgeVars, error) {
 	parentConfig := parentNet.Config()
 	if parentConfig[ovnParentOVSBridge] == "" {
-		return nil, fmt.Errorf("Missing %q setting in parent network config", ovnParentOVSBridge)
+		// Generate random OVS bridge name for parent uplink.
+		parentConfig[ovnParentOVSBridge] = RandomDevName("ovn")
+
+		// Store in parent config.
+		err := n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
+			err := tx.UpdateNetwork(parentNet.ID(), parentNet.Description(), parentConfig)
+			if err != nil {
+				return errors.Wrapf(err, "Failed saving parent network OVN OVS bridge name")
+			}
+
+			return nil
+		})
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	return &ovnParentPortBridgeVars{
@@ -768,37 +782,8 @@ func (n *ovn) setup(update bool) error {
 		return err
 	}
 
-	// Parent network must be in default project.
-	parentNet, err := LoadByName(n.state, n.config["network"])
-	if err != nil {
-		return errors.Wrapf(err, "Failed loading parent network %q", n.config["network"])
-	}
-
-	var parent *ovnParentVars
-
-	// Start a transaction as we may need to modify the config in both this network and in the parent network.
-	// So do it in an atomic fashion.
-	err = n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
-		// Setup parent port (do this first to check parent is suitable).
-		// This may need to modify both the network config and the parent network config.
-		parent, err = n.setupParentPort(tx, parentNet, routerMAC)
-		if err != nil {
-			return err
-		}
-
-		// Set the OVN network name into config if missing (used for joining cluster nodes).
-		if n.config[ovnName] == "" {
-			n.config[ovnName] = n.getNetworkPrefix()
-
-			// Store updated network config.
-			err = tx.UpdateNetwork(n.id, n.description, n.config)
-			if err != nil {
-				return errors.Wrapf(err, "Failed saving OVN network name to config")
-			}
-		}
-
-		return nil
-	})
+	// Setup parent port (do this first to check parent is suitable).
+	parent, err := n.setupParentPort(routerMAC)
 	if err != nil {
 		return err
 	}
@@ -1072,11 +1057,6 @@ func (n *ovn) setup(update bool) error {
 
 // addChassisGroupEntry adds an entry for the local OVS chassis to the OVN logical network's chassis group.
 func (n *ovn) addChassisGroupEntry() error {
-	chassisGroupName := n.getChassisGroupName()
-	if chassisGroupName == "" {
-		return fmt.Errorf("Cannot add chassis group entry, missing %q setting", ovnName)
-	}
-
 	client, err := n.getClient()
 	if err != nil {
 		return err
@@ -1090,9 +1070,9 @@ func (n *ovn) addChassisGroupEntry() error {
 	}
 
 	var priority uint = ovnChassisPriorityMax
-	err = client.ChassisGroupChassisAdd(chassisGroupName, chassisID, priority)
+	err = client.ChassisGroupChassisAdd(n.getChassisGroupName(), chassisID, priority)
 	if err != nil {
-		return errors.Wrapf(err, "Failed adding OVS chassis %q with priority %d to chassis group %q", chassisID, priority, chassisGroupName)
+		return errors.Wrapf(err, "Failed adding OVS chassis %q with priority %d to chassis group %q", chassisID, priority, n.getChassisGroupName())
 	}
 
 	return nil
@@ -1100,11 +1080,6 @@ func (n *ovn) addChassisGroupEntry() error {
 
 // deleteChassisGroupEntry deletes an entry for the local OVS chassis from the OVN logical network's chassis group.
 func (n *ovn) deleteChassisGroupEntry() error {
-	chassisGroupName := n.getChassisGroupName()
-	if chassisGroupName == "" {
-		return fmt.Errorf("Cannot delete chassis group entry, missing %q setting", ovnName)
-	}
-
 	client, err := n.getClient()
 	if err != nil {
 		return err
@@ -1117,9 +1092,9 @@ func (n *ovn) deleteChassisGroupEntry() error {
 		return errors.Wrapf(err, "Failed getting OVS Chassis ID")
 	}
 
-	err = client.ChassisGroupChassisDelete(chassisGroupName, chassisID)
+	err = client.ChassisGroupChassisDelete(n.getChassisGroupName(), chassisID)
 	if err != nil {
-		return errors.Wrapf(err, "Failed deleting OVS chassis %q from chassis group %q", chassisID, chassisGroupName)
+		return errors.Wrapf(err, "Failed deleting OVS chassis %q from chassis group %q", chassisID, n.getChassisGroupName())
 	}
 
 	return nil

From a724ed68d98cc2819532f094d67c8bc33277fb05 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Sep 2020 10:42:18 +0100
Subject: [PATCH 04/10] Revert "doc/networks: Adds ovn.ovs_bridge key to bridge
 networks"

This reverts commit b0084540e858a21147f85a1a9b3c9e3772fc0873.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 doc/networks.md | 1 -
 1 file changed, 1 deletion(-)

diff --git a/doc/networks.md b/doc/networks.md
index 63af6893b9..da37fff468 100644
--- a/doc/networks.md
+++ b/doc/networks.md
@@ -109,7 +109,6 @@ tunnel.NAME.port                | integer   | vxlan                 | 0
 tunnel.NAME.protocol            | string    | standard mode         | -                         | Tunneling protocol ("vxlan" or "gre")
 tunnel.NAME.remote              | string    | gre or vxlan          | -                         | Remote address for the tunnel (not necessary for multicast vxlan)
 tunnel.NAME.ttl                 | integer   | vxlan                 | 1                         | Specific TTL to use for multicast routing topologies
-ovn.ovs\_bridge                 | string    | -                     | -                         | Name of OVS bridge that OVN networks use to connect to this network.
 
 Those keys can be set using the lxc tool with:
 

From e2770b2509b431fb1a1d43d541e9bff642c75933 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Sep 2020 10:42:31 +0100
Subject: [PATCH 05/10] Revert "lxd/network/driver/bridge: Adds ovn.ovs_bridge
 config key for OVN networks using bridge as parent"

This reverts commit 06425c9f62cc4365d568bb6f692b40466c5525b8.

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

diff --git a/lxd/network/driver_bridge.go b/lxd/network/driver_bridge.go
index 12b9acc29c..9d04ba123b 100644
--- a/lxd/network/driver_bridge.go
+++ b/lxd/network/driver_bridge.go
@@ -226,8 +226,6 @@ func (n *bridge) Validate(config map[string]string) error {
 
 		"raw.dnsmasq": validate.IsAny,
 
-		ovnParentOVSBridge: validate.Optional(validInterfaceName),
-
 		"maas.subnet.ipv4": validate.IsAny,
 		"maas.subnet.ipv6": validate.IsAny,
 	}

From 21f75a25cc66db52875d9cf3f7074b3bf4f1ba30 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Sep 2020 10:42:42 +0100
Subject: [PATCH 06/10] Revert "lxd/network/driver/ovn: Updates
 parentPortBridgeVars to use ovn.ovs_bridge from parent network"

This reverts commit 0632782fa73855bac2f0a02abb5d4f690bd8937d.

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

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 2ec0c2576f..3d902aa9b2 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -19,6 +19,7 @@ import (
 	"github.com/lxc/lxd/lxd/db"
 	"github.com/lxc/lxd/lxd/locking"
 	"github.com/lxc/lxd/lxd/network/openvswitch"
+	"github.com/lxc/lxd/lxd/project"
 	"github.com/lxc/lxd/lxd/revert"
 	"github.com/lxc/lxd/lxd/util"
 	"github.com/lxc/lxd/shared"
@@ -34,10 +35,6 @@ const ovnChassisPriorityMax = 32767
 const ovnVolatileParentIPv4 = "volatile.network.ipv4.address"
 const ovnVolatileParentIPv6 = "volatile.network.ipv6.address"
 
-// ovnParentOVSBridge setting on the parent network indicating the name to use for the OVS bridge and prefix for
-// associated veth interfaces when using the parent network as an OVN uplink.
-const ovnParentOVSBridge = "ovn.ovs_bridge"
-
 // ovnParentVars OVN object variables derived from parent network.
 type ovnParentVars struct {
 	// Router.
@@ -507,40 +504,21 @@ func (n *ovn) parentOperationLockName(parentNet Network) string {
 }
 
 // parentPortBridgeVars returns the parent port bridge variables needed for port start/stop.
-func (n *ovn) parentPortBridgeVars(parentNet Network) (*ovnParentPortBridgeVars, error) {
-	parentConfig := parentNet.Config()
-	if parentConfig[ovnParentOVSBridge] == "" {
-		// Generate random OVS bridge name for parent uplink.
-		parentConfig[ovnParentOVSBridge] = RandomDevName("ovn")
+func (n *ovn) parentPortBridgeVars(parentNet Network) *ovnParentPortBridgeVars {
 
-		// Store in parent config.
-		err := n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
-			err := tx.UpdateNetwork(parentNet.ID(), parentNet.Description(), parentConfig)
-			if err != nil {
-				return errors.Wrapf(err, "Failed saving parent network OVN OVS bridge name")
-			}
-
-			return nil
-		})
-		if err != nil {
-			return nil, err
-		}
-	}
+	ovsBridge := fmt.Sprintf("lxdovn%d", parentNet.ID())
 
 	return &ovnParentPortBridgeVars{
-		ovsBridge: parentConfig[ovnParentOVSBridge],
-		parentEnd: fmt.Sprintf("%sa", parentConfig[ovnParentOVSBridge]),
-		ovsEnd:    fmt.Sprintf("%sb", parentConfig[ovnParentOVSBridge]),
-	}, nil
+		ovsBridge: ovsBridge,
+		parentEnd: fmt.Sprintf("%sa", ovsBridge),
+		ovsEnd:    fmt.Sprintf("%sb", ovsBridge),
+	}
 }
 
 // startParentPortBridge creates veth pair (if doesn't exist), creates OVS bridge (if doesn't exist) and
 // connects veth pair to parent bridge and OVS bridge.
 func (n *ovn) startParentPortBridge(parentNet Network) error {
-	vars, err := n.parentPortBridgeVars(parentNet)
-	if err != nil {
-		return err
-	}
+	vars := n.parentPortBridgeVars(parentNet)
 
 	// Lock parent network so that if multiple OVN networks are trying to connect to the same parent we don't
 	// race each other setting up the connection.
@@ -562,7 +540,7 @@ func (n *ovn) startParentPortBridge(parentNet Network) error {
 	}
 
 	// Ensure correct sysctls are set on uplink veth interfaces to avoid getting IPv6 link-local addresses.
-	_, err = shared.RunCommand("sysctl",
+	_, err := shared.RunCommand("sysctl",
 		fmt.Sprintf("net.ipv6.conf.%s.disable_ipv6=1", vars.parentEnd),
 		fmt.Sprintf("net.ipv6.conf.%s.disable_ipv6=1", vars.ovsEnd),
 		fmt.Sprintf("net.ipv6.conf.%s.forwarding=0", vars.parentEnd),
@@ -657,11 +635,7 @@ func (n *ovn) deleteParentPortBridge(parentNet Network) error {
 
 	// Check OVS uplink bridge exists, if it does, check how many ports it has.
 	removeVeths := false
-	vars, err := n.parentPortBridgeVars(parentNet)
-	if err != nil {
-		return err
-	}
-
+	vars := n.parentPortBridgeVars(parentNet)
 	if shared.PathExists(fmt.Sprintf("/sys/class/net/%s", vars.ovsBridge)) {
 		ovs := openvswitch.NewOVS()
 		ports, err := ovs.BridgePortList(vars.ovsBridge)

From f9c37e0c92a2ea6e7de2e4560e5b4e34abe30136 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Sep 2020 10:49:01 +0100
Subject: [PATCH 07/10] lxd/network/driver/ovn: Removes unused import

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 3d902aa9b2..798f3aa943 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -19,7 +19,6 @@ import (
 	"github.com/lxc/lxd/lxd/db"
 	"github.com/lxc/lxd/lxd/locking"
 	"github.com/lxc/lxd/lxd/network/openvswitch"
-	"github.com/lxc/lxd/lxd/project"
 	"github.com/lxc/lxd/lxd/revert"
 	"github.com/lxc/lxd/lxd/util"
 	"github.com/lxc/lxd/shared"

From a18d44cd691cd764a422f548db21241a90c8435e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Sep 2020 10:49:13 +0100
Subject: [PATCH 08/10] lxd/network/driver/ovn: Removes unnecessary network ID
 lookup

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

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 798f3aa943..b9e5276789 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -361,12 +361,7 @@ func (n *ovn) setupParentPortBridge(parentNet Network, routerMAC net.HardwareAdd
 				n.config[ovnVolatileParentIPv6] = routerExtPortIPv6.String()
 			}
 
-			networkID, err := tx.GetNetworkID(n.name)
-			if err != nil {
-				return errors.Wrapf(err, "Failed to get network ID for network %q", n.name)
-			}
-
-			err = tx.UpdateNetwork(networkID, n.description, n.config)
+			err = tx.UpdateNetwork(n.id, n.description, n.config)
 			if err != nil {
 				return errors.Wrapf(err, "Failed saving allocated parent network IPs")
 			}

From 7d593840f66105f39e8c9ac3583a590722513a2c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 4 Sep 2020 16:03:14 +0100
Subject: [PATCH 09/10] lxd/api/cluster: Start networks after cluster join

To apply settings after join has happened.

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

diff --git a/lxd/api_cluster.go b/lxd/api_cluster.go
index a9345985c4..e2e85c1872 100644
--- a/lxd/api_cluster.go
+++ b/lxd/api_cluster.go
@@ -550,6 +550,13 @@ func clusterPutJoin(d *Daemon, req api.ClusterPut) response.Response {
 			}
 		}
 
+		// Start up networks so any post-join changes can be applied now that we have a Node ID.
+		logger.Debug("Starting networks after cluster join")
+		err = networkStartup(d.State())
+		if err != nil {
+			logger.Errorf("Failed starting networks: %v", err)
+		}
+
 		client, err = cluster.Connect(req.ClusterAddress, d.endpoints.NetworkCert(), true)
 		if err != nil {
 			return err

From 0de8cc880acc823403fb4db16be0a0aebefed807 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Sep 2020 11:14:42 +0100
Subject: [PATCH 10/10] lxd/networks: Only call n.Start() during
 doNetworksCreate if client type isn't joiner

This is to avoid starting networks that rely on clustered state (such as network ID or cluster certficates) from starting on the joining node with non-clustered state.

In the case of OVN networks this can result in the local OVS uplink bridge name being derived from the local network ID which is invalid once the cluster join has completed.

Additionally it can cause the incorrect HA chassis group name to be used (which is also derived from the network ID).

For bridge networks it can cause the bridge interface's stable random MAC address to be incorrectly derived from the local server cert.

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

diff --git a/lxd/networks.go b/lxd/networks.go
index df4f027f33..b22d6c27ce 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -364,10 +364,14 @@ func doNetworksCreate(d *Daemon, req api.NetworksPost, clientType cluster.Client
 		return err
 	}
 
-	err = n.Start()
-	if err != nil {
-		n.Delete(clientType)
-		return err
+	// Only start networks when not doing a cluster pre-join phase (this ensures that networks are only started
+	// once the node has fully joined the clustered database and has consistent config with rest of the nodes).
+	if clientType != cluster.ClientTypeJoiner {
+		err = n.Start()
+		if err != nil {
+			n.Delete(clientType)
+			return err
+		}
 	}
 
 	return nil


More information about the lxc-devel mailing list