fail rollover request if rollover index already exists

core/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponse.java

@@ -39,18 +39,16 @@ public final class RolloverResponse extends ActionResponse implements ToXContent
     private Set<Map.Entry<String, Boolean>> conditionStatus;
     private boolean dryRun;
     private boolean rolledOver;
-    private boolean rolloverIndexCreated;
 
     RolloverResponse() {
     }
 
     RolloverResponse(String oldIndex, String newIndex, Set<Condition.Result> conditionResults,
-                     boolean dryRun, boolean rolledOver, boolean rolloverIndexCreated) {
+                     boolean dryRun, boolean rolledOver) {
         this.oldIndex = oldIndex;
         this.newIndex = newIndex;
         this.dryRun = dryRun;
         this.rolledOver = rolledOver;
-        this.rolloverIndexCreated = rolloverIndexCreated;
         this.conditionStatus = conditionResults.stream()
             .map(result -> new AbstractMap.SimpleEntry<>(result.condition.toString(), result.matched))
             .collect(Collectors.toSet());
@@ -91,13 +89,6 @@ public final class RolloverResponse extends ActionResponse implements ToXContent
         return rolledOver;
     }
 
-    /**
-     * Returns if the rollover index had to be explicitly created
-     */
-    public boolean isRolloverIndexCreated() {
-        return rolloverIndexCreated;
-    }
-
     @Override
     public void readFrom(StreamInput in) throws IOException {
         super.readFrom(in);
@@ -113,7 +104,6 @@ public final class RolloverResponse extends ActionResponse implements ToXContent
         conditionStatus = conditions;
         dryRun = in.readBoolean();
         rolledOver = in.readBoolean();
-        rolloverIndexCreated = in.readBoolean();
     }
 
     @Override
@@ -128,7 +118,6 @@ public final class RolloverResponse extends ActionResponse implements ToXContent
         }
         out.writeBoolean(dryRun);
         out.writeBoolean(rolledOver);
-        out.writeBoolean(rolloverIndexCreated);
     }
 
     @Override
@@ -137,7 +126,6 @@ public final class RolloverResponse extends ActionResponse implements ToXContent
         builder.field(Fields.NEW_INDEX, newIndex);
         builder.field(Fields.ROLLED_OVER, rolledOver);
         builder.field(Fields.DRY_RUN, dryRun);
-        builder.field(Fields.ROLLOVER_INDEX_CREATED, rolloverIndexCreated);
         builder.startObject(Fields.CONDITIONS);
         for (Map.Entry<String, Boolean> entry : conditionStatus) {
             builder.field(entry.getKey(), entry.getValue());
@@ -151,7 +139,6 @@ public final class RolloverResponse extends ActionResponse implements ToXContent
         static final String OLD_INDEX = "old_index";
         static final String DRY_RUN = "dry_run";
         static final String ROLLED_OVER = "rolled_over";
-        static final String ROLLOVER_INDEX_CREATED = "rollover_index_created";
         static final String CONDITIONS = "conditions";
     }
 }

core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java

@@ -43,6 +43,7 @@ import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.shard.DocsStats;
+import org.elasticsearch.indices.IndexAlreadyExistsException;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
@@ -108,36 +109,42 @@ public class TransportRolloverAction extends TransportMasterNodeAction<RolloverR
                     final String rolloverIndexName = generateRolloverIndexName(sourceIndexName);
                     if (rolloverRequest.isDryRun()) {
                         listener.onResponse(
-                            new RolloverResponse(sourceIndexName, rolloverIndexName, conditionResults, true, false,
-                                false));
+                            new RolloverResponse(sourceIndexName, rolloverIndexName, conditionResults, true, false));
                         return;
                     }
                     if (conditionResults.size() == 0 || conditionResults.stream().anyMatch(result -> result.matched)) {
-                        boolean createRolloverIndex = metaData.index(rolloverIndexName) == null;
-                        final RolloverResponse rolloverResponse =
-                            new RolloverResponse(sourceIndexName, rolloverIndexName, conditionResults, false, true,
-                                createRolloverIndex);
-                        if (createRolloverIndex) {
-                            createIndexService.createIndex(prepareCreateIndexRequest(rolloverIndexName, rolloverRequest),
-                                new ActionListener<ClusterStateUpdateResponse>() {
-                                    @Override
-                                    public void onResponse(ClusterStateUpdateResponse response) {
-                                        rollover(rolloverRequest, rolloverResponse, listener);
-                                    }
-
-                                    @Override
-                                    public void onFailure(Throwable t) {
-                                        listener.onFailure(t);
-                                    }
-                                });
-                        } else {
-                            rollover(rolloverRequest, rolloverResponse, listener);
-                        }
+                        createIndexService.createIndex(prepareCreateIndexRequest(rolloverIndexName, rolloverRequest),
+                            new ActionListener<ClusterStateUpdateResponse>() {
+                                @Override
+                                public void onResponse(ClusterStateUpdateResponse response) {
+                                    // switch the alias to point to the newly created index
+                                    indexAliasesService.indicesAliases(
+                                        prepareRolloverAliasesUpdateRequest(sourceIndexName, rolloverIndexName,
+                                            rolloverRequest),
+                                        new ActionListener<ClusterStateUpdateResponse>() {
+                                            @Override
+                                            public void onResponse(ClusterStateUpdateResponse clusterStateUpdateResponse) {
+                                                listener.onResponse(
+                                                    new RolloverResponse(sourceIndexName, rolloverIndexName,
+                                                        conditionResults, false, true));
+                                            }
+
+                                            @Override
+                                            public void onFailure(Throwable e) {
+                                                listener.onFailure(e);
+                                            }
+                                        });
+                                }
+
+                                @Override
+                                public void onFailure(Throwable t) {
+                                    listener.onFailure(t);
+                                }
+                            });
                     } else {
                         // conditions not met
                         listener.onResponse(
-                            new RolloverResponse(sourceIndexName, sourceIndexName, conditionResults, false, false,
-                                false)
+                            new RolloverResponse(sourceIndexName, sourceIndexName, conditionResults, false, false)
                         );
                     }
                 }
@@ -150,23 +157,6 @@ public class TransportRolloverAction extends TransportMasterNodeAction<RolloverR
         );
     }
 
-    private void rollover(final RolloverRequest request, final RolloverResponse response,
-                          ActionListener<RolloverResponse> listener) {
-        indexAliasesService.indicesAliases(
-            prepareRolloverAliasesUpdateRequest(response.getOldIndex(), response.getNewIndex(), request),
-            new ActionListener<ClusterStateUpdateResponse>() {
-                @Override
-                public void onResponse(ClusterStateUpdateResponse clusterStateUpdateResponse) {
-                    listener.onResponse(response);
-                }
-
-                @Override
-                public void onFailure(Throwable e) {
-                    listener.onFailure(e);
-                }
-            });
-    }
-
     static IndicesAliasesClusterStateUpdateRequest prepareRolloverAliasesUpdateRequest(String oldIndex, String newIndex,
                                                                                        RolloverRequest request) {
         final IndicesAliasesClusterStateUpdateRequest updateRequest = new IndicesAliasesClusterStateUpdateRequest()

core/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java

@@ -24,6 +24,7 @@ import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.indices.IndexAlreadyExistsException;
 import org.elasticsearch.test.ESIntegTestCase;
 
 import java.util.Map;
@@ -41,7 +42,6 @@ public class RolloverIT extends ESIntegTestCase {
         assertThat(response.getNewIndex(), equalTo("test_index-2"));
         assertThat(response.isDryRun(), equalTo(false));
         assertThat(response.isRolledOver(), equalTo(true));
-        assertThat(response.isRolloverIndexCreated(), equalTo(true));
         assertThat(response.getConditionStatus().size(), equalTo(0));
         final ClusterState state = client().admin().cluster().prepareState().get().getState();
         final IndexMetaData oldIndex = state.metaData().index("test_index-1");
@@ -59,7 +59,6 @@ public class RolloverIT extends ESIntegTestCase {
         assertThat(response.getNewIndex(), equalTo("test_index-3"));
         assertThat(response.isDryRun(), equalTo(false));
         assertThat(response.isRolledOver(), equalTo(true));
-        assertThat(response.isRolloverIndexCreated(), equalTo(true));
         assertThat(response.getConditionStatus().size(), equalTo(0));
         final ClusterState state = client().admin().cluster().prepareState().get().getState();
         final IndexMetaData oldIndex = state.metaData().index("test_index-2");
@@ -82,7 +81,6 @@ public class RolloverIT extends ESIntegTestCase {
         assertThat(response.getNewIndex(), equalTo("test_index-3"));
         assertThat(response.isDryRun(), equalTo(false));
         assertThat(response.isRolledOver(), equalTo(true));
-        assertThat(response.isRolloverIndexCreated(), equalTo(true));
         assertThat(response.getConditionStatus().size(), equalTo(0));
         final ClusterState state = client().admin().cluster().prepareState().get().getState();
         final IndexMetaData oldIndex = state.metaData().index("test_index-2");
@@ -103,7 +101,6 @@ public class RolloverIT extends ESIntegTestCase {
         assertThat(response.getNewIndex(), equalTo("test_index-2"));
         assertThat(response.isDryRun(), equalTo(true));
         assertThat(response.isRolledOver(), equalTo(false));
-        assertThat(response.isRolloverIndexCreated(), equalTo(false));
         assertThat(response.getConditionStatus().size(), equalTo(0));
         final ClusterState state = client().admin().cluster().prepareState().get().getState();
         final IndexMetaData oldIndex = state.metaData().index("test_index-1");
@@ -122,7 +119,6 @@ public class RolloverIT extends ESIntegTestCase {
         assertThat(response.getNewIndex(), equalTo("test_index-0"));
         assertThat(response.isDryRun(), equalTo(false));
         assertThat(response.isRolledOver(), equalTo(false));
-        assertThat(response.isRolloverIndexCreated(), equalTo(false));
         assertThat(response.getConditionStatus().size(), equalTo(1));
         final Map.Entry<String, Boolean> conditionEntry = response.getConditionStatus().iterator().next();
         assertThat(conditionEntry.getKey(), equalTo(new MaxAgeCondition(TimeValue.timeValueHours(4)).toString()));
@@ -140,16 +136,11 @@ public class RolloverIT extends ESIntegTestCase {
         assertAcked(prepareCreate("test_index-1").get());
         index("test_index-1", "type1", "1", "field", "value");
         flush("test_index-0", "test_index-1");
-        final RolloverResponse response = client().admin().indices().prepareRolloverIndex("test_alias").get();
-        assertThat(response.getOldIndex(), equalTo("test_index-0"));
-        assertThat(response.getNewIndex(), equalTo("test_index-1"));
-        assertThat(response.isDryRun(), equalTo(false));
-        assertThat(response.isRolledOver(), equalTo(true));
-        assertThat(response.isRolloverIndexCreated(), equalTo(false));
-        final ClusterState state = client().admin().cluster().prepareState().get().getState();
-        final IndexMetaData oldIndex = state.metaData().index("test_index-0");
-        assertFalse(oldIndex.getAliases().containsKey("test_alias"));
-        final IndexMetaData newIndex = state.metaData().index("test_index-1");
-        assertTrue(newIndex.getAliases().containsKey("test_alias"));
+        try {
+            client().admin().indices().prepareRolloverIndex("test_alias").get();
+            fail("expected failure due to existing rollover index");
+        } catch (IndexAlreadyExistsException e) {
+            assertThat(e.getIndex().getName(), equalTo("test_index-1"));
+        }
     }
 }

docs/reference/indices/rollover-index.asciidoc

@@ -3,9 +3,10 @@
 
 The rollover index API allows to switch the index pointed to by an alias given some predicates.
 In order to rollover an index, the provided alias has to point to a single index. Upon satisfying
-any of the predicates, the alias is switched to point to a new index, creating the index if it
-does not exist. The rollover API requires the old concrete index name to have `{index_prefix}-{num}`
-format, as rollover index name is generated following `{index_prefix}-{num+1}` format.
+any of the predicates, the alias is switched to point to the rollover index, if the rollover index
+already exists, the rollover fails. The rollover API requires the old concrete index name to have
+`{index_prefix}-{num}` format, as rollover index name is generated following `{index_prefix}-{num+1}`
+format.
 
 This API is syntactic sugar for changing the index pointed to by an alias given some predicate.
 
@@ -35,11 +36,11 @@ $ curl -XPOST 'http://localhost:9200/index_alias/_rollover' -d '{
 <2> Sets a condition that the index has to have at least a 1000 documents
 
 The API call above switches the index pointed to by `index_alias` from `index-1` to `index-2`, if any
-of the conditions are met. If `index-2` does not exist, it is created (using matching <<indices-templates>>
-if available). The API call returns immediately if none of the conditions are met.
+of the conditions are met. `index-2` is created (using matching <<indices-templates>> if available).
+The API call returns immediately if none of the conditions are met.
 
-The `_rollover` API is similar to <<indices-create-index>> and accepts `settings`, `mappings` and `aliases`
-to override the index create request for a non-existent rolled over index.
+The `_rollover` API is similar to <<indices-create-index>> and accepts `settings`, `mappings` and
+`aliases` to override the index create request for the rollover index.
 
 [source,js]
 --------------------------------------------------
@@ -67,8 +68,7 @@ An example response for the index rollover API:
 	"new_index": "index-2", <2>
 	"rolled_over": true, <3>
 	"dry_run": false, <4>
-	"rollover_index_created": true, <5>
-	"conditions": { <6>
+	"conditions": { <5>
 	    "[max_age: 7d]": true,
 	    "[max_docs: 1000]": true
 	}
@@ -78,6 +78,5 @@ An example response for the index rollover API:
 <2> name of the index the alias currently points to
 <3> whether the alias switch was successful
 <4> whether the rollover was dry run
-<5> whether the rolled over index had to be explicitly created
-<6> status of the evaluated request conditions
+<5> status of the evaluated request conditions
 

rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/10_basic.yaml

@@ -39,8 +39,7 @@
   - match: { old_index: logs-1 }
   - match: { new_index: logs-2 }
   - match: { rolled_over: true }
-  - match: { rollover_index_created: true }
-  - match: { simulated: false }
+  - match: { dry_run: false }
   - match: { conditions: { "[max_docs: 1]": true } }
 
   # ensure new index is created