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

Fixing issues caused by 3.0 version bumping #696

Merged
merged 9 commits into from
Oct 21, 2022
Merged

Fixing issues caused by 3.0 version bumping #696

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
e78ddd0
Update BaseNodeRequest dependency
jackiehanyang Oct 18, 2022
04620d9
Migrate client transports to Apache HttpClient / Core 5.x
jackiehanyang Oct 19, 2022
9ce2f41
Address spotlessCheck failures
jackiehanyang Oct 19, 2022
3852fb4
Upgrade bwc version because 1.1.0 cannot be upgraded directly to vers…
jackiehanyang Oct 19, 2022
87c4797
update bwc version to 2.4
jackiehanyang Oct 20, 2022
d63521b
Update bwc version
jackiehanyang Oct 20, 2022
082b9ee
update bwc download link
jackiehanyang Oct 20, 2022
f48deba
use latest 2.4 build for bwc test
jackiehanyang Oct 21, 2022
c2ee2bf
move bwc constants to build script block
jackiehanyang Oct 21, 2022
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
23 changes: 11 additions & 12 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,15 @@ buildscript {
'/latest/linux/x64/tar/builds/opensearch/plugins/opensearch-job-scheduler-' + plugin_no_snapshot + '.zip'
anomaly_detection_build_download = 'https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/' + opensearch_no_snapshot +
'/latest/linux/x64/tar/builds/opensearch/plugins/opensearch-anomaly-detection-' + plugin_no_snapshot + '.zip'
bwcOpenSearchADDownload = 'https://ci.opensearch.org/ci/dbc/bundle-build/1.1.0/20210930/linux/x64/builds/opensearch/plugins/' +
'opensearch-anomaly-detection-1.1.0.0.zip'
bwcOpenSearchJSDownload = 'https://ci.opensearch.org/ci/dbc/bundle-build/1.1.0/20210930/linux/x64/builds/opensearch/plugins/' +
'opensearch-job-scheduler-1.1.0.0.zip'
bwcOpenSearchADDownload = 'https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.4.0/latest/linux/x64/tar/builds/' +
'opensearch/plugins/opensearch-anomaly-detection-2.4.0.0.zip'
bwcOpenSearchJSDownload = 'https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.4.0/latest/linux/x64/tar/builds/' +
'opensearch/plugins/opensearch-job-scheduler-2.4.0.0.zip'
bwcVersion = "2.4.0.0"
baseName = "adBwcCluster"
bwcFilePath = "src/test/resources/org/opensearch/ad/bwc/"
bwcJobSchedulerPath = bwcFilePath + "job-scheduler/"
bwcAnomalyDetectionPath = bwcFilePath + "anomaly-detection/"

// gradle build won't print logs during test by default unless there is a failure.
// It is useful to record intermediately information like prediction precision and recall.
Expand Down Expand Up @@ -136,7 +141,7 @@ configurations.all {
force "joda-time:joda-time:${versions.joda}"
force "com.fasterxml.jackson.core:jackson-core:2.13.4"
force "commons-logging:commons-logging:${versions.commonslogging}"
force "org.apache.httpcomponents:httpcore:${versions.httpcore}"
force "org.apache.httpcomponents:httpcore5:${versions.httpcore5}"
Copy link
Member

Choose a reason for hiding this comment

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

Could we possible just remove this force? and just inherit it as is from opensearch core?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

do you know what's the initial reason of adding this force instead of inheriting it from opensearch core?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure, seems to be with our initial set up of AD. Usually from my understanding its used if we need a specific version of a dependency that doesn't align with core so we force it to be something else. I believe though since we aren't in need of anything specific we should be able to remove this line and build/testing should be fine.

Copy link
Member

Choose a reason for hiding this comment

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

Before doing that I would confirm with @ylwu-amzn or @kaituo on the reasoning. When that's figured out, if it's still needed, we should add a clarifying comment above.

force "commons-codec:commons-codec:${versions.commonscodec}"

force "org.mockito:mockito-core:2.25.0"
Expand Down Expand Up @@ -372,17 +377,11 @@ task integTestRemote(type: RestIntegTestTask) {
}
}

String bwcVersion = "1.1.0.0"
String baseName = "adBwcCluster"
String bwcFilePath = "src/test/resources/org/opensearch/ad/bwc/"
String bwcJobSchedulerPath = bwcFilePath + "job-scheduler/"
String bwcAnomalyDetectionPath = bwcFilePath + "anomaly-detection/"

2.times {i ->
testClusters {
"${baseName}$i" {
testDistribution = "ARCHIVE"
versions = ["1.1.0", opensearch_version]
versions = ["2.4.0", opensearch_version]
numberOfNodes = 3
plugin(provider(new Callable<RegularFile>(){
@Override
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/opensearch/ad/transport/ADCancelTaskNodeRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@

import java.io.IOException;

import org.opensearch.action.support.nodes.BaseNodeRequest;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.transport.TransportRequest;

public class ADCancelTaskNodeRequest extends BaseNodeRequest {
public class ADCancelTaskNodeRequest extends TransportRequest {
private String detectorId;
private String detectorTaskId;
private String userName;
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/opensearch/ad/transport/ADStatsNodeRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@

import java.io.IOException;

import org.opensearch.action.support.nodes.BaseNodeRequest;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.transport.TransportRequest;

/**
* ADStatsNodeRequest to get a nodes stat
*/
public class ADStatsNodeRequest extends BaseNodeRequest {
public class ADStatsNodeRequest extends TransportRequest {
private ADStatsRequest request;

/**
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/opensearch/ad/transport/ADTaskProfileNodeRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@

import java.io.IOException;

import org.opensearch.action.support.nodes.BaseNodeRequest;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.transport.TransportRequest;

public class ADTaskProfileNodeRequest extends BaseNodeRequest {
public class ADTaskProfileNodeRequest extends TransportRequest {
private String detectorId;

public ADTaskProfileNodeRequest(StreamInput in) throws IOException {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/opensearch/ad/transport/CronNodeRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@

import java.io.IOException;

import org.opensearch.action.support.nodes.BaseNodeRequest;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.transport.TransportRequest;

/**
* Delete model represents the request to an individual node
*/
public class CronNodeRequest extends BaseNodeRequest {
public class CronNodeRequest extends TransportRequest {

public CronNodeRequest() {}

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/opensearch/ad/transport/DeleteModelNodeRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@

import java.io.IOException;

import org.opensearch.action.support.nodes.BaseNodeRequest;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.transport.TransportRequest;

/**
* Delete model represents the request to an individual node
*/
public class DeleteModelNodeRequest extends BaseNodeRequest {
public class DeleteModelNodeRequest extends TransportRequest {

private String adID;

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/opensearch/ad/transport/ProfileNodeRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
import java.io.IOException;
import java.util.Set;

import org.opensearch.action.support.nodes.BaseNodeRequest;
import org.opensearch.ad.model.DetectorProfileName;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.transport.TransportRequest;

/**
* Class representing a nodes's profile request
*/
public class ProfileNodeRequest extends BaseNodeRequest {
public class ProfileNodeRequest extends TransportRequest {
private ProfileRequest request;

public ProfileNodeRequest(StreamInput in) throws IOException {
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/opensearch/ad/AnomalyDetectorRestTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
import java.util.Locale;
import java.util.Map;

import org.apache.http.HttpHeaders;
import org.apache.http.message.BasicHeader;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.message.BasicHeader;
import org.opensearch.ad.model.ADTask;
import org.opensearch.ad.model.AnomalyDetector;
import org.opensearch.ad.model.AnomalyDetectorExecutionInput;
Expand Down
17 changes: 10 additions & 7 deletions src/test/java/org/opensearch/ad/HistoricalAnalysisRestTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@
import java.util.Set;
import java.util.function.ToDoubleFunction;

import org.apache.http.HttpHeaders;
import org.apache.http.message.BasicHeader;
import org.apache.http.util.EntityUtils;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.ParseException;
import org.apache.hc.core5.http.io.entity.EntityUtils;
import org.apache.hc.core5.http.message.BasicHeader;
import org.junit.Before;
import org.opensearch.ad.mock.model.MockSimpleLog;
import org.opensearch.ad.model.ADTaskProfile;
Expand Down Expand Up @@ -62,7 +63,7 @@ public ToXContentObject[] getHistoricalAnomalyDetector(String detectorId, boolea
return getAnomalyDetector(detectorId, header, false, returnTask, client);
}

public ADTaskProfile getADTaskProfile(String detectorId) throws IOException {
public ADTaskProfile getADTaskProfile(String detectorId) throws IOException, ParseException {
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
Response profileResponse = TestHelpers
.makeRequest(
client(),
Expand Down Expand Up @@ -99,7 +100,8 @@ public Response ingestSimpleMockLog(
ToDoubleFunction<Integer> valueFunc,
int ipSize,
int categorySize
) throws IOException {
) throws IOException,
ParseException {
TestHelpers
.makeRequest(
client(),
Expand Down Expand Up @@ -150,7 +152,7 @@ public Response ingestSimpleMockLog(
return bulkResponse;
}

public ADTaskProfile parseADTaskProfile(Response profileResponse) throws IOException {
public ADTaskProfile parseADTaskProfile(Response profileResponse) throws IOException, ParseException {
String profileResult = EntityUtils.toString(profileResponse.getEntity());
XContentParser parser = TestHelpers.parser(profileResult);
ADTaskProfile adTaskProfile = null;
Expand All @@ -166,7 +168,8 @@ public ADTaskProfile parseADTaskProfile(Response profileResponse) throws IOExcep
return adTaskProfile;
}

protected void ingestTestDataForHistoricalAnalysis(String indexName, int detectionIntervalInMinutes) throws IOException {
protected void ingestTestDataForHistoricalAnalysis(String indexName, int detectionIntervalInMinutes) throws IOException,
ParseException {
ingestSimpleMockLog(indexName, 10, 3000, detectionIntervalInMinutes, (i) -> {
if (i % 500 == 0) {
return randomDoubleBetween(100, 1000, true);
Expand Down
50 changes: 33 additions & 17 deletions src/test/java/org/opensearch/ad/ODFERestTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,19 @@
import java.util.Optional;
import java.util.stream.Collectors;

import org.apache.http.Header;
import org.apache.http.HttpHost;
import org.apache.http.auth.AuthScope;
import org.apache.http.auth.UsernamePasswordCredentials;
import org.apache.http.client.CredentialsProvider;
import org.apache.http.conn.ssl.NoopHostnameVerifier;
import org.apache.http.impl.client.BasicCredentialsProvider;
import org.apache.http.message.BasicHeader;
import org.apache.http.ssl.SSLContextBuilder;
import org.apache.hc.client5.http.auth.AuthScope;
import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager;
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.message.BasicHeader;
import org.apache.hc.core5.http.nio.ssl.TlsStrategy;
import org.apache.hc.core5.ssl.SSLContextBuilder;
import org.apache.hc.core5.util.Timeout;
import org.junit.After;
import org.opensearch.client.Request;
import org.opensearch.client.Response;
Expand Down Expand Up @@ -139,7 +143,7 @@ protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOE
@After
protected void wipeAllODFEIndices() throws IOException {
Response response = adminClient().performRequest(new Request("GET", "/_cat/indices?format=json&expand_wildcards=all"));
XContentType xContentType = XContentType.fromMediaType(response.getEntity().getContentType().getValue());
XContentType xContentType = XContentType.fromMediaType(response.getEntity().getContentType());
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
try (
XContentParser parser = xContentType
.xContent()
Expand Down Expand Up @@ -181,14 +185,25 @@ protected static void configureHttpsClient(RestClientBuilder builder, Settings s
String password = Optional
.ofNullable(System.getProperty("password"))
.orElseThrow(() -> new RuntimeException("password is missing"));
CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
credentialsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(userName, password));
BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider();
credentialsProvider
.setCredentials(
new AuthScope(new HttpHost("localhost", 9200)),
new UsernamePasswordCredentials(userName, password.toCharArray())
);
try {
return httpClientBuilder
.setDefaultCredentialsProvider(credentialsProvider)
final TlsStrategy tlsStrategy = ClientTlsStrategyBuilder
.create()
.setSslContext(SSLContextBuilder.create().loadTrustMaterial(null, (chains, authType) -> true).build())
// disable the certificate since our testing cluster just uses the default security configuration
.setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE)
.setSSLContext(SSLContextBuilder.create().loadTrustMaterial(null, (chains, authType) -> true).build());
.setHostnameVerifier(NoopHostnameVerifier.INSTANCE)
.build();

final PoolingAsyncClientConnectionManager connectionManager = PoolingAsyncClientConnectionManagerBuilder
.create()
.setTlsStrategy(tlsStrategy)
.build();
return httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider).setConnectionManager(connectionManager);
} catch (Exception e) {
throw new RuntimeException(e);
}
Expand All @@ -197,7 +212,8 @@ protected static void configureHttpsClient(RestClientBuilder builder, Settings s
final String socketTimeoutString = settings.get(CLIENT_SOCKET_TIMEOUT);
final TimeValue socketTimeout = TimeValue
.parseTimeValue(socketTimeoutString == null ? "60s" : socketTimeoutString, CLIENT_SOCKET_TIMEOUT);
builder.setRequestConfigCallback(conf -> conf.setSocketTimeout(Math.toIntExact(socketTimeout.getMillis())));
builder
.setRequestConfigCallback(conf -> conf.setResponseTimeout(Timeout.ofMilliseconds(Math.toIntExact(socketTimeout.getMillis()))));
if (settings.hasValue(CLIENT_PATH_PREFIX)) {
builder.setPathPrefix(settings.get(CLIENT_PATH_PREFIX));
}
Expand Down
13 changes: 6 additions & 7 deletions src/test/java/org/opensearch/ad/TestHelpers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

package org.opensearch.ad;

import static org.apache.http.entity.ContentType.APPLICATION_JSON;
import static org.apache.hc.core5.http.ContentType.APPLICATION_JSON;
import static org.opensearch.ad.model.AnomalyDetectorJob.ANOMALY_DETECTOR_JOB_INDEX;
import static org.opensearch.cluster.node.DiscoveryNodeRole.BUILT_IN_ROLES;
import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
Expand Down Expand Up @@ -39,11 +39,10 @@
import java.util.function.Consumer;
import java.util.stream.IntStream;

import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.nio.entity.NStringEntity;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.util.Strings;
Expand Down Expand Up @@ -172,7 +171,7 @@ public static Response makeRequest(
String jsonEntity,
List<Header> headers
) throws IOException {
HttpEntity httpEntity = Strings.isBlank(jsonEntity) ? null : new NStringEntity(jsonEntity, ContentType.APPLICATION_JSON);
HttpEntity httpEntity = Strings.isBlank(jsonEntity) ? null : new StringEntity(jsonEntity, ContentType.APPLICATION_JSON);
return makeRequest(client, method, endpoint, params, httpEntity, headers);
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/opensearch/ad/bwc/ADBackwardsCompatibilityIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
import java.util.Set;
import java.util.stream.Collectors;

import org.apache.http.HttpEntity;
import org.apache.hc.core5.http.HttpEntity;
import org.junit.Assert;
import org.junit.Before;
import org.opensearch.ad.TestHelpers;
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/opensearch/ad/e2e/DetectionResultEvalutationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
import java.util.TimeZone;
import java.util.concurrent.TimeUnit;

import org.apache.http.HttpHeaders;
import org.apache.http.message.BasicHeader;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.message.BasicHeader;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.Logger;
import org.opensearch.ad.ODFERestTestCase;
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/opensearch/ad/mock/transport/MockADCancelTaskNodeRequest_1_0.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@

import java.io.IOException;

import org.opensearch.action.support.nodes.BaseNodeRequest;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.transport.TransportRequest;

public class MockADCancelTaskNodeRequest_1_0 extends BaseNodeRequest {
public class MockADCancelTaskNodeRequest_1_0 extends TransportRequest {
private String detectorId;
private String userName;

Expand Down
13 changes: 7 additions & 6 deletions src/test/java/org/opensearch/ad/rest/ADRestTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@
import java.util.Set;
import java.util.function.ToDoubleFunction;

import org.apache.http.HttpEntity;
import org.apache.http.HttpHeaders;
import org.apache.http.message.BasicHeader;
import org.apache.http.util.EntityUtils;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.ParseException;
import org.apache.hc.core5.http.io.entity.EntityUtils;
import org.apache.hc.core5.http.message.BasicHeader;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.Logger;
import org.opensearch.ad.TestHelpers;
Expand Down Expand Up @@ -486,7 +487,7 @@ public static ADTaskProfile waitUntilTaskReachState(RestClient client, String de
return adTaskProfile;
}

public static ADTaskProfile getADTaskProfile(RestClient client, String detectorId) throws IOException {
public static ADTaskProfile getADTaskProfile(RestClient client, String detectorId) throws IOException, ParseException {
Response profileResponse = TestHelpers
.makeRequest(
client,
Expand All @@ -499,7 +500,7 @@ public static ADTaskProfile getADTaskProfile(RestClient client, String detectorI
return parseADTaskProfile(profileResponse);
}

public static ADTaskProfile parseADTaskProfile(Response profileResponse) throws IOException {
public static ADTaskProfile parseADTaskProfile(Response profileResponse) throws IOException, ParseException {
String profileResult = EntityUtils.toString(profileResponse.getEntity());
XContentParser parser = TestHelpers.parser(profileResult);
ADTaskProfile adTaskProfile = null;
Expand Down
Loading