You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ashishkumar50 (via GitHub)" <gi...@apache.org> on 2023/07/24 04:56:35 UTC

[GitHub] [ozone] ashishkumar50 commented on a diff in pull request #5099: HDDS-8212. ozone admin printTopology command should have json output

ashishkumar50 commented on code in PR #5099:
URL: https://github.com/apache/ozone/pull/5099#discussion_r1271729775


##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -180,16 +204,119 @@ private String getAdditionNodeOutput(HddsProtos.Node node) {
 
   // Format "ipAddress(hostName):PortName1=PortValue1    OperationalState
   //     networkLocation
-  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes) {
-    nodes.forEach(node -> {
-      System.out.print(" " + getAdditionNodeOutput(node) +
-          node.getNodeID().getIpAddress() + "(" +
-          node.getNodeID().getHostName() + ")" +
-          ":" + formatPortOutput(node.getNodeID().getPortsList()));
-      System.out.println("    "
-          + node.getNodeOperationalStates(0) + "    " +
-          (node.getNodeID().getNetworkLocation() != null ?
-              node.getNodeID().getNetworkLocation() : "NA"));
-    });
+  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes,
+                                      String state) throws IOException {
+    if (json) {

Review Comment:
   Please merge multiple if else.
   
   if (json && fullInfo) {.... return;}
   else if(json && !fullInfo {... return;}



##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -180,16 +204,119 @@ private String getAdditionNodeOutput(HddsProtos.Node node) {
 
   // Format "ipAddress(hostName):PortName1=PortValue1    OperationalState
   //     networkLocation
-  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes) {
-    nodes.forEach(node -> {
-      System.out.print(" " + getAdditionNodeOutput(node) +
-          node.getNodeID().getIpAddress() + "(" +
-          node.getNodeID().getHostName() + ")" +
-          ":" + formatPortOutput(node.getNodeID().getPortsList()));
-      System.out.println("    "
-          + node.getNodeOperationalStates(0) + "    " +
-          (node.getNodeID().getNetworkLocation() != null ?
-              node.getNodeID().getNetworkLocation() : "NA"));
-    });
+  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes,
+                                      String state) throws IOException {
+    if (json) {
+      if (fullInfo) {
+        ArrayList<NodeTopologyFull> nodesJson = new ArrayList<>();
+        nodes.forEach(node -> {
+          NodeTopologyFull nodeJson =
+              new NodeTopologyFull(
+                  DatanodeDetails.getFromProtoBuf(node.getNodeID()), state);
+          nodesJson.add(nodeJson);
+        });
+        System.out.println(
+            JsonUtils.toJsonStringWithDefaultPrettyPrinter(nodesJson));
+      } else {
+        ArrayList<NodeTopologyDefault> nodesJson = new ArrayList<>();
+        nodes.forEach(node -> {
+          NodeTopologyDefault nodeJson = new NodeTopologyDefault(
+              DatanodeDetails.getFromProtoBuf(node.getNodeID()), state);
+          nodesJson.add(nodeJson);
+        });
+        System.out.println(
+            JsonUtils.toJsonStringWithDefaultPrettyPrinter(nodesJson));
+      }
+    } else {
+      // show node state
+      System.out.println("State = " + state);
+      nodes.forEach(node -> {
+        System.out.print(" " + getAdditionNodeOutput(node) +
+            node.getNodeID().getIpAddress() + "(" +
+            node.getNodeID().getHostName() + ")" +
+            ":" + formatPortOutput(node.getNodeID().getPortsList()));
+        System.out.println("    "
+            + node.getNodeOperationalStates(0) + "    " +
+            (node.getNodeID().getNetworkLocation() != null ?
+                node.getNodeID().getNetworkLocation() : "NA"));
+      });
+    }
+  }
+
+  private static class ListJsonSerializer extends
+      JsonSerializer<List<DatanodeDetails.Port>> {
+    @Override
+    public void serialize(List<DatanodeDetails.Port> value, JsonGenerator jgen,
+                          SerializerProvider provider)
+        throws IOException {
+      jgen.writeStartObject();
+      for (DatanodeDetails.Port port : value) {
+        jgen.writeNumberField(port.getName().toString(), port.getValue());
+      }
+      jgen.writeEndObject();
+    }
+  }
+
+  private static class NodeTopologyOrder {
+    private String ipAddress;
+    private String hostName;
+    private String nodeState;
+    private String operationalState;
+    private String networkLocation;
+
+    NodeTopologyOrder(DatanodeDetails node, String nState, String opState) {
+      ipAddress = node.getIpAddress();
+      hostName = node.getHostName();
+      nodeState = nState;
+      operationalState = opState;
+      networkLocation = node.getNetworkLocation();

Review Comment:
   Keep consistent as without json output
   ```
   networkLocation = (node.getNodeID().getNetworkLocation() != null ?
       node.getNodeID().getNetworkLocation() : "NA")
   ```



##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -180,16 +204,119 @@ private String getAdditionNodeOutput(HddsProtos.Node node) {
 
   // Format "ipAddress(hostName):PortName1=PortValue1    OperationalState
   //     networkLocation
-  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes) {
-    nodes.forEach(node -> {
-      System.out.print(" " + getAdditionNodeOutput(node) +
-          node.getNodeID().getIpAddress() + "(" +
-          node.getNodeID().getHostName() + ")" +
-          ":" + formatPortOutput(node.getNodeID().getPortsList()));
-      System.out.println("    "
-          + node.getNodeOperationalStates(0) + "    " +
-          (node.getNodeID().getNetworkLocation() != null ?
-              node.getNodeID().getNetworkLocation() : "NA"));
-    });
+  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes,
+                                      String state) throws IOException {
+    if (json) {
+      if (fullInfo) {
+        ArrayList<NodeTopologyFull> nodesJson = new ArrayList<>();

Review Comment:
   ```suggestion
           List<NodeTopologyFull> nodesJson = new ArrayList<>();
   ```



##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -180,16 +204,119 @@ private String getAdditionNodeOutput(HddsProtos.Node node) {
 
   // Format "ipAddress(hostName):PortName1=PortValue1    OperationalState
   //     networkLocation
-  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes) {
-    nodes.forEach(node -> {
-      System.out.print(" " + getAdditionNodeOutput(node) +
-          node.getNodeID().getIpAddress() + "(" +
-          node.getNodeID().getHostName() + ")" +
-          ":" + formatPortOutput(node.getNodeID().getPortsList()));
-      System.out.println("    "
-          + node.getNodeOperationalStates(0) + "    " +
-          (node.getNodeID().getNetworkLocation() != null ?
-              node.getNodeID().getNetworkLocation() : "NA"));
-    });
+  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes,
+                                      String state) throws IOException {
+    if (json) {
+      if (fullInfo) {
+        ArrayList<NodeTopologyFull> nodesJson = new ArrayList<>();
+        nodes.forEach(node -> {
+          NodeTopologyFull nodeJson =
+              new NodeTopologyFull(
+                  DatanodeDetails.getFromProtoBuf(node.getNodeID()), state);
+          nodesJson.add(nodeJson);
+        });
+        System.out.println(
+            JsonUtils.toJsonStringWithDefaultPrettyPrinter(nodesJson));
+      } else {
+        ArrayList<NodeTopologyDefault> nodesJson = new ArrayList<>();

Review Comment:
   ```suggestion
           List<NodeTopologyDefault> nodesJson = new ArrayList<>();
   ```



##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -136,7 +144,8 @@ public Class<?> getParentType() {
   // Format
   // Location: rack1
   //  ipAddress(hostName) OperationalState
-  private void printOrderedByLocation(List<HddsProtos.Node> nodes) {
+  private void printOrderedByLocation(List<HddsProtos.Node> nodes,
+                                      String nState) throws IOException {

Review Comment:
   ```suggestion
                                         String state) throws IOException {
   ```



##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -153,13 +162,28 @@ private void printOrderedByLocation(List<HddsProtos.Node> nodes) {
     ArrayList<String> locations = new ArrayList<>(tree.keySet());
     Collections.sort(locations);
 
-    locations.forEach(location -> {
-      System.out.println("Location: " + location);
-      tree.get(location).forEach(n -> {
-        System.out.println(" " + n.getIpAddress() + "(" + n.getHostName()
-            + ") " + state.get(n));
+    if (json) {
+      List<NodeTopologyOrder> nodesJson = new ArrayList<>();
+      locations.forEach(location -> {
+        tree.get(location).forEach(n -> {
+          NodeTopologyOrder nodeJson = new NodeTopologyOrder(n, nState,
+              state.get(n).toString());
+          nodesJson.add(nodeJson);
+        });
+      });
+      System.out.println(
+          JsonUtils.toJsonStringWithDefaultPrettyPrinter(nodesJson));
+    } else {

Review Comment:
   Remove else, return from if statement.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org