Skip to content

Commit

Permalink
Disallow ILM searchable snapshot actions that use different repositor…
Browse files Browse the repository at this point in the history 
…ies (elastic#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 elastic#68714
  • Loading branch information
@dakrone
dakrone committed Feb 10, 2021
1 parent e3f5304 commit 66c445a
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 64 deletions.
4 changes: 4 additions & 0 deletions .../plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ public MountSearchableSnapshotRequest.Storage getStorageType() {
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);
Expand Down
17 changes: 17 additions & 0 deletions ...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ public void validate(Collection<Phase> phases) {
}

validateActionsFollowingSearchableSnapshot(phases);
validateAllSearchableSnapshotActionsUseSameRepository(phases);
}

static void validateActionsFollowingSearchableSnapshot(Collection<Phase> phases) {
Expand All @@ -313,6 +314,22 @@ static void validateActionsFollowingSearchableSnapshot(Collection<Phase> phases)
}
}

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;
}
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -207,7 +208,7 @@ private static Function<String, LifecycleAction> 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:
Expand Down
35 changes: 35 additions & 0 deletions ...gin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -598,6 +599,40 @@ public void testShouldMigrateDataToTiers() {
}
}

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);
Expand Down
93 changes: 30 additions & 63 deletions ...src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,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.greaterThan;
import static org.hamcrest.Matchers.is;
Expand Down Expand Up @@ -378,6 +379,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;
Expand All @@ -401,6 +403,10 @@ public void testIdenticalSearchableSnapshotActionIsNoop() throws Exception {
}
assertThat("expected to have only one snapshot, but got: " + responseMap,
((List<Object>) responseMap.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")
Expand All @@ -421,7 +427,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;
Expand All @@ -445,6 +451,10 @@ public void testConvertingSearchableSnapshotFromFullToPartial() throws Exception
}
assertThat("expected to have only one snapshot, but got: " + responseMap,
((List<Object>) responseMap.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")
Expand All @@ -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;
Expand All @@ -489,71 +499,28 @@ public void testConvertingPartialSearchableSnapshotIntoFull() throws Exception {
}
assertThat("expected to have only one snapshot, but got: " + responseMap,
((List<Object>) responseMap.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"));
}
}

0 comments on commit 66c445a

Please sign in to comment.