The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/4653

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) ===
The filter was broken since it should have been "storage_pools_config.node_id=?"
instead.

However, the filter is not necessary since the only call site that this method
has (in cluster/membership.go) already filters out any config key which is not
node-specific.

Since this method is going to have a new call site for the new join API, that
requires all config keys (not only the node-specific ones), I dropped the filter
altogher.

This fixes #4629.

Signed-off-by: Free Ekanayaka <[email protected]>
From 009493ce0d4027b1c2673426629317a4e2e53f91 Mon Sep 17 00:00:00 2001
From: Free Ekanayaka <[email protected]>
Date: Thu, 14 Jun 2018 12:28:18 +0000
Subject: [PATCH] Drop the storage_pool_id filter from the WHERE clause of
 StoragePoolsConfig

The filter was broken since it should have been "storage_pools_config.node_id=?"
instead.

However, the filter is not necessary since the only call site that this method
has (in cluster/membership.go) already filters out any config key which is not
node-specific.

Since this method is going to have a new call site for the new join API, that
requires all config keys (not only the node-specific ones), I dropped the filter
altogher.

Signed-off-by: Free Ekanayaka <[email protected]>
---
 lxd/db/storage_pools.go      |  8 +-------
 lxd/db/storage_pools_test.go | 38 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/lxd/db/storage_pools.go b/lxd/db/storage_pools.go
index 00f5160e8..c4795e2fb 100644
--- a/lxd/db/storage_pools.go
+++ b/lxd/db/storage_pools.go
@@ -14,10 +14,6 @@ import (
 
 // StoragePoolConfigs returns a map associating each storage pool name to its
 // config values.
-//
-// The config values are the ones defined for the node this function is run
-// on. They are used by cluster.Join when a new node joins the cluster and its
-// configuration needs to be migrated to the cluster database.
 func (c *ClusterTx) StoragePoolConfigs() (map[string]map[string]string, error) 
{
        names, err := query.SelectStrings(c.tx, "SELECT name FROM 
storage_pools")
        if err != nil {
@@ -28,9 +24,7 @@ func (c *ClusterTx) StoragePoolConfigs() 
(map[string]map[string]string, error) {
                table := `
 storage_pools_config JOIN storage_pools ON 
storage_pools.id=storage_pools_config.storage_pool_id
 `
-               config, err := query.SelectConfig(
-                       c.tx, table, "storage_pools.name=? AND 
storage_pools_config.storage_pool_id=?",
-                       name, c.nodeID)
+               config, err := query.SelectConfig(c.tx, table, 
"storage_pools.name=?", name)
                if err != nil {
                        return nil, err
                }
diff --git a/lxd/db/storage_pools_test.go b/lxd/db/storage_pools_test.go
index 11e503f08..d455a64b3 100644
--- a/lxd/db/storage_pools_test.go
+++ b/lxd/db/storage_pools_test.go
@@ -8,6 +8,42 @@ import (
        "github.com/stretchr/testify/require"
 )
 
+// The StoragePoolsConfigs method returns only node-specific config values.
+func TestStoragePoolConfigs(t *testing.T) {
+       cluster, cleanup := db.NewTestCluster(t)
+       defer cleanup()
+
+       // Create a storage pool named "local" (like the default LXD clustering
+       // one), then delete it and create another one.
+       _, err := cluster.StoragePoolCreate("local", "", "dir", 
map[string]string{
+               "source": "/foo/bar",
+       })
+       require.NoError(t, err)
+
+       _, err = cluster.StoragePoolDelete("local")
+       require.NoError(t, err)
+
+       _, err = cluster.StoragePoolCreate("BTRFS", "", "dir", 
map[string]string{
+               "source": "/egg/baz",
+       })
+       require.NoError(t, err)
+
+       // Check that the config map returned by StoragePoolsConfigs actually
+       // contains the value of the "BTRFS" storage pool.
+       var config map[string]map[string]string
+
+       err = cluster.Transaction(func(tx *db.ClusterTx) error {
+               var err error
+               config, err = tx.StoragePoolConfigs()
+               return err
+       })
+       require.NoError(t, err)
+
+       assert.Equal(t, config, map[string]map[string]string{
+               "BTRFS": map[string]string{"source": "/egg/baz"},
+       })
+}
+
 func TestStoragePoolsCreatePending(t *testing.T) {
        tx, cleanup := db.NewTestClusterTx(t)
        defer cleanup()
@@ -116,7 +152,7 @@ func TestStoragePoolVolume_Ceph(t *testing.T) {
        cluster, cleanup := db.NewTestCluster(t)
        defer cleanup()
 
-       // Create a second no (beyond the default one).
+       // Create a second node (beyond the default one).
        err := cluster.Transaction(func(tx *db.ClusterTx) error {
                _, err := tx.NodeAdd("n1", "1.2.3.4:666")
                return err
_______________________________________________
lxc-devel mailing list
[email protected]
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to