Revert "glusterd: Allocate fresh port on brick (re)start"

This reverts commit 34899d7

Commit 34899d7 introduced a change, where restarting a volume or rebooting
a node result into fresh allocation of brick port. In production
environment generally administrator makes firewall configuration for a
range of ports for a volume. With commit 34899d7, on rebooting of node
or restarting a volume might result into volume start fail because
firewall might block fresh allocated port of a brick and also it will be
difficult in testing because of fresh allocation of port.

Change-Id: I7a90f69e8c267a013dc906b5228ca76e819d84ad
BUG: 1322805
Signed-off-by: Gaurav Kumar Garg <ggarg@redhat.com>
Reviewed-on: http://review.gluster.org/13989
Smoke: Gluster Build System <jenkins@build.gluster.com>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>

5 files changed, 70 insertions, 46 deletions

diff --git a/tests/bugs/glusterd/1313628-import-brick-ports-always.t b/tests/bugs/glusterd/1313628-import-brick-ports-always.t
new file mode 100755
index 00000000000..d04c4293466
--- /dev/null
+++ b/tests/bugs/glusterd/1313628-import-brick-ports-always.t
@@ -0,0 +1,47 @@
+#!/bin/bash
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../cluster.rc
+
+## Check that brick ports are always copied on import
+## --------------------------------------------------
+## This test checks that the brick ports are copied on import by checking that
+## they don't change when the following happens,
+##  - Stop a volume
+##  - Stop glusterd
+##  - Start the stopped volume
+##  - Start the stopped glusterd
+
+function get_brick_port() {
+  local VOL=$1
+  local BRICK=$2
+  $CLI2 volume status $VOL $BRICK --xml | sed -ne 's/.*<port>\([0-9]*\)<\/port>/\1/p'
+}
+
+
+cleanup
+
+TEST launch_cluster 2
+TEST $CLI1 peer probe $H2
+EXPECT_WITHIN $PROBE_TIMEOUT 1 peer_count
+
+# Create and start volume so that brick port assignment happens
+TEST $CLI1 volume create $V0 $H1:$B1/$V0 $H2:$B2/$V0
+TEST $CLI1 volume start $V0
+
+# Save port for 2nd brick
+BPORT_ORIG=$(get_brick_port $V0 $H2:$B2/$V0)
+
+# Stop volume, stop 2nd glusterd, start volume, start 2nd glusterd
+TEST $CLI1 volume stop $V0
+TEST kill_glusterd 2
+
+TEST $CLI1 volume start $V0
+TEST start_glusterd 2
+EXPECT_WITHIN $PROBE_TIMEOUT 1 peer_count
+
+# Get new port and compare with old one
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT $BPORT_ORIG get_brick_port $V0 $H2:$B2/$V0
+
+$CLI1 volume stop $V0
+
+cleanup
diff --git a/tests/features/ssl-ciphers.t b/tests/features/ssl-ciphers.t
index f5909f320ac..9ee7fc6c16f 100644
--- a/tests/features/ssl-ciphers.t
+++ b/tests/features/ssl-ciphers.t
@@ -137,7 +137,6 @@ EXPECT "`pwd`/`dirname $0`/dh1024.pem" volume_option $V0 ssl.dh-param
 TEST $CLI volume stop $V0
 TEST $CLI volume start $V0
 EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" online_brick_count
-BRICK_PORT=`brick_port $V0`
 EXPECT "Y" openssl_connect -cipher EDH -connect $H0:$BRICK_PORT
 
 # Test the cipher-list option
@@ -146,7 +145,6 @@ EXPECT AES256-SHA volume_option $V0 ssl.cipher-list
 TEST $CLI volume stop $V0
 TEST $CLI volume start $V0
 EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" online_brick_count
-BRICK_PORT=`brick_port $V0`
 EXPECT "Y" openssl_connect -cipher AES256-SHA -connect $H0:$BRICK_PORT
 EXPECT "N" openssl_connect -cipher AES128-SHA -connect $H0:$BRICK_PORT
 
@@ -156,7 +154,6 @@ EXPECT EECDH:EDH:!TLSv1 volume_option $V0 ssl.cipher-list
 TEST $CLI volume stop $V0
 TEST $CLI volume start $V0
 EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" online_brick_count
-BRICK_PORT=`brick_port $V0`
 EXPECT "N" openssl_connect -cipher AES256-SHA -connect $H0:$BRICK_PORT
 EXPECT "Y" openssl_connect -cipher EECDH -connect $H0:$BRICK_PORT
 
@@ -165,7 +162,6 @@ EXPECT invalid volume_option $V0 ssl.ec-curve
 TEST $CLI volume stop $V0
 TEST $CLI volume start $V0
 EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" online_brick_count
-BRICK_PORT=`brick_port $V0`
 EXPECT "N" openssl_connect -cipher EECDH -connect $H0:$BRICK_PORT
 
 TEST $CLI volume set $V0 ssl.ec-curve secp521r1
@@ -173,7 +169,6 @@ EXPECT secp521r1 volume_option $V0 ssl.ec-curve
 TEST $CLI volume stop $V0
 TEST $CLI volume start $V0
 EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" online_brick_count
-BRICK_PORT=`brick_port $V0`
 EXPECT "Y" openssl_connect -cipher EECDH -connect $H0:$BRICK_PORT
 
 # test revocation
diff --git a/xlators/mgmt/glusterd/src/glusterd-messages.h b/xlators/mgmt/glusterd/src/glusterd-messages.h
index 04d630001df..ffcfa7773e9 100644
--- a/xlators/mgmt/glusterd/src/glusterd-messages.h
+++ b/xlators/mgmt/glusterd/src/glusterd-messages.h
@@ -41,7 +41,7 @@
 
 #define GLUSTERD_COMP_BASE      GLFS_MSGID_GLUSTERD
 
-#define GLFS_NUM_MESSAGES       574
+#define GLFS_NUM_MESSAGES       573
 
 #define GLFS_MSGID_END          (GLUSTERD_COMP_BASE + GLFS_NUM_MESSAGES + 1)
 /* Messaged with message IDs */
@@ -4633,14 +4633,6 @@
  */
 #define GD_MSG_SNAP_RESUME_TIER_FAIL                (GLUSTERD_COMP_BASE + 573)
 
-/*!
- * @messageid
- * @diagnosis
- * @recommendedaction
- *
- */
-#define GD_MSG_PMAP_REMOVE_FAIL                (GLUSTERD_COMP_BASE + 574)
-
 /*------------*/
 #define glfs_msg_end_x GLFS_MSGID_END, "Invalid: End of messages"
 #endif /* !_GLUSTERD_MESSAGES_H_ */
diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c
index 592a2952c4b..605c4c2519f 100644
--- a/xlators/mgmt/glusterd/src/glusterd-store.c
+++ b/xlators/mgmt/glusterd/src/glusterd-store.c
@@ -2313,6 +2313,13 @@ glusterd_store_retrieve_bricks (glusterd_volinfo_t *volinfo)
                                         /* This is required to adhere to the
                                            IANA standards */
                                         brickinfo->port = 0;
+                                } else {
+                                        /* This is required to have proper ports
+                                           assigned to bricks after restart */
+                                        pmap = pmap_registry_get (THIS);
+                                        if (pmap->last_alloc <= brickinfo->port)
+                                                pmap->last_alloc =
+                                                        brickinfo->port + 1;
                                 }
                         } else if (!strncmp (key, GLUSTERD_STORE_KEY_BRICK_RDMA_PORT,
                                     strlen (GLUSTERD_STORE_KEY_BRICK_RDMA_PORT))) {
@@ -2322,7 +2329,16 @@ glusterd_store_retrieve_bricks (glusterd_volinfo_t *volinfo)
                                         /* This is required to adhere to the
                                            IANA standards */
                                         brickinfo->rdma_port = 0;
+                                } else {
+                                        /* This is required to have proper ports
+                                           assigned to bricks after restart */
+                                        pmap = pmap_registry_get (THIS);
+                                        if (pmap->last_alloc <=
+                                            brickinfo->rdma_port)
+                                                pmap->last_alloc =
+                                                        brickinfo->rdma_port +1;
                                 }
+
                         } else if (!strncmp (key, GLUSTERD_STORE_KEY_BRICK_DECOMMISSIONED,
                                              strlen (GLUSTERD_STORE_KEY_BRICK_DECOMMISSIONED))) {
                                 gf_string2int (value, &brickinfo->decommissioned);
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 66054794fd7..ba0539febd4 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -1730,22 +1730,9 @@ glusterd_volume_start_glusterfs (glusterd_volinfo_t  *volinfo,
         if (gf_is_service_running (pidfile, NULL))
                 goto connect;
 
-        /* Do a pmap registry remove on the older connected port */
-        if (brickinfo->port) {
-                ret = pmap_registry_remove (this, brickinfo->port,
-                                            brickinfo->path,
-                                            GF_PMAP_PORT_BRICKSERVER,
-                                            NULL);
-                if (ret) {
-                        gf_msg (this->name, GF_LOG_ERROR, 0,
-                                GD_MSG_PMAP_REMOVE_FAIL, "Failed to remove pmap"
-                                "registry for older signin port %d",
-                                brickinfo->port);
-                        goto out;
-                }
-        }
-
-        port = pmap_registry_alloc (THIS);
+        port = brickinfo->port;
+        if (!port)
+                port = pmap_registry_alloc (THIS);
 
         /* Build the exp_path, before starting the glusterfsd even in
            valgrind mode. Otherwise all the glusterfsd processes start
@@ -1808,22 +1795,9 @@ glusterd_volume_start_glusterfs (glusterd_volinfo_t  *volinfo,
         if (volinfo->transport_type != GF_TRANSPORT_BOTH_TCP_RDMA) {
                 runner_argprintf (&runner, "%d", port);
         } else {
-                /* Do a pmap registry remove on the older connected port */
-                if (brickinfo->rdma_port) {
-                        ret = pmap_registry_remove (this, brickinfo->rdma_port,
-                                                    brickinfo->path,
-                                                    GF_PMAP_PORT_BRICKSERVER,
-                                                    NULL);
-                        if (ret) {
-                                gf_msg (this->name, GF_LOG_ERROR, 0,
-                                        GD_MSG_PMAP_REMOVE_FAIL, "Failed to "
-                                        "remove pmap registry for older signin "
-                                        "port %d", brickinfo->rdma_port);
-                                goto out;
-                        }
-                }
-                rdma_port = pmap_registry_alloc (THIS);
-
+                rdma_port = brickinfo->rdma_port;
+                if (!rdma_port)
+                        rdma_port = pmap_registry_alloc (THIS);
                 runner_argprintf (&runner, "%d,%d", port, rdma_port);
                 runner_add_arg (&runner, "--xlator-option");
                 runner_argprintf (&runner, "%s-server.transport.rdma.listen-port=%d",