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.

Sign up for GitHub

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

Jump to bottom

Fix assert tool url normalization #2778

Merged
merged 9 commits into from
Oct 20, 2024
Merged

Fix assert tool url normalization #2778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
93773b4
Revert "Support backport of SOLR-17489"
janhoy Oct 18, 2024
dbcb79a
SOLR-17489 Fix bad backport
janhoy Oct 18, 2024
d501fd3
Revert "Fix tests to pass in branch_9x"
janhoy Oct 18, 2024
ed95966
Use SolrCLI.getSolrClient(url)
janhoy Oct 18, 2024
b931756
Revert doc change
janhoy Oct 18, 2024
e80504a
Add back /solr in PostTool
janhoy Oct 18, 2024
c660d0a
Revert /solr fix, always normalize
janhoy Oct 18, 2024
5e6ca6b
Backport BATS test that we need
epugh Oct 19, 2024
ec94b0e
Copy pattern from SolrCLI to handle hostContext.
epugh Oct 19, 2024
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
3 changes: 1 addition & 2 deletions solr/core/src/java/org/apache/solr/cli/AssertTool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.Http2SolrClient;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.HealthCheckRequest;
import org.apache.solr.client.solrj.response.CollectionAdminResponse;
Expand Down Expand Up @@ -531,7 +530,7 @@ 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)) {
janhoy marked this conversation as resolved.
Show resolved Hide resolved
final SolrRequest<CollectionAdminResponse> request =
new CollectionAdminRequest.ClusterStatus();
final CollectionAdminResponse response = request.process(client);
Expand Down
18 changes: 16 additions & 2 deletions solr/core/src/java/org/apache/solr/cli/PostTool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.PrintStream;
import java.lang.invoke.MethodHandles;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.ProtocolException;
Expand Down Expand Up @@ -74,13 +75,17 @@
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.common.util.Utils;
import org.apache.solr.util.RTimer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.xml.sax.SAXException;

public class PostTool extends ToolBase {

private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

public static final String DEFAULT_FILE_TYPES =
"xml,json,jsonl,csv,pdf,doc,docx,ppt,pptx,xls,xlsx,odt,odp,ods,ott,otp,ots,rtf,htm,html,txt,log";
static final String DATA_MODE_FILES = "files";
Expand Down Expand Up @@ -315,8 +320,17 @@ public void runImpl(CommandLine cli) throws Exception {
throw new IllegalArgumentException(
"Must specify -c / --name parameter with --solr-url to post documents.");
}
String url =
SolrCLI.normalizeSolrUrl(cli) + "/solr/" + cli.getOptionValue("name") + "/update";

String solrUrl = cli.getOptionValue("solr-url");

String hostContext = System.getProperty("hostContext", "/solr");
if (hostContext.isBlank()) {
log.warn("Invalid hostContext {} provided, setting to /solr", hostContext);
hostContext = "/solr";
}

solrUrl = SolrCLI.normalizeSolrUrl(solrUrl, true, hostContext) + hostContext;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought of the same workaround, and guess I'm medium happy about it. Would of course prefer a SolrCLI util method that hides this such as SolrCLI.getAndNormalizeSolrUrl(cli), and consider using it wherever we do raw cli.getOptionValue("solr-url")`, since we'd potentially run into same issue elsewhere.

String url = solrUrl + "/" + cli.getOptionValue("name") + "/update";
solrUpdateUrl = new URI(url);

} else if (cli.hasOption("solr-update-url")) {
Expand Down
1 change: 0 additions & 1 deletion solr/core/src/java/org/apache/solr/cli/SolrCLI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,6 @@ public static String normalizeSolrUrl(CommandLine cli) throws Exception {

String firstLiveNode = liveNodes.iterator().next();
solrUrl = ZkStateReader.from(cloudSolrClient).getBaseUrlForNodeName(firstLiveNode);
// solrUrl = normalizeSolrUrl(solrUrl, false);
}
}
}
Expand Down
14 changes: 14 additions & 0 deletions solr/packaging/test/test_assert.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,17 @@ teardown() {

run ! solr assert --cloud http://localhost:${SOLR_PORT} --exitcode
}

@test "assert for cloud mode" {
run solr start --cloud
solr assert --started http://localhost:${SOLR_PORT} --timeout 5000

run solr assert --cloud http://localhost:${SOLR_PORT}
refute_output --partial "ERROR"

run solr assert --not-cloud http://localhost:${SOLR_PORT}/solr
assert_output --partial "needn't include Solr's context-root"
assert_output --partial "ERROR: Solr is not running in standalone mode"

run ! solr assert --not-cloud http://localhost:${SOLR_PORT} --exitcode
}
4 changes: 2 additions & 2 deletions solr/packaging/test/test_placement_plugin.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ teardown() {

@test "Affinity placement plugin using sysprop" {
run solr start -c -Dsolr.placementplugin.default=affinity
solr assert --cloud http://localhost:${SOLR_PORT}/solr --timeout 3000
solr assert --cloud http://localhost:${SOLR_PORT} --timeout 3000
run solr create -c COLL_NAME
collection_exists COLL_NAME
assert_file_contains "${SOLR_LOGS_DIR}/solr.log" 'Default replica placement plugin set in solr\.placementplugin\.default to affinity'
Expand All @@ -40,7 +40,7 @@ teardown() {
@test "Random placement plugin using ENV" {
export SOLR_PLACEMENTPLUGIN_DEFAULT=random
run solr start -c
solr assert --cloud http://localhost:${SOLR_PORT}/solr --timeout 3000
solr assert --cloud http://localhost:${SOLR_PORT} --timeout 3000
run solr create -c COLL_NAME
collection_exists COLL_NAME
assert_file_contains "${SOLR_LOGS_DIR}/solr.log" 'Default replica placement plugin set in solr\.placementplugin\.default to random'
Expand Down
Loading