[lxc-devel] [lxd/master] Storage: Clustering state avoid duplicate global config when doing re-create
tomponline on Github
lxc-bot at linuxcontainers.org
Mon Dec 14 14:23:46 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 639 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201214/aff92c11/attachment-0001.bin>
-------------- next part --------------
From f5d6d54f2b257002e100e989faeb651c9ba02cdb 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 01/32] 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 849aabe23e60d7186a9f56c93606f903297da988 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:06:57 +0000
Subject: [PATCH 02/32] lxd/db/networks: Adds NetworkErrored function
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 e6f75119ca..dbc6e93a14 100644
--- a/lxd/db/networks.go
+++ b/lxd/db/networks.go
@@ -323,6 +323,11 @@ func (c *ClusterTx) NetworkCreated(project string, name string) error {
return c.networkState(project, name, networkCreated)
}
+// NetworkErrored sets the state of the given network to networkErrored.
+func (c *ClusterTx) NetworkErrored(project string, name string) error {
+ return c.networkState(project, name, networkErrored)
+}
+
func (c *ClusterTx) networkState(project string, name string, state NetworkState) error {
stmt := "UPDATE networks SET state=? WHERE project_id = (SELECT id FROM projects WHERE name = ?) AND name=?"
result, err := c.tx.Exec(stmt, state, project, name)
From ffc6845170d5e31b6df042a052c2518b4e464c83 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:07:11 +0000
Subject: [PATCH 03/32] lxd/db/networks: Changes UpdateNetwork to not set
created status
We shouldn't be allowing updates on non-created networks anyway.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/networks.go | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/lxd/db/networks.go b/lxd/db/networks.go
index dbc6e93a14..a675b01f0c 100644
--- a/lxd/db/networks.go
+++ b/lxd/db/networks.go
@@ -725,7 +725,7 @@ func (c *Cluster) CreateNetwork(projectName string, name string, description str
// UpdateNetwork updates the network with the given name.
func (c *Cluster) UpdateNetwork(project string, name, description string, config map[string]string) error {
- id, netInfo, _, err := c.GetNetworkInAnyState(project, name)
+ id, _, _, err := c.GetNetworkInAnyState(project, name)
if err != nil {
return err
}
@@ -736,14 +736,6 @@ func (c *Cluster) UpdateNetwork(project string, name, description string, config
return err
}
- // Update network status if change applied successfully.
- if netInfo.Status == api.NetworkStatusErrored {
- err = tx.NetworkCreated(project, name)
- if err != nil {
- return err
- }
- }
-
return nil
})
From 9f070059265055cbf509e0b75c78a04315e2fa67 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 04/32] 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 2befe9020253c0fa30b783cb51185234e1942657 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 05/32] 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 daf238facf..8aedb31fe2 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -428,24 +428,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
}
@@ -473,7 +467,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
From 690604735c86b2b4504294d9ba4a375975dc690d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:34:18 +0000
Subject: [PATCH 06/32] lxd/networks: Debug log consistency in doNetworksCreate
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/networks.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/networks.go b/lxd/networks.go
index 8aedb31fe2..cfb961e92e 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -439,7 +439,7 @@ func doNetworksCreate(d *Daemon, n network.Network, clientType request.ClientTyp
}
if n.LocalStatus() == api.NetworkStatusCreated {
- logger.Debug("Skipping network create as already created locally", log.Ctx{"project": n.Project(), "network": n.Name()})
+ logger.Debug("Skipping local network create as already created", log.Ctx{"project": n.Project(), "network": n.Name()})
return nil
}
From 06c96205c41ff05b6d3f2e0a0fc61913b80e2dca 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 07/32] lxd/networks: doNetworksCreate usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/networks.go | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/lxd/networks.go b/lxd/networks.go
index cfb961e92e..89f8e469e8 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)
}
@@ -298,7 +303,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 4d7df81acbf8224857fdf74f9e8ac8bf4cd323cb 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 08/32] 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 | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lxd/networks.go b/lxd/networks.go
index 89f8e469e8..813129c443 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -256,7 +256,9 @@ 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 config, as these nodes don't have any node-specific
+ // config and we don't want to create duplicate global 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 909778e8ad6a79ee8940ddb213ec95b8adae938d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:32:58 +0000
Subject: [PATCH 09/32] lxd/networks: Adds networkPartiallyCreated helper
function
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/networks.go | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/lxd/networks.go b/lxd/networks.go
index 813129c443..4aaf831ea9 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -319,6 +319,26 @@ func networksPost(d *Daemon, r *http.Request) response.Response {
return resp
}
+// networkPartiallyCreated returns true of supplied network has properties that indicate it has had previous
+// create attempts run on it but failed on one or more nodes.
+func networkPartiallyCreated(netInfo *api.Network) bool {
+ // If the network status is NetworkStatusErrored, this means create has been run in the past and has
+ // failed on one or more nodes. Hence it is partially created.
+ if netInfo.Status == api.NetworkStatusErrored {
+ return true
+ }
+
+ // If the network has global config keys, then it has previously been created by having its global config
+ // inserted, and this means it is partialled created.
+ for key := range netInfo.Config {
+ if !shared.StringInSlice(key, db.NodeSpecificNetworkConfig) {
+ return true
+ }
+ }
+
+ return false
+}
+
// networksPostCluster checks that there is a pending network in the database and then attempts to setup the
// network on each node. If all nodes are successfully setup then the network's state is set to created.
func networksPostCluster(d *Daemon, projectName string, req api.NetworksPost, clientType request.ClientType, netType network.Type) error {
From 953d5d19d1bb73897eac948f838311828de4b0e7 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 10/32] lxd/networks: Updates networksPostCluster to detect
existing global config and skip create if already exists
Rejects request if global config has been supllied.
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).
Also updates networksPostCluster to check for existing created network and set status to errored on failure.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/networks.go | 83 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 55 insertions(+), 28 deletions(-)
diff --git a/lxd/networks.go b/lxd/networks.go
index 4aaf831ea9..0df16be002 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -349,48 +349,64 @@ func networksPostCluster(d *Daemon, projectName string, req api.NetworksPost, cl
}
}
- // Check that the requested network type matches the type created when adding the local node config.
+ // Perform sanity checks if network already exists.
// If network doesn't exist yet, ignore not found error, as this will be checked by NetworkNodeConfigs().
_, netInfo, _, err := d.cluster.GetNetworkInAnyState(projectName, req.Name)
if err != nil && err != db.ErrNoSuchObject {
return err
}
- if err != db.ErrNoSuchObject && req.Type != netInfo.Type {
- return fmt.Errorf("Requested network type %q doesn't match type in existing database record %q", req.Type, netInfo.Type)
- }
+ if netInfo != nil {
+ // Check network isn't already created.
+ if netInfo.Status == api.NetworkStatusCreated {
+ return fmt.Errorf("The network is already created")
+ }
- // Add default values.
- err = netType.FillConfig(req.Config)
- if err != nil {
- return err
+ // Check the requested network type matches the type created when adding the local node config.
+ if req.Type != netInfo.Type {
+ return fmt.Errorf("Requested network type %q doesn't match type in existing database record %q", req.Type, netInfo.Type)
+ }
}
// 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 nodeName string
- var networkID int64
+ var nodeConfigs map[string]map[string]string
err = d.cluster.Transaction(func(tx *db.ClusterTx) error {
+ // Check if any global config exists already, if so we should not create global config again.
+ if netInfo != nil && networkPartiallyCreated(netInfo) {
+ if len(req.Config) > 0 {
+ return fmt.Errorf("Network already partially created. Please do not specify any global config when re-running create")
+ }
+
+ logger.Debug("Skipping global network create as global config already partially created", log.Ctx{"project": projectName, "network": req.Name})
+ return nil
+ }
+
// Fetch the network ID.
- networkID, err = tx.GetNetworkID(projectName, req.Name)
+ networkID, err := tx.GetNetworkID(projectName, req.Name)
if err != nil {
return err
}
- // Fetch the node-specific configs.
- configs, err = tx.NetworkNodeConfigs(networkID)
+ // Fetch the node-specific configs and check the network is defined for all nodes.
+ nodeConfigs, err = tx.NetworkNodeConfigs(networkID)
if err != nil {
return err
}
- // Take note of the name of this node.
- nodeName, err = tx.GetLocalNodeName()
+ // 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)
+ err = tx.CreateNetworkConfig(networkID, 0, req.Config)
+ if err != nil {
+ return err
+ }
+
+ // Assume failure unless we succeed later on.
+ return tx.NetworkErrored(projectName, req.Name)
})
if err != nil {
if err == db.ErrNoSuchObject {
@@ -406,15 +422,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
}
@@ -427,18 +441,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 b2662f5538fe2490e0d6c8516bd7421f83675ee0 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 11 Dec 2020 19:00:49 +0000
Subject: [PATCH 11/32] lxd/patches: Adds patchNetworkFixDupeConfig function
This function removes any duplicate rows in the networks_config table.
It has been possible to create duplicate rows when using OVN networks, as the auto pending nodes created were passing in global config which was then duplicated.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/patches.go | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/lxd/patches.go b/lxd/patches.go
index 8749e2a8f3..fbce54d635 100644
--- a/lxd/patches.go
+++ b/lxd/patches.go
@@ -117,6 +117,7 @@ var patches = []patch{
{name: "network_fan_enable_nat", stage: patchPostDaemonStorage, run: patchNetworkFANEnableNAT},
{name: "thinpool_typo_fix", stage: patchPostDaemonStorage, run: patchThinpoolTypoFix},
{name: "vm_rename_uuid_key", stage: patchPostDaemonStorage, run: patchVMRenameUUIDKey},
+ {name: "network_fix_dupe_config", stage: patchPostDaemonStorage, run: patchNetworkFixDupeConfig},
}
type patch struct {
@@ -181,6 +182,91 @@ func patchesApply(d *Daemon, stage patchStage) error {
// Patches begin here
+// patchNetworkFixDupeConfig removes any duplicated network config rows that have the same value.
+// This can occur when multiple create requests have been issued when setting up a clustered network.
+func patchNetworkFixDupeConfig(name string, d *Daemon) error {
+ revert := revert.New()
+ defer revert.Fail()
+
+ tx, err := d.cluster.Begin()
+ if err != nil {
+ return errors.Wrap(err, "Failed to begin transaction")
+ }
+
+ revert.Add(func() { tx.Rollback() })
+
+ // Find all duplicated config rows and return comma delimited list of affected row IDs for each dupe set.
+ stmt, err := tx.Prepare(`SELECT network_id, COALESCE(node_id,0), key, value, COUNT(*) AS rowCount, GROUP_CONCAT(id, ",") AS dupeRowIDs
+ FROM networks_config
+ GROUP BY network_id, node_id, key, value
+ HAVING rowCount > 1
+ `)
+ if err != nil {
+ return errors.Wrapf(err, "Failed preparing query")
+ }
+ defer stmt.Close()
+
+ rows, err := stmt.Query()
+ if err != nil {
+ return errors.Wrapf(err, "Failed running query")
+ }
+ defer rows.Close()
+
+ type dupeRow struct {
+ networkID int64
+ nodeID int64
+ key string
+ value string
+ rowCount int64
+ dupeRowIDs string
+ }
+
+ var dupeRows []dupeRow
+
+ for rows.Next() {
+ r := dupeRow{}
+ err = rows.Scan(&r.networkID, &r.nodeID, &r.key, &r.value, &r.rowCount, &r.dupeRowIDs)
+ if err != nil {
+ return errors.Wrapf(err, "Failed scanning rows")
+ }
+
+ dupeRows = append(dupeRows, r)
+ }
+
+ err = rows.Err()
+ if err != nil {
+ return errors.Wrap(err, "Got a row error")
+ }
+
+ for _, r := range dupeRows {
+ logger.Warn("Found duplicated network config rows", log.Ctx{"networkID": r.networkID, "nodeID": r.nodeID, "key": r.key, "value": r.value, "rowCount": r.rowCount, "dupeRowIDs": r.dupeRowIDs})
+
+ rowIDs := strings.Split(r.dupeRowIDs, ",")
+
+ // Iterate and delete all but 1 of the rowIDs so we leave just one left.
+ for i := 0; i < len(rowIDs)-1; i++ {
+ rowID, err := strconv.Atoi(rowIDs[i])
+ if err != nil {
+ return errors.Wrapf(err, "Failed converting row ID")
+ }
+
+ _, err = tx.Exec("DELETE FROM networks_config WHERE id = ?", rowID)
+ if err != nil {
+ return errors.Wrapf(err, "Failed deleting network config row with ID %d", rowID)
+ }
+ logger.Warn("Deleted duplicated network config row", log.Ctx{"networkID": r.networkID, "nodeID": r.nodeID, "key": r.key, "value": r.value, "rowCount": r.rowCount, "rowID": rowID})
+ }
+ }
+
+ err = tx.Commit()
+ if err != nil {
+ return errors.Wrap(err, "Failed to commit transaction")
+ }
+
+ revert.Success()
+ return nil
+}
+
// patchVMRenameUUIDKey renames the volatile.vm.uuid key to volatile.uuid in instance and snapshot configs.
func patchVMRenameUUIDKey(name string, d *Daemon) error {
oldUUIDKey := "volatile.vm.uuid"
From 5191c8fdf2e038bfbec33332cdea53994c6e4c60 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:06:37 +0000
Subject: [PATCH 12/32] lxd/api/cluster: Skip non-created networks when joining
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/api_cluster.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/api_cluster.go b/lxd/api_cluster.go
index b9d32cbfc3..1d804e5cf2 100644
--- a/lxd/api_cluster.go
+++ b/lxd/api_cluster.go
@@ -793,7 +793,7 @@ func clusterInitMember(d lxd.InstanceServer, client lxd.InstanceServer, memberCo
// Merge the returned networks configs with the node-specific configs provided by the user.
for _, network := range networks {
// Skip unmanaged or pending networks.
- if !network.Managed || network.Status == api.NetworkStatusPending {
+ if !network.Managed || network.Status != api.NetworkStatusCreated {
continue
}
From a74bd71e3e2fdbd3aefa5b09d87965c0b427e567 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:32:35 +0000
Subject: [PATCH 13/32] lxd/device/nic: Don't allow NICs to use networks that
are not created
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/nic_bridged.go | 2 +-
lxd/device/nic_macvlan.go | 2 +-
lxd/device/nic_ovn.go | 2 +-
lxd/device/nic_sriov.go | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/lxd/device/nic_bridged.go b/lxd/device/nic_bridged.go
index 8e43cdfbce..1cc25beecb 100644
--- a/lxd/device/nic_bridged.go
+++ b/lxd/device/nic_bridged.go
@@ -88,7 +88,7 @@ func (d *nicBridged) validateConfig(instConf instance.ConfigReader) error {
return errors.Wrapf(err, "Error loading network config for %q", d.config["network"])
}
- if n.Status() == api.NetworkStatusPending {
+ if n.Status() != api.NetworkStatusCreated {
return fmt.Errorf("Specified network is not fully created")
}
diff --git a/lxd/device/nic_macvlan.go b/lxd/device/nic_macvlan.go
index d40f3f6629..c389eb2d51 100644
--- a/lxd/device/nic_macvlan.go
+++ b/lxd/device/nic_macvlan.go
@@ -56,7 +56,7 @@ func (d *nicMACVLAN) validateConfig(instConf instance.ConfigReader) error {
return errors.Wrapf(err, "Error loading network config for %q", d.config["network"])
}
- if n.Status() == api.NetworkStatusPending {
+ if n.Status() != api.NetworkStatusCreated {
return fmt.Errorf("Specified network is not fully created")
}
diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index a7cf93d04f..038439f30e 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -87,7 +87,7 @@ func (d *nicOVN) validateConfig(instConf instance.ConfigReader) error {
return errors.Wrapf(err, "Error loading network config for %q", d.config["network"])
}
- if n.Status() == api.NetworkStatusPending {
+ if n.Status() != api.NetworkStatusCreated {
return fmt.Errorf("Specified network is not fully created")
}
diff --git a/lxd/device/nic_sriov.go b/lxd/device/nic_sriov.go
index 70d0cabc0f..8c3b724816 100644
--- a/lxd/device/nic_sriov.go
+++ b/lxd/device/nic_sriov.go
@@ -66,7 +66,7 @@ func (d *nicSRIOV) validateConfig(instConf instance.ConfigReader) error {
return errors.Wrapf(err, "Error loading network config for %q", d.config["network"])
}
- if n.Status() == api.NetworkStatusPending {
+ if n.Status() != api.NetworkStatusCreated {
return fmt.Errorf("Specified network is not fully created")
}
From 05217e4d5cf56964de2c7ec6a0bd3a00d8637254 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:45:35 +0000
Subject: [PATCH 14/32] lxd/db/networks: Renames ClusterTx
GetNonPendingNetworks to GetCreatedNetworks
And updates to return only networks in created state.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/networks.go | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/lxd/db/networks.go b/lxd/db/networks.go
index a675b01f0c..619ba11eb1 100644
--- a/lxd/db/networks.go
+++ b/lxd/db/networks.go
@@ -68,21 +68,20 @@ func (c *ClusterTx) GetNonPendingNetworkIDs() (map[string]int64, error) {
return ids, nil
}
-// GetNonPendingNetworks returns a map of api.Network associated to project and network ID.
-//
-// Pending networks are skipped.
-func (c *ClusterTx) GetNonPendingNetworks() (map[string]map[int64]api.Network, error) {
+// GetCreatedNetworks returns a map of api.Network associated to project and network ID.
+// Only networks that have are in state networkCreated are returned.
+func (c *ClusterTx) GetCreatedNetworks() (map[string]map[int64]api.Network, error) {
stmt, err := c.tx.Prepare(`SELECT projects.name, networks.id, networks.name, coalesce(networks.description, ''), networks.type, networks.state
FROM networks
JOIN projects on projects.id = networks.project_id
- WHERE networks.state != ?
+ WHERE networks.state = ?
`)
if err != nil {
return nil, err
}
defer stmt.Close()
- rows, err := stmt.Query(networkPending)
+ rows, err := stmt.Query(networkCreated)
if err != nil {
return nil, err
}
From 27ff98d9806793aa7c5e12b6885d661cc8586321 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:46:07 +0000
Subject: [PATCH 15/32] lxd/db/networks: Renames Cluster GetNonPendingNetworks
to GetCreatedNetworks
And updates to return only networks in created state.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/networks.go | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lxd/db/networks.go b/lxd/db/networks.go
index 619ba11eb1..0d003e5a2b 100644
--- a/lxd/db/networks.go
+++ b/lxd/db/networks.go
@@ -422,9 +422,9 @@ func (c *Cluster) GetNetworks(project string) ([]string, error) {
return c.networks(project, "")
}
-// GetNonPendingNetworks returns the names of all networks that are not in state networkPending.
-func (c *Cluster) GetNonPendingNetworks(project string) ([]string, error) {
- return c.networks(project, "NOT state=?", networkPending)
+// GetCreatedNetworks returns the names of all networks that are not in state networkCreated.
+func (c *Cluster) GetCreatedNetworks(project string) ([]string, error) {
+ return c.networks(project, "state=?", networkCreated)
}
// Get all networks matching the given WHERE filter (if given).
From 93f19aa218869439df5fb94d67a0b2b56d58e386 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:46:32 +0000
Subject: [PATCH 16/32] lxd/api/cluster: cluster.GetCreatedNetworks usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/api_cluster.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/api_cluster.go b/lxd/api_cluster.go
index 1d804e5cf2..9da22e2b96 100644
--- a/lxd/api_cluster.go
+++ b/lxd/api_cluster.go
@@ -1568,7 +1568,7 @@ func clusterCheckNetworksMatch(cluster *db.Cluster, reqNetworks []internalCluste
}
for _, networkProjectName := range networkProjectNames {
- networkNames, err := cluster.GetNonPendingNetworks(networkProjectName)
+ networkNames, err := cluster.GetCreatedNetworks(networkProjectName)
if err != nil && err != db.ErrNoSuchObject {
return err
}
From 4a0fe98f5caf1e105cb65c456479ce8be23bb0fe Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:46:54 +0000
Subject: [PATCH 17/32] lxd/network: tx.GetCreatedNetworks usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/network/driver_ovn.go | 8 ++++----
lxd/network/driver_physical.go | 2 +-
lxd/network/network_utils.go | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 6cd162accb..0339cd3732 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -252,7 +252,7 @@ func (n *ovn) Validate(config map[string]string) error {
var projectNetworks map[string]map[int64]api.Network
err = n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
// Get all managed networks across all projects.
- projectNetworks, err = tx.GetNonPendingNetworks()
+ projectNetworks, err = tx.GetCreatedNetworks()
if err != nil {
return errors.Wrapf(err, "Failed to load all networks")
}
@@ -753,7 +753,7 @@ func (n *ovn) allocateUplinkPortIPs(uplinkNet Network, routerMAC net.HardwareAdd
// uplinkAllAllocatedIPs gets a list of all IPv4 and IPv6 addresses allocated to OVN networks connected to uplink.
func (n *ovn) uplinkAllAllocatedIPs(tx *db.ClusterTx, uplinkNetName string) ([]net.IP, []net.IP, error) {
// Get all managed networks across all projects.
- projectNetworks, err := tx.GetNonPendingNetworks()
+ projectNetworks, err := tx.GetCreatedNetworks()
if err != nil {
return nil, nil, errors.Wrapf(err, "Failed to load all networks")
}
@@ -1051,7 +1051,7 @@ func (n *ovn) checkUplinkUse() (bool, error) {
var projectNetworks map[string]map[int64]api.Network
err = n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
- projectNetworks, err = tx.GetNonPendingNetworks()
+ projectNetworks, err = tx.GetCreatedNetworks()
return err
})
if err != nil {
@@ -2040,7 +2040,7 @@ func (n *ovn) InstanceDevicePortValidateExternalRoutes(deviceInstance instance.I
}
// Get all managed networks across all projects.
- projectNetworks, err = tx.GetNonPendingNetworks()
+ projectNetworks, err = tx.GetCreatedNetworks()
if err != nil {
return errors.Wrapf(err, "Failed to load all networks")
}
diff --git a/lxd/network/driver_physical.go b/lxd/network/driver_physical.go
index 18fcdc3c8f..51d5f16b8c 100644
--- a/lxd/network/driver_physical.go
+++ b/lxd/network/driver_physical.go
@@ -69,7 +69,7 @@ func (n *physical) checkParentUse(ourConfig map[string]string) (bool, error) {
var projectNetworks map[string]map[int64]api.Network
err = n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
- projectNetworks, err = tx.GetNonPendingNetworks()
+ projectNetworks, err = tx.GetCreatedNetworks()
return err
})
if err != nil {
diff --git a/lxd/network/network_utils.go b/lxd/network/network_utils.go
index 819ef26a9d..36b829fb97 100644
--- a/lxd/network/network_utils.go
+++ b/lxd/network/network_utils.go
@@ -157,7 +157,7 @@ func UsedBy(s *state.State, networkProjectName string, networkName string, first
var projectNetworks map[string]map[int64]api.Network
err = s.Cluster.Transaction(func(tx *db.ClusterTx) error {
- projectNetworks, err = tx.GetNonPendingNetworks()
+ projectNetworks, err = tx.GetCreatedNetworks()
return err
})
if err != nil {
From ea61fed6615de041bd1a2f5631d830e0eba320c3 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:47:09 +0000
Subject: [PATCH 18/32] lxd/networks: s.Cluster.GetCreatedNetworks usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/networks.go | 2 +-
lxd/networks_utils.go | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/networks.go b/lxd/networks.go
index 0df16be002..9273f4b269 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -1122,7 +1122,7 @@ func networkStartup(s *state.State) error {
deferredNetworks[projectName] = make([]network.Network, 0)
// Get a list of managed networks.
- networks, err := s.Cluster.GetNonPendingNetworks(projectName)
+ networks, err := s.Cluster.GetCreatedNetworks(projectName)
if err != nil {
return errors.Wrapf(err, "Failed to load networks for project %q", projectName)
}
diff --git a/lxd/networks_utils.go b/lxd/networks_utils.go
index 44d05aeb59..fabae67d4c 100644
--- a/lxd/networks_utils.go
+++ b/lxd/networks_utils.go
@@ -25,7 +25,7 @@ func networkUpdateForkdnsServersTask(s *state.State, heartbeatData *cluster.APIH
projectName := project.Default
// Get a list of managed networks
- networks, err := s.Cluster.GetNonPendingNetworks(projectName)
+ networks, err := s.Cluster.GetCreatedNetworks(projectName)
if err != nil {
return err
}
From fa93eb74e2f63539e554e9fe67332437f36c7264 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:47:24 +0000
Subject: [PATCH 19/32] lxd/patches: patchNetworkFANEnableNAT usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/patches.go | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lxd/patches.go b/lxd/patches.go
index fbce54d635..1f2f66aa21 100644
--- a/lxd/patches.go
+++ b/lxd/patches.go
@@ -390,7 +390,7 @@ INSERT INTO storage_pools_config(storage_pool_id, node_id, key, value)
// having "ipv4.nat" set is to disable NAT (bringing in line with the non-fan bridge behavior and docs).
func patchNetworkFANEnableNAT(name string, d *Daemon) error {
err := d.cluster.Transaction(func(tx *db.ClusterTx) error {
- projectNetworks, err := tx.GetNonPendingNetworks()
+ projectNetworks, err := tx.GetCreatedNetworks()
if err != nil {
return err
}
@@ -437,7 +437,7 @@ func patchNetworkFANEnableNAT(name string, d *Daemon) error {
// networks. It was decided that the OVN NIC level equivalent settings were sufficient.
func patchNetworkOVNRemoveRoutes(name string, d *Daemon) error {
err := d.cluster.Transaction(func(tx *db.ClusterTx) error {
- projectNetworks, err := tx.GetNonPendingNetworks()
+ projectNetworks, err := tx.GetCreatedNetworks()
if err != nil {
return err
}
@@ -487,7 +487,7 @@ func patchNetworkOVNRemoveRoutes(name string, d *Daemon) error {
// patchNetworkCearBridgeVolatileHwaddr removes the unsupported `volatile.bridge.hwaddr` config key from networks.
func patchNetworkOVNEnableNAT(name string, d *Daemon) error {
err := d.cluster.Transaction(func(tx *db.ClusterTx) error {
- projectNetworks, err := tx.GetNonPendingNetworks()
+ projectNetworks, err := tx.GetCreatedNetworks()
if err != nil {
return err
}
From 3153ff57e7bb3ac1dadbaccbfe5a66ad9b1d6246 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:51:58 +0000
Subject: [PATCH 20/32] test/suites/clustering: More network clustering tests
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
test/suites/clustering.sh | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/test/suites/clustering.sh b/test/suites/clustering.sh
index 2596535584..5196354c9e 100644
--- a/test/suites/clustering.sh
+++ b/test/suites/clustering.sh
@@ -834,10 +834,11 @@ test_clustering_network() {
nsenter -n -t "${LXD_PID1}" -- ip link add "${net}" type dummy # Create dummy interface to conflict with network.
LXD_DIR="${LXD_ONE_DIR}" lxc network create "${net}" --target node1
LXD_DIR="${LXD_ONE_DIR}" lxc network create "${net}" --target node2
+ LXD_DIR="${LXD_ONE_DIR}" lxc network show "${net}" | grep status: | grep -q Pending # Check has pending status.
# Run network create on other node1 (expect this to fail early due to existing interface).
! LXD_DIR="${LXD_ONE_DIR}" lxc network create "${net}" || false
- LXD_DIR="${LXD_ONE_DIR}" lxc network show "${net}" | grep status: | grep -q Pending # Check is still pending.
+ LXD_DIR="${LXD_ONE_DIR}" lxc network show "${net}" | grep status: | grep -q Errored # Check has errored status.
# Check each node status (expect both node1 and node2 to be pending as local node running created failed first).
LXD_DIR="${LXD_ONE_DIR}" lxd sql global "SELECT nodes.name,networks_nodes.state FROM nodes JOIN networks_nodes ON networks_nodes.node_id = nodes.id JOIN networks ON networks.id = networks_nodes.network_id WHERE networks.name = '${net}' AND nodes.name = 'node1'" | grep "| node1 | 0 |"
@@ -879,12 +880,14 @@ test_clustering_network() {
LXD_DIR="${LXD_ONE_DIR}" lxc network create "${net}" --target node1
LXD_DIR="${LXD_ONE_DIR}" lxc network create "${net}" --target node2
! LXD_DIR="${LXD_ONE_DIR}" lxc network create "${net}" || false
- LXD_DIR="${LXD_ONE_DIR}" lxc network show "${net}" | grep status: | grep -q Pending # Check is still pending.
+ LXD_DIR="${LXD_ONE_DIR}" lxc network show "${net}" | grep status: | grep -q Errored # Check has errored status.
nsenter -n -t "${LXD_PID1}" -- ip link delete "${net}" # Remove conflicting interface.
- LXD_DIR="${LXD_ONE_DIR}" lxc network create "${net}"
+ ! LXD_DIR="${LXD_ONE_DIR}" lxc network create "${net}" ipv4.dhcp=false || false # Check supplying global config on re-create is blocked.
+ LXD_DIR="${LXD_ONE_DIR}" lxc network create "${net}" # Check re-create succeeds.
LXD_DIR="${LXD_ONE_DIR}" lxc network show "${net}" | grep status: | grep -q Created # Check is created after fix.
nsenter -n -t "${LXD_PID1}" -- ip -details link show "${net}" | grep bridge # Check bridge exists.
nsenter -n -t "${LXD_PID2}" -- ip -details link show "${net}" | grep bridge # Check bridge exists.
+ ! LXD_DIR="${LXD_ONE_DIR}" lxc network create "${net}" || false # Check re-create is blocked after success.
# Check both nodes marked created.
LXD_DIR="${LXD_ONE_DIR}" lxd sql global "SELECT nodes.name,networks_nodes.state FROM nodes JOIN networks_nodes ON networks_nodes.node_id = nodes.id JOIN networks ON networks.id = networks_nodes.network_id WHERE networks.name = '${net}' AND nodes.name = 'node1'" | grep "| node1 | 1 |"
From 1ae6281d11fc56962f2577c5b2fb0de4cd2e373a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 10:34:33 +0000
Subject: [PATCH 21/32] lxd/storage/pools/utils: Debug log consistency in
storagePoolCreateLocal
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_pools_utils.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/storage_pools_utils.go b/lxd/storage_pools_utils.go
index cced35bbdd..1cd7af264e 100644
--- a/lxd/storage_pools_utils.go
+++ b/lxd/storage_pools_utils.go
@@ -126,7 +126,7 @@ func storagePoolCreateLocal(state *state.State, id int64, req api.StoragePoolsPo
}
if pool.LocalStatus() == api.NetworkStatusCreated {
- logger.Debug("Skipping storage pool create as already created locally", log.Ctx{"pool": pool.Name()})
+ logger.Debug("Skipping local storage pool create as already created", log.Ctx{"pool": pool.Name()})
return pool.Driver().Config(), nil
}
From 7bb39eabc98095cc65dc68e27b4784392b09bbe3 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 11 Dec 2020 17:34:19 +0000
Subject: [PATCH 22/32] lxd/db/storage/pools: Adds duplicate key detection to
getStoragePoolConfig
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/storage_pools.go | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lxd/db/storage_pools.go b/lxd/db/storage_pools.go
index b38d7a62f7..806873a030 100644
--- a/lxd/db/storage_pools.go
+++ b/lxd/db/storage_pools.go
@@ -793,6 +793,11 @@ func (c *Cluster) getStoragePoolConfig(poolID 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 storage pool ID %d", key, poolID)
+ }
+
config[key] = value
}
From e1ce389926e569d0a686b8a0c6e2ecdf991f3cda Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 11:21:46 +0000
Subject: [PATCH 23/32] lxd/storage/pools: storagePoolsPost comments line width
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_pools.go | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/lxd/storage_pools.go b/lxd/storage_pools.go
index d1ffa5470c..0b02b0a46d 100644
--- a/lxd/storage_pools.go
+++ b/lxd/storage_pools.go
@@ -149,11 +149,9 @@ func storagePoolsPost(d *Daemon, r *http.Request) response.Response {
}
if count == 1 {
- // No targetNode was specified and we're either a
- // single-node cluster or not clustered at all, so
- // create the storage pool immediately, unless there's
- // a pending storage pool (in that case we follow the
- // regular two-stage process).
+ // No targetNode was specified and we're either a single-node cluster or not clustered at
+ // all, so create the storage pool immediately, unless there's a pending storage pool
+ // (in that case we follow the regular two-stage process).
_, pool, _, err := d.cluster.GetStoragePoolInAnyState(req.Name)
if err != nil {
if err != db.ErrNoSuchObject {
From e199ae603ea91bc5e7d5f7fd83e86caac44dbacd Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 14:14:30 +0000
Subject: [PATCH 24/32] lxd/db/storage/pools: Adds StoragePoolErrored function
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/storage_pools.go | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lxd/db/storage_pools.go b/lxd/db/storage_pools.go
index 806873a030..54e1763929 100644
--- a/lxd/db/storage_pools.go
+++ b/lxd/db/storage_pools.go
@@ -477,6 +477,11 @@ func (c *ClusterTx) StoragePoolCreated(name string) error {
return c.storagePoolState(name, storagePoolCreated)
}
+// StoragePoolErrored sets the state of the given pool to storagePoolErrored.
+func (c *ClusterTx) StoragePoolErrored(name string) error {
+ return c.storagePoolState(name, storagePoolErrored)
+}
+
func (c *ClusterTx) storagePoolState(name string, state StoragePoolState) error {
stmt := "UPDATE storage_pools SET state=? WHERE name=?"
result, err := c.tx.Exec(stmt, state, name)
From 237ac3c56bc636c3d4c8e7f3e85fd94a0faf976d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 14:14:45 +0000
Subject: [PATCH 25/32] lxd/db/storage/pools: Renames
GetNonPendingStoragePoolNames to GetCreatedStoragePoolNames
And updates to only return created pools.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/storage_pools.go | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/lxd/db/storage_pools.go b/lxd/db/storage_pools.go
index 54e1763929..4a3fd9cb91 100644
--- a/lxd/db/storage_pools.go
+++ b/lxd/db/storage_pools.go
@@ -605,10 +605,9 @@ func (c *Cluster) GetStoragePoolNames() ([]string, error) {
return c.storagePools("")
}
-// GetNonPendingStoragePoolNames returns the names of all storage pools that are not
-// pending.
-func (c *Cluster) GetNonPendingStoragePoolNames() ([]string, error) {
- return c.storagePools("NOT state=?", storagePoolPending)
+// GetCreatedStoragePoolNames returns the names of all storage pools that are created.
+func (c *Cluster) GetCreatedStoragePoolNames() ([]string, error) {
+ return c.storagePools("state=?", storagePoolCreated)
}
// Get all storage pools matching the given WHERE filter (if given).
From 67d3765f4cfe5fb655ac77a87bf04ff57f97f9ac Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 14:15:10 +0000
Subject: [PATCH 26/32] lxd/api/cluster: cluster.GetCreatedStoragePoolNames
usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/api_cluster.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/api_cluster.go b/lxd/api_cluster.go
index 9da22e2b96..8301720584 100644
--- a/lxd/api_cluster.go
+++ b/lxd/api_cluster.go
@@ -1520,7 +1520,7 @@ type internalClusterPostHandoverRequest struct {
}
func clusterCheckStoragePoolsMatch(cluster *db.Cluster, reqPools []api.StoragePool) error {
- poolNames, err := cluster.GetNonPendingStoragePoolNames()
+ poolNames, err := cluster.GetCreatedStoragePoolNames()
if err != nil && err != db.ErrNoSuchObject {
return err
}
From 2bd906102d5165cc26f3a46b391d6ea48b9d4ff5 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 14:15:29 +0000
Subject: [PATCH 27/32] lxd/storage/pools/utils: Renames id arg to poolID in
storagePoolCreateLocal
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_pools_utils.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/storage_pools_utils.go b/lxd/storage_pools_utils.go
index 1cd7af264e..593173e068 100644
--- a/lxd/storage_pools_utils.go
+++ b/lxd/storage_pools_utils.go
@@ -95,7 +95,7 @@ func storagePoolCreateGlobal(state *state.State, req api.StoragePoolsPost, clien
// This performs local pool setup and updates DB record if config was changed during pool setup.
// Returns resulting config.
-func storagePoolCreateLocal(state *state.State, id int64, req api.StoragePoolsPost, clientType request.ClientType) (map[string]string, error) {
+func storagePoolCreateLocal(state *state.State, poolID int64, req api.StoragePoolsPost, clientType request.ClientType) (map[string]string, error) {
// Setup revert.
revert := revert.New()
defer revert.Fail()
@@ -160,7 +160,7 @@ func storagePoolCreateLocal(state *state.State, id int64, req api.StoragePoolsPo
// Set storage pool node to storagePoolCreated.
err = state.Cluster.Transaction(func(tx *db.ClusterTx) error {
- return tx.StoragePoolNodeCreated(id)
+ return tx.StoragePoolNodeCreated(poolID)
})
if err != nil {
return nil, err
From f8d89e45589a26b68cdfbae7f8a3b1cf7ed5ec20 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 14:15:52 +0000
Subject: [PATCH 28/32] lxd/storage: s.Cluster.GetCreatedStoragePoolNames usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/storage.go b/lxd/storage.go
index 154d11b4a0..b264e3777a 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -67,7 +67,7 @@ func resetContainerDiskIdmap(container instance.Container, srcIdmap *idmap.Idmap
}
func setupStorageDriver(s *state.State, forceCheck bool) error {
- pools, err := s.Cluster.GetNonPendingStoragePoolNames()
+ pools, err := s.Cluster.GetCreatedStoragePoolNames()
if err != nil {
if err == db.ErrNoSuchObject {
logger.Debugf("No existing storage pools detected")
From 5285eccf5747ff44b85c79d2b3ffd7866204f05b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 14:18:10 +0000
Subject: [PATCH 29/32] lxd/storage/pools: Restructures storagePoolsPost to
align with networksPost
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_pools.go | 83 ++++++++++++++++++++------------------------
1 file changed, 38 insertions(+), 45 deletions(-)
diff --git a/lxd/storage_pools.go b/lxd/storage_pools.go
index 0b02b0a46d..f5842594b6 100644
--- a/lxd/storage_pools.go
+++ b/lxd/storage_pools.go
@@ -142,68 +142,61 @@ func storagePoolsPost(d *Daemon, r *http.Request) response.Response {
}
targetNode := queryParam(r, "target")
- if targetNode == "" {
- count, err := cluster.Count(d.State())
- if err != nil {
- return response.SmartError(err)
- }
-
- if count == 1 {
- // No targetNode was specified and we're either a single-node cluster or not clustered at
- // all, so create the storage pool immediately, unless there's a pending storage pool
- // (in that case we follow the regular two-stage process).
- _, pool, _, err := d.cluster.GetStoragePoolInAnyState(req.Name)
- if err != nil {
- if err != db.ErrNoSuchObject {
- return response.InternalError(err)
- }
- err = storagePoolCreateGlobal(d.State(), req, clientType)
- if err != nil {
- return response.InternalError(err)
- }
- return resp
+ if targetNode != "" {
+ // A targetNode was specified, let's just define the node's storage without actually creating it.
+ // The only legal key values for the storage config are the ones in StoragePoolNodeConfigKeys.
+ for key := range req.Config {
+ if !shared.StringInSlice(key, db.StoragePoolNodeConfigKeys) {
+ return response.SmartError(fmt.Errorf("Config key %q may not be used as node-specific key", key))
}
+ }
- // If the existing storage pool is pending then we continue onto storagePoolsPostCluster.
- // Otherwise error as there is already a storage pool by that name.
- if pool.Status != "Pending" {
- return response.BadRequest(fmt.Errorf("The storage pool already exists"))
- }
+ err = storagePoolValidate(req.Name, req.Driver, req.Config)
+ if err != nil {
+ return response.BadRequest(err)
}
- // No targetNode was specified and we're clustered, so finalize the
- // config in the db and actually create the pool on all nodes.
- err = storagePoolsPostCluster(d, req, clientType)
+ err = d.cluster.Transaction(func(tx *db.ClusterTx) error {
+ return tx.CreatePendingStoragePool(targetNode, req.Name, req.Driver, req.Config)
+ })
if err != nil {
- return response.InternalError(err)
+ if err == db.ErrAlreadyDefined {
+ return response.BadRequest(fmt.Errorf("The storage pool already defined on node %q", targetNode))
+ }
+
+ return response.SmartError(err)
}
return resp
}
- // A targetNode was specified, let's just define the node's storage
- // without actually creating it. The only legal key values for the
- // storage config are the ones in StoragePoolNodeConfigKeys.
- for key := range req.Config {
- if !shared.StringInSlice(key, db.StoragePoolNodeConfigKeys) {
- return response.SmartError(fmt.Errorf("Config key %q may not be used as node-specific key", key))
- }
+ // Load existing pool if exists, if not don't fail.
+ _, pool, _, err := d.cluster.GetStoragePoolInAnyState(req.Name)
+ if err != nil && err != db.ErrNoSuchObject {
+ return response.InternalError(err)
}
- err = storagePoolValidate(req.Name, req.Driver, req.Config)
+ // Check if we're clustered.
+ count, err := cluster.Count(d.State())
if err != nil {
- return response.BadRequest(err)
+ return response.SmartError(err)
}
- err = d.cluster.Transaction(func(tx *db.ClusterTx) error {
- return tx.CreatePendingStoragePool(targetNode, req.Name, req.Driver, req.Config)
- })
- if err != nil {
- if err == db.ErrAlreadyDefined {
- return response.BadRequest(fmt.Errorf("The storage pool already defined on node %q", targetNode))
+ // No targetNode was specified and we're clustered or there is an existing partially created single node
+ // pool, either way finalize the config in the db and actually create the pool on all node in the cluster.
+ if count > 1 || (pool != nil && pool.Status != api.StoragePoolStatusCreated) {
+ err = storagePoolsPostCluster(d, pool, req, clientType)
+ if err != nil {
+ return response.InternalError(err)
}
- return response.SmartError(err)
+ return resp
+ }
+
+ // Create new single node storage pool.
+ err = storagePoolCreateGlobal(d.State(), req, clientType)
+ if err != nil {
+ return response.InternalError(err)
}
return resp
From c15608bc2a271f6c8e282e8202f9c770e125ab39 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 14:19:14 +0000
Subject: [PATCH 30/32] lxd/storage/pools: Updates storagePoolsPostCluster to
reject global config on re-create attempts
Avoids inserting duplicate config.
Also uses errored stated to better indicate a storage pool that has failed its initial create attempt.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_pools.go | 38 ++++++++++++++++++++++++++++++++++----
1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/lxd/storage_pools.go b/lxd/storage_pools.go
index f5842594b6..795d0daa36 100644
--- a/lxd/storage_pools.go
+++ b/lxd/storage_pools.go
@@ -202,7 +202,9 @@ func storagePoolsPost(d *Daemon, r *http.Request) response.Response {
return resp
}
-func storagePoolsPostCluster(d *Daemon, req api.StoragePoolsPost, clientType request.ClientType) error {
+// storagePoolsPostCluster handles creating storage pools after the per-node config records have been created.
+// Accepts an optional existing pool record, which will exist when perform subsequent re-create attempts.
+func storagePoolsPostCluster(d *Daemon, pool *api.StoragePool, req api.StoragePoolsPost, clientType request.ClientType) error {
// Check that no node-specific config key has been defined.
for key := range req.Config {
if shared.StringInSlice(key, db.StoragePoolNodeConfigKeys) {
@@ -210,6 +212,18 @@ func storagePoolsPostCluster(d *Daemon, req api.StoragePoolsPost, clientType req
}
}
+ if pool != nil {
+ // Check pool isn't already created.
+ if pool.Status == api.StoragePoolStatusCreated {
+ return fmt.Errorf("The storage pool is already created")
+ }
+
+ // Check the requested pool type matches the type created when adding the local node config.
+ if req.Driver != pool.Driver {
+ return fmt.Errorf("Requested storage pool driver %q doesn't match driver in existing database record %q", req.Driver, pool.Driver)
+ }
+ }
+
// Check that the pool is properly defined, fetch the node-specific configs and insert the global config.
var configs map[string]map[string]string
var nodeName string
@@ -217,13 +231,23 @@ func storagePoolsPostCluster(d *Daemon, req api.StoragePoolsPost, clientType req
err := d.cluster.Transaction(func(tx *db.ClusterTx) error {
var err error
- // Check that the pool was defined at all.
+ // Check that the pool was defined at all. Must come before partially created checks.
poolID, err = tx.GetStoragePoolID(req.Name)
if err != nil {
return err
}
- // Fetch the node-specific configs.
+ // Check if any global config exists already, if so we should not create global config again.
+ if pool != nil && storagePoolPartiallyCreated(pool) {
+ if len(req.Config) > 0 {
+ return fmt.Errorf("Storage pool already partially created. Please do not specify any global config when re-running create")
+ }
+
+ logger.Debug("Skipping global storage pool create as global config already partially created", log.Ctx{"pool": req.Name})
+ return nil
+ }
+
+ // Fetch the node-specific configs and check the pool is defined for all nodes.
configs, err = tx.GetStoragePoolNodeConfigs(poolID)
if err != nil {
return err
@@ -236,7 +260,13 @@ func storagePoolsPostCluster(d *Daemon, req api.StoragePoolsPost, clientType req
}
// Insert the global config keys.
- return tx.CreateStoragePoolConfig(poolID, 0, req.Config)
+ err = tx.CreateStoragePoolConfig(poolID, 0, req.Config)
+ if err != nil {
+ return err
+ }
+
+ // Assume failure unless we succeed later on.
+ return tx.StoragePoolErrored(req.Name)
})
if err != nil {
if err == db.ErrNoSuchObject {
From fadce21330d2ff3287b1c6e7b18bd058e1a4e826 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 14:20:16 +0000
Subject: [PATCH 31/32] lxd/storage/pools: Adds storagePoolPartiallyCreated
function
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_pools.go | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/lxd/storage_pools.go b/lxd/storage_pools.go
index 795d0daa36..ab9797a875 100644
--- a/lxd/storage_pools.go
+++ b/lxd/storage_pools.go
@@ -202,6 +202,26 @@ func storagePoolsPost(d *Daemon, r *http.Request) response.Response {
return resp
}
+// storagePoolPartiallyCreated returns true of supplied storage pool has properties that indicate it has had
+// previous create attempts run on it but failed on one or more nodes.
+func storagePoolPartiallyCreated(pool *api.StoragePool) bool {
+ // If the pool status is StoragePoolStatusErrored, this means create has been run in the past and has
+ // failed on one or more nodes. Hence it is partially created.
+ if pool.Status == api.StoragePoolStatusErrored {
+ return true
+ }
+
+ // If the pool has global config keys, then it has previously been created by having its global config
+ // inserted, and this means it is partialled created.
+ for key := range pool.Config {
+ if !shared.StringInSlice(key, db.StoragePoolNodeConfigKeys) {
+ return true
+ }
+ }
+
+ return false
+}
+
// storagePoolsPostCluster handles creating storage pools after the per-node config records have been created.
// Accepts an optional existing pool record, which will exist when perform subsequent re-create attempts.
func storagePoolsPostCluster(d *Daemon, pool *api.StoragePool, req api.StoragePoolsPost, clientType request.ClientType) error {
From ab00f1df3dbcfee277e22981f379850c385c3142 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 14 Dec 2020 14:20:54 +0000
Subject: [PATCH 32/32] test/suites/clustering: Updates storage pool status
tests
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
test/suites/clustering.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/suites/clustering.sh b/test/suites/clustering.sh
index 5196354c9e..8b7b0162bd 100644
--- a/test/suites/clustering.sh
+++ b/test/suites/clustering.sh
@@ -406,7 +406,7 @@ test_clustering_storage() {
LXD_DIR="${LXD_ONE_DIR}" lxc storage create pool1 "${driver}" source=/tmp/not/exist --target node1
LXD_DIR="${LXD_TWO_DIR}" lxc storage create pool1 "${driver}" --target node2
! LXD_DIR="${LXD_TWO_DIR}" lxc storage create pool1 "${driver}" || false
- LXD_DIR="${LXD_ONE_DIR}" lxc storage show pool1 | grep status: | grep -q Pending
+ LXD_DIR="${LXD_ONE_DIR}" lxc storage show pool1 | grep status: | grep -q Errored
LXD_DIR="${LXD_ONE_DIR}" lxc storage unset pool1 source --target node1
LXD_DIR="${LXD_TWO_DIR}" lxc storage create pool1 "${driver}"
LXD_ONE_SOURCE="$(LXD_DIR="${LXD_ONE_DIR}" lxc storage get pool1 source --target=node1)"
More information about the lxc-devel
mailing list