Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disallow ILM searchable snapshot actions that use different repositories #68856

Merged
merged 1 commit into from
Feb 10, 2021
Merged

Disallow ILM searchable snapshot actions that use different repositories #68856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .../plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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 @@ -592,6 +593,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -399,6 +401,10 @@ public void testIdenticalSearchableSnapshotActionIsNoop() throws Exception {
((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")
Expand All @@ -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;
Expand All @@ -445,6 +451,10 @@ public void testConvertingSearchableSnapshotFromFullToPartial() throws Exception
((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")
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 @@ -491,71 +501,28 @@ public void testConvertingPartialSearchableSnapshotIntoFull() throws Exception {
((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"));
}
}