diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java index 76b56809d2d5a..cef440c877388 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java @@ -117,6 +117,10 @@ public MountSearchableSnapshotRequest.Storage getStorageType() { return storageType; } + public String getSnapshotRepository() { + return snapshotRepository; + } + @Override public List toSteps(Client client, String phase, StepKey nextStepKey) { StepKey preActionBranchingKey = new StepKey(phase, NAME, CONDITIONAL_SKIP_ACTION_STEP); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java index 08712b51c46a1..62c430a4c790a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java @@ -290,6 +290,7 @@ public void validate(Collection phases) { } validateActionsFollowingSearchableSnapshot(phases); + validateAllSearchableSnapshotActionsUseSameRepository(phases); } static void validateActionsFollowingSearchableSnapshot(Collection phases) { @@ -313,6 +314,22 @@ static void validateActionsFollowingSearchableSnapshot(Collection phases) } } + static void validateAllSearchableSnapshotActionsUseSameRepository(Collection phases) { + Set 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; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java index af4e48762a790..5774fd4ff9439 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java @@ -30,6 +30,7 @@ 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 @@ private static Function getNameToActionFunction() { 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: diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java index 763e389549ea4..bd9fb017d9535 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java +++ b/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_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 void testShouldMigrateDataToTiers() { } } + public void testValidatingSearchableSnapshotRepos() { + Map hotActions = new HashMap<>(); + Map coldActions = new HashMap<>(); + Map 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 availableActions = convertActionNamesToActions(availableActionNames); Phase phase = new Phase(phaseName, TimeValue.ZERO, availableActions); diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java index a3b657e3cb9fb..86a00fe5c23ba 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java +++ b/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.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 void testIdenticalSearchableSnapshotActionIsNoop() throws Exception { .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 void testIdenticalSearchableSnapshotActionIsNoop() throws Exception { ((List>) ((Map) ((List) responseMap.get("responses")).get(0)).get("snapshots")).size(), equalTo(1)); + + Request hitCount = new Request("GET", "/" + searchableSnapMountedIndexName + "/_count"); + Map 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 void testConvertingSearchableSnapshotFromFullToPartial() throws Exception .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 void testConvertingSearchableSnapshotFromFullToPartial() throws Exception ((List>) ((Map) ((List) responseMap.get("responses")).get(0)).get("snapshots")).size(), equalTo(1)); + + Request hitCount = new Request("GET", "/" + searchableSnapMountedIndexName + "/_count"); + Map 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 void testConvertingPartialSearchableSnapshotIntoFull() throws Exception { .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 void testConvertingPartialSearchableSnapshotIntoFull() throws Exception { ((List>) ((Map) ((List) responseMap.get("responses")).get(0)).get("snapshots")).size(), equalTo(1)); + + Request hitCount = new Request("GET", "/" + searchableSnapMountedIndexName + "/_count"); + Map 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 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) - ((List) 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 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) - ((List) 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")); } }