Make unchanged ILM policy updates into noop (elastic#82240) (elastic#…

…82293)

This commit makes `PUT`-ing an ILM policy that does not change a no-op,
neither incrementing the version nor creating a new cluster state.
Resolves elastic#82065

x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/ilm/IndexLifecycleInitialisationTests.java

@@ -55,6 +55,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ExecutionException;
@@ -195,6 +196,49 @@ public void testSingleNodeCluster() throws Exception {
         });
     }
 
+    public void testNoOpPolicyUpdates() throws Exception {
+        internalCluster().startNode();
+        Map<String, Phase> phases = new HashMap<>();
+        phases.put("hot", new Phase("hot", TimeValue.ZERO, Map.of()));
+        LifecyclePolicy policy = new LifecyclePolicy("mypolicy", phases);
+
+        PutLifecycleAction.Request putLifecycleRequest = new PutLifecycleAction.Request(policy);
+        assertAcked(client().execute(PutLifecycleAction.INSTANCE, putLifecycleRequest).get());
+
+        GetLifecycleAction.Response getLifecycleResponse = client().execute(GetLifecycleAction.INSTANCE, new GetLifecycleAction.Request())
+            .get();
+        assertThat(getLifecycleResponse.getPolicies().size(), equalTo(1));
+        GetLifecycleAction.LifecyclePolicyResponseItem responseItem = getLifecycleResponse.getPolicies().get(0);
+        assertThat(responseItem.getLifecyclePolicy(), equalTo(policy));
+        assertThat(responseItem.getVersion(), equalTo(1L));
+
+        // Put the same policy in place, which should be a no-op
+        putLifecycleRequest = new PutLifecycleAction.Request(policy);
+        assertAcked(client().execute(PutLifecycleAction.INSTANCE, putLifecycleRequest).get());
+
+        getLifecycleResponse = client().execute(GetLifecycleAction.INSTANCE, new GetLifecycleAction.Request()).get();
+        assertThat(getLifecycleResponse.getPolicies().size(), equalTo(1));
+        responseItem = getLifecycleResponse.getPolicies().get(0);
+        assertThat(responseItem.getLifecyclePolicy(), equalTo(policy));
+        // Version should still be 1
+        assertThat(responseItem.getVersion(), equalTo(1L));
+
+        // Generate a brand new policy
+        Map<String, Phase> newPhases = new HashMap<>(phases);
+        newPhases.put("cold", new Phase("cold", TimeValue.timeValueDays(1), Map.of()));
+        policy = new LifecyclePolicy("mypolicy", newPhases);
+
+        putLifecycleRequest = new PutLifecycleAction.Request(policy);
+        assertAcked(client().execute(PutLifecycleAction.INSTANCE, putLifecycleRequest).get());
+
+        getLifecycleResponse = client().execute(GetLifecycleAction.INSTANCE, new GetLifecycleAction.Request()).get();
+        assertThat(getLifecycleResponse.getPolicies().size(), equalTo(1));
+        responseItem = getLifecycleResponse.getPolicies().get(0);
+        assertThat(responseItem.getLifecyclePolicy(), equalTo(policy));
+        // Version should now be 2
+        assertThat(responseItem.getVersion(), equalTo(2L));
+    }
+
     public void testExplainExecution() throws Exception {
         // start node
         logger.info("Starting server1");

x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java

@@ -24,6 +24,7 @@
 import org.elasticsearch.cluster.metadata.RepositoriesMetadata;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.threadpool.ThreadPool;
@@ -98,20 +99,34 @@ protected void masterOperation(Task task, Request request, ClusterState state, A
 
         LifecyclePolicy.validatePolicyName(request.getPolicy().getName());
 
+        {
+            IndexLifecycleMetadata lifecycleMetadata = state.metadata().custom(IndexLifecycleMetadata.TYPE, IndexLifecycleMetadata.EMPTY);
+            LifecyclePolicyMetadata existingPolicy = lifecycleMetadata.getPolicyMetadatas().get(request.getPolicy().getName());
+            // Make the request a no-op if the policy and filtered headers match exactly
+            if (isNoopUpdate(existingPolicy, request.getPolicy(), filteredHeaders)) {
+                listener.onResponse(AcknowledgedResponse.TRUE);
+                return;
+            }
+        }
+
         clusterService.submitStateUpdateTask(
             "put-lifecycle-" + request.getPolicy().getName(),
             new AckedClusterStateUpdateTask(request, listener) {
                 @Override
                 public ClusterState execute(ClusterState currentState) throws Exception {
+                    final IndexLifecycleMetadata currentMetadata = currentState.metadata()
+                        .custom(IndexLifecycleMetadata.TYPE, IndexLifecycleMetadata.EMPTY);
+                    final LifecyclePolicyMetadata existingPolicyMetadata = currentMetadata.getPolicyMetadatas()
+                        .get(request.getPolicy().getName());
+
+                    // Double-check for no-op in the state update task, in case it was changed/reset in the meantime
+                    if (isNoopUpdate(existingPolicyMetadata, request.getPolicy(), filteredHeaders)) {
+                        return currentState;
+                    }
+
                     validatePrerequisites(request.getPolicy(), currentState);
 
                     ClusterState.Builder stateBuilder = ClusterState.builder(currentState);
-                    IndexLifecycleMetadata currentMetadata = currentState.metadata().custom(IndexLifecycleMetadata.TYPE);
-                    if (currentMetadata == null) { // first time using index-lifecycle feature, bootstrap metadata
-                        currentMetadata = IndexLifecycleMetadata.EMPTY;
-                    }
-                    LifecyclePolicyMetadata existingPolicyMetadata = currentMetadata.getPolicyMetadatas()
-                        .get(request.getPolicy().getName());
                     long nextVersion = (existingPolicyMetadata == null) ? 1L : existingPolicyMetadata.getVersion() + 1L;
                     SortedMap<String, LifecyclePolicyMetadata> newPolicies = new TreeMap<>(currentMetadata.getPolicyMetadatas());
                     LifecyclePolicyMetadata lifecyclePolicyMetadata = new LifecyclePolicyMetadata(
@@ -160,6 +175,21 @@ public ClusterState execute(ClusterState currentState) throws Exception {
         );
     }
 
+    /**
+     * Returns 'true' if the ILM policy is effectually the same (same policy and headers), and thus can be a no-op update.
+     */
+    static boolean isNoopUpdate(
+        @Nullable LifecyclePolicyMetadata existingPolicy,
+        LifecyclePolicy newPolicy,
+        Map<String, String> filteredHeaders
+    ) {
+        if (existingPolicy == null) {
+            return false;
+        } else {
+            return newPolicy.equals(existingPolicy.getPolicy()) && filteredHeaders.equals(existingPolicy.getHeaders());
+        }
+    }
+
     /**
      * Validate that the license level is compliant for searchable-snapshots, that any referenced snapshot
      * repositories exist, and that any referenced SLM policies exist.

x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.ilm.action;
+
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xpack.core.ilm.LifecyclePolicy;
+import org.elasticsearch.xpack.core.ilm.LifecyclePolicyMetadata;
+import org.elasticsearch.xpack.ilm.LifecyclePolicyTestsUtils;
+
+import java.util.Map;
+
+public class TransportPutLifecycleActionTests extends ESTestCase {
+    public void testIsNoop() {
+        LifecyclePolicy policy1 = LifecyclePolicyTestsUtils.randomTimeseriesLifecyclePolicy("policy");
+        LifecyclePolicy policy2 = randomValueOtherThan(policy1, () -> LifecyclePolicyTestsUtils.randomTimeseriesLifecyclePolicy("policy"));
+
+        Map<String, String> headers1 = Map.of("foo", "bar");
+        Map<String, String> headers2 = Map.of("foo", "eggplant");
+
+        LifecyclePolicyMetadata existing = new LifecyclePolicyMetadata(policy1, headers1, randomNonNegativeLong(), randomNonNegativeLong());
+
+        assertTrue(TransportPutLifecycleAction.isNoopUpdate(existing, policy1, headers1));
+        assertFalse(TransportPutLifecycleAction.isNoopUpdate(existing, policy2, headers1));
+        assertFalse(TransportPutLifecycleAction.isNoopUpdate(existing, policy1, headers2));
+        assertFalse(TransportPutLifecycleAction.isNoopUpdate(null, policy1, headers1));
+    }
+}