From f6281619aef740d6e57ffc3b5e4183bf83651e9a Mon Sep 17 00:00:00 2001 From: Bereng Date: Mon, 19 Oct 2020 07:27:59 +0200 Subject: [PATCH] Nodetool ring testing Patch by Berenguer Blasi, reviewed by brandonwilliams for CASSANDRA-16200 --- build.xml | 16 +- .../apache/cassandra/tools/nodetool/Ring.java | 4 +- .../distributed/test/NodeToolRingTest.java | 172 ++++++++++++++++++ .../apache/cassandra/tools/ToolRunner.java | 89 ++++++++- 4 files changed, 265 insertions(+), 16 deletions(-) create mode 100644 test/distributed/org/apache/cassandra/distributed/test/NodeToolRingTest.java diff --git a/build.xml b/build.xml index 4f7d217ef4..44185341cf 100644 --- a/build.xml +++ b/build.xml @@ -18,7 +18,9 @@ ~ under the License. --> + xmlns:artifact="antlib:org.apache.maven.artifact.ant" + xmlns:if="ant:if" + xmlns:unless="ant:unless"> @@ -1610,7 +1612,8 @@ --> - + + @@ -1624,7 +1627,8 @@ --> - + + @@ -1635,7 +1639,8 @@ --> - + + @@ -2127,7 +2132,8 @@ --> - + + diff --git a/src/java/org/apache/cassandra/tools/nodetool/Ring.java b/src/java/org/apache/cassandra/tools/nodetool/Ring.java index 8b23717bfa..134c85df87 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/Ring.java +++ b/src/java/org/apache/cassandra/tools/nodetool/Ring.java @@ -244,7 +244,9 @@ public class Ring extends NodeToolCmd for (HostStatWithPort stat : hoststats) { - tokens.addAll(endpointsToTokens.get(stat.endpoint.toString())); + // Remove extra '/' from address + String addressNPort = stat.endpointWithPort.toString().replaceAll("^/", ""); + tokens.addAll(endpointsToTokens.get(addressNPort)); lastToken = tokens.get(tokens.size() - 1); } diff --git a/test/distributed/org/apache/cassandra/distributed/test/NodeToolRingTest.java b/test/distributed/org/apache/cassandra/distributed/test/NodeToolRingTest.java new file mode 100644 index 0000000000..008dd6b78f --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/NodeToolRingTest.java @@ -0,0 +1,172 @@ +/* + * 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.distributed.test; + +import java.io.IOException; +import java.util.Arrays; + +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.Test; + +import org.apache.cassandra.distributed.api.ICluster; +import org.apache.cassandra.tools.ToolRunner; +import org.apache.cassandra.tools.ToolRunner.ToolResult; +import org.assertj.core.api.Assertions; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class NodeToolRingTest extends TestBaseImpl +{ + private static ICluster cluster; + + @Before + public void setupEnv() throws IOException + { + if (cluster == null) + cluster = init(builder().withNodes(1).start()); + } + + @AfterClass + public static void teardownEnv() throws Exception + { + cluster.close(); + } + + @Test + public void testWrongArgFailsAndPrintsHelp() + { + ToolResult tool = ToolRunner.invokeNodetoolJvmDtest(cluster.get(1), "--wrongarg", "ring"); + Assertions.assertThat(tool.getStdout()).containsIgnoringCase("nodetool help"); + assertEquals(1, tool.getExitCode()); + assertTrue(tool.getCleanedStderr().isEmpty()); + } + + @Test + public void testMaybeChangeDocs() + { + // If you added, modified options or help, please update docs if necessary + + ToolResult tool = ToolRunner.invokeNodetoolJvmDtest(cluster.get(1), "help", "ring"); + + String help = "NAME\n" + " nodetool ring - Print information about the token ring\n" + + "\n" + + "SYNOPSIS\n" + + " nodetool [(-h | --host )] [(-p | --port )]\n" + + " [(-pp | --print-port)] [(-pw | --password )]\n" + + " [(-pwf | --password-file )]\n" + + " [(-u | --username )] ring [(-r | --resolve-ip)]\n" + + " [--] []\n" + + "\n" + + "OPTIONS\n" + + " -h , --host \n" + + " Node hostname or ip address\n" + + "\n" + + " -p , --port \n" + + " Remote jmx agent port number\n" + + "\n" + + " -pp, --print-port\n" + + " Operate in 4.0 mode with hosts disambiguated by port number\n" + + "\n" + + " -pw , --password \n" + + " Remote jmx agent password\n" + + "\n" + + " -pwf , --password-file \n" + + " Path to the JMX password file\n" + + "\n" + + " -r, --resolve-ip\n" + + " Show node domain names instead of IPs\n" + + "\n" + + " -u , --username \n" + + " Remote jmx agent username\n" + + "\n" + + " --\n" + + " This option can be used to separate command-line options from the\n" + + " list of argument, (useful when arguments might be mistaken for\n" + + " command-line options\n" + + "\n" + + " \n" + + " Specify a keyspace for accurate ownership information (topology\n" + + " awareness)\n" + + "\n" + + "\n"; + 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 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"); + 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"); + Assertions.assertThat(tool.getStdout()).contains("Datacenter: datacenter0"); + assertEquals(0, tool.getExitCode()); + assertTrue(tool.getCleanedStderr().isEmpty()); + } +} diff --git a/test/unit/org/apache/cassandra/tools/ToolRunner.java b/test/unit/org/apache/cassandra/tools/ToolRunner.java index 98fdaeda77..04c68fbe38 100644 --- a/test/unit/org/apache/cassandra/tools/ToolRunner.java +++ b/test/unit/org/apache/cassandra/tools/ToolRunner.java @@ -33,6 +33,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.concurrent.CompletableFuture; +import java.util.function.Supplier; import java.util.stream.Collectors; import com.google.common.base.Throwables; @@ -42,7 +43,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.cassandra.cql3.CQLTester; +import org.apache.cassandra.distributed.api.IInstance; +import org.apache.cassandra.distributed.api.NodeToolResult; import org.apache.cassandra.tools.OfflineToolUtils.SystemExitException; +import org.apache.cassandra.utils.Pair; +import org.assertj.core.util.Lists; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -238,6 +243,52 @@ public class ToolRunner allArgs.add(klass); allArgs.addAll(Arrays.asList(args)); + Supplier runMe = new Supplier() + { + @Override + public Integer get() + { + return runClassAsTool(klass, args); + } + }; + + Pair res = invokeSupplier(runMe, stdin); + return new ToolResult(allArgs, + res.right.getExitCode() == -1 ? -1 : res.left, + res.right.getStdout(), + res.right.getStderr(), + res.right.getException()); + + } + + public static ToolResult invokeNodetoolJvmDtest(IInstance node, String... args) + { + Supplier runMe = new Supplier() + { + @Override + public NodeToolResult get() + { + return node.nodetoolResult(args); + } + }; + + Pair res = invokeSupplier(runMe); + // Some jvm dtest nodetool commands capture stdout/err in the NodeToolResult, some don't. So we have to concat both. + return new ToolResult(Arrays.asList(args), + res.left, + res.right.getExitCode() == -1 ? -1 : res.left.getRc(), + res.right.getStdout() + res.left.getStdout(), + res.right.getStderr() + res.left.getStderr(), + res.right.getException()); + } + + public static Pair invokeSupplier(Supplier runMe) + { + return invokeSupplier(runMe, null); + } + + public static Pair invokeSupplier(Supplier runMe, InputStream stdin) + { PrintStream originalSysOut = System.out; PrintStream originalSysErr = System.err; InputStream originalSysIn = System.in; @@ -248,23 +299,24 @@ public class ToolRunner System.setIn(stdin == null ? originalSysIn : stdin); - try (PrintStream newOut = new PrintStream(out); - PrintStream newErr = new PrintStream(err)) + T res = null; + try(PrintStream newOut = new PrintStream(out); PrintStream newErr = new PrintStream(err)) { System.setOut(newOut); System.setErr(newErr); - int rc = runClassAsTool(klass, args); + res = runMe.get(); out.flush(); err.flush(); - return new ToolResult(allArgs, rc, out.toString(), err.toString(), null); + return Pair.create(res, new ToolResult(Lists.emptyList(), 0, out.toString(), err.toString(), null)); } - catch (Exception e) + catch(Exception e) { - return new ToolResult(allArgs, - -1, - out.toString(), - err.toString() + "\n" + Throwables.getStackTraceAsString(e), - e); + return Pair.create(res, + new ToolResult(Lists.emptyList(), + -1, + out.toString(), + err.toString() + "\n" + Throwables.getStackTraceAsString(e), + e)); } finally { @@ -286,6 +338,7 @@ public class ToolRunner private final String stdout; private final String stderr; private final Exception e; + private final NodeToolResult ntRes; private ToolResult(List allArgs, int exitCode, String stdout, String stderr, Exception e) { @@ -294,6 +347,22 @@ public class ToolRunner this.stdout = stdout; this.stderr = stderr; this.e = e; + this.ntRes = null; + } + + private ToolResult(List allArgs, NodeToolResult ntRes, int exitCode, String stdout, String stderr, Exception e) + { + this.allArgs = allArgs; + this.exitCode = exitCode; + this.stdout = stdout; + this.stderr = stderr; + this.e = e; + this.ntRes = ntRes; + } + + public NodeToolResult getNodeToolResult() + { + return ntRes; } public int getExitCode()