Make feature reset API response more informative

client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java

@@ -8,6 +8,7 @@
 
 package org.elasticsearch.client.feature;
 
+import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.ObjectParser;
@@ -47,24 +48,30 @@ public static ResetFeaturesResponse parse(XContentParser parser) {
     public static class ResetFeatureStateStatus {
         private final String featureName;
         private final String status;
+        private final Exception exception;
 
         private static final ParseField FEATURE_NAME = new ParseField("feature_name");
         private static final ParseField STATUS = new ParseField("status");
+        private static final ParseField EXCEPTION = new ParseField("exception");
 
-        private static final ConstructingObjectParser<ResetFeatureStateStatus, Void> PARSER =  new ConstructingObjectParser<>(
-            "features", true, (a, ctx) -> new ResetFeatureStateStatus((String) a[0], (String) a[1])
+        private static final ConstructingObjectParser<ResetFeatureStateStatus, Void> PARSER = new ConstructingObjectParser<>(
+            "features", true,
+            (a, ctx) -> new ResetFeatureStateStatus((String) a[0], (String) a[1], (ElasticsearchException) a[2])
         );
 
         static {
             PARSER.declareField(ConstructingObjectParser.constructorArg(),
                 (p, c) -> p.text(), FEATURE_NAME, ObjectParser.ValueType.STRING);
             PARSER.declareField(ConstructingObjectParser.constructorArg(),
                 (p, c) -> p.text(), STATUS, ObjectParser.ValueType.STRING);
+            PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(),
+                (p, c) -> ElasticsearchException.failureFromXContent(p), EXCEPTION);
         }
 
-        ResetFeatureStateStatus(String featureName, String status) {
+        ResetFeatureStateStatus(String featureName, String status, Exception exception) {
             this.featureName = featureName;
             this.status = status;
+            this.exception = exception;
         }
 
         public static ResetFeatureStateStatus parse(XContentParser parser, Void ctx) {
@@ -78,5 +85,9 @@ public String getFeatureName() {
         public String getStatus() {
             return status;
         }
+
+        public Exception getException() {
+            return exception;
+        }
     }
 }

client/rest-high-level/src/test/java/org/elasticsearch/client/FeaturesIT.java

@@ -12,9 +12,15 @@
 import org.elasticsearch.client.feature.GetFeaturesResponse;
 import org.elasticsearch.client.feature.ResetFeaturesRequest;
 import org.elasticsearch.client.feature.ResetFeaturesResponse;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.search.SearchModule;
 
 import java.io.IOException;
+import java.util.Collections;
+import java.util.Set;
+import java.util.stream.Collectors;
 
+import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.notNullValue;
 
@@ -34,13 +40,25 @@ public void testGetFeatures() throws IOException {
     public void testResetFeatures() throws IOException {
         ResetFeaturesRequest request = new ResetFeaturesRequest();
 
+        // need superuser privileges to execute the reset
+        RestHighLevelClient adminHighLevelClient = new RestHighLevelClient(
+            adminClient(),
+            (client) -> {},
+            new SearchModule(Settings.EMPTY, Collections.emptyList()).getNamedXContents());
         ResetFeaturesResponse response = execute(request,
-            highLevelClient().features()::resetFeatures, highLevelClient().features()::resetFeaturesAsync);
+            adminHighLevelClient.features()::resetFeatures,
+            adminHighLevelClient.features()::resetFeaturesAsync);
 
         assertThat(response, notNullValue());
         assertThat(response.getFeatures(), notNullValue());
         assertThat(response.getFeatures().size(), greaterThan(1));
         assertTrue(response.getFeatures().stream().anyMatch(
             feature -> "tasks".equals(feature.getFeatureName()) && "SUCCESS".equals(feature.getStatus())));
+
+        Set<String> statuses = response.getFeatures().stream()
+            .map(ResetFeaturesResponse.ResetFeatureStateStatus::getStatus)
+            .collect(Collectors.toSet());
+
+        assertThat(statuses, contains("SUCCESS"));
     }
 }

client/rest-high-level/src/test/java/org/elasticsearch/client/snapshots/ResetFeaturesResponseTests.java

@@ -0,0 +1,64 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.client.snapshots;
+
+import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse;
+import org.elasticsearch.client.AbstractResponseTestCase;
+import org.elasticsearch.client.feature.ResetFeaturesResponse;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.XContentType;
+
+import java.io.IOException;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import static org.hamcrest.Matchers.everyItem;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.in;
+import static org.hamcrest.Matchers.is;
+
+public class ResetFeaturesResponseTests extends AbstractResponseTestCase<ResetFeatureStateResponse, ResetFeaturesResponse> {
+
+    @Override
+    protected ResetFeatureStateResponse createServerTestInstance(
+        XContentType xContentType) {
+        return new org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse(
+            randomList(
+                10,
+                () -> randomBoolean()
+                    ? ResetFeatureStateResponse.ResetFeatureStateStatus.success(randomAlphaOfLengthBetween(6, 10))
+                    : ResetFeatureStateResponse.ResetFeatureStateStatus.failure(
+                        randomAlphaOfLengthBetween(6, 10), "something went wrong")
+            )
+        );
+    }
+
+    @Override
+    protected ResetFeaturesResponse doParseToClientInstance(XContentParser parser) throws IOException {
+        return ResetFeaturesResponse.parse(parser);
+    }
+
+    @Override
+    protected void assertInstances(ResetFeatureStateResponse serverTestInstance, ResetFeaturesResponse clientInstance) {
+
+        assertNotNull(serverTestInstance.getFeatureStateResetStatusList());
+        assertNotNull(clientInstance.getFeatures());
+
+        assertThat(clientInstance.getFeatures(), hasSize(serverTestInstance.getFeatureStateResetStatusList().size()));
+
+        Map<String, String> clientFeatures = clientInstance.getFeatures()
+            .stream()
+            .collect(Collectors.toMap(f -> f.getFeatureName(), f -> f.getStatus()));
+        Map<String, String> serverFeatures = serverTestInstance.getFeatureStateResetStatusList()
+            .stream()
+            .collect(Collectors.toMap(f -> f.getFeatureName(), f -> f.getStatus().toString()));
+
+        assertThat(clientFeatures.entrySet(), everyItem(is(in(serverFeatures.entrySet()))));
+    }
+}

docs/reference/features/apis/reset-features-api.asciidoc

@@ -8,7 +8,7 @@ experimental::[]
 
 Clears all of the the state information stored in system indices by {es} features, including the security and machine learning indices.
 
-WARNING: Intended for development and testing use only. Do not reset features on a production cluster.  
+WARNING: Intended for development and testing use only. Do not reset features on a production cluster.
 
 [source,console]
 -----------------------------------
@@ -26,9 +26,11 @@ POST /_features/_reset
 
 Return a cluster to the same state as a new installation by resetting the feature state for all {es} features. This deletes all state information stored in system indices.
 
-Note that select features might provide a way to reset particular system indices. Using this API resets _all_ features, both those that are built-in and implemented as plugins. 
+The response code is `HTTP 200` if state is successfully reset for all features, `HTTP 207` if there is a mixture of successes and failures, and `HTTP 500` if the reset operation fails for all features.
 
-To list the features that will be affected, use the <<get-features-api,get features API>>. 
+Note that select features might provide a way to reset particular system indices. Using this API resets _all_ features, both those that are built-in and implemented as plugins.
+
+To list the features that will be affected, use the <<get-features-api,get features API>>.
 
 IMPORTANT: The features installed on the node you submit this request to are the features that will be reset. Run on the master node if you have any doubts about which plugins are installed on individual nodes.
 

server/src/internalClusterTest/java/org/elasticsearch/snapshots/FeatureStateResetApiIT.java

@@ -8,10 +8,13 @@
 
 package org.elasticsearch.snapshots;
 
+import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateAction;
 import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateRequest;
 import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse;
 import org.elasticsearch.action.admin.indices.get.GetIndexResponse;
+import org.elasticsearch.client.Client;
+import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.indices.SystemIndexDescriptor;
@@ -23,8 +26,10 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import static org.hamcrest.Matchers.arrayContaining;
+import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 
@@ -35,6 +40,7 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
         List<Class<? extends Plugin>> plugins = new ArrayList<>(super.nodePlugins());
         plugins.add(SystemIndexTestPlugin.class);
         plugins.add(SecondSystemIndexTestPlugin.class);
+        plugins.add(EvilSystemIndexTestPlugin.class);
         return plugins;
     }
 
@@ -62,10 +68,11 @@ public void testResetSystemIndices() throws Exception {
 
         // call the reset API
         ResetFeatureStateResponse apiResponse = client().execute(ResetFeatureStateAction.INSTANCE, new ResetFeatureStateRequest()).get();
-        assertThat(apiResponse.getItemList(), containsInAnyOrder(
-            new ResetFeatureStateResponse.ResetFeatureStateStatus("SystemIndexTestPlugin", "SUCCESS"),
-            new ResetFeatureStateResponse.ResetFeatureStateStatus("SecondSystemIndexTestPlugin", "SUCCESS"),
-            new ResetFeatureStateResponse.ResetFeatureStateStatus("tasks", "SUCCESS")
+        assertThat(apiResponse.getFeatureStateResetStatusList(), containsInAnyOrder(
+            ResetFeatureStateResponse.ResetFeatureStateStatus.success("SystemIndexTestPlugin"),
+            ResetFeatureStateResponse.ResetFeatureStateStatus.success("SecondSystemIndexTestPlugin"),
+            ResetFeatureStateResponse.ResetFeatureStateStatus.success("EvilSystemIndexTestPlugin"),
+            ResetFeatureStateResponse.ResetFeatureStateStatus.success("tasks")
         ));
 
         // verify that both indices are gone
@@ -94,6 +101,25 @@ public void testResetSystemIndices() throws Exception {
         assertThat(response.getIndices(), arrayContaining("my_index"));
     }
 
+    /**
+     * Evil test - what if the cleanup method fails?
+     */
+    public void testFailFastOnStateCleanupFailure() throws Exception {
+        try {
+            EvilSystemIndexTestPlugin.beEvil = true;
+            ResetFeatureStateResponse resetFeatureStateResponse = client().execute(ResetFeatureStateAction.INSTANCE,
+                new ResetFeatureStateRequest()).get();
+
+            List<String> failedFeatures = resetFeatureStateResponse.getFeatureStateResetStatusList().stream()
+                .filter(status -> status.getStatus() == ResetFeatureStateResponse.ResetFeatureStateStatus.Status.FAILURE)
+                .map(ResetFeatureStateResponse.ResetFeatureStateStatus::getFeatureName)
+                .collect(Collectors.toList());
+            assertThat(failedFeatures, contains("EvilSystemIndexTestPlugin"));
+        } finally {
+            EvilSystemIndexTestPlugin.beEvil = false;
+        }
+    }
+
     /**
      * A test plugin with patterns for system indices and associated indices.
      */
@@ -145,4 +171,34 @@ public String getFeatureDescription() {
             return "A second test plugin";
         }
     }
+
+    /**
+     * An evil test plugin to test failure cases.
+     */
+    public static class EvilSystemIndexTestPlugin extends Plugin implements SystemIndexPlugin {
+
+        private static boolean beEvil = false;
+
+        @Override
+        public String getFeatureName() {
+            return "EvilSystemIndexTestPlugin";
+        }
+
+        @Override
+        public String getFeatureDescription() {
+            return "a plugin that can be very bad";
+        }
+
+        @Override
+        public void cleanUpFeature(
+            ClusterService clusterService,
+            Client client,
+            ActionListener<ResetFeatureStateResponse.ResetFeatureStateStatus> listener) {
+            if (beEvil == false) {
+                listener.onResponse(ResetFeatureStateResponse.ResetFeatureStateStatus.success(getFeatureName()));
+            } else {
+                listener.onResponse(ResetFeatureStateResponse.ResetFeatureStateStatus.failure(getFeatureName(), "problem!"));
+            }
+        }
+    }
 }