From 5dedef81b6ef91d462ce49ded4e148dfc17deee2 Mon Sep 17 00:00:00 2001 From: Atin Mukherjee Date: Wed, 19 Mar 2014 11:30:22 +0530 Subject: cli: remove-brick no longer defaults to commit-force Problem : When gluster volume remove-brick is executed with out any option, it defaults to force commit which results in data loss. Fix : remove-brick can not be executed with out explicit option, user needs to provide the option in the command line else the command will throw back an usage error. Earlier usage : volume remove-brick [replica ] ... [start|stop|status|commit|force] Current usage : volume remove-brick [replica ] ... Change-Id: I2a49131f782a6c0dcd03b4dc8ebe5907999b0b49 BUG: 1077682 Signed-off-by: Atin Mukherjee Reviewed-on: http://review.gluster.org/7292 Tested-by: Gluster Build System Reviewed-by: Shyamsundar Ranganathan Reviewed-by: Vijay Bellur --- cli/src/cli-cmd-parser.c | 10 ++++------ cli/src/cli-cmd-volume.c | 3 ++- tests/basic/volume.t | 2 +- tests/bugs/bug-1077682.t | 34 ++++++++++++++++++++++++++++++++++ tests/bugs/bug-867252.t | 2 +- tests/bugs/bug-878004.t | 4 ++-- tests/bugs/bug-961669.t | 2 +- 7 files changed, 45 insertions(+), 12 deletions(-) create mode 100644 tests/bugs/bug-1077682.t diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c index 1513e0c5e..9777e655f 100644 --- a/cli/src/cli-cmd-parser.c +++ b/cli/src/cli-cmd-parser.c @@ -1118,7 +1118,7 @@ cli_cmd_volume_remove_brick_parse (const char **words, int wordcount, GF_ASSERT (words); GF_ASSERT (options); - if (wordcount < 4) + if (wordcount < 5) goto out; dict = dict_new (); @@ -1136,7 +1136,7 @@ cli_cmd_volume_remove_brick_parse (const char **words, int wordcount, brick_index = 3; w = str_getunamb (words[3], type_opword); if (w && !strcmp ("replica", w)) { - if (wordcount < 5) { + if (wordcount < 6) { ret = -1; goto out; } @@ -1158,10 +1158,8 @@ cli_cmd_volume_remove_brick_parse (const char **words, int wordcount, w = str_getunamb (words[wordcount - 1], opwords); if (!w) { - /* Should be default 'force' */ - command = GF_OP_CMD_COMMIT_FORCE; - if (question) - *question = 1; + ret = -1; + goto out; } else { /* handled this option */ wordcount--; diff --git a/cli/src/cli-cmd-volume.c b/cli/src/cli-cmd-volume.c index e334ddc84..3b927d2bd 100644 --- a/cli/src/cli-cmd-volume.c +++ b/cli/src/cli-cmd-volume.c @@ -2299,7 +2299,8 @@ struct cli_cmd volume_cmds[] = { cli_cmd_volume_add_brick_cbk, "add brick to volume "}, - { "volume remove-brick [replica ] ... [start|stop|status|commit|force]", + { "volume remove-brick [replica ] ..." + " ", cli_cmd_volume_remove_brick_cbk, "remove brick from volume "}, diff --git a/tests/basic/volume.t b/tests/basic/volume.t index 2f9096055..23b740af1 100755 --- a/tests/basic/volume.t +++ b/tests/basic/volume.t @@ -22,7 +22,7 @@ EXPECT 'Started' volinfo_field $V0 'Status'; TEST $CLI volume add-brick $V0 $H0:$B0/${V0}{9,10,11,12}; EXPECT '12' brick_count $V0 -TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}{1,2,3,4}; +TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}{1,2,3,4} force; EXPECT '8' brick_count $V0 TEST $CLI volume stop $V0; diff --git a/tests/bugs/bug-1077682.t b/tests/bugs/bug-1077682.t new file mode 100644 index 000000000..2923c5f66 --- /dev/null +++ b/tests/bugs/bug-1077682.t @@ -0,0 +1,34 @@ +#!/bin/bash + +. $(dirname $0)/../include.rc +. $(dirname $0)/../volume.rc + +function get-task-status() +{ + $CLI $COMMAND | grep -o $PATTERN + if [ ${PIPESTATUS[0]} -ne 0 ]; + then + return 1 + fi + return 0 +} + +cleanup; + +TEST glusterd +TEST pidof glusterd + +TEST $CLI volume create $V0 $H0:$B0/${V0}{1,2,3,4} +TEST $CLI volume start $V0 +TEST ! $CLI volume remove-brick $V0 $H0:$B0/${V0}1 +TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}2 force +TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}3 start + +EXPECT_WITHIN 10 "completed" remove_brick_status_completed_field "$V0" \ +"$H0:$B0/${V0}3" + +TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}3 commit +TEST killall glusterd +TEST glusterd + +cleanup diff --git a/tests/bugs/bug-867252.t b/tests/bugs/bug-867252.t index 8309ed9b9..17edcd9c5 100644 --- a/tests/bugs/bug-867252.t +++ b/tests/bugs/bug-867252.t @@ -35,7 +35,7 @@ EXPECT '1' brick_count $V0 TEST $CLI volume add-brick $V0 $H0:$B0/${V0}2; EXPECT '2' brick_count $V0 -TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}2; +TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}2 force; EXPECT '1' brick_count $V0 cleanup; diff --git a/tests/bugs/bug-878004.t b/tests/bugs/bug-878004.t index 5bee4c62f..407fd6ecc 100644 --- a/tests/bugs/bug-878004.t +++ b/tests/bugs/bug-878004.t @@ -19,10 +19,10 @@ function brick_count() TEST $CLI volume start $V0 -TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}2; +TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}2 force; EXPECT '2' brick_count $V0 -TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}3; +TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}3 force; EXPECT '1' brick_count $V0 cleanup; diff --git a/tests/bugs/bug-961669.t b/tests/bugs/bug-961669.t index 751a63df2..77896481c 100644 --- a/tests/bugs/bug-961669.t +++ b/tests/bugs/bug-961669.t @@ -27,7 +27,7 @@ function remove_brick_start { } function remove_brick { - $CLI volume remove-brick $V0 replica 2 $H0:$B0/${V0}{1,4,7} 2>&1|grep -oE 'success|failed' + $CLI volume remove-brick $V0 replica 2 $H0:$B0/${V0}{1,4,7} force 2>&1|grep -oE 'success|failed' } #remove-brick start variant -- cgit