cli: Mark port as N/A in volume status when process is not online

Change-Id: Ie11c7331e3bc58c0f934f424dde4341cdffb9e2c
BUG: 861542
Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>
Reviewed-on: http://review.gluster.org/4048
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

3 files changed, 88 insertions, 14 deletions

diff --git a/cli/src/cli-cmd-volume.c b/cli/src/cli-cmd-volume.c
index ac70cd5c068..f7586c44c11 100644
--- a/cli/src/cli-cmd-volume.c
+++ b/cli/src/cli-cmd-volume.c
@@ -1490,7 +1490,10 @@ void
 cli_print_detailed_status (cli_volume_status_t *status)
 {
         cli_out ("%-20s : %-20s", "Brick", status->brick);
-        cli_out ("%-20s : %-20d", "Port", status->port);
+        if (status->online)
+                cli_out ("%-20s : %-20d", "Port", status->port);
+        else
+                cli_out ("%-20s : %-20s", "Port", "N/A");
         cli_out ("%-20s : %-20c", "Online", (status->online) ? 'Y' : 'N');
         cli_out ("%-20s : %-20s", "Pid", status->pid_str);
 
@@ -1565,10 +1568,18 @@ cli_print_brick_status (cli_volume_status_t *status)
                         printf ("%s", p);
                         while (num_tabs-- != 0)
                                 printf ("\t");
-                        if (status->port)
-                                cli_out ("%d\t%c\t%s",
-                                         status->port, status->online?'Y':'N',
-                                         status->pid_str);
+                        if (status->port) {
+                                if (status->online)
+                                        cli_out ("%d\t%c\t%s",
+                                                 status->port,
+                                                 status->online?'Y':'N',
+                                                 status->pid_str);
+                                else
+                                        cli_out ("%s\t%c\t%s",
+                                                 "N/A",
+                                                 status->online?'Y':'N',
+                                                 status->pid_str);
+                        }
                         else
                                 cli_out ("%s\t%c\t%s",
                                          "N/A", status->online?'Y':'N',
diff --git a/cli/src/cli-xml-output.c b/cli/src/cli-xml-output.c
index 004ded7657f..34b046cc976 100644
--- a/cli/src/cli-xml-output.c
+++ b/cli/src/cli-xml-output.c
@@ -230,15 +230,6 @@ cli_xml_output_vol_status_common (xmlTextWriterPtr writer, dict_t *dict,
         XML_RET_CHECK_AND_GOTO (ret, out);
 
         memset (key, 0, sizeof (key));
-        snprintf (key, sizeof (key), "brick%d.port", brick_index);
-        ret = dict_get_int32 (dict, key, &port);
-        if (ret)
-                goto out;
-        ret = xmlTextWriterWriteFormatElement (writer, (xmlChar *)"port",
-                                               "%d", port);
-        XML_RET_CHECK_AND_GOTO (ret, out);
-
-        memset (key, 0, sizeof (key));
         snprintf (key, sizeof (key), "brick%d.status", brick_index);
         ret = dict_get_int32 (dict, key, &status);
         if (ret)
@@ -249,6 +240,27 @@ cli_xml_output_vol_status_common (xmlTextWriterPtr writer, dict_t *dict,
         *online = status;
 
         memset (key, 0, sizeof (key));
+        snprintf (key, sizeof (key), "brick%d.port", brick_index);
+        ret = dict_get_int32 (dict, key, &port);
+        if (ret)
+                goto out;
+
+        /* If the process is either offline or doesn't provide a port (shd)
+         * port = "N/A"
+         * else print the port number of the process.
+         */
+
+        if (*online == 1 && port != 0)
+                ret = xmlTextWriterWriteFormatElement (writer,
+                                                       (xmlChar *)"port",
+                                                       "%d", port);
+        else
+                ret = xmlTextWriterWriteFormatElement (writer,
+                                                       (xmlChar *)"port",
+                                                       "%s", "N/A");
+        XML_RET_CHECK_AND_GOTO (ret, out);
+
+        memset (key, 0, sizeof (key));
         snprintf (key, sizeof (key), "brick%d.pid", brick_index);
         ret = dict_get_int32 (dict, key, &pid);
         if (ret)
diff --git a/tests/bugs/bug-861542.t b/tests/bugs/bug-861542.t
new file mode 100755
index 00000000000..8aef8146ece
--- /dev/null
+++ b/tests/bugs/bug-861542.t
@@ -0,0 +1,51 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+
+cleanup;
+
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume info;
+# Distributed volume with a single brick was chosen solely for the ease of
+#implementing the test case (to be precise, for the ease of extracting the port number).
+TEST $CLI volume create $V0 $H0:$B0/brick0;
+
+TEST $CLI volume start $V0;
+
+function port_field()
+{
+    local vol=$1;
+    local opt=$2;
+    if [ $opt -eq '0' ]; then
+        $CLI volume status $vol | grep "brick0" | awk '{print $3}';
+    else
+        $CLI volume status $vol detail | grep "^Port " | awk '{print $3}';
+    fi
+}
+
+function xml_port_field()
+{
+    local vol=$1;
+    local opt=$2;
+        $CLI --xml volume status $vol $opt | \
+#Find the first occurrence of the string between <port> and </port>
+        sed -r 's/<port>/&\n/;s/<\/port>/\n&/;s/^.*\n(.*)\n.*$/\1/'| \
+        grep -v xml | tr -d '\n';
+}
+
+TEST $CLI volume status $V0;
+TEST $CLI volume status $V0 detail;
+TEST $CLI --xml volume status $V0;
+TEST $CLI --xml volume status $V0 detail;
+
+# Kill the brick process. After this, port number for the killed (in this case brick) process must be "N/A".
+kill `cat /var/lib/glusterd/vols/$V0/run/$H0-d-backends-brick0.pid`
+
+EXPECT "N/A" port_field $V0 '0'; # volume status
+EXPECT "N/A" port_field $V0 '1'; # volume status detail
+
+EXPECT "N/A" xml_port_field $V0 '';
+EXPECT "N/A" xml_port_field $V0 'detail';
+
+cleanup;