The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/5531
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) === This is a first pass at mitigating #5434. We check that the node to be removed has no custom volumes and also require confirmation by default when using ```---force```.
From 77b114a675a3619062803a7d99bb8721a2f00660 Mon Sep 17 00:00:00 2001 From: Free Ekanayaka <[email protected]> Date: Wed, 27 Feb 2019 12:07:13 +0100 Subject: [PATCH 1/4] Use capital case in error messages returned by db.NodeInfo.IsEmpty() Signed-off-by: Free Ekanayaka <[email protected]> --- lxd/db/node.go | 10 +++++----- lxd/db/node_test.go | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lxd/db/node.go b/lxd/db/node.go index c28c5ad199..769043411e 100644 --- a/lxd/db/node.go +++ b/lxd/db/node.go @@ -328,15 +328,15 @@ func (c *ClusterTx) NodeIsEmpty(id int64) (string, error) { // Check if the node has any containers. containers, err := query.SelectStrings(c.tx, "SELECT name FROM containers WHERE node_id=?", id) if err != nil { - return "", errors.Wrapf(err, "failed to get containers for node %d", id) + return "", errors.Wrapf(err, "Failed to get containers for node %d", id) } if len(containers) > 0 { message := fmt.Sprintf( - "node still has the following containers: %s", strings.Join(containers, ", ")) + "Node still has the following containers: %s", strings.Join(containers, ", ")) return message, nil } - // Check if the node has any images available only in. + // Check if the node has any images available only in it. images := []struct { fingerprint string nodeID int64 @@ -357,7 +357,7 @@ SELECT fingerprint, node_id FROM images JOIN images_nodes ON images.id=images_no defer stmt.Close() err = query.SelectObjects(stmt, dest) if err != nil { - return "", errors.Wrapf(err, "failed to get image list for node %d", id) + return "", errors.Wrapf(err, "Failed to get image list for node %d", id) } index := map[string][]int64{} // Map fingerprints to IDs of nodes for _, image := range images { @@ -376,7 +376,7 @@ SELECT fingerprint, node_id FROM images JOIN images_nodes ON images.id=images_no if len(fingerprints) > 0 { message := fmt.Sprintf( - "node still has the following images: %s", strings.Join(fingerprints, ", ")) + "Node still has the following images: %s", strings.Join(fingerprints, ", ")) return message, nil } diff --git a/lxd/db/node_test.go b/lxd/db/node_test.go index e1c8a66378..720865f173 100644 --- a/lxd/db/node_test.go +++ b/lxd/db/node_test.go @@ -232,7 +232,7 @@ INSERT INTO containers (id, node_id, name, architecture, type, project_id) VALUE message, err = tx.NodeIsEmpty(id) require.NoError(t, err) - assert.Equal(t, "node still has the following containers: foo", message) + assert.Equal(t, "Node still has the following containers: foo", message) err = tx.NodeClear(id) require.NoError(t, err) @@ -262,7 +262,7 @@ INSERT INTO images_nodes(image_id, node_id) VALUES(1, ?)`, id) message, err := tx.NodeIsEmpty(id) require.NoError(t, err) - assert.Equal(t, "node still has the following images: abc", message) + assert.Equal(t, "Node still has the following images: abc", message) // Insert a new image entry for node 1 (the default node). _, err = tx.Tx().Exec(` From 16f2c51741e1452dc5a94f8eaa0cbe796636ed79 Mon Sep 17 00:00:00 2001 From: Free Ekanayaka <[email protected]> Date: Wed, 27 Feb 2019 12:07:54 +0100 Subject: [PATCH 2/4] db.NodeInfo.IsEmpty(): a node with custom volumes is not empty Signed-off-by: Free Ekanayaka <[email protected]> --- lxd/db/node.go | 13 +++++++++++++ lxd/db/node_test.go | 22 ++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/lxd/db/node.go b/lxd/db/node.go index 769043411e..cc318177dc 100644 --- a/lxd/db/node.go +++ b/lxd/db/node.go @@ -380,6 +380,19 @@ SELECT fingerprint, node_id FROM images JOIN images_nodes ON images.id=images_no return message, nil } + // Check if the node has any custom volumes. + volumes, err := query.SelectStrings( + c.tx, "SELECT name FROM storage_volumes WHERE node_id=? AND type=?", + id, StoragePoolVolumeTypeCustom) + if err != nil { + return "", errors.Wrapf(err, "Failed to get custom volumes for node %d", id) + } + if len(volumes) > 0 { + message := fmt.Sprintf( + "Node still has the following custom volumes: %s", strings.Join(volumes, ", ")) + return message, nil + } + return "", nil } diff --git a/lxd/db/node_test.go b/lxd/db/node_test.go index 720865f173..4094c2cba7 100644 --- a/lxd/db/node_test.go +++ b/lxd/db/node_test.go @@ -274,6 +274,28 @@ INSERT INTO images_nodes(image_id, node_id) VALUES(1, 1)`) assert.Equal(t, "", message) } +// A node is considered empty only if it has no custom volumes on it. +func TestNodeIsEmpty_CustomVolumes(t *testing.T) { + tx, cleanup := db.NewTestClusterTx(t) + defer cleanup() + + id, err := tx.NodeAdd("buzz", "1.2.3.4:666") + require.NoError(t, err) + + _, err = tx.Tx().Exec(` +INSERT INTO storage_pools (id, name, driver) VALUES (1, 'local', 'zfs')`) + require.NoError(t, err) + + _, err = tx.Tx().Exec(` +INSERT INTO storage_volumes(name, storage_pool_id, node_id, type, project_id) + VALUES ('data', 1, ?, ?, 1)`, id, db.StoragePoolVolumeTypeCustom) + require.NoError(t, err) + + message, err := tx.NodeIsEmpty(id) + require.NoError(t, err) + assert.Equal(t, "Node still has the following custom volumes: data", message) +} + // If there are 2 online nodes, return the address of the one with the least // number of containers. func TestNodeWithLeastContainers(t *testing.T) { From a241c418ca6142d7baf363ddea30bc4b23204ee1 Mon Sep 17 00:00:00 2001 From: Free Ekanayaka <[email protected]> Date: Wed, 27 Feb 2019 12:08:48 +0100 Subject: [PATCH 3/4] Add integration test checking that nodes with custom volumes can't be removed Signed-off-by: Free Ekanayaka <[email protected]> --- test/suites/clustering.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/suites/clustering.sh b/test/suites/clustering.sh index c33e416848..2afb455ebb 100644 --- a/test/suites/clustering.sh +++ b/test/suites/clustering.sh @@ -145,6 +145,11 @@ test_clustering_membership() { ! LXD_DIR="${LXD_FOUR_DIR}" lxc cluster remove node3 || false LXD_DIR="${LXD_TWO_DIR}" lxc image delete testimage + # Trying to delete a node which has a custom volume on it results in an error. + LXD_DIR="${LXD_FOUR_DIR}" lxc storage volume create data v1 + ! LXD_DIR="${LXD_FOUR_DIR}" lxc cluster remove node3 || false + LXD_DIR="${LXD_FOUR_DIR}" lxc storage volume delete data v1 + # The image got deleted from the LXD_DIR tree. # shellcheck disable=2086 [ "$(ls ${LXD_FOUR_DIR}/images)" = "" ] || false From 21982c80fa98b7fc15aa61462a9fd43cb2a088d9 Mon Sep 17 00:00:00 2001 From: Free Ekanayaka <[email protected]> Date: Wed, 27 Feb 2019 12:25:54 +0100 Subject: [PATCH 4/4] Prompt for confirmation when using --delete to remove a server Signed-off-by: Free Ekanayaka <[email protected]> --- lxc/cluster.go | 34 +++++++++++++++++++++++++++++++++- test/suites/clustering.sh | 4 ++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/lxc/cluster.go b/lxc/cluster.go index afc13e6274..b47b232ac2 100644 --- a/lxc/cluster.go +++ b/lxc/cluster.go @@ -1,6 +1,7 @@ package main import ( + "bufio" "fmt" "os" "sort" @@ -10,6 +11,7 @@ import ( "github.com/spf13/cobra" yaml "gopkg.in/yaml.v2" + "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" cli "github.com/lxc/lxd/shared/cmd" "github.com/lxc/lxd/shared/i18n" @@ -235,7 +237,8 @@ type cmdClusterRemove struct { global *cmdGlobal cluster *cmdCluster - flagForce bool + flagForce bool + flagNonInteractive bool } func (c *cmdClusterRemove) Command() *cobra.Command { @@ -248,10 +251,31 @@ func (c *cmdClusterRemove) Command() *cobra.Command { cmd.RunE = c.Run cmd.Flags().BoolVarP(&c.flagForce, "force", "f", false, i18n.G("Force removing a member, even if degraded")) + cmd.Flags().BoolVarP(&c.flagNonInteractive, "quiet", "q", true, i18n.G("Don't require user confirmation for using --force")) return cmd } +func (c *cmdClusterRemove) promptConfirmation(name string) error { + reader := bufio.NewReader(os.Stdin) + fmt.Printf(i18n.G(`After a server is removed from the cluster, it won't be able to manage +anymore the containers, images or custom volumes it might currently +have on it. + +The --force flag should only be used if the server has died and is not +expected to come back. + +Are you really sure you want to force removing %s? (yes/no): `), name) + input, _ := reader.ReadString('\n') + input = strings.TrimSuffix(input, "\n") + + if !shared.StringInSlice(strings.ToLower(input), []string{i18n.G("yes")}) { + return fmt.Errorf(i18n.G("User aborted delete operation")) + } + + return nil +} + func (c *cmdClusterRemove) Run(cmd *cobra.Command, args []string) error { // Sanity checks exit, err := c.global.CheckArgs(cmd, args, 1, 1) @@ -267,6 +291,14 @@ func (c *cmdClusterRemove) Run(cmd *cobra.Command, args []string) error { resource := resources[0] + // Prompt for confiromation if --force is used. + if !c.flagNonInteractive && c.flagForce { + err := c.promptConfirmation(resource.name) + if err != nil { + return err + } + } + // Delete the cluster member err = resource.server.DeleteClusterMember(resource.name, c.flagForce) if err != nil { diff --git a/test/suites/clustering.sh b/test/suites/clustering.sh index 2afb455ebb..bee2480296 100644 --- a/test/suites/clustering.sh +++ b/test/suites/clustering.sh @@ -125,7 +125,7 @@ test_clustering_membership() { ! LXD_DIR="${LXD_TWO_DIR}" lxc network delete "${bridge}" || false # Force the removal of the degraded node. - LXD_DIR="${LXD_TWO_DIR}" lxc cluster remove node3 --force + LXD_DIR="${LXD_TWO_DIR}" lxc cluster remove node3 -q --force # Sleep a bit to let a heartbeat occur and update the list of raft nodes # everywhere, showing that node 4 has been promoted to database node. @@ -507,7 +507,7 @@ test_clustering_storage() { LXD_DIR="${LXD_ONE_DIR}" lxc stop bar --force LXD_DIR="${LXD_ONE_DIR}" lxc config set cluster.offline_threshold 20 - LXD_DIR="${LXD_ONE_DIR}" lxc cluster remove node3 --force + LXD_DIR="${LXD_ONE_DIR}" lxc cluster remove node3 -q --force LXD_DIR="${LXD_ONE_DIR}" lxc delete bar
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
