SOLR-13672: Cloud -> Zk Status page now parses response from Zookeeper 3.5.5 correctly

(Back ported from 8 commits on master branch)

solr/CHANGES.txt

@@ -84,6 +84,8 @@ Bug Fixes
 * SOLR-13679: Default style of ExplainDocTransformer registered via solrconfig.xml should be same as default style
   of ExplainDocTransformer registered in defaultFactories (Munendra S N)
 
+* SOLR-13672: Cloud -> Zk Status page now parses response from Zookeeper 3.5.5 correctly (Jörn Franke, janhoy, Shawn Heisey)
+
 Other Changes
 ----------------------
 

solr/core/src/java/org/apache/solr/handler/admin/ZookeeperStatusHandler.java

@@ -28,6 +28,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.stream.Collectors;
 
@@ -46,7 +47,7 @@ import org.slf4j.LoggerFactory;
  *
  * @since solr 7.5
  */
-public final class ZookeeperStatusHandler extends RequestHandlerBase {
+public class ZookeeperStatusHandler extends RequestHandlerBase {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   private static final int ZOOKEEPER_DEFAULT_PORT = 2181;
@@ -99,6 +100,10 @@ public final class ZookeeperStatusHandler extends RequestHandlerBase {
     for (String zk : zookeepers) {
       try {
         Map<String, Object> stat = monitorZookeeper(zk);
+        if (stat.containsKey("errors")) {
+          errors.addAll((List<String>)stat.get("errors"));
+          stat.remove("errors");
+        }
         details.add(stat);
         if ("true".equals(String.valueOf(stat.get("ok")))) {
           numOk++;
@@ -113,14 +118,13 @@ public final class ZookeeperStatusHandler extends RequestHandlerBase {
           standalone++;
         }
       } catch (SolrException se) {
-        log.warn("Failed talking to zookeeper" + zk, se);
+        log.warn("Failed talking to zookeeper " + zk, se);
         errors.add(se.getMessage());
-        zkStatus.put("errors", errors);
         Map<String, Object> stat = new HashMap<>();
         stat.put("host", zk);
         stat.put("ok", false);
-        zkStatus.put("status", STATUS_YELLOW);
+        status = STATUS_YELLOW;
-        return zkStatus;
+        details.add(stat);
       }
     }
     zkStatus.put("details", details);
@@ -178,38 +182,39 @@ public final class ZookeeperStatusHandler extends RequestHandlerBase {
     return zkStatus;
   }
 
-  private Map<String, Object> monitorZookeeper(String zkHostPort) throws SolrException {
+  protected Map<String, Object> monitorZookeeper(String zkHostPort) throws SolrException {
     Map<String, Object> obj = new HashMap<>();
+    List<String> errors = new ArrayList<>();
     obj.put("host", zkHostPort);
     List<String> lines = getZkRawResponse(zkHostPort, "ruok");
+    validateZkRawResponse(lines, zkHostPort, "ruok");
     boolean ok = "imok".equals(lines.get(0));
-    if (ok == false) {
-      log.warn("Check 4lw.commands.whitelist setting in zookeeper configuration file, ZK response {}", lines.get(0));
-      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, lines.get(0) + " Check 4lw.commands.whitelist setting in zookeeper configuration file.");
-    }
     obj.put("ok", ok);
     lines = getZkRawResponse(zkHostPort, "mntr");
-    String[] parts;
+    validateZkRawResponse(lines, zkHostPort, "mntr");
     for (String line : lines) {
-      parts = line.split("\t");
+      String[] parts = line.split("\t");
       if (parts.length >= 2) {
         obj.put(parts[0], parts[1]);
       } else {
-        log.warn("Check 4lw.commands.whitelist setting in zookeeper configuration file, ZK response {}", line);
+        String err = String.format(Locale.ENGLISH, "Unexpected line in 'mntr' response from Zookeeper %s: %s", zkHostPort, line);
-        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, line + " Check 4lw.commands.whitelist setting in zookeeper configuration file.");
+        log.warn(err);
+        errors.add(err);
       }
     }
     lines = getZkRawResponse(zkHostPort, "conf");
-
+    validateZkRawResponse(lines, zkHostPort, "conf");
     for (String line : lines) {
-      parts = line.split("=");
+      String[] parts = line.split("=");
       if (parts.length >= 2) {
         obj.put(parts[0], parts[1]);
-      } else {
+      } else if (!line.startsWith("membership:")) {
-        log.warn("Check 4lw.commands.whitelist setting in zookeeper configuration file, ZK response {}", line);
+        String err = String.format(Locale.ENGLISH, "Unexpected line in 'conf' response from Zookeeper %s: %s", zkHostPort, line);
-        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, line + " Check 4lw.commands.whitelist setting in zookeeper configuration file.");
+        log.warn(err);
+        errors.add(err);
       }
     }
+    obj.put("errors", errors);
     return obj;
   }
   
@@ -219,7 +224,7 @@ public final class ZookeeperStatusHandler extends RequestHandlerBase {
    * @param fourLetterWordCommand the custom 4-letter command to send to Zookeeper
    * @return a list of lines returned from Zookeeper
    */
-  private List<String> getZkRawResponse(String zkHostPort, String fourLetterWordCommand) {
+  protected List<String> getZkRawResponse(String zkHostPort, String fourLetterWordCommand) {
     String[] hostPort = zkHostPort.split(":");
     String host = hostPort[0];
     int port = ZOOKEEPER_DEFAULT_PORT;
@@ -235,12 +240,30 @@ public final class ZookeeperStatusHandler extends RequestHandlerBase {
       out.println(fourLetterWordCommand);
       List<String> response = in.lines().collect(Collectors.toList());
       log.debug("Got response from ZK on host {} and port {}: {}", host, port, response);
-      if (response == null || response.isEmpty()) {
-        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Empty response from Zookeeper " + zkHostPort);
-      }
       return response;
     } catch (IOException e) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed talking to Zookeeper " + zkHostPort, e);
     }
   }
+
+  /**
+   * Takes the raw response lines returned by {@link #getZkRawResponse(String, String)} and runs some validations
+   * @param response the lines
+   * @param zkHostPort the host
+   * @param fourLetterWordCommand the 4lw command
+   * @return true if validation succeeds
+   * @throws SolrException if validation fails
+   */
+  protected boolean validateZkRawResponse(List<String> response, String zkHostPort, String fourLetterWordCommand) {
+    if (response == null || response.isEmpty() || (response.size() == 1 && response.get(0).trim().isEmpty())) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Empty response from Zookeeper " + zkHostPort);
+    }
+    if (response.size() == 1 && response.get(0).contains("not in the whitelist")) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Could not execute " + fourLetterWordCommand +
+          " towards ZK host " + zkHostPort + ". Add this line to the 'zoo.cfg' " +
+          "configuration file on each zookeeper node: '4lw.commands.whitelist=mntr,conf,ruok'. See also chapter " +
+          "'Setting Up an External ZooKeeper Ensemble' in the Solr Reference Guide.");
+    }
+    return true;
+  }
 }

solr/core/src/test/org/apache/solr/handler/admin/ZookeeperStatusHandlerTest.java

@@ -20,6 +20,8 @@ package org.apache.solr.handler.admin;
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.net.URL;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ExecutionException;
@@ -32,15 +34,24 @@ import org.apache.solr.client.solrj.impl.HttpSolrClient;
 import org.apache.solr.client.solrj.request.GenericSolrRequest;
 import org.apache.solr.client.solrj.response.DelegationTokenResponse;
 import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.util.NamedList;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.mockito.Answers;
+import org.mockito.ArgumentMatchers;
+import org.noggit.JSONUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 public class ZookeeperStatusHandlerTest extends SolrCloudTestCase {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
@@ -87,4 +98,67 @@ public class ZookeeperStatusHandlerTest extends SolrCloudTestCase {
     assertTrue(Integer.parseInt((String) details.get("zk_znode_count")) > 50);
     solr.close();
   }
+
+  @Test
+  public void testEnsembleStatusMock() {
+    assumeWorkingMockito();
+    ZookeeperStatusHandler zkStatusHandler = mock(ZookeeperStatusHandler.class);
+    when(zkStatusHandler.getZkRawResponse("zoo1:2181", "ruok")).thenReturn(Arrays.asList("imok"));
+    when(zkStatusHandler.getZkRawResponse("zoo1:2181", "mntr")).thenReturn(
+        Arrays.asList("zk_version\t3.5.5-390fe37ea45dee01bf87dc1c042b5e3dcce88653, built on 05/03/2019 12:07 GMT",
+        "zk_avg_latency\t1"));
+    when(zkStatusHandler.getZkRawResponse("zoo1:2181", "conf")).thenReturn(
+        Arrays.asList("clientPort=2181",
+        "secureClientPort=-1",
+        "thisIsUnexpected",
+        "membership: "));
+
+    when(zkStatusHandler.getZkRawResponse("zoo2:2181", "ruok")).thenReturn(Arrays.asList(""));
+
+    when(zkStatusHandler.getZkRawResponse("zoo3:2181", "ruok")).thenReturn(Arrays.asList("imok"));
+    when(zkStatusHandler.getZkRawResponse("zoo3:2181", "mntr")).thenReturn(
+        Arrays.asList("mntr is not executed because it is not in the whitelist.")); // Actual response from ZK if not whitelisted
+    when(zkStatusHandler.getZkRawResponse("zoo3:2181", "conf")).thenReturn(
+        Arrays.asList("clientPort=2181"));
+
+    when(zkStatusHandler.getZkStatus(anyString())).thenCallRealMethod();
+    when(zkStatusHandler.monitorZookeeper(anyString())).thenCallRealMethod();
+    when(zkStatusHandler.validateZkRawResponse(ArgumentMatchers.any(), any(), any())).thenAnswer(Answers.CALLS_REAL_METHODS);
+
+    Map<String, Object> mockStatus = zkStatusHandler.getZkStatus("zoo1:2181,zoo2:2181,zoo3:2181");
+    String expected = "{\n" +
+        "  \"ensembleSize\":3,\n" +
+        "  \"details\":[\n" +
+        "    {\n" +
+        "      \"zk_version\":\"3.5.5-390fe37ea45dee01bf87dc1c042b5e3dcce88653, built on 05/03/2019 12:07 GMT\",\n" +
+        "      \"zk_avg_latency\":\"1\",\n" +
+        "      \"host\":\"zoo1:2181\",\n" +
+        "      \"clientPort\":\"2181\",\n" +
+        "      \"secureClientPort\":\"-1\",\n" +
+        "      \"ok\":true},\n" +
+        "    {\n" +
+        "      \"host\":\"zoo2:2181\",\n" +
+        "      \"ok\":false},\n" +
+        "    {\n" +
+        "      \"host\":\"zoo3:2181\",\n" +
+        "      \"ok\":false}],\n" +
+        "  \"zkHost\":\"zoo1:2181,zoo2:2181,zoo3:2181\",\n" +
+        "  \"errors\":[\n" +
+        "    \"Unexpected line in 'conf' response from Zookeeper zoo1:2181: thisIsUnexpected\",\n" +
+        "    \"Empty response from Zookeeper zoo2:2181\",\n" +
+        "    \"Could not execute mntr towards ZK host zoo3:2181. Add this line to the 'zoo.cfg' configuration file on each zookeeper node: '4lw.commands.whitelist=mntr,conf,ruok'. See also chapter 'Setting Up an External ZooKeeper Ensemble' in the Solr Reference Guide.\"],\n" +
+        "  \"status\":\"yellow\"}";
+    assertEquals(expected, JSONUtil.toJSON(mockStatus));
+  }
+
+  @Test(expected = SolrException.class)
+  public void validateNotWhitelisted() {
+    new ZookeeperStatusHandler(null).validateZkRawResponse(Collections.singletonList("mntr is not executed because it is not in the whitelist."),
+        "zoo1:2181", "mntr");
+  }
+
+  @Test(expected = SolrException.class)
+  public void validateEmptyResponse() {
+    new ZookeeperStatusHandler(null).validateZkRawResponse(Collections.emptyList(), "zoo1:2181", "mntr");
+  }
 }

solr/webapp/web/js/angular/controllers/cloud.js

@@ -530,7 +530,7 @@ var zkStatusSubController = function($scope, ZookeeperStatus) {
     $scope.initZookeeper = function() {
       ZookeeperStatus.monitor({}, function(data) {
         $scope.zkState = data.zkStatus;
-        $scope.mainKeys = ["ok", "clientPort", "zk_server_state", "zk_version", 
+        $scope.mainKeys = ["ok", "clientPort", "secureClientPort", "zk_server_state", "zk_version",
           "zk_approximate_data_size", "zk_znode_count", "zk_num_alive_connections"];
         $scope.detailKeys = ["dataDir", "dataLogDir", 
           "zk_avg_latency", "zk_max_file_descriptor_count", "zk_watch_count", 
@@ -538,7 +538,14 @@ var zkStatusSubController = function($scope, ZookeeperStatus) {
           "tickTime", "maxClientCnxns", "minSessionTimeout", "maxSessionTimeout"];
         $scope.ensembleMainKeys = ["serverId", "electionPort", "quorumPort"];
         $scope.ensembleDetailKeys = ["peerType", "electionAlg", "initLimit", "syncLimit",
-          "zk_followers", "zk_synced_followers", "zk_pending_syncs"];
+          "zk_followers", "zk_synced_followers", "zk_pending_syncs",
+          "server.1", "server.2", "server.3", "server.4", "server.5"];
+        $scope.notEmptyRow = function(key) {
+          for (hostId in $scope.zkState.details) {
+            if (key in $scope.zkState.details[hostId]) return true;
+          }
+          return false;
+        };
       });
     };
 

solr/webapp/web/partials/cloud.html

@@ -67,7 +67,7 @@ limitations under the License.
               {{host[key]}}
             </td>
           </tr>
-          <tr ng-repeat="key in ensembleDetailKeys" ng-show="showDetails && zkState.mode === 'ensemble'">
+          <tr ng-repeat="key in ensembleDetailKeys | filter: notEmptyRow" ng-show="showDetails && zkState.mode === 'ensemble'">
             <td>{{key}}</td>
             <td ng-repeat="host in zkState.details">
               {{host[key]}}