From 20bbe5bae4e84b5a42fc71e3bde88906e6ba716e Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Tue, 10 Mar 2020 10:56:12 -0600 Subject: [PATCH] Fix Rollover handing of hidden aliases (#53146) Prior to this commit, rollover did not propagate the `is_hidden` alias property when rollover over an index. This commit ensures that an alias that's rollover over will remain hidden. --- .../rollover/TransportRolloverAction.java | 17 +++-- .../admin/indices/rollover/RolloverIT.java | 63 +++++++++++++++++ .../TransportRolloverActionTests.java | 68 ++++++++++++++++++- 3 files changed, 139 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java index 31d01e01361dd..d763206433c04 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java @@ -36,6 +36,7 @@ import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.AliasAction; +import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.AliasOrIndex; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; @@ -122,7 +123,8 @@ protected void masterOperation(Task task, final RolloverRequest rolloverRequest, validate(metaData, rolloverRequest); final AliasOrIndex.Alias alias = (AliasOrIndex.Alias) metaData.getAliasAndIndexLookup().get(rolloverRequest.getAlias()); final IndexMetaData indexMetaData = alias.getWriteIndex(); - final boolean explicitWriteIndex = Boolean.TRUE.equals(indexMetaData.getAliases().get(alias.getAliasName()).writeIndex()); + final AliasMetaData aliasMetaData = indexMetaData.getAliases().get(alias.getAliasName()); + final boolean explicitWriteIndex = Boolean.TRUE.equals(aliasMetaData.writeIndex()); final String sourceProvidedName = indexMetaData.getSettings().get(IndexMetaData.SETTING_INDEX_PROVIDED_NAME, indexMetaData.getIndex().getName()); final String sourceIndexName = indexMetaData.getIndex().getName(); @@ -162,7 +164,8 @@ public void onResponse(IndicesStatsResponse statsResponse) { public ClusterState execute(ClusterState currentState) throws Exception { ClusterState newState = createIndexService.applyCreateIndexRequest(currentState, createIndexRequest); newState = indexAliasesService.applyAliasActions(newState, - rolloverAliasToNewIndex(sourceIndexName, rolloverIndexName, rolloverRequest, explicitWriteIndex)); + rolloverAliasToNewIndex(sourceIndexName, rolloverIndexName, rolloverRequest, explicitWriteIndex, + aliasMetaData.isHidden())); RolloverInfo rolloverInfo = new RolloverInfo(rolloverRequest.getAlias(), metConditions, threadPool.absoluteTimeInMillis()); return ClusterState.builder(newState) @@ -210,15 +213,15 @@ public void onFailure(Exception e) { * alias pointing to multiple indices will have to be an explicit write index (ie. the old index alias has is_write_index set to true) * in which case, after the rollover, the new index will need to be the explicit write index. */ - static List rolloverAliasToNewIndex(String oldIndex, String newIndex, RolloverRequest request, - boolean explicitWriteIndex) { + static List rolloverAliasToNewIndex(String oldIndex, String newIndex, RolloverRequest request, boolean explicitWriteIndex, + @Nullable Boolean isHidden) { if (explicitWriteIndex) { return unmodifiableList(Arrays.asList( - new AliasAction.Add(newIndex, request.getAlias(), null, null, null, true, null), - new AliasAction.Add(oldIndex, request.getAlias(), null, null, null, false, null))); + new AliasAction.Add(newIndex, request.getAlias(), null, null, null, true, isHidden), + new AliasAction.Add(oldIndex, request.getAlias(), null, null, null, false, isHidden))); } else { return unmodifiableList(Arrays.asList( - new AliasAction.Add(newIndex, request.getAlias(), null, null, null, null, null), + new AliasAction.Add(newIndex, request.getAlias(), null, null, null, null, isHidden), new AliasAction.Remove(oldIndex, request.getAlias()))); } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java index 1cabb0020fe7b..09dc25e6dedd3 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java @@ -47,6 +47,7 @@ import static org.hamcrest.Matchers.everyItem; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.collection.IsEmptyCollection.empty; import static org.hamcrest.core.CombinableMatcher.both; import static org.hamcrest.number.OrderingComparison.greaterThanOrEqualTo; @@ -432,4 +433,66 @@ public void testRolloverWithClosedWriteIndex() throws Exception { assertEquals(writeIndexPrefix + "000001", rolloverResponse.getOldIndex()); assertEquals(writeIndexPrefix + "000002", rolloverResponse.getNewIndex()); } + + public void testRolloverWithHiddenAliasesAndExplicitWriteIndex() { + long beforeTime = client().threadPool().absoluteTimeInMillis() - 1000L; + final String indexNamePrefix = "test_index_hidden-"; + final String firstIndexName = indexNamePrefix + "000001"; + final String secondIndexName = indexNamePrefix + "000002"; + + final String aliasName = "test_alias"; + assertAcked(prepareCreate(firstIndexName).addAlias(new Alias(aliasName).writeIndex(true).isHidden(true)).get()); + index(aliasName, SINGLE_MAPPING_NAME, "1", "field", "value"); + refresh(); + final RolloverResponse response = client().admin().indices().prepareRolloverIndex(aliasName).get(); + assertThat(response.getOldIndex(), equalTo(firstIndexName)); + assertThat(response.getNewIndex(), equalTo(secondIndexName)); + assertThat(response.isDryRun(), equalTo(false)); + assertThat(response.isRolledOver(), equalTo(true)); + assertThat(response.getConditionStatus().size(), equalTo(0)); + final ClusterState state = client().admin().cluster().prepareState().get().getState(); + final IndexMetaData oldIndex = state.metaData().index(firstIndexName); + assertTrue(oldIndex.getAliases().containsKey(aliasName)); + assertTrue(oldIndex.getAliases().get(aliasName).isHidden()); + assertFalse(oldIndex.getAliases().get(aliasName).writeIndex()); + final IndexMetaData newIndex = state.metaData().index(secondIndexName); + assertTrue(newIndex.getAliases().containsKey(aliasName)); + assertTrue(newIndex.getAliases().get(aliasName).isHidden()); + assertTrue(newIndex.getAliases().get(aliasName).writeIndex()); + assertThat(oldIndex.getRolloverInfos().size(), equalTo(1)); + assertThat(oldIndex.getRolloverInfos().get(aliasName).getAlias(), equalTo(aliasName)); + assertThat(oldIndex.getRolloverInfos().get(aliasName).getMetConditions(), is(empty())); + assertThat(oldIndex.getRolloverInfos().get(aliasName).getTime(), + is(both(greaterThanOrEqualTo(beforeTime)).and(lessThanOrEqualTo(client().threadPool().absoluteTimeInMillis() + 1000L)))); + } + + public void testRolloverWithHiddenAliasesAndImplicitWriteIndex() { + long beforeTime = client().threadPool().absoluteTimeInMillis() - 1000L; + final String indexNamePrefix = "test_index_hidden-"; + final String firstIndexName = indexNamePrefix + "000001"; + final String secondIndexName = indexNamePrefix + "000002"; + + final String aliasName = "test_alias"; + assertAcked(prepareCreate(firstIndexName).addAlias(new Alias(aliasName).isHidden(true)).get()); + index(aliasName, SINGLE_MAPPING_NAME, "1", "field", "value"); + refresh(); + final RolloverResponse response = client().admin().indices().prepareRolloverIndex(aliasName).get(); + assertThat(response.getOldIndex(), equalTo(firstIndexName)); + assertThat(response.getNewIndex(), equalTo(secondIndexName)); + assertThat(response.isDryRun(), equalTo(false)); + assertThat(response.isRolledOver(), equalTo(true)); + assertThat(response.getConditionStatus().size(), equalTo(0)); + final ClusterState state = client().admin().cluster().prepareState().get().getState(); + final IndexMetaData oldIndex = state.metaData().index(firstIndexName); + assertFalse(oldIndex.getAliases().containsKey(aliasName)); + final IndexMetaData newIndex = state.metaData().index(secondIndexName); + assertTrue(newIndex.getAliases().containsKey(aliasName)); + assertTrue(newIndex.getAliases().get(aliasName).isHidden()); + assertThat(newIndex.getAliases().get(aliasName).writeIndex(), nullValue()); + assertThat(oldIndex.getRolloverInfos().size(), equalTo(1)); + assertThat(oldIndex.getRolloverInfos().get(aliasName).getAlias(), equalTo(aliasName)); + assertThat(oldIndex.getRolloverInfos().get(aliasName).getMetConditions(), is(empty())); + assertThat(oldIndex.getRolloverInfos().get(aliasName).getTime(), + is(both(greaterThanOrEqualTo(beforeTime)).and(lessThanOrEqualTo(client().threadPool().absoluteTimeInMillis() + 1000L)))); + } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java index 2b4743224453a..0e95d6b627eca 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java @@ -91,7 +91,9 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; @@ -225,7 +227,7 @@ public void testRolloverAliasActions() { String targetIndex = randomAlphaOfLength(10); final RolloverRequest rolloverRequest = new RolloverRequest(sourceAlias, targetIndex); - List actions = TransportRolloverAction.rolloverAliasToNewIndex(sourceIndex, targetIndex, rolloverRequest, false); + List actions = TransportRolloverAction.rolloverAliasToNewIndex(sourceIndex, targetIndex, rolloverRequest, false, null); assertThat(actions, hasSize(2)); boolean foundAdd = false; boolean foundRemove = false; @@ -249,7 +251,7 @@ public void testRolloverAliasActionsWithExplicitWriteIndex() { String sourceIndex = randomAlphaOfLength(10); String targetIndex = randomAlphaOfLength(10); final RolloverRequest rolloverRequest = new RolloverRequest(sourceAlias, targetIndex); - List actions = TransportRolloverAction.rolloverAliasToNewIndex(sourceIndex, targetIndex, rolloverRequest, true); + List actions = TransportRolloverAction.rolloverAliasToNewIndex(sourceIndex, targetIndex, rolloverRequest, true, null); assertThat(actions, hasSize(2)); boolean foundAddWrite = false; @@ -272,6 +274,68 @@ public void testRolloverAliasActionsWithExplicitWriteIndex() { assertTrue(foundRemoveWrite); } + public void testRolloverAliasActionsWithHiddenAliasAndExplicitWriteIndex() { + String sourceAlias = randomAlphaOfLength(10); + String sourceIndex = randomAlphaOfLength(10); + String targetIndex = randomAlphaOfLength(10); + final RolloverRequest rolloverRequest = new RolloverRequest(sourceAlias, targetIndex); + List actions = TransportRolloverAction.rolloverAliasToNewIndex(sourceIndex, targetIndex, rolloverRequest, true, true); + + assertThat(actions, hasSize(2)); + boolean foundAddWrite = false; + boolean foundRemoveWrite = false; + for (AliasAction action : actions) { + assertThat(action, instanceOf(AliasAction.Add.class)); + AliasAction.Add addAction = (AliasAction.Add) action; + if (action.getIndex().equals(targetIndex)) { + assertEquals(sourceAlias, addAction.getAlias()); + assertTrue(addAction.writeIndex()); + assertTrue(addAction.isHidden()); + foundAddWrite = true; + } else if (action.getIndex().equals(sourceIndex)) { + assertEquals(sourceAlias, addAction.getAlias()); + assertFalse(addAction.writeIndex()); + assertTrue(addAction.isHidden()); + foundRemoveWrite = true; + } else { + throw new AssertionError("Unknown index [" + action.getIndex() + "]"); + } + } + assertTrue(foundAddWrite); + assertTrue(foundRemoveWrite); + } + + public void testRolloverAliasActionsWithHiddenAliasAndImplicitWriteIndex() { + String sourceAlias = randomAlphaOfLength(10); + String sourceIndex = randomAlphaOfLength(10); + String targetIndex = randomAlphaOfLength(10); + final RolloverRequest rolloverRequest = new RolloverRequest(sourceAlias, targetIndex); + List actions = TransportRolloverAction.rolloverAliasToNewIndex(sourceIndex, targetIndex, rolloverRequest, false, true); + + assertThat(actions, hasSize(2)); + boolean foundAddWrite = false; + boolean foundRemoveWrite = false; + for (AliasAction action : actions) { + if (action.getIndex().equals(targetIndex)) { + assertThat(action, instanceOf(AliasAction.Add.class)); + AliasAction.Add addAction = (AliasAction.Add) action; + assertEquals(sourceAlias, addAction.getAlias()); + assertThat(addAction.writeIndex(), nullValue()); + assertTrue(addAction.isHidden()); + foundAddWrite = true; + } else if (action.getIndex().equals(sourceIndex)) { + assertThat(action, instanceOf(AliasAction.Remove.class)); + AliasAction.Remove removeAction = (AliasAction.Remove) action; + assertEquals(sourceAlias, removeAction.getAlias()); + foundRemoveWrite = true; + } else { + throw new AssertionError("Unknown index [" + action.getIndex() + "]"); + } + } + assertTrue(foundAddWrite); + assertTrue(foundRemoveWrite); + } + public void testValidation() { String index1 = randomAlphaOfLength(10); String aliasWithWriteIndex = randomAlphaOfLength(10);