From a04592cce9aaa6ccb8a038bc3b4e31bc125d1d10 Mon Sep 17 00:00:00 2001 From: Sanju Rakonde Date: Tue, 16 Jun 2020 18:03:21 +0530 Subject: [PATCH 453/456] glusterd: add-brick command failure Problem: add-brick operation is failing when replica or disperse count is not mentioned in the add-brick command. Reason: with commit a113d93 we are checking brick order while doing add-brick operation for replica and disperse volumes. If replica count or disperse count is not mentioned in the command, the dict get is failing and resulting add-brick operation failure. > upstream patch: https://review.gluster.org/#/c/glusterfs/+/24581/ > fixes: #1306 > Change-Id: Ie957540e303bfb5f2d69015661a60d7e72557353 > Signed-off-by: Sanju Rakonde BUG: 1847081 Change-Id: Ie957540e303bfb5f2d69015661a60d7e72557353 Signed-off-by: Sanju Rakonde Reviewed-on: https://code.engineering.redhat.com/gerrit/203867 Tested-by: RHGS Build Bot Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- tests/bugs/glusterd/brick-order-check-add-brick.t | 40 ++++++++++++++++++++++ tests/cluster.rc | 11 ++++-- xlators/mgmt/glusterd/src/glusterd-brick-ops.c | 39 ++++++++++++++------- xlators/mgmt/glusterd/src/glusterd-utils.c | 30 ++--------------- xlators/mgmt/glusterd/src/glusterd-utils.h | 3 +- xlators/mgmt/glusterd/src/glusterd-volume-ops.c | 41 +++++++++++++++++++---- 6 files changed, 115 insertions(+), 49 deletions(-) create mode 100644 tests/bugs/glusterd/brick-order-check-add-brick.t diff --git a/tests/bugs/glusterd/brick-order-check-add-brick.t b/tests/bugs/glusterd/brick-order-check-add-brick.t new file mode 100644 index 0000000..29f0ed1 --- /dev/null +++ b/tests/bugs/glusterd/brick-order-check-add-brick.t @@ -0,0 +1,40 @@ +#!/bin/bash +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../volume.rc +. $(dirname $0)/../../cluster.rc +. $(dirname $0)/../../snapshot.rc + +cleanup; + +TEST verify_lvm_version; +#Create cluster with 3 nodes +TEST launch_cluster 3 -NO_DEBUG -NO_FORCE +TEST setup_lvm 3 + +TEST $CLI_1 peer probe $H2 +TEST $CLI_1 peer probe $H3 +EXPECT_WITHIN $PROBE_TIMEOUT 2 peer_count + +TEST $CLI_1 volume create $V0 replica 3 $H1:$L1/$V0 $H2:$L2/$V0 $H3:$L3/$V0 +EXPECT '1 x 3 = 3' volinfo_field $V0 'Number of Bricks' +EXPECT 'Created' volinfo_field $V0 'Status' + +TEST $CLI_1 volume start $V0 +EXPECT 'Started' volinfo_field $V0 'Status' + +#add-brick with or without mentioning the replica count should not fail +TEST $CLI_1 volume add-brick $V0 replica 3 $H1:$L1/${V0}_1 $H2:$L2/${V0}_1 $H3:$L3/${V0}_1 +EXPECT '2 x 3 = 6' volinfo_field $V0 'Number of Bricks' + +TEST $CLI_1 volume add-brick $V0 $H1:$L1/${V0}_2 $H2:$L2/${V0}_2 $H3:$L3/${V0}_2 +EXPECT '3 x 3 = 9' volinfo_field $V0 'Number of Bricks' + +#adding bricks from same host should fail the brick order check +TEST ! $CLI_1 volume add-brick $V0 $H1:$L1/${V0}_3 $H1:$L1/${V0}_4 $H1:$L1/${V0}_5 +EXPECT '3 x 3 = 9' volinfo_field $V0 'Number of Bricks' + +#adding bricks from same host with force should succeed +TEST $CLI_1 volume add-brick $V0 $H1:$L1/${V0}_3 $H1:$L1/${V0}_4 $H1:$L1/${V0}_5 force +EXPECT '4 x 3 = 12' volinfo_field $V0 'Number of Bricks' + +cleanup diff --git a/tests/cluster.rc b/tests/cluster.rc index 99be8e7..8b73153 100644 --- a/tests/cluster.rc +++ b/tests/cluster.rc @@ -11,7 +11,7 @@ function launch_cluster() { define_backends $count; define_hosts $count; define_glusterds $count $2; - define_clis $count; + define_clis $count $3; start_glusterds; } @@ -133,8 +133,13 @@ function define_clis() { lopt1="--log-file=$logdir/$logfile1" - eval "CLI_$i='$CLI --glusterd-sock=${!b}/glusterd/gd.sock $lopt'"; - eval "CLI$i='$CLI --glusterd-sock=${!b}/glusterd/gd.sock $lopt1'"; + if [ "$2" == "-NO_FORCE" ]; then + eval "CLI_$i='$CLI_NO_FORCE --glusterd-sock=${!b}/glusterd/gd.sock $lopt'"; + eval "CLI$i='$CLI_NO_FORCE --glusterd-sock=${!b}/glusterd/gd.sock $lopt1'"; + else + eval "CLI_$i='$CLI --glusterd-sock=${!b}/glusterd/gd.sock $lopt'"; + eval "CLI$i='$CLI --glusterd-sock=${!b}/glusterd/gd.sock $lopt1'"; + fi done } diff --git a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c index 121346c..5ae577a 100644 --- a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c @@ -1576,20 +1576,35 @@ glusterd_op_stage_add_brick(dict_t *dict, char **op_errstr, dict_t *rsp_dict) /* Check brick order if the volume type is replicate or disperse. If * force at the end of command not given then check brick order. + * doing this check at the originator node is sufficient. */ - if (!is_force) { - if ((volinfo->type == GF_CLUSTER_TYPE_REPLICATE) || - (volinfo->type == GF_CLUSTER_TYPE_DISPERSE)) { - ret = glusterd_check_brick_order(dict, msg, volinfo->type); - if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_BAD_BRKORDER, - "Not adding brick because of " - "bad brick order. %s", - msg); - *op_errstr = gf_strdup(msg); - goto out; - } + if (is_origin_glusterd(dict) && !is_force) { + ret = 0; + if (volinfo->type == GF_CLUSTER_TYPE_REPLICATE) { + gf_msg_debug(this->name, 0, + "Replicate cluster type " + "found. Checking brick order."); + if (replica_count) + ret = glusterd_check_brick_order(dict, msg, volinfo->type, + replica_count); + else + ret = glusterd_check_brick_order(dict, msg, volinfo->type, + volinfo->replica_count); + } else if (volinfo->type == GF_CLUSTER_TYPE_DISPERSE) { + gf_msg_debug(this->name, 0, + "Disperse cluster type" + " found. Checking brick order."); + ret = glusterd_check_brick_order(dict, msg, volinfo->type, + volinfo->disperse_count); + } + if (ret) { + gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_BAD_BRKORDER, + "Not adding brick because of " + "bad brick order. %s", + msg); + *op_errstr = gf_strdup(msg); + goto out; } } diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index 6f904ae..545e688 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -14802,7 +14802,8 @@ glusterd_compare_addrinfo(struct addrinfo *first, struct addrinfo *next) * volume are present on the same server */ int32_t -glusterd_check_brick_order(dict_t *dict, char *err_str, int32_t type) +glusterd_check_brick_order(dict_t *dict, char *err_str, int32_t type, + int32_t sub_count) { int ret = -1; int i = 0; @@ -14819,7 +14820,6 @@ glusterd_check_brick_order(dict_t *dict, char *err_str, int32_t type) char *tmpptr = NULL; char *volname = NULL; int32_t brick_count = 0; - int32_t sub_count = 0; struct addrinfo *ai_info = NULL; char brick_addr[128] = { 0, @@ -14870,31 +14870,6 @@ glusterd_check_brick_order(dict_t *dict, char *err_str, int32_t type) goto out; } - if (type != GF_CLUSTER_TYPE_DISPERSE) { - ret = dict_get_int32n(dict, "replica-count", SLEN("replica-count"), - &sub_count); - if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_DICT_GET_FAILED, - "Bricks check : Could" - " not retrieve replica count"); - goto out; - } - gf_msg_debug(this->name, 0, - "Replicate cluster type " - "found. Checking brick order."); - } else { - ret = dict_get_int32n(dict, "disperse-count", SLEN("disperse-count"), - &sub_count); - if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_DICT_GET_FAILED, - "Bricks check : Could" - " not retrieve disperse count"); - goto out; - } - gf_msg(this->name, GF_LOG_INFO, 0, GD_MSG_DISPERSE_CLUSTER_FOUND, - "Disperse cluster type" - " found. Checking brick order."); - } brick_list_dup = brick_list_ptr = gf_strdup(brick_list); /* Resolve hostnames and get addrinfo */ while (i < brick_count) { @@ -14989,5 +14964,6 @@ out: ai_list_tmp2 = ai_list_tmp1; } free(ai_list_tmp2); + gf_msg_debug("glusterd", 0, "Returning %d", ret); return ret; } diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h index e2e2454..5f5de82 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.h +++ b/xlators/mgmt/glusterd/src/glusterd-utils.h @@ -883,6 +883,7 @@ char * search_brick_path_from_proc(pid_t brick_pid, char *brickpath); int32_t -glusterd_check_brick_order(dict_t *dict, char *err_str, int32_t type); +glusterd_check_brick_order(dict_t *dict, char *err_str, int32_t type, + int32_t sub_count); #endif diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c index 8da2ff3..134b04c 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c @@ -1024,6 +1024,8 @@ glusterd_op_stage_create_volume(dict_t *dict, char **op_errstr, int32_t local_brick_count = 0; int32_t i = 0; int32_t type = 0; + int32_t replica_count = 0; + int32_t disperse_count = 0; char *brick = NULL; char *tmpptr = NULL; xlator_t *this = NULL; @@ -1119,15 +1121,42 @@ glusterd_op_stage_create_volume(dict_t *dict, char **op_errstr, } if (!is_force) { - if ((type == GF_CLUSTER_TYPE_REPLICATE) || - (type == GF_CLUSTER_TYPE_DISPERSE)) { - ret = glusterd_check_brick_order(dict, msg, type); + if (type == GF_CLUSTER_TYPE_REPLICATE) { + ret = dict_get_int32n(dict, "replica-count", + SLEN("replica-count"), &replica_count); if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_BAD_BRKORDER, - "Not creating volume because of " - "bad brick order"); + gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_DICT_GET_FAILED, + "Bricks check : Could" + " not retrieve replica count"); + goto out; + } + gf_msg_debug(this->name, 0, + "Replicate cluster type " + "found. Checking brick order."); + ret = glusterd_check_brick_order(dict, msg, type, + replica_count); + } else if (type == GF_CLUSTER_TYPE_DISPERSE) { + ret = dict_get_int32n(dict, "disperse-count", + SLEN("disperse-count"), &disperse_count); + if (ret) { + gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_DICT_GET_FAILED, + "Bricks check : Could" + " not retrieve disperse count"); goto out; } + gf_msg_debug(this->name, 0, + "Disperse cluster type" + " found. Checking brick order."); + ret = glusterd_check_brick_order(dict, msg, type, + disperse_count); + } + if (ret) { + gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_BAD_BRKORDER, + "Not creating the volume because of " + "bad brick order. %s", + msg); + *op_errstr = gf_strdup(msg); + goto out; } } } -- 1.8.3.1