Disallow ILM searchable snapshot actions that use different repositories (#68856)

This commit adds validation when updating or creating an ILM policy that mulitple searchable
snapshot actions all use the same repository. We currently do not have the infrastructure to support
switching a searchable snapshot from one repository to another, so until we have that we will
disallow this (edge case) when creating a policy.

Relates to #68714

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

@@ -117,6 +117,10 @@ public class SearchableSnapshotAction implements LifecycleAction {
         return storageType;
     }
 
+    public String getSnapshotRepository() {
+        return snapshotRepository;
+    }
+
     @Override
     public List<Step> toSteps(Client client, String phase, StepKey nextStepKey) {
         StepKey preActionBranchingKey = new StepKey(phase, NAME, CONDITIONAL_SKIP_ACTION_STEP);

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

@@ -290,6 +290,7 @@ public class TimeseriesLifecycleType implements LifecycleType {
         }
 
         validateActionsFollowingSearchableSnapshot(phases);
+        validateAllSearchableSnapshotActionsUseSameRepository(phases);
     }
 
     static void validateActionsFollowingSearchableSnapshot(Collection<Phase> phases) {
@@ -313,6 +314,22 @@ public class TimeseriesLifecycleType implements LifecycleType {
         }
     }
 
+    static void validateAllSearchableSnapshotActionsUseSameRepository(Collection<Phase> phases) {
+        Set<String> allRepos = phases.stream()
+            .flatMap(phase -> phase.getActions().entrySet().stream())
+            .filter(e -> e.getKey().equals(SearchableSnapshotAction.NAME))
+            .map(Map.Entry::getValue)
+            .map(action -> (SearchableSnapshotAction) action)
+            .map(SearchableSnapshotAction::getSnapshotRepository)
+            .collect(Collectors.toSet());
+
+        if (allRepos.size() > 1) {
+            throw new IllegalArgumentException("policy specifies [" + SearchableSnapshotAction.NAME +
+                "] action multiple times with differing repositories " + allRepos +
+                ", the same repository must be used for all searchable snapshot actions");
+        }
+    }
+
     private static boolean definesAllocationRules(AllocateAction action) {
         return action.getRequire().isEmpty() == false || action.getInclude().isEmpty() == false || action.getExclude().isEmpty() == false;
     }

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

@@ -30,6 +30,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
 
+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;
@@ -207,7 +208,7 @@ public class LifecyclePolicyTests extends AbstractSerializingTestCase<LifecycleP
                     case UnfollowAction.NAME:
                         return new UnfollowAction();
                     case SearchableSnapshotAction.NAME:
-                        return new SearchableSnapshotAction(randomAlphaOfLengthBetween(1, 10));
+                        return new SearchableSnapshotAction("repo", randomBoolean(), randomStorageType());
                     case MigrateAction.NAME:
                         return new MigrateAction(false);
                     case RollupILMAction.NAME:

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

@@ -39,6 +39,7 @@ import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.VALID_HOT
 import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.VALID_PHASES;
 import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.VALID_WARM_ACTIONS;
 import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.WARM_PHASE;
+import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.validateAllSearchableSnapshotActionsUseSameRepository;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
@@ -592,6 +593,40 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase {
         }
     }
 
+    public void testValidatingSearchableSnapshotRepos() {
+        Map<String, LifecycleAction> hotActions = new HashMap<>();
+        Map<String, LifecycleAction> coldActions = new HashMap<>();
+        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));
+
+            Phase hotPhase = new Phase(HOT_PHASE, TimeValue.ZERO, hotActions);
+            Phase coldPhase = new Phase(HOT_PHASE, TimeValue.ZERO, coldActions);
+            Phase frozenPhase = new Phase(HOT_PHASE, TimeValue.ZERO, frozenActions);
+
+            validateAllSearchableSnapshotActionsUseSameRepository(Arrays.asList(hotPhase, coldPhase, frozenPhase));
+        }
+
+        {
+            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));
+
+            Phase hotPhase = new Phase(HOT_PHASE, TimeValue.ZERO, hotActions);
+            Phase coldPhase = new Phase(HOT_PHASE, TimeValue.ZERO, coldActions);
+            Phase frozenPhase = new Phase(HOT_PHASE, TimeValue.ZERO, frozenActions);
+
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> validateAllSearchableSnapshotActionsUseSameRepository(Arrays.asList(hotPhase, coldPhase, frozenPhase)));
+            assertThat(e.getMessage(), containsString("policy specifies [searchable_snapshot]" +
+                " action multiple times with differing repositories [repo2, repo1]," +
+                " the same repository must be used for all searchable snapshot actions"));
+        }
+    }
+
     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);

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

@@ -56,6 +56,7 @@ import static org.elasticsearch.xpack.TimeSeriesRestDriver.getNumberOfSegments;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.getStepKeyForIndex;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.indexDocument;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.rolloverMaxOneDocCondition;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.is;
@@ -374,6 +375,7 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
             .put(LifecycleSettings.LIFECYCLE_NAME, policy)
             .build());
         ensureGreen(index);
+        indexDocument(client(), index, true);
 
         final String searchableSnapMountedIndexName = (storage == MountSearchableSnapshotRequest.Storage.FULL_COPY ?
             SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX : SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX) + index;
@@ -399,6 +401,10 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
             ((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));
     }
 
     @SuppressWarnings("unchecked")
@@ -419,7 +425,7 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
             .put(LifecycleSettings.LIFECYCLE_NAME, policy)
             .build());
         ensureGreen(index);
-        indexDocument(client(), index);
+        indexDocument(client(), index, true);
 
         final String searchableSnapMountedIndexName = SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX +
             SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX + index;
@@ -445,6 +451,10 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
             ((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));
     }
 
     @SuppressWarnings("unchecked")
@@ -465,7 +475,7 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
             .put(LifecycleSettings.LIFECYCLE_NAME, policy)
             .build());
         ensureGreen(index);
-        indexDocument(client(), index);
+        indexDocument(client(), index, true);
 
         final String searchableSnapMountedIndexName = SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX +
             SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX + index;
@@ -491,71 +501,28 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
             ((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));
     }
 
-    @SuppressWarnings("unchecked")
-    @AwaitsFix(bugUrl = "functionality not yet implemented")
-    public void testSecondSearchableSnapshotChangesRepo() throws Exception {
-        String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT);
+    public void testSecondSearchableSnapshotUsingDifferentRepoThrows() throws Exception {
         String secondRepo = randomAlphaOfLengthBetween(10, 20);
         createSnapshotRepo(client(), snapshotRepo, randomBoolean());
         createSnapshotRepo(client(), secondRepo, randomBoolean());
-        createPolicy(client(), policy, null, null,
-            new Phase("cold", TimeValue.ZERO,
-                singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(),
-                    MountSearchableSnapshotRequest.Storage.FULL_COPY))),
-            new Phase("frozen", TimeValue.ZERO,
-                singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(secondRepo, randomBoolean(),
-                    MountSearchableSnapshotRequest.Storage.SHARED_CACHE))),
-            null
-        );
-
-        createIndex(index, Settings.builder()
-            .put(LifecycleSettings.LIFECYCLE_NAME, policy)
-            .build());
-        ensureGreen(index);
-        indexDocument(client(), index);
-
-        final String searchableSnapMountedIndexName = SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX +
-            SearchableSnapshotAction.FULL_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);
-
-        // Check first repo has exactly 1 snapshot
-        {
-            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));
-        }
-
-        // Check second repo has exactly 1 snapshot
-        {
-            Request getSnaps = new Request("GET", "/_snapshot/" + secondRepo + "/_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));
-        }
+        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))),
+                new Phase("frozen", TimeValue.ZERO,
+                    singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(secondRepo, randomBoolean(),
+                        MountSearchableSnapshotRequest.Storage.SHARED_CACHE))),
+                null
+            ));
+
+        assertThat(e.getMessage(),
+            containsString("policy specifies [searchable_snapshot] action multiple times with differing repositories"));
     }
 }