Skip to content

Commit

Permalink
Fix autoscaling Knative properties
Browse files Browse the repository at this point in the history
This PR the following issues:
- The generated `config-autoscaler` configmap does not set the namespace which should always be `knative-serving` (see for example https://knative.dev/docs/serving/autoscaling/scale-to-zero/#enable-scale-to-zero).
- Some properties do add the annotations at Knative service metadata level, and it should add them at Knative service spec template metadata level.
- The property `quarkus.knative.global-auto-scaling.containerConcurrency` (hard limit option) wrongly uses the `config-autoscaler` configmap and it should use `config-defaults` configmap (see https://knative.dev/docs/serving/autoscaling/concurrency/#hard-limit)

Moreover, add the autoscaling Knative properties to the Knative documentation.

Fix quarkusio#23786

(cherry picked from commit 3f797dc)
  • Loading branch information
Sgitario authored and gsmet committed Mar 1, 2022
1 parent 26419a1 commit 3e3a3b7
Show file tree
Hide file tree
Showing 15 changed files with 307 additions and 42 deletions.
25 changes: 24 additions & 1 deletion docs/src/main/asciidoc/deploying-to-kubernetes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,12 @@ The generated service can be customized using the following properties:
| quarkus.knative.readiness-probe | Probe | | ( see Probe )
| quarkus.knative.sidecars | Map<String, Container> | |
| quarkus.knative.revision-name | String | |
| quarkus.knative.traffic | Traffic[] | | ( see Traffic)
| quarkus.knative.traffic | Traffic[] | | ( see Traffic )
| quarkus.knative.min-scale | int | See link:https://knative.dev/docs/serving/autoscaling/scale-bounds/#lower-bound[link] |
| quarkus.knative.max-scale | int | See link:https://knative.dev/docs/serving/autoscaling/scale-bounds/#upper-bound[link] |
| quarkus.knative.scale-to-zero-enabled | boolean | See link:https://knative.dev/docs/serving/autoscaling/scale-to-zero/#enable-scale-to-zero[link] | true
| quarkus.knative.revision-auto-scaling | AutoScalingConfig | | ( see AutoScalingConfig )
| quarkus.knative.global-auto-scaling | GlobalAutoScalingConfig | | ( see GlobalAutoScalingConfig )
|====

.Traffic
Expand All @@ -1103,6 +1108,24 @@ The generated service can be customized using the following properties:
| percent | Logn | Indicates the percent of traffic that is be routed to this revision | 100
|====

.AutoScalingConfig
|====
| Property | Type | Description | Default Value
| auto-scaler-class | String | The auto-scaler class. Possible values: `kpa` for Knative Pod Autoscaler, `hpa` for Horizontal Pod Autoscaler | kpa
| metric | String | The autoscaling metric to use. Possible values (concurency, rps, cpu) |
| target | int | This value specifies the autoscaling target |
| container-concurrency | int | The exact amount of requests allowed to the replica at a time |
| target-utilization-percentage | int | This value specifies a percentage of the target to actually be targeted by the autoscaler |
|====

.GlobalAutoScalingConfig
|====
| Property | Type | Description | Default Value
| auto-scaler-class | String | The auto-scaler class. Possible values: `kpa` for Knative Pod Autoscaler, `hpa` for Horizontal Pod Autoscaler | kpa
| container-concurrency | int | The exact amount of requests allowed to the replica at a time |
| target-utilization-percentage | int | This value specifies a percentage of the target to actually be targeted by the autoscaler |
| requests-per-second | Logn | The requests per second per replica |
|====

=== Deployment targets

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package io.quarkus.kubernetes.deployment;

import io.dekorate.kubernetes.decorator.ResourceProvidingDecorator;
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;

/**
* This class was created to workaround https://github.com/dekorateio/dekorate/issues/869.
* Once this issue is fixed, we can delete this and use the provided by Dekorate.
*/
public class AddConfigMapDecorator extends ResourceProvidingDecorator<KubernetesListBuilder> {

private static final String DEFAULT_NAMESPACE = null;

private final String name;
private final String namespace;

public AddConfigMapDecorator(String name) {
this(name, DEFAULT_NAMESPACE);
}

public AddConfigMapDecorator(String name, String namespace) {
this.name = name;
this.namespace = namespace;
}

public void visit(KubernetesListBuilder list) {
if (contains(list, "v1", "ConfigMap", name)) {
return;
}

list.addNewConfigMapItem()
.withNewMetadata()
.withName(name)
.withNamespace(namespace)
.endMetadata()
.endConfigMapItem();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package io.quarkus.kubernetes.deployment;

import java.util.Map;

import io.dekorate.kubernetes.decorator.NamedResourceDecorator;
import io.dekorate.utils.Maps;
import io.fabric8.knative.serving.v1.ServiceFluent;
import io.fabric8.kubernetes.api.model.ObjectMeta;

/**
* This class was created to workaround https://github.com/dekorateio/dekorate/issues/869.
* Once this issue is fixed, we can delete this and use the provided by Dekorate.
*/
public class ApplyAnnotationsToServiceTemplate extends NamedResourceDecorator<ServiceFluent<?>> {

private static final String SERVICE_KIND = "Service";

private final Map<String, String> annotations;

public ApplyAnnotationsToServiceTemplate(String name, String... annotations) {
this(name, Maps.from(annotations));
}

public ApplyAnnotationsToServiceTemplate(String name, Map<String, String> annotations) {
super(SERVICE_KIND, name);
this.annotations = annotations;
}

@Override
public void andThenVisit(ServiceFluent<?> service, ObjectMeta resourceMeta) {
service.editOrNewSpec().editOrNewTemplate().editOrNewMetadata()
.addToAnnotations(annotations)
.endMetadata().endTemplate().endSpec();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ public class GlobalAutoScalingConfig {
/**
* The exact amount of requests allowed to the replica at a time.
* Its default value is “0”, which means an unlimited number of requests are allowed to flow Integer>o the replica.
*
*
* @see <a href="https://knative.dev/docs/serving/autoscaling/concurrency/#hard-limit">Knative Knative: Configuring
* concurrency: Hard Limit</a>
* @return the container concurrenct or zero if its not bound.
*/
Optional<Integer> containerConcurrency;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,13 @@
import io.dekorate.knative.decorator.AddSecretVolumeToRevisionDecorator;
import io.dekorate.knative.decorator.AddSidecarToRevisionDecorator;
import io.dekorate.knative.decorator.ApplyGlobalAutoscalingClassDecorator;
import io.dekorate.knative.decorator.ApplyGlobalContainerConcurrencyDecorator;
import io.dekorate.knative.decorator.ApplyGlobalRequestsPerSecondTargetDecorator;
import io.dekorate.knative.decorator.ApplyGlobalTargetUtilizationDecorator;
import io.dekorate.knative.decorator.ApplyLocalAutoscalingClassDecorator;
import io.dekorate.knative.decorator.ApplyLocalAutoscalingMetricDecorator;
import io.dekorate.knative.decorator.ApplyLocalAutoscalingTargetDecorator;
import io.dekorate.knative.decorator.ApplyLocalContainerConcurrencyDecorator;
import io.dekorate.knative.decorator.ApplyLocalTargetUtilizationPercentageDecorator;
import io.dekorate.knative.decorator.ApplyMaxScaleDecorator;
import io.dekorate.knative.decorator.ApplyMinScaleDecorator;
import io.dekorate.knative.decorator.ApplyRevisionNameDecorator;
import io.dekorate.knative.decorator.ApplyTrafficDecorator;
import io.dekorate.kubernetes.config.EnvBuilder;
import io.dekorate.kubernetes.decorator.AddConfigMapDataDecorator;
import io.dekorate.kubernetes.decorator.AddConfigMapResourceProvidingDecorator;
import io.dekorate.kubernetes.decorator.AddEnvVarDecorator;
import io.dekorate.kubernetes.decorator.AddLabelDecorator;
import io.dekorate.kubernetes.decorator.ApplicationContainerDecorator;
Expand Down Expand Up @@ -66,7 +58,24 @@
public class KnativeProcessor {

private static final int KNATIVE_PRIORITY = DEFAULT_PRIORITY;

private static final String LATEST_REVISION = "latest";
/**
* The following properties must be set to workaround
* this Dekorate issue: https://github.com/dekorateio/dekorate/issues/869.
* Once this issue is fixed, we can get rid of these properties and use the Dekorate knative decorators.
*/
private static final String KNATIVE_CONFIG_AUTOSCALER = "config-autoscaler";
private static final String KNATIVE_CONFIG_DEFAULTS = "config-defaults";
private static final String KNATIVE_SERVING = "knative-serving";
private static final String KNATIVE_MIN_SCALE = "autoscaling.knative.dev/minScale";
private static final String KNATIVE_MAX_SCALE = "autoscaling.knative.dev/maxScale";
private static final String KNATIVE_AUTOSCALING_METRIC = "autoscaling.knative.dev/metric";
private static final String KNATIVE_AUTOSCALING_CLASS = "autoscaling.knative.dev/class";
private static final String KNATIVE_AUTOSCALING_CLASS_SUFFIX = ".autoscaling.knative.dev";
private static final String KNATIVE_UTILIZATION_PERCENTAGE = "autoscaling.knative.dev/target-utilization-percentage";
private static final String KNATIVE_AUTOSCALING_TARGET = "autoscaling.knative.dev/target";
private static final String KNATIVE_CONTAINER_CONCURRENCY = "container-concurrency";

@BuildStep
public void checkKnative(ApplicationInfoBuildItem applicationInfo, KnativeConfig config,
Expand Down Expand Up @@ -160,59 +169,94 @@ public List<DecoratorBuildItem> createDecorators(ApplicationInfoBuildItem applic
new AddLabelDecorator(name, "serving.knative.dev/visibility", "cluster-local")));
}

config.minScale.ifPresent(min -> result.add(new DecoratorBuildItem(KNATIVE, new ApplyMinScaleDecorator(name, min))));

config.maxScale.ifPresent(max -> result.add(new DecoratorBuildItem(KNATIVE, new ApplyMaxScaleDecorator(name, max))));

/**
* Once the Dekorate issue is fixed https://github.com/dekorateio/dekorate/issues/869,
* we should replace ApplyAnnotationsToServiceTemplate by ApplyMinScaleDecorator.
*/
config.minScale.map(String::valueOf).ifPresent(min -> result.add(new DecoratorBuildItem(KNATIVE,
new ApplyAnnotationsToServiceTemplate(name, KNATIVE_MIN_SCALE, min))));
/**
* Once the Dekorate issue is fixed https://github.com/dekorateio/dekorate/issues/869,
* we should replace ApplyAnnotationsToServiceTemplate by ApplyMaxScaleDecorator.
*/
config.maxScale.map(String::valueOf).ifPresent(max -> result.add(new DecoratorBuildItem(KNATIVE,
new ApplyAnnotationsToServiceTemplate(name, KNATIVE_MAX_SCALE, max))));
/**
* Once the Dekorate issue is fixed https://github.com/dekorateio/dekorate/issues/869,
* we should replace ApplyAnnotationsToServiceTemplate by ApplyLocalAutoscalingClassDecorator.
*/
config.revisionAutoScaling.autoScalerClass.map(AutoScalerClassConverter::convert)
.ifPresent(a -> result.add(new DecoratorBuildItem(KNATIVE, new ApplyLocalAutoscalingClassDecorator(name, a))));

.ifPresent(a -> result.add(new DecoratorBuildItem(KNATIVE, new ApplyAnnotationsToServiceTemplate(name,
KNATIVE_AUTOSCALING_CLASS, a.name().toLowerCase() + KNATIVE_AUTOSCALING_CLASS_SUFFIX))));
/**
* Once the Dekorate issue is fixed https://github.com/dekorateio/dekorate/issues/869,
* we should replace ApplyAnnotationsToServiceTemplate by ApplyLocalAutoscalingMetricDecorator.
*/
config.revisionAutoScaling.metric.map(AutoScalingMetricConverter::convert)
.ifPresent(m -> result.add(new DecoratorBuildItem(KNATIVE, new ApplyLocalAutoscalingMetricDecorator(name, m))));
.ifPresent(m -> result.add(new DecoratorBuildItem(KNATIVE,
new ApplyAnnotationsToServiceTemplate(name, KNATIVE_AUTOSCALING_METRIC, m.name().toLowerCase()))));

config.revisionAutoScaling.containerConcurrency
.ifPresent(
c -> result.add(new DecoratorBuildItem(KNATIVE, new ApplyLocalContainerConcurrencyDecorator(name, c))));

config.revisionAutoScaling.targetUtilizationPercentage
/**
* Once the Dekorate issue is fixed https://github.com/dekorateio/dekorate/issues/869,
* we should replace ApplyAnnotationsToServiceTemplate by ApplyLocalTargetUtilizationPercentageDecorator.
*/
config.revisionAutoScaling.targetUtilizationPercentage.map(String::valueOf)
.ifPresent(t -> result
.add(new DecoratorBuildItem(KNATIVE, new ApplyLocalTargetUtilizationPercentageDecorator(name, t))));
config.revisionAutoScaling.target
.ifPresent(t -> result.add(new DecoratorBuildItem(KNATIVE, new ApplyLocalAutoscalingTargetDecorator(name, t))));

.add(new DecoratorBuildItem(KNATIVE,
new ApplyAnnotationsToServiceTemplate(name, KNATIVE_UTILIZATION_PERCENTAGE, t))));
/**
* Once the Dekorate issue is fixed https://github.com/dekorateio/dekorate/issues/869,
* we should replace ApplyAnnotationsToServiceTemplate by ApplyLocalAutoscalingTargetDecorator.
*/
config.revisionAutoScaling.target.map(String::valueOf)
.ifPresent(t -> result.add(new DecoratorBuildItem(KNATIVE,
new ApplyAnnotationsToServiceTemplate(name, KNATIVE_AUTOSCALING_TARGET, t))));
config.globalAutoScaling.autoScalerClass
.map(AutoScalerClassConverter::convert)
.ifPresent(a -> {
result.add(
new DecoratorBuildItem(KNATIVE, new AddConfigMapResourceProvidingDecorator("config-autoscaler")));
new DecoratorBuildItem(KNATIVE,
new AddConfigMapDecorator(KNATIVE_CONFIG_AUTOSCALER, KNATIVE_SERVING)));
result.add(new DecoratorBuildItem(KNATIVE, new ApplyGlobalAutoscalingClassDecorator(a)));
});

config.globalAutoScaling.containerConcurrency
config.globalAutoScaling.containerConcurrency.map(String::valueOf)
.ifPresent(c -> {
result.add(new DecoratorBuildItem(KNATIVE, new AddConfigMapResourceProvidingDecorator("config-defaults")));
result.add(new DecoratorBuildItem(KNATIVE, new ApplyGlobalContainerConcurrencyDecorator(c)));
result.add(new DecoratorBuildItem(KNATIVE,
new AddConfigMapDecorator(KNATIVE_CONFIG_DEFAULTS, KNATIVE_SERVING)));
/**
* Once the Dekorate issue is fixed https://github.com/dekorateio/dekorate/issues/869,
* we should replace ApplyAnnotationsToServiceTemplate by ApplyGlobalContainerConcurrencyDecorator.
*/
result.add(new DecoratorBuildItem(KNATIVE,
new AddConfigMapDataDecorator(KNATIVE_CONFIG_DEFAULTS, KNATIVE_CONTAINER_CONCURRENCY, c)));
});

config.globalAutoScaling.requestsPerSecond
.ifPresent(r -> {
result.add(
new DecoratorBuildItem(KNATIVE, new AddConfigMapResourceProvidingDecorator("config-autoscaler")));
new DecoratorBuildItem(KNATIVE,
new AddConfigMapDecorator(KNATIVE_CONFIG_AUTOSCALER, KNATIVE_SERVING)));
result.add(new DecoratorBuildItem(KNATIVE, new ApplyGlobalRequestsPerSecondTargetDecorator(r)));
});

config.globalAutoScaling.targetUtilizationPercentage
.ifPresent(t -> {
result.add(
new DecoratorBuildItem(KNATIVE, new AddConfigMapResourceProvidingDecorator("config-autoscaler")));
new DecoratorBuildItem(KNATIVE,
new AddConfigMapDecorator(KNATIVE_CONFIG_AUTOSCALER, KNATIVE_SERVING)));
result.add(new DecoratorBuildItem(KNATIVE, new ApplyGlobalTargetUtilizationDecorator(t)));
});

if (!config.scaleToZeroEnabled) {
result.add(new DecoratorBuildItem(KNATIVE, new AddConfigMapResourceProvidingDecorator("config-autoscaler")));
result.add(new DecoratorBuildItem(KNATIVE, new AddConfigMapDecorator(KNATIVE_CONFIG_AUTOSCALER, KNATIVE_SERVING)));
result.add(
new DecoratorBuildItem(KNATIVE, new AddConfigMapDataDecorator("config-autoscaler", "enable-scale-to-zero",
String.valueOf(config.scaleToZeroEnabled))));
new DecoratorBuildItem(KNATIVE,
new AddConfigMapDataDecorator(KNATIVE_CONFIG_AUTOSCALER, "enable-scale-to-zero",
String.valueOf(config.scaleToZeroEnabled))));
}

result.add(new DecoratorBuildItem(KNATIVE, new ApplyServiceTypeDecorator(name, config.getServiceType().name())));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import io.quarkus.test.ProdModeTestResults;
import io.quarkus.test.QuarkusProdModeTest;

public class KnativeGlobalContainerConcurrency {
public class KnativeGlobalContainerConcurrencyTest {

@RegisterExtension
static final QuarkusProdModeTest config = new QuarkusProdModeTest()
Expand All @@ -40,7 +40,8 @@ public void assertGeneratedResources() throws IOException {
.deserializeAsList(kubernetesDir.resolve("knative.yml"));

assertThat(kubernetesList).filteredOn(i -> "ConfigMap".equals(i.getKind())).singleElement().satisfies(c -> {
assertThat(c.getMetadata()).satisfies(m -> assertThat(m.getName()).isEqualTo("config-autoscaler"));
assertThat(c.getMetadata().getName()).isEqualTo("config-defaults");
assertThat(c.getMetadata().getNamespace()).isEqualTo("knative-serving");
assertThat(c).isInstanceOfSatisfying(ConfigMap.class, m -> {
assertThat(m.getData()).contains(entry("container-concurrency", "100"));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import io.quarkus.test.ProdModeTestResults;
import io.quarkus.test.QuarkusProdModeTest;

public class KnativeGlobalRequestsPerSecond {
public class KnativeGlobalRequestsPerSecondTest {

@RegisterExtension
static final QuarkusProdModeTest config = new QuarkusProdModeTest()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.io.IOException;
import java.nio.file.Path;
import java.util.List;
import java.util.Map;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand Down Expand Up @@ -40,10 +41,10 @@ public void assertGeneratedResources() throws IOException {
.deserializeAsList(kubernetesDir.resolve("knative.yml"));

assertThat(kubernetesList).filteredOn(i -> "Service".equals(i.getKind())).singleElement().satisfies(i -> {
assertThat(i).isInstanceOfSatisfying(Service.class, s -> {
assertThat(s.getMetadata().getAnnotations()).contains(entry("autoscaling.knative.dev/minScale", "3"));
assertThat(s.getMetadata().getAnnotations()).contains(entry("autoscaling.knative.dev/maxScale", "5"));
});
Service service = (Service) i;
Map<String, String> annotations = service.getSpec().getTemplate().getMetadata().getAnnotations();
assertThat(annotations).contains(entry("autoscaling.knative.dev/minScale", "3"));
assertThat(annotations).contains(entry("autoscaling.knative.dev/maxScale", "5"));
});
}
}
Loading

0 comments on commit 3e3a3b7

Please sign in to comment.