Replace PhaseAfterStep with PhaseCompleteStep (#33398)

This removes `PhaseAfterStep` in favor of a new `PhaseCompleteStep`. This step
in only a marker that the `LifecyclePolicyRunner` needs to halt until the time
indicated for entering the next phase.

This also fixes a bug where phase times were encapsulated into the policy
instead of dynamically adjusting to policy changes.

Supersedes #33140, which it replaces
Relates to #29823

Author: dakrone

client/rest-high-level/src/test/java/org/elasticsearch/client/IndexLifecycleIT.java

@@ -145,35 +145,39 @@ public void testExplainLifecycle() throws Exception {
             highLevelClient().indexLifecycle()::putLifecyclePolicyAsync);
         assertTrue(putResponse.isAcknowledged());
 
-        createIndex("foo", Settings.builder().put("index.lifecycle.name", policy.getName()).build());
-        createIndex("baz", Settings.builder().put("index.lifecycle.name", policy.getName()).build());
+        createIndex("foo-01", Settings.builder().put("index.lifecycle.name", policy.getName())
+            .put("index.lifecycle.rollover_alias", "foo-alias").build(), "", "\"foo-alias\" : {}");
+
+        createIndex("baz-01", Settings.builder().put("index.lifecycle.name", policy.getName())
+            .put("index.lifecycle.rollover_alias", "baz-alias").build(), "", "\"baz-alias\" : {}");
+
         createIndex("squash", Settings.EMPTY);
         assertBusy(() -> {
-            GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices("foo", "baz");
+            GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices("foo-01", "baz-01");
             GetSettingsResponse settingsResponse = highLevelClient().indices().getSettings(getSettingsRequest, RequestOptions.DEFAULT);
-            assertThat(settingsResponse.getSetting("foo", "index.lifecycle.name"), equalTo(policy.getName()));
-            assertThat(settingsResponse.getSetting("baz", "index.lifecycle.name"), equalTo(policy.getName()));
-            assertThat(settingsResponse.getSetting("foo", "index.lifecycle.phase"), equalTo("hot"));
-            assertThat(settingsResponse.getSetting("baz", "index.lifecycle.phase"), equalTo("hot"));
+            assertThat(settingsResponse.getSetting("foo-01", "index.lifecycle.name"), equalTo(policy.getName()));
+            assertThat(settingsResponse.getSetting("baz-01", "index.lifecycle.name"), equalTo(policy.getName()));
+            assertThat(settingsResponse.getSetting("foo-01", "index.lifecycle.phase"), equalTo("hot"));
+            assertThat(settingsResponse.getSetting("baz-01", "index.lifecycle.phase"), equalTo("hot"));
         });
 
         ExplainLifecycleRequest req = new ExplainLifecycleRequest();
-        req.indices("foo", "baz", "squash");
+        req.indices("foo-01", "baz-01", "squash");
         ExplainLifecycleResponse response = execute(req, highLevelClient().indexLifecycle()::explainLifecycle,
                 highLevelClient().indexLifecycle()::explainLifecycleAsync);
         Map<String, IndexLifecycleExplainResponse> indexResponses = response.getIndexResponses();
         assertEquals(3, indexResponses.size());
-        IndexLifecycleExplainResponse fooResponse = indexResponses.get("foo");
+        IndexLifecycleExplainResponse fooResponse = indexResponses.get("foo-01");
         assertNotNull(fooResponse);
         assertTrue(fooResponse.managedByILM());
-        assertEquals("foo", fooResponse.getIndex());
+        assertEquals("foo-01", fooResponse.getIndex());
         assertEquals("hot", fooResponse.getPhase());
         assertEquals("rollover", fooResponse.getAction());
         assertEquals("attempt_rollover", fooResponse.getStep());
-        IndexLifecycleExplainResponse bazResponse = indexResponses.get("baz");
+        IndexLifecycleExplainResponse bazResponse = indexResponses.get("baz-01");
         assertNotNull(bazResponse);
         assertTrue(bazResponse.managedByILM());
-        assertEquals("baz", bazResponse.getIndex());
+        assertEquals("baz-01", bazResponse.getIndex());
         assertEquals("hot", bazResponse.getPhase());
         assertEquals("rollover", bazResponse.getAction());
         assertEquals("attempt_rollover", bazResponse.getStep());

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

@@ -543,6 +543,14 @@ protected static void createIndex(String name, Settings settings, String mapping
         client().performRequest(request);
     }
 
+    protected static void createIndex(String name, Settings settings, String mapping, String aliases) throws IOException {
+        Request request = new Request("PUT", "/" + name);
+        request.setJsonEntity("{\n \"settings\": " + Strings.toString(settings)
+            + ", \"mappings\" : {" + mapping + "}"
+            + ", \"aliases\": {" + aliases + "} }");
+        client().performRequest(request);
+    }
+
     protected static void deleteIndex(String name) throws IOException {
         Request request = new Request("DELETE", "/" + name);
         client().performRequest(request);

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicy.java

@@ -28,7 +28,6 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.function.Function;
-import java.util.function.LongSupplier;
 import java.util.stream.Collectors;
 
 /**
@@ -165,10 +164,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
      *
      * @param client The Elasticsearch Client to use during execution of {@link AsyncActionStep}
      *               and {@link AsyncWaitStep} steps.
-     * @param nowSupplier The supplier of the current time for {@link PhaseAfterStep} steps.
      * @return The list of {@link Step} objects in order of their execution.
      */
-    public List<Step> toSteps(Client client, LongSupplier nowSupplier) {
+    public List<Step> toSteps(Client client) {
         List<Step> steps = new ArrayList<>();
         List<Phase> orderedPhases = type.getOrderedPhases(phases);
         ListIterator<Phase> phaseIterator = orderedPhases.listIterator(orderedPhases.size());
@@ -187,8 +185,8 @@ public List<Step> toSteps(Client client, LongSupplier nowSupplier) {
             if (phase != null) {
                 // after step should have the name of the previous phase since the index is still in the
                 // previous phase until the after condition is reached
-                Step.StepKey afterStepKey = new Step.StepKey(previousPhase.getName(), PhaseAfterStep.NAME, PhaseAfterStep.NAME);
-                Step phaseAfterStep = new PhaseAfterStep(nowSupplier, phase.getAfter(), afterStepKey, lastStepKey);
+                Step.StepKey afterStepKey = new Step.StepKey(previousPhase.getName(), PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
+                Step phaseAfterStep = new PhaseCompleteStep(afterStepKey, lastStepKey);
                 steps.add(phaseAfterStep);
                 lastStepKey = phaseAfterStep.getKey();
             }
@@ -211,8 +209,8 @@ public List<Step> toSteps(Client client, LongSupplier nowSupplier) {
 
         if (phase != null) {
             // The very first after step is in a phase before the hot phase so call this "new"
-            Step.StepKey afterStepKey = new Step.StepKey("new", PhaseAfterStep.NAME, PhaseAfterStep.NAME);
-            Step phaseAfterStep = new PhaseAfterStep(nowSupplier, phase.getAfter(), afterStepKey, lastStepKey);
+            Step.StepKey afterStepKey = new Step.StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
+            Step phaseAfterStep = new PhaseCompleteStep(afterStepKey, lastStepKey);
             steps.add(phaseAfterStep);
             lastStepKey = phaseAfterStep.getKey();
         }
@@ -289,7 +287,7 @@ private StepKey getNextAfterStep(String currentPhaseName) {
             // there are no more steps we should execute
             return TerminalPolicyStep.KEY;
         } else {
-            return new StepKey(currentPhaseName, PhaseAfterStep.NAME, PhaseAfterStep.NAME);
+            return new StepKey(currentPhaseName, PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
         }
     }
 
@@ -306,7 +304,7 @@ private StepKey getAfterStepBeforePhase(String currentPhaseName) {
                 // InitializePolicyContextStep
                 return InitializePolicyContextStep.KEY;
             }
-            return new StepKey(prevPhaseName, PhaseAfterStep.NAME, PhaseAfterStep.NAME);
+            return new StepKey(prevPhaseName, PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
         }
     }
 

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/PhaseAfterStep.java

@@ -0,0 +1,18 @@
+/*
+ * 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.indexlifecycle;
+
+/**
+ * This is essentially a marker that a phase has ended, and we need to check
+ * the age of an index before proceeding to the next phase.
+ */
+public class PhaseCompleteStep extends Step {
+    public static final String NAME = "complete";
+
+    public PhaseCompleteStep(StepKey key, StepKey nextStepKey) {
+        super(key, nextStepKey);
+    }
+}

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/PhaseCompleteStep.java

@@ -30,7 +30,6 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.function.Function;
-import java.util.function.LongSupplier;
 import java.util.stream.Collectors;
 
 import static org.hamcrest.Matchers.equalTo;
@@ -170,11 +169,10 @@ protected Reader<LifecyclePolicy> instanceReader() {
 
     public void testFirstAndLastSteps() {
         Client client = mock(Client.class);
-        LongSupplier nowSupplier = () -> 0L;
         lifecycleName = randomAlphaOfLengthBetween(1, 20);
         Map<String, Phase> phases = new LinkedHashMap<>();
         LifecyclePolicy policy = new LifecyclePolicy(TestLifecycleType.INSTANCE, lifecycleName, phases);
-        List<Step> steps = policy.toSteps(client, nowSupplier);
+        List<Step> steps = policy.toSteps(client);
         assertThat(steps.size(), equalTo(2));
         assertThat(steps.get(0), instanceOf(InitializePolicyContextStep.class));
         assertThat(steps.get(0).getKey(), equalTo(new StepKey("new", "init", "init")));
@@ -184,7 +182,6 @@ public void testFirstAndLastSteps() {
 
     public void testToStepsWithOneStep() {
         Client client = mock(Client.class);
-        LongSupplier nowSupplier = () -> 0L;
         MockStep mockStep = new MockStep(
             new Step.StepKey("test", "test", "test"), TerminalPolicyStep.KEY);
 
@@ -196,8 +193,8 @@ public void testToStepsWithOneStep() {
         phases.put(firstPhase.getName(), firstPhase);
         LifecyclePolicy policy = new LifecyclePolicy(TestLifecycleType.INSTANCE, lifecycleName, phases);
         StepKey firstStepKey = InitializePolicyContextStep.KEY;
-        StepKey secondStepKey = new StepKey("new", PhaseAfterStep.NAME, PhaseAfterStep.NAME);
-        List<Step> steps = policy.toSteps(client, nowSupplier);
+        StepKey secondStepKey = new StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
+        List<Step> steps = policy.toSteps(client);
         assertThat(steps.size(), equalTo(4));
         assertSame(steps.get(0).getKey(), firstStepKey);
         assertThat(steps.get(0).getNextStepKey(), equalTo(secondStepKey));
@@ -210,13 +207,12 @@ public void testToStepsWithOneStep() {
 
     public void testToStepsWithTwoPhases() {
         Client client = mock(Client.class);
-        LongSupplier nowSupplier = () -> 0L;
         MockStep secondActionStep = new MockStep(new StepKey("second_phase", "test2", "test"), TerminalPolicyStep.KEY);
-        MockStep secondAfter = new MockStep(new StepKey("first_phase", PhaseAfterStep.NAME, PhaseAfterStep.NAME),
+        MockStep secondAfter = new MockStep(new StepKey("first_phase", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME),
                 secondActionStep.getKey());
         MockStep firstActionAnotherStep = new MockStep(new StepKey("first_phase", "test", "bar"), secondAfter.getKey());
         MockStep firstActionStep = new MockStep(new StepKey("first_phase", "test", "foo"), firstActionAnotherStep.getKey());
-        MockStep firstAfter = new MockStep(new StepKey("new", PhaseAfterStep.NAME, PhaseAfterStep.NAME), firstActionStep.getKey());
+        MockStep firstAfter = new MockStep(new StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME), firstActionStep.getKey());
         MockStep init = new MockStep(InitializePolicyContextStep.KEY, firstAfter.getKey());
 
         lifecycleName = randomAlphaOfLengthBetween(1, 20);
@@ -231,17 +227,17 @@ public void testToStepsWithTwoPhases() {
         phases.put(secondPhase.getName(), secondPhase);
         LifecyclePolicy policy = new LifecyclePolicy(TestLifecycleType.INSTANCE, lifecycleName, phases);
 
-        List<Step> steps = policy.toSteps(client, nowSupplier);
+        List<Step> steps = policy.toSteps(client);
         assertThat(steps.size(), equalTo(7));
         assertThat(steps.get(0).getClass(), equalTo(InitializePolicyContextStep.class));
         assertThat(steps.get(0).getKey(), equalTo(init.getKey()));
         assertThat(steps.get(0).getNextStepKey(), equalTo(init.getNextStepKey()));
-        assertThat(steps.get(1).getClass(), equalTo(PhaseAfterStep.class));
+        assertThat(steps.get(1).getClass(), equalTo(PhaseCompleteStep.class));
         assertThat(steps.get(1).getKey(), equalTo(firstAfter.getKey()));
         assertThat(steps.get(1).getNextStepKey(), equalTo(firstAfter.getNextStepKey()));
         assertThat(steps.get(2), equalTo(firstActionStep));
         assertThat(steps.get(3), equalTo(firstActionAnotherStep));
-        assertThat(steps.get(4).getClass(), equalTo(PhaseAfterStep.class));
+        assertThat(steps.get(4).getClass(), equalTo(PhaseCompleteStep.class));
         assertThat(steps.get(4).getKey(), equalTo(secondAfter.getKey()));
         assertThat(steps.get(4).getNextStepKey(), equalTo(secondAfter.getNextStepKey()));
         assertThat(steps.get(5), equalTo(secondActionStep));
@@ -337,7 +333,7 @@ public void testGetNextValidStep() {
         currentStep = new StepKey("phase_1", "action_3", "step_missing");
         nextStep = policy.getNextValidStep(currentStep);
         assertNotNull(nextStep);
-        assertEquals(new StepKey("phase_1", PhaseAfterStep.NAME, PhaseAfterStep.NAME), nextStep);
+        assertEquals(new StepKey("phase_1", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME), nextStep);
 
         // current action exists but step does not and action is last in the
         // last phase
@@ -356,7 +352,7 @@ public void testGetNextValidStep() {
         currentStep = new StepKey("phase_1", "action_4", "step_2");
         nextStep = policy.getNextValidStep(currentStep);
         assertNotNull(nextStep);
-        assertEquals(new StepKey("phase_1", PhaseAfterStep.NAME, PhaseAfterStep.NAME), nextStep);
+        assertEquals(new StepKey("phase_1", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME), nextStep);
 
         // current action no longer exists and action was last in the last phase
         currentStep = new StepKey("phase_4", "action_4", "step_2");
@@ -368,7 +364,7 @@ public void testGetNextValidStep() {
         currentStep = new StepKey("phase_3", "action_2", "step_2");
         nextStep = policy.getNextValidStep(currentStep);
         assertNotNull(nextStep);
-        assertEquals(new StepKey("phase_2", PhaseAfterStep.NAME, PhaseAfterStep.NAME), nextStep);
+        assertEquals(new StepKey("phase_2", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME), nextStep);
 
         // current phase no longer exists and was last phase
         currentStep = new StepKey("phase_5", "action_2", "step_2");