From a04592cce9aaa6ccb8a038bc3b4e31bc125d1d10 Mon Sep 17 00:00:00 2001

From: Sanju Rakonde <srakonde@redhat.com>

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 <srakonde@redhat.com>

BUG: 1847081

Change-Id: Ie957540e303bfb5f2d69015661a60d7e72557353

Signed-off-by: Sanju Rakonde <srakonde@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/203867

Tested-by: RHGS Build Bot <nigelb@redhat.com>

Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>

---

 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