Operator/index templates by grcevski · Pull Request #90143 · elastic/elasticsearch

grcevski · 2022-09-19T17:29:03Z

This PR adds support for /_index_template/ and /_component_template/ in file based settings.

Relates to #89183

Update unit tests to match

elasticsearchmachine · 2022-09-19T17:29:26Z

Hi @grcevski, I've created a changelog YAML for you.

elasticsearchmachine · 2022-09-19T17:29:26Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

…csearch into operator/index_templates

andreidan

Thanks for working on this Nikola

This looks great, I've left a few very minor suggestions

...usterTest/java/org/elasticsearch/reservedstate/service/ComponentTemplatesFileSettingsIT.java

andreidan · 2022-10-11T14:58:01Z

...earch/action/admin/indices/template/reservedstate/ReservedComposableIndexTemplateAction.java

+/**
+ * This {@link ReservedClusterStateHandler} is responsible for reserved state
+ * CRUD operations on composable index templates and component templates, e.g. file based settings.
+ * <p>
+ * Internally it uses {@link MetadataIndexTemplateService} to add, update and delete both composable
+ * index templates and component templates. The reserved state handler is implemented as a joint handler
+ * for both component templates and composable index templates, because of the inherent interdependencies
+ * of the two separate types. For example, one cannot create composable index templates without first the
+ * component definitions being in the cluster state, however, the opposite is true when deleting. This
+ * circular dependency makes it impossible for separation of the two handlers.
+ */


This is awesome ❤️

andreidan · 2022-10-13T05:55:45Z

...earch/action/admin/indices/template/reservedstate/ReservedComposableIndexTemplateAction.java

+    public static String componentName(String name) {
+        return COMPONENT_PREFIX + name;
+    }
+
+    public static String nameFromComponentName(String name) {
+        assert name.startsWith(COMPONENT_PREFIX);
+        return name.substring(COMPONENT_PREFIX.length());
+    }
+
+    public static String composableIndexName(String name) {
+        return COMPOSABLE_PREFIX + name;
+    }
+
+    public static String nameFromComposableIndexName(String name) {
+        assert name.startsWith(COMPOSABLE_PREFIX);
+        return name.substring(COMPOSABLE_PREFIX.length());
+    }


Can we maybe document the purpose of these prefixes? It was a bit trappy for me to identify why we do this PREFIX concatenation and parsing.

Maybe componentName can become reservedComponentName ? And nameFromComponentName -> componentNameFromReservedName? (or something along those lines)

andreidan · 2022-10-13T06:10:15Z

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java

        return ClusterState.builder(currentState).metadata(Metadata.builder(currentState.metadata()).put(name, finalIndexTemplate)).build();
    }

+    public Map<String, List<String>> v2TemplateOverlaps(


Shall we document this method and since it's now called explicitly maybe add a test in MetadataIndexTemplateServiceTests ?

Great suggestion, let me do that!

…state/service/ComponentTemplatesFileSettingsIT.java Fix double wording. Co-authored-by: Andrei Dan <andrei.dan@elastic.co>

…csearch into operator/index_templates

andreidan · 2022-10-14T12:50:54Z

@grcevski A final question that just dawned on me.

How are we handling existing clusters that might have component/composable templates that start with the chosen prefix for the reserved templates?
Will they essentially become reserved when upgrading to a version that has the reserved templates feature?

grcevski · 2022-10-14T13:52:45Z

How are we handling existing clusters that might have component/composable templates that start with the chosen prefix for the reserved templates? Will they essentially become reserved when upgrading to a version that has the reserved templates feature?

I don't think it should be a problem, nothing should happen to the templates, but maybe I'm missing a case here.

Essentially we only add the prefix when we store the names of the templates in the reserved cluster state section of the metadata, the templates themselves get stored in the custom storage without the prefix. To store any reserved state we must have defined a template in the file settings, otherwise there's nothing that modifies the reserved sections of the cluster state metadata.

Here's what I think the potential situations might be:

We have an existing composable index template called composable_index_template:foo which uses the same prefix we intend to use for reservation in the metadata.
- We want to add new reserved template foo through file based settings. We store composable_index_template:foo in the reserved section of the metadata and a template with name foo in the composable index templates metadata. There should be nothing that prevents this, we'll end up with two different composable index templates, one called foo another called composable_index_template:foo, but only foo is reserved. If one were to look at the full metadata dump, it might look confusing, but I think that's the extent of it.
- Now we want to modify a template through the REST calls, say through TransportPutComponentTemplateAction, which defines the modified keys as Set.of(ReservedComposableIndexTemplateAction.reservedComponentName(request.name())), essentially prefixing the name with composable_index_template:.
  - If we wanted to modify foo through REST the modified keys we produce for conflict detection will be composable_index_template:foo, which will match the reserved keys metadata and we'll disallow the modification.
  - if we wanted to modify composable_index_template:foo through REST, the modified keys will produce a doubled prefix composable_index_template:composable_index_template:foo which won't match our reserved metadata and the REST call will be allowed.
We have an existing composable index template called foo. In this case the file based settings will overwrite it and put reserved metadata with composable_index_template:foo. This is the same behaviour as all other file based settings.

This was a very good call-out, I'm going to write some tests to see if I can break the logic and cause a conflict where there shouldn't be one if the user had already used our prefix in the name of a template.

andreidan · 2022-10-18T13:52:12Z

@grcevski thanks so much for the detailed explanation and extra tests 🚀 ❤️

Just toyed around a bit with upgrading an 8.4 cluster that had component_template:abc and composable template composable_template:logs_data_stream and indeed modifying and retrieving them through REST works like a champ after upgrade.

Nikola Grcevski added 5 commits September 15, 2022 14:57

Reserved state actions for composable templates

6b1c49b

WIP: Combined component templates handler

51a2799

Finish joint index template reserved handler

6c21513

Update unit tests to match

Improve tests and fix some bugs.

9e7491f

Add more tests

e84034c

grcevski added >enhancement :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team Team:Data Management (obsolete) DO NOT USE. This team no longer exists. v8.5.0 labels Sep 19, 2022

grcevski mentioned this pull request Sep 19, 2022

File Based Settings for Elasticsearch #89183

Closed

10 tasks

elasticsearchmachine removed the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Sep 19, 2022

grcevski and others added 3 commits September 19, 2022 13:29

Update docs/changelog/90143.yaml

befdf68

Fix test

bce701e

Merge branch 'operator/index_templates' of github.com:grcevski/elasti…

2b76c4b

…csearch into operator/index_templates

grcevski requested a review from dakrone September 20, 2022 13:51

csoulios added v8.6.0 and removed v8.5.0 labels Sep 21, 2022

dakrone requested a review from andreidan October 6, 2022 15:32

andreidan approved these changes Oct 13, 2022

View reviewed changes

grcevski and others added 5 commits October 13, 2022 14:57

Update server/src/internalClusterTest/java/org/elasticsearch/reserved…

f6f6128

…state/service/ComponentTemplatesFileSettingsIT.java Fix double wording. Co-authored-by: Andrei Dan <andrei.dan@elastic.co>

Merge main

4ff83c1

Rename and document better.

d3ace1f

Add more tests

159ae7b

Merge branch 'operator/index_templates' of github.com:grcevski/elasti…

582799b

…csearch into operator/index_templates

Add more tests

9d0a1c8

grcevski merged commit 9b0363e into elastic:main Oct 14, 2022

grcevski deleted the operator/index_templates branch October 14, 2022 17:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Operator/index templates#90143

Operator/index templates#90143
grcevski merged 14 commits intoelastic:mainfrom
grcevski:operator/index_templates

grcevski commented Sep 19, 2022

Uh oh!

elasticsearchmachine commented Sep 19, 2022

Uh oh!

elasticsearchmachine commented Sep 19, 2022

Uh oh!

andreidan left a comment

Uh oh!

Uh oh!

andreidan Oct 11, 2022

Uh oh!

andreidan Oct 13, 2022

Uh oh!

grcevski Oct 13, 2022

Uh oh!

andreidan Oct 13, 2022

Uh oh!

grcevski Oct 13, 2022

Uh oh!

andreidan commented Oct 14, 2022

Uh oh!

grcevski commented Oct 14, 2022

Uh oh!

andreidan commented Oct 18, 2022 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

grcevski commented Sep 19, 2022

Uh oh!

elasticsearchmachine commented Sep 19, 2022

Uh oh!

elasticsearchmachine commented Sep 19, 2022

Uh oh!

andreidan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andreidan Oct 11, 2022

Choose a reason for hiding this comment

Uh oh!

andreidan Oct 13, 2022

Choose a reason for hiding this comment

Uh oh!

grcevski Oct 13, 2022

Choose a reason for hiding this comment

Uh oh!

andreidan Oct 13, 2022

Choose a reason for hiding this comment

Uh oh!

grcevski Oct 13, 2022

Choose a reason for hiding this comment

Uh oh!

andreidan commented Oct 14, 2022

Uh oh!

grcevski commented Oct 14, 2022

Uh oh!

andreidan commented Oct 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andreidan commented Oct 18, 2022 •

edited

Loading