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

Reply via email to