Change Horizontal Pod Autoscaler to scale on OpenTelemetry Collector …

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

@@ -33,10 +33,14 @@ type OpenTelemetryCollectorSpec struct {
 	// +optional
 	Args map[string]string `json:"args,omitempty"`
 
-	// Replicas is the number of pod instances for the underlying OpenTelemetry Collector
+	// Replicas is the number of pod instances for the underlying OpenTelemetry Collector. Set this if your are not using autoscaling
 	// +optional
 	Replicas *int32 `json:"replicas,omitempty"`
 
+	// MinReplicas sets a lower bound to the autoscaling feature.  Set this if your are using autoscaling. 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.  Set
+                  this if your are using autoscaling. 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)
@@ -475,7 +480,7 @@ spec:
                 x-kubernetes-list-type: atomic
               replicas:
                 description: Replicas is the number of pod instances for the underlying
-                  OpenTelemetry Collector
+                  OpenTelemetry Collector. Set this if your are not using autoscaling
                 format: int32
                 type: integer
               resources:

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.  Set
+                  this if your are using autoscaling. 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)
@@ -473,7 +478,7 @@ spec:
                 x-kubernetes-list-type: atomic
               replicas:
                 description: Replicas is the number of pod instances for the underlying
-                  OpenTelemetry Collector
+                  OpenTelemetry Collector. Set this if your are not using autoscaling
                 format: int32
                 type: integer
               resources:

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.  Set this if your are using autoscaling. 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>
@@ -1496,7 +1505,7 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector.
         <td><b>replicas</b></td>
         <td>integer</td>
         <td>
-          Replicas is the number of pod instances for the underlying OpenTelemetry Collector<br/>
+          Replicas is the number of pod instances for the underlying OpenTelemetry Collector. Set this if your are not using autoscaling<br/>
           <br/>
             <i>Format</i>: int32<br/>
         </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

@@ -52,6 +52,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 +82,13 @@ 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
+			}
 		}
 
 		for k, v := range desired.Annotations {

pkg/collector/upgrade/v0_56_0.go

@@ -0,0 +1,74 @@
+// 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 (
+	"context"
+	"fmt"
+
+	autoscalingv1 "k8s.io/api/autoscaling/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
+)
+
+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
+	}
+
+	// Add minReplicas
+	one := int32(1)
+	otelcol.Spec.MinReplicas = &one
+
+	// Find the existing HPA for this collector and upgrade it if necessary
+	listOptions := []client.ListOption{
+		client.InNamespace(otelcol.Namespace),
+		client.MatchingLabels(map[string]string{
+			"app.kubernetes.io/instance":   fmt.Sprintf("%s.%s", otelcol.Namespace, otelcol.Name),
+			"app.kubernetes.io/managed-by": "opentelemetry-operator",
+		}),
+	}
+
+	hpaList := &autoscalingv1.HorizontalPodAutoscalerList{}
+	ctx := context.Background()
+	if err := u.Client.List(ctx, hpaList, listOptions...); err != nil {
+		return nil, err
+	}
+
+	for i := range hpaList.Items {
+		existing := hpaList.Items[i]
+		// If there is an autoscaler based on Deployment, replace it with one based on OpenTelemetryCollector
+		if existing.Spec.ScaleTargetRef.Kind == "Deployment" {
+			updated := existing.DeepCopy()
+			updated.Spec.ScaleTargetRef = autoscalingv1.CrossVersionObjectReference{
+				Kind:       "OpenTelemetryCollector",
+				Name:       naming.OpenTelemetryCollectorName(otelcol.Name),
+				APIVersion: v1alpha1.GroupVersion.String(),
+			}
+			patch := client.MergeFrom(&existing)
+			err := u.Client.Patch(ctx, updated, patch)
+			if err != nil {
+				return nil, err
+			}
+		}
+	}
+
+	u.Log.Info("in upgrade0_56_0", "Otel Instance", otelcol.Name, "Upgrade version", u.Version.String())
+
+	return otelcol, nil
+}

pkg/collector/upgrade/v0_56_0_test.go

@@ -0,0 +1,59 @@
+// 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 (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/tools/record"
+
+	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
+	"github.com/open-telemetry/opentelemetry-operator/internal/version"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/collector/upgrade"
+)
+
+func Test0_56_0Upgrade(t *testing.T) {
+	one := int32(1)
+	three := int32(3)
+
+	collectorInstance := v1alpha1.OpenTelemetryCollector{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "OpenTelemetryCollector",
+			APIVersion: "v1alpha1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "my-instance",
+			Namespace: "somewhere",
+		},
+		Spec: v1alpha1.OpenTelemetryCollectorSpec{
+			Replicas:    &one,
+			MaxReplicas: &three,
+		},
+	}
+
+	collectorInstance.Status.Version = "0.55.0"
+	versionUpgrade := &upgrade.VersionUpgrade{
+		Log:      logger,
+		Version:  version.Get(),
+		Client:   k8sClient,
+		Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize),
+	}
+	upgradedInstance, err := versionUpgrade.ManagedInstance(context.Background(), collectorInstance)
+	assert.NoError(t, err)
+	assert.Equal(t, one, *upgradedInstance.Spec.MinReplicas)
+}

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
+