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

QA: Create xpack yaml features #31403

Merged
merged 4 commits into from
Jun 26, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
7 changes: 0 additions & 7 deletions qa/mixed-cluster/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,6 @@ for (Version version : bwcVersions.wireCompatible) {
tasks.getByName("${baseName}#mixedClusterTestRunner").configure {
/* To support taking index snapshots, we have to set path.repo setting */
systemProperty 'tests.path.repo', new File(buildDir, "cluster/shared/repo")
if ('zip'.equals(extension.distribution)) {
systemProperty 'tests.rest.blacklist', [
'cat.templates/10_basic/No templates',
'cat.templates/10_basic/Sort templates',
'cat.templates/10_basic/Multiple template',
].join(',')
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
---
"No templates":
- skip:
features: default_shards
features: default_shards, no_xpack
- do:
cat.templates: {}

Expand Down Expand Up @@ -177,7 +177,7 @@
---
"Sort templates":
- skip:
features: default_shards
features: default_shards, no_xpack
- do:
indices.put_template:
name: test
Expand Down Expand Up @@ -227,7 +227,7 @@
---
"Multiple template":
- skip:
features: default_shards
features: default_shards, no_xpack
- do:
indices.put_template:
name: test_1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.http.ssl.SSLContexts;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksAction;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.RestClient;
Expand All @@ -41,6 +42,8 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.xcontent.DeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
Expand Down Expand Up @@ -91,13 +94,38 @@ public abstract class ESRestTestCase extends ESTestCase {
/**
* Convert the entity from a {@link Response} into a map of maps.
*/
public Map<String, Object> entityAsMap(Response response) throws IOException {
public static Map<String, Object> entityAsMap(Response response) throws IOException {
Copy link
Member Author

Choose a reason for hiding this comment

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

I made this static so I could make hasXPack static. There was no need for it to use the fancy registries provided by createParser.

XContentType xContentType = XContentType.fromMediaTypeOrFormat(response.getEntity().getContentType().getValue());
try (XContentParser parser = createParser(xContentType.xContent(), response.getEntity().getContent())) {
// EMPTY and THROW are fine here because `.map` doesn't use named x content or deprecation
try (XContentParser parser = xContentType.xContent().createParser(
NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
response.getEntity().getContent())) {
return parser.map();
}
}

/**
* Does the cluster being tested have xpack installed?
*/
public static boolean hasXPack() throws IOException {
Copy link
Member Author

Choose a reason for hiding this comment

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

I made this static so that we could use it easily from feature. I feel like if client() is static then this can be.

RestClient client = adminClient();
if (client == null) {
throw new IllegalStateException("must be called inside of a rest test case test");
}
Map<?, ?> response = entityAsMap(client.performRequest(new Request("GET", "_nodes/plugins")));
Map<?, ?> nodes = (Map<?, ?>) response.get("nodes");
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to do any null checks here? Im a-ok not doing it given we always know it will come back w/ the data we want/need. I know its just a call to _nodes/plugins so I assume this is legit.

Copy link
Member Author

Choose a reason for hiding this comment

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

I figure we change the format of _nodes/plugins rarely enough that we're probably safe without them. If we change the format a NullPointException is the least of our problems, I think.

for (Map.Entry<?, ?> node : nodes.entrySet()) {
Map<?, ?> nodeInfo = (Map<?, ?>) node.getValue();
for (Object module: (List<?>) nodeInfo.get("modules")) {
Map<?, ?> moduleInfo = (Map<?, ?>) module;
if (moduleInfo.get("name").toString().startsWith("x-pack-")) {
return true;
}
}
}
return false;
}

private static List<HttpHost> clusterHosts;
/**
* A client for the running Elasticsearch cluster
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@
import java.util.Arrays;
import java.util.List;

import org.elasticsearch.test.rest.ESRestTestCase;

import static java.util.Collections.unmodifiableList;

import java.io.IOException;

/**
* Allows to register additional features supported by the tests runner.
* This way any runner can add extra features and use proper skip sections to avoid
Expand Down Expand Up @@ -53,11 +57,23 @@ private Features() {
* Tells whether all the features provided as argument are supported
*/
public static boolean areAllSupported(List<String> features) {
for (String feature : features) {
if (!SUPPORTED.contains(feature)) {
return false;
try {
for (String feature : features) {
if (feature.equals("xpack")) {
if (false == ESRestTestCase.hasXPack()) {
return false;
}
} else if (feature.equals("no_xpack")) {
if (ESRestTestCase.hasXPack()) {
return false;
}
} else if (false == SUPPORTED.contains(feature)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason we just dont return SUPPORTED.contains(feature) for the last if branch?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to keep looping to check for the next feature.

return false;
}
}
return true;
} catch (IOException e) {
throw new RuntimeException("error checking if xpack is available", e);
}
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,16 @@ public class MlRestTestStateCleaner {

private final Logger logger;
private final RestClient adminClient;
private final ESRestTestCase testCase;

public MlRestTestStateCleaner(Logger logger, RestClient adminClient, ESRestTestCase testCase) {
public MlRestTestStateCleaner(Logger logger, RestClient adminClient) {
this.logger = logger;
this.adminClient = adminClient;
this.testCase = testCase;
}

public void clearMlMetadata() throws IOException {
deleteAllDatafeeds();
deleteAllJobs();
// indices will be deleted by the ESIntegTestCase class
// indices will be deleted by the ESRestTestCase class
}

@SuppressWarnings("unchecked")
Expand All @@ -41,7 +39,7 @@ private void deleteAllDatafeeds() throws IOException {
final Response datafeedsResponse = adminClient.performRequest(datafeedsRequest);
@SuppressWarnings("unchecked")
final List<Map<String, Object>> datafeeds =
(List<Map<String, Object>>) XContentMapValues.extractValue("datafeeds", testCase.entityAsMap(datafeedsResponse));
(List<Map<String, Object>>) XContentMapValues.extractValue("datafeeds", ESRestTestCase.entityAsMap(datafeedsResponse));
Copy link
Member Author

Choose a reason for hiding this comment

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

Checkstyle forced me to change this and the rest of the changes to this file come about because of the change here.

if (datafeeds == null) {
return;
}
Expand Down Expand Up @@ -83,7 +81,7 @@ private void deleteAllJobs() throws IOException {
final Response response = adminClient.performRequest(jobsRequest);
@SuppressWarnings("unchecked")
final List<Map<String, Object>> jobConfigs =
(List<Map<String, Object>>) XContentMapValues.extractValue("jobs", testCase.entityAsMap(response));
(List<Map<String, Object>>) XContentMapValues.extractValue("jobs", ESRestTestCase.entityAsMap(response));
if (jobConfigs == null) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,16 @@ public class RollupRestTestStateCleaner {

private final Logger logger;
private final RestClient adminClient;
private final ESRestTestCase testCase;

public RollupRestTestStateCleaner(Logger logger, RestClient adminClient, ESRestTestCase testCase) {
public RollupRestTestStateCleaner(Logger logger, RestClient adminClient) {
this.logger = logger;
this.adminClient = adminClient;
this.testCase = testCase;
}

public void clearRollupMetadata() throws Exception {
deleteAllJobs();
waitForPendingTasks();
// indices will be deleted by the ESIntegTestCase class
// indices will be deleted by the ESRestTestCase class
}

private void waitForPendingTasks() throws Exception {
Expand Down Expand Up @@ -75,7 +73,7 @@ private void waitForPendingTasks() throws Exception {
@SuppressWarnings("unchecked")
private void deleteAllJobs() throws Exception {
Response response = adminClient.performRequest("GET", "/_xpack/rollup/job/_all");
Map<String, Object> jobs = testCase.entityAsMap(response);
Map<String, Object> jobs = ESRestTestCase.entityAsMap(response);
Copy link
Member Author

Choose a reason for hiding this comment

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

Checkstyle forced me to change this and the rest of the changes to this file come about because of the change here.

@SuppressWarnings("unchecked")
List<Map<String, Object>> jobConfigs =
(List<Map<String, Object>>) XContentMapValues.extractValue("jobs", jobs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ public void cleanup() throws Exception {
*/
private void clearMlState() throws Exception {
if (isMachineLearningTest()) {
new MlRestTestStateCleaner(logger, adminClient(), this).clearMlMetadata();
new MlRestTestStateCleaner(logger, adminClient()).clearMlMetadata();
}
}

Expand All @@ -263,7 +263,7 @@ private void clearMlState() throws Exception {
*/
private void clearRollupState() throws Exception {
if (isRollupTest()) {
new RollupRestTestStateCleaner(logger, adminClient(), this).clearRollupMetadata();
new RollupRestTestStateCleaner(logger, adminClient()).clearRollupMetadata();
}
}

Expand Down
3 changes: 0 additions & 3 deletions x-pack/qa/core-rest-tests-with-security/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ integTestRunner {
'index/10_with_id/Index with ID',
'indices.get_alias/10_basic/Get alias against closed indices',
'indices.get_alias/20_empty/Check empty aliases when getting all aliases via /_alias',
'cat.templates/10_basic/No templates',
'cat.templates/10_basic/Sort templates',
'cat.templates/10_basic/Multiple template',
].join(',')

systemProperty 'tests.rest.cluster.username', System.getProperty('tests.rest.cluster.username', 'test_user')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,7 @@ public static void openJob(RestClient client, String jobId) throws IOException {

@After
public void clearMlState() throws Exception {
new MlRestTestStateCleaner(logger, adminClient(), this).clearMlMetadata();
new MlRestTestStateCleaner(logger, adminClient()).clearMlMetadata();
XPackRestTestHelper.waitForPendingTasks(adminClient());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ private static String responseEntityToString(Response response) throws IOExcepti

@After
public void clearMlState() throws Exception {
new MlRestTestStateCleaner(logger, adminClient(), this).clearMlMetadata();
new MlRestTestStateCleaner(logger, adminClient()).clearMlMetadata();
XPackRestTestHelper.waitForPendingTasks(adminClient());
}
}