[TEST] Update destructive operations and disable close IT tests (#21274)

These tests had a single method due to the fact that es didn't support resetting settings when they were first written. They can now be rewritten to have separate methods and an after method that resets the setting that is left behind.

core/src/test/java/org/elasticsearch/indices/state/CloseIndexDisableCloseAllIT.java

@@ -18,99 +18,48 @@
  */
 package org.elasticsearch.indices.state;
 
-import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
 import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
-import org.elasticsearch.action.admin.indices.close.CloseIndexResponse;
 import org.elasticsearch.action.admin.indices.close.TransportCloseIndexAction;
-import org.elasticsearch.action.support.DestructiveOperations;
-import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.ESIntegTestCase;
-import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
+import org.junit.After;
-import org.elasticsearch.test.ESIntegTestCase.Scope;
 
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.notNullValue;
 
-@ClusterScope(scope=Scope.TEST, numDataNodes=2)
 public class CloseIndexDisableCloseAllIT extends ESIntegTestCase {
-    // Combined multiple tests into one, because cluster scope is test.
+
-    // The cluster scope is test b/c we can't clear cluster settings.
+    @After
-    public void testCloseAllRequiresName() {
+    public void afterTest() {
-        Settings clusterSettings = Settings.builder()
+        Settings settings = Settings.builder().put(TransportCloseIndexAction.CLUSTER_INDICES_CLOSE_ENABLE_SETTING.getKey(), (String)null)
-                .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), true)
                 .build();
-        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(clusterSettings));
+        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
+    }
+
+    public void testCloseAllRequiresName() {
         createIndex("test1", "test2", "test3");
-        ClusterHealthResponse healthResponse = client().admin().cluster().prepareHealth().setWaitForGreenStatus().execute().actionGet();
-        assertThat(healthResponse.isTimedOut(), equalTo(false));
 
-        // Close all explicitly
+        assertAcked(client().admin().indices().prepareClose("test3", "test2"));
-        try {
-            client().admin().indices().prepareClose("_all").execute().actionGet();
-            fail();
-        } catch (IllegalArgumentException e) {
-        }
-
-        // Close all wildcard
-        try {
-            client().admin().indices().prepareClose("*").execute().actionGet();
-            fail();
-        } catch (IllegalArgumentException e) {
-        }
-
-        // Close all wildcard
-        try {
-            client().admin().indices().prepareClose("test*").execute().actionGet();
-            fail();
-        } catch (IllegalArgumentException e) {
-        }
-
-        // Close all wildcard
-        try {
-            client().admin().indices().prepareClose("*", "-test1").execute().actionGet();
-            fail();
-        } catch (IllegalArgumentException e) {
-        }
-
-        // Close all wildcard
-        try {
-            client().admin().indices().prepareClose("*", "-test1", "+test1").execute().actionGet();
-            fail();
-        } catch (IllegalArgumentException e) {
-        }
-
-        CloseIndexResponse closeIndexResponse = client().admin().indices().prepareClose("test3", "test2").execute().actionGet();
-        assertThat(closeIndexResponse.isAcknowledged(), equalTo(true));
         assertIndexIsClosed("test2", "test3");
 
         // disable closing
-        Client client = client();
         createIndex("test_no_close");
-        healthResponse = client.admin().cluster().prepareHealth().setWaitForGreenStatus().execute().actionGet();
+        Settings settings = Settings.builder().put(TransportCloseIndexAction.CLUSTER_INDICES_CLOSE_ENABLE_SETTING.getKey(), false).build();
-        assertThat(healthResponse.isTimedOut(), equalTo(false));
+        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
-        client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder().put(TransportCloseIndexAction.CLUSTER_INDICES_CLOSE_ENABLE_SETTING.getKey(), false)).get();
 
-        try {
+        IllegalStateException illegalStateException = expectThrows(IllegalStateException.class,
-            client.admin().indices().prepareClose("test_no_close").execute().actionGet();
+                () -> client().admin().indices().prepareClose("test_no_close").get());
-            fail("exception expected");
+        assertEquals(illegalStateException.getMessage(),
-        } catch (IllegalStateException ex) {
+                "closing indices is disabled - set [cluster.indices.close.enable: true] to enable it. NOTE: closed indices still " +
-            assertEquals(ex.getMessage(), "closing indices is disabled - set [cluster.indices.close.enable: true] to enable it. NOTE: closed indices still consume a significant amount of diskspace");
+                        "consume a significant amount of diskspace");
-        }
     }
 
     private void assertIndexIsClosed(String... indices) {
-        checkIndexState(IndexMetaData.State.CLOSE, indices);
-    }
-
-    private void checkIndexState(IndexMetaData.State state, String... indices) {
         ClusterStateResponse clusterStateResponse = client().admin().cluster().prepareState().execute().actionGet();
         for (String index : indices) {
             IndexMetaData indexMetaData = clusterStateResponse.getState().metaData().indices().get(index);
-            assertThat(indexMetaData, notNullValue());
+            assertNotNull(indexMetaData);
-            assertThat(indexMetaData.getState(), equalTo(state));
+            assertEquals(IndexMetaData.State.CLOSE, indexMetaData.getState());
         }
     }
 }

core/src/test/java/org/elasticsearch/operateAllIndices/DestructiveOperationsIT.java

@@ -0,0 +1,147 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.operateAllIndices;
+
+import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
+import org.elasticsearch.action.support.DestructiveOperations;
+import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.test.ESIntegTestCase;
+import org.junit.After;
+
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.hamcrest.Matchers.equalTo;
+
+public class DestructiveOperationsIT extends ESIntegTestCase {
+
+    @After
+    public void afterTest() {
+        Settings settings = Settings.builder().put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), (String)null).build();
+        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
+    }
+
+    public void testDeleteIndexIsRejected() throws Exception {
+        Settings settings = Settings.builder()
+                .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), true)
+                .build();
+        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
+
+        createIndex("index1", "1index");
+
+        // Should succeed, since no wildcards
+        assertAcked(client().admin().indices().prepareDelete("1index").get());
+
+        expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareDelete("i*").get());
+        expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareDelete("_all").get());
+    }
+
+    public void testDeleteIndexDefaultBehaviour() throws Exception {
+        if (randomBoolean()) {
+            Settings settings = Settings.builder()
+                    .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false)
+                    .build();
+            assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
+        }
+
+        createIndex("index1", "1index");
+
+        if (randomBoolean()) {
+            assertAcked(client().admin().indices().prepareDelete("_all").get());
+        } else {
+            assertAcked(client().admin().indices().prepareDelete("*").get());
+        }
+
+        assertThat(client().admin().indices().prepareExists("_all").get().isExists(), equalTo(false));
+    }
+
+    public void testCloseIndexIsRejected() throws Exception {
+        Settings settings = Settings.builder()
+                .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), true)
+                .build();
+        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
+
+        createIndex("index1", "1index");
+
+        // Should succeed, since no wildcards
+        assertAcked(client().admin().indices().prepareClose("1index").get());
+
+        expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareClose("i*").get());
+        expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareClose("_all").get());
+    }
+
+    public void testCloseIndexDefaultBehaviour() throws Exception {
+        if (randomBoolean()) {
+            Settings settings = Settings.builder()
+                    .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false)
+                    .build();
+            assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
+        }
+
+        createIndex("index1", "1index");
+
+        if (randomBoolean()) {
+            assertAcked(client().admin().indices().prepareClose("_all").get());
+        } else {
+            assertAcked(client().admin().indices().prepareClose("*").get());
+        }
+
+        ClusterState state = client().admin().cluster().prepareState().get().getState();
+        for (ObjectObjectCursor<String, IndexMetaData> indexMetaDataObjectObjectCursor : state.getMetaData().indices()) {
+            assertEquals(IndexMetaData.State.CLOSE, indexMetaDataObjectObjectCursor.value.getState());
+        }
+    }
+
+    public void testOpenIndexIsRejected() throws Exception {
+        Settings settings = Settings.builder()
+                .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), true)
+                .build();
+        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
+
+        createIndex("index1", "1index");
+        assertAcked(client().admin().indices().prepareClose("1index", "index1").get());
+
+        expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareOpen("i*").get());
+        expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareOpen("_all").get());
+    }
+
+    public void testOpenIndexDefaultBehaviour() throws Exception {
+        if (randomBoolean()) {
+            Settings settings = Settings.builder()
+                    .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false)
+                    .build();
+            assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
+        }
+
+        createIndex("index1", "1index");
+        assertAcked(client().admin().indices().prepareClose("1index", "index1").get());
+
+        if (randomBoolean()) {
+            assertAcked(client().admin().indices().prepareOpen("_all").get());
+        } else {
+            assertAcked(client().admin().indices().prepareOpen("*").get());
+        }
+
+        ClusterState state = client().admin().cluster().prepareState().get().getState();
+        for (ObjectObjectCursor<String, IndexMetaData> indexMetaDataObjectObjectCursor : state.getMetaData().indices()) {
+            assertEquals(IndexMetaData.State.OPEN, indexMetaDataObjectObjectCursor.value.getState());
+        }
+    }
+}

core/src/test/java/org/elasticsearch/operateAllIndices/DestructiveOperationsIntegrationIT.java

@@ -1,109 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.operateAllIndices;
-
-import org.elasticsearch.action.support.DestructiveOperations;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.test.ESIntegTestCase;
-
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
-import static org.hamcrest.Matchers.equalTo;
-
-@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST)
-public class DestructiveOperationsIntegrationIT extends ESIntegTestCase {
-    // One test for test performance, since cluster scope is test
-    // The cluster scope is test b/c we can't clear cluster settings.
-    public void testDestructiveOperations() throws Exception {
-        Settings settings = Settings.builder()
-                .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), true)
-                .build();
-        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
-
-        assertAcked(client().admin().indices().prepareCreate("index1").get());
-        assertAcked(client().admin().indices().prepareCreate("1index").get());
-
-        // Should succeed, since no wildcards
-        assertAcked(client().admin().indices().prepareDelete("1index").get());
-
-        try {
-            // should fail since index1 is the only index.
-            client().admin().indices().prepareDelete("i*").get();
-            fail();
-        } catch (IllegalArgumentException e) {
-        }
-
-        try {
-            client().admin().indices().prepareDelete("_all").get();
-            fail();
-        } catch (IllegalArgumentException e) {
-        }
-
-        settings = Settings.builder()
-                .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false)
-                .build();
-        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
-
-        assertAcked(client().admin().indices().prepareDelete("_all").get());
-        assertThat(client().admin().indices().prepareExists("_all").get().isExists(), equalTo(false));
-
-        // end delete index:
-        // close index:
-        settings = Settings.builder()
-                .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), true)
-                .build();
-        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
-
-        assertAcked(client().admin().indices().prepareCreate("index1").get());
-        assertAcked(client().admin().indices().prepareCreate("1index").get());
-        // Should succeed, since no wildcards
-        assertAcked(client().admin().indices().prepareClose("1index").get());
-
-        try {
-            client().admin().indices().prepareClose("_all").get();
-            fail();
-        } catch (IllegalArgumentException e) {
-        }
-        try {
-            assertAcked(client().admin().indices().prepareOpen("_all").get());
-            fail();
-        } catch (IllegalArgumentException e) {
-        }
-        try {
-            client().admin().indices().prepareClose("*").get();
-            fail();
-        } catch (IllegalArgumentException e) {
-        }
-        try {
-            assertAcked(client().admin().indices().prepareOpen("*").get());
-            fail();
-        } catch (IllegalArgumentException e) {
-        }
-
-        settings = Settings.builder()
-                .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false)
-                .build();
-        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
-        assertAcked(client().admin().indices().prepareClose("_all").get());
-        assertAcked(client().admin().indices().prepareOpen("_all").get());
-
-        // end close index:
-        client().admin().indices().prepareDelete("_all").get();
-    }
-}