Support specifying multiple templates names in delete composable index template api #70094

martijnvg · 2021-03-08T15:58:52Z

Add support to delete composable index templates api to specify multiple template
names separated by a comma.

Change to cleanup template logic for rest tests to remove all composable index templates via a single delete composable index template request. This to optimize the cleanup logic. After each rest test we delete all templates. So deleting templates this via a single api call (and thus single cluster state update) saves a lot of time considering the number of rest tests.

If this pr is accepted then I will do the same change for the delete component template api.

Relates to #69973

…x template api. Add support to delete composable index templates api to specify multiple template names separated by a comma. Change to cleanup template logic for rest tests to remove all composable index templates via a single delete composable index template request. This to optimize the cleanup logic. After each rest test we delete all templates. So deleting templates this via a single api call (and thus single cluster state update) saves a lot of time considering the number of rest tests. If this pr is accepted then I will do the same change for the delete component template api. Relates to elastic#69973

elasticmachine · 2021-03-08T15:58:55Z

Pinging @elastic/es-core-features (Team:Core/Features)

martijnvg · 2021-03-08T16:33:08Z

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

@@ -641,8 +641,10 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS
    static ClusterState innerRemoveIndexTemplateV2(ClusterState currentState, String name) {


The name should be a string[], and also the name field on request class. ~~Otherwise the request can't be introspected correctly by security.~~

martijnvg · 2021-03-08T16:45:19Z

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

@@ -575,18 +575,14 @@ private void wipeCluster() throws Exception {
                    Request getTemplatesRequest = new Request("GET", "_index_template");
                    Map<String, Object> composableIndexTemplates = XContentHelper.convertToMap(JsonXContent.jsonXContent,
                        EntityUtils.toString(adminClient().performRequest(getTemplatesRequest).getEntity()), false);
-                    List<String> names = ((List<?>) composableIndexTemplates.get("index_templates")).stream()
+                    String names = ((List<?>) composableIndexTemplates.get("index_templates")).stream()


and obviously bwc logic is needed...

dakrone · 2021-03-08T18:09:08Z

I think we may want to reconsider supporting wildcards if we want to support comma-separated values. We already have strangeness with response codes when you mix concrete names with a *, for example, DELETE /_index_template/* already doesn't return an error if there are no matching templates, but what about DELETE /_index_template/foo,*? or DELETE /_index_template/foo,bar-*,*?

At some point it becomes very hard to describe to the user whether we did the thing they asked for when we can't be fine-grained in our response. Because we have some issues with this in other APIs, we originally didn't add this for the composable index templates.

martijnvg · 2021-03-08T18:51:46Z

I'm not a big fan of adding support for comma-separated values too. I just like to remove all templates in a single request (cluster state update) just for reducing the time it takes to run the tests.

So currently these delete APIs already support simple wildcards. However the xpack templates we need to exclude have many different names and are not sharing a common prefix (See ESRestTestCase#isXPackTemplate). Also simple wildcard matching is not enough and we need some kind of negation, because we need exclude pattern like: -ilm-history,-.*,-metrics*,-logs*,-synthetics*,-security_audit_log,-saml-service-provider. In order exclude these pattern, multiple expressions still need to be provided.

I'm not sure what is best here. Looks like our testing infrastructure does benefit from being able to remove all templates besides the templates managed by ES. Perhaps if comma-separated values are provided then we should prohibit the use of *?

dakrone · 2021-03-08T20:04:42Z

Perhaps if comma-separated values are provided then we should prohibit the use of *?

I think we could do:

if * is specified, delete all templates (no error)
if foo-* is specified, delete everything starting with foo
if foo,bar,baz are specified, delete those three templates exactly, don't support wildcards, throw an error if any of the three don't exist.

What do you think?

martijnvg · 2021-03-08T20:29:55Z

👍 that proposal sounds good to me.

…_templates_comma_seperated

martijnvg · 2021-03-10T12:31:21Z

@dakrone Do you want to take another look at this PR?

dakrone · 2021-03-10T14:28:14Z

@martijnvg yep, I will look at it today.

dakrone

LGTM, thanks Martijn! I left one really minor comment

dakrone · 2021-03-10T17:24:14Z

.../elasticsearch/action/admin/indices/template/delete/DeleteComposableIndexTemplateAction.java

-            this.name = name;
-            return this;
+        public Request(String... names) {
+            this.names = names;


Minor nit, can you do something like:

Suggested change

this.names = names;

this.names = Objects.requireNonNull(names, "templates to delete must not be null");

…_templates_comma_seperated

…x template api Backporting elastic#70094 to 7.12 branch. Add support to delete composable index templates api to specify multiple template names separated by a comma. Change to cleanup template logic for rest tests to remove all composable index templates via a single delete composable index template request. This to optimize the cleanup logic. After each rest test we delete all templates. So deleting templates this via a single api call (and thus single cluster state update) saves a lot of time considering the number of rest tests. Relates to elastic#69973

…x template api (elastic#70094) Add support to delete composable index templates api to specify multiple template names separated by a comma. Change to cleanup template logic for rest tests to remove all composable index templates via a single delete composable index template request. This to optimize the cleanup logic. After each rest test we delete all templates. So deleting templates this via a single api call (and thus single cluster state update) saves a lot of time considering the number of rest tests. If this pr is accepted then I will do the same change for the delete component template api. Relates to elastic#69973

… APIs is not supported. The delete legacy template API doesn't support comma-separated list of names in any version. For versions 7.12 and before, the delete component API and delete composable index template API don't support comma-separated list of names. From 7.13 and later, the delete component API and delete composable index template API do support comma-separated list of names. Relates to elastic#70094 and elastic#70314

… APIs is not supported (#70649) The delete legacy template API doesn't support comma-separated list of names in any version. For versions 7.12 and before, the delete component API and delete composable index template API don't support comma-separated list of names. From 7.13 and later, the delete component API and delete composable index template API do support comma-separated list of names. Relates to #70094 and #70314 Co-authored-by: James Rodewig <[email protected]>

… APIs is not supported (elastic#70649) The delete legacy template API doesn't support comma-separated list of names in any version. For versions 7.12 and before, the delete component API and delete composable index template API don't support comma-separated list of names. From 7.13 and later, the delete component API and delete composable index template API do support comma-separated list of names. Relates to elastic#70094 and elastic#70314 Co-authored-by: James Rodewig <[email protected]>

… APIs is not supported (#70797) Backporting #70649 to 7.11 branch. The delete legacy template API doesn't support comma-separated list of names in any version. For versions 7.12 and before, the delete component API and delete composable index template API don't support comma-separated list of names. From 7.13 and later, the delete component API and delete composable index template API do support comma-separated list of names. Relates to #70094 and #70314 Co-authored-by: James Rodewig <[email protected]>

… APIs is not supported (#70799) Backporting #70649 to 7.10 branch. The delete legacy template API doesn't support comma-separated list of names in any version. For versions 7.12 and before, the delete component API and delete composable index template API don't support comma-separated list of names. From 7.13 and later, the delete component API and delete composable index template API do support comma-separated list of names. Relates to #70094 and #70314 Co-authored-by: James Rodewig <[email protected]>

martijnvg added >enhancement :Data Management/Indices APIs APIs to create and manage indices and templates v8.0.0 v7.13.0 labels Mar 8, 2021

martijnvg requested review from dakrone and jakelandis March 8, 2021 15:58

elasticmachine added the Team:Data Management Meta label for data/management team label Mar 8, 2021

martijnvg commented Mar 8, 2021

View reviewed changes

martijnvg marked this pull request as draft March 8, 2021 16:45

iter

9223b82

martijnvg added 2 commits March 9, 2021 09:44

Merge remote-tracking branch 'es/master' into delete_composable_index…

1facc1b

…_templates_comma_seperated

iter2

fcdc9ae

martijnvg marked this pull request as ready for review March 9, 2021 09:54

martijnvg added 2 commits March 10, 2021 11:52

Merge remote-tracking branch 'es/master' into delete_composable_index…

3b95f66

…_templates_comma_seperated

iter

59234cf

dakrone approved these changes Mar 10, 2021

View reviewed changes

martijnvg added 2 commits March 11, 2021 09:53

Merge remote-tracking branch 'es/master' into delete_composable_index…

63c8a40

…_templates_comma_seperated

iter

871c78a

martijnvg mentioned this pull request Mar 11, 2021

MixedClusterClientYamlTestSuiteIT times out more frequently lately #69973

Closed

martijnvg merged commit 36044dd into elastic:master Mar 11, 2021

martijnvg added the backport pending label Mar 11, 2021

martijnvg added a commit that referenced this pull request Mar 11, 2021

Re-enable bwc tests after backporting #70094 to 7.x branch. (#70304)

4f323b7

martijnvg removed the backport pending label Mar 11, 2021

martijnvg mentioned this pull request Mar 11, 2021

Support specifying multiple templates names in delete composable index template api #70335

Closed

probakowski pushed a commit to probakowski/elasticsearch that referenced this pull request Mar 15, 2021

disable bwc tests for backporting elastic#70094

626f957

martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Mar 16, 2021

Disable bwc tests for elastic#70094

05f0ae7

martijnvg added a commit that referenced this pull request Mar 16, 2021

Disable bwc tests for #70094 (#70444)

485b41a

martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Mar 16, 2021

Enable bwc tests after backporting elastic#70094

20cd6fa

martijnvg added a commit that referenced this pull request Mar 16, 2021

Enable bwc tests after backporting #70094 (#70445)

4cf8bf9

martijnvg mentioned this pull request Mar 22, 2021

Adjust the docs that comma-separated list of names in delete template APIs is not supported #70649

Merged

martijnvg mentioned this pull request Mar 24, 2021

[7.11] Adjust the docs that comma-separated list of names in delete template APIs is not supported #70797

Merged

martijnvg mentioned this pull request Mar 24, 2021

[7.10] Adjust the docs that comma-separated list of names in delete template APIs is not supported #70799

Merged

stevejgordon mentioned this pull request Apr 21, 2021

7.13.0 Meta Ticket elastic/elasticsearch-net#5584

Closed

62 tasks

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support specifying multiple templates names in delete composable index template api #70094

Support specifying multiple templates names in delete composable index template api #70094

martijnvg commented Mar 8, 2021

elasticmachine commented Mar 8, 2021

martijnvg Mar 8, 2021 •

edited

Loading

martijnvg Mar 8, 2021

dakrone commented Mar 8, 2021 •

edited

Loading

martijnvg commented Mar 8, 2021

dakrone commented Mar 8, 2021

martijnvg commented Mar 8, 2021

martijnvg commented Mar 10, 2021

dakrone commented Mar 10, 2021

dakrone left a comment

dakrone Mar 10, 2021

		@@ -641,8 +641,10 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS
		static ClusterState innerRemoveIndexTemplateV2(ClusterState currentState, String name) {

	this.names = names;
	this.names = Objects.requireNonNull(names, "templates to delete must not be null");

Support specifying multiple templates names in delete composable index template api #70094

Support specifying multiple templates names in delete composable index template api #70094

Conversation

martijnvg commented Mar 8, 2021

elasticmachine commented Mar 8, 2021

martijnvg Mar 8, 2021 • edited Loading

Choose a reason for hiding this comment

martijnvg Mar 8, 2021

Choose a reason for hiding this comment

dakrone commented Mar 8, 2021 • edited Loading

martijnvg commented Mar 8, 2021

dakrone commented Mar 8, 2021

martijnvg commented Mar 8, 2021

martijnvg commented Mar 10, 2021

dakrone commented Mar 10, 2021

dakrone left a comment

Choose a reason for hiding this comment

dakrone Mar 10, 2021

Choose a reason for hiding this comment

martijnvg Mar 8, 2021 •

edited

Loading

dakrone commented Mar 8, 2021 •

edited

Loading