Browse Source

Reject autoscaling policies with no deciders (#67284)

A policy that resolves to no deciders after defaulting can never deliver
any autoscaling capacities. To help operators not instantiate such
policies, this commit adds validation that the resulting policy must
resolve to at least one decider.
Henning Andersen 4 years ago
parent
commit
a794743d43

+ 4 - 4
x-pack/plugin/autoscaling/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/autoscaling/delete_autoscaling_policy.yml

@@ -44,13 +44,13 @@
       autoscaling.put_autoscaling_policy:
         name: my_autoscaling_policy_1
         body:
-          roles: []
+          roles: [ "data" ]
 
   - do:
       autoscaling.put_autoscaling_policy:
         name: my_autoscaling_policy_2
         body:
-          roles: []
+          roles: [ "data" ]
 
   - do:
       autoscaling.delete_autoscaling_policy:
@@ -72,13 +72,13 @@
       autoscaling.put_autoscaling_policy:
         name: my_autoscaling_policy_delete
         body:
-          roles: []
+          roles: [ "data" ]
 
   - do:
       autoscaling.put_autoscaling_policy:
         name: my_autoscaling_policy_keep
         body:
-          roles: []
+          roles: [ "data" ]
 
   - do:
       autoscaling.delete_autoscaling_policy:

+ 11 - 2
x-pack/plugin/autoscaling/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/autoscaling/put_autoscaling_policy.yml

@@ -4,7 +4,7 @@
       autoscaling.put_autoscaling_policy:
         name: my_autoscaling_policy
         body:
-          roles: [ master ]
+          roles: [ data_content ]
 
   - match: { "acknowledged": true }
 
@@ -19,7 +19,7 @@
   - do:
       autoscaling.get_autoscaling_policy:
         name: my_autoscaling_policy
-  - match: { roles: [ master ] }
+  - match: { roles: [ data_content ] }
   - match: { deciders.fixed: {} }
 
   # update roles
@@ -112,3 +112,12 @@
           policy:
             fixed:
               storage: bad
+
+---
+"Test put autoscaling policy with no default deciders":
+  - do:
+      catch: bad_request
+      autoscaling.put_autoscaling_policy:
+        name: my_autoscaling_policy
+        body:
+          roles: [ ]

+ 18 - 2
x-pack/plugin/autoscaling/src/internalClusterTest/java/org/elasticsearch/xpack/autoscaling/action/TransportPutAutoscalingPolicyActionIT.java

@@ -11,9 +11,12 @@ import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.xpack.autoscaling.AutoscalingIntegTestCase;
 import org.elasticsearch.xpack.autoscaling.AutoscalingMetadata;
-import org.elasticsearch.xpack.autoscaling.AutoscalingTestCase;
 import org.elasticsearch.xpack.autoscaling.policy.AutoscalingPolicy;
 
+import java.util.List;
+import java.util.TreeMap;
+import java.util.TreeSet;
+
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.xpack.autoscaling.AutoscalingTestCase.mutateAutoscalingDeciders;
 import static org.elasticsearch.xpack.autoscaling.AutoscalingTestCase.randomAutoscalingDeciders;
@@ -39,7 +42,7 @@ public class TransportPutAutoscalingPolicyActionIT extends AutoscalingIntegTestC
         final AutoscalingPolicy policy = putRandomAutoscalingPolicy();
         final AutoscalingPolicy updatedPolicy = new AutoscalingPolicy(
             policy.name(),
-            AutoscalingTestCase.randomRoles(),
+            new TreeSet<>(randomSubsetOf(List.of("data", "data_content", "data_hot", "data_warm", "data_cold"))),
             mutateAutoscalingDeciders(policy.deciders())
         );
         putAutoscalingPolicy(updatedPolicy);
@@ -73,6 +76,19 @@ public class TransportPutAutoscalingPolicyActionIT extends AutoscalingIntegTestC
         );
     }
 
+    public void testPutNoDeciderPolicy() {
+        String policyName = randomAlphaOfLength(8);
+        IllegalArgumentException exception = expectThrows(
+            IllegalArgumentException.class,
+            () -> putAutoscalingPolicy(new AutoscalingPolicy(policyName, new TreeSet<>(), new TreeMap<>()))
+        );
+
+        assertThat(
+            exception.getMessage(),
+            containsString("no default nor user configured deciders for policy [" + policyName + "] with roles [[]]")
+        );
+    }
+
     private AutoscalingPolicy putRandomAutoscalingPolicy() {
         final AutoscalingPolicy policy = randomAutoscalingPolicy();
         putAutoscalingPolicy(policy);

+ 6 - 0
x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCalculateCapacityService.java

@@ -50,6 +50,12 @@ public class AutoscalingCalculateCapacityService implements PolicyValidator {
 
     public void validate(AutoscalingPolicy policy) {
         policy.deciders().forEach((name, configuration) -> validate(name, configuration, policy.roles()));
+        SortedMap<String, Settings> deciders = addDefaultDeciders(policy);
+        if (deciders.isEmpty()) {
+            throw new IllegalArgumentException(
+                "no default nor user configured deciders for policy [" + policy.name() + "] with roles [" + policy.roles() + "]"
+            );
+        }
     }
 
     private void validate(final String deciderName, final Settings configuration, SortedSet<String> roles) {

+ 15 - 0
x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCalculateCapacityServiceTests.java

@@ -335,6 +335,21 @@ public class AutoscalingCalculateCapacityServiceTests extends AutoscalingTestCas
         );
     }
 
+    public void testValidateNotEmptyDeciders() {
+        AutoscalingCalculateCapacityService service = new AutoscalingCalculateCapacityService(Set.of(new FixedAutoscalingDeciderService()));
+        String policyName = randomAlphaOfLength(8);
+        AutoscalingPolicy policy = new AutoscalingPolicy(
+            policyName,
+            new TreeSet<>(randomBoolean() ? Set.of() : Set.of("master")),
+            new TreeMap<>()
+        );
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> service.validate(policy));
+        assertThat(
+            exception.getMessage(),
+            equalTo("no default nor user configured deciders for policy [" + policyName + "] with roles [" + policy.roles() + "]")
+        );
+    }
+
     public void testValidateSettingName() {
         AutoscalingCalculateCapacityService service = new AutoscalingCalculateCapacityService(Set.of(new FixedAutoscalingDeciderService()));
         Set<String> legalNames = Set.of(