Browse Source

Allow ILM move-to-step without `action` or `name` (#75435)

* Allow ILM move-to-step without `action` or `name`

This commit enhances ILM's move-to-step API to allow dropping the `name`, or dropping both the
`action` and `name`. For example:

```json
POST /_ilm/move/foo-1
{
  "current_step": {
    "phase": "hot",
    "action": "rollover",
    "name": "check-rollover-ready"
  },
  "next_step": {
    "phase": "warm",
    "action": "forcemerge"
  }
}
```

Will move to the first step in the `forcemerge` action in the `warm` phase (without having to know
the specific step name).

Another example:

```json
POST /_ilm/move/foo-1
{
  "current_step": {
    "phase": "hot",
    "action": "rollover",
    "name": "check-rollover-ready"
  },
  "next_step": {
    "phase": "warm"
  }
}
```

Will move to the first step in the `warm` phase (without having to know the specific action name).

Bear in mind that the execution order is still entirely an implementation detail, so "first" in the
above sentences means the first step that ILM would execute.

Resolves #58128

* Apply Andrei's wording change (thanks!)

Co-authored-by: Andrei Dan <andrei.dan@elastic.co>

* Log index and policy name when the concrete step key can't be resolved

Co-authored-by: Andrei Dan <andrei.dan@elastic.co>
Lee Hinman 4 years ago
parent
commit
3d5843a236

+ 10 - 2
docs/reference/ilm/apis/move-to-step.asciidoc

@@ -36,6 +36,12 @@ The request will fail if the current step does not match the step currently
 being executed for the index. This is to prevent the index from being moved from
 an unexpected step into the next step.
 
+When specifying the target (`next_step`) to which the index will be moved, either the `name` or both
+the `action` and `name` fields are optional. If only the phase is specified, the index will move to
+the first step of the first action in the target phase. If the phase and action are specified, the index will move to
+the first step of the specified action in the specified phase. Only actions specified in the ILM
+policy are considered valid, an index cannot move to a step that is not part of its policy.
+
 [[ilm-move-to-step-path-params]]
 ==== {api-path-parms-title}
 
@@ -152,14 +158,16 @@ POST _ilm/move/my-index-000001
   },
   "next_step": { <2>
     "phase": "warm",
-    "action": "forcemerge",
-    "name": "forcemerge"
+    "action": "forcemerge", <3>
+    "name": "forcemerge" <4>
   }
 }
 --------------------------------------------------
 // TEST[continued]
 <1> The step that the index is expected to be in
 <2> The step that you want to execute
+<3> The optional action to which the index will be moved
+<4> The optional step name to which the index will be moved
 
 If the request succeeds, you receive the following result:
 

+ 131 - 8
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/action/MoveToStepAction.java

@@ -7,18 +7,21 @@
  */
 package org.elasticsearch.xpack.core.ilm.action;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.ActionType;
 import org.elasticsearch.action.support.master.AcknowledgedRequest;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
-import org.elasticsearch.common.xcontent.ParseField;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.ConstructingObjectParser;
+import org.elasticsearch.common.xcontent.ParseField;
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.xpack.core.ilm.Step.StepKey;
 
 import java.io.IOException;
@@ -39,19 +42,22 @@ public class MoveToStepAction extends ActionType<AcknowledgedResponse> {
             new ConstructingObjectParser<>("move_to_step_request", false,
                 (a, index) -> {
                     StepKey currentStepKey = (StepKey) a[0];
-                    StepKey nextStepKey = (StepKey) a[1];
+                    PartialStepKey nextStepKey = (PartialStepKey) a[1];
                     return new Request(index, currentStepKey, nextStepKey);
                 });
+
         static {
+            // The current step uses the strict parser (meaning it requires all three parts of a stepkey)
             PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, name) -> StepKey.parse(p), CURRENT_KEY_FIELD);
-            PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, name) -> StepKey.parse(p), NEXT_KEY_FIELD);
+            // The target step uses the parser that allows specifying only the phase, or the phase and action
+            PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, name) -> PartialStepKey.parse(p), NEXT_KEY_FIELD);
         }
 
         private String index;
         private StepKey currentStepKey;
-        private StepKey nextStepKey;
+        private PartialStepKey nextStepKey;
 
-        public Request(String index, StepKey currentStepKey, StepKey nextStepKey) {
+        public Request(String index, StepKey currentStepKey, PartialStepKey nextStepKey) {
             this.index = index;
             this.currentStepKey = currentStepKey;
             this.nextStepKey = nextStepKey;
@@ -61,7 +67,12 @@ public class MoveToStepAction extends ActionType<AcknowledgedResponse> {
             super(in);
             this.index = in.readString();
             this.currentStepKey = new StepKey(in);
-            this.nextStepKey = new StepKey(in);
+            if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
+                this.nextStepKey = new PartialStepKey(in);
+            } else {
+                StepKey spec = new StepKey(in);
+                this.nextStepKey = new PartialStepKey(spec.getPhase(), spec.getAction(), spec.getName());
+            }
         }
 
         public Request() {
@@ -75,7 +86,7 @@ public class MoveToStepAction extends ActionType<AcknowledgedResponse> {
             return currentStepKey;
         }
 
-        public StepKey getNextStepKey() {
+        public PartialStepKey getNextStepKey() {
             return nextStepKey;
         }
 
@@ -93,7 +104,14 @@ public class MoveToStepAction extends ActionType<AcknowledgedResponse> {
             super.writeTo(out);
             out.writeString(index);
             currentStepKey.writeTo(out);
-            nextStepKey.writeTo(out);
+            if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
+                nextStepKey.writeTo(out);
+            } else {
+                String action = nextStepKey.getAction();
+                String name = nextStepKey.getName();
+                StepKey key = new StepKey(nextStepKey.getPhase(), action == null ? "" : action, name == null ? "" : name);
+                key.writeTo(out);
+            }
         }
 
         @Override
@@ -126,5 +144,110 @@ public class MoveToStepAction extends ActionType<AcknowledgedResponse> {
                 .field(NEXT_KEY_FIELD.getPreferredName(), nextStepKey)
                 .endObject();
         }
+
+        /**
+         * A PartialStepKey is like a {@link StepKey}, however, the action and step name are optional.
+         */
+        public static class PartialStepKey implements Writeable, ToXContentObject {
+            private final String phase;
+            private final String action;
+            private final String name;
+
+            public static final ParseField PHASE_FIELD = new ParseField("phase");
+            public static final ParseField ACTION_FIELD = new ParseField("action");
+            public static final ParseField NAME_FIELD = new ParseField("name");
+            private static final ConstructingObjectParser<PartialStepKey, Void> PARSER =
+                new ConstructingObjectParser<>("step_specification",
+                    a -> new PartialStepKey((String) a[0], (String) a[1], (String) a[2]));
+            static {
+                PARSER.declareString(ConstructingObjectParser.constructorArg(), PHASE_FIELD);
+                PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), ACTION_FIELD);
+                PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), NAME_FIELD);
+            }
+
+            public PartialStepKey(String phase, @Nullable String action, @Nullable String name) {
+                this.phase = phase;
+                this.action = action;
+                this.name = name;
+                if (name != null && action == null) {
+                    throw new IllegalArgumentException("phase; phase and action; or phase, action, and step must be provided, " +
+                        "but a step name was specified without a corresponding action");
+                }
+            }
+
+            public PartialStepKey(StreamInput in) throws IOException {
+                this.phase = in.readString();
+                this.action = in.readOptionalString();
+                this.name = in.readOptionalString();
+                if (name != null && action == null) {
+                    throw new IllegalArgumentException("phase; phase and action; or phase, action, and step must be provided, " +
+                        "but a step name was specified without a corresponding action");
+                }
+            }
+
+            public static PartialStepKey parse(XContentParser parser) {
+                return PARSER.apply(parser, null);
+            }
+
+            @Override
+            public void writeTo(StreamOutput out) throws IOException {
+                out.writeString(phase);
+                out.writeOptionalString(action);
+                out.writeOptionalString(name);
+            }
+
+            @Nullable
+            public String getPhase() {
+                return phase;
+            }
+
+            @Nullable
+            public String getAction() {
+                return action;
+            }
+
+            @Nullable
+            public String getName() {
+                return name;
+            }
+
+            @Override
+            public int hashCode() {
+                return Objects.hash(phase, action, name);
+            }
+
+            @Override
+            public boolean equals(Object obj) {
+                if (obj == null) {
+                    return false;
+                }
+                if (getClass() != obj.getClass()) {
+                    return false;
+                }
+                PartialStepKey other = (PartialStepKey) obj;
+                return Objects.equals(phase, other.phase) &&
+                    Objects.equals(action, other.action) &&
+                    Objects.equals(name, other.name);
+            }
+
+            @Override
+            public String toString() {
+                return Strings.toString(this);
+            }
+
+            @Override
+            public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+                builder.startObject();
+                builder.field(PHASE_FIELD.getPreferredName(), phase);
+                if (action != null) {
+                    builder.field(ACTION_FIELD.getPreferredName(), action);
+                }
+                if (name != null) {
+                    builder.field(NAME_FIELD.getPreferredName(), name);
+                }
+                builder.endObject();
+                return builder;
+            }
+        }
     }
 }

+ 15 - 3
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/action/MoveToStepRequestTests.java

@@ -27,7 +27,7 @@ public class MoveToStepRequestTests extends AbstractSerializingTestCase<Request>
 
     @Override
     protected Request createTestInstance() {
-        return new Request(index, stepKeyTests.createTestInstance(), stepKeyTests.createTestInstance());
+        return new Request(index, stepKeyTests.createTestInstance(), randomStepSpecification());
     }
 
     @Override
@@ -49,7 +49,7 @@ public class MoveToStepRequestTests extends AbstractSerializingTestCase<Request>
     protected Request mutateInstance(Request request) {
         String index = request.getIndex();
         StepKey currentStepKey = request.getCurrentStepKey();
-        StepKey nextStepKey = request.getNextStepKey();
+        Request.PartialStepKey nextStepKey = request.getNextStepKey();
 
         switch (between(0, 2)) {
             case 0:
@@ -59,7 +59,7 @@ public class MoveToStepRequestTests extends AbstractSerializingTestCase<Request>
                 currentStepKey = stepKeyTests.mutateInstance(currentStepKey);
                 break;
             case 2:
-                nextStepKey = stepKeyTests.mutateInstance(nextStepKey);
+                nextStepKey = randomValueOtherThan(nextStepKey, MoveToStepRequestTests::randomStepSpecification);
                 break;
             default:
                 throw new AssertionError("Illegal randomisation branch");
@@ -67,4 +67,16 @@ public class MoveToStepRequestTests extends AbstractSerializingTestCase<Request>
 
         return new Request(index, currentStepKey, nextStepKey);
     }
+
+    private static Request.PartialStepKey randomStepSpecification() {
+        if (randomBoolean()) {
+            StepKey key = stepKeyTests.createTestInstance();
+            return new Request.PartialStepKey(key.getPhase(), key.getAction(), key.getName());
+        } else {
+            String phase = randomAlphaOfLength(10);
+            String action = randomBoolean() ? null : randomAlphaOfLength(6);
+            String name = action == null ? null : (randomBoolean() ? null : randomAlphaOfLength(6));
+            return new Request.PartialStepKey(phase, action, name);
+        }
+    }
 }

+ 119 - 0
x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeseriesMoveToStepIT.java

@@ -17,6 +17,7 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.test.rest.ESRestTestCase;
 import org.elasticsearch.xpack.core.ilm.DeleteAction;
+import org.elasticsearch.xpack.core.ilm.ForceMergeAction;
 import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
 import org.elasticsearch.xpack.core.ilm.PhaseCompleteStep;
 import org.elasticsearch.xpack.core.ilm.RolloverAction;
@@ -237,4 +238,122 @@ public class TimeseriesMoveToStepIT extends ESRestTestCase {
         });
     }
 
+    public void testMoveToStepWithoutStepName() throws Exception {
+        createNewSingletonPolicy(client(), policy, "warm", new ForceMergeAction(1, null), TimeValue.timeValueHours(1));
+        createIndexWithSettings(client(), index, alias, Settings.builder()
+            .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
+            .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
+            .put(LifecycleSettings.LIFECYCLE_NAME, policy));
+
+        // move to a step
+        Request moveToStepRequest = new Request("POST", "_ilm/move/" + index);
+        moveToStepRequest.setJsonEntity("{\n" +
+            "  \"current_step\": {\n" +
+            "    \"phase\": \"new\",\n" +
+            "    \"action\": \"complete\",\n" +
+            "    \"name\": \"complete\"\n" +
+            "  },\n" +
+            "  \"next_step\": {\n" +
+            "    \"phase\": \"warm\",\n" +
+            "    \"action\": \"forcemerge\"\n" +
+            "  }\n" +
+            "}");
+
+        assertOK(client().performRequest(moveToStepRequest));
+
+        // Make sure we actually move on to and execute the forcemerge action
+        assertBusy(() -> {
+            assertThat(getStepKeyForIndex(client(), index), equalTo(PhaseCompleteStep.finalStep("warm").getKey()));
+        }, 30, TimeUnit.SECONDS);
+    }
+
+    public void testMoveToStepWithoutAction() throws Exception {
+        createNewSingletonPolicy(client(), policy, "warm", new ForceMergeAction(1, null), TimeValue.timeValueHours(1));
+        createIndexWithSettings(client(), index, alias, Settings.builder()
+            .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
+            .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
+            .put(LifecycleSettings.LIFECYCLE_NAME, policy));
+
+        // move to a step
+        Request moveToStepRequest = new Request("POST", "_ilm/move/" + index);
+        moveToStepRequest.setJsonEntity("{\n" +
+            "  \"current_step\": {\n" +
+            "    \"phase\": \"new\",\n" +
+            "    \"action\": \"complete\",\n" +
+       "    \"name\": \"complete\"\n" +
+            "  },\n" +
+            "  \"next_step\": {\n" +
+            "    \"phase\": \"warm\"\n" +
+            "  }\n" +
+            "}");
+
+        assertOK(client().performRequest(moveToStepRequest));
+
+        // Make sure we actually move on to and execute the forcemerge action
+        assertBusy(() -> {
+            assertThat(getStepKeyForIndex(client(), index), equalTo(PhaseCompleteStep.finalStep("warm").getKey()));
+        }, 30, TimeUnit.SECONDS);
+    }
+
+    public void testInvalidToMoveToStepWithoutActionButWithName() throws Exception {
+        createNewSingletonPolicy(client(), policy, "warm", new ForceMergeAction(1, null), TimeValue.timeValueHours(1));
+        createIndexWithSettings(client(), index, alias, Settings.builder()
+            .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
+            .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
+            .put(LifecycleSettings.LIFECYCLE_NAME, policy));
+
+        // move to a step with an invalid request
+        Request moveToStepRequest = new Request("POST", "_ilm/move/" + index);
+        moveToStepRequest.setJsonEntity("{\n" +
+            "  \"current_step\": {\n" +
+            "    \"phase\": \"new\",\n" +
+            "    \"action\": \"complete\",\n" +
+            "    \"name\": \"complete\"\n" +
+            "  },\n" +
+            "  \"next_step\": {\n" +
+            "    \"phase\": \"warm\",\n" +
+            "    \"name\": \"forcemerge\"\n" +
+            "  }\n" +
+            "}");
+
+        assertBusy(() -> {
+            ResponseException exception =
+                expectThrows(ResponseException.class, () -> client().performRequest(moveToStepRequest));
+            String responseEntityAsString = EntityUtils.toString(exception.getResponse().getEntity());
+            String expectedErrorMessage = "phase; phase and action; or phase, action, and step must be provided, " +
+                "but a step name was specified without a corresponding action";
+            assertThat(responseEntityAsString, containsStringIgnoringCase(expectedErrorMessage));
+        });
+    }
+
+    public void testResolveToNonexistentStep() throws Exception {
+        createNewSingletonPolicy(client(), policy, "warm", new ForceMergeAction(1, null), TimeValue.timeValueHours(1));
+        createIndexWithSettings(client(), index, alias, Settings.builder()
+            .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
+            .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
+            .put(LifecycleSettings.LIFECYCLE_NAME, policy));
+
+        // move to a step with an invalid request
+        Request moveToStepRequest = new Request("POST", "_ilm/move/" + index);
+        moveToStepRequest.setJsonEntity("{\n" +
+            "  \"current_step\": {\n" +
+            "    \"phase\": \"new\",\n" +
+            "    \"action\": \"complete\",\n" +
+            "    \"name\": \"complete\"\n" +
+            "  },\n" +
+            "  \"next_step\": {\n" +
+            "    \"phase\": \"warm\",\n" +
+            "    \"action\": \"shrink\"\n" +
+            "  }\n" +
+            "}");
+
+        assertBusy(() -> {
+            ResponseException exception =
+                expectThrows(ResponseException.class, () -> client().performRequest(moveToStepRequest));
+            String responseEntityAsString = EntityUtils.toString(exception.getResponse().getEntity());
+            String expectedErrorMessage = "unable to determine concrete step key from target next step key: " +
+                "{\\\"phase\\\":\\\"warm\\\",\\\"action\\\":\\\"shrink\\\"}";
+            assertThat(responseEntityAsString, containsStringIgnoringCase(expectedErrorMessage));
+        });
+    }
 }

+ 22 - 2
x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleService.java

@@ -7,6 +7,7 @@
 package org.elasticsearch.xpack.ilm;
 
 import com.carrotsearch.hppc.cursors.ObjectCursor;
+
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
@@ -23,14 +24,15 @@ import org.elasticsearch.common.Priority;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.component.Lifecycle.State;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
+import org.elasticsearch.core.Nullable;
+import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.gateway.GatewayService;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.shard.IndexEventListener;
+import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.plugins.ShutdownAwarePlugin;
 import org.elasticsearch.shutdown.PluginShutdownService;
-import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.xpack.core.XPackField;
 import org.elasticsearch.xpack.core.ilm.CheckShrinkReadyStep;
@@ -102,6 +104,24 @@ public class IndexLifecycleService
         lifecycleRunner.maybeRunAsyncAction(clusterState, indexMetadata, policyName, nextStepKey);
     }
 
+    /**
+     * Resolve the given phase, action, and name into a real {@link StepKey}. The phase is always
+     * required, but the action and name are optional. If a name is specified, an action is also required.
+     */
+    public StepKey resolveStepKey(ClusterState state, Index index, String phase, @Nullable String action, @Nullable String name) {
+        if (name == null) {
+            if (action == null) {
+                return this.policyRegistry.getFirstStepForPhase(state, index, phase);
+            } else {
+                return this.policyRegistry.getFirstStepForPhaseAndAction(state, index, phase, action);
+            }
+        } else {
+            assert action != null :
+                "action should never be null because we don't allow constructing a partial step key with only a phase and name";
+            return new StepKey(phase, action, name);
+        }
+    }
+
     /**
      * Move the cluster state to an arbitrary step for the provided index.
      *

+ 48 - 0
x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/PolicyStepsRegistry.java

@@ -14,6 +14,7 @@ import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.Diff;
 import org.elasticsearch.cluster.DiffableUtils;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.DeprecationHandler;
@@ -146,6 +147,53 @@ public class PolicyStepsRegistry {
     }
 
     /**
+     * Return all ordered steps for the current policy for the index. Does not
+     * resolve steps using the phase caching, but only for the currently existing policy.
+     */
+    private List<Step> getAllStepsForIndex(ClusterState state, Index index) {
+        final Metadata metadata = state.metadata();
+        if (metadata.hasIndex(index) == false) {
+            throw new IllegalArgumentException("index " + index + " does not exist in the current cluster state");
+        }
+        final IndexMetadata indexMetadata = metadata.index(index);
+        final String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexMetadata.getSettings());
+        final LifecyclePolicyMetadata policyMetadata = lifecyclePolicyMap.get(policyName);
+        if (policyMetadata == null) {
+            throw new IllegalArgumentException("the policy [" + policyName + "] for index" + index + " does not exist");
+        }
+        final LifecyclePolicySecurityClient policyClient = new LifecyclePolicySecurityClient(client, ClientHelper.INDEX_LIFECYCLE_ORIGIN,
+            policyMetadata.getHeaders());
+        return policyMetadata.getPolicy().toSteps(policyClient, licenseState);
+    }
+
+    /**
+     * Given an index and a phase name, return the {@link Step.StepKey} for the
+     * first step in that phase, if it exists, or null otherwise.
+     */
+    @Nullable
+    public Step.StepKey getFirstStepForPhase(ClusterState state, Index index, String phase) {
+        return getAllStepsForIndex(state, index).stream()
+            .map(Step::getKey)
+            .filter(stepKey -> phase.equals(stepKey.getPhase()))
+            .findFirst()
+            .orElse(null);
+    }
+
+    /**
+     * Given an index, phase name, and action name, return the {@link Step.StepKey}
+     * for the first step in that phase, if it exists, or null otherwise.
+     */
+    @Nullable
+    public Step.StepKey getFirstStepForPhaseAndAction(ClusterState state, Index index, String phase, String action) {
+        return getAllStepsForIndex(state, index).stream()
+            .map(Step::getKey)
+            .filter(stepKey -> phase.equals(stepKey.getPhase()))
+            .filter(stepKey -> action.equals(stepKey.getAction()))
+            .findFirst()
+            .orElse(null);
+    }
+
+    /*
      * Parses the step keys from the {@code phaseDef} for the given phase.
      * Returns null if there's a parsing error.
      */

+ 48 - 4
x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportMoveToStepAction.java

@@ -9,6 +9,7 @@ package org.elasticsearch.xpack.ilm.action;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
@@ -24,6 +25,8 @@ import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
+import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
+import org.elasticsearch.xpack.core.ilm.Step;
 import org.elasticsearch.xpack.core.ilm.action.MoveToStepAction;
 import org.elasticsearch.xpack.core.ilm.action.MoveToStepAction.Request;
 import org.elasticsearch.xpack.ilm.IndexLifecycleService;
@@ -48,12 +51,53 @@ public class TransportMoveToStepAction extends TransportMasterNodeAction<Request
             listener.onFailure(new IllegalArgumentException("index [" + request.getIndex() + "] does not exist"));
             return;
         }
-        clusterService.submitStateUpdateTask("index[" + request.getIndex() + "]-move-to-step",
+
+        final String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexMetadata.getSettings());
+        if (policyName == null) {
+            listener.onFailure(new IllegalArgumentException("index [" + request.getIndex() + "] is not managed by ILM"));
+            return;
+        }
+
+        final Request.PartialStepKey abstractTargetKey = request.getNextStepKey();
+        final String targetStr = abstractTargetKey.getPhase() + "/" + abstractTargetKey.getAction() + "/" + abstractTargetKey.getName();
+
+        // Resolve the key that could have optional parts into one
+        // that is totally concrete given the existing policy and index
+        Step.StepKey concreteTargetStepKey = indexLifecycleService.resolveStepKey(state, indexMetadata.getIndex(),
+            abstractTargetKey.getPhase(), abstractTargetKey.getAction(), abstractTargetKey.getName());
+
+        // We do a pre-check here before invoking the cluster state update just so we can skip the submission if the request is bad.
+        if (concreteTargetStepKey == null) {
+            // This means we weren't able to find the key they specified
+            String message = "cannot move index [" + indexMetadata.getIndex().getName() + "] with policy [" +
+                policyName + "]: unable to determine concrete step key from target next step key: " + abstractTargetKey;
+            logger.warn(message);
+            listener.onFailure(new IllegalArgumentException(message));
+            return;
+        }
+
+        clusterService.submitStateUpdateTask("index[" + request.getIndex() + "]-move-to-step-" + targetStr,
             new AckedClusterStateUpdateTask(request, listener) {
+                final SetOnce<Step.StepKey> concreteTargetKey = new SetOnce<>();
+
                 @Override
                 public ClusterState execute(ClusterState currentState) {
+                    // Resolve the key that could have optional parts into one
+                    // that is totally concrete given the existing policy and index
+                    Step.StepKey concreteTargetStepKey = indexLifecycleService.resolveStepKey(state, indexMetadata.getIndex(),
+                        abstractTargetKey.getPhase(), abstractTargetKey.getAction(), abstractTargetKey.getName());
+
+                    // Make one more check, because it could have changed in the meantime. If that is the case, the request is ignored.
+                    if (concreteTargetStepKey == null) {
+                        // This means we weren't able to find the key they specified
+                        logger.error("unable to move index " + indexMetadata.getIndex() + " as we are unable to resolve a concrete " +
+                            "step key from target next step key: " + abstractTargetKey);
+                        return currentState;
+                    }
+
+                    concreteTargetKey.set(concreteTargetStepKey);
                     return indexLifecycleService.moveClusterStateToStep(currentState, indexMetadata.getIndex(), request.getCurrentStepKey(),
-                        request.getNextStepKey());
+                        concreteTargetKey.get());
                 }
 
                 @Override
@@ -62,10 +106,10 @@ public class TransportMoveToStepAction extends TransportMasterNodeAction<Request
                     if (newIndexMetadata == null) {
                         // The index has somehow been deleted - there shouldn't be any opportunity for this to happen, but just in case.
                         logger.debug("index [" + indexMetadata.getIndex() + "] has been deleted after moving to step [" +
-                            request.getNextStepKey() + "], skipping async action check");
+                            concreteTargetKey.get() + "], skipping async action check");
                         return;
                     }
-                    indexLifecycleService.maybeRunAsyncAction(newState, newIndexMetadata, request.getNextStepKey());
+                    indexLifecycleService.maybeRunAsyncAction(newState, newIndexMetadata, concreteTargetKey.get());
                 }
             });
     }