This is an automated email from the ASF dual-hosted git repository. dcapwell pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push: new 88b18a6 jvm-dtest nodetool doesn't handle System.exit 88b18a6 is described below commit 88b18a603c0affa05679b11c2e7d91324fa8f719 Author: David Capwell <dcapw...@apache.org> AuthorDate: Thu Nov 12 15:39:51 2020 -0800 jvm-dtest nodetool doesn't handle System.exit patch by David Capwell; reviewed by Jordan West, Yifan Cai for CASSANDRA-16270 --- .../apache/cassandra/tools/nodetool/Verify.java | 1 + .../cassandra/distributed/impl/Instance.java | 36 +++++++++++++-- .../cassandra/distributed/test/NodeToolTest.java | 51 ++++++++++++++++------ .../apache/cassandra/tools/OfflineToolUtils.java | 10 ----- .../cassandra/tools/SystemExitException.java | 29 ++++++++++++ .../org/apache/cassandra/tools/ToolRunner.java | 2 - 6 files changed, 100 insertions(+), 29 deletions(-) diff --git a/src/java/org/apache/cassandra/tools/nodetool/Verify.java b/src/java/org/apache/cassandra/tools/nodetool/Verify.java index d2ae151..872a124 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/Verify.java +++ b/src/java/org/apache/cassandra/tools/nodetool/Verify.java @@ -74,6 +74,7 @@ public class Verify extends NodeToolCmd if (checkOwnsTokens && !extendedVerify) { out.println("Token verification requires --extended-verify"); + // if System.exit gets removed, make sure to update org.apache.cassandra.distributed.test.NodeToolTest.testNodetoolSystemExit System.exit(1); } diff --git a/test/distributed/org/apache/cassandra/distributed/impl/Instance.java b/test/distributed/org/apache/cassandra/distributed/impl/Instance.java index b7d97f7..7ccb59b 100644 --- a/test/distributed/org/apache/cassandra/distributed/impl/Instance.java +++ b/test/distributed/org/apache/cassandra/distributed/impl/Instance.java @@ -24,6 +24,7 @@ import java.io.File; import java.io.IOException; import java.io.PrintStream; import java.net.InetSocketAddress; +import java.security.Permission; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -78,7 +79,6 @@ import org.apache.cassandra.distributed.api.NodeToolResult; import org.apache.cassandra.distributed.api.SimpleQueryResult; import org.apache.cassandra.distributed.mock.nodetool.InternalNodeProbe; import org.apache.cassandra.distributed.mock.nodetool.InternalNodeProbeFactory; -import org.apache.cassandra.exceptions.StartupException; import org.apache.cassandra.gms.Gossiper; import org.apache.cassandra.hints.HintsService; import org.apache.cassandra.index.SecondaryIndexManager; @@ -102,7 +102,6 @@ import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.DefaultFSErrorHandler; import org.apache.cassandra.service.PendingRangeCalculatorService; import org.apache.cassandra.service.QueryState; -import org.apache.cassandra.service.StartupChecks; import org.apache.cassandra.service.StorageService; import org.apache.cassandra.service.StorageServiceMBean; import org.apache.cassandra.streaming.StreamReceiveTask; @@ -110,6 +109,7 @@ import org.apache.cassandra.streaming.StreamTransferTask; import org.apache.cassandra.streaming.async.StreamingInboundHandler; import org.apache.cassandra.tools.NodeTool; import org.apache.cassandra.tools.Output; +import org.apache.cassandra.tools.SystemExitException; import org.apache.cassandra.tracing.TraceState; import org.apache.cassandra.tracing.Tracing; import org.apache.cassandra.transport.messages.ResultMessage; @@ -607,7 +607,35 @@ public class Instance extends IsolatedExecutor implements IInvokableInstance try (CapturingOutput output = new CapturingOutput()) { DTestNodeTool nodetool = new DTestNodeTool(withNotifications, output.delegate); - int rc = nodetool.execute(commandAndArgs); + // install security manager to get informed about the exit-code + System.setSecurityManager(new SecurityManager() + { + public void checkExit(int status) + { + throw new SystemExitException(status); + } + + public void checkPermission(Permission perm) + { + } + + public void checkPermission(Permission perm, Object context) + { + } + }); + int rc; + try + { + rc = nodetool.execute(commandAndArgs); + } + catch (SystemExitException e) + { + rc = e.status; + } + finally + { + System.setSecurityManager(null); + } return new NodeToolResult(commandAndArgs, rc, new ArrayList<>(nodetool.notifications.notifications), nodetool.latestError, @@ -705,6 +733,8 @@ public class Instance extends IsolatedExecutor implements IInvokableInstance protected void err(Throwable e) { + if (e instanceof SystemExitException) + return; super.err(e); latestError = e; } diff --git a/test/distributed/org/apache/cassandra/distributed/test/NodeToolTest.java b/test/distributed/org/apache/cassandra/distributed/test/NodeToolTest.java index adafe2b..248d824 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/NodeToolTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/NodeToolTest.java @@ -18,35 +18,58 @@ package org.apache.cassandra.distributed.test; +import java.io.IOException; + +import org.junit.AfterClass; +import org.junit.BeforeClass; import org.junit.Test; -import org.apache.cassandra.distributed.api.ICluster; +import org.apache.cassandra.distributed.Cluster; import org.apache.cassandra.distributed.api.NodeToolResult; import static org.junit.Assert.assertEquals; public class NodeToolTest extends TestBaseImpl { + private static Cluster CLUSTER; + + @BeforeClass + public static void before() throws IOException + { + CLUSTER = init(Cluster.build().withNodes(1).start()); + } + + @AfterClass + public static void after() + { + if (CLUSTER != null) + CLUSTER.close(); + } + @Test public void testCommands() throws Throwable { - try (ICluster cluster = init(builder().withNodes(1).start())) - { - assertEquals(0, cluster.get(1).nodetool("help")); - assertEquals(0, cluster.get(1).nodetool("flush")); - assertEquals(1, cluster.get(1).nodetool("not_a_legal_command")); - } + assertEquals(0, CLUSTER.get(1).nodetool("help")); + assertEquals(0, CLUSTER.get(1).nodetool("flush")); + assertEquals(1, CLUSTER.get(1).nodetool("not_a_legal_command")); } @Test public void testCaptureConsoleOutput() throws Throwable { - try (ICluster cluster = init(builder().withNodes(1).start())) - { - NodeToolResult ringResult = cluster.get(1).nodetoolResult("ring"); - ringResult.asserts().stdoutContains("Datacenter: datacenter0"); - ringResult.asserts().stdoutContains("127.0.0.1 rack0 Up Normal"); - assertEquals("Non-empty error output", "", ringResult.getStderr()); - } + NodeToolResult ringResult = CLUSTER.get(1).nodetoolResult("ring"); + ringResult.asserts().stdoutContains("Datacenter: datacenter0"); + ringResult.asserts().stdoutContains("127.0.0.1 rack0 Up Normal"); + assertEquals("Non-empty error output", "", ringResult.getStderr()); + } + + @Test + public void testNodetoolSystemExit() + { + // Verify currently calls System.exit, this test uses that knowlege to test System.exit behavior in jvm-dtest + CLUSTER.get(1).nodetoolResult("verify", "--check-tokens") + .asserts() + .failure() + .stdoutContains("Token verification requires --extended-verify"); } } diff --git a/test/unit/org/apache/cassandra/tools/OfflineToolUtils.java b/test/unit/org/apache/cassandra/tools/OfflineToolUtils.java index ae82fd8..bcb7020 100644 --- a/test/unit/org/apache/cassandra/tools/OfflineToolUtils.java +++ b/test/unit/org/apache/cassandra/tools/OfflineToolUtils.java @@ -203,16 +203,6 @@ public abstract class OfflineToolUtils initialThreads = Arrays.asList(threads.getThreadInfo(threads.getAllThreadIds())); } - public static class SystemExitException extends Error - { - public final int status; - - public SystemExitException(int status) - { - this.status = status; - } - } - public static String findOneSSTable(String ks, String cf) throws IOException { File cfDir = sstableDir(ks, cf); diff --git a/test/unit/org/apache/cassandra/tools/SystemExitException.java b/test/unit/org/apache/cassandra/tools/SystemExitException.java new file mode 100644 index 0000000..0d92746 --- /dev/null +++ b/test/unit/org/apache/cassandra/tools/SystemExitException.java @@ -0,0 +1,29 @@ +/* + * 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; + +public class SystemExitException extends Error +{ + public final int status; + + public SystemExitException(int status) + { + this.status = status; + } +} diff --git a/test/unit/org/apache/cassandra/tools/ToolRunner.java b/test/unit/org/apache/cassandra/tools/ToolRunner.java index 04c68fb..6b0be58 100644 --- a/test/unit/org/apache/cassandra/tools/ToolRunner.java +++ b/test/unit/org/apache/cassandra/tools/ToolRunner.java @@ -32,7 +32,6 @@ import java.util.HashMap; 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; @@ -45,7 +44,6 @@ 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; --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org