From e7c15135ba9fc019ed7c5ef8f56809dfd327d73d Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 22 Aug 2019 15:49:17 +0200 Subject: [PATCH] Acknowledge Indices Were Wiped Successfully in REST Tests (#45832) 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 b3840f96dcd10..4513e5d98f793 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); @@ -513,14 +513,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 @@ -563,6 +556,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.