ILM: Allow searchable_snapshot to follow searchable_snapshot

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

@@ -20,6 +20,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -61,7 +62,7 @@ public class TimeseriesLifecycleType implements LifecycleType {
         ForceMergeAction.NAME, RollupILMAction.NAME, SearchableSnapshotAction.NAME);
     // a set of actions that cannot be defined (executed) after the managed index has been mounted as searchable snapshot
     static final Set<String> ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT = Sets.newHashSet(ShrinkAction.NAME, ForceMergeAction.NAME,
-        FreezeAction.NAME, SearchableSnapshotAction.NAME, RollupILMAction.NAME);
+        FreezeAction.NAME, RollupILMAction.NAME);
 
     static {
         if (RollupV2.isEnabled()) {
@@ -294,23 +295,48 @@ public void validate(Collection<Phase> phases) {
     }
 
     static void validateActionsFollowingSearchableSnapshot(Collection<Phase> phases) {
-        boolean hotPhaseContainsSearchableSnapshot = phases.stream()
-            .filter(phase -> HOT_PHASE.equals(phase.getName()))
-            .anyMatch(phase -> phase.getActions().containsKey(SearchableSnapshotAction.NAME));
-        if (hotPhaseContainsSearchableSnapshot) {
-            String phasesDefiningIllegalActions = phases.stream()
-                // we're looking for prohibited actions in phases other than hot
-                .filter(phase -> HOT_PHASE.equals(phase.getName()) == false)
-                // filter the phases that define illegal actions
-                .filter(phase ->
-                    Collections.disjoint(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT, phase.getActions().keySet()) == false)
-                .map(Phase::getName)
-                .collect(Collectors.joining(","));
-            if (Strings.hasText(phasesDefiningIllegalActions)) {
-                throw new IllegalArgumentException("phases [" + phasesDefiningIllegalActions + "] define one or more of " +
-                    ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT + " actions which are not allowed after a " +
-                    "managed index is mounted as a searchable snapshot");
+        // invalid configurations can occur if searchable_snapshot is defined in the `hot` phase, with subsequent invalid actions
+        // being defined in the warm/cold/frozen phases, or if it is defined in the `cold` phase with subsequent invalid actions
+        // being defined in the frozen phase
+
+        Optional<Phase> hotPhaseWithSearchableSnapshot = phases.stream()
+            .filter(phase -> phase.getName().equals(HOT_PHASE))
+            .filter(phase -> phase.getActions().containsKey(SearchableSnapshotAction.NAME))
+            .findAny();
+
+        final List<Phase> phasesFollowingSearchableSnapshot = new ArrayList<>(phases.size());
+        if (hotPhaseWithSearchableSnapshot.isPresent()) {
+            for (Phase phase : phases) {
+                if (phase.getName().equals(HOT_PHASE) == false) {
+                    phasesFollowingSearchableSnapshot.add(phase);
+                }
             }
+        } else {
+            // let's see if the cold phase defines `searchable_snapshot`
+            Optional<Phase> coldPhaseWithSearchableSnapshot = phases.stream()
+                .filter(phase -> phase.getName().equals(COLD_PHASE))
+                .filter(phase -> phase.getActions().containsKey(SearchableSnapshotAction.NAME))
+                .findAny();
+            if (coldPhaseWithSearchableSnapshot.isPresent()) {
+                for (Phase phase : phases) {
+                    if (phase.getName().equals(FROZEN_PHASE)) {
+                        phasesFollowingSearchableSnapshot.add(phase);
+                        break;
+                    }
+                }
+            }
+        }
+
+        final String phasesDefiningIllegalActions = phasesFollowingSearchableSnapshot.stream()
+            // filter the phases that define illegal actions
+            .filter(phase ->
+                Collections.disjoint(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT, phase.getActions().keySet()) == false)
+            .map(Phase::getName)
+            .collect(Collectors.joining(","));
+        if (Strings.hasText(phasesDefiningIllegalActions)) {
+            throw new IllegalArgumentException("phases [" + phasesDefiningIllegalActions + "] define one or more of " +
+                ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT + " actions which are not allowed after a " +
+                "managed index is mounted as a searchable snapshot");
         }
     }
 

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

@@ -136,7 +136,16 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l
             phaseNames.add(0, TimeseriesLifecycleType.HOT_PHASE);
         }
         boolean hotPhaseContainsSearchableSnap = false;
-        for (String phase : phaseNames) {
+        boolean coldPhaseContainsSearchableSnap = false;
+        // let's order the phases so we can reason about actions in a previous phase in order to generate a random *valid* policy
+        List<String> orderedPhases = new ArrayList<>(phaseNames.size());
+        for (String validPhase : TimeseriesLifecycleType.VALID_PHASES) {
+            if (phaseNames.contains(validPhase)) {
+                orderedPhases.add(validPhase);
+            }
+        }
+
+        for (String phase : orderedPhases) {
             TimeValue after = TimeValue.parseTimeValue(randomTimeValue(0, 1000000000, "s", "m", "h", "d"), "test_after");
             Map<String, LifecycleAction> actions = new HashMap<>();
             List<String> actionNames = randomSubsetOf(validActions.apply(phase));
@@ -150,12 +159,23 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l
                 if (actionNames.contains(SearchableSnapshotAction.NAME)) {
                     hotPhaseContainsSearchableSnap = true;
                 }
-            } else {
+            }
+            if (phase.equals(TimeseriesLifecycleType.COLD_PHASE)) {
                 if (hotPhaseContainsSearchableSnap) {
                     // let's make sure the other phases don't configure actions that conflict with a possible `searchable_snapshot` action
                     // configured in the hot phase
                     actionNames.removeAll(TimeseriesLifecycleType.ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT);
                 }
+
+                if (actionNames.contains(SearchableSnapshotAction.NAME)) {
+                    coldPhaseContainsSearchableSnap = true;
+                }
+            } else {
+                if (hotPhaseContainsSearchableSnap || coldPhaseContainsSearchableSnap) {
+                    // let's make sure the other phases don't configure actions that conflict with a possible `searchable_snapshot` action
+                    // configured in a previous phase (hot/cold)
+                    actionNames.removeAll(TimeseriesLifecycleType.ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT);
+                }
             }
 
             for (String action : actionNames) {

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

@@ -204,21 +204,75 @@ public void testValidateConflictingDataMigrationConfigurations() {
     }
 
     public void testActionsThatCannotFollowSearchableSnapshot() {
-        assertThat(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT.size(), is(5));
+        assertThat(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT.size(), is(4));
         assertThat(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT, containsInAnyOrder(ShrinkAction.NAME, FreezeAction.NAME,
-            ForceMergeAction.NAME, RollupILMAction.NAME, SearchableSnapshotAction.NAME));
+            ForceMergeAction.NAME, RollupILMAction.NAME));
     }
 
     public void testValidateActionsFollowingSearchableSnapshot() {
-        Phase hotPhase = new Phase("hot", TimeValue.ZERO, Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
-        Phase warmPhase = new Phase("warm", TimeValue.ZERO, Map.of(ShrinkAction.NAME, new ShrinkAction(1, null)));
-        Phase coldPhase = new Phase("cold", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction()));
-
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
-            () -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(hotPhase, warmPhase, coldPhase)));
-        assertThat(e.getMessage(), is(
-            "phases [warm,cold] define one or more of [searchable_snapshot, forcemerge, freeze, shrink, rollup] actions" +
-            " which are not allowed after a managed index is mounted as a searchable snapshot"));
+        {
+            Phase hotPhase = new Phase("hot", TimeValue.ZERO, Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
+            Phase warmPhase = new Phase("warm", TimeValue.ZERO, Map.of(ShrinkAction.NAME, new ShrinkAction(1, null)));
+            Phase coldPhase = new Phase("cold", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction()));
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(hotPhase, warmPhase, coldPhase)));
+            assertThat(e.getMessage(), is(
+                "phases [warm,cold] define one or more of [forcemerge, freeze, shrink, rollup] actions" +
+                    " which are not allowed after a managed index is mounted as a searchable snapshot"));
+        }
+
+        {
+            Phase warmPhase = new Phase("warm", TimeValue.ZERO,
+                Map.of(ShrinkAction.NAME, new ShrinkAction(1, null)));
+            Phase coldPhase = new Phase("cold", TimeValue.ZERO,
+                Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
+            Phase frozenPhase = new Phase("frozen", TimeValue.ZERO,
+                Map.of(FreezeAction.NAME, new FreezeAction()));
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(warmPhase, coldPhase, frozenPhase)));
+            assertThat(e.getMessage(), is(
+                "phases [frozen] define one or more of [forcemerge, freeze, shrink, rollup] actions" +
+                    " which are not allowed after a managed index is mounted as a searchable snapshot"));
+        }
+
+        {
+            Phase hotPhase = new Phase("hot", TimeValue.ZERO,
+                Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
+            Phase warmPhase = new Phase("warm", TimeValue.ZERO,
+                Map.of(ShrinkAction.NAME, new ShrinkAction(1, null)));
+            Phase coldPhase = new Phase("cold", TimeValue.ZERO,
+                Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
+            Phase frozenPhase = new Phase("frozen", TimeValue.ZERO,
+                Map.of(FreezeAction.NAME, new FreezeAction()));
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(hotPhase, warmPhase, coldPhase,
+                    frozenPhase)));
+            assertThat(e.getMessage(), is(
+                "phases [warm,frozen] define one or more of [forcemerge, freeze, shrink, rollup] actions" +
+                    " which are not allowed after a managed index is mounted as a searchable snapshot"));
+        }
+
+        {
+            Phase hot = new Phase("hot", TimeValue.ZERO, Map.of(RolloverAction.NAME, new RolloverAction(null, null, 1L),
+            SearchableSnapshotAction.NAME, new SearchableSnapshotAction(randomAlphaOfLengthBetween(4, 10))));
+            Phase warm = new Phase("warm", TimeValue.ZERO, Map.of(ForceMergeAction.NAME, new ForceMergeAction(1, null)));
+            Phase cold = new Phase("cold", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction()));
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(warm, hot, cold)));
+            assertThat(e.getMessage(), is(
+                "phases [warm,cold] define one or more of [forcemerge, freeze, shrink, rollup] actions" +
+                    " which are not allowed after a managed index is mounted as a searchable snapshot"));
+        }
+
+        {
+            Phase frozenPhase = new Phase("frozen", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction(),
+                SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
+            try {
+                TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(frozenPhase));
+            } catch (Exception e) {
+                fail("unexpected exception while validating phase [ "+ frozenPhase +" ] but got [" + e.getMessage()+ "]");
+            }
+        }
     }
 
     public void testGetOrderedPhases() {

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

@@ -48,6 +48,7 @@
 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;
@@ -224,7 +225,7 @@ public void testCreateInvalidPolicy() {
             )
         );
 
-        assertThat(exception.getMessage(), is("phases [warm,cold] define one or more of [searchable_snapshot, forcemerge, freeze, shrink, rollup]" +
+        assertThat(exception.getMessage(), is("phases [warm,cold] define one or more of [forcemerge, freeze, shrink, rollup]" +
             " actions which are not allowed after a managed index is mounted as a searchable snapshot"));
     }
 
@@ -459,23 +460,32 @@ public void testConvertingSearchableSnapshotFromFullToPartial() throws Exception
 
     @SuppressWarnings("unchecked")
     public void testConvertingPartialSearchableSnapshotIntoFull() throws Exception {
-        String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT);
+        String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT) +"-000001";
         createSnapshotRepo(client(), snapshotRepo, randomBoolean());
-        createPolicy(client(), policy, null, null,
-            new Phase("cold", TimeValue.ZERO,
-                singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(),
-                    MountSearchableSnapshotRequest.Storage.SHARED_CACHE))),
+        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, 1L))),
+            null, null,
             new Phase("frozen", TimeValue.ZERO,
                 singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(),
                     MountSearchableSnapshotRequest.Storage.FULL_COPY))),
             null
         );
 
-        createIndex(index, Settings.builder()
-            .put(LifecycleSettings.LIFECYCLE_NAME, policy)
-            .build());
+        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(), index, true);
+        indexDocument(client(), alias, true);
+        rolloverMaxOneDocCondition(client(), alias);
 
         final String searchableSnapMountedIndexName = SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX +
             SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX + index;