[lxc-devel] [lxd/master] lxd/storage: Properly handle driver config changes

[stgraber on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 370 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20191203/970ef8d0/attachment-0001.bin>
-------------- next part --------------
From 4223e11bf9070b46726051ec8b1b6147539d379c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 3 Dec 2019 15:58:18 -0500
Subject: [PATCH] lxd/storage: Properly handle driver config changes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #6537

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/storage/backend_lxd.go |  2 +-
 lxd/storage_pools.go       |  5 +++--
 lxd/storage_pools_utils.go | 30 ++++++++++++++++--------------
 3 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 0a548b9142..18840f0008 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -71,7 +71,7 @@ func (b *lxdBackend) create(dbPool *api.StoragePoolsPost, localOnly bool, op *op
 		return err
 	}
 
-	// If dealing with a remote storage pool, we're done now
+	// If dealing with a remote storage pool, we're done now.
 	if b.driver.Info().Remote && localOnly {
 		return nil
 	}
diff --git a/lxd/storage_pools.go b/lxd/storage_pools.go
index ffc03e92c2..eb3ba41abe 100644
--- a/lxd/storage_pools.go
+++ b/lxd/storage_pools.go
@@ -124,7 +124,7 @@ func storagePoolsPost(d *Daemon, r *http.Request) response.Response {
 			return response.NotFound(err)
 		}
 
-		err = storagePoolCreateLocal(d.State(), poolID, req, true)
+		_, err = storagePoolCreateLocal(d.State(), poolID, req, true)
 		if err != nil {
 			return response.SmartError(err)
 		}
@@ -239,10 +239,11 @@ func storagePoolsPostCluster(d *Daemon, req api.StoragePoolsPost) error {
 		return err
 	}
 
-	err = storagePoolCreateLocal(d.State(), poolID, req, false)
+	updatedConfig, err := storagePoolCreateLocal(d.State(), poolID, req, false)
 	if err != nil {
 		return err
 	}
+	req.Config = updatedConfig
 
 	// Notify all other nodes to create the pool.
 	notifier, err := cluster.NewNotifier(d.State(), d.endpoints.NetworkCert(), cluster.NotifyAll)
diff --git a/lxd/storage_pools_utils.go b/lxd/storage_pools_utils.go
index 61b57705bc..7d74398c85 100644
--- a/lxd/storage_pools_utils.go
+++ b/lxd/storage_pools_utils.go
@@ -228,7 +228,7 @@ func storagePoolCreateGlobal(state *state.State, req api.StoragePoolsPost) error
 		dbStoragePoolDeleteAndUpdateCache(state.Cluster, req.Name)
 	}()
 
-	err = storagePoolCreateLocal(state, id, req, false)
+	_, err = storagePoolCreateLocal(state, id, req, false)
 	if err != nil {
 		return err
 	}
@@ -238,7 +238,7 @@ func storagePoolCreateGlobal(state *state.State, req api.StoragePoolsPost) error
 }
 
 // This performs all non-db related work needed to create the pool.
-func storagePoolCreateLocal(state *state.State, id int64, req api.StoragePoolsPost, isNotification bool) error {
+func storagePoolCreateLocal(state *state.State, id int64, req api.StoragePoolsPost, isNotification bool) (map[string]string, error) {
 	tryUndo := true
 
 	// Make a copy of the req for later diff.
@@ -250,13 +250,13 @@ func storagePoolCreateLocal(state *state.State, id int64, req api.StoragePoolsPo
 	pool, err := storagePools.CreatePool(state, id, &updatedReq, isNotification, nil)
 	if err != storageDrivers.ErrUnknownDriver {
 		if err != nil {
-			return err
+			return nil, err
 		}
 
 		// Mount the pool
 		_, err = pool.Mount()
 		if err != nil {
-			return err
+			return nil, err
 		}
 
 		// Record the updated config.
@@ -274,7 +274,7 @@ func storagePoolCreateLocal(state *state.State, id int64, req api.StoragePoolsPo
 		// Load the old storage struct
 		s, err := storagePoolInit(state, req.Name)
 		if err != nil {
-			return err
+			return nil, err
 		}
 
 		// If this is a clustering notification for a ceph storage, we don't
@@ -283,13 +283,13 @@ func storagePoolCreateLocal(state *state.State, id int64, req api.StoragePoolsPo
 		// create the storage pool directory.
 		if s, ok := s.(*storageCeph); ok && isNotification {
 			volumeMntPoint := storagePools.GetStoragePoolVolumeMountPoint(s.pool.Name, s.volume.Name)
-			return os.MkdirAll(volumeMntPoint, 0711)
+			return nil, os.MkdirAll(volumeMntPoint, 0711)
 		}
 
 		// Create the pool
 		err = s.StoragePoolCreate()
 		if err != nil {
-			return err
+			return nil, err
 		}
 
 		updatedConfig = s.GetStoragePoolWritable().Config
@@ -308,19 +308,21 @@ func storagePoolCreateLocal(state *state.State, id int64, req api.StoragePoolsPo
 	// to the path the user gave us and update the config in the storage
 	// callback. So diff the config here to see if something like this has
 	// happened.
-	configDiff, _ := storagePools.ConfigDiff(req.Config, updatedConfig)
-	if len(configDiff) > 0 {
-		// Create the database entry for the storage pool.
-		err = state.Cluster.StoragePoolUpdate(req.Name, req.Description, updatedConfig)
-		if err != nil {
-			return fmt.Errorf("Error inserting %s into database: %s", req.Name, err)
+	if !isNotification {
+		configDiff, _ := storagePools.ConfigDiff(req.Config, updatedConfig)
+		if len(configDiff) > 0 {
+			// Create the database entry for the storage pool.
+			err = state.Cluster.StoragePoolUpdate(req.Name, req.Description, updatedConfig)
+			if err != nil {
+				return nil, fmt.Errorf("Error inserting %s into database: %s", req.Name, err)
+			}
 		}
 	}
 
 	// Success, update the closure to mark that the changes should be kept.
 	tryUndo = false
 
-	return nil
+	return updatedConfig, nil
 }
 
 // Helper around the low-level DB API, which also updates the driver names cache.