[lxc-devel] [lxd/master] Network: Clustering state avoid duplicate global config when doing re-create

tomponline on Github lxc-bot at linuxcontainers.org
Fri Dec 11 17:26:13 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 486 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201211/a36fda2f/attachment.bin>
-------------- next part --------------
From 689ca1fd3da951dcc790ed8d7fdcfd5b3c3fb3f1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 11 Dec 2020 17:19:49 +0000
Subject: [PATCH 1/7] lxd/db/networks: Adds duplicate key detection to
 getNetworkConfig

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

diff --git a/lxd/db/networks.go b/lxd/db/networks.go
index dece3638a6..e6f75119ca 100644
--- a/lxd/db/networks.go
+++ b/lxd/db/networks.go
@@ -670,6 +670,11 @@ func (c *Cluster) getNetworkConfig(id int64) (map[string]string, error) {
 		key = r[0].(string)
 		value = r[1].(string)
 
+		_, found := config[key]
+		if found {
+			return nil, fmt.Errorf("Duplicate config row found for key %q for network ID %d", key, id)
+		}
+
 		config[key] = value
 	}
 

From fec13a6a73b02ea609cdca005ece4538ae758615 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 11 Dec 2020 17:20:04 +0000
Subject: [PATCH 2/7] lxd/network/driver/ovn: Reject instance port start if
 cannot find DHCP options

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

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 4d7a6a3851..6cd162accb 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -2151,6 +2151,10 @@ func (n *ovn) InstanceDevicePortAdd(instanceUUID string, instanceName string, de
 		if err != nil {
 			return "", err
 		}
+
+		if dhcpV4ID == "" {
+			return "", fmt.Errorf("Could not find DHCPv4 options for instance port")
+		}
 	}
 
 	if dhcpv6Subnet != nil {
@@ -2159,6 +2163,10 @@ func (n *ovn) InstanceDevicePortAdd(instanceUUID string, instanceName string, de
 			return "", err
 		}
 
+		if dhcpV4ID == "" {
+			return "", fmt.Errorf("Could not find DHCPv6 options for instance port")
+		}
+
 		// If port isn't going to have fully dynamic IPs allocated by OVN, and instead only static IPv4
 		// addresses have been added, then add an EUI64 static IPv6 address so that the switch port has an
 		// IPv6 address that will be used to generate a DNS record. This works around a limitation in OVN

From c3c36e878ad6b02078b8dace1892527bec553e73 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 11 Dec 2020 17:20:33 +0000
Subject: [PATCH 3/7] lxd/networks: doNetworksCreate usage

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

diff --git a/lxd/networks.go b/lxd/networks.go
index daf238facf..cf28c7d846 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -198,9 +198,14 @@ func networksPost(d *Daemon, r *http.Request) response.Response {
 	clientType := request.UserAgentClientType(r.Header.Get("User-Agent"))
 
 	if isClusterNotification(r) {
+		n, err := network.LoadByName(d.State(), projectName, req.Name)
+		if err != nil {
+			return response.SmartError(err)
+		}
+
 		// This is an internal request which triggers the actual creation of the network across all nodes
 		// after they have been previously defined.
-		err = doNetworksCreate(d, projectName, req, clientType)
+		err = doNetworksCreate(d, n, clientType)
 		if err != nil {
 			return response.SmartError(err)
 		}

From 319053c314e310ef561c5de381d4ed5e118b0e86 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 11 Dec 2020 17:20:52 +0000
Subject: [PATCH 4/7] lxd/networks: When auto creating pending nodes, don't
 pass global config into DB function in networksPost

We don't want to store global config yet and this can cause duplicates.

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

diff --git a/lxd/networks.go b/lxd/networks.go
index cf28c7d846..7701b03d6c 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -256,7 +256,8 @@ func networksPost(d *Daemon, r *http.Request) response.Response {
 				}
 
 				for _, node := range nodes {
-					err = tx.CreatePendingNetwork(node.Name, projectName, req.Name, netType.DBType(), req.Config)
+					// Don't pass in any node-specific config.
+					err = tx.CreatePendingNetwork(node.Name, projectName, req.Name, netType.DBType(), nil)
 					if err != nil && errors.Cause(err) != db.ErrAlreadyDefined {
 						return errors.Wrapf(err, "Failed creating pending network for node %q", node.Name)
 					}

From 5450215ae9344dcb5495420bdb402f1d82b00ee0 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 11 Dec 2020 17:21:35 +0000
Subject: [PATCH 5/7] lxd/networks: doNetworksCreate usage

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

diff --git a/lxd/networks.go b/lxd/networks.go
index 7701b03d6c..3f334e5a8d 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -304,7 +304,12 @@ func networksPost(d *Daemon, r *http.Request) response.Response {
 	}
 	revert.Add(func() { d.cluster.DeleteNetwork(projectName, req.Name) })
 
-	err = doNetworksCreate(d, projectName, req, clientType)
+	n, err := network.LoadByName(d.State(), projectName, req.Name)
+	if err != nil {
+		return response.SmartError(err)
+	}
+
+	err = doNetworksCreate(d, n, clientType)
 	if err != nil {
 		return response.SmartError(err)
 	}

From bda2c2d9ffc3e2e68779e624df8e20184e6bc4b7 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 11 Dec 2020 17:22:12 +0000
Subject: [PATCH 6/7] lxd/networks: Updates networksPostCluster to detect
 existing global config and skip create if needed

Allows network re-creation attempts after failed creation without causing duplicate global config.

Also re-works notifications so that stored global and node config is used, rather than what was sent in the subsequent request (will be the same on first time).

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

diff --git a/lxd/networks.go b/lxd/networks.go
index 3f334e5a8d..fef87b60e8 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -339,14 +339,8 @@ func networksPostCluster(d *Daemon, projectName string, req api.NetworksPost, cl
 		return fmt.Errorf("Requested network type %q doesn't match type in existing database record %q", req.Type, netInfo.Type)
 	}
 
-	// Add default values.
-	err = netType.FillConfig(req.Config)
-	if err != nil {
-		return err
-	}
-
 	// Check that the network is properly defined, get the node-specific configs and merge with global config.
-	var configs map[string]map[string]string
+	var nodeConfigs map[string]map[string]string
 	var nodeName string
 	var networkID int64
 	err = d.cluster.Transaction(func(tx *db.ClusterTx) error {
@@ -357,7 +351,7 @@ func networksPostCluster(d *Daemon, projectName string, req api.NetworksPost, cl
 		}
 
 		// Fetch the node-specific configs.
-		configs, err = tx.NetworkNodeConfigs(networkID)
+		nodeConfigs, err = tx.NetworkNodeConfigs(networkID)
 		if err != nil {
 			return err
 		}
@@ -368,6 +362,20 @@ func networksPostCluster(d *Daemon, projectName string, req api.NetworksPost, cl
 			return err
 		}
 
+		// Check if any global config exists already, if so we should not create global config again.
+		for key := range netInfo.Config {
+			if !shared.StringInSlice(key, db.NodeSpecificNetworkConfig) {
+				logger.Warn("Skipping global network create as global config already exists", log.Ctx{"project": projectName, "network": req.Name})
+				return nil
+			}
+		}
+
+		// Add default values if we are inserting global config for first time.
+		err = netType.FillConfig(req.Config)
+		if err != nil {
+			return err
+		}
+
 		// Insert the global config keys.
 		return tx.CreateNetworkConfig(networkID, 0, req.Config)
 	})
@@ -385,15 +393,13 @@ func networksPostCluster(d *Daemon, projectName string, req api.NetworksPost, cl
 		return err
 	}
 
-	// Create the network on this node.
-	nodeReq := req
-
-	// Merge node specific config items into global config.
-	for key, value := range configs[nodeName] {
-		nodeReq.Config[key] = value
+	// Load the network from the database for the local node.
+	n, err := network.LoadByName(d.State(), projectName, req.Name)
+	if err != nil {
+		return err
 	}
 
-	err = doNetworksCreate(d, projectName, nodeReq, clientType)
+	err = doNetworksCreate(d, n, clientType)
 	if err != nil {
 		return err
 	}
@@ -406,18 +412,31 @@ func networksPostCluster(d *Daemon, projectName string, req api.NetworksPost, cl
 			return err
 		}
 
-		nodeReq := req
+		// Create fresh request based on existing network to send to node.
+		nodeReq := api.NetworksPost{
+			NetworkPut: api.NetworkPut{
+				Config:      n.Config(),
+				Description: n.Description(),
+			},
+			Name: n.Name(),
+			Type: n.Type(),
+		}
+
+		// Remove node-specific config keys.
+		for _, key := range db.NodeSpecificNetworkConfig {
+			delete(nodeReq.Config, key)
+		}
 
 		// Merge node specific config items into global config.
-		for key, value := range configs[server.Environment.ServerName] {
+		for key, value := range nodeConfigs[server.Environment.ServerName] {
 			nodeReq.Config[key] = value
 		}
 
-		err = client.UseProject(projectName).CreateNetwork(nodeReq)
+		err = client.UseProject(n.Project()).CreateNetwork(nodeReq)
 		if err != nil {
 			return err
 		}
-		logger.Debug("Created network on cluster member", log.Ctx{"project": projectName, "network": req.Name, "member": server.Environment.ServerName})
+		logger.Debug("Created network on cluster member", log.Ctx{"project": n.Project(), "network": n.Name(), "member": server.Environment.ServerName, "config": nodeReq.Config})
 
 		return nil
 	})

From 880a616d29b8d8570f2fd824ce7003d75e884451 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 11 Dec 2020 17:23:37 +0000
Subject: [PATCH 7/7] lxd/networks: Updates doNetworksCreate to accept a
 Network rather than load its own

This is useful as it can be preloaded and used in other places to avoid duplicate queries.

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

diff --git a/lxd/networks.go b/lxd/networks.go
index fef87b60e8..690c66f170 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -458,24 +458,18 @@ func networksPostCluster(d *Daemon, projectName string, req api.NetworksPost, cl
 
 // Create the network on the system. The clusterNotification flag is used to indicate whether creation request
 // is coming from a cluster notification (and if so we should not delete the database record on error).
-func doNetworksCreate(d *Daemon, projectName string, req api.NetworksPost, clientType request.ClientType) error {
+func doNetworksCreate(d *Daemon, n network.Network, clientType request.ClientType) error {
 	revert := revert.New()
 	defer revert.Fail()
 
-	// Start the network.
-	n, err := network.LoadByName(d.State(), projectName, req.Name)
-	if err != nil {
-		return err
-	}
-
 	// Validate so that when run on a cluster node the full config (including node specific config) is checked.
-	err = n.Validate(n.Config())
+	err := n.Validate(n.Config())
 	if err != nil {
 		return err
 	}
 
 	if n.LocalStatus() == api.NetworkStatusCreated {
-		logger.Debug("Skipping network create as already created locally", log.Ctx{"project": projectName, "network": n.Name()})
+		logger.Debug("Skipping network create as already created locally", log.Ctx{"project": n.Project(), "network": n.Name()})
 		return nil
 	}
 
@@ -503,7 +497,7 @@ func doNetworksCreate(d *Daemon, projectName string, req api.NetworksPost, clien
 	if err != nil {
 		return err
 	}
-	logger.Debug("Marked network local status as created", log.Ctx{"project": projectName, "network": req.Name})
+	logger.Debug("Marked network local status as created", log.Ctx{"project": n.Project(), "network": n.Name()})
 
 	revert.Success()
 	return nil


More information about the lxc-devel mailing list