From 473468d7630e80a7ccb6d2974f4e65fbd2f5294f Mon Sep 17 00:00:00 2001
From: David Roberts <dave.roberts@elastic.co>
Date: Tue, 11 Feb 2020 15:22:21 +0000
Subject: [PATCH] [ML] Better error when persistent task assignment disabled
 (#52014)

Changes the misleading error message when attempting to open
a job while the "cluster.persistent_tasks.allocation.enable"
setting is set to "none" to a clearer message that names the
setting.

Closes #51956
---
 .../decider/EnableAssignmentDecider.java      |  3 +-
 .../xpack/ml/integration/MlJobIT.java         | 36 +++++++++++++++++++
 .../ml/action/TransportOpenJobAction.java     | 16 ++++++++-
 .../rest-api-spec/test/ml/jobs_crud.yml       |  2 +-
 4 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/persistent/decider/EnableAssignmentDecider.java b/server/src/main/java/org/elasticsearch/persistent/decider/EnableAssignmentDecider.java
index 525e1379a40..a33f3628743 100644
--- a/server/src/main/java/org/elasticsearch/persistent/decider/EnableAssignmentDecider.java
+++ b/server/src/main/java/org/elasticsearch/persistent/decider/EnableAssignmentDecider.java
@@ -43,6 +43,7 @@ public class EnableAssignmentDecider {
 
     public static final Setting<Allocation> CLUSTER_TASKS_ALLOCATION_ENABLE_SETTING =
         new Setting<>("cluster.persistent_tasks.allocation.enable", Allocation.ALL.toString(), Allocation::fromString, Dynamic, NodeScope);
+    public static final String ALLOCATION_NONE_EXPLANATION = "no persistent task assignments are allowed due to cluster settings";
 
     private volatile Allocation enableAssignment;
 
@@ -64,7 +65,7 @@ public class EnableAssignmentDecider {
      */
     public AssignmentDecision canAssign() {
         if (enableAssignment == Allocation.NONE) {
-            return new AssignmentDecision(AssignmentDecision.Type.NO, "no persistent task assignments are allowed due to cluster settings");
+            return new AssignmentDecision(AssignmentDecision.Type.NO, ALLOCATION_NONE_EXPLANATION);
         }
         return AssignmentDecision.YES;
     }
diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlJobIT.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlJobIT.java
index cfd760df926..68669b0dbc8 100644
--- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlJobIT.java
+++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlJobIT.java
@@ -398,6 +398,42 @@ public class MlJobIT extends ESRestTestCase {
                         "avoid the clash by assigning a dedicated results index"));
     }
 
+    public void testOpenJobFailsWhenPersistentTaskAssignmentDisabled() throws Exception {
+        String jobId = "open-job-with-persistent-task-assignment-disabled";
+        createFarequoteJob(jobId);
+
+        Request disablePersistentTaskAssignmentRequest = new Request("PUT", "_cluster/settings");
+        disablePersistentTaskAssignmentRequest.setJsonEntity("{\n" +
+            "  \"transient\": {\n" +
+            "    \"cluster.persistent_tasks.allocation.enable\": \"none\"\n" +
+            "  }\n" +
+            "}");
+        Response disablePersistentTaskAssignmentResponse = client().performRequest(disablePersistentTaskAssignmentRequest);
+        assertThat(entityAsMap(disablePersistentTaskAssignmentResponse), hasEntry("acknowledged", true));
+
+        try {
+            ResponseException exception = expectThrows(
+                ResponseException.class,
+                () -> client().performRequest(
+                new Request("POST", MachineLearning.BASE_PATH + "anomaly_detectors/" + jobId + "/_open")));
+            assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(429));
+            assertThat(EntityUtils.toString(exception.getResponse().getEntity()),
+                containsString("Cannot open jobs because persistent task assignment is disabled by the " +
+                    "[cluster.persistent_tasks.allocation.enable] setting"));
+        } finally {
+            // Try to revert the cluster setting change even if the test fails,
+            // because otherwise this setting will cause many other tests to fail
+            Request enablePersistentTaskAssignmentRequest = new Request("PUT", "_cluster/settings");
+            enablePersistentTaskAssignmentRequest.setJsonEntity("{\n" +
+                "  \"transient\": {\n" +
+                "    \"cluster.persistent_tasks.allocation.enable\": \"all\"\n" +
+                "  }\n" +
+                "}");
+            Response enablePersistentTaskAssignmentResponse = client().performRequest(disablePersistentTaskAssignmentRequest);
+            assertThat(entityAsMap(enablePersistentTaskAssignmentResponse), hasEntry("acknowledged", true));
+        }
+    }
+
     public void testDeleteJob() throws Exception {
         String jobId = "delete-job-job";
         String indexName = AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT;
diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java
index 4985a776681..16bc970d4a5 100644
--- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java
+++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java
@@ -36,6 +36,7 @@ import org.elasticsearch.persistent.PersistentTaskState;
 import org.elasticsearch.persistent.PersistentTasksCustomMetaData;
 import org.elasticsearch.persistent.PersistentTasksExecutor;
 import org.elasticsearch.persistent.PersistentTasksService;
+import org.elasticsearch.persistent.decider.EnableAssignmentDecider;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.tasks.TaskId;
 import org.elasticsearch.threadpool.ThreadPool;
@@ -558,7 +559,13 @@ public class TransportOpenJobAction extends TransportMasterNodeAction<OpenJobAct
                         assignment.isAssigned() == false) {
                     OpenJobAction.JobParams params = (OpenJobAction.JobParams) persistentTask.getParams();
                     // Assignment has failed on the master node despite passing our "fast fail" validation
-                    exception = makeNoSuitableNodesException(logger, params.getJobId(), assignment.getExplanation());
+                    if (assignment.equals(AWAITING_UPGRADE)) {
+                        exception = makeCurrentlyBeingUpgradedException(logger, params.getJobId(), assignment.getExplanation());
+                    } else if (assignment.getExplanation().contains("[" + EnableAssignmentDecider.ALLOCATION_NONE_EXPLANATION + "]")) {
+                        exception = makeAssignmentsNotAllowedException(logger, params.getJobId());
+                    } else {
+                        exception = makeNoSuitableNodesException(logger, params.getJobId(), assignment.getExplanation());
+                    }
                     // The persistent task should be cancelled so that the observed outcome is the
                     // same as if the "fast fail" validation on the coordinating node had failed
                     shouldCancel = true;
@@ -598,6 +605,13 @@ public class TransportOpenJobAction extends TransportMasterNodeAction<OpenJobAct
             RestStatus.TOO_MANY_REQUESTS, detail);
     }
 
+    static ElasticsearchException makeAssignmentsNotAllowedException(Logger logger, String jobId) {
+        String msg = "Cannot open jobs because persistent task assignment is disabled by the ["
+            + EnableAssignmentDecider.CLUSTER_TASKS_ALLOCATION_ENABLE_SETTING.getKey() + "] setting";
+        logger.warn("[{}] {}", jobId, msg);
+        return new ElasticsearchStatusException(msg, RestStatus.TOO_MANY_REQUESTS);
+    }
+
     static ElasticsearchException makeCurrentlyBeingUpgradedException(Logger logger, String jobId, String explanation) {
         String msg = "Cannot open jobs when upgrade mode is enabled";
         logger.warn("[{}] {}", jobId, msg);
diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml
index 9f32ff22ded..ac2dd9557e5 100644
--- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml
+++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml
@@ -1425,7 +1425,7 @@
   - match: { job_id: "persistent-task-allocation-allowed-test" }
 
   - do:
-      catch: /no persistent task assignments are allowed due to cluster settings/
+      catch: /Cannot open jobs because persistent task assignment is disabled by the \[cluster.persistent_tasks.allocation.enable\] setting/
       ml.open_job:
         job_id: persistent-task-allocation-allowed-test