Change horizontal pod autoscaler to use otelcol scale subresource

Makefile

@@ -152,7 +152,7 @@ e2e:
 	$(KUTTL) test
 
 .PHONY: prepare-e2e
-prepare-e2e: kuttl set-test-image-vars set-image-controller container container-target-allocator start-kind load-image-all
+prepare-e2e: kuttl set-test-image-vars set-image-controller container container-target-allocator start-kind install-metrics-server load-image-all
 	mkdir -p tests/_build/crds tests/_build/manifests
 	$(KUSTOMIZE) build config/default -o tests/_build/manifests/01-opentelemetry-operator.yaml
 	$(KUSTOMIZE) build config/crd -o tests/_build/crds/
@@ -184,6 +184,10 @@ container-target-allocator:
 start-kind:
 	kind create cluster --config $(KIND_CONFIG)
 
+.PHONY: install-metrics-server
+install-metrics-server:
+	./hack/install-metrics-server.sh
+
 .PHONY: load-image-all
 load-image-all: load-image-operator load-image-target-allocator
 

apis/v1alpha1/opentelemetrycollector_types.go

@@ -37,6 +37,10 @@ type OpenTelemetryCollectorSpec struct {
 	// +optional
 	Replicas *int32 `json:"replicas,omitempty"`
 
+	// MinReplicas sets a lower bound to the autoscaling feature.  It must be at least 1
+	// +optional
+	MinReplicas *int32 `json:"minReplicas,omitempty"`
+
 	// MaxReplicas sets an upper bound to the autoscaling feature. If MaxReplicas is set autoscaling is enabled.
 	// +optional
 	MaxReplicas *int32 `json:"maxReplicas,omitempty"`

apis/v1alpha1/opentelemetrycollector_webhook.go

@@ -120,6 +120,15 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error {
 		if r.Spec.Replicas != nil && *r.Spec.Replicas > *r.Spec.MaxReplicas {
 			return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, replicas must not be greater than maxReplicas")
 		}
+
+		if r.Spec.MinReplicas != nil && *r.Spec.MinReplicas > *r.Spec.MaxReplicas {
+			return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, minReplicas must not be greater than maxReplicas")
+		}
+
+		if r.Spec.MinReplicas != nil && *r.Spec.MinReplicas < int32(1) {
+			return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, minReplicas should be one or more")
+		}
+
 	}
 
 	return nil

bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml

@@ -223,6 +223,11 @@ spec:
                   If MaxReplicas is set autoscaling is enabled.
                 format: int32
                 type: integer
+              minReplicas:
+                description: MinReplicas sets a lower bound to the autoscaling feature.  It
+                  must be at least 1
+                format: int32
+                type: integer
               mode:
                 description: Mode represents how the collector should be deployed
                   (deployment, daemonset, statefulset or sidecar)

config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml

@@ -221,6 +221,11 @@ spec:
                   If MaxReplicas is set autoscaling is enabled.
                 format: int32
                 type: integer
+              minReplicas:
+                description: MinReplicas sets a lower bound to the autoscaling feature.  It
+                  must be at least 1
+                format: int32
+                type: integer
               mode:
                 description: Mode represents how the collector should be deployed
                   (deployment, daemonset, statefulset or sidecar)

docs/api.md

@@ -1455,6 +1455,15 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector.
             <i>Format</i>: int32<br/>
         </td>
         <td>false</td>
+      </tr><tr>
+        <td><b>minReplicas</b></td>
+        <td>integer</td>
+        <td>
+          MinReplicas sets a lower bound to the autoscaling feature.  It must be at least 1<br/>
+          <br/>
+            <i>Format</i>: int32<br/>
+        </td>
+        <td>false</td>
       </tr><tr>
         <td><b>mode</b></td>
         <td>enum</td>

hack/install-metrics-server.sh

@@ -0,0 +1,7 @@
+#!/bin/bash
+
+# Install metrics-server on kind clusters for autoscale tests.  Note: This is not needed for minikube,
+# you can just add --addons "metrics-server" to the start command.
+kubectl apply -f https://github.com/kubernetes-sigs/metrics-server/releases/latest/download/components.yaml
+kubectl patch deployment -n kube-system metrics-server --type "json" -p '[{"op": "add", "path": "/spec/template/spec/containers/0/args/-", "value": --kubelet-insecure-tls}]'
+kubectl wait --for=condition=available deployment/metrics-server -n kube-system  --timeout=5m

pkg/collector/horizontalpodautoscaler.go

@@ -35,17 +35,18 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al
 
 	return autoscalingv1.HorizontalPodAutoscaler{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        naming.Collector(otelcol),
+			Name:        naming.HorizontalPodAutoscaler(otelcol),
 			Namespace:   otelcol.Namespace,
 			Labels:      labels,
 			Annotations: annotations,
 		},
 		Spec: autoscalingv1.HorizontalPodAutoscalerSpec{
 			ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
-				APIVersion: "apps/v1",
-				Kind:       "Deployment",
-				Name:       naming.Collector(otelcol),
+				APIVersion: v1alpha1.GroupVersion.String(),
+				Kind:       "OpenTelemetryCollector",
+				Name:       naming.OpenTelemetryCollector(otelcol),
 			},
+
 			MinReplicas:                    otelcol.Spec.Replicas,
 			MaxReplicas:                    *otelcol.Spec.MaxReplicas,
 			TargetCPUUtilizationPercentage: &cpuTarget,

pkg/collector/reconcile/deployment.go

@@ -99,11 +99,6 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D
 			updated.ObjectMeta.Labels[k] = v
 		}
 
-		if params.Instance.Spec.MaxReplicas != nil && desired.Labels["app.kubernetes.io/component"] == "opentelemetry-collector" {
-			currentReplicas := currentReplicasWithHPA(params.Instance.Spec, existing.Status.Replicas)
-			updated.Spec.Replicas = &currentReplicas
-		}
-
 		// Selector is an immutable field, if set, we cannot modify it otherwise we will face reconciliation error.
 		updated.Spec.Selector = existing.Spec.Selector.DeepCopy()
 

pkg/collector/reconcile/horizontalpodautoscaler.go

@@ -24,7 +24,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
+	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/collector"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 )
 
 // +kubebuilder:rbac:groups=autoscaling,resources=horizontalpodautoscalers,verbs=get;list;watch;create;update;patch;delete
@@ -52,6 +54,7 @@ func HorizontalPodAutoscalers(ctx context.Context, params Params) error {
 }
 
 func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv1.HorizontalPodAutoscaler) error {
+	one := int32(1)
 	for _, obj := range expected {
 		desired := obj
 
@@ -81,9 +84,23 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect
 		}
 
 		updated.OwnerReferences = desired.OwnerReferences
-		updated.Spec.MinReplicas = params.Instance.Spec.Replicas
 		if params.Instance.Spec.MaxReplicas != nil {
 			updated.Spec.MaxReplicas = *params.Instance.Spec.MaxReplicas
+			if params.Instance.Spec.MinReplicas != nil {
+				updated.Spec.MinReplicas = params.Instance.Spec.MinReplicas
+			} else {
+				updated.Spec.MinReplicas = &one
+			}
+		}
+
+		// If the deployment has an autoscaler based on Deployment, replace it with one based on OpenTelemetryCollector
+		// Note: this is for version 0.56.0 and can eventually be removed
+		if existing.Spec.ScaleTargetRef.Kind == "Deployment" {
+			updated.Spec.ScaleTargetRef = autoscalingv1.CrossVersionObjectReference{
+				Kind:       "OpenTelemetryCollector",
+				Name:       naming.OpenTelemetryCollectorName(params.Instance.Name),
+				APIVersion: v1alpha1.GroupVersion.String(),
+			}
 		}
 
 		for k, v := range desired.Annotations {

pkg/collector/upgrade/v0_56_0.go

@@ -0,0 +1,32 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package upgrade
+
+import (
+	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
+)
+
+func upgrade0_56_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) {
+	// return if this does not use an autoscaler
+	if otelcol.Spec.MaxReplicas == nil {
+		return otelcol, nil
+	}
+
+	u.Log.Info("in upgrade0_56_0", "Otel Instance", otelcol.Name, "Upgrade version", u.Version.String())
+	one := int32(1)
+	otelcol.Spec.MinReplicas = &one
+
+	return otelcol, nil
+}

pkg/collector/upgrade/v0_56_0_test.go

@@ -0,0 +1,24 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package upgrade_test
+
+import (
+	"fmt"
+	"testing"
+)
+
+func Test0_56_0Upgrade(t *testing.T) {
+	fmt.Println("NYI")
+}

pkg/collector/upgrade/versions.go

@@ -73,6 +73,10 @@ var (
 			Version: *semver.MustParse("0.43.0"),
 			upgrade: upgrade0_43_0,
 		},
+		{
+			Version: *semver.MustParse("0.56.0"),
+			upgrade: upgrade0_56_0,
+		},
 	}
 
 	// Latest represents the latest version that we need to upgrade. This is not necessarily the latest known version.

pkg/naming/main.go

@@ -54,6 +54,21 @@ func Collector(otelcol v1alpha1.OpenTelemetryCollector) string {
 	return DNSName(Truncate("%s-collector", 63, otelcol.Name))
 }
 
+// HorizontalPodAutoscaler builds the autoscaler name based on the instance.
+func HorizontalPodAutoscaler(otelcol v1alpha1.OpenTelemetryCollector) string {
+	return DNSName(Truncate("%s-collector", 63, otelcol.Name))
+}
+
+// HorizontalPodAutoscaler builds the collector (deployment/daemonset) name based on the instance.
+func OpenTelemetryCollector(otelcol v1alpha1.OpenTelemetryCollector) string {
+	return DNSName(Truncate("%s", 63, otelcol.Name))
+}
+
+// HorizontalPodAutoscaler builds the collector (deployment/daemonset) name based on the instance.
+func OpenTelemetryCollectorName(otelcolName string) string {
+	return DNSName(Truncate("%s", 63, otelcolName))
+}
+
 // TargetAllocator returns the TargetAllocator deployment resource name.
 func TargetAllocator(otelcol v1alpha1.OpenTelemetryCollector) string {
 	return DNSName(Truncate("%s-targetallocator", 63, otelcol.Name))

tests/e2e/autoscale/00-assert.yaml

@@ -0,0 +1,16 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: simplest-collector
+status:
+  readyReplicas: 1
+
+---
+apiVersion: autoscaling/v1
+kind: HorizontalPodAutoscaler
+metadata:
+  name: simplest-collector
+spec:
+  minReplicas: 1
+  maxReplicas: 2
+

tests/e2e/autoscale/00-install.yaml

@@ -0,0 +1,32 @@
+apiVersion: opentelemetry.io/v1alpha1
+kind: OpenTelemetryCollector
+metadata:
+  name: simplest
+spec:
+  minReplicas: 1
+  maxReplicas: 2
+  resources:
+    limits:
+      cpu: 500m
+      memory: 128Mi
+    requests:
+      cpu: 5m
+      memory: 64Mi
+
+  config: |
+    receivers:
+      otlp:
+        protocols:
+          grpc:
+          http:
+    processors:
+
+    exporters:
+      logging:
+
+    service:
+      pipelines:
+        traces:
+          receivers: [otlp]
+          processors: []
+          exporters: [logging]

tests/e2e/autoscale/01-assert.yaml

@@ -0,0 +1,18 @@
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: tracegen
+status:
+  conditions:
+    - status: "True"
+      type: Complete
+
+---
+apiVersion: opentelemetry.io/v1alpha1
+kind: OpenTelemetryCollector
+
+metadata:
+  name: simplest
+status:
+  scale:
+    replicas: 2

tests/e2e/autoscale/01-install.yaml

@@ -0,0 +1,19 @@
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: tracegen
+spec:
+  template:
+    spec:
+      containers:
+        - name: tracegen
+          image: ghcr.io/open-telemetry/opentelemetry-collector-contrib/tracegen:latest
+          command:
+            - "./tracegen"
+          args:
+            - -otlp-endpoint=simplest-collector-headless:4317
+            - -otlp-insecure
+            - -duration=1m
+            - -workers=20
+      restartPolicy: Never
+  backoffLimit: 4