The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8270
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
Implements suggestion from https://github.com/lxc/lxd/pull/8161#discussion_r524643693
From 354df8872444809406ccfd14bff49401b07e52de Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 17 Dec 2020 15:08:34 +0000
Subject: [PATCH 1/6] lxd/db/cluster/update: Modifies updateFromV43 and
updateFromV42 to use IFNULL(node_id, -1) to avoid nodes with 0 ID
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/db/cluster/update.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/db/cluster/update.go b/lxd/db/cluster/update.go
index 614f4845a9..9dbf256818 100644
--- a/lxd/db/cluster/update.go
+++ b/lxd/db/cluster/update.go
@@ -88,7 +88,7 @@ var updates = map[int]schema.Update{
// This can occur when multiple create requests have been issued when setting
up a clustered storage pool.
func updateFromV42(tx *sql.Tx) error {
// Find all duplicated config rows and return comma delimited list of
affected row IDs for each dupe set.
- stmt, err := tx.Prepare(`SELECT storage_pool_id, COALESCE(node_id,0),
key, value, COUNT(*) AS rowCount, GROUP_CONCAT(id, ",") AS dupeRowIDs
+ stmt, err := tx.Prepare(`SELECT storage_pool_id, IFNULL(node_id, -1),
key, value, COUNT(*) AS rowCount, GROUP_CONCAT(id, ",") AS dupeRowIDs
FROM storage_pools_config
GROUP BY storage_pool_id, node_id, key, value
HAVING rowCount > 1
@@ -157,7 +157,7 @@ func updateFromV42(tx *sql.Tx) error {
// This can occur when multiple create requests have been issued when setting
up a clustered network.
func updateFromV41(tx *sql.Tx) error {
// 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
+ stmt, err := tx.Prepare(`SELECT network_id, IFNULL(node_id, -1), key,
value, COUNT(*) AS rowCount, GROUP_CONCAT(id, ",") AS dupeRowIDs
FROM networks_config
GROUP BY network_id, node_id, key, value
HAVING rowCount > 1
From 245c92e80fa301ee429ec700aa800d97e357ff30 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 17 Dec 2020 14:55:57 +0000
Subject: [PATCH 2/6] lxd/db/cluster: Adds updateFromV43 patch that adds unique
index to storage_pools_config and networks_config table
Prevents duplicate config rows for the same node and key being inserted.
Fixes #8260
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/db/cluster/schema.go | 4 +++-
lxd/db/cluster/update.go | 13 +++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/lxd/db/cluster/schema.go b/lxd/db/cluster/schema.go
index 897ea06d22..c0165dd17c 100644
--- a/lxd/db/cluster/schema.go
+++ b/lxd/db/cluster/schema.go
@@ -304,6 +304,7 @@ CREATE TABLE "networks_nodes" (
FOREIGN KEY (network_id) REFERENCES "networks" (id) ON DELETE CASCADE,
FOREIGN KEY (node_id) REFERENCES nodes (id) ON DELETE CASCADE
);
+CREATE UNIQUE INDEX networks_unique_network_id_node_id_key ON networks_config
(network_id, IFNULL(node_id, -1), key);
CREATE TABLE nodes (
id INTEGER PRIMARY KEY,
name TEXT NOT NULL,
@@ -495,6 +496,7 @@ CREATE TABLE storage_pools_nodes (
FOREIGN KEY (storage_pool_id) REFERENCES storage_pools (id) ON DELETE
CASCADE,
FOREIGN KEY (node_id) REFERENCES nodes (id) ON DELETE CASCADE
);
+CREATE UNIQUE INDEX storage_pools_unique_storage_pool_id_node_id_key ON
storage_pools_config (storage_pool_id, IFNULL(node_id, -1), key);
CREATE TABLE "storage_volumes" (
id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
name TEXT NOT NULL,
@@ -591,5 +593,5 @@ CREATE TABLE storage_volumes_snapshots_config (
UNIQUE (storage_volume_snapshot_id, key)
);
-INSERT INTO schema (version, updated_at) VALUES (43, strftime("%s"))
+INSERT INTO schema (version, updated_at) VALUES (44, strftime("%s"))
`
diff --git a/lxd/db/cluster/update.go b/lxd/db/cluster/update.go
index 9dbf256818..a64fefc1a7 100644
--- a/lxd/db/cluster/update.go
+++ b/lxd/db/cluster/update.go
@@ -82,6 +82,19 @@ var updates = map[int]schema.Update{
41: updateFromV40,
42: updateFromV41,
43: updateFromV42,
+ 44: updateFromV43,
+}
+
+// updateFromV43 adds a unique index to the storage_pools_config and
networks_config tables.
+func updateFromV43(tx *sql.Tx) error {
+ _, err := tx.Exec(`CREATE UNIQUE INDEX
storage_pools_unique_storage_pool_id_node_id_key ON storage_pools_config
(storage_pool_id, IFNULL(node_id, -1), key);
+ CREATE UNIQUE INDEX networks_unique_network_id_node_id_key ON
networks_config (network_id, IFNULL(node_id, -1), key);
+ `)
+ if err != nil {
+ return errors.Wrapf(err, "Failed adding unique index to
storage_pools_config and networks_config tables")
+ }
+
+ return nil
}
// updateFromV42 removes any duplicated storage pool config rows that have the
same value.
From 528c3c6cd9383e4f74e28cbae08ad766890b23cf Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 17 Dec 2020 17:19:30 +0000
Subject: [PATCH 3/6] shared/util: Adds StringHasPrefix function
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
shared/util.go | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/shared/util.go b/shared/util.go
index 66fba3f287..b7561ceb7c 100644
--- a/shared/util.go
+++ b/shared/util.go
@@ -621,6 +621,16 @@ func StringInSlice(key string, list []string) bool {
return false
}
+// StringHasPrefix returns true if value has one of the supplied prefixes.
+func StringHasPrefix(value string, prefixes ...string) bool {
+ for _, prefix := range prefixes {
+ if strings.HasPrefix(value, prefix) {
+ return true
+ }
+ }
+ return false
+}
+
func IntInSlice(key int, list []int) bool {
for _, entry := range list {
if entry == key {
From ba2d8f96a95f3c708d1d7034d6648afe75437927 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 17 Dec 2020 17:19:46 +0000
Subject: [PATCH 4/6] lxd/device/disk: Adds sourceIsLocalPath function
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/device/disk.go | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index dc37ddad17..aace0c7a10 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -58,6 +58,24 @@ func (d *disk) isRequired(devConfig deviceConfig.Device)
bool {
return false
}
+// sourceIsLocalPath returns true if the source supplied should be considered
a local path on the host.
+// It returns false if the disk source is empty, a VM cloud-init config drive,
or a remote ceph/cephfs path.
+func (d *disk) sourceIsLocalPath(source string) bool {
+ if source == "" {
+ return false
+ }
+
+ if source == diskSourceCloudInit {
+ return false
+ }
+
+ if shared.StringHasPrefix(d.config["source"], "ceph:", "cephfs:") {
+ return false
+ }
+
+ return true
+}
+
// validateConfig checks the supplied config for correctness.
func (d *disk) validateConfig(instConf instance.ConfigReader) error {
if !instanceSupported(instConf.Type(), instancetype.Container,
instancetype.VM) {
From 96737423117df32dbc4f8d3ca48d6cf78d0dc6d9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 17 Dec 2020 17:20:00 +0000
Subject: [PATCH 5/6] lxd/device/disk: Use shared.StringHasPrefix when
validating ceph/cephfs prefixes
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/device/disk.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index aace0c7a10..dcfe7c96fb 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -148,7 +148,7 @@ func (d *disk) validateConfig(instConf
instance.ConfigReader) error {
}
// Check ceph options are only used when ceph or cephfs type source is
specified.
- if !(strings.HasPrefix(d.config["source"], "ceph:") ||
strings.HasPrefix(d.config["source"], "cephfs:")) &&
(d.config["ceph.cluster_name"] != "" || d.config["ceph.user_name"] != "") {
+ if !shared.StringHasPrefix(d.config["source"], "ceph:", "cephfs:") &&
(d.config["ceph.cluster_name"] != "" || d.config["ceph.user_name"] != "") {
return fmt.Errorf("Invalid options
ceph.cluster_name/ceph.user_name for source %q", d.config["source"])
}
From 36533867c873ceca1a2227a511455b7f7cbde7ff Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 17 Dec 2020 17:20:16 +0000
Subject: [PATCH 6/6] lxd/device/disk: Use d.sourceIsLocalPath when validating
source host path exists
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/device/disk.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index dcfe7c96fb..440b21f103 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -173,8 +173,8 @@ func (d *disk) validateConfig(instConf
instance.ConfigReader) error {
// source path exists when the disk device is required, is not an
external ceph/cephfs source and is not a
// VM cloud-init drive. We only check this when an instance is loaded
to avoid validating snapshot configs
// that may contain older config that no longer exists which can
prevent migrations.
- if d.inst != nil && d.config["pool"] == "" && d.config["source"] != ""
&& d.config["source"] != diskSourceCloudInit && d.isRequired(d.config) &&
!shared.PathExists(shared.HostPath(d.config["source"])) &&
!strings.HasPrefix(d.config["source"], "ceph:") &&
!strings.HasPrefix(d.config["source"], "cephfs:") {
- return fmt.Errorf("Missing source %q for disk %q",
d.config["source"], d.name)
+ if d.inst != nil && d.config["pool"] == "" && d.isRequired(d.config) &&
d.sourceIsLocalPath(d.config["source"]) &&
!shared.PathExists(shared.HostPath(d.config["source"])) {
+ return fmt.Errorf("Missing source path %q for disk %q",
d.config["source"], d.name)
}
if d.config["pool"] != "" {
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
