Fix nodetool ring, status output when DNS resolution or port printing are in use

Patch by Adam Holmberg and Scott Wolfenhaut; reviewed by Berenguer Blasi, Brandon Williams and Ekaterina Dimitrova for CASSANDRA-16283

CHANGES.txt

@@ -1,4 +1,5 @@
 4.0-beta5
+ * Fix nodetool ring, status output when DNS resolution or port printing are in use (CASSANDRA-16283)
  * Upgrade Jacoco to 0.8.6 (for Java 11 support) (CASSANDRA-16365)
  * Move excessive repair debug loggings to trace level (CASSANDRA-16406)
  * Restore validation of each message's protocol version (CASSANDRA-16374)

src/java/org/apache/cassandra/locator/SimpleSnitch.java

@@ -25,10 +25,11 @@ package org.apache.cassandra.locator;
 public class SimpleSnitch extends AbstractEndpointSnitch
 {
     public static final String DATA_CENTER_NAME = "datacenter1";
+    public static final String RACK_NAME = "rack1";
 
     public String getRack(InetAddressAndPort endpoint)
     {
-        return "rack1";
+        return RACK_NAME;
     }
 
     public String getDatacenter(InetAddressAndPort endpoint)

src/java/org/apache/cassandra/service/StorageService.java

@@ -5170,7 +5170,7 @@ public class StorageService extends NotificationBroadcasterSupport implements IE
     {
         LinkedHashMap<InetAddressAndPort, Float> result = getEffectiveOwnership(keyspace);
         LinkedHashMap<String, Float> asStrings = new LinkedHashMap<>();
-        result.entrySet().stream().forEachOrdered(entry -> asStrings.put(entry.getKey().toString(), entry.getValue()));
+        result.entrySet().stream().forEachOrdered(entry -> asStrings.put(entry.getKey().getHostAddressAndPort(), entry.getValue()));
         return asStrings;
     }
 

src/java/org/apache/cassandra/tools/nodetool/HostStatWithPort.java

@@ -42,6 +42,6 @@ public class HostStatWithPort extends HostStat
 
         return resolveIp ?
                endpointWithPort.address.getHostName() + ':' + endpointWithPort.port :
-               endpointWithPort.toString();
+               endpointWithPort.getHostAddressAndPort();
     }
 }

src/java/org/apache/cassandra/tools/nodetool/Ring.java

@@ -23,7 +23,6 @@ import io.airlift.airline.Command;
 import io.airlift.airline.Option;
 
 import java.io.PrintStream;
-import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.text.DecimalFormat;
 import java.util.ArrayList;
@@ -34,6 +33,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 
+import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
 import org.apache.cassandra.tools.NodeProbe;
 import org.apache.cassandra.tools.NodeTool;
 import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
@@ -49,192 +49,76 @@ public class Ring extends NodeToolCmd
     @Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description = "Show node domain names instead of IPs")
     private boolean resolveIp = false;
 
+    private PrintStream out;
+    private EndpointSnitchInfoMBean epSnitchInfo;
+    private Collection<String> liveNodes, deadNodes, joiningNodes, leavingNodes, movingNodes;
+    private Map<String, String> loadMap;
+
     @Override
     public void execute(NodeProbe probe)
     {
-        PrintStream out = probe.output().out;
+        out = probe.output().out;
+        liveNodes = probe.getLiveNodes(true);
+        deadNodes = probe.getUnreachableNodes(true);
+        joiningNodes = probe.getJoiningNodes(true);
+        leavingNodes = probe.getLeavingNodes(true);
+        movingNodes = probe.getMovingNodes(true);
+        loadMap = probe.getLoadMap(true);
+        epSnitchInfo = probe.getEndpointSnitchInfoProxy();
+
+        Map<String, String> tokensToEndpoints = probe.getTokenToEndpointMap(true);
+        LinkedHashMultimap<String, String> endpointsToTokens = LinkedHashMultimap.create();
+        boolean haveVnodes = false;
+        for (Map.Entry<String, String> entry : tokensToEndpoints.entrySet())
+        {
+            haveVnodes |= endpointsToTokens.containsKey(entry.getValue());
+            endpointsToTokens.put(entry.getValue(), entry.getKey());
+        }
+
+        int maxAddressLength = Collections.max(endpointsToTokens.keys(),
+                                               Comparator.comparingInt(String::length)).length();
+
+        String formatPlaceholder = "%%-%ds  %%-12s%%-7s%%-8s%%-16s%%-20s%%-44s%%n";
+        String format = format(formatPlaceholder, maxAddressLength);
+
+        StringBuilder errors = new StringBuilder();
+        boolean showEffectiveOwnership = true;
+
+        // Calculate per-token ownership of the ring
+        Map<String, Float> ownerships;
         try
         {
-            Map<String, String> tokensToEndpoints = probe.getTokenToEndpointMap(printPort);
+            ownerships = probe.effectiveOwnershipWithPort(keyspace);
-            LinkedHashMultimap<String, String> endpointsToTokens = LinkedHashMultimap.create();
-            boolean haveVnodes = false;
-            for (Map.Entry<String, String> entry : tokensToEndpoints.entrySet())
-            {
-                haveVnodes |= endpointsToTokens.containsKey(entry.getValue());
-                endpointsToTokens.put(entry.getValue(), entry.getKey());
-            }
-
-            int maxAddressLength = Collections.max(endpointsToTokens.keys(), new Comparator<String>()
-            {
-                @Override
-                public int compare(String first, String second)
-                {
-                    return Integer.compare(first.length(), second.length());
-                }
-            }).length();
-
-            String formatPlaceholder = "%%-%ds  %%-12s%%-7s%%-8s%%-16s%%-20s%%-44s%%n";
-            String format = format(formatPlaceholder, maxAddressLength);
-
-            StringBuilder errors = new StringBuilder();
-            boolean showEffectiveOwnership = true;
-
-            if (printPort)
-            {
-                // Calculate per-token ownership of the ring
-                Map<String, Float> ownerships;
-                try
-                {
-                    ownerships = probe.effectiveOwnershipWithPort(keyspace);
-                }
-                catch (IllegalStateException ex)
-                {
-                    ownerships = probe.getOwnershipWithPort();
-                    errors.append("Note: ").append(ex.getMessage()).append("%n");
-                    showEffectiveOwnership = false;
-                }
-                catch (IllegalArgumentException ex)
-                {
-                    out.printf("%nError: %s%n", ex.getMessage());
-                    return;
-                }
-
-
-                out.println();
-                for (Entry<String, SetHostStatWithPort> entry : NodeTool.getOwnershipByDcWithPort(probe, resolveIp, tokensToEndpoints, ownerships).entrySet())
-                    printDc(probe, format, entry.getKey(), endpointsToTokens, entry.getValue(), showEffectiveOwnership);
-
-                if (haveVnodes)
-                {
-                    out.println("  Warning: \"nodetool ring\" is used to output all the tokens of a node.");
-                    out.println("  To view status related info of a node use \"nodetool status\" instead.\n");
-                }
-
-                out.printf("%n  " + errors.toString());
-            }
-            else
-            {
-                // Calculate per-token ownership of the ring
-                Map<InetAddress, Float> ownerships;
-                try
-                {
-                    ownerships = probe.effectiveOwnership(keyspace);
-                }
-                catch (IllegalStateException ex)
-                {
-                    ownerships = probe.getOwnership();
-                    errors.append("Note: ").append(ex.getMessage()).append("%n");
-                    showEffectiveOwnership = false;
-                }
-                catch (IllegalArgumentException ex)
-                {
-                    out.printf("%nError: %s%n", ex.getMessage());
-                    return;
-                }
-
-
-                out.println();
-                for (Entry<String, SetHostStat> entry : NodeTool.getOwnershipByDc(probe, resolveIp, tokensToEndpoints, ownerships).entrySet())
-                    printDc(probe, format, entry.getKey(), endpointsToTokens, entry.getValue(), showEffectiveOwnership);
-
-                if (haveVnodes)
-                {
-                    out.println("  Warning: \"nodetool ring\" is used to output all the tokens of a node.");
-                    out.println("  To view status related info of a node use \"nodetool status\" instead.\n");
-                }
-
-                out.printf("%n  " + errors.toString());
-            }
-        } catch (Exception e)
-        {
-            e.printStackTrace();
-            throw e;
         }
-    }
+        catch (IllegalStateException ex)
-
-    private void printDc(NodeProbe probe, String format,
-                         String dc,
-                         LinkedHashMultimap<String, String> endpointsToTokens,
-                         SetHostStat hoststats,boolean showEffectiveOwnership)
-    {
-        PrintStream out = probe.output().out;
-        Collection<String> liveNodes = probe.getLiveNodes(false);
-        Collection<String> deadNodes = probe.getUnreachableNodes(false);
-        Collection<String> joiningNodes = probe.getJoiningNodes(false);
-        Collection<String> leavingNodes = probe.getLeavingNodes(false);
-        Collection<String> movingNodes = probe.getMovingNodes(false);
-        Map<String, String> loadMap = probe.getLoadMap(false);
-
-        out.println("Datacenter: " + dc);
-        out.println("==========");
-
-        // get the total amount of replicas for this dc and the last token in this dc's ring
-        List<String> tokens = new ArrayList<>();
-        String lastToken = "";
-
-        for (HostStat stat : hoststats)
         {
-            tokens.addAll(endpointsToTokens.get(stat.endpoint.getHostAddress()));
+            ownerships = probe.getOwnershipWithPort();
-            lastToken = tokens.get(tokens.size() - 1);
+            errors.append("Note: ").append(ex.getMessage()).append("%n");
+            showEffectiveOwnership = false;
+        }
+        catch (IllegalArgumentException ex)
+        {
+            out.printf("%nError: %s%n", ex.getMessage());
+            return;
         }
 
-        out.printf(format, "Address", "Rack", "Status", "State", "Load", "Owns", "Token");
-
-        if (hoststats.size() > 1)
-            out.printf(format, "", "", "", "", "", "", lastToken);
-        else
-            out.println();
-
-        for (HostStat stat : hoststats)
-        {
-            String endpoint = stat.endpoint.getHostAddress();
-            String rack;
-            try
-            {
-                rack = probe.getEndpointSnitchInfoProxy().getRack(endpoint);
-            }
-            catch (UnknownHostException e)
-            {
-                rack = "Unknown";
-            }
-
-            String status = liveNodes.contains(endpoint)
-                    ? "Up"
-                    : deadNodes.contains(endpoint)
-                            ? "Down"
-                            : "?";
-
-            String state = "Normal";
-
-            if (joiningNodes.contains(endpoint))
-                state = "Joining";
-            else if (leavingNodes.contains(endpoint))
-                state = "Leaving";
-            else if (movingNodes.contains(endpoint))
-                state = "Moving";
-
-            String load = loadMap.containsKey(endpoint)
-                    ? loadMap.get(endpoint)
-                    : "?";
-            String owns = stat.owns != null && showEffectiveOwnership? new DecimalFormat("##0.00%").format(stat.owns) : "?";
-            out.printf(format, stat.ipOrDns(), rack, status, state, load, owns, stat.token);
-        }
         out.println();
+        for (Entry<String, SetHostStatWithPort> entry : NodeTool.getOwnershipByDcWithPort(probe, resolveIp, tokensToEndpoints, ownerships).entrySet())
+            printDc(format, entry.getKey(), endpointsToTokens, entry.getValue(), showEffectiveOwnership);
+
+        if (haveVnodes)
+        {
+            out.println("  Warning: \"nodetool ring\" is used to output all the tokens of a node.");
+            out.println("  To view status related info of a node use \"nodetool status\" instead.\n");
+        }
+
+        out.printf("%n  " + errors.toString());
     }
 
-    private void printDc(NodeProbe probe, String format,
+    private void printDc(String format, String dc,
-                         String dc,
                          LinkedHashMultimap<String, String> endpointsToTokens,
-                         SetHostStatWithPort hoststats,boolean showEffectiveOwnership)
+                         SetHostStatWithPort hoststats, boolean showEffectiveOwnership)
     {
-        PrintStream out = probe.output().out;
-        Collection<String> liveNodes = probe.getLiveNodes(true);
-        Collection<String> deadNodes = probe.getUnreachableNodes(true);
-        Collection<String> joiningNodes = probe.getJoiningNodes(true);
-        Collection<String> leavingNodes = probe.getLeavingNodes(true);
-        Collection<String> movingNodes = probe.getMovingNodes(true);
-        Map<String, String> loadMap = probe.getLoadMap(true);
-
         out.println("Datacenter: " + dc);
         out.println("==========");
 
@@ -244,9 +128,7 @@ public class Ring extends NodeToolCmd
 
         for (HostStatWithPort stat : hoststats)
         {
-            // Remove extra '/' from address
+            tokens.addAll(endpointsToTokens.get(stat.endpointWithPort.getHostAddressAndPort()));
-            String addressNPort = stat.endpointWithPort.toString().replaceAll("^/", "");
-            tokens.addAll(endpointsToTokens.get(addressNPort));
             lastToken = tokens.get(tokens.size() - 1);
         }
 
@@ -259,11 +141,11 @@ public class Ring extends NodeToolCmd
 
         for (HostStatWithPort stat : hoststats)
         {
-            String endpoint = stat.endpoint.toString();
+            String endpoint = stat.endpointWithPort.getHostAddressAndPort();
             String rack;
             try
             {
-                rack = probe.getEndpointSnitchInfoProxy().getRack(endpoint);
+                rack = epSnitchInfo.getRack(endpoint);
             }
             catch (UnknownHostException e)
             {
@@ -289,7 +171,7 @@ public class Ring extends NodeToolCmd
                           ? loadMap.get(endpoint)
                           : "?";
             String owns = stat.owns != null && showEffectiveOwnership? new DecimalFormat("##0.00%").format(stat.owns) : "?";
-            out.printf(format, stat.ipOrDns(), rack, status, state, load, owns, stat.token);
+            out.printf(format, stat.ipOrDns(printPort), rack, status, state, load, owns, stat.token);
         }
         out.println();
     }

src/java/org/apache/cassandra/tools/nodetool/SetHostStatWithPort.java

@@ -50,7 +50,7 @@ public class SetHostStatWithPort implements Iterable<HostStatWithPort>
     public void add(String token, String host, Map<String, Float> ownerships) throws UnknownHostException
     {
         InetAddressAndPort endpoint = InetAddressAndPort.getByName(host);
-        Float owns = ownerships.get(endpoint.toString());
+        Float owns = ownerships.get(endpoint.getHostAddressAndPort());
         hostStats.add(new HostStatWithPort(token, endpoint, resolveIp, owns));
     }
 }

src/java/org/apache/cassandra/tools/nodetool/Status.java

@@ -57,14 +57,14 @@ public class Status extends NodeToolCmd
     public void execute(NodeProbe probe)
     {
         PrintStream out = probe.output().out;
-        joiningNodes = probe.getJoiningNodes(printPort);
+        joiningNodes = probe.getJoiningNodes(true);
-        leavingNodes = probe.getLeavingNodes(printPort);
+        leavingNodes = probe.getLeavingNodes(true);
-        movingNodes = probe.getMovingNodes(printPort);
+        movingNodes = probe.getMovingNodes(true);
-        loadMap = probe.getLoadMap(printPort);
+        loadMap = probe.getLoadMap(true);
-        Map<String, String> tokensToEndpoints = probe.getTokenToEndpointMap(printPort);
+        Map<String, String> tokensToEndpoints = probe.getTokenToEndpointMap(true);
-        liveNodes = probe.getLiveNodes(printPort);
+        liveNodes = probe.getLiveNodes(true);
-        unreachableNodes = probe.getUnreachableNodes(printPort);
+        unreachableNodes = probe.getUnreachableNodes(true);
-        hostIDMap = probe.getHostIdMap(printPort);
+        hostIDMap = probe.getHostIdMap(true);
         epSnitchInfo = probe.getEndpointSnitchInfoProxy();
 
         StringBuilder errors = new StringBuilder();
@@ -102,14 +102,13 @@ public class Status extends NodeToolCmd
 
             ArrayListMultimap<String, HostStatWithPort> hostToTokens = ArrayListMultimap.create();
             for (HostStatWithPort stat : dc.getValue())
-                hostToTokens.put(stat.ipOrDns(printPort), stat);
+                hostToTokens.put(stat.endpointWithPort.getHostAddressAndPort(), stat);
 
             for (String endpoint : hostToTokens.keySet())
             {
                 Float owns = ownerships.get(endpoint);
                 List<HostStatWithPort> tokens = hostToTokens.get(endpoint);
-                addNode(endpoint, owns, tokens.get(0).ipOrDns(printPort), tokens.get(0).token, tokens.size(),
+                addNode(endpoint, owns, tokens.get(0), tokens.size(), hasEffectiveOwns, tableBuilder);
-                        hasEffectiveOwns, tableBuilder);
             }
         }
 
@@ -146,10 +145,10 @@ public class Status extends NodeToolCmd
             tableBuilder.add("--", "Address", "Load", "Tokens", owns, "Host ID", "Rack");
     }
 
-    private void addNode(String endpoint, Float owns, String epDns, String token, int size, boolean hasEffectiveOwns,
+    private void addNode(String endpoint, Float owns, HostStatWithPort hostStat, int size, boolean hasEffectiveOwns,
                            TableBuilder tableBuilder)
     {
-        String status, state, load, strOwns, hostID, rack;
+        String status, state, load, strOwns, hostID, rack, epDns;
         if (liveNodes.contains(endpoint)) status = "U";
         else if (unreachableNodes.contains(endpoint)) status = "D";
         else status = "?";
@@ -166,14 +165,16 @@ public class Status extends NodeToolCmd
         try
         {
             rack = epSnitchInfo.getRack(endpoint);
-        } catch (UnknownHostException e)
+        }
+        catch (UnknownHostException e)
         {
             throw new RuntimeException(e);
         }
 
+        epDns = hostStat.ipOrDns(printPort);
         if (isTokenPerNode)
         {
-            tableBuilder.add(statusAndState, epDns, load, strOwns, hostID, token, rack);
+            tableBuilder.add(statusAndState, epDns, load, strOwns, hostID, hostStat.token, rack);
         }
         else
         {

test/distributed/org/apache/cassandra/distributed/test/NodeToolTest.java

@@ -52,7 +52,7 @@ public class NodeToolTest extends TestBaseImpl
     }
 
     @Test
-    public void testCommands() throws Throwable
+    public void testCommands()
     {
         assertEquals(0, NODE.nodetool("help"));
         assertEquals(0, NODE.nodetool("flush"));
@@ -60,11 +60,11 @@ public class NodeToolTest extends TestBaseImpl
     }
 
     @Test
-    public void testCaptureConsoleOutput() throws Throwable
+    public void testCaptureConsoleOutput()
     {
         NodeToolResult ringResult = NODE.nodetoolResult("ring");
         ringResult.asserts().stdoutContains("Datacenter: datacenter0");
-        ringResult.asserts().stdoutContains("127.0.0.1  rack0       Up     Normal");
+        ringResult.asserts().stdoutContains("127.0.0.1       rack0       Up     Normal");
         assertEquals("Non-empty error output", "", ringResult.getStderr());
     }
 

test/unit/org/apache/cassandra/tools/nodetool/RingTest.java

@@ -16,44 +16,96 @@
  * limitations under the License.
  */
 
-package org.apache.cassandra.distributed.test;
+package org.apache.cassandra.tools.nodetool;
 
-import java.io.IOException;
 import java.util.Arrays;
 
-import org.junit.AfterClass;
+import org.junit.BeforeClass;
-import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
 
-import org.apache.cassandra.distributed.api.ICluster;
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.locator.SimpleSnitch;
+import org.apache.cassandra.service.StorageService;
 import org.apache.cassandra.tools.ToolRunner;
-import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.apache.cassandra.utils.FBUtilities;
 import org.assertj.core.api.Assertions;
 
+import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.Matchers.matchesPattern;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
-public class NodeToolRingTest extends TestBaseImpl
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class RingTest extends CQLTester
 {
-    private static ICluster cluster;
+    private static String token;
 
-    @Before
+    @BeforeClass
-    public void setupEnv() throws IOException
+    public static void setup() throws Exception
     {
-        if (cluster == null)
+        StorageService.instance.initServer();
-            cluster = init(builder().withNodes(1).start());
+        token = StorageService.instance.getTokens().get(0);
+        startJMXServer();
     }
 
-    @AfterClass
+    /**
-    public static void teardownEnv() throws Exception
+     * Validate output, making sure the table mappings work with various host-modifying arguments in use.
+     */
+    @Test
+    public void testRingOutput()
     {
-        cluster.close();
+        final HostStatWithPort host = new HostStatWithPort(null, FBUtilities.getBroadcastAddressAndPort(),
+                                                           false, null);
+        validateRingOutput(host.ipOrDns(false),
+                            "ring");
+        Arrays.asList("-pp", "--print-port").forEach(arg -> {
+            validateRingOutput(host.ipOrDns(true),
+                               "-pp", "ring");
+        });
+
+        final HostStatWithPort hostResolved = new HostStatWithPort(null, FBUtilities.getBroadcastAddressAndPort(),
+                                                                   true, null);
+        Arrays.asList("-r", "--resolve-ip").forEach(arg -> {
+            validateRingOutput(hostResolved.ipOrDns(false),
+                               "ring", "-r");
+        });
+        validateRingOutput(hostResolved.ipOrDns(true),
+                            "-pp", "ring", "-r");
+    }
+
+    private void validateRingOutput(String hostForm, String... args)
+    {
+        ToolRunner.ToolResult nodetool = ToolRunner.invokeNodetool(args);
+        nodetool.assertOnCleanExit();
+        /*
+         Datacenter: datacenter1
+         ==========
+         Address         Rack        Status State   Load            Owns                Token
+
+         127.0.0.1       rack1       Up     Normal  45.71 KiB       100.00%             4652409154190094022
+
+         */
+        String[] lines = nodetool.getStdout().split("\\R");
+        assertThat(lines[1].trim(), endsWith(SimpleSnitch.DATA_CENTER_NAME));
+        assertThat(lines[3], matchesPattern("Address *Rack *Status *State *Load *Owns *Token *"));
+        String hostRing = lines[lines.length-4].trim(); // this command has a couple extra newlines and an empty error message at the end. Not messing with it.
+        assertThat(hostRing, startsWith(hostForm));
+        assertThat(hostRing, containsString(SimpleSnitch.RACK_NAME));
+        assertThat(hostRing, containsString("Up"));
+        assertThat(hostRing, containsString("Normal"));
+        assertThat(hostRing, matchesPattern(".*\\d+\\.\\d+ KiB.*"));
+        assertThat(hostRing, matchesPattern(".*\\d+\\.\\d+%.*"));
+        assertThat(hostRing, endsWith(token));
+        assertThat(hostRing, not(containsString("?")));
     }
 
     @Test
     public void testWrongArgFailsAndPrintsHelp()
     {
-        ToolResult tool = ToolRunner.invokeNodetoolJvmDtest(cluster.get(1), "--wrongarg", "ring");
+        ToolRunner.ToolResult tool = ToolRunner.invokeNodetool("--wrongarg", "ring");
         Assertions.assertThat(tool.getStdout()).containsIgnoringCase("nodetool help");
         assertEquals(1, tool.getExitCode());
         assertTrue(tool.getCleanedStderr().isEmpty());
@@ -64,8 +116,8 @@ public class NodeToolRingTest extends TestBaseImpl
     {
         // If you added, modified options or help, please update docs if necessary
 
-        ToolResult tool = ToolRunner.invokeNodetoolJvmDtest(cluster.get(1), "help", "ring");
+        ToolRunner.ToolResult tool = ToolRunner.invokeNodetool("help", "ring");
-        
+
         String help = "NAME\n" + "        nodetool ring - Print information about the token ring\n"
                       + "\n"
                       + "SYNOPSIS\n"
@@ -110,63 +162,18 @@ public class NodeToolRingTest extends TestBaseImpl
         Assertions.assertThat(tool.getStdout()).isEqualTo(help);
     }
 
-    @Test
-    public void testRing()
-    {
-        ToolResult tool = ToolRunner.invokeNodetoolJvmDtest(cluster.get(1), "ring");
-        
-        Assertions.assertThat(tool.getStdout())
-                  .contains("Datacenter: datacenter0")
-                  .contains("Address    Rack        Status State   Load            Owns                Token")
-                  .contains("127.0.0.1  rack0       Up     Normal")
-                  .contains("100.00%             9223372036854775807");
-        assertEquals(0, tool.getExitCode());
-        assertTrue(tool.getCleanedStderr().isEmpty());
-    }
-
-    @Test
-    public void testRingPrintPort()
-    {
-        Arrays.asList("-pp", "--print-port").forEach(arg -> {
-            ToolResult tool = ToolRunner.invokeNodetoolJvmDtest(cluster.get(1), arg, "ring");
-            Assertions.assertThat(tool.getStdout())
-                      .contains("Datacenter: datacenter0")
-                      .contains("Address         Rack        Status State   Load            Owns                Token")
-                      .contains("Unknown")
-                      .contains("100.00%             9223372036854775807");
-            assertEquals(0, tool.getExitCode());
-            assertTrue(tool.getCleanedStderr().isEmpty());
-        });
-    }
-
-    @Test
-    public void testRingResolve()
-    {
-        Arrays.asList("-r", "--resolve-ip").forEach(arg -> {
-            ToolResult tool = ToolRunner.invokeNodetoolJvmDtest(cluster.get(1), "ring", arg);
-            Assertions.assertThat(tool.getStdout())
-                      .contains("Datacenter: datacenter0")
-                      .contains("Address    Rack        Status State   Load            Owns                Token")
-                      .contains("localhost")
-                      .contains("rack0       Up     Normal")
-                      .contains("100.00%             9223372036854775807");
-            assertEquals(0, tool.getExitCode());
-            assertTrue(tool.getCleanedStderr().isEmpty());
-        });
-    }
-
     @Test
     public void testRingKeyspace()
     {
         // Bad KS
-        ToolResult tool = ToolRunner.invokeNodetoolJvmDtest(cluster.get(1), "ring", "mockks");
+        ToolRunner.ToolResult tool = ToolRunner.invokeNodetool("ring", "mockks");
         Assertions.assertThat(tool.getStdout()).contains("The keyspace mockks, does not exist");
         assertEquals(0, tool.getExitCode());
         assertTrue(tool.getCleanedStderr().isEmpty());
 
         // Good KS
-        tool = ToolRunner.invokeNodetoolJvmDtest(cluster.get(1), "ring", "system_schema");
+        tool = ToolRunner.invokeNodetool("ring", "system_schema");
-        Assertions.assertThat(tool.getStdout()).contains("Datacenter: datacenter0");
+        Assertions.assertThat(tool.getStdout()).contains("Datacenter: datacenter1");
         assertEquals(0, tool.getExitCode());
         assertTrue(tool.getCleanedStderr().isEmpty());
     }

test/unit/org/apache/cassandra/tools/nodetool/StatusTest.java

@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.locator.SimpleSnitch;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.utils.FBUtilities;
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.Matchers.matchesPattern;
+import static org.junit.Assert.assertThat;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class StatusTest extends CQLTester
+{
+    private static String localHostId;
+    private static String token;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        localHostId = StorageService.instance.getLocalHostId();
+        token = StorageService.instance.getTokens().get(0);
+        startJMXServer();
+    }
+
+    /**
+     * Validate output, making sure the table mappings work with various host-modifying arguments in use.
+     */
+    @Test
+    public void testStatusOutput()
+    {
+        HostStatWithPort host = new HostStatWithPort(null, FBUtilities.getBroadcastAddressAndPort(), false, null);
+        validateStatusOutput(host.ipOrDns(false),
+                            "status");
+        validateStatusOutput(host.ipOrDns(true),
+                            "-pp", "status");
+        host = new HostStatWithPort(null, FBUtilities.getBroadcastAddressAndPort(), true, null);
+        validateStatusOutput(host.ipOrDns(false),
+                            "status", "-r");
+        validateStatusOutput(host.ipOrDns(true),
+                            "-pp", "status", "-r");
+    }
+
+    private void validateStatusOutput(String hostForm, String... args)
+    {
+        ToolRunner.ToolResult nodetool = ToolRunner.invokeNodetool(args);
+        nodetool.assertOnCleanExit();
+        /*
+         Datacenter: datacenter1
+         =======================
+         Status=Up/Down
+         |/ State=Normal/Leaving/Joining/Moving
+         --  Address    Load       Owns (effective)  Host ID                               Token                Rack
+         UN  localhost  45.71 KiB  100.0%            0b1b5e91-ad3b-444e-9c24-50578486978a  1849950853373272258  rack1
+         */
+        String[] lines = nodetool.getStdout().split("\\R");
+        assertThat(lines[0].trim(), endsWith(SimpleSnitch.DATA_CENTER_NAME));
+        String hostStatus = lines[lines.length-1].trim();
+        assertThat(hostStatus, startsWith("UN"));
+        assertThat(hostStatus, containsString(hostForm));
+        assertThat(hostStatus, matchesPattern(".*\\d+\\.\\d+ KiB.*"));
+        assertThat(hostStatus, matchesPattern(".*\\d+\\.\\d+%.*"));
+        assertThat(hostStatus, containsString(localHostId));
+        assertThat(hostStatus, containsString(token));
+        assertThat(hostStatus, endsWith(SimpleSnitch.RACK_NAME));
+        assertThat(hostStatus, not(containsString("?")));
+    }
+}

test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java

@@ -18,7 +18,6 @@
 
 package org.apache.cassandra.tools.nodetool.stats;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -27,7 +26,6 @@ import java.util.regex.Pattern;
 
 import org.apache.commons.lang3.StringUtils;
 
-import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -35,7 +33,6 @@ import org.junit.runner.RunWith;
 import org.apache.cassandra.OrderedJUnit4ClassRunner;
 import org.apache.cassandra.cql3.CQLTester;
 import org.apache.cassandra.service.StorageService;
-import org.apache.cassandra.tools.NodeProbe;
 import org.apache.cassandra.tools.ToolRunner;
 import org.apache.cassandra.tools.ToolRunner.ToolResult;
 import org.assertj.core.api.Assertions;
@@ -49,20 +46,11 @@ import static org.junit.Assert.assertTrue;
 @RunWith(OrderedJUnit4ClassRunner.class)
 public class NodetoolTableStatsTest extends CQLTester
 {
-    private static NodeProbe probe;
-
     @BeforeClass
     public static void setup() throws Exception
     {
         StorageService.instance.initServer();
         startJMXServer();
-        probe = new NodeProbe(jmxHost, jmxPort);
-    }
-
-    @AfterClass
-    public static void teardown() throws IOException
-    {
-        probe.close();
     }
 
     @Test