So tests and fixtures can use `with expected_requests():` and have
cleanup be taken care for them. I just discovered that some tests do not
clean up after themselves and when running all tests in a certain order,
this causes unrelated tests to fail.
Fix by using the context everywhere, getting guaranteed cleanup after
each test.
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).
Legacy scripts and tests for nodetool, might pass JVM args like
-Dcom.sun.jndi.rmiURLParsing=legacy. Ignore these, by dropping anything
that starts with -D from the command line args.
Currently all scylla-tools assume that the operation/command is in
argv[1]. This is not very flexible, because most programs allow global
options (that are not dependent on the current operation/command) to be
passed before the operation name on the command line. Notably C*'s
nodetool is one such program and indeed scripts and tests using nodetool
do utilize this.
This patch makes this more flexible. Instead of looking at argv[1], do
an initial option parsing with boost::program_options to locate the
operation parameter. This initial parser knows about the global options,
and the operation positional argument. It allows for unrecognized
positional and non-positional arguments, but only after the command.
With this, any combination of global options + operation is allowed, in
any order.
When a tool application is invoked with an unknown operation, an error
message is printed, which includes all the known operations, with all
their aliases. This is collected in `std::vector<std::string_view>`. The
problem is that the vector containing alias names, is returned as a
value, so the code ends up creating views to temporaries.
Fix this by returning alias vector with const&.
Fixes: #17584Closesscylladb/scylladb#17586
Currently, error handling is done via catching
http::unexpected_status_error and re-throwing an std::runtime_error.
Turns out this no longer works, because this error will only be thrown
by the http client, if the request had an expected reply code set.
The scylla_rest_client doesn't set an expected reply code, so this
exception was never thrown for some time now.
Furthermore, even when the above worked, it was not too user-friendly as
the error message only included the reply-code, but not the reply
itself.
So in this patch this is fixed:
* The handling of http::unexpected_status_error is removed, we don't
want to use this mechanism, because it yields very terse error
messages.
* Instead, the status code of the request is checked explicitely and all
cases where it is not 200 are handled.
* A new api_request_failed exception is added, which is throw for all
non-200 statuses with the extracted error message from the server (if
any).
* This exception is caught by main, the error message is printed and
scylla-nodetool returns with a new distinct error-code: 4.
With this, all cases where the request fails on ScyllaDB are handled and
we shouldn't hit cases where a nodetool command fails with some
obscure JSON parsing error, because the error reply has different JSON
schema than the expected happy-path reply.
This was recently found to produce a crash. Add a simple regression
test, to make sure future changes don't re-introduce problems with this
rarely used code-path.
Testing the new scylla nodetool tool.
The tests can be run aginst both implementations of nodetool: the
scylla-native one and the cassandra one. They all pass with both
implementations.
