Browse Source

Restrict ILM frozen phase to searchable snapshot actions only (#70158)

This commit changes the frozen phase within ILM in the following ways:

- The `searchable_snapshot` action now no longer takes a `storage` parameter. The storage type is
determined by the phase within which it is invoked (shared cache for frozen and full copy for
everything else).
- The frozen phase in ILM now no longer allows *any* actions other than `searchable_snapshot`
- If a frozen phase is provided, it *must* include a `searchable_snapshot` action.

These changes may seem breaking, but since they are intended to go back to 7.12 which has not been
released yet, they are not truly breaking changes.
Lee Hinman 4 years ago
parent
commit
67f13bb679

+ 2 - 2
build.gradle

@@ -189,8 +189,8 @@ tasks.register("verifyVersions") {
  * after the backport of the backcompat code is complete.
  */
 
-boolean bwc_tests_enabled = true
-String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */
+boolean bwc_tests_enabled = false
+String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/70158" /* place a PR link here when committing bwc changes */
 /*
  * FIPS 140-2 behavior was fixed in 7.11.0. Before that there is no way to run elasticsearch in a
  * JVM that is properly configured to be in fips mode with BCFIPS. For now we need to disable

+ 3 - 9
docs/reference/ilm/actions/ilm-searchable-snapshot.asciidoc

@@ -36,7 +36,8 @@ To keep the snapshot, set `delete_searchable_snapshot` to `false` in the delete
 `snapshot_repository`::
 (Required, string)
 Specifies where to store the snapshot. 
-See <<snapshots-register-repository>> for more information.
+See <<snapshots-register-repository>> for more information. In non-frozen phases the snapshot will
+be mounted as a `full_copy`, and in frozen phases mounted with the `shared_cache` storage type.
 
 `force_merge_index`::
 (Optional, Boolean)
@@ -52,12 +53,6 @@ the shards are relocating, in which case they will not be merged.
 The `searchable_snapshot` action will continue executing even if not all shards
 are force merged.
 
-`storage`::
-(Optional, string)
-Specifies the type of snapshot that should be mounted for a searchable snapshot. This corresponds to
-the <<searchable-snapshots-api-mount-query-params, `storage` option when mounting a snapshot>>.
-Defaults to `full_copy` in non-frozen phases, or `shared_cache` in the frozen phase.
-
 [[ilm-searchable-snapshot-ex]]
 ==== Examples
 [source,console]
@@ -69,8 +64,7 @@ PUT _ilm/policy/my_policy
       "cold": {
         "actions": {
           "searchable_snapshot" : {
-            "snapshot_repository" : "backing_repo",
-            "storage": "shared_cache"
+            "snapshot_repository" : "backing_repo"
           }
         }
       }

+ 10 - 56
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java

@@ -13,7 +13,6 @@ import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.health.ClusterHealthStatus;
 import org.elasticsearch.cluster.metadata.IndexAbstraction;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
-import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
@@ -44,7 +43,6 @@ public class SearchableSnapshotAction implements LifecycleAction {
 
     public static final ParseField SNAPSHOT_REPOSITORY = new ParseField("snapshot_repository");
     public static final ParseField FORCE_MERGE_INDEX = new ParseField("force_merge_index");
-    public static final ParseField STORAGE = new ParseField("storage");
     public static final String CONDITIONAL_DATASTREAM_CHECK_KEY = BranchingStep.NAME + "-on-datastream-check";
     public static final String CONDITIONAL_SKIP_ACTION_STEP = BranchingStep.NAME + "-check-prerequisites";
     public static final String CONDITIONAL_SKIP_GENERATE_AND_CLEAN = BranchingStep.NAME + "-check-existing-snapshot";
@@ -53,21 +51,11 @@ public class SearchableSnapshotAction implements LifecycleAction {
     public static final String PARTIAL_RESTORED_INDEX_PREFIX = "partial-";
 
     private static final ConstructingObjectParser<SearchableSnapshotAction, Void> PARSER = new ConstructingObjectParser<>(NAME,
-        a -> {
-            String storageName = (String) a[2];
-            final MountSearchableSnapshotRequest.Storage storageType;
-            if (storageName == null) {
-                storageType = null;
-            } else {
-                storageType = MountSearchableSnapshotRequest.Storage.fromString(storageName);
-            }
-            return new SearchableSnapshotAction((String) a[0], a[1] == null || (boolean) a[1], storageType);
-        });
+        a -> new SearchableSnapshotAction((String) a[0], a[1] == null || (boolean) a[1]));
 
     static {
         PARSER.declareString(ConstructingObjectParser.constructorArg(), SNAPSHOT_REPOSITORY);
         PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), FORCE_MERGE_INDEX);
-        PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), STORAGE);
     }
 
 
@@ -77,21 +65,17 @@ public class SearchableSnapshotAction implements LifecycleAction {
 
     private final String snapshotRepository;
     private final boolean forceMergeIndex;
-    @Nullable
-    private final MountSearchableSnapshotRequest.Storage storageType;
 
-    public SearchableSnapshotAction(String snapshotRepository, boolean forceMergeIndex,
-                                    @Nullable MountSearchableSnapshotRequest.Storage type) {
+    public SearchableSnapshotAction(String snapshotRepository, boolean forceMergeIndex) {
         if (Strings.hasText(snapshotRepository) == false) {
             throw new IllegalArgumentException("the snapshot repository must be specified");
         }
         this.snapshotRepository = snapshotRepository;
         this.forceMergeIndex = forceMergeIndex;
-        this.storageType = type;
     }
 
     public SearchableSnapshotAction(String snapshotRepository) {
-        this(snapshotRepository, true, null);
+        this(snapshotRepository, true);
     }
 
     public SearchableSnapshotAction(StreamInput in) throws IOException {
@@ -101,22 +85,12 @@ public class SearchableSnapshotAction implements LifecycleAction {
         } else {
             this.forceMergeIndex = true;
         }
-        if (in.getVersion().onOrAfter(Version.V_7_12_0)) {
-            this.storageType = in.readOptionalEnum(MountSearchableSnapshotRequest.Storage.class);
-        } else {
-            this.storageType = null;
-        }
     }
 
     boolean isForceMergeIndex() {
         return forceMergeIndex;
     }
 
-    @Nullable
-    public MountSearchableSnapshotRequest.Storage getStorageType() {
-        return storageType;
-    }
-
     public String getSnapshotRepository() {
         return snapshotRepository;
     }
@@ -290,28 +264,15 @@ public class SearchableSnapshotAction implements LifecycleAction {
      * Resolves the prefix to be used for the mounted index depending on the provided key
      */
     String getRestoredIndexPrefix(StepKey currentKey) {
-        if (storageType == null) {
-            if (currentKey.getPhase().equals(TimeseriesLifecycleType.FROZEN_PHASE)) {
-                return PARTIAL_RESTORED_INDEX_PREFIX;
-            } else {
-                return FULL_RESTORED_INDEX_PREFIX;
-            }
-        }
-        switch (storageType) {
-            case FULL_COPY:
-                return FULL_RESTORED_INDEX_PREFIX;
-            case SHARED_CACHE:
-                return PARTIAL_RESTORED_INDEX_PREFIX;
-            default:
-                throw new IllegalArgumentException("unexpected storage type: " + storageType);
+        if (currentKey.getPhase().equals(TimeseriesLifecycleType.FROZEN_PHASE)) {
+            return PARTIAL_RESTORED_INDEX_PREFIX;
+        } else {
+            return FULL_RESTORED_INDEX_PREFIX;
         }
     }
 
-    // Resolves the storage type from a Nullable to non-Nullable type
+    // Resolves the storage type depending on which phase the index is in
     MountSearchableSnapshotRequest.Storage getConcreteStorageType(StepKey currentKey) {
-        if (storageType != null) {
-            return storageType;
-        }
         if (currentKey.getPhase().equals(TimeseriesLifecycleType.FROZEN_PHASE)) {
             return MountSearchableSnapshotRequest.Storage.SHARED_CACHE;
         } else {
@@ -335,9 +296,6 @@ public class SearchableSnapshotAction implements LifecycleAction {
         if (out.getVersion().onOrAfter(Version.V_7_10_0)) {
             out.writeBoolean(forceMergeIndex);
         }
-        if (out.getVersion().onOrAfter(Version.V_7_12_0)) {
-            out.writeOptionalEnum(storageType);
-        }
     }
 
     @Override
@@ -345,9 +303,6 @@ public class SearchableSnapshotAction implements LifecycleAction {
         builder.startObject();
         builder.field(SNAPSHOT_REPOSITORY.getPreferredName(), snapshotRepository);
         builder.field(FORCE_MERGE_INDEX.getPreferredName(), forceMergeIndex);
-        if (storageType != null) {
-            builder.field(STORAGE.getPreferredName(), storageType);
-        }
         builder.endObject();
         return builder;
     }
@@ -361,12 +316,11 @@ public class SearchableSnapshotAction implements LifecycleAction {
             return false;
         }
         SearchableSnapshotAction that = (SearchableSnapshotAction) o;
-        return Objects.equals(snapshotRepository, that.snapshotRepository) &&
-            Objects.equals(storageType, that.storageType);
+        return Objects.equals(snapshotRepository, that.snapshotRepository);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(snapshotRepository, storageType);
+        return Objects.hash(snapshotRepository);
     }
 }

+ 20 - 6
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java

@@ -50,8 +50,7 @@ public class TimeseriesLifecycleType implements LifecycleType {
     static final List<String> ORDERED_VALID_WARM_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, ReadOnlyAction.NAME,
         AllocateAction.NAME, MigrateAction.NAME, ShrinkAction.NAME, ForceMergeAction.NAME);
     static final List<String> ORDERED_VALID_COLD_ACTIONS;
-    static final List<String> ORDERED_VALID_FROZEN_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME,
-        ReadOnlyAction.NAME, SearchableSnapshotAction.NAME, AllocateAction.NAME, MigrateAction.NAME, FreezeAction.NAME);
+    static final List<String> ORDERED_VALID_FROZEN_ACTIONS = Collections.singletonList(SearchableSnapshotAction.NAME);
     static final List<String> ORDERED_VALID_DELETE_ACTIONS = Arrays.asList(WaitForSnapshotAction.NAME, DeleteAction.NAME);
     static final Set<String> VALID_HOT_ACTIONS;
     static final Set<String> VALID_WARM_ACTIONS = Sets.newHashSet(ORDERED_VALID_WARM_ACTIONS);
@@ -136,10 +135,8 @@ public class TimeseriesLifecycleType implements LifecycleType {
             }
         }
 
-        if (phase.getActions().get(SearchableSnapshotAction.NAME) != null && phase.getName().equals(FROZEN_PHASE) == false) {
-            // the `searchable_snapshot` action defines migration rules itself, so no need to inject a migrate action, unless we're in the
-            // frozen phase (as the migrate action would also include the `data_frozen` role which is not guaranteed to be included by all
-            // types of searchable snapshots)
+        if (phase.getActions().get(SearchableSnapshotAction.NAME) != null) {
+            // Searchable snapshots automatically set their own allocation rules, no need to configure them with a migrate step.
             return false;
         }
 
@@ -301,6 +298,7 @@ public class TimeseriesLifecycleType implements LifecycleType {
 
         validateActionsFollowingSearchableSnapshot(phases);
         validateAllSearchableSnapshotActionsUseSameRepository(phases);
+        validateFrozenPhaseHasSearchableSnapshotAction(phases);
     }
 
     static void validateActionsFollowingSearchableSnapshot(Collection<Phase> phases) {
@@ -413,6 +411,22 @@ public class TimeseriesLifecycleType implements LifecycleType {
         return Strings.collectionToCommaDelimitedString(errors);
     }
 
+    /**
+     * Require that the "frozen" phase configured in a policy has a searchable snapshot action.
+     */
+    static void validateFrozenPhaseHasSearchableSnapshotAction(Collection<Phase> phases) {
+        Optional<Phase> maybeFrozenPhase = phases.stream()
+            .filter(p -> FROZEN_PHASE.equals(p.getName()))
+            .findFirst();
+
+        maybeFrozenPhase.ifPresent(p -> {
+            if (p.getActions().containsKey(SearchableSnapshotAction.NAME) == false) {
+                throw new IllegalArgumentException("policy specifies the [" + FROZEN_PHASE + "] phase without a corresponding [" +
+                    SearchableSnapshotAction.NAME + "] action, but a searchable snapshot action is required in the frozen phase");
+            }
+        });
+    }
+
     private static boolean definesAllocationRules(AllocateAction action) {
         return action.getRequire().isEmpty() == false || action.getInclude().isEmpty() == false || action.getExclude().isEmpty() == false;
     }

+ 26 - 4
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java

@@ -29,8 +29,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 
-import static org.elasticsearch.xpack.core.ilm.SearchableSnapshotActionTests.randomStorageType;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.mockito.Mockito.mock;
@@ -128,7 +128,10 @@ public class LifecyclePolicyTests extends AbstractSerializingTestCase<LifecycleP
 
     public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String lifecycleName) {
         List<String> phaseNames = randomSubsetOf(
-            between(0, TimeseriesLifecycleType.ORDERED_VALID_PHASES.size() - 1), TimeseriesLifecycleType.ORDERED_VALID_PHASES);
+            between(0, TimeseriesLifecycleType.ORDERED_VALID_PHASES.size() - 1), TimeseriesLifecycleType.ORDERED_VALID_PHASES).stream()
+            // Remove the frozen phase, we'll randomly re-add it later
+            .filter(pn -> TimeseriesLifecycleType.FROZEN_PHASE.equals(pn) == false)
+            .collect(Collectors.toList());
         Map<String, Phase> phases = new HashMap<>(phaseNames.size());
         Function<String, Set<String>> validActions = getPhaseToValidActions();
         Function<String, LifecycleAction> randomAction = getNameToActionFunction();
@@ -189,6 +192,17 @@ public class LifecyclePolicyTests extends AbstractSerializingTestCase<LifecycleP
             }
             phases.put(phase, new Phase(phase, after, actions));
         }
+        // Add a frozen phase if neither the hot nor cold phase contains a searchable snapshot action
+        if (hotPhaseContainsSearchableSnap == false && coldPhaseContainsSearchableSnap == false && randomBoolean()) {
+            TimeValue frozenTime = prev == null ? TimeValue.parseTimeValue(randomTimeValue(0, 100000, "s", "m", "h", "d"), "test") :
+                TimeValue.timeValueSeconds(prev.seconds() + randomIntBetween(60, 600));
+            phases.put(TimeseriesLifecycleType.FROZEN_PHASE,
+                new Phase(TimeseriesLifecycleType.FROZEN_PHASE, frozenTime,
+                    Collections.singletonMap(SearchableSnapshotAction.NAME,
+                        new SearchableSnapshotAction(randomAlphaOfLength(10), randomBoolean()))));
+        } else {
+            phases.remove(TimeseriesLifecycleType.FROZEN_PHASE);
+        }
         return new LifecyclePolicy(TimeseriesLifecycleType.INSTANCE, lifecycleName, phases);
     }
 
@@ -234,7 +248,7 @@ public class LifecyclePolicyTests extends AbstractSerializingTestCase<LifecycleP
                     case UnfollowAction.NAME:
                         return new UnfollowAction();
                     case SearchableSnapshotAction.NAME:
-                        return new SearchableSnapshotAction("repo", randomBoolean(), randomStorageType());
+                        return new SearchableSnapshotAction("repo", randomBoolean());
                     case MigrateAction.NAME:
                         return new MigrateAction(false);
                     case RollupILMAction.NAME:
@@ -269,8 +283,16 @@ public class LifecyclePolicyTests extends AbstractSerializingTestCase<LifecycleP
                 name = name + randomAlphaOfLengthBetween(1, 5);
                 break;
             case 1:
+                // Remove the frozen phase, because it makes a lot of invalid phases when randomly mutating an existing policy
+                phases.remove(TimeseriesLifecycleType.FROZEN_PHASE);
+                // Remove a random phase
+                if (phases.size() > 0) {
+                    phases.remove(new ArrayList<>(phases.keySet()).remove(randomIntBetween(0, phases.size() - 1)));
+                }
                 String phaseName = randomValueOtherThanMany(phases::containsKey,
-                    () -> randomFrom(TimeseriesLifecycleType.ORDERED_VALID_PHASES));
+                        () -> randomFrom(TimeseriesLifecycleType.ORDERED_VALID_PHASES.stream()
+                            .filter(pn -> TimeseriesLifecycleType.FROZEN_PHASE.equals(pn) == false)
+                            .collect(Collectors.toList())));
                 phases = new LinkedHashMap<>(phases);
                 phases.put(phaseName, new Phase(phaseName, null, Collections.emptyMap()));
                 break;

+ 2 - 30
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotActionTests.java

@@ -6,7 +6,6 @@
  */
 package org.elasticsearch.xpack.core.ilm;
 
-import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.xpack.core.ilm.Step.StepKey;
@@ -61,28 +60,15 @@ public class SearchableSnapshotActionTests extends AbstractActionTestCase<Search
     }
 
     public void testPrefixAndStorageTypeDefaults() {
-        SearchableSnapshotAction action = new SearchableSnapshotAction("repo", randomBoolean(), null);
+        SearchableSnapshotAction action = new SearchableSnapshotAction("repo", randomBoolean());
         StepKey nonFrozenKey = new StepKey(randomFrom("hot", "warm", "cold", "delete"), randomAlphaOfLength(5), randomAlphaOfLength(5));
         StepKey frozenKey = new StepKey("frozen", randomAlphaOfLength(5), randomAlphaOfLength(5));
 
-        assertThat(action.getStorageType(), equalTo(null));
         assertThat(action.getConcreteStorageType(nonFrozenKey), equalTo(MountSearchableSnapshotRequest.Storage.FULL_COPY));
         assertThat(action.getRestoredIndexPrefix(nonFrozenKey), equalTo(SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX));
 
         assertThat(action.getConcreteStorageType(frozenKey), equalTo(MountSearchableSnapshotRequest.Storage.SHARED_CACHE));
         assertThat(action.getRestoredIndexPrefix(frozenKey), equalTo(SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX));
-
-        action = new SearchableSnapshotAction("repo", randomBoolean(), MountSearchableSnapshotRequest.Storage.FULL_COPY);
-        assertThat(action.getConcreteStorageType(nonFrozenKey), equalTo(MountSearchableSnapshotRequest.Storage.FULL_COPY));
-        assertThat(action.getRestoredIndexPrefix(nonFrozenKey), equalTo(SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX));
-        assertThat(action.getConcreteStorageType(frozenKey), equalTo(MountSearchableSnapshotRequest.Storage.FULL_COPY));
-        assertThat(action.getRestoredIndexPrefix(frozenKey), equalTo(SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX));
-
-        action = new SearchableSnapshotAction("repo", randomBoolean(), MountSearchableSnapshotRequest.Storage.SHARED_CACHE);
-        assertThat(action.getConcreteStorageType(nonFrozenKey), equalTo(MountSearchableSnapshotRequest.Storage.SHARED_CACHE));
-        assertThat(action.getRestoredIndexPrefix(nonFrozenKey), equalTo(SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX));
-        assertThat(action.getConcreteStorageType(frozenKey), equalTo(MountSearchableSnapshotRequest.Storage.SHARED_CACHE));
-        assertThat(action.getRestoredIndexPrefix(frozenKey), equalTo(SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX));
     }
 
     private List<StepKey> expectedStepKeysWithForceMerge(String phase) {
@@ -125,20 +111,6 @@ public class SearchableSnapshotActionTests extends AbstractActionTestCase<Search
             new StepKey(phase, NAME, SwapAliasesAndDeleteSourceIndexStep.NAME));
     }
 
-    @Nullable
-    public static MountSearchableSnapshotRequest.Storage randomStorageType() {
-        if (randomBoolean()) {
-            // null is the same as a full copy, it just means it was not specified
-            if (randomBoolean()) {
-                return null;
-            } else {
-                return MountSearchableSnapshotRequest.Storage.FULL_COPY;
-            }
-        } else {
-            return MountSearchableSnapshotRequest.Storage.SHARED_CACHE;
-        }
-    }
-
     @Override
     protected SearchableSnapshotAction doParseInstance(XContentParser parser) throws IOException {
         return SearchableSnapshotAction.parse(parser);
@@ -160,6 +132,6 @@ public class SearchableSnapshotActionTests extends AbstractActionTestCase<Search
     }
 
     static SearchableSnapshotAction randomInstance() {
-        return new SearchableSnapshotAction(randomAlphaOfLengthBetween(5, 10), randomBoolean(), randomStorageType());
+        return new SearchableSnapshotAction(randomAlphaOfLengthBetween(5, 10), randomBoolean());
     }
 }

+ 26 - 7
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java

@@ -45,6 +45,7 @@ import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.VALID_WAR
 import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.WARM_PHASE;
 import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.validateAllSearchableSnapshotActionsUseSameRepository;
 import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.validateMonotonicallyIncreasingPhaseTimings;
+import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.validateFrozenPhaseHasSearchableSnapshotAction;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
@@ -667,7 +668,7 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase {
             Map<String, LifecycleAction> actions = new HashMap<>();
             actions.put(TEST_SEARCHABLE_SNAPSHOT_ACTION.getWriteableName(), TEST_SEARCHABLE_SNAPSHOT_ACTION);
             Phase phase = new Phase(FROZEN_PHASE, TimeValue.ZERO, actions);
-            assertThat(TimeseriesLifecycleType.shouldInjectMigrateStepForPhase(phase), is(true));
+            assertThat(TimeseriesLifecycleType.shouldInjectMigrateStepForPhase(phase), is(false));
         }
 
     }
@@ -678,9 +679,9 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase {
         Map<String, LifecycleAction> frozenActions = new HashMap<>();
 
         {
-            hotActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo1", randomBoolean(), null));
-            coldActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo1", randomBoolean(), null));
-            frozenActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo1", randomBoolean(), null));
+            hotActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo1", randomBoolean()));
+            coldActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo1", randomBoolean()));
+            frozenActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo1", randomBoolean()));
 
             Phase hotPhase = new Phase(HOT_PHASE, TimeValue.ZERO, hotActions);
             Phase coldPhase = new Phase(HOT_PHASE, TimeValue.ZERO, coldActions);
@@ -690,9 +691,9 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase {
         }
 
         {
-            hotActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo1", randomBoolean(), null));
-            coldActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo2", randomBoolean(), null));
-            frozenActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo1", randomBoolean(), null));
+            hotActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo1", randomBoolean()));
+            coldActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo2", randomBoolean()));
+            frozenActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo1", randomBoolean()));
 
             Phase hotPhase = new Phase(HOT_PHASE, TimeValue.ZERO, hotActions);
             Phase coldPhase = new Phase(HOT_PHASE, TimeValue.ZERO, coldActions);
@@ -788,6 +789,24 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase {
         }
     }
 
+    public void testValidateFrozenPhaseHasSearchableSnapshot() {
+        {
+            Map<String, LifecycleAction> frozenActions = new HashMap<>();
+            frozenActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo1", randomBoolean()));
+            Phase frozenPhase = new Phase(FROZEN_PHASE, TimeValue.ZERO, frozenActions);
+            validateFrozenPhaseHasSearchableSnapshotAction(Collections.singleton(frozenPhase));
+        }
+
+        {
+            Map<String, LifecycleAction> frozenActions = new HashMap<>();
+            Phase frozenPhase = new Phase(FROZEN_PHASE, TimeValue.ZERO, frozenActions);
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> validateFrozenPhaseHasSearchableSnapshotAction(Collections.singleton(frozenPhase)));
+            assertThat(e.getMessage(), containsString("policy specifies the [frozen] phase without a corresponding " +
+                "[searchable_snapshot] action, but a searchable snapshot action is required in the frozen phase"));
+        }
+    }
+
     private void assertNextActionName(String phaseName, String currentAction, String expectedNextAction, String... availableActionNames) {
         Map<String, LifecycleAction> availableActions = convertActionNamesToActions(availableActionNames);
         Phase phase = new Phase(phaseName, TimeValue.ZERO, availableActions);

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

@@ -180,14 +180,10 @@ public final class TimeSeriesRestDriver {
         coldActions.put(SetPriorityAction.NAME, new SetPriorityAction(0));
         coldActions.put(AllocateAction.NAME, new AllocateAction(0, singletonMap("_name", "javaRestTest-0,javaRestTest-1,javaRestTest-2," +
             "javaRestTest-3"), null, null));
-        Map<String, LifecycleAction> frozenActions = new HashMap<>();
-        frozenActions.put(SetPriorityAction.NAME, new SetPriorityAction(2));
-        frozenActions.put(AllocateAction.NAME, new AllocateAction(0, singletonMap("_name", ""), null, null));
         Map<String, Phase> phases = new HashMap<>();
         phases.put("hot", new Phase("hot", hotTime, hotActions));
         phases.put("warm", new Phase("warm", TimeValue.ZERO, warmActions));
         phases.put("cold", new Phase("cold", TimeValue.ZERO, coldActions));
-        phases.put("frozen", new Phase("frozen", TimeValue.ZERO, frozenActions));
         phases.put("delete", new Phase("delete", TimeValue.ZERO, singletonMap(DeleteAction.NAME, new DeleteAction())));
         LifecyclePolicy lifecyclePolicy = new LifecyclePolicy(policyName, phases);
         // PUT policy

+ 2 - 3
x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/LifecycleLicenseIT.java

@@ -40,7 +40,6 @@ import static org.elasticsearch.xpack.TimeSeriesRestDriver.createSnapshotRepo;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.explainIndex;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.indexDocument;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.rolloverMaxOneDocCondition;
-import static org.elasticsearch.xpack.core.ilm.SearchableSnapshotActionTests.randomStorageType;
 import static org.hamcrest.CoreMatchers.containsStringIgnoringCase;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.is;
@@ -72,7 +71,7 @@ public class LifecycleLicenseIT extends ESRestTestCase {
 
         ResponseException exception = expectThrows(ResponseException.class,
             () -> createNewSingletonPolicy(client(), policy, "cold",
-                new SearchableSnapshotAction(snapshotRepo, true, randomStorageType())));
+                new SearchableSnapshotAction(snapshotRepo, true)));
         assertThat(EntityUtils.toString(exception.getResponse().getEntity()),
             containsStringIgnoringCase("policy [" + policy + "] defines the [" + SearchableSnapshotAction.NAME + "] action but the " +
                 "current license is non-compliant for [searchable-snapshots]"));
@@ -82,7 +81,7 @@ public class LifecycleLicenseIT extends ESRestTestCase {
     public void testSearchableSnapshotActionErrorsOnInvalidLicense() throws Exception {
         String snapshotRepo = randomAlphaOfLengthBetween(4, 10);
         createSnapshotRepo(client(), snapshotRepo, randomBoolean());
-        createNewSingletonPolicy(client(), policy, "cold", new SearchableSnapshotAction(snapshotRepo, true, null));
+        createNewSingletonPolicy(client(), policy, "cold", new SearchableSnapshotAction(snapshotRepo, true));
 
         createComposableTemplate(client(), "template-name", dataStream,
             new Template(Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, policy).build(), null, null));

+ 23 - 89
x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java

@@ -36,7 +36,6 @@ import org.elasticsearch.xpack.core.ilm.SearchableSnapshotAction;
 import org.elasticsearch.xpack.core.ilm.SetPriorityAction;
 import org.elasticsearch.xpack.core.ilm.ShrinkAction;
 import org.elasticsearch.xpack.core.ilm.Step;
-import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest;
 import org.junit.Before;
 
 import java.io.IOException;
@@ -51,7 +50,6 @@ import java.util.concurrent.TimeUnit;
 import static java.util.Collections.singletonMap;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.createComposableTemplate;
-import static org.elasticsearch.xpack.TimeSeriesRestDriver.createIndexWithSettings;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.createNewSingletonPolicy;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.createPolicy;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.createSnapshotRepo;
@@ -76,7 +74,7 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
         dataStream = "logs-" + randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
         policy = "policy-" + randomAlphaOfLength(5);
         snapshotRepo = randomAlphaOfLengthBetween(10, 20);
-        logger.info("--> running [{}] with data stream [{}], snapshot repot [{}] and policy [{}]", getTestName(), dataStream,
+        logger.info("--> running [{}] with data stream [{}], snapshot repo [{}] and policy [{}]", getTestName(), dataStream,
             snapshotRepo, policy);
     }
 
@@ -87,7 +85,7 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
 
     public void testSearchableSnapshotAction() throws Exception {
         createSnapshotRepo(client(), snapshotRepo, randomBoolean());
-        createNewSingletonPolicy(client(), policy, "cold", new SearchableSnapshotAction(snapshotRepo, true, null));
+        createNewSingletonPolicy(client(), policy, "cold", new SearchableSnapshotAction(snapshotRepo, true));
 
         createComposableTemplate(client(), randomAlphaOfLengthBetween(5, 10).toLowerCase(), dataStream,
             new Template(Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, policy).build(), null, null));
@@ -113,7 +111,7 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
 
     public void testSearchableSnapshotForceMergesIndexToOneSegment() throws Exception {
         createSnapshotRepo(client(), snapshotRepo, randomBoolean());
-        createNewSingletonPolicy(client(), policy, "cold", new SearchableSnapshotAction(snapshotRepo, true, null));
+        createNewSingletonPolicy(client(), policy, "cold", new SearchableSnapshotAction(snapshotRepo, true));
 
         createComposableTemplate(client(), randomAlphaOfLengthBetween(5, 10).toLowerCase(), dataStream, new Template(null, null, null));
 
@@ -363,19 +361,20 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
 
     @SuppressWarnings("unchecked")
     public void testIdenticalSearchableSnapshotActionIsNoop() throws Exception {
-        String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT);
+        String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT) + "-000001";
         createSnapshotRepo(client(), snapshotRepo, randomBoolean());
-        MountSearchableSnapshotRequest.Storage storage = randomBoolean() ?
-            MountSearchableSnapshotRequest.Storage.FULL_COPY : MountSearchableSnapshotRequest.Storage.SHARED_CACHE;
+        Map<String, LifecycleAction> hotActions = new HashMap<>();
+        hotActions.put(RolloverAction.NAME, new RolloverAction(null, null, null, 1L));
+        hotActions.put(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean()));
         createPolicy(client(), policy, null, null,
+            new Phase("hot", TimeValue.ZERO, hotActions),
             new Phase("cold", TimeValue.ZERO,
-                singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(), storage))),
-            new Phase("frozen", TimeValue.ZERO,
-                singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(), storage))),
+                singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean()))),
             null
         );
 
-        createIndex(index, Settings.EMPTY);
+        createIndex(index, Settings.builder().put(RolloverAction.LIFECYCLE_ROLLOVER_ALIAS, "alias").build(),
+            null, "\"alias\": {\"is_write_index\": true}");
         ensureGreen(index);
         indexDocument(client(), index, true);
 
@@ -383,8 +382,7 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
         // `index_not_found_exception`
         updateIndexSettings(index, Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, policy));
 
-        final String searchableSnapMountedIndexName = (storage == MountSearchableSnapshotRequest.Storage.FULL_COPY ?
-            SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX : SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX) + index;
+        final String searchableSnapMountedIndexName = SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX + index;
 
         assertBusy(() -> {
             logger.info("--> waiting for [{}] to exist...", searchableSnapMountedIndexName);
@@ -393,7 +391,7 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
 
         assertBusy(() -> {
             Step.StepKey stepKeyForIndex = getStepKeyForIndex(client(), searchableSnapMountedIndexName);
-            assertThat(stepKeyForIndex.getPhase(), is("frozen"));
+            assertThat(stepKeyForIndex.getPhase(), is("cold"));
             assertThat(stepKeyForIndex.getName(), is(PhaseCompleteStep.NAME));
         }, 30, TimeUnit.SECONDS);
 
@@ -419,11 +417,9 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
         createSnapshotRepo(client(), snapshotRepo, randomBoolean());
         createPolicy(client(), policy, null, null,
             new Phase("cold", TimeValue.ZERO,
-                singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(),
-                    MountSearchableSnapshotRequest.Storage.FULL_COPY))),
+                singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean()))),
             new Phase("frozen", TimeValue.ZERO,
-                singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(),
-                    MountSearchableSnapshotRequest.Storage.SHARED_CACHE))),
+                singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean()))),
             null
         );
 
@@ -465,65 +461,6 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
         assertThat("expected a single document but got: " + count, (int) count.get("count"), equalTo(1));
     }
 
-    @SuppressWarnings("unchecked")
-    public void testConvertingPartialSearchableSnapshotIntoFull() throws Exception {
-        String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT) +"-000001";
-        createSnapshotRepo(client(), snapshotRepo, randomBoolean());
-        createPolicy(client(), policy,
-            new Phase("hot", TimeValue.ZERO,
-                Map.of(
-                    SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(),
-                    MountSearchableSnapshotRequest.Storage.SHARED_CACHE),
-                    RolloverAction.NAME, new RolloverAction(null, null, null, 1L))),
-            null, null,
-            new Phase("frozen", TimeValue.ZERO,
-                singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(),
-                    MountSearchableSnapshotRequest.Storage.FULL_COPY))),
-            null
-        );
-
-        String alias = "alias-" + randomAlphaOfLengthBetween(5, 10);
-        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)
-                .put(RolloverAction.LIFECYCLE_ROLLOVER_ALIAS, alias),
-            true);
-
-        ensureGreen(index);
-        indexDocument(client(), alias, true);
-        rolloverMaxOneDocCondition(client(), alias);
-
-        final String searchableSnapMountedIndexName = SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX +
-            SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX + index;
-
-        assertBusy(() -> {
-            logger.info("--> waiting for [{}] to exist...", searchableSnapMountedIndexName);
-            assertTrue(indexExists(searchableSnapMountedIndexName));
-        }, 30, TimeUnit.SECONDS);
-
-        assertBusy(() -> {
-            Step.StepKey stepKeyForIndex = getStepKeyForIndex(client(), searchableSnapMountedIndexName);
-            assertThat(stepKeyForIndex.getPhase(), is("frozen"));
-            assertThat(stepKeyForIndex.getName(), is(PhaseCompleteStep.NAME));
-        }, 30, TimeUnit.SECONDS);
-
-        Request getSnaps = new Request("GET", "/_snapshot/" + snapshotRepo + "/_all");
-        Response response = client().performRequest(getSnaps);
-        Map<String, Object> responseMap;
-        try (InputStream is = response.getEntity().getContent()) {
-            responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true);
-        }
-        assertThat("expected to have only one snapshot, but got: " + responseMap,
-            ((List<Map<String, Object>>)
-                ((Map<String, Object>)
-                    ((List<Object>) responseMap.get("responses")).get(0)).get("snapshots")).size(), equalTo(1));
-
-        Request hitCount = new Request("GET", "/" + searchableSnapMountedIndexName + "/_count");
-        Map<String, Object> count = entityAsMap(client().performRequest(hitCount));
-        assertThat("expected a single document but got: " + count, (int) count.get("count"), equalTo(1));
-    }
-
     public void testSecondSearchableSnapshotUsingDifferentRepoThrows() throws Exception {
         String secondRepo = randomAlphaOfLengthBetween(10, 20);
         createSnapshotRepo(client(), snapshotRepo, randomBoolean());
@@ -531,11 +468,9 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
             createPolicy(client(), policy, null, null,
                 new Phase("cold", TimeValue.ZERO,
-                    singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(),
-                        MountSearchableSnapshotRequest.Storage.FULL_COPY))),
+                    singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean()))),
                 new Phase("frozen", TimeValue.ZERO,
-                    singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(secondRepo, randomBoolean(),
-                        MountSearchableSnapshotRequest.Storage.SHARED_CACHE))),
+                    singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(secondRepo, randomBoolean()))),
                 null
             ));
 
@@ -548,15 +483,14 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
         createPolicy(client(), policy,
             new Phase("hot", TimeValue.ZERO, Map.of(RolloverAction.NAME, new RolloverAction(null, null, null, 1L),
                 SearchableSnapshotAction.NAME, new SearchableSnapshotAction(
-                    snapshotRepo, randomBoolean(), MountSearchableSnapshotRequest.Storage.FULL_COPY))
+                    snapshotRepo, randomBoolean()))
             ),
             new Phase("warm", TimeValue.ZERO, Map.of(MigrateAction.NAME, new MigrateAction(true))),
             // this time transition condition will make sure we catch ILM in the warm phase so we can assert the warm migrate action
             // didn't re-configure the tier allocation settings set by the searchable_snapshot action in the hot phase
             // we'll use the origination date to kick off ILM to complete the policy
             new Phase("cold", TimeValue.timeValueDays(5L), Map.of(MigrateAction.NAME, new MigrateAction(true))),
-            new Phase("frozen", TimeValue.ZERO, Map.of(MigrateAction.NAME, new MigrateAction(true))),
-            null
+            null, null
         );
 
         createComposableTemplate(client(), randomAlphaOfLengthBetween(5, 10).toLowerCase(), dataStream,
@@ -592,12 +526,12 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
             ZonedDateTime.now().toInstant().toEpochMilli() - TimeValue.timeValueDays(100).getMillis()));
 
         // let's wait for ILM to finish
-        assertBusy(() -> assertThat(getStepKeyForIndex(client(), restoredIndex), is(PhaseCompleteStep.finalStep("frozen").getKey())));
+        assertBusy(() -> assertThat(getStepKeyForIndex(client(), restoredIndex), is(PhaseCompleteStep.finalStep("cold").getKey())));
 
-        Map<String, Object> frozenIndexSettings = getIndexSettingsAsMap(restoredIndex);
+        Map<String, Object> coldIndexSettings = getIndexSettingsAsMap(restoredIndex);
         // the frozen phase should've reconfigured the allocation preference
-        assertThat(frozenIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER),
-            is("data_frozen,data_cold,data_warm,data_hot"));
+        assertThat(coldIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER),
+            is("data_cold,data_warm,data_hot"));
     }
 
 }