Skip to content

Convert ILM and SLM histories into hidden indices #51456

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Feb 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ public class MetaDataCreateIndexService {
* These index patterns will be converted to hidden indices, at which point they should be removed from this list.
*/
private static final CharacterRunAutomaton DOT_INDICES_EXCLUSIONS = new CharacterRunAutomaton(Regex.simpleMatchToAutomaton(
".slm-history-*",
".watch-history-*",
".ml-anomalies-*",
".ml-notifications-*",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,6 @@ public void testValidateDotIndex() {
public void testIndexNameExclusionsList() {
// this test case should be removed when DOT_INDICES_EXCLUSIONS is empty
List<String> excludedNames = Arrays.asList(
".slm-history-" + randomAlphaOfLength(5).toLowerCase(Locale.ROOT),
".watch-history-" + randomAlphaOfLength(5).toLowerCase(Locale.ROOT),
".ml-anomalies-" + randomAlphaOfLength(5).toLowerCase(Locale.ROOT),
".ml-notifications-" + randomAlphaOfLength(5).toLowerCase(Locale.ROOT),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -570,8 +570,11 @@ private void wipeCluster() throws Exception {
}

protected static void wipeAllIndices() throws IOException {
boolean includeHidden = minimumNodeVersion().onOrAfter(Version.V_7_7_0);
try {
final Response response = adminClient().performRequest(new Request("DELETE", "*"));
final Request deleteReq = new Request("DELETE", "*");
deleteReq.addParameter("expand_wildcards", "open,closed" + (includeHidden ? ",hidden" : ""));
final Response response = adminClient().performRequest(deleteReq);
try (InputStream is = response.getEntity().getContent()) {
assertTrue((boolean) XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true).get("acknowledged"));
}
Expand Down Expand Up @@ -1211,7 +1214,7 @@ protected static Version minimumNodeVersion() throws IOException {
final Request request = new Request("GET", "_nodes");
request.addParameter("filter_path", "nodes.*.version");

final Response response = client().performRequest(request);
final Response response = adminClient().performRequest(request);
final Map<String, Object> nodes = ObjectPath.createFromResponse(response).evaluate("nodes");

Version minVersion = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
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";
// version 2: converted to hidden index
public static final int INDEX_TEMPLATE_VERSION = 2;

public static final String SLM_TEMPLATE_VERSION_VARIABLE = "xpack.slm.template.version";
public static final String SLM_TEMPLATE_NAME = ".slm-history";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class IndexTemplateConfig {

private final String templateName;
private final String fileName;
private final String version;
private final int version;
private final String versionProperty;

/**
Expand All @@ -27,14 +27,17 @@ public class IndexTemplateConfig {
* {@code {"myTemplateVersion": "${my.version.property}"}}
* With {@code version = "42"; versionProperty = "my.version.property"} will result in {@code {"myTemplateVersion": "42"}}.
*
* @param templateName The name that will be used for the index template. Literal, include the version in this string if
* Note that this code does not automatically insert the {@code version} index template property - include that in the JSON file
* defining the template, preferably using the version variable provided to this constructor.
*
* @param templateName The name that will be used for the index template. Literal, include the version in this string if
* it should be used.
* @param fileName The filename the template should be loaded from. Literal, should include leading {@literal /} and
* extension if necessary.
* @param version The version of the template. Substituted for {@code versionProperty} as described above.
* @param versionProperty The property that will be replaced with the {@code version} string as described above.
*/
public IndexTemplateConfig(String templateName, String fileName, String version, String versionProperty) {
public IndexTemplateConfig(String templateName, String fileName, int version, String versionProperty) {
this.templateName = templateName;
this.fileName = fileName;
this.version = version;
Expand All @@ -49,14 +52,20 @@ public String getTemplateName() {
return templateName;
}

public int getVersion() {
return version;
}

/**
* Loads the template from disk as a UTF-8 byte array.
* @return The template as a UTF-8 byte array.
*/
public byte[] loadBytes() {
String template = TemplateUtils.loadTemplate(fileName, version,
Pattern.quote("${" + versionProperty + "}"));
final String versionPattern = Pattern.quote("${" + versionProperty + "}");
String template = TemplateUtils.loadTemplate(fileName, Integer.toString(version), versionPattern);
assert template != null && template.length() > 0;
assert Pattern.compile("\"version\"\\s*:\\s*" + version).matcher(template).find()
: "index template must have a version property set to the given version property";
return template.getBytes(StandardCharsets.UTF_8);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.cluster.ClusterChangedEvent;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.ClusterStateListener;
import org.elasticsearch.cluster.metadata.IndexTemplateMetaData;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.settings.Settings;
Expand All @@ -30,6 +31,7 @@
import org.elasticsearch.xpack.core.ilm.action.PutLifecycleAction;

import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
Expand Down Expand Up @@ -129,16 +131,24 @@ public void clusterChanged(ClusterChangedEvent event) {

private void addTemplatesIfMissing(ClusterState state) {
final List<IndexTemplateConfig> indexTemplates = getTemplateConfigs();
for (IndexTemplateConfig template : indexTemplates) {
final String templateName = template.getTemplateName();
for (IndexTemplateConfig newTemplate : indexTemplates) {
final String templateName = newTemplate.getTemplateName();
final AtomicBoolean creationCheck = templateCreationsInProgress.computeIfAbsent(templateName, key -> new AtomicBoolean(false));
if (creationCheck.compareAndSet(false, true)) {
if (!state.metaData().getTemplates().containsKey(templateName)) {
IndexTemplateMetaData currentTemplate = state.metaData().getTemplates().get(templateName);
if (Objects.isNull(currentTemplate)) {
logger.debug("adding index template [{}] for [{}], because it doesn't exist", templateName, getOrigin());
putTemplate(template, creationCheck);
putTemplate(newTemplate, creationCheck);
} else if (Objects.isNull(currentTemplate.getVersion()) || newTemplate.getVersion() > currentTemplate.getVersion()) {
// IndexTemplateConfig now enforces templates contain a `version` property, so if the template doesn't have one we can
// safely assume it's an old version of the template.
logger.info("upgrading index template [{}] for [{}] from version [{}] to version [{}]",
templateName, getOrigin(), currentTemplate.getVersion(), newTemplate.getVersion());
putTemplate(newTemplate, creationCheck);
} else {
creationCheck.set(false);
logger.trace("not adding index template [{}] for [{}], because it already exists", templateName, getOrigin());
logger.trace("not adding index template [{}] for [{}], because it already exists at version [{}]",
templateName, getOrigin(), currentTemplate.getVersion());
}
} else {
logger.trace("skipping the creation of index template [{}] for [{}], because its creation is in progress",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@ public static void loadTemplateIntoMap(String resource, Map<String, IndexTemplat
public static String loadTemplate(String resource, String version, String versionProperty) {
try {
BytesReference source = load(resource);
validate(source);
final String filteredJson = filter(source, version, versionProperty);
validate(new BytesArray(filteredJson));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious about this change, why the change to validation post-filter? Was it causing an issue?

Copy link
Contributor Author

@gwbrown gwbrown Feb 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, bare placeholders (e.g. "version": ${my.variable}) didn't work with the validation before the variable replacement because that's not valid JSON, and version must be a bare integer, not a string. Previously, all variables were strings (e.g. "field": "${my.variable}", note the quotes around the variable).

Besides, it's more important that the JSON is valid after we do the variable replacement, rather than before, isn't it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name "filter" makes it seem like the filteredJson is a subset of the original, in which case I would think it makes sense to validate the entire JSON instead of a subset. With your explanation it definitely makes sense (the name is just misleading).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a misleading name - I would have taken the opportunity to change it, but Dimitris is going to do so in #51765, as part of modifying IndexTemplateRegistry (and associated classes) to allow other variable substitutions in addition to the version.


return filter(source, version, versionProperty);
return filteredJson;
} catch (Exception e) {
throw new IllegalArgumentException("Unable to load template [" + resource + "]", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public final class WatcherIndexTemplateRegistryField {
// version 9: add a user field defining which user executed the watch
// version 10: add support for foreach path in actions
// Note: if you change this, also inform the kibana team around the watcher-ui
public static final String INDEX_TEMPLATE_VERSION = "10";
public static final int INDEX_TEMPLATE_VERSION = 10;
public static final String HISTORY_TEMPLATE_NAME = ".watch-history-" + INDEX_TEMPLATE_VERSION;
public static final String HISTORY_TEMPLATE_NAME_NO_ILM = ".watch-history-no-ilm-" + INDEX_TEMPLATE_VERSION;
public static final String TRIGGERED_TEMPLATE_NAME = ".triggered_watches";
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugin/core/src/main/resources/ilm-history.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"index.auto_expand_replicas": "0-1",
"index.lifecycle.name": "ilm-history-ilm-policy",
"index.lifecycle.rollover_alias": "ilm-history-${xpack.ilm_history.template.version}",
"index.hidden": true,
"index.format": 1
},
"mappings": {
Expand Down Expand Up @@ -79,5 +80,6 @@
}
}
}
}
},
"version": ${xpack.ilm_history.template.version}
}
6 changes: 4 additions & 2 deletions x-pack/plugin/core/src/main/resources/slm-history.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"index.auto_expand_replicas": "0-1",
"index.lifecycle.name": "slm-history-ilm-policy",
"index.lifecycle.rollover_alias": ".slm-history-${xpack.slm.template.version}",
"index.hidden": true,
"index.format": 1
},
"mappings": {
Expand Down Expand Up @@ -55,5 +56,6 @@
}
}
}
}
}
},
"version": ${xpack.slm.template.version}
}
3 changes: 2 additions & 1 deletion x-pack/plugin/core/src/main/resources/triggered-watches.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,6 @@
}
}
}
}
},
"version": ${xpack.watcher.template.version}
}
Original file line number Diff line number Diff line change
Expand Up @@ -611,5 +611,6 @@
}
}
}
}
},
"version": ${xpack.watcher.template.version}
}
3 changes: 2 additions & 1 deletion x-pack/plugin/core/src/main/resources/watch-history.json
Original file line number Diff line number Diff line change
Expand Up @@ -562,5 +562,6 @@
}
}
}
}
},
"version": ${xpack.watcher.template.version}
}
3 changes: 2 additions & 1 deletion x-pack/plugin/core/src/main/resources/watches.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,6 @@
}
}
}
}
},
"version": ${xpack.watcher.template.version}
}
Loading