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