From 0ff86b93023474f7814c36e27d3073c00a18f46e Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 22 Aug 2019 13:05:14 +0200 Subject: [PATCH] Acknowledge Indices Were Wiped Successfully in REST Tests In internal test clusters tests we check that wiping all indices was acknowledged but in REST tests we didn't. This aligns the behavior in both kinds of tests. Relates #45605 which might be caused by unacked deletes that were just slow. --- .../test/rest/ESRestTestCase.java | 43 +++++++++++-------- .../integration/DataFrameRestTestCase.java | 13 +----- .../sql/qa/security/SqlSecurityTestCase.java | 20 +++------ 3 files changed, 33 insertions(+), 43 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java index feb022045ac37..0f7ad85b46567 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java @@ -190,7 +190,7 @@ protected String getTestRestCluster() { } return cluster; } - + /** * Helper class to check warnings in REST responses with sensitivity to versions * used in the target cluster. @@ -199,14 +199,14 @@ public static class VersionSensitiveWarningsHandler implements WarningsHandler { Set requiredSameVersionClusterWarnings = new HashSet<>(); Set allowedWarnings = new HashSet<>(); final Set testNodeVersions; - + public VersionSensitiveWarningsHandler(Set nodeVersions) { this.testNodeVersions = nodeVersions; } /** * Adds to the set of warnings that are all required in responses if the cluster - * is formed from nodes all running the exact same version as the client. + * is formed from nodes all running the exact same version as the client. * @param requiredWarnings a set of required warnings */ public void current(String... requiredWarnings) { @@ -214,11 +214,11 @@ public void current(String... requiredWarnings) { } /** - * Adds to the set of warnings that are permissible (but not required) when running + * Adds to the set of warnings that are permissible (but not required) when running * in mixed-version clusters or those that differ in version from the test client. * @param allowedWarnings optional warnings that will be ignored if received */ - public void compatible(String... allowedWarnings) { + public void compatible(String... allowedWarnings) { this.allowedWarnings.addAll(Arrays.asList(allowedWarnings)); } @@ -239,15 +239,15 @@ public boolean warningsShouldFailRequest(List warnings) { return false; } } - + private boolean isExclusivelyTargetingCurrentVersionCluster() { assertFalse("Node versions running in the cluster are missing", testNodeVersions.isEmpty()); - return testNodeVersions.size() == 1 && + return testNodeVersions.size() == 1 && testNodeVersions.iterator().next().equals(Version.CURRENT); - } - + } + } - + public static RequestOptions expectVersionSpecificWarnings(Consumer expectationsSetter) { Builder builder = RequestOptions.DEFAULT.toBuilder(); VersionSensitiveWarningsHandler warningsHandler = new VersionSensitiveWarningsHandler(nodeVersions); @@ -508,14 +508,7 @@ private void wipeCluster() throws Exception { if (preserveIndicesUponCompletion() == false) { // wipe indices - try { - adminClient().performRequest(new Request("DELETE", "*")); - } catch (ResponseException e) { - // 404 here just means we had no indexes - if (e.getResponse().getStatusLine().getStatusCode() != 404) { - throw e; - } - } + wipeAllIndices(); } // wipe index templates @@ -558,6 +551,20 @@ private void wipeCluster() throws Exception { assertThat("Found in progress snapshots [" + inProgressSnapshots.get() + "].", inProgressSnapshots.get(), anEmptyMap()); } + protected static void wipeAllIndices() throws IOException { + try { + final Response response = adminClient().performRequest(new Request("DELETE", "*")); + try (InputStream is = response.getEntity().getContent()) { + assertTrue((boolean) XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true).get("acknowledged")); + } + } catch (ResponseException e) { + // 404 here just means we had no indexes + if (e.getResponse().getStatusLine().getStatusCode() != 404) { + throw e; + } + } + } + /** * Wipe fs snapshots we created one by one and all repositories so that the next test can create the repositories fresh and they'll * start empty. There isn't an API to delete all snapshots. There is an API to delete all snapshot repositories but that leaves all of diff --git a/x-pack/plugin/data-frame/qa/single-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameRestTestCase.java b/x-pack/plugin/data-frame/qa/single-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameRestTestCase.java index 09a6f1ee56ab4..8c4376ab5da61 100644 --- a/x-pack/plugin/data-frame/qa/single-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameRestTestCase.java +++ b/x-pack/plugin/data-frame/qa/single-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameRestTestCase.java @@ -355,7 +355,7 @@ public void waitForDataFrame() throws Exception { public static void removeIndices() throws Exception { // we might have disabled wiping indices, but now its time to get rid of them // note: can not use super.cleanUpCluster() as this method must be static - wipeIndices(); + wipeAllIndices(); } public void wipeDataFrameTransforms() throws IOException { @@ -403,17 +403,6 @@ protected static void waitForPendingDataFrameTasks() throws Exception { waitForPendingTasks(adminClient(), taskName -> taskName.startsWith(DataFrameField.TASK_NAME) == false); } - protected static void wipeIndices() throws IOException { - try { - adminClient().performRequest(new Request("DELETE", "*")); - } catch (ResponseException e) { - // 404 here just means we had no indexes - if (e.getResponse().getStatusLine().getStatusCode() != 404) { - throw e; - } - } - } - static int getDataFrameCheckpoint(String transformId) throws IOException { Response statsResponse = client().performRequest(new Request("GET", DATAFRAME_ENDPOINT + transformId + "/_stats")); diff --git a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/SqlSecurityTestCase.java b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/SqlSecurityTestCase.java index 313d0cdb5cf7f..aaf028181a156 100644 --- a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/SqlSecurityTestCase.java +++ b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/SqlSecurityTestCase.java @@ -13,7 +13,6 @@ import org.elasticsearch.action.fieldcaps.FieldCapabilitiesAction; import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest; import org.elasticsearch.client.Request; -import org.elasticsearch.client.ResponseException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -97,14 +96,14 @@ private static Path lookupAuditLog() { } return Paths.get(auditLogFileString); } - + @SuppressForbidden(reason="security doesn't work with mock filesystem") private static Path lookupRolledOverAuditLog() { String auditLogFileString = System.getProperty("tests.audit.yesterday.logfile"); if (null == auditLogFileString) { throw new IllegalStateException("tests.audit.yesterday.logfile must be set to run this test. It should be automatically " + "set by gradle."); - } + } return Paths.get(auditLogFileString); } @@ -120,7 +119,7 @@ private static Path lookupRolledOverAuditLog() { * How much of the audit log was written before the test started. */ private static long auditLogWrittenBeforeTestStart; - + /** * If the audit log file rolled over. This is a rare case possible only at midnight. */ @@ -188,7 +187,7 @@ public void setInitialAuditLogOffset() { } catch (IOException e) { throw new RuntimeException(e); } - + // The log file can roll over without being caught by assertLogs() method: in those tests where exceptions are being handled // and no audit logs being read (and, thus, assertLogs() is not called) - for example testNoMonitorMain() method: there are no // calls to auditLogs(), and the method could run while the audit file is rolled over. @@ -205,12 +204,7 @@ public void setInitialAuditLogOffset() { @AfterClass public static void wipeIndicesAfterTests() throws IOException { try { - adminClient().performRequest(new Request("DELETE", "*")); - } catch (ResponseException e) { - // 404 here just means we had no indexes - if (e.getResponse().getStatusLine().getStatusCode() != 404) { - throw e; - } + wipeAllIndices(); } finally { // Clear the static state so other subclasses can reuse it later oneTimeSetup = false; @@ -586,7 +580,7 @@ public void assertLogs() throws Exception { if (sm != null) { sm.checkPermission(new SpecialPermission()); } - + BufferedReader[] logReaders = new BufferedReader[2]; AccessController.doPrivileged((PrivilegedAction) () -> { try { @@ -604,7 +598,7 @@ public void assertLogs() throws Exception { throw new RuntimeException(e); } }); - + // The "index" is used as a way of reading from both rolled over file and current audit file in order: rolled over file // first, then the audit log file. Very rarely we will read from the rolled over file: when the test happened to run // at midnight and the audit file rolled over during the test.