From ecb3ebd7962f1b4814d8b048945446fce5ccad80 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 16 Aug 2019 14:17:34 -0600 Subject: [PATCH] Clean SLM and ongoing snapshots in test framework (#45564) Adjusts the cluster cleanup routine in ESRestTestCase to clean up SLM test cases, and optionally wait for all snapshots to be deleted. Waiting for all snapshots to be deleted, rather than failing if any are in progress, is necessary for tests which use SLM policies because SLM policies may be in the process of executing when the test ends. --- .../SnapshotClientDocumentationIT.java | 5 ++ .../test/rest/ESRestTestCase.java | 63 +++++++++++++++++-- .../xpack/slm/SnapshotLifecycleIT.java | 32 ++-------- 3 files changed, 69 insertions(+), 31 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java index 8a1b7db1455..0213f32068c 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java @@ -95,6 +95,11 @@ public class SnapshotClientDocumentationIT extends ESRestHighLevelClientTestCase private static final String snapshotName = "test_snapshot"; private static final String indexName = "test_index"; + @Override + protected boolean waitForAllSnapshotsWiped() { + return true; + } + public void testSnapshotCreateRepository() throws IOException { RestHighLevelClient client = highLevelClient(); 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 f432dc2a374..b3840f96dcd 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 @@ -26,6 +26,7 @@ import org.apache.http.message.BasicHeader; import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy; import org.apache.http.ssl.SSLContexts; import org.apache.http.util.EntityUtils; +import org.apache.lucene.util.SetOnce; import org.elasticsearch.Version; import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksAction; import org.elasticsearch.client.Request; @@ -80,11 +81,13 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Predicate; import static java.util.Collections.sort; import static java.util.Collections.unmodifiableList; +import static org.hamcrest.Matchers.anEmptyMap; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.equalTo; @@ -468,6 +471,12 @@ public abstract class ESRestTestCase extends ESTestCase { return false; } + /** + * Returns whether to wait to make absolutely certain that all snapshots + * have been deleted. + */ + protected boolean waitForAllSnapshotsWiped() { return false; } + private void wipeCluster() throws Exception { // Cleanup rollup before deleting indices. A rollup job might have bulks in-flight, @@ -478,7 +487,29 @@ public abstract class ESRestTestCase extends ESTestCase { waitForPendingRollupTasks(); } - final Map>> inProgressSnapshots = wipeSnapshots(); + // Clean up SLM policies before trying to wipe snapshots so that no new ones get started by SLM after wiping + if (nodeVersions.first().onOrAfter(Version.V_7_4_0)) { // SLM was introduced in version 7.4 + deleteAllSLMPolicies(); + } + + SetOnce>>> inProgressSnapshots = new SetOnce<>(); + if (waitForAllSnapshotsWiped()) { + AtomicReference>>> snapshots = new AtomicReference<>(); + try { + // Repeatedly delete the snapshots until there aren't any + assertBusy(() -> { + snapshots.set(wipeSnapshots()); + assertThat(snapshots.get(), anEmptyMap()); + }, 2, TimeUnit.MINUTES); + // At this point there should be no snaphots + inProgressSnapshots.set(snapshots.get()); + } catch (AssertionError e) { + // This will cause an error at the end of this method, but do the rest of the cleanup first + inProgressSnapshots.set(snapshots.get()); + } + } else { + inProgressSnapshots.set(wipeSnapshots()); + } if (preserveIndicesUponCompletion() == false) { // wipe indices @@ -526,10 +557,10 @@ public abstract class ESRestTestCase extends ESTestCase { } if (hasXPack && false == preserveILMPoliciesUponCompletion()) { - deleteAllPolicies(); + deleteAllILMPolicies(); } - assertTrue("Found in progress snapshots [" + inProgressSnapshots + "].", inProgressSnapshots.isEmpty()); + assertThat("Found in progress snapshots [" + inProgressSnapshots.get() + "].", inProgressSnapshots.get(), anEmptyMap()); } /** @@ -634,7 +665,7 @@ public abstract class ESRestTestCase extends ESTestCase { waitForPendingTasks(adminClient(), taskName -> taskName.startsWith("xpack/rollup/job") == false); } - private static void deleteAllPolicies() throws IOException { + private static void deleteAllILMPolicies() throws IOException { Map policies; try { @@ -657,6 +688,29 @@ public abstract class ESRestTestCase extends ESTestCase { } } + private static void deleteAllSLMPolicies() throws IOException { + Map policies; + + try { + Response response = adminClient().performRequest(new Request("GET", "/_slm/policy")); + policies = entityAsMap(response); + } catch (ResponseException e) { + if (RestStatus.METHOD_NOT_ALLOWED.getStatus() == e.getResponse().getStatusLine().getStatusCode()) { + // If bad request returned, SLM is not enabled. + return; + } + throw e; + } + + if (policies == null || policies.isEmpty()) { + return; + } + + for (String policyName : policies.keySet()) { + adminClient().performRequest(new Request("DELETE", "/_slm/policy/" + policyName)); + } + } + /** * Logs a message if there are still running tasks. The reasoning is that any tasks still running are state the is trying to bleed into * other tests. @@ -954,6 +1008,7 @@ public abstract class ESRestTestCase extends ESTestCase { case ".watches": case "logstash-index-template": case "security_audit_log": + case ".slm-history": return true; default: return false; diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecycleIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecycleIT.java index f03281e6669..8e8ed294c08 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecycleIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecycleIT.java @@ -42,6 +42,11 @@ import static org.hamcrest.Matchers.startsWith; public class SnapshotLifecycleIT extends ESRestTestCase { + @Override + protected boolean waitForAllSnapshotsWiped() { + return true; + } + public void testMissingRepo() throws Exception { SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("test-policy", "snap", "*/1 * * * * ?", "missing-repo", Collections.emptyMap()); @@ -115,12 +120,6 @@ public class SnapshotLifecycleIT extends ESRestTestCase { Request delReq = new Request("DELETE", "/_slm/policy/" + policyName); assertOK(client().performRequest(delReq)); - - // It's possible there could have been a snapshot in progress when the - // policy is deleted, so wait for it to be finished - assertBusy(() -> { - assertThat(wipeSnapshots().size(), equalTo(0)); - }); } @SuppressWarnings("unchecked") @@ -159,9 +158,6 @@ public class SnapshotLifecycleIT extends ESRestTestCase { } assertHistoryIsPresent(policyName, false, repoName); }); - - Request delReq = new Request("DELETE", "/_slm/policy/" + policyName); - assertOK(client().performRequest(delReq)); } public void testPolicyManualExecution() throws Exception { @@ -207,15 +203,6 @@ public class SnapshotLifecycleIT extends ESRestTestCase { } }); } - - Request delReq = new Request("DELETE", "/_slm/policy/" + policyName); - assertOK(client().performRequest(delReq)); - - // It's possible there could have been a snapshot in progress when the - // policy is deleted, so wait for it to be finished - assertBusy(() -> { - assertThat(wipeSnapshots().size(), equalTo(0)); - }); } @SuppressWarnings("unchecked") @@ -269,15 +256,6 @@ public class SnapshotLifecycleIT extends ESRestTestCase { // Cancel the snapshot since it is not going to complete quickly assertOK(client().performRequest(new Request("DELETE", "/_snapshot/" + repoId + "/" + snapshotName))); } - - Request delReq = new Request("DELETE", "/_slm/policy/" + policyName); - assertOK(client().performRequest(delReq)); - - // It's possible there could have been a snapshot in progress when the - // policy is deleted, so wait for it to be finished - assertBusy(() -> { - assertThat(wipeSnapshots().size(), equalTo(0)); - }); } @SuppressWarnings("unchecked")