Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-16800: Move towards using the root of Solr as the -solrUrl in the CLI. #1808

Merged
merged 27 commits into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9c6bb46
add bats test
epugh Jul 25, 2023
48adb63
AssertionFailureException is only used by the AssertTool, not shared …
epugh Jul 25, 2023
42279c4
Add testing for resolving a solr url.
epugh Jul 25, 2023
cd63cca
simplify test
epugh Jul 25, 2023
ade359a
change up some of the urls.
epugh Jul 25, 2023
e5513ea
changes to not need /solr, but allow it...
epugh Jul 25, 2023
174b815
reuse existing options..
epugh Jul 25, 2023
f9de312
Time to change the default ;-)
epugh Jul 25, 2023
5216d50
follow pattern of other tools...
epugh Jul 25, 2023
4da518f
be able to handle urls with and without the /solr
epugh Jul 25, 2023
695121f
and update the test!
epugh Jul 25, 2023
1f82eb9
lint
epugh Jul 27, 2023
7294d92
better informational message
epugh Jul 27, 2023
d65936c
more lint
epugh Jul 27, 2023
fcae5a6
Disable the variability of the hostContext to get SolrCloudExampleTes…
epugh Jul 27, 2023
4982fe5
Merge remote-tracking branch 'upstream/main' into SOLR-16800
epugh Aug 6, 2023
d46aedd
update asserts to match new warning message
epugh Aug 6, 2023
b0194ec
descriptions are sentences
epugh Aug 6, 2023
63253b2
Now that solrUrl dont require a /solr, we can simplify some logic
epugh Aug 6, 2023
449cc46
convert more to using short url
epugh Aug 6, 2023
aaa00b6
fix pathing issues
epugh Aug 6, 2023
e7a960c
Responding to PR feedback
epugh Aug 8, 2023
774e453
Merge remote-tracking branch 'upstream/main' into SOLR-16800
epugh Aug 8, 2023
8f3290b
Eliminate some nesting to make code simpler to read.
epugh Aug 8, 2023
8d2e695
Let's hide this solrUrl better from end users.
epugh Aug 8, 2023
e55d338
update missing parameters in commands.
epugh Aug 8, 2023
87ccab1
track change
epugh Aug 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions solr/core/src/java/org/apache/solr/cli/AssertTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.apache.commons.cli.Option;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.Http2SolrClient;
import org.apache.solr.client.solrj.request.HealthCheckRequest;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.NamedList;
Expand Down Expand Up @@ -198,10 +197,10 @@ protected int runAssert(CommandLine cli) throws Exception {
ret += assertSolrNotRunning(cli.getOptionValue("S"));
}
if (cli.hasOption("c")) {
ret += assertSolrRunningInCloudMode(cli.getOptionValue("c"));
ret += assertSolrRunningInCloudMode(SolrCLI.resolveSolrUrl(cli.getOptionValue("c")));
}
if (cli.hasOption("C")) {
ret += assertSolrNotRunningInCloudMode(cli.getOptionValue("C"));
ret += assertSolrNotRunningInCloudMode(SolrCLI.resolveSolrUrl(cli.getOptionValue("C")));
}
return ret;
}
Expand Down Expand Up @@ -348,11 +347,11 @@ public static String userForDir(Path pathToDir) {
}
}

private static int exitOrException(String msg) throws SolrCLI.AssertionFailureException {
private static int exitOrException(String msg) throws AssertionFailureException {
if (useExitCode) {
return 1;
} else {
throw new SolrCLI.AssertionFailureException(message != null ? message : msg);
throw new AssertionFailureException(message != null ? message : msg);
}
}

Expand All @@ -370,8 +369,14 @@ private static boolean isSolrRunningOn(String url) throws Exception {
}

private static boolean runningSolrIsCloud(String url) throws Exception {
try (final SolrClient client = new Http2SolrClient.Builder(url).build()) {
try (final SolrClient client = SolrCLI.getSolrClient(url)) {
return SolrCLI.isCloudMode(client);
}
}

public static class AssertionFailureException extends Exception {
public AssertionFailureException(String message) {
super(message);
}
}
}
8 changes: 2 additions & 6 deletions solr/core/src/java/org/apache/solr/cli/AuthTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,8 @@ public List<Option> getOptions() {
.desc(
"This is where any authentication related configuration files, if any, would be placed.")
.build(),
Option.builder("solrUrl").argName("solrUrl").hasArg().desc("Solr URL.").build(),
Option.builder("zkHost")
.argName("zkHost")
.hasArg()
.desc("ZooKeeper host to connect to.")
.build(),
SolrCLI.OPTION_SOLRURL,
SolrCLI.OPTION_ZKHOST,
SolrCLI.OPTION_VERBOSE);
}

Expand Down
3 changes: 1 addition & 2 deletions solr/core/src/java/org/apache/solr/cli/ConfigTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.Option;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.impl.Http2SolrClient;
import org.apache.solr.common.util.NamedList;
import org.noggit.CharArr;
import org.noggit.JSONWriter;
Expand Down Expand Up @@ -109,7 +108,7 @@ public void runImpl(CommandLine cli) throws Exception {
echo("\nPOSTing request to Config API: " + solrUrl + updatePath);
echo(jsonBody);

try (SolrClient solrClient = new Http2SolrClient.Builder(solrUrl).build()) {
try (SolrClient solrClient = SolrCLI.getSolrClient(solrUrl)) {
NamedList<Object> result = SolrCLI.postJsonToSolr(solrClient, updatePath, jsonBody);
Integer statusCode = (Integer) result.findRecursive("responseHeader", "status");
if (statusCode == 0) {
Expand Down
2 changes: 1 addition & 1 deletion solr/core/src/java/org/apache/solr/cli/CreateTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public List<Option> getOptions() {
@Override
public void runImpl(CommandLine cli) throws Exception {
SolrCLI.raiseLogLevelUnlessVerbose(cli);
String solrUrl = cli.getOptionValue("solrUrl", SolrCLI.DEFAULT_SOLR_URL);
String solrUrl = SolrCLI.resolveSolrUrl(cli);

try (var solrClient = SolrCLI.getSolrClient(solrUrl)) {
if (SolrCLI.isCloudMode(solrClient)) {
Expand Down
2 changes: 1 addition & 1 deletion solr/core/src/java/org/apache/solr/cli/DeleteTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public List<Option> getOptions() {
@Override
public void runImpl(CommandLine cli) throws Exception {
SolrCLI.raiseLogLevelUnlessVerbose(cli);
String solrUrl = cli.getOptionValue("solrUrl", SolrCLI.DEFAULT_SOLR_URL);
String solrUrl = SolrCLI.resolveSolrUrl(cli);

try (var solrClient = SolrCLI.getSolrClient(solrUrl)) {
if (SolrCLI.isCloudMode(solrClient)) {
Expand Down
69 changes: 33 additions & 36 deletions solr/core/src/java/org/apache/solr/cli/ExportTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,39 @@ public String getName() {

@Override
public List<Option> getOptions() {
return OPTIONS;
return List.of(
Option.builder("url")
.hasArg()
.required()
.desc("Address of the collection, example http://localhost:8983/solr/gettingstarted.")
.build(),
Option.builder("out")
.hasArg()
.required(false)
.desc(
"Path to output the exported data, and optionally the file name, defaults to 'collection-name'.")
.build(),
Option.builder("format")
.hasArg()
.required(false)
.desc("Output format for exported docs (json, jsonl or javabin), defaulting to json.")
.build(),
Option.builder("compress").required(false).desc("Compress the output.").build(),
Option.builder("limit")
.hasArg()
.required(false)
.desc("Maximum number of docs to download. Default is 100, use -1 for all docs.")
.build(),
Option.builder("query")
.hasArg()
.required(false)
.desc("A custom query, default is '*:*'.")
.build(),
Option.builder("fields")
.hasArg()
.required(false)
.desc("Comma separated list of fields to export. By default all fields are fetched.")
.build());
}

public abstract static class Info {
Expand Down Expand Up @@ -230,41 +262,6 @@ void accept(SolrDocument document) throws IOException {
void end() throws IOException {}
}

private static final List<Option> OPTIONS =
List.of(
Option.builder("url")
.hasArg()
.required()
.desc("Address of the collection, example http://localhost:8983/solr/gettingstarted.")
.build(),
Option.builder("out")
.hasArg()
.required(false)
.desc(
"Path to output the exported data, and optionally the file name, defaults to 'collection-name'.")
.build(),
Option.builder("format")
.hasArg()
.required(false)
.desc("Output format for exported docs (json, jsonl or javabin), defaulting to json.")
.build(),
Option.builder("compress").required(false).desc("Compress the output.").build(),
Option.builder("limit")
.hasArg()
.required(false)
.desc("Maximum number of docs to download. Default is 100, use -1 for all docs.")
.build(),
Option.builder("query")
.hasArg()
.required(false)
.desc("A custom query, default is '*:*'.")
.build(),
Option.builder("fields")
.hasArg()
.required(false)
.desc("Comma separated list of fields to export. By default all fields are fetched.")
.build());

static class JsonWithLinesSink extends DocsSink {
private final CharArr charArr = new CharArr(1024 * 2);
JSONWriter jsonWriter = new JSONWriter(charArr, -1);
Expand Down
23 changes: 5 additions & 18 deletions solr/core/src/java/org/apache/solr/cli/PackageTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ public String getName() {
return "package";
}

public static String solrUrl = null;
public static String solrBaseUrl = null;
public PackageManager packageManager;
public RepositoryManager repositoryManager;

Expand All @@ -79,19 +77,16 @@ public void runImpl(CommandLine cli) throws Exception {
printHelp();
return;
}

solrUrl = cli.getOptionValues("solrUrl")[cli.getOptionValues("solrUrl").length - 1];
solrBaseUrl = solrUrl.replaceAll("/solr$", ""); // strip out ending "/solr"
log.info("Solr url:{}, solr base url: {}", solrUrl, solrBaseUrl);
String solrUrl = SolrCLI.resolveSolrUrl(cli);
String zkHost = SolrCLI.getZkHost(cli);
if (zkHost == null) {
throw new SolrException(ErrorCode.INVALID_STATE, "Package manager runs only in SolrCloud");
}

log.info("ZK: {}", zkHost);

try (SolrClient solrClient = new Http2SolrClient.Builder(solrBaseUrl).build()) {
packageManager = new PackageManager(solrClient, solrBaseUrl, zkHost);
try (SolrClient solrClient = new Http2SolrClient.Builder(solrUrl).build()) {
packageManager = new PackageManager(solrClient, solrUrl, zkHost);
try {
repositoryManager = new RepositoryManager(solrClient, packageManager);

Expand Down Expand Up @@ -242,7 +237,7 @@ public void runImpl(CommandLine cli) throws Exception {

} catch (Exception ex) {
// We need to print this since SolrCLI drops the stack trace in favour
// of brevity. Package tool should surely print full stacktraces!
// of brevity. Package tool should surely print the full stacktrace!
ex.printStackTrace();
throw ex;
}
Expand Down Expand Up @@ -314,15 +309,7 @@ private Pair<String, String> parsePackageVersion(String arg) {
@Override
public List<Option> getOptions() {
return List.of(
Option.builder("solrUrl")
.argName("URL")
.hasArg()
.required(true)
.desc(
"Address of the Solr Web application, defaults to: "
+ SolrCLI.DEFAULT_SOLR_URL
+ '.')
.build(),
SolrCLI.OPTION_SOLRURL,
Option.builder("collections")
.argName("COLLECTIONS")
.hasArg()
Expand Down
4 changes: 3 additions & 1 deletion solr/core/src/java/org/apache/solr/cli/RunExampleTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ protected void runExample(CommandLine cli, String exampleName) throws Exception
+ " }\n");

File filmsJsonFile = new File(exampleDir, "films/films.json");
String updateUrl = String.format(Locale.ROOT, "%s/%s/update/json", solrUrl, collectionName);
String updateUrl = String.format(Locale.ROOT, "%s/%s/update", solrUrl, collectionName);
echo("Indexing films example docs from " + filmsJsonFile.getAbsolutePath());
String[] args =
new String[] {
Expand All @@ -410,6 +410,8 @@ protected void runExample(CommandLine cli, String exampleName) throws Exception
updateUrl,
"-type",
"application/json",
"-filetypes",
"json",
exampleDir.toString()
};
PostTool postTool = new PostTool();
Expand Down
54 changes: 43 additions & 11 deletions solr/core/src/java/org/apache/solr/cli/SolrCLI.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public class SolrCLI implements CLIO {
TimeUnit.NANOSECONDS.convert(1, TimeUnit.MINUTES);

private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
public static final String DEFAULT_SOLR_URL = "http://localhost:8983/solr";
public static final String DEFAULT_SOLR_URL = "http://localhost:8983";
public static final String ZK_HOST = "localhost:9983";

public static final Option OPTION_ZKHOST =
Expand All @@ -86,7 +86,8 @@ public class SolrCLI implements CLIO {
.required(false)
.desc(
"Zookeeper connection string; unnecessary if ZK_HOST is defined in solr.in.sh; otherwise, defaults to "
+ ZK_HOST)
+ ZK_HOST
+ '.')
.longOpt("zkHost")
.build();
public static final Option OPTION_SOLRURL =
Expand All @@ -96,7 +97,8 @@ public class SolrCLI implements CLIO {
.required(false)
.desc(
"Base Solr URL, which can be used to determine the zkHost if that's not known; defaults to: "
+ DEFAULT_SOLR_URL)
+ DEFAULT_SOLR_URL
+ '.')
.build();
public static final Option OPTION_VERBOSE =
Option.builder("verbose").required(false).desc("Enable more verbose command output.").build();
Expand Down Expand Up @@ -382,6 +384,13 @@ public static boolean exceptionIsAuthRelated(Exception exc) {
}

public static SolrClient getSolrClient(String solrUrl) {
// today we require all urls to end in /solr, however in the future we will need to support the
// /api url end point instead.
// The /solr/ check is because sometimes a full url is passed in, like
// http://localhost:8983/solr/films_shard1_replica_n1/.
if (!solrUrl.endsWith("/solr") && !solrUrl.contains("/solr/")) {
solrUrl = solrUrl + "/solr";
}
return new Http2SolrClient.Builder(solrUrl).withMaxConnectionsPerHost(32).build();
}

Expand Down Expand Up @@ -444,8 +453,34 @@ private static void printHelp() {
print("Pass -help or -h after any COMMAND to see command-specific usage information,");
print("such as: ./solr start -help or ./solr stop -h");
}

/**
* Get the base URL of a live Solr instance from either the solrUrl command-line option from
* Strips off the end of solrUrl any /solr when a legacy solrUrl like http://localhost:8983/solr
* is used, and warns those users. In the future we'll have url's with /api as well.
*
* @param solrUrl with /solr stripped off.
epugh marked this conversation as resolved.
Show resolved Hide resolved
* @return the truncated if need solrUrl.
*/
public static String resolveSolrUrl(String solrUrl) {
epugh marked this conversation as resolved.
Show resolved Hide resolved
if (solrUrl != null) {
if (solrUrl.indexOf("/solr") > -1) { //
String newSolrUrl = solrUrl.substring(0, solrUrl.indexOf("/solr"));
CLIO.out(
"'solrUrl' needn't include Solr's context-root (e.g. \"/solr\"); correcting from ["
epugh marked this conversation as resolved.
Show resolved Hide resolved
+ solrUrl
+ "] to ["
+ newSolrUrl
+ "]");
solrUrl = newSolrUrl;
}
if (solrUrl.endsWith("/")) {
solrUrl = solrUrl.substring(0, solrUrl.length() - 1);
}
}
return solrUrl;
}
/**
* Get the base URL of a live Solr instance from either the solrUrl command-line option or from
* ZooKeeper.
*/
public static String resolveSolrUrl(CommandLine cli) throws Exception {
Expand Down Expand Up @@ -474,6 +509,8 @@ public static String resolveSolrUrl(CommandLine cli) throws Exception {
solrUrl = ZkStateReader.from(cloudSolrClient).getBaseUrlForNodeName(firstLiveNode);
}
}
} else {
solrUrl = resolveSolrUrl(solrUrl);
epugh marked this conversation as resolved.
Show resolved Hide resolved
}
return solrUrl;
}
Expand All @@ -488,7 +525,6 @@ public static String getZkHost(CommandLine cli) throws Exception {
return zkHost;
}

// find it using the localPort
String solrUrl = cli.getOptionValue("solrUrl");
if (solrUrl == null) {
solrUrl = DEFAULT_SOLR_URL;
Expand All @@ -497,6 +533,8 @@ public static String getZkHost(CommandLine cli) throws Exception {
"Neither -zkHost or -solrUrl parameters provided so assuming solrUrl is "
+ DEFAULT_SOLR_URL
+ ".");
} else {
solrUrl = resolveSolrUrl(solrUrl);
}

try (var solrClient = getSolrClient(solrUrl)) {
Expand Down Expand Up @@ -568,10 +606,4 @@ public static boolean isCloudMode(SolrClient solrClient) throws SolrServerExcept
solrClient.request(new GenericSolrRequest(SolrRequest.METHOD.GET, SYSTEM_INFO_PATH));
return "solrcloud".equals(systemInfo.get("mode"));
}

public static class AssertionFailureException extends Exception {
public AssertionFailureException(String message) {
super(message);
}
}
}
2 changes: 1 addition & 1 deletion solr/core/src/java/org/apache/solr/cli/StatusTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public void runImpl(CommandLine cli) throws Exception {
}

int maxWaitSecs = Integer.parseInt(cli.getOptionValue("maxWaitSecs", "0"));
String solrUrl = cli.getOptionValue("solrUrl");
String solrUrl = SolrCLI.resolveSolrUrl(cli);
if (maxWaitSecs > 0) {
int solrPort = (new URL(solrUrl)).getPort();
echo("Waiting up to " + maxWaitSecs + " seconds to see Solr running on port " + solrPort);
Expand Down
Loading
Loading