From f7dafc774d9d52b3e078659b7240107c2e611386 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Tue, 11 Jun 2019 17:34:40 -0600 Subject: [PATCH 1/3] Include SLM policy name in Snapshot metadata Keep track of which SLM policy in the metadata field of the Snapshots taken by SLM. This allows users to more easily understand where the snapshot came from, and will enable future SLM features such as retention policies. --- .../create/CreateSnapshotRequest.java | 2 +- .../SnapshotLifecyclePolicy.java | 44 ++++++++++++++++- .../SnapshotLifecycleIT.java | 26 ++++++++++ .../SnapshotLifecyclePolicyTests.java | 48 +++++++++++++++++++ 4 files changed, 118 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java index 566d6863f967d..a872bef39b71e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java @@ -163,7 +163,7 @@ public ActionRequestValidationException validate() { return validationException; } - private static int metadataSize(Map userMetadata) { + public static int metadataSize(Map userMetadata) { if (userMetadata == null) { return 0; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java index 48ad91450134e..44d33dd39e74b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java @@ -30,6 +30,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -57,6 +58,7 @@ public class SnapshotLifecyclePolicy extends AbstractDiffable PARSER = @@ -169,6 +171,30 @@ public ActionRequestValidationException validate() { } } + if (configuration.containsKey("metadata")) { + if (configuration.get("metadata") instanceof Map == false) { + err.addValidationError("invalid configuration.metadata [" + configuration.get("metadata") + + "]: must be an object if present"); + } else { + @SuppressWarnings("unchecked") + Map metadata = (Map) configuration.get("metadata"); + if (metadata.containsKey(POLICY_ID_METADATA_FIELD)) { + err.addValidationError("invalid configuration.metadata: field name [" + POLICY_ID_METADATA_FIELD + + "] is reserved and will be added automatically"); + } else { + Map metadataWithPolicyField = addPolicyNameToMetadata(metadata); + int serializedSizeOriginal = CreateSnapshotRequest.metadataSize(metadata); + int serializedSizeWithMetadata = CreateSnapshotRequest.metadataSize(metadataWithPolicyField); + int policyNameAddedBytes = serializedSizeWithMetadata - serializedSizeOriginal; + if (serializedSizeWithMetadata > CreateSnapshotRequest.MAXIMUM_METADATA_BYTES) { + err.addValidationError("invalid configuration.metadata: must be smaller than [" + + (CreateSnapshotRequest.MAXIMUM_METADATA_BYTES - policyNameAddedBytes) + + "] bytes, but is [" + serializedSizeOriginal + "] bytes"); + } + } + } + } + // Repository validation, validation of whether the repository actually exists happens // elsewhere as it requires cluster state if (Strings.hasText(repository) == false) { @@ -178,6 +204,17 @@ public ActionRequestValidationException validate() { return err.validationErrors().size() == 0 ? null : err; } + private Map addPolicyNameToMetadata(final Map metadata) { + Map newMetadata; + if (metadata == null) { + newMetadata = new HashMap<>(); + } else { + newMetadata = new HashMap<>(metadata); + } + newMetadata.put(POLICY_ID_METADATA_FIELD, this.id); + return newMetadata; + } + /** * Since snapshots need to be uniquely named, this method will resolve any date math used in * the provided name, as well as appending a unique identifier so expressions that may overlap @@ -198,7 +235,12 @@ public String generateSnapshotName(Context context) { */ public CreateSnapshotRequest toRequest() { CreateSnapshotRequest req = new CreateSnapshotRequest(repository, generateSnapshotName(new ResolverContext())); - req.source(configuration); + @SuppressWarnings("unchecked") + Map metadata = (Map) configuration.get("metadata"); + Map metadataWithAddedPolicyName = addPolicyNameToMetadata(metadata); + Map mergedConfiguration = new HashMap<>(configuration); + mergedConfiguration.put("metadata", metadataWithAddedPolicyName); + req.source(mergedConfiguration); req.waitForCompletion(false); return req; } diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java index 59710aa6d8f96..1fcaced913dfa 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java @@ -85,6 +85,10 @@ public void testFullPolicySnapshot() throws Exception { Map snapResponse = ((List>) snapshotResponseMap.get("snapshots")).get(0); assertThat(snapResponse.get("snapshot").toString(), startsWith("snap-")); assertThat((List)snapResponse.get("indices"), equalTo(Collections.singletonList(indexName))); + Map metadata = (Map) snapResponse.get("metadata"); + assertNotNull(metadata); + assertThat(metadata.get("policy"), equalTo(policyName)); + assertHistoryIsPresent(policyName, true, repoId); // Check that the last success date was written to the cluster state Request getReq = new Request("GET", "/_slm/policy/" + policyName); @@ -194,6 +198,9 @@ public void testPolicyManualExecution() throws Exception { snapshotResponseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); } assertThat(snapshotResponseMap.size(), greaterThan(0)); + final Map metadata = extractMetadata(snapshotResponseMap, snapshotName); + assertNotNull(metadata); + assertThat(metadata.get("policy"), equalTo(policyName)); assertHistoryIsPresent(policyName, true, repoId); } catch (ResponseException e) { fail("expected snapshot to exist but it does not: " + EntityUtils.toString(e.getResponse().getEntity())); @@ -211,6 +218,16 @@ public void testPolicyManualExecution() throws Exception { }); } + @SuppressWarnings("unchecked") + private static Map extractMetadata(Map snapshotResponseMap, String snapshotPrefix) { + List> snapshots = ((List>) snapshotResponseMap.get("snapshots")); + return snapshots.stream() + .filter(snapshot -> ((String) snapshot.get("snapshot")).startsWith(snapshotPrefix)) + .map(snapshot -> (Map) snapshot.get("metadata")) + .findFirst() + .orElse(null); + } + // This method should be called inside an assertBusy, it has no retry logic of its own private void assertHistoryIsPresent(String policyName, boolean success, String repository) throws IOException { final Request historySearchRequest = new Request("GET", ".slm-history*/_search"); @@ -263,6 +280,15 @@ private void createSnapshotPolicy(String policyName, String snapshotNamePattern, Map snapConfig = new HashMap<>(); snapConfig.put("indices", Collections.singletonList(indexPattern)); snapConfig.put("ignore_unavailable", ignoreUnavailable); +// if (randomBoolean()) { + if (true) { + Map metadata = new HashMap<>(); + int fieldCount = randomIntBetween(2,5); + for (int i = 0; i < fieldCount; i++) { + metadata.put(randomValueOtherThanMany(key -> "policy".equals(key) || metadata.containsKey(key), + () -> randomAlphaOfLength(5)), randomAlphaOfLength(4)); + } + } SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy(policyName, snapshotNamePattern, schedule, repoId, snapConfig); Request putLifecycle = new Request("PUT", "/_slm/policy/" + policyName); diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecyclePolicyTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecyclePolicyTests.java index 190c378937a17..c2aac7120ab8a 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecyclePolicyTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecyclePolicyTests.java @@ -17,6 +17,7 @@ import java.util.HashMap; import java.util.Map; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; @@ -71,6 +72,53 @@ public void testValidation() { "invalid schedule [ ]: must not be empty")); } + public void testMetadataValidation() { + { + Map configuration = new HashMap<>(); + final String metadataString = randomAlphaOfLength(10); + configuration.put("metadata", metadataString); + + SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("mypolicy", "", + "1 * * * * ?", "myrepo", configuration); + ValidationException e = policy.validate(); + assertThat(e.validationErrors(), contains("invalid configuration.metadata [" + metadataString + + "]: must be an object if present")); + } + + { + Map metadata = new HashMap<>(); + metadata.put("policy", randomAlphaOfLength(5)); + Map configuration = new HashMap<>(); + configuration.put("metadata", metadata); + + SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("mypolicy", "", + "1 * * * * ?", "myrepo", configuration); + ValidationException e = policy.validate(); + assertThat(e.validationErrors(), contains("invalid configuration.metadata: field name [policy] is reserved and " + + "will be added automatically")); + } + + { + Map metadata = new HashMap<>(); + final int fieldCount = randomIntBetween(67, 100); // 67 is the smallest field count with these sizes that causes an error + final int keyBytes = 5; // chosen arbitrarily + final int valueBytes = 4; // chosen arbitrarily + int totalBytes = fieldCount * (keyBytes + valueBytes + 6 /* bytes of overhead per key/value pair */) + 1; + for (int i = 0; i < fieldCount; i++) { + metadata.put(randomValueOtherThanMany(key -> "policy".equals(key) || metadata.containsKey(key), + () -> randomAlphaOfLength(keyBytes)), randomAlphaOfLength(valueBytes)); + } + Map configuration = new HashMap<>(); + configuration.put("metadata", metadata); + + SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("mypolicy", "", + "1 * * * * ?", "myrepo", configuration); + ValidationException e = policy.validate(); + assertThat(e.validationErrors(), contains("invalid configuration.metadata: must be smaller than [1004] bytes, but is [" + + totalBytes + "] bytes")); + } + } + @Override protected SnapshotLifecyclePolicy doParseInstance(XContentParser parser) throws IOException { return SnapshotLifecyclePolicy.parse(parser, id); From 4e713445ee0e1b166ecef34241aa72a68f7af3f1 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Tue, 11 Jun 2019 17:43:17 -0600 Subject: [PATCH 2/3] Remove commented code --- .../xpack/snapshotlifecycle/SnapshotLifecycleIT.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java index 1fcaced913dfa..1fa0391d5987e 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java @@ -280,8 +280,7 @@ private void createSnapshotPolicy(String policyName, String snapshotNamePattern, Map snapConfig = new HashMap<>(); snapConfig.put("indices", Collections.singletonList(indexPattern)); snapConfig.put("ignore_unavailable", ignoreUnavailable); -// if (randomBoolean()) { - if (true) { + if (randomBoolean()) { Map metadata = new HashMap<>(); int fieldCount = randomIntBetween(2,5); for (int i = 0; i < fieldCount; i++) { From df716b39e9b195551da60caa4f22600f9c14f266 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Wed, 12 Jun 2019 15:16:27 -0600 Subject: [PATCH 3/3] Review feedback --- .../snapshotlifecycle/SnapshotLifecyclePolicy.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java index 44d33dd39e74b..5db1996a45982 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java @@ -59,6 +59,7 @@ public class SnapshotLifecyclePolicy extends AbstractDiffable PARSER = @@ -171,15 +172,15 @@ public ActionRequestValidationException validate() { } } - if (configuration.containsKey("metadata")) { - if (configuration.get("metadata") instanceof Map == false) { - err.addValidationError("invalid configuration.metadata [" + configuration.get("metadata") + + if (configuration.containsKey(METADATA_FIELD_NAME)) { + if (configuration.get(METADATA_FIELD_NAME) instanceof Map == false) { + err.addValidationError("invalid configuration." + METADATA_FIELD_NAME + " [" + configuration.get(METADATA_FIELD_NAME) + "]: must be an object if present"); } else { @SuppressWarnings("unchecked") - Map metadata = (Map) configuration.get("metadata"); + Map metadata = (Map) configuration.get(METADATA_FIELD_NAME); if (metadata.containsKey(POLICY_ID_METADATA_FIELD)) { - err.addValidationError("invalid configuration.metadata: field name [" + POLICY_ID_METADATA_FIELD + + err.addValidationError("invalid configuration." + METADATA_FIELD_NAME + ": field name [" + POLICY_ID_METADATA_FIELD + "] is reserved and will be added automatically"); } else { Map metadataWithPolicyField = addPolicyNameToMetadata(metadata); @@ -187,7 +188,7 @@ public ActionRequestValidationException validate() { int serializedSizeWithMetadata = CreateSnapshotRequest.metadataSize(metadataWithPolicyField); int policyNameAddedBytes = serializedSizeWithMetadata - serializedSizeOriginal; if (serializedSizeWithMetadata > CreateSnapshotRequest.MAXIMUM_METADATA_BYTES) { - err.addValidationError("invalid configuration.metadata: must be smaller than [" + + err.addValidationError("invalid configuration." + METADATA_FIELD_NAME + ": must be smaller than [" + (CreateSnapshotRequest.MAXIMUM_METADATA_BYTES - policyNameAddedBytes) + "] bytes, but is [" + serializedSizeOriginal + "] bytes"); }