From c2425ca1354072ec733a369edd2402b33dadae2e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Botond=20D=C3=A9nes?= <bdenes@scylladb.com>
Date: Fri, 16 Feb 2024 08:19:27 -0500
Subject: [PATCH] tools/scylla-nodetool: add --rest-api-port option

This option is an alternative to --port|-p and takes precedence over it.
This is meant to aid the switch from the legacy nodetool to the native
one. Users of the legacy nodetool pass the port of JMX to --port. We
need a way to provide both the JMX port (via --port) and also the REST
API port, which only the native nodetool will interpret. So we add this
new --rest-api-port, which when provided, overwrites the --port|-p
option. To ensure the legacy nodeotol doesn't try to interpret this,
this option can also be provided as -Dcom.scylladb.apiPort (which is
substituted to --rest-api-port behind the scenes).
---
 test/nodetool/test_nodetool.py | 13 +++++++++++++
 tools/scylla-nodetool.cc       | 16 +++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/test/nodetool/test_nodetool.py b/test/nodetool/test_nodetool.py
index 1285f7b76a..d76c598aa9 100644
--- a/test/nodetool/test_nodetool.py
+++ b/test/nodetool/test_nodetool.py
@@ -97,3 +97,16 @@ def test_jvm_options(nodetool_path, rest_api_mock_server, scylla_only):
     subprocess.run([nodetool_path, "nodetool", "compact", "-h", ip, "-p", port, jvm_opt], check=True)
     subprocess.run([nodetool_path, "nodetool", "compact", "-h", ip, jvm_opt, "-p", port], check=True)
     subprocess.run([nodetool_path, "nodetool", jvm_opt, "compact", "-h", ip, "-p", port], check=True)
+
+
+def test_alternative_api_port(nodetool_path, rest_api_mock_server, scylla_only):
+    set_expected_requests(rest_api_mock_server, [
+        expected_request("POST", "/storage_service/compact", multiple=expected_request.MULTIPLE)])
+
+    ip, port = rest_api_mock_server
+    port = str(port)
+
+    subprocess.run([nodetool_path, "nodetool", "compact", "-h", ip, "-p", "1", "--rest-api-port", port], check=True)
+    subprocess.run([nodetool_path, "nodetool", "compact", "-h", ip, "-p", "1", f"--rest-api-port={port}"], check=True)
+    subprocess.run([nodetool_path, "nodetool", "compact", "-h", ip, "-p", "1", f"-Dcom.scylladb.apiPort={port}"],
+                   check=True)
diff --git a/tools/scylla-nodetool.cc b/tools/scylla-nodetool.cc
index 78eafc0545..0694773de5 100644
--- a/tools/scylla-nodetool.cc
+++ b/tools/scylla-nodetool.cc
@@ -2646,6 +2646,7 @@ void version_operation(scylla_rest_client& client, const bpo::variables_map& vm)
 const std::vector<operation_option> global_options{
     typed_option<sstring>("host,h", "localhost", "the hostname or ip address of the ScyllaDB node"),
     typed_option<uint16_t>("port,p", 10000, "the port of the REST API of the ScyllaDB node"),
+    typed_option<uint16_t>("rest-api-port", "the port of the REST API of the ScyllaDB node; takes precedence over --port|-p"),
     typed_option<sstring>("password", "Remote jmx agent password (unused)"),
     typed_option<sstring>("password-file", "Path to the JMX password file (unused)"),
     typed_option<sstring>("username,u", "Remote jmx agent username (unused)"),
@@ -2654,6 +2655,7 @@ const std::vector<operation_option> global_options{
 
 const std::map<std::string_view, std::string_view> option_substitutions{
     {"-h", "--host"},
+    {"-Dcom.scylladb.apiPort", "--rest-api-port"},
     {"-pw", "--password"},
     {"-pwf", "--password-file"},
     {"-pp", "--print-port"},
@@ -3495,12 +3497,14 @@ std::vector<char*> massage_argv(int argc, char** argv) {
             continue;
         }
 
+        std::string arg = argv[i];
+
         // Java JVM options, they look like -Dkey=value, or just -Dkey
-        if (argv[i][1] == 'D') {
+        // We leave -Dcom.scylladb.apiPort, it will be substituted to --rest-api-port
+        if (argv[i][1] == 'D' && !arg.starts_with("-Dcom.scylladb.apiPort=")) {
             continue;
         }
 
-        std::string arg = argv[i];
         std::string arg_key;
         std::optional<std::string> arg_value;
 
@@ -3573,7 +3577,13 @@ For more information, see: https://opensource.docs.scylladb.com/stable/operating
             if (operation.name() == "help") {
                 help_operation(app.get_config(), app_config);
             } else {
-                scylla_rest_client client(app_config["host"].as<sstring>(), app_config["port"].as<uint16_t>());
+                uint16_t port{};
+                if (app_config.count("rest-api-port")) {
+                    port = app_config["rest-api-port"].as<uint16_t>();
+                } else {
+                    port = app_config["port"].as<uint16_t>();
+                }
+                scylla_rest_client client(app_config["host"].as<sstring>(), port);
                 get_operations_with_func().at(operation)(client, app_config);
             }
         } catch (std::invalid_argument& e) {