Merge branch 'cassandra-6.0' into trunk

Author: smiklosovic

CHANGES.txt

@@ -4,6 +4,7 @@
  * Allow nodetool garbagecollect to take a user defined list of SSTables (CASSANDRA-16767)
  * Add a guardrail for misprepared statements (CASSANDRA-21139)
 Merged from 6.0:
+ * Offline nodetool commands should not print network options in help (CASSANDRA-20876)
  * Defer creation of the system_cluster_metadata keyspace until CMS initialization (CASSANDRA-21477)
  * Support direct I/O for background SSTable writes (CASSANDRA-21134)
  * Relax assertion on partitioner instances in SinglePartitionReadCommand (CASSANDRA-21251)

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

@@ -30,7 +30,7 @@
 import picocli.CommandLine.Option;
 
 @Command(name = "history", description = "Print previously executed nodetool commands")
-public class History extends AbstractCommand
+public class History extends AbstractCommand implements LocalCommand
 {
     @Option(paramLabel = "commands",
     names = { "-n", "--num", "--number-of-commands" },

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

@@ -0,0 +1,34 @@
+/*
+ * 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;
+
+/**
+ * Marker interface for nodetool commands that are known to run entirely locally,
+ * without requiring a JMX connection to the node.
+ * <p>
+ * Commands that implement this interface will not display JMX connection options
+ * (host, port, username, password, etc.) in their help output, since those options
+ * are not applicable to local commands.
+ * <p>
+ * Note: this is different from commands whose locality is determined at runtime
+ * (e.g. {@link Sjk}), which override {@link AbstractCommand#shouldConnect()} instead.
+ */
+public interface LocalCommand
+{
+}

src/java/org/apache/cassandra/tools/nodetool/layout/CassandraCliHelpLayout.java

@@ -35,6 +35,7 @@
 
 import org.apache.cassandra.tools.nodetool.CommandUtils;
 import org.apache.cassandra.tools.nodetool.JmxConnect;
+import org.apache.cassandra.tools.nodetool.LocalCommand;
 import org.apache.cassandra.tools.nodetool.NodetoolCommand;
 import org.apache.cassandra.utils.Pair;
 
@@ -65,9 +66,11 @@
  * Help factory for the Cassandra nodetool to generate the help output in the format
  * of the airline help output, which is used as the default layout for the Cassandra nodetool.
  * <p>
- * Note, that JMX connect options are always shown in the help output and are not hidden. The
- * {@link JmxConnect} class is used to connect to a C* node via JMX, but the opttions are not
- * part of the command hierarchy to allow reusage of the commands in other contexts.
+ * JMX connect options (host, port, username, password, etc.) are shown in the help output for
+ * all commands that require a JMX connection. Commands that implement {@link org.apache.cassandra.tools.nodetool.LocalCommand}
+ * are known at the metadata level to run locally, and their help output suppresses the JMX options.
+ * Commands whose locality is only determined at runtime (e.g. {@link org.apache.cassandra.tools.nodetool.Sjk})
+ * continue to show JMX options since the decision cannot be made statically.
  */
 public class CassandraCliHelpLayout extends CommandLine.Help
 {
@@ -257,6 +260,10 @@ private static List<CommandLine.Model.OptionSpec> parentCommandOptionsWithJmxOpt
         if (commandSpec.helpCommand())
             return Collections.emptyList();
 
+        // If the command is a LocalCommand, it runs locally and does not need JMX connection options
+        // in its help output. This is a static, metadata-level check based on the class hierarchy.
+        boolean shouldShowJmx = !(commandSpec.userObject() instanceof LocalCommand);
+
         List<CommandLine.Model.CommandSpec> hierarhy = new LinkedList<>();
         CommandLine.Model.CommandSpec curr;
         CommandLine.Model.CommandSpec command = commandSpec;
@@ -271,11 +278,16 @@ private static List<CommandLine.Model.OptionSpec> parentCommandOptionsWithJmxOpt
         {
             for (CommandLine.Model.OptionSpec option : spec.options())
             {
-                // JmxConnect and NodetoolCommand options are always shown in the help output for backwards compatibility.
+                // JmxConnect options are shown for online commands only; NodetoolCommand options are always shown for backwards compatibility.
                 if (option.userObject() instanceof Field &&
                     (((Field) option.userObject()).getDeclaringClass().equals(JmxConnect.class) ||
                      ((Field) option.userObject()).getDeclaringClass().equals(NodetoolCommand.class)))
+                {
+                    if (((Field) option.userObject()).getDeclaringClass().equals(JmxConnect.class) && !shouldShowJmx)
+                        continue;
+
                     options.add(option);
+                }
                 else
                 {
                     if (option.hidden() || option.scopeType() == CommandLine.ScopeType.LOCAL)

test/resources/nodetool/help/history

@@ -2,27 +2,8 @@ NAME
         nodetool history - Print previously executed nodetool commands
 
 SYNOPSIS
-        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]
-                [(-pw <password> | --password <password>)]
-                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]
-                [(-u <username> | --username <username>)] history
-                [(-n <commands> | --number-of-commands <commands>)]
+        nodetool history [(-n <commands> | --number-of-commands <commands>)]
 
 OPTIONS
-        -h <host>, --host <host>
-            Node hostname or ip address
-
         -n <commands>, --num <commands>, --number-of-commands <commands>
             Number of commands to print, defaults to 1000.
-
-        -p <port>, --port <port>
-            Remote jmx agent port number
-
-        -pw <password>, --password <password>
-            Remote jmx agent password
-
-        -pwf <passwordFilePath>, --password-file <passwordFilePath>
-            Path to the JMX password file
-
-        -u <username>, --username <username>
-            Remote jmx agent username

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

@@ -113,6 +113,36 @@ public void testNodetoolCommandsMustBeSelfContained()
                    affected.isEmpty());
     }
 
+    /**
+     * Ensures that all commands implementing {@code LocalCommand} do not establish a JMX connection.
+     */
+    @Test
+    public void testLocalCommandsShouldNotConnect() throws Exception
+    {
+        CommandLine root = NodeTool.createCommandLine(CommandLine.defaultFactory());
+        List<String> failedCommands = new ArrayList<>();
+
+        commandTreeWalker(root, cmd -> {
+            Object userObject = cmd.getCommandSpec().userObject();
+            if (userObject instanceof LocalCommand && userObject instanceof AbstractCommand)
+            {
+                AbstractCommand abstractCommand = (AbstractCommand) userObject;
+                try
+                {
+                    if (abstractCommand.shouldConnect())
+                        failedCommands.add(fullCommandName(cmd));
+                }
+                catch (Exception e)
+                {
+                    failedCommands.add(fullCommandName(cmd) + " (" + e.getMessage() + ")");
+                }
+            }
+        });
+
+        assertTrue("The following commands implement LocalCommand but shouldConnect() does not return false: " + failedCommands,
+                   failedCommands.isEmpty());
+    }
+
     /**
      * For a given command, follows the {@code @ParentCommand} chain and collects all
      * options and parameters declared on each parent.