From a268c957b203d2432c638b517658dbee0e42a168 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Tue, 23 Apr 2019 15:06:27 -0600 Subject: [PATCH 01/18] Record SLM history into an index It is useful to have a record of the actions that Snapshot Lifecycle Management takes, especially for the purposes of alerting when a snapshot fails or has not been taken sucessfully for a certain amount of time. This adds the infrastracture to record SLM actions into an index that can be queried at leisure, along with a lifecycle policy so that this history does not grow without bound. --- .../history/SnapshotCreationHistoryItem.java | 139 +++++++++ .../history/SnapshotHistoryItem.java | 159 +++++++++++ .../history/SnapshotHistoryStore.java | 89 ++++++ .../SnapshotLifecycleTemplateRegistry.java | 90 ++++++ .../resources/slm-history-ilm-policy.json | 10 + .../core/src/main/resources/slm-history.json | 61 ++++ .../SnapshotCreationHistoryItemTests.java | 108 +++++++ .../history/SnapshotHistoryStoreTests.java | 91 ++++++ ...napshotLifecycleTemplateRegistryTests.java | 265 ++++++++++++++++++ .../SnapshotLifecycleIT.java | 44 +++ .../xpack/indexlifecycle/IndexLifecycle.java | 18 +- .../SnapshotLifecycleTask.java | 36 ++- ...ansportExecuteSnapshotLifecycleAction.java | 7 +- .../SnapshotLifecycleServiceTests.java | 2 +- .../SnapshotLifecycleTaskTests.java | 15 +- 15 files changed, 1118 insertions(+), 16 deletions(-) create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItem.java create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java create mode 100644 x-pack/plugin/core/src/main/resources/slm-history-ilm-policy.json create mode 100644 x-pack/plugin/core/src/main/resources/slm-history.json create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItemTests.java create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItem.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItem.java new file mode 100644 index 0000000000000..340021a2315d2 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItem.java @@ -0,0 +1,139 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.snapshotlifecycle.history; + +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; +import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.json.JsonXContent; + +import java.io.IOException; +import java.util.Collections; +import java.util.Map; +import java.util.Objects; + +import static org.elasticsearch.ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE; + +public class SnapshotCreationHistoryItem extends SnapshotHistoryItem { + private static final String CREATE_OPERATION = "CREATE"; + + private final Map snapshotConfiguration; + @Nullable + private final String errorDetails; + + static final ParseField SNAPSHOT_CONFIG = new ParseField("configuration"); + static final ParseField ERROR_DETAILS = new ParseField("error_details"); + + @SuppressWarnings("unchecked") + private static final ConstructingObjectParser PARSER = + new ConstructingObjectParser<>("snapshot_lifecycle_history_item", true, + (a, id) -> { + final long timestamp = (long) a[0]; + final String policyId = (String) a[1]; + final String repository = (String) a[2]; + final String snapshotName = (String) a[3]; + final String operation = (String) a[4]; + final boolean success = (boolean) a[5]; + final Map snapshotConfiguration = (Map) a[6]; + final String errorDetails = (String) a[7]; + return new SnapshotCreationHistoryItem(timestamp, policyId, repository, snapshotName, operation, success, + snapshotConfiguration, errorDetails); + }); + + static { + PARSER.declareLong(ConstructingObjectParser.constructorArg(), TIMESTAMP); + PARSER.declareString(ConstructingObjectParser.constructorArg(), POLICY_ID); + PARSER.declareString(ConstructingObjectParser.constructorArg(), REPOSITORY); + PARSER.declareString(ConstructingObjectParser.constructorArg(), SNAPSHOT_NAME); + PARSER.declareString(ConstructingObjectParser.constructorArg(), OPERATION); + PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), SUCCESS); + PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.map(), SNAPSHOT_CONFIG); + PARSER.declareStringOrNull(ConstructingObjectParser.constructorArg(), ERROR_DETAILS); + } + + SnapshotCreationHistoryItem(long timestamp, String policyId, String repository, String snapshotName, String operation, + boolean success, Map snapshotConfiguration, String errorDetails) { + super(timestamp, policyId, repository, snapshotName, operation, success); + this.snapshotConfiguration = Objects.requireNonNull(snapshotConfiguration); + this.errorDetails = errorDetails; + } + + public static SnapshotCreationHistoryItem successRecord(long timestamp, String policyId, CreateSnapshotRequest request, + Map snapshotConfiguration) { + return new SnapshotCreationHistoryItem(timestamp, policyId, request.repository(), request.snapshot(), CREATE_OPERATION, true, + snapshotConfiguration, null); + } + + public static SnapshotCreationHistoryItem failureRecord(long timeStamp, String policyId, CreateSnapshotRequest request, + Map snapshotConfiguration, + Exception exception) throws IOException { + ToXContent.Params stacktraceParams = new ToXContent.MapParams(Collections.singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false")); + String exceptionString; + try (XContentBuilder causeXContentBuilder = JsonXContent.contentBuilder()) { + causeXContentBuilder.startObject(); + ElasticsearchException.generateThrowableXContent(causeXContentBuilder, stacktraceParams, exception); + causeXContentBuilder.endObject(); + exceptionString = BytesReference.bytes(causeXContentBuilder).utf8ToString(); + } + return new SnapshotCreationHistoryItem(timeStamp, policyId, request.repository(), request.snapshot(), CREATE_OPERATION, false, + snapshotConfiguration, exceptionString); + } + + public SnapshotCreationHistoryItem(StreamInput in) throws IOException { + super(in); + this.snapshotConfiguration = in.readMap(); + this.errorDetails = in.readOptionalString(); + } + + public Map getSnapshotConfiguration() { + return snapshotConfiguration; + } + + public String getErrorDetails() { + return errorDetails; + } + + @Override + protected void innerWriteTo(StreamOutput out) throws IOException { + out.writeMap(snapshotConfiguration); + out.writeOptionalString(errorDetails); + } + + @Override + protected XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException { + builder.field(SNAPSHOT_CONFIG.getPreferredName(), snapshotConfiguration); + builder.field(ERROR_DETAILS.getPreferredName(), errorDetails); + return builder; + } + + public static SnapshotCreationHistoryItem parse(XContentParser parser, String name) { + return PARSER.apply(parser, name); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + SnapshotCreationHistoryItem that = (SnapshotCreationHistoryItem) o; + return Objects.equals(getSnapshotConfiguration(), that.getSnapshotConfiguration()) && + Objects.equals(getErrorDetails(), that.getErrorDetails()); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), getSnapshotConfiguration(), getErrorDetails()); + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java new file mode 100644 index 0000000000000..6fad844bd25d6 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java @@ -0,0 +1,159 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.snapshotlifecycle.history; + +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.Objects; + +/** + * Represents an entry in the Snapshot Lifecycle Management history index. Subclass this to add fields which are specific to a type of + * operation (e.g. creation, deletion, any others). + * + * In addition to the abstract methods that subclasses must implement, subclasses should also implement: + * 1) a constructor that takes a {@link StreamInput} for deserialization. Remember to call {@code super(in)} before reading any new fields. + * 2) an xContent parser using {@link org.elasticsearch.common.xcontent.ConstructingObjectParser}. This will need to parse the fields that + * are part of {@link SnapshotHistoryItem} as well as new fields. + */ +public abstract class SnapshotHistoryItem implements Writeable, ToXContentObject { + + protected final long timestamp; + protected final String policyId; + protected final String repository; + protected final String snapshotName; + protected final String operation; + protected final boolean success; + + static final ParseField TIMESTAMP = new ParseField("@timestamp"); + static final ParseField POLICY_ID = new ParseField("policy"); + static final ParseField REPOSITORY = new ParseField("repository"); + static final ParseField SNAPSHOT_NAME = new ParseField("snapshot_name"); + static final ParseField OPERATION = new ParseField("operation"); + static final ParseField SUCCESS = new ParseField("success"); + + public SnapshotHistoryItem(long timestamp, String policyId, String repository, String snapshotName, String operation, boolean success) { + this.timestamp = timestamp; + this.policyId = Objects.requireNonNull(policyId); + this.repository = Objects.requireNonNull(repository); + this.snapshotName = Objects.requireNonNull(snapshotName); + this.operation = Objects.requireNonNull(operation); + this.success = success; + } + + public SnapshotHistoryItem(StreamInput in) throws IOException { + this.timestamp = in.readVLong(); + this.policyId = in.readString(); + this.repository = in.readString(); + this.snapshotName = in.readString(); + this.operation = in.readString(); + this.success = in.readBoolean(); + } + + public long getTimestamp() { + return timestamp; + } + + public String getPolicyId() { + return policyId; + } + + public String getRepository() { + return repository; + } + + public String getSnapshotName() { + return snapshotName; + } + + public String getOperation() { + return operation; + } + + public boolean isSuccess() { + return success; + } + + @Override + public final void writeTo(StreamOutput out) throws IOException { + out.writeVLong(timestamp); + out.writeString(policyId); + out.writeString(repository); + out.writeString(snapshotName); + out.writeString(operation); + out.writeBoolean(success); + innerWriteTo(out); + } + + @Override + public final XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + { + builder.timeField(TIMESTAMP.getPreferredName(), "timestamp_string", timestamp); + builder.field(POLICY_ID.getPreferredName(), policyId); + builder.field(REPOSITORY.getPreferredName(), repository); + builder.field(SNAPSHOT_NAME.getPreferredName(), snapshotName); + builder.field(OPERATION.getPreferredName(), operation); + builder.field(SUCCESS.getPreferredName(), success); + innerToXContent(builder, params); + } + builder.endObject(); + + return builder; + } + + @Override + public String toString() { + return Strings.toString(this); + } + + /** + * Write any fields that are introduced in the subclass here. This is called as part of + * {@link SnapshotHistoryItem#writeTo(StreamOutput)}, you do not need to (and should not) override that method. All fields that are + * part of the {@link SnapshotHistoryItem} class will already be written to the stream, so only write new fields introduced in the + * subclass. + * @param out The output stream + * @throws IOException if an error occurs while writing to the output stream + */ + protected abstract void innerWriteTo(StreamOutput out) throws IOException; + + /** + * Write any fields that are introduced in the subclass here. This is called as part of + * {@link SnapshotHistoryItem#toXContent(XContentBuilder, Params)}, you do not need to (and should not) override that method. All fields + * that are part of the {@link SnapshotHistoryItem} class will already be written to the stream, so only write new fields introduced in + * the subclass. + * @param builder The xContent builder + * @param params Parameters + * @return The XContent builder + * @throws IOException if an error occurs while writing to the builder + */ + protected abstract XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException; + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SnapshotHistoryItem that = (SnapshotHistoryItem) o; + return isSuccess() == that.isSuccess() && + timestamp == that.getTimestamp() && + Objects.equals(getPolicyId(), that.getPolicyId()) && + Objects.equals(getRepository(), that.getRepository()) && + Objects.equals(getSnapshotName(), that.getSnapshotName()) && + Objects.equals(getOperation(), that.getOperation()); + } + + @Override + public int hashCode() { + return Objects.hash(getTimestamp(), getPolicyId(), getRepository(), getSnapshotName(), getOperation(), isSuccess()); + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java new file mode 100644 index 0000000000000..0e9964b6a6a8c --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java @@ -0,0 +1,89 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.snapshotlifecycle.history; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.ClusterChangedEvent; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateListener; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.time.DateFormatter; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; + +import java.io.IOException; +import java.time.Instant; +import java.time.ZoneId; +import java.time.ZonedDateTime; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.INDEX_TEMPLATE_VERSION; + +public class SnapshotHistoryStore implements ClusterStateListener { + private static final Logger logger = LogManager.getLogger(SnapshotHistoryStore.class); + private static final DateFormatter indexTimeFormat = DateFormatter.forPattern("yyyy.MM"); + + public static final String SLM_HISTORY_INDEX_PREFIX = ".slm-history-" + INDEX_TEMPLATE_VERSION + "-"; + + private final SnapshotLifecycleTemplateRegistry registry; + private final Client client; + private final ZoneId timeZone; + private final AtomicBoolean readyToIndex = new AtomicBoolean(false); + + public SnapshotHistoryStore(SnapshotLifecycleTemplateRegistry registry, Client client, ZoneId timeZone, + ClusterService clusterService) { + this.registry = registry; + this.client = client; + this.timeZone = timeZone; + clusterService.addListener(this); + } + + /** + * Attempts to asynchronously index a snapshot lifecycle management history entry + * + * @param item The entry to index + */ + public void putAsync(SnapshotHistoryItem item) { + final ZonedDateTime dateTime = ZonedDateTime.ofInstant(Instant.ofEpochMilli(item.getTimestamp()), timeZone); + final String index = getHistoryIndexNameForTime(dateTime); + logger.trace("about to index snapshot history item in index [{}]: [{}]", index, item); + try (XContentBuilder builder = XContentFactory.jsonBuilder()) { + item.toXContent(builder, ToXContent.EMPTY_PARAMS); + IndexRequest request = new IndexRequest(index) + .source(builder); + client.index(request, ActionListener.wrap(indexResponse -> { + logger.debug("successfully indexed snapshot history item with id [{}] in index [{}]: [{}]", + indexResponse.getId(), index, item); + }, exception -> { + logger.error(new ParameterizedMessage("failed to index snapshot history item in index [{}]: [{}]", + index, item), exception); + })); + } catch (IOException exception) { + logger.error(new ParameterizedMessage("failed to index snapshot history item in index [{}]: [{}]", + index, item), exception); + } + } + + boolean validate(ClusterState state) { + return registry.validate(state); + } + + static String getHistoryIndexNameForTime(ZonedDateTime time) { + return SLM_HISTORY_INDEX_PREFIX + indexTimeFormat.format(time); + } + + @Override + public void clusterChanged(ClusterChangedEvent event) { + readyToIndex.set(validate(event.state())); + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java new file mode 100644 index 0000000000000..b46379cb058e9 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java @@ -0,0 +1,90 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.snapshotlifecycle.history; + +import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.indexlifecycle.IndexLifecycleMetadata; +import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicy; +import org.elasticsearch.xpack.core.template.IndexTemplateConfig; +import org.elasticsearch.xpack.core.template.IndexTemplateRegistry; +import org.elasticsearch.xpack.core.template.LifecyclePolicyConfig; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.elasticsearch.xpack.core.ClientHelper.INDEX_LIFECYCLE_ORIGIN; + +public class SnapshotLifecycleTemplateRegistry extends IndexTemplateRegistry { + // history (please add a comment why you increased the version here) + // version 1: initial + public static final String INDEX_TEMPLATE_VERSION = "1"; + + public static final String SLM_TEMPLATE_VERSION_VARIABLE = "xpack.slm.template.version"; + public static final String SLM_TEMPLATE_NAME = ".slm-history"; + + public static final String SLM_POLICY_NAME = "slm-history-ilm-policy"; + + public static final IndexTemplateConfig TEMPLATE_SLM_HISTORY = new IndexTemplateConfig( + SLM_TEMPLATE_NAME, + "/slm-history.json", + INDEX_TEMPLATE_VERSION, + SLM_TEMPLATE_VERSION_VARIABLE + ); + + public static final LifecyclePolicyConfig SLM_HISTORY_POLICY = new LifecyclePolicyConfig( + SLM_POLICY_NAME, + "/slm-history-ilm-policy.json" + ); + + public SnapshotLifecycleTemplateRegistry(Settings nodeSettings, ClusterService clusterService, ThreadPool threadPool, Client client, + NamedXContentRegistry xContentRegistry) { + super(nodeSettings, clusterService, threadPool, client, xContentRegistry); + } + + @Override + protected List getTemplateConfigs() { + return Collections.singletonList(TEMPLATE_SLM_HISTORY); + } + + @Override + protected List getPolicyConfigs() { + return Collections.singletonList(SLM_HISTORY_POLICY); + } + + @Override + protected String getOrigin() { + return INDEX_LIFECYCLE_ORIGIN; // TODO use separate SLM origin? + } + + public boolean validate(ClusterState state) { + boolean allTemplatesPresent = getTemplateConfigs().stream() + .map(IndexTemplateConfig::getTemplateName) + .allMatch(name -> state.metaData().getTemplates().containsKey(name)); + + Optional> maybePolicies = Optional + .ofNullable(state.metaData().custom(IndexLifecycleMetadata.TYPE)) + .map(IndexLifecycleMetadata::getPolicies); + Set policyNames = getPolicyConfigs().stream() + .map(LifecyclePolicyConfig::getPolicyName) + .collect(Collectors.toSet()); + + boolean allPoliciesPresent = maybePolicies + .map(policies -> policies.keySet() + .containsAll(policyNames)) + .orElse(false); + return allTemplatesPresent && allPoliciesPresent; + } +} diff --git a/x-pack/plugin/core/src/main/resources/slm-history-ilm-policy.json b/x-pack/plugin/core/src/main/resources/slm-history-ilm-policy.json new file mode 100644 index 0000000000000..e45e6b25e8f7b --- /dev/null +++ b/x-pack/plugin/core/src/main/resources/slm-history-ilm-policy.json @@ -0,0 +1,10 @@ +{ + "phases": { + "delete": { + "min_age": "7d", + "actions": { + "delete": {} + } + } + } +} diff --git a/x-pack/plugin/core/src/main/resources/slm-history.json b/x-pack/plugin/core/src/main/resources/slm-history.json new file mode 100644 index 0000000000000..429105f94862a --- /dev/null +++ b/x-pack/plugin/core/src/main/resources/slm-history.json @@ -0,0 +1,61 @@ +{ + "index_patterns": [ + ".slm-history-${xpack.slm.template.version}*" + ], + "order": 2147483647, + "settings": { + "index.number_of_shards": 1, + "index.number_of_replicas": 0, + "index.auto_expand_replicas": "0-1", + "index.lifecycle.name": "slm-history-ilm-policy", + "index.format": 1 + }, + "mappings": { + "_doc": { + "_meta": { + "slm-history-version": "${xpack.slm.template.version}" + }, + "dynamic": false, + "properties": { + "@timestamp": { + "type": "date", + "format": "epoch_millis" + }, + "policy": { + "type": "keyword" + }, + "repository": { + "type": "keyword" + }, + "snapshot_name":{ + "type": "keyword" + }, + "operation": { + "type": "keyword" + }, + "success": { + "type": "boolean" + }, + "configuration": { + "type": "object", + "dynamic": false, + "properties": { + "indices": { + "type": "keyword" + }, + "partial": { + "type": "boolean" + }, + "include_global_state": { + "type": "boolean" + } + } + }, + "error_details": { + "type": "text", + "index": false + } + } + } + } +} \ No newline at end of file diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItemTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItemTests.java new file mode 100644 index 0000000000000..3d9b46aeed372 --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItemTests.java @@ -0,0 +1,108 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.snapshotlifecycle.history; + +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.AbstractSerializingTestCase; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; + +public class SnapshotCreationHistoryItemTests extends AbstractSerializingTestCase { + + @Override + protected SnapshotCreationHistoryItem doParseInstance(XContentParser parser) throws IOException { + return SnapshotCreationHistoryItem.parse(parser, this.getClass().getCanonicalName()); + } + + @Override + protected Writeable.Reader instanceReader() { + return SnapshotCreationHistoryItem::new; + } + + @Override + protected SnapshotCreationHistoryItem createTestInstance() { + long timestamp = randomNonNegativeLong(); + String policyId = randomAlphaOfLengthBetween(5, 10); + String repository = randomAlphaOfLengthBetween(5, 10); + String snapshotName = randomAlphaOfLengthBetween(5, 10); + String operation = randomAlphaOfLengthBetween(5, 10); + boolean success = randomBoolean(); + Map snapshotConfig = randomSnapshotConfiguration(); + String errorDetails = randomBoolean() ? null : randomAlphaOfLengthBetween(10, 20); + + return new SnapshotCreationHistoryItem(timestamp, policyId, repository, snapshotName, operation, success, snapshotConfig, + errorDetails); + } + + @Override + protected SnapshotCreationHistoryItem mutateInstance(SnapshotCreationHistoryItem instance) { + final int branch = between(0, 7); + switch (branch) { + case 0: // New timestamp + return new SnapshotCreationHistoryItem( + randomValueOtherThan(instance.getTimestamp(), ESTestCase::randomNonNegativeLong), + instance.getPolicyId(), instance.getRepository(), instance.getSnapshotName(), instance.getOperation(), + instance.isSuccess(), instance.getSnapshotConfiguration(), instance.getErrorDetails()); + case 1: // new policyId + return new SnapshotCreationHistoryItem(instance.getTimestamp(), + randomValueOtherThan(instance.getPolicyId(), () -> randomAlphaOfLengthBetween(5, 10)), + instance.getSnapshotName(), instance.getRepository(), instance.getOperation(), instance.isSuccess(), + instance.getSnapshotConfiguration(), instance.getErrorDetails()); + case 2: // new repo name + return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getSnapshotName(), + randomValueOtherThan(instance.getRepository(), () -> randomAlphaOfLengthBetween(5, 10)), + instance.getOperation(), instance.isSuccess(), instance.getSnapshotConfiguration(), instance.getErrorDetails()); + case 3: + return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), + randomValueOtherThan(instance.getSnapshotName(), () -> randomAlphaOfLengthBetween(5, 10)), + instance.getOperation(), instance.isSuccess(), instance.getSnapshotConfiguration(), instance.getErrorDetails()); + case 4: + return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), + instance.getSnapshotName(), + randomValueOtherThan(instance.getOperation(), () -> randomAlphaOfLengthBetween(5, 10)), + instance.isSuccess(), instance.getSnapshotConfiguration(), instance.getErrorDetails()); + case 5: + return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), + instance.getSnapshotName(), + instance.getOperation(), + instance.isSuccess() == false, + instance.getSnapshotConfiguration(), instance.getErrorDetails()); + case 6: + return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), + instance.getSnapshotName(), instance.getOperation(), instance.isSuccess(), + randomValueOtherThan(instance.getSnapshotConfiguration(), + SnapshotCreationHistoryItemTests::randomSnapshotConfiguration), + instance.getErrorDetails()); + case 7: + return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), + instance.getSnapshotName(), instance.getOperation(), instance.isSuccess(), instance.getSnapshotConfiguration(), + randomValueOtherThan(instance.getErrorDetails(), () -> randomAlphaOfLengthBetween(10, 20))); + default: + throw new IllegalArgumentException("illegal randomization: " + branch); + } + } + + public static Map randomSnapshotConfiguration() { + Map configuration = new HashMap<>(); + configuration.put("indices", Arrays.asList(generateRandomStringArray(1, 10, false, false))); + if (frequently()) { + configuration.put("ignore_unavailable", randomBoolean()); + } + if (frequently()) { + configuration.put("include_global_state", randomBoolean()); + } + if (frequently()) { + configuration.put("partial", randomBoolean()); + } + return configuration; + } +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java new file mode 100644 index 0000000000000..272d48de652cb --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java @@ -0,0 +1,91 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.snapshotlifecycle.history; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; +import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.junit.Before; +import org.mockito.ArgumentCaptor; + +import java.time.Instant; +import java.time.ZoneOffset; +import java.util.Map; + +import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotCreationHistoryItemTests.randomSnapshotConfiguration; +import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore.getHistoryIndexNameForTime; +import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.INDEX_TEMPLATE_VERSION; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.core.IsEqual.equalTo; +import static org.mockito.Matchers.notNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class SnapshotHistoryStoreTests extends ESTestCase { + + private ClusterService clusterService; + private Client client; + private SnapshotHistoryStore historyStore; + private SnapshotLifecycleTemplateRegistry registry; + + @Before + public void setup() { + Settings settings = Settings.builder().put("node.name", randomAlphaOfLength(10)).build(); + client = mock(Client.class); + ThreadPool threadPool = mock(ThreadPool.class); + when(client.threadPool()).thenReturn(threadPool); + when(client.settings()).thenReturn(settings); + when(threadPool.getThreadContext()).thenReturn(new ThreadContext(settings)); + clusterService = mock(ClusterService.class); + registry = mock(SnapshotLifecycleTemplateRegistry.class); + + historyStore = new SnapshotHistoryStore(registry, client, ZoneOffset.UTC, clusterService); + } + + @SuppressWarnings("unchecked") + public void testPut() { + String policyId = randomAlphaOfLength(5); + String repository = randomAlphaOfLength(6); + String snapshotId = randomAlphaOfLength(7); + CreateSnapshotRequest request = new CreateSnapshotRequest(repository, snapshotId); + Map config = randomBoolean() ? null : randomSnapshotConfiguration(); + final long timestamp = randomNonNegativeLong(); + SnapshotCreationHistoryItem record = SnapshotCreationHistoryItem.successRecord(timestamp, policyId, request, config); + + historyStore.putAsync(record); + ArgumentCaptor indexRequest = ArgumentCaptor.forClass(IndexRequest.class); + verify(client, times(1)).index(indexRequest.capture(), notNull(ActionListener.class)); + + assertEquals(getHistoryIndexNameForTime(Instant.ofEpochMilli(timestamp).atZone(ZoneOffset.UTC)), + indexRequest.getValue().index()); + final String indexedDocument = indexRequest.getValue().source().utf8ToString(); + assertThat(indexedDocument, containsString(policyId)); + assertThat(indexedDocument, containsString(repository)); + assertThat(indexedDocument, containsString(snapshotId)); + } + + + public void testIndexNameGeneration() { + String indexTemplateVersion = INDEX_TEMPLATE_VERSION; + assertThat(getHistoryIndexNameForTime(Instant.ofEpochMilli((long) 0).atZone(ZoneOffset.UTC)), + equalTo(".slm-history-"+ indexTemplateVersion +"-1970.01")); + assertThat(getHistoryIndexNameForTime(Instant.ofEpochMilli(100000000000L).atZone(ZoneOffset.UTC)), + equalTo(".slm-history-" + indexTemplateVersion + "-1973.03")); + assertThat(getHistoryIndexNameForTime(Instant.ofEpochMilli(1416582852000L).atZone(ZoneOffset.UTC)), + equalTo(".slm-history-" + indexTemplateVersion + "-2014.11")); + assertThat(getHistoryIndexNameForTime(Instant.ofEpochMilli(2833165811000L).atZone(ZoneOffset.UTC)), + equalTo(".slm-history-" + indexTemplateVersion + "-2059.10")); + } +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java new file mode 100644 index 0000000000000..99f7f8c2af5a0 --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java @@ -0,0 +1,265 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.snapshotlifecycle.history; + +import org.elasticsearch.Version; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequest; +import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.client.AdminClient; +import org.elasticsearch.client.Client; +import org.elasticsearch.client.IndicesAdminClient; +import org.elasticsearch.cluster.ClusterChangedEvent; +import org.elasticsearch.cluster.ClusterModule; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.block.ClusterBlocks; +import org.elasticsearch.cluster.metadata.IndexTemplateMetaData; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.EsExecutors; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.indexlifecycle.DeleteAction; +import org.elasticsearch.xpack.core.indexlifecycle.IndexLifecycleMetadata; +import org.elasticsearch.xpack.core.indexlifecycle.LifecycleAction; +import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicy; +import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicyMetadata; +import org.elasticsearch.xpack.core.indexlifecycle.LifecycleType; +import org.elasticsearch.xpack.core.indexlifecycle.OperationMode; +import org.elasticsearch.xpack.core.indexlifecycle.TimeseriesLifecycleType; +import org.elasticsearch.xpack.core.indexlifecycle.action.PutLifecycleAction; +import org.junit.Before; +import org.mockito.ArgumentCaptor; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static org.elasticsearch.mock.orig.Mockito.verify; +import static org.elasticsearch.mock.orig.Mockito.when; +import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.SLM_POLICY_NAME; +import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.SLM_TEMPLATE_NAME; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyObject; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verifyZeroInteractions; + +public class SnapshotLifecycleTemplateRegistryTests extends ESTestCase { + private SnapshotLifecycleTemplateRegistry registry; + private NamedXContentRegistry xContentRegistry; + private ClusterService clusterService; + private ThreadPool threadPool; + private Client client; + + @Before + public void createRegistryAndClient() { + threadPool = mock(ThreadPool.class); + when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); + when(threadPool.generic()).thenReturn(EsExecutors.newDirectExecutorService()); + + client = mock(Client.class); + when(client.threadPool()).thenReturn(threadPool); + AdminClient adminClient = mock(AdminClient.class); + IndicesAdminClient indicesAdminClient = mock(IndicesAdminClient.class); + when(adminClient.indices()).thenReturn(indicesAdminClient); + when(client.admin()).thenReturn(adminClient); + doAnswer(invocationOnMock -> { + ActionListener listener = + (ActionListener) invocationOnMock.getArguments()[1]; + listener.onResponse(new TestPutIndexTemplateResponse(true)); + return null; + }).when(indicesAdminClient).putTemplate(any(PutIndexTemplateRequest.class), any(ActionListener.class)); + + clusterService = mock(ClusterService.class); + List entries = new ArrayList<>(ClusterModule.getNamedXWriteables()); + entries.addAll(Arrays.asList( + new NamedXContentRegistry.Entry(LifecycleType.class, new ParseField(TimeseriesLifecycleType.TYPE), + (p) -> TimeseriesLifecycleType.INSTANCE), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(DeleteAction.NAME), DeleteAction::parse))); + xContentRegistry = new NamedXContentRegistry(entries); + registry = new SnapshotLifecycleTemplateRegistry(Settings.EMPTY, clusterService, threadPool, client, xContentRegistry); + } + + public void testThatNonExistingTemplatesAreAddedImmediately() { + DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); + DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); + + ClusterChangedEvent event = createClusterChangedEvent(Collections.emptyList(), nodes); + registry.clusterChanged(event); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(PutIndexTemplateRequest.class); + verify(client.admin().indices(), times(registry.getTemplateConfigs().size())).putTemplate(argumentCaptor.capture(), anyObject()); + + // now delete one template from the cluster state and lets retry + ClusterChangedEvent newEvent = createClusterChangedEvent(Collections.emptyList(), nodes); + registry.clusterChanged(newEvent); + ArgumentCaptor captor = ArgumentCaptor.forClass(PutIndexTemplateRequest.class); + verify(client.admin().indices(), times(registry.getTemplateConfigs().size() + 1)).putTemplate(captor.capture(), anyObject()); + PutIndexTemplateRequest req = captor.getAllValues().stream() + .filter(r -> r.name().equals(SLM_TEMPLATE_NAME)) + .findFirst() + .orElseThrow(() -> new AssertionError("expected the slm history template to be put")); + assertThat(req.settings().get("index.lifecycle.name"), equalTo(SLM_POLICY_NAME)); + } + + public void testThatNonExistingPoliciesAreAddedImmediately() { + DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); + DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); + + ClusterChangedEvent event = createClusterChangedEvent(Collections.emptyList(), nodes); + registry.clusterChanged(event); + verify(client, times(registry.getPolicyConfigs().size())).execute(eq(PutLifecycleAction.INSTANCE), anyObject(), anyObject()); + } + + public void testPolicyAlreadyExists() { + DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); + DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); + + Map policyMap = new HashMap<>(); + List policies = registry.getPolicyConfigs().stream() + .map(policyConfig -> policyConfig.load(xContentRegistry)) + .collect(Collectors.toList()); + assertThat(policies, hasSize(1)); + LifecyclePolicy policy = policies.get(0); + policyMap.put(policy.getName(), policy); + ClusterChangedEvent event = createClusterChangedEvent(Collections.emptyList(), policyMap, nodes); + registry.clusterChanged(event); + verify(client, times(0)).execute(eq(PutLifecycleAction.INSTANCE), anyObject(), anyObject()); + } + + public void testThatTemplatesExist() throws IOException { + DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); + DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); + + Map policyMap = new HashMap<>(); + String policyStr = "{\"phases\":{\"delete\":{\"min_age\":\"1m\",\"actions\":{\"delete\":{}}}}}"; + List policies = registry.getPolicyConfigs().stream() + .map(policyConfig -> policyConfig.load(xContentRegistry)) + .collect(Collectors.toList()); + assertThat(policies, hasSize(1)); + LifecyclePolicy policy = policies.get(0); + try (XContentParser parser = XContentType.JSON.xContent() + .createParser(xContentRegistry, LoggingDeprecationHandler.THROW_UNSUPPORTED_OPERATION, policyStr)) { + LifecyclePolicy different = LifecyclePolicy.parse(parser, policy.getName()); + policyMap.put(policy.getName(), different); + ClusterChangedEvent event = createClusterChangedEvent(Collections.emptyList(), policyMap, nodes); + registry.clusterChanged(event); + verify(client, times(0)).execute(eq(PutLifecycleAction.INSTANCE), anyObject(), anyObject()); + } + } + + public void testThatMissingMasterNodeDoesNothing() { + DiscoveryNode localNode = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); + DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").add(localNode).build(); + + ClusterChangedEvent event = createClusterChangedEvent(Arrays.asList(SLM_TEMPLATE_NAME), nodes); + registry.clusterChanged(event); + + verifyZeroInteractions(client); + } + + public void testPolicyAlreadyExistsButDiffers() throws IOException { + DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); + DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); + + Map policyMap = new HashMap<>(); + String policyStr = "{\"phases\":{\"delete\":{\"min_age\":\"1m\",\"actions\":{\"delete\":{}}}}}"; + List policies = registry.getPolicyConfigs().stream() + .map(policyConfig -> policyConfig.load(xContentRegistry)) + .collect(Collectors.toList()); + assertThat(policies, hasSize(1)); + LifecyclePolicy policy = policies.get(0); + try (XContentParser parser = XContentType.JSON.xContent() + .createParser(xContentRegistry, LoggingDeprecationHandler.THROW_UNSUPPORTED_OPERATION, policyStr)) { + LifecyclePolicy different = LifecyclePolicy.parse(parser, policy.getName()); + policyMap.put(policy.getName(), different); + ClusterChangedEvent event = createClusterChangedEvent(Collections.emptyList(), policyMap, nodes); + registry.clusterChanged(event); + verify(client, times(0)).execute(eq(PutLifecycleAction.INSTANCE), anyObject(), anyObject()); + } + } + + public void testValidate() { + assertFalse(registry.validate(createClusterState(Settings.EMPTY, Collections.emptyList(), Collections.emptyMap(), null))); + assertFalse(registry.validate(createClusterState(Settings.EMPTY, List.of(SLM_TEMPLATE_NAME), Collections.emptyMap(), null))); + + Map policyMap = new HashMap<>(); + policyMap.put(SLM_POLICY_NAME, new LifecyclePolicy(SLM_POLICY_NAME, new HashMap<>())); + assertFalse(registry.validate(createClusterState(Settings.EMPTY, Collections.emptyList(), policyMap, null))); + + assertTrue(registry.validate(createClusterState(Settings.EMPTY, List.of(SLM_TEMPLATE_NAME), policyMap, null))); + } + + // ------------- + + private ClusterChangedEvent createClusterChangedEvent(List existingTemplateNames, DiscoveryNodes nodes) { + return createClusterChangedEvent(existingTemplateNames, Collections.emptyMap(), nodes); + } + + private ClusterChangedEvent createClusterChangedEvent(List existingTemplateNames, + Map existingPolicies, + DiscoveryNodes nodes) { + ClusterState cs = createClusterState(Settings.EMPTY, existingTemplateNames, existingPolicies, nodes); + ClusterChangedEvent realEvent = new ClusterChangedEvent("created-from-test", cs, + ClusterState.builder(new ClusterName("test")).build()); + ClusterChangedEvent event = spy(realEvent); + when(event.localNodeMaster()).thenReturn(nodes.isLocalNodeElectedMaster()); + + return event; + } + + private ClusterState createClusterState(Settings nodeSettings, + List existingTemplateNames, + Map existingPolicies, + DiscoveryNodes nodes) { + ImmutableOpenMap.Builder indexTemplates = ImmutableOpenMap.builder(); + for (String name : existingTemplateNames) { + indexTemplates.put(name, mock(IndexTemplateMetaData.class)); + } + + Map existingILMMeta = existingPolicies.entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, e -> new LifecyclePolicyMetadata(e.getValue(), Collections.emptyMap(), 1, 1))); + IndexLifecycleMetadata ilmMeta = new IndexLifecycleMetadata(existingILMMeta, OperationMode.RUNNING); + + return ClusterState.builder(new ClusterName("test")) + .metaData(MetaData.builder() + .templates(indexTemplates.build()) + .transientSettings(nodeSettings) + .putCustom(IndexLifecycleMetadata.TYPE, ilmMeta) + .build()) + .blocks(new ClusterBlocks.Builder().build()) + .nodes(nodes) + .build(); + } + + private static class TestPutIndexTemplateResponse extends AcknowledgedResponse { + TestPutIndexTemplateResponse(boolean acknowledged) { + super(acknowledged); + } + } +} 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 57dbe7d77fac3..e4e5d92a899a8 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 @@ -36,6 +36,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.startsWith; public class SnapshotLifecycleIT extends ESRestTestCase { @@ -103,6 +104,8 @@ public void testFullPolicySnapshot() throws Exception { String lastSnapshotName = (String) lastSuccessObject.get("snapshot_name"); assertThat(lastSnapshotName, startsWith("snap-")); + + assertHistoryIsPresent(policyName, true, repoId); }); Request delReq = new Request("DELETE", "/_slm/policy/" + policyName); @@ -149,6 +152,7 @@ public void testPolicyFailure() throws Exception { assertNotNull(snapshotName); assertThat(snapshotName, startsWith("snap-")); } + assertHistoryIsPresent(policyName, false, repoName); }); Request delReq = new Request("DELETE", "/_slm/policy/" + policyName); @@ -190,6 +194,7 @@ public void testPolicyManualExecution() throws Exception { snapshotResponseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); } assertThat(snapshotResponseMap.size(), greaterThan(0)); + assertHistoryIsPresent(policyName, true, repoId); } catch (ResponseException e) { fail("expected snapshot to exist but it does not: " + EntityUtils.toString(e.getResponse().getEntity())); } @@ -206,6 +211,45 @@ public void testPolicyManualExecution() throws Exception { }); } + private void assertHistoryIsPresent(String policyName, boolean success, String repository) throws IOException { + final Request historySearchRequest = new Request("GET", ".slm-history*/_search"); + historySearchRequest.setJsonEntity("{\n" + + " \"query\": {\n" + + " \"bool\": {\n" + + " \"must\": [\n" + + " {\n" + + " \"term\": {\n" + + " \"policy\": \"" + policyName + "\"\n" + + " }\n" + + " },\n" + + " {\n" + + " \"term\": {\n" + + " \"success\": " + success + "\n" + + " }\n" + + " },\n" + + " {\n" + + " \"term\": {\n" + + " \"repository\": \"" + repository + "\"\n" + + " }\n" + + " },\n" + + " {\n" + + " \"term\": {\n" + + " \"operation\": \"CREATE\"\n" + + " }\n" + + " }\n" + + " ]\n" + + " }\n" + + " }\n" + + "}"); + Response historyResponse = client().performRequest(historySearchRequest); + Map historyResponseMap; + try (InputStream is = historyResponse.getEntity().getContent()) { + historyResponseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); + } + assertThat((int)((Map) ((Map) historyResponseMap.get("hits")).get("total")).get("value"), + greaterThanOrEqualTo(1)); + } + private void createSnapshotPolicy(String policyName, String snapshotNamePattern, String schedule, String repoId, String indexPattern, boolean ignoreUnavailable) throws IOException { Map snapConfig = new HashMap<>(); diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java index 1136969ae2d38..1ccefbcedd978 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java @@ -60,7 +60,13 @@ import org.elasticsearch.xpack.core.indexlifecycle.action.RetryAction; import org.elasticsearch.xpack.core.indexlifecycle.action.StartILMAction; import org.elasticsearch.xpack.core.indexlifecycle.action.StopILMAction; +import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecycleMetadata; +import org.elasticsearch.xpack.core.snapshotlifecycle.action.DeleteSnapshotLifecycleAction; import org.elasticsearch.xpack.core.snapshotlifecycle.action.ExecuteSnapshotLifecycleAction; +import org.elasticsearch.xpack.core.snapshotlifecycle.action.GetSnapshotLifecycleAction; +import org.elasticsearch.xpack.core.snapshotlifecycle.action.PutSnapshotLifecycleAction; +import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore; +import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry; import org.elasticsearch.xpack.indexlifecycle.action.RestDeleteLifecycleAction; import org.elasticsearch.xpack.indexlifecycle.action.RestExplainLifecycleAction; import org.elasticsearch.xpack.indexlifecycle.action.RestGetLifecycleAction; @@ -81,12 +87,8 @@ import org.elasticsearch.xpack.indexlifecycle.action.TransportRetryAction; import org.elasticsearch.xpack.indexlifecycle.action.TransportStartILMAction; import org.elasticsearch.xpack.indexlifecycle.action.TransportStopILMAction; -import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecycleMetadata; import org.elasticsearch.xpack.snapshotlifecycle.SnapshotLifecycleService; import org.elasticsearch.xpack.snapshotlifecycle.SnapshotLifecycleTask; -import org.elasticsearch.xpack.core.snapshotlifecycle.action.DeleteSnapshotLifecycleAction; -import org.elasticsearch.xpack.core.snapshotlifecycle.action.GetSnapshotLifecycleAction; -import org.elasticsearch.xpack.core.snapshotlifecycle.action.PutSnapshotLifecycleAction; import org.elasticsearch.xpack.snapshotlifecycle.action.RestDeleteSnapshotLifecycleAction; import org.elasticsearch.xpack.snapshotlifecycle.action.RestExecuteSnapshotLifecycleAction; import org.elasticsearch.xpack.snapshotlifecycle.action.RestGetSnapshotLifecycleAction; @@ -110,6 +112,7 @@ public class IndexLifecycle extends Plugin implements ActionPlugin { private final SetOnce indexLifecycleInitialisationService = new SetOnce<>(); private final SetOnce snapshotLifecycleService = new SetOnce<>(); + private final SetOnce snapshotHistoryStore = new SetOnce<>(); private Settings settings; private boolean enabled; private boolean transportClientMode; @@ -156,9 +159,12 @@ public Collection createComponents(Client client, ClusterService cluster } indexLifecycleInitialisationService.set(new IndexLifecycleService(settings, client, clusterService, threadPool, getClock(), System::currentTimeMillis, xContentRegistry)); + SnapshotLifecycleTemplateRegistry templateRegistry = new SnapshotLifecycleTemplateRegistry(settings, clusterService, threadPool, + client, xContentRegistry); + snapshotHistoryStore.set(new SnapshotHistoryStore(templateRegistry, client, getClock().getZone(), clusterService)); snapshotLifecycleService.set(new SnapshotLifecycleService(settings, - () -> new SnapshotLifecycleTask(client, clusterService), clusterService, getClock())); - return Arrays.asList(indexLifecycleInitialisationService.get(), snapshotLifecycleService.get()); + () -> new SnapshotLifecycleTask(client, clusterService, snapshotHistoryStore.get()), clusterService, getClock())); + return Arrays.asList(indexLifecycleInitialisationService.get(), snapshotLifecycleService.get(), snapshotHistoryStore.get()); } @Override diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java index e11c59048ea5d..90ff9250ec36e 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java @@ -8,6 +8,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; @@ -27,6 +28,8 @@ import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotInvocationRecord; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecycleMetadata; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicyMetadata; +import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotCreationHistoryItem; +import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore; import org.elasticsearch.xpack.indexlifecycle.LifecyclePolicySecurityClient; import java.io.IOException; @@ -44,17 +47,19 @@ public class SnapshotLifecycleTask implements SchedulerEngine.Listener { private final Client client; private final ClusterService clusterService; + private final SnapshotHistoryStore historyStore; - public SnapshotLifecycleTask(final Client client, final ClusterService clusterService) { + public SnapshotLifecycleTask(final Client client, final ClusterService clusterService, final SnapshotHistoryStore historyStore) { this.client = client; this.clusterService = clusterService; + this.historyStore = historyStore; } @Override public void triggered(SchedulerEngine.Event event) { logger.debug("snapshot lifecycle policy task triggered from job [{}]", event.getJobName()); - final Optional snapshotName = maybeTakeSnapshot(event.getJobName(), client, clusterService); + final Optional snapshotName = maybeTakeSnapshot(event.getJobName(), client, clusterService, historyStore); // Would be cleaner if we could use Optional#ifPresentOrElse snapshotName.ifPresent(name -> @@ -72,7 +77,8 @@ public void triggered(SchedulerEngine.Event event) { * state in the policy's metadata * @return An optional snapshot name if the request was issued successfully */ - public static Optional maybeTakeSnapshot(final String jobId, final Client client, final ClusterService clusterService) { + public static Optional maybeTakeSnapshot(final String jobId, final Client client, final ClusterService clusterService, + final SnapshotHistoryStore historyStore) { Optional maybeMetadata = getSnapPolicyMetadata(jobId, clusterService.state()); String snapshotName = maybeMetadata.map(policyMetadata -> { CreateSnapshotRequest request = policyMetadata.getPolicy().toRequest(); @@ -85,16 +91,36 @@ public static Optional maybeTakeSnapshot(final String jobId, final Clien public void onResponse(CreateSnapshotResponse createSnapshotResponse) { logger.debug("snapshot response for [{}]: {}", policyMetadata.getPolicy().getId(), Strings.toString(createSnapshotResponse)); + final long timestamp = Instant.now().toEpochMilli(); clusterService.submitStateUpdateTask("slm-record-success-" + policyMetadata.getPolicy().getId(), - WriteJobStatus.success(policyMetadata.getPolicy().getId(), request.snapshot(), Instant.now().toEpochMilli())); + WriteJobStatus.success(policyMetadata.getPolicy().getId(), request.snapshot(), timestamp)); + historyStore.putAsync(SnapshotCreationHistoryItem.successRecord(timestamp, + policyMetadata.getPolicy().getId(), + request, + policyMetadata.getPolicy().getConfig())); } @Override public void onFailure(Exception e) { logger.error("failed to issue create snapshot request for snapshot lifecycle policy [{}]: {}", policyMetadata.getPolicy().getId(), e); + final long timestamp = Instant.now().toEpochMilli(); clusterService.submitStateUpdateTask("slm-record-failure-" + policyMetadata.getPolicy().getId(), - WriteJobStatus.failure(policyMetadata.getPolicy().getId(), request.snapshot(), Instant.now().toEpochMilli(), e)); + WriteJobStatus.failure(policyMetadata.getPolicy().getId(), request.snapshot(), timestamp, e)); + final SnapshotCreationHistoryItem failureRecord; + try { + failureRecord = SnapshotCreationHistoryItem.failureRecord(timestamp, + policyMetadata.getPolicy().getId(), + request, + policyMetadata.getPolicy().getConfig(), + e); + historyStore.putAsync(failureRecord); + } catch (IOException ex) { + // This shouldn't happen unless there's an issue with serializing the original exception, which shouldn't happen + logger.error(new ParameterizedMessage( + "failed to record snapshot creation failure for snapshot lifecycle policy [{}]", + policyMetadata.getPolicy().getId()), e); + } } }); return request.snapshot(); diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportExecuteSnapshotLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportExecuteSnapshotLifecycleAction.java index 883a153b01610..8ff080dc542ad 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportExecuteSnapshotLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportExecuteSnapshotLifecycleAction.java @@ -25,6 +25,7 @@ import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecycleMetadata; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicyMetadata; import org.elasticsearch.xpack.core.snapshotlifecycle.action.ExecuteSnapshotLifecycleAction; +import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore; import org.elasticsearch.xpack.snapshotlifecycle.SnapshotLifecycleService; import org.elasticsearch.xpack.snapshotlifecycle.SnapshotLifecycleTask; @@ -37,14 +38,16 @@ public class TransportExecuteSnapshotLifecycleAction private static final Logger logger = LogManager.getLogger(TransportExecuteSnapshotLifecycleAction.class); private final Client client; + private final SnapshotHistoryStore historyStore; @Inject public TransportExecuteSnapshotLifecycleAction(TransportService transportService, ClusterService clusterService, ThreadPool threadPool, ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver, - Client client) { + Client client, SnapshotHistoryStore historyStore) { super(ExecuteSnapshotLifecycleAction.NAME, transportService, clusterService, threadPool, actionFilters, indexNameExpressionResolver, ExecuteSnapshotLifecycleAction.Request::new); this.client = client; + this.historyStore = historyStore; } @Override protected String executor() { @@ -80,7 +83,7 @@ protected void masterOperation(final ExecuteSnapshotLifecycleAction.Request requ } final Optional snapshotName = SnapshotLifecycleTask.maybeTakeSnapshot(SnapshotLifecycleService.getJobId(policyMetadata), - client, clusterService); + client, clusterService, historyStore); if (snapshotName.isPresent()) { listener.onResponse(new ExecuteSnapshotLifecycleAction.Response(snapshotName.get())); } else { diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java index 794ca7df213de..801b774d418df 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java @@ -297,7 +297,7 @@ class FakeSnapshotTask extends SnapshotLifecycleTask { private final Consumer onTriggered; FakeSnapshotTask(Consumer onTriggered) { - super(null, null); + super(null, null, null); this.onTriggered = onTriggered; } diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java index 02e9705008bb7..360ba077c24c3 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java @@ -30,6 +30,7 @@ import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecycleMetadata; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicy; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicyMetadata; +import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore; import java.io.IOException; import java.util.Arrays; @@ -42,6 +43,10 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.startsWith; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; public class SnapshotLifecycleTaskTests extends ESTestCase { @@ -76,6 +81,8 @@ public void testSkipCreatingSnapshotWhenJobDoesNotMatch() { .build()) .build(); + final SnapshotHistoryStore historyStore = mock(SnapshotHistoryStore.class); + final ThreadPool threadPool = new TestThreadPool("test"); try (ClusterService clusterService = ClusterServiceUtils.createClusterService(state, threadPool); VerifyingClient client = new VerifyingClient(threadPool, (a, r, l) -> { @@ -83,12 +90,13 @@ public void testSkipCreatingSnapshotWhenJobDoesNotMatch() { return null; })) { - SnapshotLifecycleTask task = new SnapshotLifecycleTask(client, clusterService); + SnapshotLifecycleTask task = new SnapshotLifecycleTask(client, clusterService, historyStore); // Trigger the event, but since the job name does not match, it should // not run the function to create a snapshot task.triggered(new SchedulerEngine.Event("nonexistent-job", System.currentTimeMillis(), System.currentTimeMillis())); } + verify(historyStore, times(0)).putAsync(any()); threadPool.shutdownNow(); } @@ -128,6 +136,8 @@ public void testCreateSnapshotOnTrigger() { " }" + "}"; + final SnapshotHistoryStore historyStore = mock(SnapshotHistoryStore.class); + final AtomicBoolean called = new AtomicBoolean(false); try (ClusterService clusterService = ClusterServiceUtils.createClusterService(state, threadPool); // This verifying client will verify that we correctly invoked @@ -159,13 +169,14 @@ public void testCreateSnapshotOnTrigger() { } })) { - SnapshotLifecycleTask task = new SnapshotLifecycleTask(client, clusterService); + SnapshotLifecycleTask task = new SnapshotLifecycleTask(client, clusterService, historyStore); // Trigger the event with a matching job name for the policy task.triggered(new SchedulerEngine.Event(SnapshotLifecycleService.getJobId(slpm), System.currentTimeMillis(), System.currentTimeMillis())); assertTrue("snapshot should be triggered once", called.get()); } + verify(historyStore, times(1)).putAsync(any()); threadPool.shutdownNow(); } From e5e61d33bd8c470be9c40ae79751d0a4a8f02d02 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 2 May 2019 09:23:57 -0600 Subject: [PATCH 02/18] Correct ILM policy for monthly indices --- .../plugin/core/src/main/resources/slm-history-ilm-policy.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/resources/slm-history-ilm-policy.json b/x-pack/plugin/core/src/main/resources/slm-history-ilm-policy.json index e45e6b25e8f7b..8bccc4d23cb46 100644 --- a/x-pack/plugin/core/src/main/resources/slm-history-ilm-policy.json +++ b/x-pack/plugin/core/src/main/resources/slm-history-ilm-policy.json @@ -1,7 +1,7 @@ { "phases": { "delete": { - "min_age": "7d", + "min_age": "60d", "actions": { "delete": {} } From 05b9512e40b69a128c084de98359dd15b633a920 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 2 May 2019 11:50:46 -0600 Subject: [PATCH 03/18] Improve history store tests --- .../history/SnapshotHistoryStoreTests.java | 72 +++++++++++++++---- 1 file changed, 58 insertions(+), 14 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java index 272d48de652cb..cc84f1ad01aaf 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java @@ -18,6 +18,7 @@ import org.junit.Before; import org.mockito.ArgumentCaptor; +import java.io.IOException; import java.time.Instant; import java.time.ZoneOffset; import java.util.Map; @@ -55,25 +56,68 @@ public void setup() { } @SuppressWarnings("unchecked") - public void testPut() { + public void testPut() throws IOException { String policyId = randomAlphaOfLength(5); String repository = randomAlphaOfLength(6); String snapshotId = randomAlphaOfLength(7); CreateSnapshotRequest request = new CreateSnapshotRequest(repository, snapshotId); - Map config = randomBoolean() ? null : randomSnapshotConfiguration(); + Map config = randomSnapshotConfiguration(); final long timestamp = randomNonNegativeLong(); - SnapshotCreationHistoryItem record = SnapshotCreationHistoryItem.successRecord(timestamp, policyId, request, config); - - historyStore.putAsync(record); - ArgumentCaptor indexRequest = ArgumentCaptor.forClass(IndexRequest.class); - verify(client, times(1)).index(indexRequest.capture(), notNull(ActionListener.class)); - - assertEquals(getHistoryIndexNameForTime(Instant.ofEpochMilli(timestamp).atZone(ZoneOffset.UTC)), - indexRequest.getValue().index()); - final String indexedDocument = indexRequest.getValue().source().utf8ToString(); - assertThat(indexedDocument, containsString(policyId)); - assertThat(indexedDocument, containsString(repository)); - assertThat(indexedDocument, containsString(snapshotId)); + { + SnapshotCreationHistoryItem record = SnapshotCreationHistoryItem.successRecord(timestamp, policyId, request, config); + + historyStore.putAsync(record); + ArgumentCaptor indexRequest = ArgumentCaptor.forClass(IndexRequest.class); + verify(client, times(1)).index(indexRequest.capture(), notNull(ActionListener.class)); + + assertEquals(getHistoryIndexNameForTime(Instant.ofEpochMilli(timestamp).atZone(ZoneOffset.UTC)), + indexRequest.getValue().index()); + final String indexedDocument = indexRequest.getValue().source().utf8ToString(); + assertThat(indexedDocument, containsString(policyId)); + assertThat(indexedDocument, containsString(repository)); + assertThat(indexedDocument, containsString(snapshotId)); + if (config != null) { + assertContainsMap(indexedDocument, config); + } + } + + { + final String cause = randomAlphaOfLength(9); + Exception failureException = new RuntimeException(cause); + SnapshotCreationHistoryItem record = SnapshotCreationHistoryItem.failureRecord(timestamp, policyId, request, config, + failureException); + historyStore.putAsync(record); + ArgumentCaptor indexRequest = ArgumentCaptor.forClass(IndexRequest.class); + verify(client, times(2)).index(indexRequest.capture(), notNull(ActionListener.class)); + + assertEquals(getHistoryIndexNameForTime(Instant.ofEpochMilli(timestamp).atZone(ZoneOffset.UTC)), + indexRequest.getValue().index()); + final String indexedDocument = indexRequest.getValue().source().utf8ToString(); + assertThat(indexedDocument, containsString(policyId)); + assertThat(indexedDocument, containsString(repository)); + assertThat(indexedDocument, containsString(snapshotId)); + if (config != null) { + assertContainsMap(indexedDocument, config); + } + assertThat(indexedDocument, containsString("runtime_exception")); + assertThat(indexedDocument, containsString(cause)); + } + } + + @SuppressWarnings("unchecked") + private void assertContainsMap(String indexedDocument, Map map) { + map.forEach((k, v) -> { + assertThat(indexedDocument, containsString(k)); + if (v instanceof Map) { + assertContainsMap(indexedDocument, (Map) v); + } if (v instanceof Iterable) { + ((Iterable) v).forEach(elem -> { + assertThat(indexedDocument, containsString(elem.toString())); + }); + } else { + assertThat(indexedDocument, containsString(v.toString())); + } + }); } From bd96c6ffe61aee455d872234dddd771221233a55 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 3 May 2019 14:29:12 -0600 Subject: [PATCH 04/18] De-inheritify SnapshotHistoryItem --- .../history/SnapshotCreationHistoryItem.java | 139 --------------- .../history/SnapshotHistoryItem.java | 167 ++++++++++++------ ...sts.java => SnapshotHistoryItemTests.java} | 34 ++-- .../history/SnapshotHistoryStoreTests.java | 6 +- .../SnapshotLifecycleTask.java | 8 +- 5 files changed, 138 insertions(+), 216 deletions(-) delete mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItem.java rename x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/{SnapshotCreationHistoryItemTests.java => SnapshotHistoryItemTests.java} (71%) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItem.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItem.java deleted file mode 100644 index 340021a2315d2..0000000000000 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItem.java +++ /dev/null @@ -1,139 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -package org.elasticsearch.xpack.core.snapshotlifecycle.history; - -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; -import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.xcontent.ConstructingObjectParser; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.json.JsonXContent; - -import java.io.IOException; -import java.util.Collections; -import java.util.Map; -import java.util.Objects; - -import static org.elasticsearch.ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE; - -public class SnapshotCreationHistoryItem extends SnapshotHistoryItem { - private static final String CREATE_OPERATION = "CREATE"; - - private final Map snapshotConfiguration; - @Nullable - private final String errorDetails; - - static final ParseField SNAPSHOT_CONFIG = new ParseField("configuration"); - static final ParseField ERROR_DETAILS = new ParseField("error_details"); - - @SuppressWarnings("unchecked") - private static final ConstructingObjectParser PARSER = - new ConstructingObjectParser<>("snapshot_lifecycle_history_item", true, - (a, id) -> { - final long timestamp = (long) a[0]; - final String policyId = (String) a[1]; - final String repository = (String) a[2]; - final String snapshotName = (String) a[3]; - final String operation = (String) a[4]; - final boolean success = (boolean) a[5]; - final Map snapshotConfiguration = (Map) a[6]; - final String errorDetails = (String) a[7]; - return new SnapshotCreationHistoryItem(timestamp, policyId, repository, snapshotName, operation, success, - snapshotConfiguration, errorDetails); - }); - - static { - PARSER.declareLong(ConstructingObjectParser.constructorArg(), TIMESTAMP); - PARSER.declareString(ConstructingObjectParser.constructorArg(), POLICY_ID); - PARSER.declareString(ConstructingObjectParser.constructorArg(), REPOSITORY); - PARSER.declareString(ConstructingObjectParser.constructorArg(), SNAPSHOT_NAME); - PARSER.declareString(ConstructingObjectParser.constructorArg(), OPERATION); - PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), SUCCESS); - PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.map(), SNAPSHOT_CONFIG); - PARSER.declareStringOrNull(ConstructingObjectParser.constructorArg(), ERROR_DETAILS); - } - - SnapshotCreationHistoryItem(long timestamp, String policyId, String repository, String snapshotName, String operation, - boolean success, Map snapshotConfiguration, String errorDetails) { - super(timestamp, policyId, repository, snapshotName, operation, success); - this.snapshotConfiguration = Objects.requireNonNull(snapshotConfiguration); - this.errorDetails = errorDetails; - } - - public static SnapshotCreationHistoryItem successRecord(long timestamp, String policyId, CreateSnapshotRequest request, - Map snapshotConfiguration) { - return new SnapshotCreationHistoryItem(timestamp, policyId, request.repository(), request.snapshot(), CREATE_OPERATION, true, - snapshotConfiguration, null); - } - - public static SnapshotCreationHistoryItem failureRecord(long timeStamp, String policyId, CreateSnapshotRequest request, - Map snapshotConfiguration, - Exception exception) throws IOException { - ToXContent.Params stacktraceParams = new ToXContent.MapParams(Collections.singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false")); - String exceptionString; - try (XContentBuilder causeXContentBuilder = JsonXContent.contentBuilder()) { - causeXContentBuilder.startObject(); - ElasticsearchException.generateThrowableXContent(causeXContentBuilder, stacktraceParams, exception); - causeXContentBuilder.endObject(); - exceptionString = BytesReference.bytes(causeXContentBuilder).utf8ToString(); - } - return new SnapshotCreationHistoryItem(timeStamp, policyId, request.repository(), request.snapshot(), CREATE_OPERATION, false, - snapshotConfiguration, exceptionString); - } - - public SnapshotCreationHistoryItem(StreamInput in) throws IOException { - super(in); - this.snapshotConfiguration = in.readMap(); - this.errorDetails = in.readOptionalString(); - } - - public Map getSnapshotConfiguration() { - return snapshotConfiguration; - } - - public String getErrorDetails() { - return errorDetails; - } - - @Override - protected void innerWriteTo(StreamOutput out) throws IOException { - out.writeMap(snapshotConfiguration); - out.writeOptionalString(errorDetails); - } - - @Override - protected XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(SNAPSHOT_CONFIG.getPreferredName(), snapshotConfiguration); - builder.field(ERROR_DETAILS.getPreferredName(), errorDetails); - return builder; - } - - public static SnapshotCreationHistoryItem parse(XContentParser parser, String name) { - return PARSER.apply(parser, name); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - if (!super.equals(o)) return false; - SnapshotCreationHistoryItem that = (SnapshotCreationHistoryItem) o; - return Objects.equals(getSnapshotConfiguration(), that.getSnapshotConfiguration()) && - Objects.equals(getErrorDetails(), that.getErrorDetails()); - } - - @Override - public int hashCode() { - return Objects.hash(super.hashCode(), getSnapshotConfiguration(), getErrorDetails()); - } -} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java index 6fad844bd25d6..6d68f9fb69f2d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java @@ -6,28 +6,37 @@ package org.elasticsearch.xpack.core.snapshotlifecycle.history; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.json.JsonXContent; import java.io.IOException; +import java.util.Collections; +import java.util.Map; import java.util.Objects; -/** - * Represents an entry in the Snapshot Lifecycle Management history index. Subclass this to add fields which are specific to a type of - * operation (e.g. creation, deletion, any others). - * - * In addition to the abstract methods that subclasses must implement, subclasses should also implement: - * 1) a constructor that takes a {@link StreamInput} for deserialization. Remember to call {@code super(in)} before reading any new fields. - * 2) an xContent parser using {@link org.elasticsearch.common.xcontent.ConstructingObjectParser}. This will need to parse the fields that - * are part of {@link SnapshotHistoryItem} as well as new fields. - */ -public abstract class SnapshotHistoryItem implements Writeable, ToXContentObject { +import static org.elasticsearch.ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE; +public class SnapshotHistoryItem implements Writeable, ToXContentObject { + static final ParseField TIMESTAMP = new ParseField("@timestamp"); + static final ParseField POLICY_ID = new ParseField("policy"); + static final ParseField REPOSITORY = new ParseField("repository"); + static final ParseField SNAPSHOT_NAME = new ParseField("snapshot_name"); + static final ParseField OPERATION = new ParseField("operation"); + static final ParseField SUCCESS = new ParseField("success"); + private static final String CREATE_OPERATION = "CREATE"; protected final long timestamp; protected final String policyId; protected final String repository; @@ -35,20 +44,75 @@ public abstract class SnapshotHistoryItem implements Writeable, ToXContentObject protected final String operation; protected final boolean success; - static final ParseField TIMESTAMP = new ParseField("@timestamp"); - static final ParseField POLICY_ID = new ParseField("policy"); - static final ParseField REPOSITORY = new ParseField("repository"); - static final ParseField SNAPSHOT_NAME = new ParseField("snapshot_name"); - static final ParseField OPERATION = new ParseField("operation"); - static final ParseField SUCCESS = new ParseField("success"); + private final Map snapshotConfiguration; + @Nullable + private final String errorDetails; + + static final ParseField SNAPSHOT_CONFIG = new ParseField("configuration"); + static final ParseField ERROR_DETAILS = new ParseField("error_details"); + + @SuppressWarnings("unchecked") + private static final ConstructingObjectParser PARSER = + new ConstructingObjectParser<>("snapshot_lifecycle_history_item", true, + (a, id) -> { + final long timestamp = (long) a[0]; + final String policyId = (String) a[1]; + final String repository = (String) a[2]; + final String snapshotName = (String) a[3]; + final String operation = (String) a[4]; + final boolean success = (boolean) a[5]; + final Map snapshotConfiguration = (Map) a[6]; + final String errorDetails = (String) a[7]; + return new SnapshotHistoryItem(timestamp, policyId, repository, snapshotName, operation, success, + snapshotConfiguration, errorDetails); + }); + + static { + PARSER.declareLong(ConstructingObjectParser.constructorArg(), TIMESTAMP); + PARSER.declareString(ConstructingObjectParser.constructorArg(), POLICY_ID); + PARSER.declareString(ConstructingObjectParser.constructorArg(), REPOSITORY); + PARSER.declareString(ConstructingObjectParser.constructorArg(), SNAPSHOT_NAME); + PARSER.declareString(ConstructingObjectParser.constructorArg(), OPERATION); + PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), SUCCESS); + PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.map(), SNAPSHOT_CONFIG); + PARSER.declareStringOrNull(ConstructingObjectParser.constructorArg(), ERROR_DETAILS); + } - public SnapshotHistoryItem(long timestamp, String policyId, String repository, String snapshotName, String operation, boolean success) { + public static SnapshotHistoryItem parse(XContentParser parser, String name) { + return PARSER.apply(parser, name); + } + + SnapshotHistoryItem(long timestamp, String policyId, String repository, String snapshotName, String operation, + boolean success, Map snapshotConfiguration, String errorDetails) { this.timestamp = timestamp; this.policyId = Objects.requireNonNull(policyId); this.repository = Objects.requireNonNull(repository); this.snapshotName = Objects.requireNonNull(snapshotName); this.operation = Objects.requireNonNull(operation); this.success = success; + this.snapshotConfiguration = Objects.requireNonNull(snapshotConfiguration); + this.errorDetails = errorDetails; + } + + public static SnapshotHistoryItem successRecord(long timestamp, String policyId, CreateSnapshotRequest request, + Map snapshotConfiguration) { + return new SnapshotHistoryItem(timestamp, policyId, request.repository(), request.snapshot(), CREATE_OPERATION, true, + snapshotConfiguration, null); + } + + public static SnapshotHistoryItem failureRecord(long timeStamp, String policyId, CreateSnapshotRequest request, + Map snapshotConfiguration, + Exception exception) throws IOException { + ToXContent.Params stacktraceParams = new ToXContent.MapParams(Collections.singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false")); + String exceptionString; + try (XContentBuilder causeXContentBuilder = JsonXContent.contentBuilder()) { + causeXContentBuilder.startObject(); + ElasticsearchException.generateThrowableXContent(causeXContentBuilder, stacktraceParams, exception); + causeXContentBuilder.endObject(); + exceptionString = BytesReference.bytes(causeXContentBuilder).utf8ToString(); + } + return new SnapshotHistoryItem(timeStamp, policyId, request.repository(), request.snapshot(), CREATE_OPERATION, false, + snapshotConfiguration, exceptionString); } public SnapshotHistoryItem(StreamInput in) throws IOException { @@ -58,6 +122,16 @@ public SnapshotHistoryItem(StreamInput in) throws IOException { this.snapshotName = in.readString(); this.operation = in.readString(); this.success = in.readBoolean(); + this.snapshotConfiguration = in.readMap(); + this.errorDetails = in.readOptionalString(); + } + + public Map getSnapshotConfiguration() { + return snapshotConfiguration; + } + + public String getErrorDetails() { + return errorDetails; } public long getTimestamp() { @@ -92,7 +166,8 @@ public final void writeTo(StreamOutput out) throws IOException { out.writeString(snapshotName); out.writeString(operation); out.writeBoolean(success); - innerWriteTo(out); + out.writeMap(snapshotConfiguration); + out.writeOptionalString(errorDetails); } @Override @@ -105,55 +180,41 @@ public final XContentBuilder toXContent(XContentBuilder builder, Params params) builder.field(SNAPSHOT_NAME.getPreferredName(), snapshotName); builder.field(OPERATION.getPreferredName(), operation); builder.field(SUCCESS.getPreferredName(), success); - innerToXContent(builder, params); + builder.field(SNAPSHOT_CONFIG.getPreferredName(), snapshotConfiguration); + builder.field(ERROR_DETAILS.getPreferredName(), errorDetails); } builder.endObject(); return builder; } - @Override - public String toString() { - return Strings.toString(this); - } - - /** - * Write any fields that are introduced in the subclass here. This is called as part of - * {@link SnapshotHistoryItem#writeTo(StreamOutput)}, you do not need to (and should not) override that method. All fields that are - * part of the {@link SnapshotHistoryItem} class will already be written to the stream, so only write new fields introduced in the - * subclass. - * @param out The output stream - * @throws IOException if an error occurs while writing to the output stream - */ - protected abstract void innerWriteTo(StreamOutput out) throws IOException; - - /** - * Write any fields that are introduced in the subclass here. This is called as part of - * {@link SnapshotHistoryItem#toXContent(XContentBuilder, Params)}, you do not need to (and should not) override that method. All fields - * that are part of the {@link SnapshotHistoryItem} class will already be written to the stream, so only write new fields introduced in - * the subclass. - * @param builder The xContent builder - * @param params Parameters - * @return The XContent builder - * @throws IOException if an error occurs while writing to the builder - */ - protected abstract XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException; - @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; + boolean result; + if (this == o) result = true; + if (o == null || getClass() != o.getClass()) result = false; + SnapshotHistoryItem that1 = (SnapshotHistoryItem) o; + result = isSuccess() == that1.isSuccess() && + timestamp == that1.getTimestamp() && + Objects.equals(getPolicyId(), that1.getPolicyId()) && + Objects.equals(getRepository(), that1.getRepository()) && + Objects.equals(getSnapshotName(), that1.getSnapshotName()) && + Objects.equals(getOperation(), that1.getOperation()); + if (!result) return false; SnapshotHistoryItem that = (SnapshotHistoryItem) o; - return isSuccess() == that.isSuccess() && - timestamp == that.getTimestamp() && - Objects.equals(getPolicyId(), that.getPolicyId()) && - Objects.equals(getRepository(), that.getRepository()) && - Objects.equals(getSnapshotName(), that.getSnapshotName()) && - Objects.equals(getOperation(), that.getOperation()); + return Objects.equals(getSnapshotConfiguration(), that.getSnapshotConfiguration()) && + Objects.equals(getErrorDetails(), that.getErrorDetails()); } @Override public int hashCode() { - return Objects.hash(getTimestamp(), getPolicyId(), getRepository(), getSnapshotName(), getOperation(), isSuccess()); + return Objects.hash(Objects.hash(getTimestamp(), getPolicyId(), getRepository(), getSnapshotName(), getOperation(), isSuccess()), getSnapshotConfiguration(), getErrorDetails()); + } + + @Override + public String toString() { + return Strings.toString(this); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItemTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItemTests.java similarity index 71% rename from x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItemTests.java rename to x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItemTests.java index 3d9b46aeed372..0622398e08fbe 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotCreationHistoryItemTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItemTests.java @@ -16,20 +16,20 @@ import java.util.HashMap; import java.util.Map; -public class SnapshotCreationHistoryItemTests extends AbstractSerializingTestCase { +public class SnapshotHistoryItemTests extends AbstractSerializingTestCase { @Override - protected SnapshotCreationHistoryItem doParseInstance(XContentParser parser) throws IOException { - return SnapshotCreationHistoryItem.parse(parser, this.getClass().getCanonicalName()); + protected SnapshotHistoryItem doParseInstance(XContentParser parser) throws IOException { + return SnapshotHistoryItem.parse(parser, this.getClass().getCanonicalName()); } @Override - protected Writeable.Reader instanceReader() { - return SnapshotCreationHistoryItem::new; + protected Writeable.Reader instanceReader() { + return SnapshotHistoryItem::new; } @Override - protected SnapshotCreationHistoryItem createTestInstance() { + protected SnapshotHistoryItem createTestInstance() { long timestamp = randomNonNegativeLong(); String policyId = randomAlphaOfLengthBetween(5, 10); String repository = randomAlphaOfLengthBetween(5, 10); @@ -39,51 +39,51 @@ protected SnapshotCreationHistoryItem createTestInstance() { Map snapshotConfig = randomSnapshotConfiguration(); String errorDetails = randomBoolean() ? null : randomAlphaOfLengthBetween(10, 20); - return new SnapshotCreationHistoryItem(timestamp, policyId, repository, snapshotName, operation, success, snapshotConfig, + return new SnapshotHistoryItem(timestamp, policyId, repository, snapshotName, operation, success, snapshotConfig, errorDetails); } @Override - protected SnapshotCreationHistoryItem mutateInstance(SnapshotCreationHistoryItem instance) { + protected SnapshotHistoryItem mutateInstance(SnapshotHistoryItem instance) { final int branch = between(0, 7); switch (branch) { case 0: // New timestamp - return new SnapshotCreationHistoryItem( + return new SnapshotHistoryItem( randomValueOtherThan(instance.getTimestamp(), ESTestCase::randomNonNegativeLong), instance.getPolicyId(), instance.getRepository(), instance.getSnapshotName(), instance.getOperation(), instance.isSuccess(), instance.getSnapshotConfiguration(), instance.getErrorDetails()); case 1: // new policyId - return new SnapshotCreationHistoryItem(instance.getTimestamp(), + return new SnapshotHistoryItem(instance.getTimestamp(), randomValueOtherThan(instance.getPolicyId(), () -> randomAlphaOfLengthBetween(5, 10)), instance.getSnapshotName(), instance.getRepository(), instance.getOperation(), instance.isSuccess(), instance.getSnapshotConfiguration(), instance.getErrorDetails()); case 2: // new repo name - return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getSnapshotName(), + return new SnapshotHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getSnapshotName(), randomValueOtherThan(instance.getRepository(), () -> randomAlphaOfLengthBetween(5, 10)), instance.getOperation(), instance.isSuccess(), instance.getSnapshotConfiguration(), instance.getErrorDetails()); case 3: - return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), + return new SnapshotHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), randomValueOtherThan(instance.getSnapshotName(), () -> randomAlphaOfLengthBetween(5, 10)), instance.getOperation(), instance.isSuccess(), instance.getSnapshotConfiguration(), instance.getErrorDetails()); case 4: - return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), + return new SnapshotHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), instance.getSnapshotName(), randomValueOtherThan(instance.getOperation(), () -> randomAlphaOfLengthBetween(5, 10)), instance.isSuccess(), instance.getSnapshotConfiguration(), instance.getErrorDetails()); case 5: - return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), + return new SnapshotHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), instance.getSnapshotName(), instance.getOperation(), instance.isSuccess() == false, instance.getSnapshotConfiguration(), instance.getErrorDetails()); case 6: - return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), + return new SnapshotHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), instance.getSnapshotName(), instance.getOperation(), instance.isSuccess(), randomValueOtherThan(instance.getSnapshotConfiguration(), - SnapshotCreationHistoryItemTests::randomSnapshotConfiguration), + SnapshotHistoryItemTests::randomSnapshotConfiguration), instance.getErrorDetails()); case 7: - return new SnapshotCreationHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), + return new SnapshotHistoryItem(instance.getTimestamp(), instance.getPolicyId(), instance.getRepository(), instance.getSnapshotName(), instance.getOperation(), instance.isSuccess(), instance.getSnapshotConfiguration(), randomValueOtherThan(instance.getErrorDetails(), () -> randomAlphaOfLengthBetween(10, 20))); default: diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java index cc84f1ad01aaf..1fd289e1f2238 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java @@ -23,7 +23,7 @@ import java.time.ZoneOffset; import java.util.Map; -import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotCreationHistoryItemTests.randomSnapshotConfiguration; +import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryItemTests.randomSnapshotConfiguration; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore.getHistoryIndexNameForTime; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.INDEX_TEMPLATE_VERSION; import static org.hamcrest.Matchers.containsString; @@ -64,7 +64,7 @@ public void testPut() throws IOException { Map config = randomSnapshotConfiguration(); final long timestamp = randomNonNegativeLong(); { - SnapshotCreationHistoryItem record = SnapshotCreationHistoryItem.successRecord(timestamp, policyId, request, config); + SnapshotHistoryItem record = SnapshotHistoryItem.successRecord(timestamp, policyId, request, config); historyStore.putAsync(record); ArgumentCaptor indexRequest = ArgumentCaptor.forClass(IndexRequest.class); @@ -84,7 +84,7 @@ public void testPut() throws IOException { { final String cause = randomAlphaOfLength(9); Exception failureException = new RuntimeException(cause); - SnapshotCreationHistoryItem record = SnapshotCreationHistoryItem.failureRecord(timestamp, policyId, request, config, + SnapshotHistoryItem record = SnapshotHistoryItem.failureRecord(timestamp, policyId, request, config, failureException); historyStore.putAsync(record); ArgumentCaptor indexRequest = ArgumentCaptor.forClass(IndexRequest.class); diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java index 90ff9250ec36e..c6e64f14270a9 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java @@ -28,7 +28,7 @@ import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotInvocationRecord; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecycleMetadata; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicyMetadata; -import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotCreationHistoryItem; +import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryItem; import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore; import org.elasticsearch.xpack.indexlifecycle.LifecyclePolicySecurityClient; @@ -94,7 +94,7 @@ public void onResponse(CreateSnapshotResponse createSnapshotResponse) { final long timestamp = Instant.now().toEpochMilli(); clusterService.submitStateUpdateTask("slm-record-success-" + policyMetadata.getPolicy().getId(), WriteJobStatus.success(policyMetadata.getPolicy().getId(), request.snapshot(), timestamp)); - historyStore.putAsync(SnapshotCreationHistoryItem.successRecord(timestamp, + historyStore.putAsync(SnapshotHistoryItem.successRecord(timestamp, policyMetadata.getPolicy().getId(), request, policyMetadata.getPolicy().getConfig())); @@ -107,9 +107,9 @@ public void onFailure(Exception e) { final long timestamp = Instant.now().toEpochMilli(); clusterService.submitStateUpdateTask("slm-record-failure-" + policyMetadata.getPolicy().getId(), WriteJobStatus.failure(policyMetadata.getPolicy().getId(), request.snapshot(), timestamp, e)); - final SnapshotCreationHistoryItem failureRecord; + final SnapshotHistoryItem failureRecord; try { - failureRecord = SnapshotCreationHistoryItem.failureRecord(timestamp, + failureRecord = SnapshotHistoryItem.failureRecord(timestamp, policyMetadata.getPolicy().getId(), request, policyMetadata.getPolicy().getConfig(), From 0bde4fc74b52c88a867f8775b7444fa7f38f314d Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 3 May 2019 14:53:10 -0600 Subject: [PATCH 05/18] Use policy to build history item instead of request --- .../history/SnapshotHistoryItem.java | 16 +++--- .../history/SnapshotHistoryStoreTests.java | 50 ++++++++++++------- .../SnapshotLifecycleTask.java | 11 +--- .../SnapshotLifecyclePolicyTests.java | 10 ++-- 4 files changed, 49 insertions(+), 38 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java index 6d68f9fb69f2d..a5587b5fb5a2f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.core.snapshotlifecycle.history; import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; @@ -21,6 +20,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicy; import java.io.IOException; import java.util.Collections; @@ -94,14 +94,12 @@ public static SnapshotHistoryItem parse(XContentParser parser, String name) { this.errorDetails = errorDetails; } - public static SnapshotHistoryItem successRecord(long timestamp, String policyId, CreateSnapshotRequest request, - Map snapshotConfiguration) { - return new SnapshotHistoryItem(timestamp, policyId, request.repository(), request.snapshot(), CREATE_OPERATION, true, - snapshotConfiguration, null); + public static SnapshotHistoryItem successRecord(long timestamp, SnapshotLifecyclePolicy policy, String snapshotName) { + return new SnapshotHistoryItem(timestamp, policy.getId(), policy.getRepository(), snapshotName, CREATE_OPERATION, true, + policy.getConfig(), null); } - public static SnapshotHistoryItem failureRecord(long timeStamp, String policyId, CreateSnapshotRequest request, - Map snapshotConfiguration, + public static SnapshotHistoryItem failureRecord(long timeStamp, SnapshotLifecyclePolicy policy, String snapshotName, Exception exception) throws IOException { ToXContent.Params stacktraceParams = new ToXContent.MapParams(Collections.singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false")); String exceptionString; @@ -111,8 +109,8 @@ public static SnapshotHistoryItem failureRecord(long timeStamp, String policyId, causeXContentBuilder.endObject(); exceptionString = BytesReference.bytes(causeXContentBuilder).utf8ToString(); } - return new SnapshotHistoryItem(timeStamp, policyId, request.repository(), request.snapshot(), CREATE_OPERATION, false, - snapshotConfiguration, exceptionString); + return new SnapshotHistoryItem(timeStamp, policy.getId(), policy.getRepository(), snapshotName, CREATE_OPERATION, false, + policy.getConfig(), exceptionString); } public SnapshotHistoryItem(StreamInput in) throws IOException { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java index 1fd289e1f2238..8942ed235d0a8 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.core.snapshotlifecycle.history; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.service.ClusterService; @@ -15,15 +14,16 @@ import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicy; import org.junit.Before; import org.mockito.ArgumentCaptor; import java.io.IOException; import java.time.Instant; import java.time.ZoneOffset; +import java.util.HashMap; import java.util.Map; -import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryItemTests.randomSnapshotConfiguration; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore.getHistoryIndexNameForTime; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.INDEX_TEMPLATE_VERSION; import static org.hamcrest.Matchers.containsString; @@ -58,13 +58,12 @@ public void setup() { @SuppressWarnings("unchecked") public void testPut() throws IOException { String policyId = randomAlphaOfLength(5); - String repository = randomAlphaOfLength(6); - String snapshotId = randomAlphaOfLength(7); - CreateSnapshotRequest request = new CreateSnapshotRequest(repository, snapshotId); - Map config = randomSnapshotConfiguration(); + SnapshotLifecyclePolicy policy = randomSnapshotLifecyclePolicy(policyId); final long timestamp = randomNonNegativeLong(); + SnapshotLifecyclePolicy.ResolverContext context = new SnapshotLifecyclePolicy.ResolverContext(timestamp); + String snapshotId = policy.generateSnapshotName(context); { - SnapshotHistoryItem record = SnapshotHistoryItem.successRecord(timestamp, policyId, request, config); + SnapshotHistoryItem record = SnapshotHistoryItem.successRecord(timestamp, policy, snapshotId); historyStore.putAsync(record); ArgumentCaptor indexRequest = ArgumentCaptor.forClass(IndexRequest.class); @@ -73,19 +72,18 @@ public void testPut() throws IOException { assertEquals(getHistoryIndexNameForTime(Instant.ofEpochMilli(timestamp).atZone(ZoneOffset.UTC)), indexRequest.getValue().index()); final String indexedDocument = indexRequest.getValue().source().utf8ToString(); - assertThat(indexedDocument, containsString(policyId)); - assertThat(indexedDocument, containsString(repository)); + assertThat(indexedDocument, containsString(policy.getId())); + assertThat(indexedDocument, containsString(policy.getRepository())); assertThat(indexedDocument, containsString(snapshotId)); - if (config != null) { - assertContainsMap(indexedDocument, config); + if (policy.getConfig() != null) { + assertContainsMap(indexedDocument, policy.getConfig()); } } { final String cause = randomAlphaOfLength(9); Exception failureException = new RuntimeException(cause); - SnapshotHistoryItem record = SnapshotHistoryItem.failureRecord(timestamp, policyId, request, config, - failureException); + SnapshotHistoryItem record = SnapshotHistoryItem.failureRecord(timestamp, policy, snapshotId, failureException); historyStore.putAsync(record); ArgumentCaptor indexRequest = ArgumentCaptor.forClass(IndexRequest.class); verify(client, times(2)).index(indexRequest.capture(), notNull(ActionListener.class)); @@ -93,11 +91,11 @@ public void testPut() throws IOException { assertEquals(getHistoryIndexNameForTime(Instant.ofEpochMilli(timestamp).atZone(ZoneOffset.UTC)), indexRequest.getValue().index()); final String indexedDocument = indexRequest.getValue().source().utf8ToString(); - assertThat(indexedDocument, containsString(policyId)); - assertThat(indexedDocument, containsString(repository)); + assertThat(indexedDocument, containsString(policy.getId())); + assertThat(indexedDocument, containsString(policy.getRepository())); assertThat(indexedDocument, containsString(snapshotId)); - if (config != null) { - assertContainsMap(indexedDocument, config); + if (policy.getConfig() != null) { + assertContainsMap(indexedDocument, policy.getConfig()); } assertThat(indexedDocument, containsString("runtime_exception")); assertThat(indexedDocument, containsString(cause)); @@ -132,4 +130,22 @@ public void testIndexNameGeneration() { assertThat(getHistoryIndexNameForTime(Instant.ofEpochMilli(2833165811000L).atZone(ZoneOffset.UTC)), equalTo(".slm-history-" + indexTemplateVersion + "-2059.10")); } + + public static SnapshotLifecyclePolicy randomSnapshotLifecyclePolicy(String id) { + Map config = new HashMap<>(); + for (int i = 0; i < randomIntBetween(2, 5); i++) { + config.put(randomAlphaOfLength(4), randomAlphaOfLength(4)); + } + return new SnapshotLifecyclePolicy(id, + randomAlphaOfLength(4), + randomSchedule(), + randomAlphaOfLength(4), + config); + } + + private static String randomSchedule() { + return randomIntBetween(0, 59) + " " + + randomIntBetween(0, 59) + " " + + randomIntBetween(0, 12) + " * * ?"; + } } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java index c6e64f14270a9..a31942ad4774e 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java @@ -94,10 +94,7 @@ public void onResponse(CreateSnapshotResponse createSnapshotResponse) { final long timestamp = Instant.now().toEpochMilli(); clusterService.submitStateUpdateTask("slm-record-success-" + policyMetadata.getPolicy().getId(), WriteJobStatus.success(policyMetadata.getPolicy().getId(), request.snapshot(), timestamp)); - historyStore.putAsync(SnapshotHistoryItem.successRecord(timestamp, - policyMetadata.getPolicy().getId(), - request, - policyMetadata.getPolicy().getConfig())); + historyStore.putAsync(SnapshotHistoryItem.successRecord(timestamp, policyMetadata.getPolicy(), request.snapshot())); } @Override @@ -109,11 +106,7 @@ public void onFailure(Exception e) { WriteJobStatus.failure(policyMetadata.getPolicy().getId(), request.snapshot(), timestamp, e)); final SnapshotHistoryItem failureRecord; try { - failureRecord = SnapshotHistoryItem.failureRecord(timestamp, - policyMetadata.getPolicy().getId(), - request, - policyMetadata.getPolicy().getConfig(), - e); + failureRecord = SnapshotHistoryItem.failureRecord(timestamp, policyMetadata.getPolicy(), request.snapshot(), e); historyStore.putAsync(failureRecord); } catch (IOException ex) { // This shouldn't happen unless there's an issue with serializing the original exception, which shouldn't happen 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 cbce432406b0d..190c378937a17 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 @@ -78,11 +78,15 @@ protected SnapshotLifecyclePolicy doParseInstance(XContentParser parser) throws @Override protected SnapshotLifecyclePolicy createTestInstance() { + id = randomAlphaOfLength(5); + return randomSnapshotLifecyclePolicy(id); + } + + public static SnapshotLifecyclePolicy randomSnapshotLifecyclePolicy(String id) { Map config = new HashMap<>(); for (int i = 0; i < randomIntBetween(2, 5); i++) { config.put(randomAlphaOfLength(4), randomAlphaOfLength(4)); } - id = randomAlphaOfLength(5); return new SnapshotLifecyclePolicy(id, randomAlphaOfLength(4), randomSchedule(), @@ -90,7 +94,7 @@ protected SnapshotLifecyclePolicy createTestInstance() { config); } - private String randomSchedule() { + private static String randomSchedule() { return randomIntBetween(0, 59) + " " + randomIntBetween(0, 59) + " " + randomIntBetween(0, 12) + " * * ?"; @@ -114,7 +118,7 @@ protected SnapshotLifecyclePolicy mutateInstance(SnapshotLifecyclePolicy instanc case 2: return new SnapshotLifecyclePolicy(instance.getId(), instance.getName(), - randomValueOtherThan(instance.getSchedule(), this::randomSchedule), + randomValueOtherThan(instance.getSchedule(), SnapshotLifecyclePolicyTests::randomSchedule), instance.getRepository(), instance.getConfig()); case 3: From bed7945cbf302e9a563f72fb82d1ec7d2ffd892a Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 3 May 2019 14:54:55 -0600 Subject: [PATCH 06/18] Javadoc on SnapshotHistoryItem --- .../core/snapshotlifecycle/history/SnapshotHistoryItem.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java index a5587b5fb5a2f..c191aed399ed5 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java @@ -29,6 +29,10 @@ import static org.elasticsearch.ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE; +/** + * Represents the record of a Snapshot Lifecycle Management action, so that it + * can be indexed in a history index or recorded to a log in a structured way + */ public class SnapshotHistoryItem implements Writeable, ToXContentObject { static final ParseField TIMESTAMP = new ParseField("@timestamp"); static final ParseField POLICY_ID = new ParseField("policy"); From 242f478d0c1f71d065e844878a9bc7dd936480eb Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 3 May 2019 14:56:16 -0600 Subject: [PATCH 07/18] SnapshotHistoryStore javadoc --- .../core/snapshotlifecycle/history/SnapshotHistoryStore.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java index 0e9964b6a6a8c..b279cb52e3047 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java @@ -29,6 +29,10 @@ import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.INDEX_TEMPLATE_VERSION; +/** + * Records Snapshot Lifecycle Management actions as represented by {@link SnapshotHistoryItem} into an index + * for the purposes of querying and alerting. + */ public class SnapshotHistoryStore implements ClusterStateListener { private static final Logger logger = LogManager.getLogger(SnapshotHistoryStore.class); private static final DateFormatter indexTimeFormat = DateFormatter.forPattern("yyyy.MM"); From 0e7847b28e780f4ff314a972f00613fb756a3155 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 3 May 2019 16:22:29 -0600 Subject: [PATCH 08/18] Replace mock with verifying subclass --- .../SnapshotLifecycleTaskTests.java | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java index 360ba077c24c3..730e098fd76e2 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotAction; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; +import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.MetaData; @@ -30,15 +31,20 @@ import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecycleMetadata; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicy; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicyMetadata; +import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryItem; import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore; +import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry; import java.io.IOException; +import java.time.ZoneId; +import java.time.ZoneOffset; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -81,14 +87,14 @@ public void testSkipCreatingSnapshotWhenJobDoesNotMatch() { .build()) .build(); - final SnapshotHistoryStore historyStore = mock(SnapshotHistoryStore.class); - final ThreadPool threadPool = new TestThreadPool("test"); try (ClusterService clusterService = ClusterServiceUtils.createClusterService(state, threadPool); VerifyingClient client = new VerifyingClient(threadPool, (a, r, l) -> { fail("should not have tried to take a snapshot"); return null; })) { + SnapshotHistoryStore historyStore = new VerifyingHistoryStore(null, client, ZoneOffset.UTC, clusterService, + item -> fail("should not have tried to store an item")); SnapshotLifecycleTask task = new SnapshotLifecycleTask(client, clusterService, historyStore); @@ -96,7 +102,6 @@ public void testSkipCreatingSnapshotWhenJobDoesNotMatch() { // not run the function to create a snapshot task.triggered(new SchedulerEngine.Event("nonexistent-job", System.currentTimeMillis(), System.currentTimeMillis())); } - verify(historyStore, times(0)).putAsync(any()); threadPool.shutdownNow(); } @@ -214,4 +219,20 @@ private SnapshotLifecyclePolicyMetadata makePolicyMeta(final String id) { .setModifiedDate(1) .build(); } + + public static class VerifyingHistoryStore extends SnapshotHistoryStore { + + Consumer verifier; + + public VerifyingHistoryStore(SnapshotLifecycleTemplateRegistry registry, Client client, ZoneId timeZone, + ClusterService clusterService, Consumer verifier) { + super(registry, client, timeZone, clusterService); + this.verifier = verifier; + } + + @Override + public void putAsync(SnapshotHistoryItem item) { + verifier.accept(item); + } + } } From afb30283181ce3bb42eb1ff83bc0202a3945fb81 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 3 May 2019 16:45:06 -0600 Subject: [PATCH 09/18] Replace more mocking --- .../SnapshotLifecycleTaskTests.java | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java index 730e098fd76e2..95b2d7e212eae 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.snapshotlifecycle; +import org.apache.lucene.util.SetOnce; import org.elasticsearch.Version; import org.elasticsearch.action.Action; import org.elasticsearch.action.ActionListener; @@ -49,10 +50,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.startsWith; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; public class SnapshotLifecycleTaskTests extends ESTestCase { @@ -93,7 +90,7 @@ public void testSkipCreatingSnapshotWhenJobDoesNotMatch() { fail("should not have tried to take a snapshot"); return null; })) { - SnapshotHistoryStore historyStore = new VerifyingHistoryStore(null, client, ZoneOffset.UTC, clusterService, + SnapshotHistoryStore historyStore = new VerifyingHistoryStore(null, null, ZoneOffset.UTC, clusterService, item -> fail("should not have tried to store an item")); SnapshotLifecycleTask task = new SnapshotLifecycleTask(client, clusterService, historyStore); @@ -141,16 +138,15 @@ public void testCreateSnapshotOnTrigger() { " }" + "}"; - final SnapshotHistoryStore historyStore = mock(SnapshotHistoryStore.class); - - final AtomicBoolean called = new AtomicBoolean(false); + final AtomicBoolean clientCalled = new AtomicBoolean(false); + final SetOnce snapshotName = new SetOnce<>(); try (ClusterService clusterService = ClusterServiceUtils.createClusterService(state, threadPool); // This verifying client will verify that we correctly invoked // client.admin().createSnapshot(...) with the appropriate // request. It also returns a mock real response VerifyingClient client = new VerifyingClient(threadPool, (action, request, listener) -> { - assertFalse(called.getAndSet(true)); + assertFalse(clientCalled.getAndSet(true)); assertThat(action, instanceOf(CreateSnapshotAction.class)); assertThat(request, instanceOf(CreateSnapshotRequest.class)); @@ -159,6 +155,7 @@ public void testCreateSnapshotOnTrigger() { SnapshotLifecyclePolicy policy = slpm.getPolicy(); assertThat(req.snapshot(), startsWith(policy.getName() + "-")); assertThat(req.repository(), equalTo(policy.getRepository())); + snapshotName.set(req.snapshot()); if (req.indices().length > 0) { assertThat(Arrays.asList(req.indices()), equalTo(policy.getConfig().get("indices"))); } @@ -173,15 +170,25 @@ public void testCreateSnapshotOnTrigger() { return null; } })) { + final AtomicBoolean historyStoreCalled = new AtomicBoolean(false); + SnapshotHistoryStore historyStore = new VerifyingHistoryStore(null, null, ZoneOffset.UTC, clusterService, + item -> { + assertFalse(historyStoreCalled.getAndSet(true)); + final SnapshotLifecyclePolicy policy = slpm.getPolicy(); + assertEquals(policy.getId(), item.getPolicyId()); + assertEquals(policy.getRepository(), item.getRepository()); + assertEquals(policy.getConfig(), item.getSnapshotConfiguration()); + assertEquals(snapshotName.get(), item.getSnapshotName()); + }); SnapshotLifecycleTask task = new SnapshotLifecycleTask(client, clusterService, historyStore); // Trigger the event with a matching job name for the policy task.triggered(new SchedulerEngine.Event(SnapshotLifecycleService.getJobId(slpm), System.currentTimeMillis(), System.currentTimeMillis())); - assertTrue("snapshot should be triggered once", called.get()); + assertTrue("snapshot should be triggered once", clientCalled.get()); + assertTrue("history store should be called once", historyStoreCalled.get()); } - verify(historyStore, times(1)).putAsync(any()); threadPool.shutdownNow(); } From 24a468ed8012c51abfbae60bd4f1816d2bed9ead Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 3 May 2019 17:14:39 -0600 Subject: [PATCH 10/18] More Javadoc --- .../history/SnapshotLifecycleTemplateRegistry.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java index b46379cb058e9..cfadf4c475cd0 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java @@ -27,6 +27,10 @@ import static org.elasticsearch.xpack.core.ClientHelper.INDEX_LIFECYCLE_ORIGIN; +/** + * Manages the index template and associated ILM policy for the Snapshot + * Lifecycle Management history index. + */ public class SnapshotLifecycleTemplateRegistry extends IndexTemplateRegistry { // history (please add a comment why you increased the version here) // version 1: initial From 1d77840aefefb9d4d0c2078ce53b2fa364b48862 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Mon, 6 May 2019 17:16:15 -0600 Subject: [PATCH 11/18] Add setting to disable history index In addition to being a good addition, this also fixes some of the ILM tests. --- .../core/indexlifecycle/LifecycleSettings.java | 5 +++++ .../history/SnapshotHistoryStore.java | 12 ++++++++++-- .../SnapshotLifecycleTemplateRegistry.java | 10 ++++++++++ .../history/SnapshotHistoryStoreTests.java | 18 +++++++++++++++++- ...SnapshotLifecycleTemplateRegistryTests.java | 9 +++++++++ .../xpack/indexlifecycle/IndexLifecycle.java | 5 +++-- .../IndexLifecycleInitialisationTests.java | 3 +++ .../SnapshotLifecycleTaskTests.java | 3 ++- 8 files changed, 59 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecycleSettings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecycleSettings.java index 3cfd8556244a9..0a157b8197a10 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecycleSettings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecycleSettings.java @@ -16,10 +16,15 @@ public class LifecycleSettings { public static final String LIFECYCLE_NAME = "index.lifecycle.name"; public static final String LIFECYCLE_INDEXING_COMPLETE = "index.lifecycle.indexing_complete"; + public static final String SLM_HISTORY_INDEX_ENABLED = "slm.history_index_enabled"; + public static final Setting LIFECYCLE_POLL_INTERVAL_SETTING = Setting.timeSetting(LIFECYCLE_POLL_INTERVAL, TimeValue.timeValueMinutes(10), TimeValue.timeValueSeconds(1), Setting.Property.Dynamic, Setting.Property.NodeScope); public static final Setting LIFECYCLE_NAME_SETTING = Setting.simpleString(LIFECYCLE_NAME, Setting.Property.Dynamic, Setting.Property.IndexScope); public static final Setting LIFECYCLE_INDEXING_COMPLETE_SETTING = Setting.boolSetting(LIFECYCLE_INDEXING_COMPLETE, false, Setting.Property.Dynamic, Setting.Property.IndexScope); + + public static final Setting SLM_HISTORY_INDEX_ENABLED_SETTING = Setting.boolSetting(SLM_HISTORY_INDEX_ENABLED, true, + Setting.Property.NodeScope); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java index b279cb52e3047..8ebfa6423bcb9 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java @@ -16,6 +16,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateListener; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -27,6 +28,7 @@ import java.time.ZonedDateTime; import java.util.concurrent.atomic.AtomicBoolean; +import static org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings.SLM_HISTORY_INDEX_ENABLED_SETTING; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.INDEX_TEMPLATE_VERSION; /** @@ -43,12 +45,13 @@ public class SnapshotHistoryStore implements ClusterStateListener { private final Client client; private final ZoneId timeZone; private final AtomicBoolean readyToIndex = new AtomicBoolean(false); + private final boolean slmHistoryEnabled; - public SnapshotHistoryStore(SnapshotLifecycleTemplateRegistry registry, Client client, ZoneId timeZone, - ClusterService clusterService) { + public SnapshotHistoryStore(Settings nodeSettings, Client client, ZoneId timeZone, ClusterService clusterService, SnapshotLifecycleTemplateRegistry registry) { this.registry = registry; this.client = client; this.timeZone = timeZone; + slmHistoryEnabled = SLM_HISTORY_INDEX_ENABLED_SETTING.get(nodeSettings); clusterService.addListener(this); } @@ -58,6 +61,11 @@ public SnapshotHistoryStore(SnapshotLifecycleTemplateRegistry registry, Client c * @param item The entry to index */ public void putAsync(SnapshotHistoryItem item) { + if (slmHistoryEnabled == false) { + logger.trace("not recording snapshot history item because [{}] is [false]: [{}]", + SLM_HISTORY_INDEX_ENABLED_SETTING.getKey(), item); + return; + } final ZonedDateTime dateTime = ZonedDateTime.ofInstant(Instant.ofEpochMilli(item.getTimestamp()), timeZone); final String index = getHistoryIndexNameForTime(dateTime); logger.trace("about to index snapshot history item in index [{}]: [{}]", index, item); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java index cfadf4c475cd0..27e5e63013e56 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistry.java @@ -26,6 +26,7 @@ import java.util.stream.Collectors; import static org.elasticsearch.xpack.core.ClientHelper.INDEX_LIFECYCLE_ORIGIN; +import static org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings.SLM_HISTORY_INDEX_ENABLED_SETTING; /** * Manages the index template and associated ILM policy for the Snapshot @@ -53,18 +54,27 @@ public class SnapshotLifecycleTemplateRegistry extends IndexTemplateRegistry { "/slm-history-ilm-policy.json" ); + private final boolean slmHistoryEnabled; + public SnapshotLifecycleTemplateRegistry(Settings nodeSettings, ClusterService clusterService, ThreadPool threadPool, Client client, NamedXContentRegistry xContentRegistry) { super(nodeSettings, clusterService, threadPool, client, xContentRegistry); + slmHistoryEnabled = SLM_HISTORY_INDEX_ENABLED_SETTING.get(nodeSettings); } @Override protected List getTemplateConfigs() { + if (slmHistoryEnabled == false) { + return Collections.emptyList(); + } return Collections.singletonList(TEMPLATE_SLM_HISTORY); } @Override protected List getPolicyConfigs() { + if (slmHistoryEnabled == false) { + return Collections.emptyList(); + } return Collections.singletonList(SLM_HISTORY_POLICY); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java index 8942ed235d0a8..e3bdd3da94d83 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java @@ -24,10 +24,12 @@ import java.util.HashMap; import java.util.Map; +import static org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings.SLM_HISTORY_INDEX_ENABLED_SETTING; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore.getHistoryIndexNameForTime; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.INDEX_TEMPLATE_VERSION; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.core.IsEqual.equalTo; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.notNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -52,7 +54,21 @@ public void setup() { clusterService = mock(ClusterService.class); registry = mock(SnapshotLifecycleTemplateRegistry.class); - historyStore = new SnapshotHistoryStore(registry, client, ZoneOffset.UTC, clusterService); + historyStore = new SnapshotHistoryStore(settings, client, ZoneOffset.UTC, clusterService, registry); + } + + public void testNoActionIfDisabled() { + Settings settings = Settings.builder().put(SLM_HISTORY_INDEX_ENABLED_SETTING.getKey(), false).build(); + SnapshotHistoryStore disabledHistoryStore = new SnapshotHistoryStore(settings, client, ZoneOffset.UTC, clusterService, registry); + String policyId = randomAlphaOfLength(5); + SnapshotLifecyclePolicy policy = randomSnapshotLifecyclePolicy(policyId); + final long timestamp = randomNonNegativeLong(); + SnapshotLifecyclePolicy.ResolverContext context = new SnapshotLifecyclePolicy.ResolverContext(timestamp); + String snapshotId = policy.generateSnapshotName(context); + SnapshotHistoryItem record = SnapshotHistoryItem.successRecord(timestamp, policy, snapshotId); + + disabledHistoryStore.putAsync(record); + verify(client, times(0)).index(any(), any()); } @SuppressWarnings("unchecked") diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java index 99f7f8c2af5a0..08b41ef5901e5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java @@ -57,6 +57,7 @@ import static org.elasticsearch.mock.orig.Mockito.verify; import static org.elasticsearch.mock.orig.Mockito.when; +import static org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings.SLM_HISTORY_INDEX_ENABLED_SETTING; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.SLM_POLICY_NAME; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.SLM_TEMPLATE_NAME; import static org.hamcrest.Matchers.equalTo; @@ -106,6 +107,14 @@ public void createRegistryAndClient() { registry = new SnapshotLifecycleTemplateRegistry(Settings.EMPTY, clusterService, threadPool, client, xContentRegistry); } + public void testDisabledDoesNotAddTemplates() { + Settings settings = Settings.builder().put(SLM_HISTORY_INDEX_ENABLED_SETTING.getKey(), false).build(); + SnapshotLifecycleTemplateRegistry disabledRegistry = new SnapshotLifecycleTemplateRegistry(settings, clusterService, threadPool, + client, xContentRegistry); + assertThat(disabledRegistry.getTemplateConfigs(), hasSize(0)); + assertThat(disabledRegistry.getPolicyConfigs(), hasSize(0)); + } + public void testThatNonExistingTemplatesAreAddedImmediately() { DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java index 1ccefbcedd978..3d8f546e4c8a8 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java @@ -146,7 +146,8 @@ public List> getSettings() { LifecycleSettings.LIFECYCLE_POLL_INTERVAL_SETTING, LifecycleSettings.LIFECYCLE_NAME_SETTING, LifecycleSettings.LIFECYCLE_INDEXING_COMPLETE_SETTING, - RolloverAction.LIFECYCLE_ROLLOVER_ALIAS_SETTING); + RolloverAction.LIFECYCLE_ROLLOVER_ALIAS_SETTING, + LifecycleSettings.SLM_HISTORY_INDEX_ENABLED_SETTING); } @Override @@ -161,7 +162,7 @@ public Collection createComponents(Client client, ClusterService cluster getClock(), System::currentTimeMillis, xContentRegistry)); SnapshotLifecycleTemplateRegistry templateRegistry = new SnapshotLifecycleTemplateRegistry(settings, clusterService, threadPool, client, xContentRegistry); - snapshotHistoryStore.set(new SnapshotHistoryStore(templateRegistry, client, getClock().getZone(), clusterService)); + snapshotHistoryStore.set(new SnapshotHistoryStore(settings, client, getClock().getZone(), clusterService, templateRegistry)); snapshotLifecycleService.set(new SnapshotLifecycleService(settings, () -> new SnapshotLifecycleTask(client, clusterService, snapshotHistoryStore.get()), clusterService, getClock())); return Arrays.asList(indexLifecycleInitialisationService.get(), snapshotLifecycleService.get(), snapshotHistoryStore.get()); diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleInitialisationTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleInitialisationTests.java index a1a37beb1d129..490804bdd6f74 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleInitialisationTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleInitialisationTests.java @@ -101,6 +101,9 @@ protected Settings nodeSettings(int nodeOrdinal) { settings.put(XPackSettings.GRAPH_ENABLED.getKey(), false); settings.put(XPackSettings.LOGSTASH_ENABLED.getKey(), false); settings.put(LifecycleSettings.LIFECYCLE_POLL_INTERVAL, "1s"); + + // This is necessary to prevent SLM installing a lifecycle policy, these tests assume a blank slate + settings.put(LifecycleSettings.SLM_HISTORY_INDEX_ENABLED_SETTING.getKey(), false); return settings.build(); } diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java index 95b2d7e212eae..3e468325d02ec 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java @@ -21,6 +21,7 @@ import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.TriFunction; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.ESTestCase; @@ -233,7 +234,7 @@ public static class VerifyingHistoryStore extends SnapshotHistoryStore { public VerifyingHistoryStore(SnapshotLifecycleTemplateRegistry registry, Client client, ZoneId timeZone, ClusterService clusterService, Consumer verifier) { - super(registry, client, timeZone, clusterService); + super(Settings.EMPTY, client, timeZone, clusterService, registry); this.verifier = verifier; } From db674c6de730266e6fd3eb3c0ecbf20bd7dd1b13 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Tue, 7 May 2019 16:49:17 -0600 Subject: [PATCH 12/18] Improve integration test reliability --- .../SnapshotLifecycleIT.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 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 e4e5d92a899a8..59710aa6d8f96 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 @@ -211,6 +211,7 @@ public void testPolicyManualExecution() throws Exception { }); } + // 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"); historySearchRequest.setJsonEntity("{\n" + @@ -241,13 +242,20 @@ private void assertHistoryIsPresent(String policyName, boolean success, String r " }\n" + " }\n" + "}"); - Response historyResponse = client().performRequest(historySearchRequest); - Map historyResponseMap; - try (InputStream is = historyResponse.getEntity().getContent()) { - historyResponseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); + Response historyResponse; + try { + historyResponse = client().performRequest(historySearchRequest); + Map historyResponseMap; + try (InputStream is = historyResponse.getEntity().getContent()) { + historyResponseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); + } + assertThat((int)((Map) ((Map) historyResponseMap.get("hits")).get("total")).get("value"), + greaterThanOrEqualTo(1)); + } catch (ResponseException e) { + // Throw AssertionError instead of an exception if the search fails so that assertBusy works as expected + logger.error(e); + fail("failed to perform search:" + e.getMessage()); } - assertThat((int)((Map) ((Map) historyResponseMap.get("hits")).get("total")).get("value"), - greaterThanOrEqualTo(1)); } private void createSnapshotPolicy(String policyName, String snapshotNamePattern, String schedule, String repoId, From 483cd516781afbd0515dde84073c1d112a07c1fd Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Tue, 7 May 2019 17:31:58 -0600 Subject: [PATCH 13/18] De-mockify registry tests --- ...napshotLifecycleTemplateRegistryTests.java | 196 +++++++++++------- 1 file changed, 125 insertions(+), 71 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java index 08b41ef5901e5..9efc7a9025b99 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java @@ -7,12 +7,13 @@ package org.elasticsearch.xpack.core.snapshotlifecycle.history; import org.elasticsearch.Version; +import org.elasticsearch.action.Action; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateAction; import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequest; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.client.AdminClient; -import org.elasticsearch.client.Client; -import org.elasticsearch.client.IndicesAdminClient; import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.ClusterName; @@ -24,15 +25,17 @@ import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.TriFunction; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.EsExecutors; -import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.client.NoOpClient; +import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.indexlifecycle.DeleteAction; import org.elasticsearch.xpack.core.indexlifecycle.IndexLifecycleMetadata; @@ -43,8 +46,8 @@ import org.elasticsearch.xpack.core.indexlifecycle.OperationMode; import org.elasticsearch.xpack.core.indexlifecycle.TimeseriesLifecycleType; import org.elasticsearch.xpack.core.indexlifecycle.action.PutLifecycleAction; +import org.junit.After; import org.junit.Before; -import org.mockito.ArgumentCaptor; import java.io.IOException; import java.util.ArrayList; @@ -53,51 +56,31 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; -import static org.elasticsearch.mock.orig.Mockito.verify; import static org.elasticsearch.mock.orig.Mockito.when; import static org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings.SLM_HISTORY_INDEX_ENABLED_SETTING; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.SLM_POLICY_NAME; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.SLM_TEMPLATE_NAME; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyObject; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.doAnswer; +import static org.hamcrest.Matchers.instanceOf; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verifyZeroInteractions; public class SnapshotLifecycleTemplateRegistryTests extends ESTestCase { private SnapshotLifecycleTemplateRegistry registry; private NamedXContentRegistry xContentRegistry; private ClusterService clusterService; private ThreadPool threadPool; - private Client client; + private VerifyingClient client; @Before public void createRegistryAndClient() { - threadPool = mock(ThreadPool.class); - when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); - when(threadPool.generic()).thenReturn(EsExecutors.newDirectExecutorService()); - - client = mock(Client.class); - when(client.threadPool()).thenReturn(threadPool); - AdminClient adminClient = mock(AdminClient.class); - IndicesAdminClient indicesAdminClient = mock(IndicesAdminClient.class); - when(adminClient.indices()).thenReturn(indicesAdminClient); - when(client.admin()).thenReturn(adminClient); - doAnswer(invocationOnMock -> { - ActionListener listener = - (ActionListener) invocationOnMock.getArguments()[1]; - listener.onResponse(new TestPutIndexTemplateResponse(true)); - return null; - }).when(indicesAdminClient).putTemplate(any(PutIndexTemplateRequest.class), any(ActionListener.class)); - - clusterService = mock(ClusterService.class); + threadPool = new TestThreadPool(this.getClass().getName()); + client = new VerifyingClient(threadPool); + clusterService = ClusterServiceUtils.createClusterService(threadPool); List entries = new ArrayList<>(ClusterModule.getNamedXWriteables()); entries.addAll(Arrays.asList( new NamedXContentRegistry.Entry(LifecycleType.class, new ParseField(TimeseriesLifecycleType.TYPE), @@ -107,6 +90,13 @@ public void createRegistryAndClient() { registry = new SnapshotLifecycleTemplateRegistry(Settings.EMPTY, clusterService, threadPool, client, xContentRegistry); } + @After + @Override + public void tearDown() throws Exception { + super.tearDown(); + threadPool.shutdownNow(); + } + public void testDisabledDoesNotAddTemplates() { Settings settings = Settings.builder().put(SLM_HISTORY_INDEX_ENABLED_SETTING.getKey(), false).build(); SnapshotLifecycleTemplateRegistry disabledRegistry = new SnapshotLifecycleTemplateRegistry(settings, clusterService, threadPool, @@ -115,34 +105,67 @@ public void testDisabledDoesNotAddTemplates() { assertThat(disabledRegistry.getPolicyConfigs(), hasSize(0)); } - public void testThatNonExistingTemplatesAreAddedImmediately() { + public void testThatNonExistingTemplatesAreAddedImmediately() throws Exception { DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); ClusterChangedEvent event = createClusterChangedEvent(Collections.emptyList(), nodes); + + AtomicInteger calledTimes = new AtomicInteger(0); + client.setVerifier((action, request, listener) -> { + if (action instanceof PutIndexTemplateAction) { + calledTimes.incrementAndGet(); + assertThat(action, instanceOf(PutIndexTemplateAction.class)); + assertThat(request, instanceOf(PutIndexTemplateRequest.class)); + final PutIndexTemplateRequest putRequest = (PutIndexTemplateRequest) request; + assertThat(putRequest.name(), equalTo(SLM_TEMPLATE_NAME)); + assertThat(putRequest.settings().get("index.lifecycle.name"), equalTo(SLM_POLICY_NAME)); + assertNotNull(listener); + return new TestPutIndexTemplateResponse(true); + } else if (action instanceof PutLifecycleAction) { + // Ignore this, it's verified in another test + return new PutLifecycleAction.Response(true); + } else { + fail("client called with unexpected request:" + request.toString()); + return null; + } + }); registry.clusterChanged(event); - ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(PutIndexTemplateRequest.class); - verify(client.admin().indices(), times(registry.getTemplateConfigs().size())).putTemplate(argumentCaptor.capture(), anyObject()); + assertBusy(() -> assertThat(calledTimes.get(), equalTo(registry.getTemplateConfigs().size()))); + calledTimes.set(0); // now delete one template from the cluster state and lets retry ClusterChangedEvent newEvent = createClusterChangedEvent(Collections.emptyList(), nodes); registry.clusterChanged(newEvent); - ArgumentCaptor captor = ArgumentCaptor.forClass(PutIndexTemplateRequest.class); - verify(client.admin().indices(), times(registry.getTemplateConfigs().size() + 1)).putTemplate(captor.capture(), anyObject()); - PutIndexTemplateRequest req = captor.getAllValues().stream() - .filter(r -> r.name().equals(SLM_TEMPLATE_NAME)) - .findFirst() - .orElseThrow(() -> new AssertionError("expected the slm history template to be put")); - assertThat(req.settings().get("index.lifecycle.name"), equalTo(SLM_POLICY_NAME)); + assertBusy(() -> assertThat(calledTimes.get(), equalTo(1))); } - public void testThatNonExistingPoliciesAreAddedImmediately() { + public void testThatNonExistingPoliciesAreAddedImmediately() throws Exception { DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); + AtomicInteger calledTimes = new AtomicInteger(0); + client.setVerifier((action, request, listener) -> { + if (action instanceof PutLifecycleAction) { + calledTimes.incrementAndGet(); + assertThat(action, instanceOf(PutLifecycleAction.class)); + assertThat(request, instanceOf(PutLifecycleAction.Request.class)); + final PutLifecycleAction.Request putRequest = (PutLifecycleAction.Request) request; + assertThat(putRequest.getPolicy().getName(), equalTo(SLM_POLICY_NAME)); + assertNotNull(listener); + return new PutLifecycleAction.Response(true); + } else if (action instanceof PutIndexTemplateAction) { + // Ignore this, it's verified in another test + return new TestPutIndexTemplateResponse(true); + } else { + fail("client called with unexpected request:" + request.toString()); + return null; + } + }); + ClusterChangedEvent event = createClusterChangedEvent(Collections.emptyList(), nodes); registry.clusterChanged(event); - verify(client, times(registry.getPolicyConfigs().size())).execute(eq(PutLifecycleAction.INSTANCE), anyObject(), anyObject()); + assertBusy(() -> assertThat(calledTimes.get(), equalTo(1))); } public void testPolicyAlreadyExists() { @@ -156,12 +179,24 @@ public void testPolicyAlreadyExists() { assertThat(policies, hasSize(1)); LifecyclePolicy policy = policies.get(0); policyMap.put(policy.getName(), policy); + + client.setVerifier((action, request, listener) -> { + if (action instanceof PutIndexTemplateAction) { + // Ignore this, it's verified in another test + return new TestPutIndexTemplateResponse(true); + } else if (action instanceof PutLifecycleAction) { + fail("if the policy already exists it should be re-put"); + } else { + fail("client called with unexpected request:" + request.toString()); + } + return null; + }); + ClusterChangedEvent event = createClusterChangedEvent(Collections.emptyList(), policyMap, nodes); registry.clusterChanged(event); - verify(client, times(0)).execute(eq(PutLifecycleAction.INSTANCE), anyObject(), anyObject()); } - public void testThatTemplatesExist() throws IOException { + public void testPolicyAlreadyExistsButDiffers() throws IOException { DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); @@ -172,13 +207,25 @@ public void testThatTemplatesExist() throws IOException { .collect(Collectors.toList()); assertThat(policies, hasSize(1)); LifecyclePolicy policy = policies.get(0); + + client.setVerifier((action, request, listener) -> { + if (action instanceof PutIndexTemplateAction) { + // Ignore this, it's verified in another test + return new TestPutIndexTemplateResponse(true); + } else if (action instanceof PutLifecycleAction) { + fail("if the policy already exists it should be re-put"); + } else { + fail("client called with unexpected request:" + request.toString()); + } + return null; + }); + try (XContentParser parser = XContentType.JSON.xContent() - .createParser(xContentRegistry, LoggingDeprecationHandler.THROW_UNSUPPORTED_OPERATION, policyStr)) { + .createParser(xContentRegistry, LoggingDeprecationHandler.THROW_UNSUPPORTED_OPERATION, policyStr)) { LifecyclePolicy different = LifecyclePolicy.parse(parser, policy.getName()); policyMap.put(policy.getName(), different); ClusterChangedEvent event = createClusterChangedEvent(Collections.emptyList(), policyMap, nodes); registry.clusterChanged(event); - verify(client, times(0)).execute(eq(PutLifecycleAction.INSTANCE), anyObject(), anyObject()); } } @@ -186,31 +233,13 @@ public void testThatMissingMasterNodeDoesNothing() { DiscoveryNode localNode = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").add(localNode).build(); + client.setVerifier((a,r,l) -> { + fail("if the master is missing nothing should happen"); + return null; + }); + ClusterChangedEvent event = createClusterChangedEvent(Arrays.asList(SLM_TEMPLATE_NAME), nodes); registry.clusterChanged(event); - - verifyZeroInteractions(client); - } - - public void testPolicyAlreadyExistsButDiffers() throws IOException { - DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); - DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); - - Map policyMap = new HashMap<>(); - String policyStr = "{\"phases\":{\"delete\":{\"min_age\":\"1m\",\"actions\":{\"delete\":{}}}}}"; - List policies = registry.getPolicyConfigs().stream() - .map(policyConfig -> policyConfig.load(xContentRegistry)) - .collect(Collectors.toList()); - assertThat(policies, hasSize(1)); - LifecyclePolicy policy = policies.get(0); - try (XContentParser parser = XContentType.JSON.xContent() - .createParser(xContentRegistry, LoggingDeprecationHandler.THROW_UNSUPPORTED_OPERATION, policyStr)) { - LifecyclePolicy different = LifecyclePolicy.parse(parser, policy.getName()); - policyMap.put(policy.getName(), different); - ClusterChangedEvent event = createClusterChangedEvent(Collections.emptyList(), policyMap, nodes); - registry.clusterChanged(event); - verify(client, times(0)).execute(eq(PutLifecycleAction.INSTANCE), anyObject(), anyObject()); - } } public void testValidate() { @@ -226,6 +255,31 @@ public void testValidate() { // ------------- + /** + * A client that delegates to a verifying function for action/request/listener + */ + public static class VerifyingClient extends NoOpClient { + + private TriFunction, ActionRequest, ActionListener, ActionResponse> verifier; + + VerifyingClient(ThreadPool threadPool) { + super(threadPool); + } + + @Override + @SuppressWarnings("unchecked") + protected void doExecute(Action action, + Request request, + ActionListener listener) { + listener.onResponse((Response) verifier.apply(action, request, listener)); + } + + public VerifyingClient setVerifier(TriFunction, ActionRequest, ActionListener, ActionResponse> verifier) { + this.verifier = verifier; + return this; + } + } + private ClusterChangedEvent createClusterChangedEvent(List existingTemplateNames, DiscoveryNodes nodes) { return createClusterChangedEvent(existingTemplateNames, Collections.emptyMap(), nodes); } From 38deb63ba8ee741cf8d7ef1493ab7070b6944755 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Tue, 7 May 2019 17:35:58 -0600 Subject: [PATCH 14/18] Explicitly fail if the verifier isn't set --- .../history/SnapshotLifecycleTemplateRegistryTests.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java index 9efc7a9025b99..7c086d53de408 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotLifecycleTemplateRegistryTests.java @@ -260,7 +260,10 @@ public void testValidate() { */ public static class VerifyingClient extends NoOpClient { - private TriFunction, ActionRequest, ActionListener, ActionResponse> verifier; + private TriFunction, ActionRequest, ActionListener, ActionResponse> verifier = (a,r,l) -> { + fail("verifier not set"); + return null; + }; VerifyingClient(ThreadPool threadPool) { super(threadPool); From ea8b159f9c3d66ed2e0507c905bf426f9624c235 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Tue, 7 May 2019 17:50:34 -0600 Subject: [PATCH 15/18] De-mockify history store tests --- .../history/SnapshotHistoryStoreTests.java | 134 +++++++++++------- 1 file changed, 84 insertions(+), 50 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java index e3bdd3da94d83..9ac5cabe2e446 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java @@ -6,55 +6,71 @@ package org.elasticsearch.xpack.core.snapshotlifecycle.history; -import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.index.IndexAction; import org.elasticsearch.action.index.IndexRequest; -import org.elasticsearch.client.Client; +import org.elasticsearch.action.index.IndexResponse; +import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.indexlifecycle.DeleteAction; +import org.elasticsearch.xpack.core.indexlifecycle.LifecycleAction; +import org.elasticsearch.xpack.core.indexlifecycle.LifecycleType; +import org.elasticsearch.xpack.core.indexlifecycle.TimeseriesLifecycleType; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicy; +import org.junit.After; import org.junit.Before; -import org.mockito.ArgumentCaptor; -import java.io.IOException; import java.time.Instant; import java.time.ZoneOffset; +import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import static org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings.SLM_HISTORY_INDEX_ENABLED_SETTING; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore.getHistoryIndexNameForTime; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.INDEX_TEMPLATE_VERSION; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.core.IsEqual.equalTo; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.notNull; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; public class SnapshotHistoryStoreTests extends ESTestCase { + private ThreadPool threadPool; private ClusterService clusterService; - private Client client; + private SnapshotLifecycleTemplateRegistryTests.VerifyingClient client; private SnapshotHistoryStore historyStore; private SnapshotLifecycleTemplateRegistry registry; @Before public void setup() { - Settings settings = Settings.builder().put("node.name", randomAlphaOfLength(10)).build(); - client = mock(Client.class); - ThreadPool threadPool = mock(ThreadPool.class); - when(client.threadPool()).thenReturn(threadPool); - when(client.settings()).thenReturn(settings); - when(threadPool.getThreadContext()).thenReturn(new ThreadContext(settings)); - clusterService = mock(ClusterService.class); - registry = mock(SnapshotLifecycleTemplateRegistry.class); - - historyStore = new SnapshotHistoryStore(settings, client, ZoneOffset.UTC, clusterService, registry); + threadPool = new TestThreadPool(this.getClass().getName()); + client = new SnapshotLifecycleTemplateRegistryTests.VerifyingClient(threadPool); + clusterService = ClusterServiceUtils.createClusterService(threadPool); + List entries = new ArrayList<>(ClusterModule.getNamedXWriteables()); + entries.addAll(Arrays.asList( + new NamedXContentRegistry.Entry(LifecycleType.class, new ParseField(TimeseriesLifecycleType.TYPE), + (p) -> TimeseriesLifecycleType.INSTANCE), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(DeleteAction.NAME), DeleteAction::parse))); + NamedXContentRegistry xContentRegistry = new NamedXContentRegistry(entries); + registry = new SnapshotLifecycleTemplateRegistry(Settings.EMPTY, clusterService, threadPool, client, xContentRegistry); + + historyStore = new SnapshotHistoryStore(Settings.EMPTY, client, ZoneOffset.UTC, clusterService, registry); + } + + @After + @Override + public void tearDown() throws Exception { + super.tearDown(); + threadPool.shutdownNow(); } public void testNoActionIfDisabled() { @@ -67,12 +83,15 @@ public void testNoActionIfDisabled() { String snapshotId = policy.generateSnapshotName(context); SnapshotHistoryItem record = SnapshotHistoryItem.successRecord(timestamp, policy, snapshotId); + client.setVerifier((a,r,l) -> { + fail("the history store is disabled, no action should have been taken"); + return null; + }); disabledHistoryStore.putAsync(record); - verify(client, times(0)).index(any(), any()); } @SuppressWarnings("unchecked") - public void testPut() throws IOException { + public void testPut() throws Exception { String policyId = randomAlphaOfLength(5); SnapshotLifecyclePolicy policy = randomSnapshotLifecyclePolicy(policyId); final long timestamp = randomNonNegativeLong(); @@ -81,40 +100,55 @@ public void testPut() throws IOException { { SnapshotHistoryItem record = SnapshotHistoryItem.successRecord(timestamp, policy, snapshotId); + AtomicInteger calledTimes = new AtomicInteger(0); + client.setVerifier((action, request, listener) -> { + calledTimes.incrementAndGet(); + assertThat(action, instanceOf(IndexAction.class)); + assertThat(request, instanceOf(IndexRequest.class)); + IndexRequest indexRequest = (IndexRequest) request; + assertEquals(getHistoryIndexNameForTime(Instant.ofEpochMilli(timestamp).atZone(ZoneOffset.UTC)), indexRequest.index()); + final String indexedDocument = indexRequest.source().utf8ToString(); + assertThat(indexedDocument, containsString(policy.getId())); + assertThat(indexedDocument, containsString(policy.getRepository())); + assertThat(indexedDocument, containsString(snapshotId)); + if (policy.getConfig() != null) { + assertContainsMap(indexedDocument, policy.getConfig()); + } + assertNotNull(listener); + return new IndexResponse(); + }); + historyStore.putAsync(record); - ArgumentCaptor indexRequest = ArgumentCaptor.forClass(IndexRequest.class); - verify(client, times(1)).index(indexRequest.capture(), notNull(ActionListener.class)); - - assertEquals(getHistoryIndexNameForTime(Instant.ofEpochMilli(timestamp).atZone(ZoneOffset.UTC)), - indexRequest.getValue().index()); - final String indexedDocument = indexRequest.getValue().source().utf8ToString(); - assertThat(indexedDocument, containsString(policy.getId())); - assertThat(indexedDocument, containsString(policy.getRepository())); - assertThat(indexedDocument, containsString(snapshotId)); - if (policy.getConfig() != null) { - assertContainsMap(indexedDocument, policy.getConfig()); - } + assertBusy(() -> assertThat(calledTimes.get(), equalTo(1))); } { final String cause = randomAlphaOfLength(9); Exception failureException = new RuntimeException(cause); SnapshotHistoryItem record = SnapshotHistoryItem.failureRecord(timestamp, policy, snapshotId, failureException); + + AtomicInteger calledTimes = new AtomicInteger(0); + client.setVerifier((action, request, listener) -> { + calledTimes.incrementAndGet(); + assertThat(action, instanceOf(IndexAction.class)); + assertThat(request, instanceOf(IndexRequest.class)); + IndexRequest indexRequest = (IndexRequest) request; + assertEquals(getHistoryIndexNameForTime(Instant.ofEpochMilli(timestamp).atZone(ZoneOffset.UTC)), indexRequest.index()); + final String indexedDocument = indexRequest.source().utf8ToString(); + assertThat(indexedDocument, containsString(policy.getId())); + assertThat(indexedDocument, containsString(policy.getRepository())); + assertThat(indexedDocument, containsString(snapshotId)); + if (policy.getConfig() != null) { + assertContainsMap(indexedDocument, policy.getConfig()); + } + assertThat(indexedDocument, containsString("runtime_exception")); + assertThat(indexedDocument, containsString(cause)); + assertNotNull(listener); + return new IndexResponse(); + }); + historyStore.putAsync(record); - ArgumentCaptor indexRequest = ArgumentCaptor.forClass(IndexRequest.class); - verify(client, times(2)).index(indexRequest.capture(), notNull(ActionListener.class)); - - assertEquals(getHistoryIndexNameForTime(Instant.ofEpochMilli(timestamp).atZone(ZoneOffset.UTC)), - indexRequest.getValue().index()); - final String indexedDocument = indexRequest.getValue().source().utf8ToString(); - assertThat(indexedDocument, containsString(policy.getId())); - assertThat(indexedDocument, containsString(policy.getRepository())); - assertThat(indexedDocument, containsString(snapshotId)); - if (policy.getConfig() != null) { - assertContainsMap(indexedDocument, policy.getConfig()); - } - assertThat(indexedDocument, containsString("runtime_exception")); - assertThat(indexedDocument, containsString(cause)); + assertBusy(() -> assertThat(calledTimes.get(), equalTo(1))); } } From b4e124c499664dcb2b18d8f4bbd8093cd53e1008 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Tue, 7 May 2019 18:14:54 -0600 Subject: [PATCH 16/18] Remove some unused code --- .../history/SnapshotHistoryItem.java | 3 ++- .../history/SnapshotHistoryStore.java | 21 ++------------- .../history/SnapshotHistoryStoreTests.java | 27 ++----------------- .../xpack/indexlifecycle/IndexLifecycle.java | 2 +- .../SnapshotLifecycleTaskTests.java | 10 +++---- 5 files changed, 11 insertions(+), 52 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java index c191aed399ed5..1c5a9ef751e9d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java @@ -212,7 +212,8 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(Objects.hash(getTimestamp(), getPolicyId(), getRepository(), getSnapshotName(), getOperation(), isSuccess()), getSnapshotConfiguration(), getErrorDetails()); + return Objects.hash(getTimestamp(), getPolicyId(), getRepository(), getSnapshotName(), getOperation(), isSuccess(), + getSnapshotConfiguration(), getErrorDetails()); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java index 8ebfa6423bcb9..83ceb17ba3a80 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStore.java @@ -12,10 +12,6 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.client.Client; -import org.elasticsearch.cluster.ClusterChangedEvent; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.ClusterStateListener; -import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.xcontent.ToXContent; @@ -26,7 +22,6 @@ import java.time.Instant; import java.time.ZoneId; import java.time.ZonedDateTime; -import java.util.concurrent.atomic.AtomicBoolean; import static org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings.SLM_HISTORY_INDEX_ENABLED_SETTING; import static org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry.INDEX_TEMPLATE_VERSION; @@ -35,24 +30,20 @@ * Records Snapshot Lifecycle Management actions as represented by {@link SnapshotHistoryItem} into an index * for the purposes of querying and alerting. */ -public class SnapshotHistoryStore implements ClusterStateListener { +public class SnapshotHistoryStore { private static final Logger logger = LogManager.getLogger(SnapshotHistoryStore.class); private static final DateFormatter indexTimeFormat = DateFormatter.forPattern("yyyy.MM"); public static final String SLM_HISTORY_INDEX_PREFIX = ".slm-history-" + INDEX_TEMPLATE_VERSION + "-"; - private final SnapshotLifecycleTemplateRegistry registry; private final Client client; private final ZoneId timeZone; - private final AtomicBoolean readyToIndex = new AtomicBoolean(false); private final boolean slmHistoryEnabled; - public SnapshotHistoryStore(Settings nodeSettings, Client client, ZoneId timeZone, ClusterService clusterService, SnapshotLifecycleTemplateRegistry registry) { - this.registry = registry; + public SnapshotHistoryStore(Settings nodeSettings, Client client, ZoneId timeZone) { this.client = client; this.timeZone = timeZone; slmHistoryEnabled = SLM_HISTORY_INDEX_ENABLED_SETTING.get(nodeSettings); - clusterService.addListener(this); } /** @@ -86,16 +77,8 @@ public void putAsync(SnapshotHistoryItem item) { } } - boolean validate(ClusterState state) { - return registry.validate(state); - } static String getHistoryIndexNameForTime(ZonedDateTime time) { return SLM_HISTORY_INDEX_PREFIX + indexTimeFormat.format(time); } - - @Override - public void clusterChanged(ClusterChangedEvent event) { - readyToIndex.set(validate(event.state())); - } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java index 9ac5cabe2e446..9457b2a36dd1e 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java @@ -9,29 +9,17 @@ import org.elasticsearch.action.index.IndexAction; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.index.IndexResponse; -import org.elasticsearch.cluster.ClusterModule; -import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.ParseField; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.xpack.core.indexlifecycle.DeleteAction; -import org.elasticsearch.xpack.core.indexlifecycle.LifecycleAction; -import org.elasticsearch.xpack.core.indexlifecycle.LifecycleType; -import org.elasticsearch.xpack.core.indexlifecycle.TimeseriesLifecycleType; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicy; import org.junit.After; import org.junit.Before; import java.time.Instant; import java.time.ZoneOffset; -import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; @@ -45,25 +33,14 @@ public class SnapshotHistoryStoreTests extends ESTestCase { private ThreadPool threadPool; - private ClusterService clusterService; private SnapshotLifecycleTemplateRegistryTests.VerifyingClient client; private SnapshotHistoryStore historyStore; - private SnapshotLifecycleTemplateRegistry registry; @Before public void setup() { threadPool = new TestThreadPool(this.getClass().getName()); client = new SnapshotLifecycleTemplateRegistryTests.VerifyingClient(threadPool); - clusterService = ClusterServiceUtils.createClusterService(threadPool); - List entries = new ArrayList<>(ClusterModule.getNamedXWriteables()); - entries.addAll(Arrays.asList( - new NamedXContentRegistry.Entry(LifecycleType.class, new ParseField(TimeseriesLifecycleType.TYPE), - (p) -> TimeseriesLifecycleType.INSTANCE), - new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(DeleteAction.NAME), DeleteAction::parse))); - NamedXContentRegistry xContentRegistry = new NamedXContentRegistry(entries); - registry = new SnapshotLifecycleTemplateRegistry(Settings.EMPTY, clusterService, threadPool, client, xContentRegistry); - - historyStore = new SnapshotHistoryStore(Settings.EMPTY, client, ZoneOffset.UTC, clusterService, registry); + historyStore = new SnapshotHistoryStore(Settings.EMPTY, client, ZoneOffset.UTC); } @After @@ -75,7 +52,7 @@ public void tearDown() throws Exception { public void testNoActionIfDisabled() { Settings settings = Settings.builder().put(SLM_HISTORY_INDEX_ENABLED_SETTING.getKey(), false).build(); - SnapshotHistoryStore disabledHistoryStore = new SnapshotHistoryStore(settings, client, ZoneOffset.UTC, clusterService, registry); + SnapshotHistoryStore disabledHistoryStore = new SnapshotHistoryStore(settings, client, ZoneOffset.UTC); String policyId = randomAlphaOfLength(5); SnapshotLifecyclePolicy policy = randomSnapshotLifecyclePolicy(policyId); final long timestamp = randomNonNegativeLong(); diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java index 3d8f546e4c8a8..e07533667e94c 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java @@ -162,7 +162,7 @@ public Collection createComponents(Client client, ClusterService cluster getClock(), System::currentTimeMillis, xContentRegistry)); SnapshotLifecycleTemplateRegistry templateRegistry = new SnapshotLifecycleTemplateRegistry(settings, clusterService, threadPool, client, xContentRegistry); - snapshotHistoryStore.set(new SnapshotHistoryStore(settings, client, getClock().getZone(), clusterService, templateRegistry)); + snapshotHistoryStore.set(new SnapshotHistoryStore(settings, client, getClock().getZone())); snapshotLifecycleService.set(new SnapshotLifecycleService(settings, () -> new SnapshotLifecycleTask(client, clusterService, snapshotHistoryStore.get()), clusterService, getClock())); return Arrays.asList(indexLifecycleInitialisationService.get(), snapshotLifecycleService.get(), snapshotHistoryStore.get()); diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java index 3e468325d02ec..999486415a141 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java @@ -35,7 +35,6 @@ import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicyMetadata; import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryItem; import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotHistoryStore; -import org.elasticsearch.xpack.core.snapshotlifecycle.history.SnapshotLifecycleTemplateRegistry; import java.io.IOException; import java.time.ZoneId; @@ -91,7 +90,7 @@ public void testSkipCreatingSnapshotWhenJobDoesNotMatch() { fail("should not have tried to take a snapshot"); return null; })) { - SnapshotHistoryStore historyStore = new VerifyingHistoryStore(null, null, ZoneOffset.UTC, clusterService, + SnapshotHistoryStore historyStore = new VerifyingHistoryStore(null, ZoneOffset.UTC, item -> fail("should not have tried to store an item")); SnapshotLifecycleTask task = new SnapshotLifecycleTask(client, clusterService, historyStore); @@ -172,7 +171,7 @@ public void testCreateSnapshotOnTrigger() { } })) { final AtomicBoolean historyStoreCalled = new AtomicBoolean(false); - SnapshotHistoryStore historyStore = new VerifyingHistoryStore(null, null, ZoneOffset.UTC, clusterService, + SnapshotHistoryStore historyStore = new VerifyingHistoryStore(null, ZoneOffset.UTC, item -> { assertFalse(historyStoreCalled.getAndSet(true)); final SnapshotLifecyclePolicy policy = slpm.getPolicy(); @@ -232,9 +231,8 @@ public static class VerifyingHistoryStore extends SnapshotHistoryStore { Consumer verifier; - public VerifyingHistoryStore(SnapshotLifecycleTemplateRegistry registry, Client client, ZoneId timeZone, - ClusterService clusterService, Consumer verifier) { - super(Settings.EMPTY, client, timeZone, clusterService, registry); + public VerifyingHistoryStore(Client client, ZoneId timeZone, Consumer verifier) { + super(Settings.EMPTY, client, timeZone); this.verifier = verifier; } From 64cbe40fa8eeb15895d8d87297d2d8ac74f5dec6 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Wed, 8 May 2019 16:43:19 -0600 Subject: [PATCH 17/18] Remove unnecessary _meta mapping --- x-pack/plugin/core/src/main/resources/slm-history.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/x-pack/plugin/core/src/main/resources/slm-history.json b/x-pack/plugin/core/src/main/resources/slm-history.json index 429105f94862a..762c398b2d9a2 100644 --- a/x-pack/plugin/core/src/main/resources/slm-history.json +++ b/x-pack/plugin/core/src/main/resources/slm-history.json @@ -12,9 +12,6 @@ }, "mappings": { "_doc": { - "_meta": { - "slm-history-version": "${xpack.slm.template.version}" - }, "dynamic": false, "properties": { "@timestamp": { From 422daab62b0958d165ee0c77f3476180d10706a6 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 9 May 2019 17:16:08 -0600 Subject: [PATCH 18/18] Disable ILM in ML tests due to NamedWriteable problem SLM automatically setting up an index + lifecycle policy leads to `index_lifecycle` custom metadata in the cluster state, which some of the ML tests don't know how to deal with due to setting up custom `NamedXContentRegistry`s. Watcher would cause the same problem, but it is already disabled (for the same reason). --- x-pack/plugin/ml/qa/native-multi-node-tests/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/build.gradle b/x-pack/plugin/ml/qa/native-multi-node-tests/build.gradle index e6fd8412c948b..15b25cfac7600 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/build.gradle +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/build.gradle @@ -40,6 +40,7 @@ integTestCluster { setting 'xpack.security.enabled', 'true' setting 'xpack.ml.enabled', 'true' setting 'xpack.watcher.enabled', 'false' + setting 'xpack.ilm.enabled', 'false' setting 'logger.org.elasticsearch.xpack.ml.datafeed', 'TRACE' setting 'xpack.monitoring.enabled', 'false' setting 'xpack.security.authc.token.enabled', 'true'