Configure Autoscaling deciders using settings (#64428) (#65390)

Autoscaling deciders are now configured using settings, allowing an
easy way to add default values as well as building upon the existing
setting code. This makes it easier to add new deciders since there
is no need for a separate configuration object.

Author: henningandersen

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

@@ -62,6 +62,24 @@
   - do:
       autoscaling.delete_autoscaling_policy:
         name: my_autoscaling_policy
+
+---
+"Test put autoscaling policy with non-existent roles":
+  - do:
+      catch: bad_request
+      autoscaling.put_autoscaling_policy:
+        name: my_autoscaling_policy
+        body:
+          roles: [ non-existing ]
+
+---
+"Test add autoscaling policy with no roles":
+  - do:
+      catch: bad_request
+      autoscaling.put_autoscaling_policy:
+        name: my_autoscaling_policy
+        body: {}
+
 ---
 "Test put autoscaling policy with non-existent decider":
   - do:
@@ -74,18 +92,23 @@
             does_not_exist: {}
 
 ---
-"Test put autoscaling policy with non-existent roles":
+"Test put autoscaling policy with non-existent decider setting":
   - do:
       catch: bad_request
       autoscaling.put_autoscaling_policy:
         name: my_autoscaling_policy
         body:
-          roles: [ non-existing ]
+          deciders:
+            fixed:
+              does_not_exist: 0
 
 ---
-"Test add autoscaling policy with no roles":
+"Test put autoscaling policy with bad setting value":
   - do:
       catch: bad_request
       autoscaling.put_autoscaling_policy:
         name: my_autoscaling_policy
-        body: {}
+        body:
+          policy:
+            fixed:
+              storage: bad

x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/Autoscaling.java

@@ -44,10 +44,8 @@
 import org.elasticsearch.xpack.autoscaling.action.TransportGetAutoscalingPolicyAction;
 import org.elasticsearch.xpack.autoscaling.action.TransportPutAutoscalingPolicyAction;
 import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingCalculateCapacityService;
-import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingDeciderConfiguration;
 import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingDeciderResult;
 import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingDeciderService;
-import org.elasticsearch.xpack.autoscaling.capacity.FixedAutoscalingDeciderConfiguration;
 import org.elasticsearch.xpack.autoscaling.capacity.FixedAutoscalingDeciderService;
 import org.elasticsearch.xpack.autoscaling.rest.RestDeleteAutoscalingPolicyHandler;
 import org.elasticsearch.xpack.autoscaling.rest.RestGetAutoscalingCapacityHandler;
@@ -178,14 +176,9 @@ public List<NamedWriteableRegistry.Entry> getNamedWriteables() {
         return org.elasticsearch.common.collect.List.of(
             new NamedWriteableRegistry.Entry(Metadata.Custom.class, AutoscalingMetadata.NAME, AutoscalingMetadata::new),
             new NamedWriteableRegistry.Entry(NamedDiff.class, AutoscalingMetadata.NAME, AutoscalingMetadata.AutoscalingMetadataDiff::new),
-            new NamedWriteableRegistry.Entry(
-                AutoscalingDeciderConfiguration.class,
-                FixedAutoscalingDeciderConfiguration.NAME,
-                FixedAutoscalingDeciderConfiguration::new
-            ),
             new NamedWriteableRegistry.Entry(
                 AutoscalingDeciderResult.Reason.class,
-                FixedAutoscalingDeciderConfiguration.NAME,
+                FixedAutoscalingDeciderService.NAME,
                 FixedAutoscalingDeciderService.FixedReason::new
             )
         );
@@ -194,12 +187,7 @@ public List<NamedWriteableRegistry.Entry> getNamedWriteables() {
     @Override
     public List<NamedXContentRegistry.Entry> getNamedXContent() {
         return org.elasticsearch.common.collect.List.of(
-            new NamedXContentRegistry.Entry(Metadata.Custom.class, new ParseField(AutoscalingMetadata.NAME), AutoscalingMetadata::parse),
-            new NamedXContentRegistry.Entry(
-                AutoscalingDeciderConfiguration.class,
-                new ParseField(FixedAutoscalingDeciderConfiguration.NAME),
-                FixedAutoscalingDeciderConfiguration::parse
-            )
+            new NamedXContentRegistry.Entry(Metadata.Custom.class, new ParseField(AutoscalingMetadata.NAME), AutoscalingMetadata::parse)
         );
     }
 
@@ -213,11 +201,11 @@ public void loadExtensions(ExtensionLoader loader) {
     }
 
     @Override
-    public Collection<AutoscalingDeciderService<? extends AutoscalingDeciderConfiguration>> deciders() {
+    public Collection<AutoscalingDeciderService> deciders() {
         return org.elasticsearch.common.collect.List.of(new FixedAutoscalingDeciderService());
     }
 
-    public Set<AutoscalingDeciderService<? extends AutoscalingDeciderConfiguration>> createDeciderServices() {
+    public Set<AutoscalingDeciderService> createDeciderServices() {
         return autoscalingExtensions.stream().flatMap(p -> p.deciders().stream()).collect(Collectors.toSet());
     }
 }

x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/AutoscalingExtension.java

@@ -6,7 +6,6 @@
 
 package org.elasticsearch.xpack.autoscaling;
 
-import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingDeciderConfiguration;
 import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingDeciderService;
 
 import java.util.Collection;
@@ -16,5 +15,5 @@ public interface AutoscalingExtension {
      * Get the list of decider services for this plugin. This is called after createComponents has been called.
      * @return list of decider services
      */
-    Collection<AutoscalingDeciderService<? extends AutoscalingDeciderConfiguration>> deciders();
+    Collection<AutoscalingDeciderService> deciders();
 }

x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/action/PolicyValidator.java

@@ -0,0 +1,20 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+package org.elasticsearch.xpack.autoscaling.action;
+
+import org.elasticsearch.xpack.autoscaling.policy.AutoscalingPolicy;
+
+/**
+ * Validator to check autoscaling policies
+ */
+public interface PolicyValidator {
+    /**
+     * Validate the given policy, e.g., check decider names, setting names and values.
+     * @param policy the policy to validate
+     */
+    void validate(AutoscalingPolicy policy);
+}

x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/action/PutAutoscalingPolicyAction.java

@@ -15,14 +15,14 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingDeciderConfiguration;
 import org.elasticsearch.xpack.autoscaling.policy.AutoscalingPolicy;
 
 import java.io.IOException;
 import java.util.AbstractMap;
-import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -31,7 +31,6 @@
 import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.TreeSet;
-import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
@@ -54,9 +53,9 @@ public static class Request extends AcknowledgedRequest<Request> {
                 @SuppressWarnings("unchecked")
                 final List<String> roles = (List<String>) c[0];
                 @SuppressWarnings("unchecked")
-                List<Map.Entry<String, AutoscalingDeciderConfiguration>> deciders =
+                List<Map.Entry<String, Settings>> deciders =
                     // help spotless format this
-                    (List<Map.Entry<String, AutoscalingDeciderConfiguration>>) c[1];
+                    (List<Map.Entry<String, Settings>>) c[1];
                 return new Request(
                     name,
                     roles != null ? Collections.unmodifiableSortedSet(new TreeSet<>(roles)) : null,
@@ -66,47 +65,43 @@ public static class Request extends AcknowledgedRequest<Request> {
                 );
             });
             PARSER.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), AutoscalingPolicy.ROLES_FIELD);
-            PARSER.declareNamedObjects(
-                ConstructingObjectParser.optionalConstructorArg(),
-                (p, c, n) -> new AbstractMap.SimpleEntry<>(n, p.namedObject(AutoscalingDeciderConfiguration.class, n, null)),
-                AutoscalingPolicy.DECIDERS_FIELD
-            );
+            PARSER.declareNamedObjects(ConstructingObjectParser.optionalConstructorArg(), (p, c, n) -> {
+                p.nextToken();
+                return new AbstractMap.SimpleEntry<>(n, Settings.fromXContent(p));
+            }, AutoscalingPolicy.DECIDERS_FIELD);
         }
 
         private final String name;
         private final SortedSet<String> roles;
-        private final SortedMap<String, AutoscalingDeciderConfiguration> deciders;
+        private final SortedMap<String, Settings> deciders;
 
         public static Request parse(final XContentParser parser, final String name) {
             return PARSER.apply(parser, name);
         }
 
-        public Request(
-            final String name,
-            final SortedSet<String> roles,
-            final SortedMap<String, AutoscalingDeciderConfiguration> deciders
-        ) {
+        public Request(final String name, final SortedSet<String> roles, final SortedMap<String, Settings> deciders) {
             this.name = name;
             this.roles = roles;
             this.deciders = deciders;
         }
 
         public Request(final StreamInput in) throws IOException {
             super(in);
-            name = in.readString();
+            this.name = in.readString();
             if (in.readBoolean()) {
-                roles = Collections.unmodifiableSortedSet(new TreeSet<>(in.readSet(StreamInput::readString)));
+                this.roles = Collections.unmodifiableSortedSet(new TreeSet<>(in.readSet(StreamInput::readString)));
             } else {
-                roles = null;
+                this.roles = null;
             }
             if (in.readBoolean()) {
-                deciders = new TreeMap<>(
-                    in.readNamedWriteableList(AutoscalingDeciderConfiguration.class)
-                        .stream()
-                        .collect(Collectors.toMap(AutoscalingDeciderConfiguration::name, Function.identity()))
-                );
+                int deciderCount = in.readInt();
+                SortedMap<String, Settings> deciders = new TreeMap<>();
+                for (int i = 0; i < deciderCount; ++i) {
+                    deciders.put(in.readString(), Settings.readSettingsFromStream(in));
+                }
+                this.deciders = Collections.unmodifiableSortedMap(deciders);
             } else {
-                deciders = null;
+                this.deciders = null;
             }
         }
 
@@ -122,7 +117,11 @@ public void writeTo(final StreamOutput out) throws IOException {
             }
             if (deciders != null) {
                 out.writeBoolean(true);
-                out.writeNamedWriteableList(Collections.unmodifiableList(new ArrayList<>(deciders.values())));
+                out.writeInt(deciders.size());
+                for (Map.Entry<String, Settings> entry : deciders.entrySet()) {
+                    out.writeString(entry.getKey());
+                    Settings.writeSettingsToStream(entry.getValue(), out);
+                }
             } else {
                 out.writeBoolean(false);
             }
@@ -136,7 +135,7 @@ public SortedSet<String> roles() {
             return roles;
         }
 
-        public SortedMap<String, AutoscalingDeciderConfiguration> deciders() {
+        public SortedMap<String, Settings> deciders() {
             return deciders;
         }
 

x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/action/TransportPutAutoscalingPolicyAction.java

@@ -23,6 +23,7 @@
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 import org.elasticsearch.xpack.autoscaling.AutoscalingMetadata;
+import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingCalculateCapacityService;
 import org.elasticsearch.xpack.autoscaling.policy.AutoscalingPolicy;
 import org.elasticsearch.xpack.autoscaling.policy.AutoscalingPolicyMetadata;
 
@@ -34,13 +35,27 @@ public class TransportPutAutoscalingPolicyAction extends AcknowledgedTransportMa
 
     private static final Logger logger = LogManager.getLogger(TransportPutAutoscalingPolicyAction.class);
 
+    private final PolicyValidator policyValidator;
+
     @Inject
     public TransportPutAutoscalingPolicyAction(
         final TransportService transportService,
         final ClusterService clusterService,
         final ThreadPool threadPool,
         final ActionFilters actionFilters,
-        final IndexNameExpressionResolver indexNameExpressionResolver
+        final IndexNameExpressionResolver indexNameExpressionResolver,
+        final AutoscalingCalculateCapacityService.Holder policyValidatorHolder
+    ) {
+        this(transportService, clusterService, threadPool, actionFilters, indexNameExpressionResolver, policyValidatorHolder.get());
+    }
+
+    TransportPutAutoscalingPolicyAction(
+        final TransportService transportService,
+        final ClusterService clusterService,
+        final ThreadPool threadPool,
+        final ActionFilters actionFilters,
+        final IndexNameExpressionResolver indexNameExpressionResolver,
+        final PolicyValidator policyValidator
     ) {
         super(
             PutAutoscalingPolicyAction.NAME,
@@ -52,6 +67,7 @@ public TransportPutAutoscalingPolicyAction(
             indexNameExpressionResolver,
             ThreadPool.Names.SAME
         );
+        this.policyValidator = policyValidator;
     }
 
     @Override
@@ -63,7 +79,7 @@ protected void masterOperation(
         clusterService.submitStateUpdateTask("put-autoscaling-policy", new AckedClusterStateUpdateTask(request, listener) {
             @Override
             public ClusterState execute(final ClusterState currentState) {
-                return putAutoscalingPolicy(currentState, request, logger);
+                return putAutoscalingPolicy(currentState, request, policyValidator, logger);
             }
         });
     }
@@ -76,6 +92,7 @@ protected ClusterBlockException checkBlock(final PutAutoscalingPolicyAction.Requ
     static ClusterState putAutoscalingPolicy(
         final ClusterState currentState,
         final PutAutoscalingPolicyAction.Request request,
+        final PolicyValidator policyValidator,
         final Logger logger
     ) {
         // we allow putting policies with roles that not all nodes in the cluster may understand currently (but the current master must
@@ -110,6 +127,8 @@ static ClusterState putAutoscalingPolicy(
             );
         }
 
+        policyValidator.validate(updatedPolicy);
+
         final SortedMap<String, AutoscalingPolicyMetadata> newPolicies = new TreeMap<>(currentMetadata.policies());
         final AutoscalingPolicyMetadata newPolicyMetadata = new AutoscalingPolicyMetadata(updatedPolicy);
         final AutoscalingPolicyMetadata oldPolicyMetadata = newPolicies.put(request.name(), newPolicyMetadata);