From c5f0e1d258933705274865f5b279aa2f00749b03 Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Thu, 23 Jun 2022 15:08:35 +0200 Subject: [PATCH 01/28] Change horizontal pod autoscaler to use otelcol scale subresource Signed-off-by: Kevin Earls --- apis/v1alpha1/opentelemetrycollector_types.go | 4 ++++ .../opentelemetry.io_opentelemetrycollectors.yaml | 4 ++++ .../opentelemetry.io_opentelemetrycollectors.yaml | 4 ++++ docs/api.md | 9 +++++++++ pkg/collector/horizontalpodautoscaler.go | 8 ++++---- pkg/collector/horizontalpodautoscaler_test.go | 3 ++- pkg/collector/reconcile/deployment.go | 5 ----- pkg/collector/reconcile/horizontalpodautoscaler.go | 2 +- .../reconcile/horizontalpodautoscaler_test.go | 10 +++++----- 9 files changed, 33 insertions(+), 16 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 25a0e60db7..6e7c550998 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/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 + // +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"` diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index 173ea68836..3b49922713 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -223,6 +223,10 @@ spec: If MaxReplicas is set autoscaling is enabled. format: int32 type: integer + minReplicas: + description: MinReplicas sets a lower bound to the autoscaling feature + format: int32 + type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 60335aeff5..2c5033ba68 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -221,6 +221,10 @@ spec: If MaxReplicas is set autoscaling is enabled. format: int32 type: integer + minReplicas: + description: MinReplicas sets a lower bound to the autoscaling feature + format: int32 + type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/docs/api.md b/docs/api.md index 6e947a1127..80e6c6015f 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1455,6 +1455,15 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. Format: int32
false + + minReplicas + integer + + MinReplicas sets a lower bound to the autoscaling feature
+
+ Format: int32
+ + false mode enum diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go index 334ae87bd6..927d9b35f4 100644 --- a/pkg/collector/horizontalpodautoscaler.go +++ b/pkg/collector/horizontalpodautoscaler.go @@ -35,18 +35,18 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al return autoscalingv1.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ - Name: naming.Collector(otelcol), + Name: otelcol.Name, Namespace: otelcol.Namespace, Labels: labels, Annotations: annotations, }, Spec: autoscalingv1.HorizontalPodAutoscalerSpec{ ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "OpenTelemetryCollector", Name: naming.Collector(otelcol), }, - MinReplicas: otelcol.Spec.Replicas, + MinReplicas: &otelcol.Spec.MinReplicas, MaxReplicas: *otelcol.Spec.MaxReplicas, TargetCPUUtilizationPercentage: &cpuTarget, }, diff --git a/pkg/collector/horizontalpodautoscaler_test.go b/pkg/collector/horizontalpodautoscaler_test.go index b168816549..2740329e87 100644 --- a/pkg/collector/horizontalpodautoscaler_test.go +++ b/pkg/collector/horizontalpodautoscaler_test.go @@ -36,6 +36,7 @@ func TestHPA(t *testing.T) { }, Spec: v1alpha1.OpenTelemetryCollectorSpec{ Replicas: &minReplicas, + MinReplicas: minReplicas, MaxReplicas: &maxReplicas, }, } @@ -44,7 +45,7 @@ func TestHPA(t *testing.T) { hpa := HorizontalPodAutoscaler(cfg, logger, otelcol) // verify - assert.Equal(t, "my-instance-collector", hpa.Name) + assert.Equal(t, "my-instance", hpa.Name) assert.Equal(t, "my-instance-collector", hpa.Labels["app.kubernetes.io/name"]) assert.Equal(t, int32(3), *hpa.Spec.MinReplicas) assert.Equal(t, int32(5), hpa.Spec.MaxReplicas) diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index b57a030c9f..99aa3a5a31 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/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 { - currentReplicas := currentReplicasWithHPA(params.Instance.Spec, existing.Status.Replicas) - updated.Spec.Replicas = ¤tReplicas - } - // Selector is an immutable field, if set, we cannot modify it otherwise we will face reconciliation error. updated.Spec.Selector = existing.Spec.Selector.DeepCopy() diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 6d0bca72a2..0ee3504907 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -81,7 +81,7 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect } updated.OwnerReferences = desired.OwnerReferences - updated.Spec.MinReplicas = params.Instance.Spec.Replicas + updated.Spec.MinReplicas = ¶ms.Instance.Spec.MinReplicas if params.Instance.Spec.MaxReplicas != nil { updated.Spec.MaxReplicas = *params.Instance.Spec.MaxReplicas } diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index 62b901da37..a0cb4abf08 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -42,7 +42,7 @@ func TestExpectedHPA(t *testing.T) { err := expectedHorizontalPodAutoscalers(context.Background(), params, []autoscalingv1.HorizontalPodAutoscaler{expectedHPA}) assert.NoError(t, err) - exists, err := populateObjectIfExists(t, &autoscalingv1.HorizontalPodAutoscaler{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) + exists, err := populateObjectIfExists(t, &autoscalingv1.HorizontalPodAutoscaler{}, types.NamespacedName{Namespace: "default", Name: "test"}) assert.NoError(t, err) assert.True(t, exists) }) @@ -55,16 +55,15 @@ func TestExpectedHPA(t *testing.T) { updateParms.Instance.Spec.MaxReplicas = &maxReplicas updatedHPA := collector.HorizontalPodAutoscaler(updateParms.Config, logger, updateParms.Instance) - createObjectIfNotExists(t, "test-collector", &updatedHPA) + createObjectIfNotExists(t, "test", &updatedHPA) err := expectedHorizontalPodAutoscalers(context.Background(), updateParms, []autoscalingv1.HorizontalPodAutoscaler{updatedHPA}) assert.NoError(t, err) actual := autoscalingv1.HorizontalPodAutoscaler{} - exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collector"}) + exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test"}) assert.NoError(t, err) assert.True(t, exists) - assert.Equal(t, int32(1), *actual.Spec.MinReplicas) assert.Equal(t, int32(3), actual.Spec.MaxReplicas) }) @@ -73,7 +72,7 @@ func TestExpectedHPA(t *testing.T) { assert.NoError(t, err) actual := v1.Deployment{} - exists, _ := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collecto"}) + exists, _ := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test"}) assert.False(t, exists) }) } @@ -112,6 +111,7 @@ func paramsWithHPA() Params { }}, Config: string(configYAML), Replicas: &minReplicas, + MinReplicas: minReplicas, MaxReplicas: &maxReplicas, }, }, From 04abf4347a30f1dda745416950d233f6c8a7c64b Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Thu, 23 Jun 2022 15:54:32 +0200 Subject: [PATCH 02/28] Fix name Signed-off-by: Kevin Earls --- pkg/collector/horizontalpodautoscaler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go index 927d9b35f4..28947f0fa7 100644 --- a/pkg/collector/horizontalpodautoscaler.go +++ b/pkg/collector/horizontalpodautoscaler.go @@ -44,7 +44,7 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ APIVersion: v1alpha1.GroupVersion.String(), Kind: "OpenTelemetryCollector", - Name: naming.Collector(otelcol), + Name: otelcol.Name, }, MinReplicas: &otelcol.Spec.MinReplicas, MaxReplicas: *otelcol.Spec.MaxReplicas, From e6e73daa098935c2d0e3b62024955bd825ac96af Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Thu, 23 Jun 2022 15:08:35 +0200 Subject: [PATCH 03/28] Change horizontal pod autoscaler to use otelcol scale subresource Signed-off-by: Kevin Earls --- apis/v1alpha1/opentelemetrycollector_types.go | 4 ++++ .../opentelemetry.io_opentelemetrycollectors.yaml | 4 ++++ .../opentelemetry.io_opentelemetrycollectors.yaml | 4 ++++ docs/api.md | 9 +++++++++ pkg/collector/horizontalpodautoscaler.go | 8 ++++---- pkg/collector/horizontalpodautoscaler_test.go | 3 ++- pkg/collector/reconcile/deployment.go | 5 ----- pkg/collector/reconcile/horizontalpodautoscaler.go | 2 +- .../reconcile/horizontalpodautoscaler_test.go | 10 +++++----- 9 files changed, 33 insertions(+), 16 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 25a0e60db7..6e7c550998 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/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 + // +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"` diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index 173ea68836..3b49922713 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -223,6 +223,10 @@ spec: If MaxReplicas is set autoscaling is enabled. format: int32 type: integer + minReplicas: + description: MinReplicas sets a lower bound to the autoscaling feature + format: int32 + type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 60335aeff5..2c5033ba68 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -221,6 +221,10 @@ spec: If MaxReplicas is set autoscaling is enabled. format: int32 type: integer + minReplicas: + description: MinReplicas sets a lower bound to the autoscaling feature + format: int32 + type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/docs/api.md b/docs/api.md index 6e947a1127..80e6c6015f 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1455,6 +1455,15 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. Format: int32
false + + minReplicas + integer + + MinReplicas sets a lower bound to the autoscaling feature
+
+ Format: int32
+ + false mode enum diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go index 334ae87bd6..927d9b35f4 100644 --- a/pkg/collector/horizontalpodautoscaler.go +++ b/pkg/collector/horizontalpodautoscaler.go @@ -35,18 +35,18 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al return autoscalingv1.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ - Name: naming.Collector(otelcol), + Name: otelcol.Name, Namespace: otelcol.Namespace, Labels: labels, Annotations: annotations, }, Spec: autoscalingv1.HorizontalPodAutoscalerSpec{ ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "OpenTelemetryCollector", Name: naming.Collector(otelcol), }, - MinReplicas: otelcol.Spec.Replicas, + MinReplicas: &otelcol.Spec.MinReplicas, MaxReplicas: *otelcol.Spec.MaxReplicas, TargetCPUUtilizationPercentage: &cpuTarget, }, diff --git a/pkg/collector/horizontalpodautoscaler_test.go b/pkg/collector/horizontalpodautoscaler_test.go index b168816549..2740329e87 100644 --- a/pkg/collector/horizontalpodautoscaler_test.go +++ b/pkg/collector/horizontalpodautoscaler_test.go @@ -36,6 +36,7 @@ func TestHPA(t *testing.T) { }, Spec: v1alpha1.OpenTelemetryCollectorSpec{ Replicas: &minReplicas, + MinReplicas: minReplicas, MaxReplicas: &maxReplicas, }, } @@ -44,7 +45,7 @@ func TestHPA(t *testing.T) { hpa := HorizontalPodAutoscaler(cfg, logger, otelcol) // verify - assert.Equal(t, "my-instance-collector", hpa.Name) + assert.Equal(t, "my-instance", hpa.Name) assert.Equal(t, "my-instance-collector", hpa.Labels["app.kubernetes.io/name"]) assert.Equal(t, int32(3), *hpa.Spec.MinReplicas) assert.Equal(t, int32(5), hpa.Spec.MaxReplicas) diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index b57a030c9f..99aa3a5a31 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/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 { - currentReplicas := currentReplicasWithHPA(params.Instance.Spec, existing.Status.Replicas) - updated.Spec.Replicas = ¤tReplicas - } - // Selector is an immutable field, if set, we cannot modify it otherwise we will face reconciliation error. updated.Spec.Selector = existing.Spec.Selector.DeepCopy() diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 6d0bca72a2..0ee3504907 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -81,7 +81,7 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect } updated.OwnerReferences = desired.OwnerReferences - updated.Spec.MinReplicas = params.Instance.Spec.Replicas + updated.Spec.MinReplicas = ¶ms.Instance.Spec.MinReplicas if params.Instance.Spec.MaxReplicas != nil { updated.Spec.MaxReplicas = *params.Instance.Spec.MaxReplicas } diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index 62b901da37..a0cb4abf08 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -42,7 +42,7 @@ func TestExpectedHPA(t *testing.T) { err := expectedHorizontalPodAutoscalers(context.Background(), params, []autoscalingv1.HorizontalPodAutoscaler{expectedHPA}) assert.NoError(t, err) - exists, err := populateObjectIfExists(t, &autoscalingv1.HorizontalPodAutoscaler{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) + exists, err := populateObjectIfExists(t, &autoscalingv1.HorizontalPodAutoscaler{}, types.NamespacedName{Namespace: "default", Name: "test"}) assert.NoError(t, err) assert.True(t, exists) }) @@ -55,16 +55,15 @@ func TestExpectedHPA(t *testing.T) { updateParms.Instance.Spec.MaxReplicas = &maxReplicas updatedHPA := collector.HorizontalPodAutoscaler(updateParms.Config, logger, updateParms.Instance) - createObjectIfNotExists(t, "test-collector", &updatedHPA) + createObjectIfNotExists(t, "test", &updatedHPA) err := expectedHorizontalPodAutoscalers(context.Background(), updateParms, []autoscalingv1.HorizontalPodAutoscaler{updatedHPA}) assert.NoError(t, err) actual := autoscalingv1.HorizontalPodAutoscaler{} - exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collector"}) + exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test"}) assert.NoError(t, err) assert.True(t, exists) - assert.Equal(t, int32(1), *actual.Spec.MinReplicas) assert.Equal(t, int32(3), actual.Spec.MaxReplicas) }) @@ -73,7 +72,7 @@ func TestExpectedHPA(t *testing.T) { assert.NoError(t, err) actual := v1.Deployment{} - exists, _ := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collecto"}) + exists, _ := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test"}) assert.False(t, exists) }) } @@ -112,6 +111,7 @@ func paramsWithHPA() Params { }}, Config: string(configYAML), Replicas: &minReplicas, + MinReplicas: minReplicas, MaxReplicas: &maxReplicas, }, }, From 6ba5e9309723aa1e63ecc1ab0718ac844f2509bc Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Thu, 23 Jun 2022 15:54:32 +0200 Subject: [PATCH 04/28] Fix name Signed-off-by: Kevin Earls --- pkg/collector/horizontalpodautoscaler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go index 927d9b35f4..28947f0fa7 100644 --- a/pkg/collector/horizontalpodautoscaler.go +++ b/pkg/collector/horizontalpodautoscaler.go @@ -44,7 +44,7 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ APIVersion: v1alpha1.GroupVersion.String(), Kind: "OpenTelemetryCollector", - Name: naming.Collector(otelcol), + Name: otelcol.Name, }, MinReplicas: &otelcol.Spec.MinReplicas, MaxReplicas: *otelcol.Spec.MaxReplicas, From 949a29f303464e225b53af8d5c7285d8273bed8b Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Wed, 29 Jun 2022 10:06:54 +0200 Subject: [PATCH 05/28] Moved autoscale values into their own spec, added scale up test Signed-off-by: Kevin Earls --- apis/v1alpha1/opentelemetrycollector_types.go | 18 ++++-- .../opentelemetrycollector_webhook.go | 11 +++- apis/v1alpha1/zz_generated.deepcopy.go | 31 +++++++-- ...ntelemetry.io_opentelemetrycollectors.yaml | 24 ++++--- ...ntelemetry.io_opentelemetrycollectors.yaml | 24 ++++--- docs/api.md | 63 +++++++++++++------ pkg/collector/horizontalpodautoscaler.go | 9 +-- pkg/collector/horizontalpodautoscaler_test.go | 10 +-- pkg/collector/reconcile/deployment.go | 8 +-- pkg/collector/reconcile/deployment_test.go | 6 +- .../reconcile/horizontalpodautoscaler.go | 6 +- .../reconcile/horizontalpodautoscaler_test.go | 20 +++--- pkg/naming/main.go | 10 +++ tests/e2e/autoscale/00-assert.yaml | 16 +++++ tests/e2e/autoscale/00-install.yaml | 34 ++++++++++ tests/e2e/autoscale/01-assert.yaml | 18 ++++++ tests/e2e/autoscale/01-install.yaml | 19 ++++++ 17 files changed, 252 insertions(+), 75 deletions(-) create mode 100644 tests/e2e/autoscale/00-assert.yaml create mode 100644 tests/e2e/autoscale/00-install.yaml create mode 100644 tests/e2e/autoscale/01-assert.yaml create mode 100644 tests/e2e/autoscale/01-install.yaml diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 6e7c550998..a14f7c1248 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -19,6 +19,17 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// AutoScaleSpec defines the common elements used for create HPAs +type AutoScaleSpec struct { + // MinReplicas sets a lower bound to the autoscaling feature. + // +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"` +} + // OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. type OpenTelemetryCollectorSpec struct { // Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation for details. @@ -37,13 +48,8 @@ type OpenTelemetryCollectorSpec struct { // +optional Replicas *int32 `json:"replicas,omitempty"` - // MinReplicas sets a lower bound to the autoscaling feature - // +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"` + AutoScaleSpec `json:"autoscale,omitempty"` // ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) // +optional diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index f4a6b4f068..db42d94b7e 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -112,14 +112,19 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error { } // validate autoscale with horizontal pod autoscaler - if r.Spec.MaxReplicas != nil { - if *r.Spec.MaxReplicas < int32(1) { + if r.Spec.AutoScaleSpec.MaxReplicas != nil { + if *r.Spec.AutoScaleSpec.MaxReplicas < int32(1) { return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, maxReplicas should be defined and more than one") } - if r.Spec.Replicas != nil && *r.Spec.Replicas > *r.Spec.MaxReplicas { + if r.Spec.Replicas != nil && *r.Spec.Replicas > *r.Spec.AutoScaleSpec.MaxReplicas { return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, replicas must not be greater than maxReplicas") } + + if r.Spec.Replicas != nil && *r.Spec.Replicas < *r.Spec.AutoScaleSpec.MinReplicas { + return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, replicas must not be less than minReplicas") + } + } return nil diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index ef246bbdf0..a7c1140ea6 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -24,6 +24,31 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AutoScaleSpec) DeepCopyInto(out *AutoScaleSpec) { + *out = *in + if in.MinReplicas != nil { + in, out := &in.MinReplicas, &out.MinReplicas + *out = new(int32) + **out = **in + } + if in.MaxReplicas != nil { + in, out := &in.MaxReplicas, &out.MaxReplicas + *out = new(int32) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AutoScaleSpec. +func (in *AutoScaleSpec) DeepCopy() *AutoScaleSpec { + if in == nil { + return nil + } + out := new(AutoScaleSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Exporter) DeepCopyInto(out *Exporter) { *out = *in @@ -264,11 +289,7 @@ func (in *OpenTelemetryCollectorSpec) DeepCopyInto(out *OpenTelemetryCollectorSp *out = new(int32) **out = **in } - if in.MaxReplicas != nil { - in, out := &in.MaxReplicas, &out.MaxReplicas - *out = new(int32) - **out = **in - } + in.AutoScaleSpec.DeepCopyInto(&out.AutoScaleSpec) out.TargetAllocator = in.TargetAllocator if in.SecurityContext != nil { in, out := &in.SecurityContext, &out.SecurityContext diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index 3b49922713..50323d4585 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -59,6 +59,21 @@ spec: description: Args is the set of arguments to pass to the OpenTelemetry Collector binary type: object + autoscale: + description: AutoScaleSpec defines the common elements used for create + HPAs + properties: + maxReplicas: + description: MaxReplicas sets an upper bound to the autoscaling + feature. If MaxReplicas is set autoscaling is enabled. + format: int32 + type: integer + minReplicas: + description: MinReplicas sets a lower bound to the autoscaling + feature. + format: int32 + type: integer + type: object config: description: Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation @@ -218,15 +233,6 @@ spec: description: ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) type: string - maxReplicas: - description: MaxReplicas sets an upper bound to the autoscaling feature. - If MaxReplicas is set autoscaling is enabled. - format: int32 - type: integer - minReplicas: - description: MinReplicas sets a lower bound to the autoscaling feature - format: int32 - type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 2c5033ba68..c3d51f3441 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -57,6 +57,21 @@ spec: description: Args is the set of arguments to pass to the OpenTelemetry Collector binary type: object + autoscale: + description: AutoScaleSpec defines the common elements used for create + HPAs + properties: + maxReplicas: + description: MaxReplicas sets an upper bound to the autoscaling + feature. If MaxReplicas is set autoscaling is enabled. + format: int32 + type: integer + minReplicas: + description: MinReplicas sets a lower bound to the autoscaling + feature. + format: int32 + type: integer + type: object config: description: Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation @@ -216,15 +231,6 @@ spec: description: ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) type: string - maxReplicas: - description: MaxReplicas sets an upper bound to the autoscaling feature. - If MaxReplicas is set autoscaling is enabled. - format: int32 - type: integer - minReplicas: - description: MinReplicas sets a lower bound to the autoscaling feature - format: int32 - type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/docs/api.md b/docs/api.md index 80e6c6015f..3ffab9ab7f 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1404,6 +1404,13 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. Args is the set of arguments to pass to the OpenTelemetry Collector binary
false + + autoscale + object + + AutoScaleSpec defines the common elements used for create HPAs
+ + false config string @@ -1446,24 +1453,6 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent)
false - - maxReplicas - integer - - MaxReplicas sets an upper bound to the autoscaling feature. If MaxReplicas is set autoscaling is enabled.
-
- Format: int32
- - false - - minReplicas - integer - - MinReplicas sets a lower bound to the autoscaling feature
-
- Format: int32
- - false mode enum @@ -1579,6 +1568,44 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. +### OpenTelemetryCollector.spec.autoscale +[↩ Parent](#opentelemetrycollectorspec) + + + +AutoScaleSpec defines the common elements used for create HPAs + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
maxReplicasinteger + MaxReplicas sets an upper bound to the autoscaling feature. If MaxReplicas is set autoscaling is enabled.
+
+ Format: int32
+ 		false
minReplicasinteger + MinReplicas sets a lower bound to the autoscaling feature.
+
+ Format: int32
+ 		false
+ + ### OpenTelemetryCollector.spec.env[index] [↩ Parent](#opentelemetrycollectorspec) diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go index 28947f0fa7..d8000a6526 100644 --- a/pkg/collector/horizontalpodautoscaler.go +++ b/pkg/collector/horizontalpodautoscaler.go @@ -35,7 +35,7 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al return autoscalingv1.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ - Name: otelcol.Name, + Name: naming.HorizontalPodAutoscaler(otelcol), Namespace: otelcol.Namespace, Labels: labels, Annotations: annotations, @@ -44,10 +44,11 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ APIVersion: v1alpha1.GroupVersion.String(), Kind: "OpenTelemetryCollector", - Name: otelcol.Name, + Name: naming.OpenTelemetryCollector(otelcol), }, - MinReplicas: &otelcol.Spec.MinReplicas, - MaxReplicas: *otelcol.Spec.MaxReplicas, + + MinReplicas: otelcol.Spec.AutoScaleSpec.MinReplicas, + MaxReplicas: *otelcol.Spec.AutoScaleSpec.MaxReplicas, TargetCPUUtilizationPercentage: &cpuTarget, }, } diff --git a/pkg/collector/horizontalpodautoscaler_test.go b/pkg/collector/horizontalpodautoscaler_test.go index 2740329e87..efe6e87f61 100644 --- a/pkg/collector/horizontalpodautoscaler_test.go +++ b/pkg/collector/horizontalpodautoscaler_test.go @@ -35,9 +35,11 @@ func TestHPA(t *testing.T) { Name: "my-instance", }, Spec: v1alpha1.OpenTelemetryCollectorSpec{ - Replicas: &minReplicas, - MinReplicas: minReplicas, - MaxReplicas: &maxReplicas, + Replicas: &minReplicas, + AutoScaleSpec: v1alpha1.AutoScaleSpec{ + MinReplicas: &minReplicas, + MaxReplicas: &maxReplicas, + }, }, } @@ -45,7 +47,7 @@ func TestHPA(t *testing.T) { hpa := HorizontalPodAutoscaler(cfg, logger, otelcol) // verify - assert.Equal(t, "my-instance", hpa.Name) + assert.Equal(t, "my-instance-collector", hpa.Name) assert.Equal(t, "my-instance-collector", hpa.Labels["app.kubernetes.io/name"]) assert.Equal(t, int32(3), *hpa.Spec.MinReplicas) assert.Equal(t, int32(5), hpa.Spec.MaxReplicas) diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index 99aa3a5a31..b0bbd90d93 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/pkg/collector/reconcile/deployment.go @@ -150,12 +150,12 @@ func deleteDeployments(ctx context.Context, params Params, expected []appsv1.Dep // currentReplicasWithHPA calculates deployment replicas if HPA is enabled. func currentReplicasWithHPA(spec v1alpha1.OpenTelemetryCollectorSpec, curr int32) int32 { - if curr < *spec.Replicas { - return *spec.Replicas + if curr < *spec.AutoScaleSpec.MinReplicas { + return *spec.AutoScaleSpec.MinReplicas } - if curr > *spec.MaxReplicas { - return *spec.MaxReplicas + if curr > *spec.AutoScaleSpec.MaxReplicas { + return *spec.AutoScaleSpec.MaxReplicas } return curr diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index 5a513d098b..a37f365bd1 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -239,8 +239,10 @@ func TestCurrentReplicasWithHPA(t *testing.T) { minReplicas := int32(2) maxReplicas := int32(5) spec := v1alpha1.OpenTelemetryCollectorSpec{ - Replicas: &minReplicas, - MaxReplicas: &maxReplicas, + AutoScaleSpec: v1alpha1.AutoScaleSpec{ + MinReplicas: &minReplicas, + MaxReplicas: &maxReplicas, + }, } res := currentReplicasWithHPA(spec, 10) diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 0ee3504907..4354efb61e 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -34,7 +34,7 @@ func HorizontalPodAutoscalers(ctx context.Context, params Params) error { desired := []autoscalingv1.HorizontalPodAutoscaler{} // check if autoscale mode is on, e.g MaxReplicas is not nil - if params.Instance.Spec.MaxReplicas != nil { + if params.Instance.Spec.AutoScaleSpec.MaxReplicas != nil { desired = append(desired, collector.HorizontalPodAutoscaler(params.Config, params.Log, params.Instance)) } @@ -81,9 +81,9 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect } updated.OwnerReferences = desired.OwnerReferences - updated.Spec.MinReplicas = ¶ms.Instance.Spec.MinReplicas + updated.Spec.MinReplicas = params.Instance.Spec.AutoScaleSpec.MinReplicas if params.Instance.Spec.MaxReplicas != nil { - updated.Spec.MaxReplicas = *params.Instance.Spec.MaxReplicas + updated.Spec.MaxReplicas = *params.Instance.Spec.AutoScaleSpec.MaxReplicas } for k, v := range desired.Annotations { diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index a0cb4abf08..b912899e3f 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -42,7 +42,7 @@ func TestExpectedHPA(t *testing.T) { err := expectedHorizontalPodAutoscalers(context.Background(), params, []autoscalingv1.HorizontalPodAutoscaler{expectedHPA}) assert.NoError(t, err) - exists, err := populateObjectIfExists(t, &autoscalingv1.HorizontalPodAutoscaler{}, types.NamespacedName{Namespace: "default", Name: "test"}) + exists, err := populateObjectIfExists(t, &autoscalingv1.HorizontalPodAutoscaler{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) assert.NoError(t, err) assert.True(t, exists) }) @@ -52,18 +52,20 @@ func TestExpectedHPA(t *testing.T) { maxReplicas := int32(3) updateParms := paramsWithHPA() updateParms.Instance.Spec.Replicas = &minReplicas + updateParms.Instance.Spec.MinReplicas = &minReplicas updateParms.Instance.Spec.MaxReplicas = &maxReplicas updatedHPA := collector.HorizontalPodAutoscaler(updateParms.Config, logger, updateParms.Instance) - createObjectIfNotExists(t, "test", &updatedHPA) + createObjectIfNotExists(t, "test-collector", &updatedHPA) err := expectedHorizontalPodAutoscalers(context.Background(), updateParms, []autoscalingv1.HorizontalPodAutoscaler{updatedHPA}) assert.NoError(t, err) actual := autoscalingv1.HorizontalPodAutoscaler{} - exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test"}) + exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collector"}) assert.NoError(t, err) assert.True(t, exists) + assert.Equal(t, int32(1), *actual.Spec.MinReplicas) assert.Equal(t, int32(3), actual.Spec.MaxReplicas) }) @@ -72,7 +74,7 @@ func TestExpectedHPA(t *testing.T) { assert.NoError(t, err) actual := v1.Deployment{} - exists, _ := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test"}) + exists, _ := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collecto"}) assert.False(t, exists) }) } @@ -109,10 +111,12 @@ func paramsWithHPA() Params { }, NodePort: 0, }}, - Config: string(configYAML), - Replicas: &minReplicas, - MinReplicas: minReplicas, - MaxReplicas: &maxReplicas, + Config: string(configYAML), + Replicas: &minReplicas, + AutoScaleSpec: v1alpha1.AutoScaleSpec{ + MinReplicas: &minReplicas, + MaxReplicas: &maxReplicas, + }, }, }, Scheme: testScheme, diff --git a/pkg/naming/main.go b/pkg/naming/main.go index f1124fa616..2e2d32c06c 100644 --- a/pkg/naming/main.go +++ b/pkg/naming/main.go @@ -54,6 +54,16 @@ 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)) +} + // TargetAllocator returns the TargetAllocator deployment resource name. func TargetAllocator(otelcol v1alpha1.OpenTelemetryCollector) string { return DNSName(Truncate("%s-targetallocator", 63, otelcol.Name)) diff --git a/tests/e2e/autoscale/00-assert.yaml b/tests/e2e/autoscale/00-assert.yaml new file mode 100644 index 0000000000..e1397797b5 --- /dev/null +++ b/tests/e2e/autoscale/00-assert.yaml @@ -0,0 +1,16 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: simplest-collector +status: + readyReplicas: 2 + +--- +apiVersion: autoscaling/v1 +kind: HorizontalPodAutoscaler +metadata: + name: simplest-collector +spec: + minReplicas: 2 + maxReplicas: 10 + diff --git a/tests/e2e/autoscale/00-install.yaml b/tests/e2e/autoscale/00-install.yaml new file mode 100644 index 0000000000..1802f32ebb --- /dev/null +++ b/tests/e2e/autoscale/00-install.yaml @@ -0,0 +1,34 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest +spec: + replicas: 1 + autoscale: + minReplicas: 2 + maxReplicas: 10 + 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] \ No newline at end of file diff --git a/tests/e2e/autoscale/01-assert.yaml b/tests/e2e/autoscale/01-assert.yaml new file mode 100644 index 0000000000..caedff8257 --- /dev/null +++ b/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: 10 \ No newline at end of file diff --git a/tests/e2e/autoscale/01-install.yaml b/tests/e2e/autoscale/01-install.yaml new file mode 100644 index 0000000000..833ba51c71 --- /dev/null +++ b/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=10 + restartPolicy: Never + backoffLimit: 4 \ No newline at end of file From f088feaf1bdcc66e8cd8b5c70f4ad70adb93aee5 Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Wed, 29 Jun 2022 10:18:36 +0200 Subject: [PATCH 06/28] Added a period to a comment to make the linter stop complaining Signed-off-by: Kevin Earls --- apis/v1alpha1/opentelemetrycollector_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index a14f7c1248..fbe6e1952e 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -19,7 +19,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// AutoScaleSpec defines the common elements used for create HPAs +// AutoScaleSpec defines the common elements used for create HPAs. type AutoScaleSpec struct { // MinReplicas sets a lower bound to the autoscaling feature. // +optional From 34f44f73f575ff3bceb8a5695a8dfd8f76552bca Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Wed, 29 Jun 2022 10:29:49 +0200 Subject: [PATCH 07/28] Reran make generate after linting Signed-off-by: Kevin Earls --- .../manifests/opentelemetry.io_opentelemetrycollectors.yaml | 2 +- .../crd/bases/opentelemetry.io_opentelemetrycollectors.yaml | 2 +- docs/api.md | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index 50323d4585..8135717960 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -61,7 +61,7 @@ spec: type: object autoscale: description: AutoScaleSpec defines the common elements used for create - HPAs + HPAs. properties: maxReplicas: description: MaxReplicas sets an upper bound to the autoscaling diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index c3d51f3441..838d6d3dec 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -59,7 +59,7 @@ spec: type: object autoscale: description: AutoScaleSpec defines the common elements used for create - HPAs + HPAs. properties: maxReplicas: description: MaxReplicas sets an upper bound to the autoscaling diff --git a/docs/api.md b/docs/api.md index 3ffab9ab7f..f32c53790e 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1408,7 +1408,7 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. autoscale object - AutoScaleSpec defines the common elements used for create HPAs
+ AutoScaleSpec defines the common elements used for create HPAs.
false @@ -1573,7 +1573,7 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. -AutoScaleSpec defines the common elements used for create HPAs +AutoScaleSpec defines the common elements used for create HPAs. From 4974844a0d7405c29ce147959ec8027f59abab97 Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Wed, 29 Jun 2022 10:46:58 +0200 Subject: [PATCH 08/28] Fix autoscale test Signed-off-by: Kevin Earls --- tests/e2e/autoscale/00-install.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/autoscale/00-install.yaml b/tests/e2e/autoscale/00-install.yaml index 1802f32ebb..0ed42b342c 100644 --- a/tests/e2e/autoscale/00-install.yaml +++ b/tests/e2e/autoscale/00-install.yaml @@ -3,7 +3,7 @@ kind: OpenTelemetryCollector metadata: name: simplest spec: - replicas: 1 + replicas: 2 autoscale: minReplicas: 2 maxReplicas: 10 From 88575f64f2a6a60fe2dc57fe42f90375cdcfec50 Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Thu, 30 Jun 2022 10:14:18 +0200 Subject: [PATCH 09/28] Updated to autoscaling/v2beta2 as v1 is not available in k8s 1.24 Signed-off-by: Kevin Earls --- .../opentelemetrycollector_controller.go | 5 +-- pkg/collector/horizontalpodautoscaler.go | 31 ++++++++++++++----- pkg/collector/horizontalpodautoscaler_test.go | 5 ++- .../reconcile/horizontalpodautoscaler.go | 13 ++++---- .../reconcile/horizontalpodautoscaler_test.go | 13 ++++---- 5 files changed, 44 insertions(+), 23 deletions(-) diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go index 5c5f799c55..82ce49679f 100644 --- a/controllers/opentelemetrycollector_controller.go +++ b/controllers/opentelemetrycollector_controller.go @@ -21,7 +21,8 @@ import ( "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" - autoscalingv1 "k8s.io/api/autoscaling/v1" + autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -179,7 +180,7 @@ func (r *OpenTelemetryCollectorReconciler) SetupWithManager(mgr ctrl.Manager) er Owns(&corev1.ServiceAccount{}). Owns(&corev1.Service{}). Owns(&appsv1.Deployment{}). - Owns(&autoscalingv1.HorizontalPodAutoscaler{}). + Owns(&autoscalingv2beta2.HorizontalPodAutoscaler{}). Owns(&appsv1.DaemonSet{}). Owns(&appsv1.StatefulSet{}). Complete(r) diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go index d8000a6526..cc7851cf78 100644 --- a/pkg/collector/horizontalpodautoscaler.go +++ b/pkg/collector/horizontalpodautoscaler.go @@ -16,7 +16,8 @@ package collector import ( "github.com/go-logr/logr" - autoscalingv1 "k8s.io/api/autoscaling/v1" + autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" @@ -26,30 +27,44 @@ import ( const defaultCPUTarget int32 = 90 -func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) autoscalingv1.HorizontalPodAutoscaler { +func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) autoscalingv2beta2.HorizontalPodAutoscaler { labels := Labels(otelcol, cfg.LabelsFilter()) labels["app.kubernetes.io/name"] = naming.Collector(otelcol) annotations := Annotations(otelcol) cpuTarget := defaultCPUTarget - return autoscalingv1.HorizontalPodAutoscaler{ + targetCPUUtilization := autoscalingv2beta2.MetricSpec{ + Type: autoscalingv2beta2.ResourceMetricSourceType, + Resource: &autoscalingv2beta2.ResourceMetricSource{ + Name: corev1.ResourceCPU, + Target: autoscalingv2beta2.MetricTarget{ + Type: autoscalingv2beta2.UtilizationMetricType, + AverageUtilization: &cpuTarget, + }, + }, + ContainerResource: nil, + External: nil, + } + metrics := []autoscalingv2beta2.MetricSpec{targetCPUUtilization} + + return autoscalingv2beta2.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ Name: naming.HorizontalPodAutoscaler(otelcol), Namespace: otelcol.Namespace, Labels: labels, Annotations: annotations, }, - Spec: autoscalingv1.HorizontalPodAutoscalerSpec{ - ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ + Spec: autoscalingv2beta2.HorizontalPodAutoscalerSpec{ + ScaleTargetRef: autoscalingv2beta2.CrossVersionObjectReference{ APIVersion: v1alpha1.GroupVersion.String(), Kind: "OpenTelemetryCollector", Name: naming.OpenTelemetryCollector(otelcol), }, - MinReplicas: otelcol.Spec.AutoScaleSpec.MinReplicas, - MaxReplicas: *otelcol.Spec.AutoScaleSpec.MaxReplicas, - TargetCPUUtilizationPercentage: &cpuTarget, + MinReplicas: otelcol.Spec.AutoScaleSpec.MinReplicas, + MaxReplicas: *otelcol.Spec.AutoScaleSpec.MaxReplicas, + Metrics: metrics, }, } } diff --git a/pkg/collector/horizontalpodautoscaler_test.go b/pkg/collector/horizontalpodautoscaler_test.go index efe6e87f61..8fdb2e1868 100644 --- a/pkg/collector/horizontalpodautoscaler_test.go +++ b/pkg/collector/horizontalpodautoscaler_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" @@ -51,5 +52,7 @@ func TestHPA(t *testing.T) { assert.Equal(t, "my-instance-collector", hpa.Labels["app.kubernetes.io/name"]) assert.Equal(t, int32(3), *hpa.Spec.MinReplicas) assert.Equal(t, int32(5), hpa.Spec.MaxReplicas) - assert.Equal(t, int32(90), *hpa.Spec.TargetCPUUtilizationPercentage) + assert.Equal(t, 1, len(hpa.Spec.Metrics)) + assert.Equal(t, corev1.ResourceCPU, hpa.Spec.Metrics[0].Resource.Name) + assert.Equal(t, int32(90), *hpa.Spec.Metrics[0].Resource.Target.AverageUtilization) } diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 4354efb61e..f56d5883ac 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -18,7 +18,8 @@ import ( "context" "fmt" - autoscalingv1 "k8s.io/api/autoscaling/v1" + autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -31,7 +32,7 @@ import ( // HorizontalPodAutoscaler reconciles HorizontalPodAutoscalers if autoscale is true and replicas is nil. func HorizontalPodAutoscalers(ctx context.Context, params Params) error { - desired := []autoscalingv1.HorizontalPodAutoscaler{} + desired := []autoscalingv2beta2.HorizontalPodAutoscaler{} // check if autoscale mode is on, e.g MaxReplicas is not nil if params.Instance.Spec.AutoScaleSpec.MaxReplicas != nil { @@ -51,7 +52,7 @@ func HorizontalPodAutoscalers(ctx context.Context, params Params) error { return nil } -func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv1.HorizontalPodAutoscaler) error { +func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv2beta2.HorizontalPodAutoscaler) error { for _, obj := range expected { desired := obj @@ -59,7 +60,7 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect return fmt.Errorf("failed to set controller reference: %w", err) } - existing := &autoscalingv1.HorizontalPodAutoscaler{} + existing := &autoscalingv2beta2.HorizontalPodAutoscaler{} nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} err := params.Client.Get(ctx, nns, existing) if k8serrors.IsNotFound(err) { @@ -105,7 +106,7 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect return nil } -func deleteHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv1.HorizontalPodAutoscaler) error { +func deleteHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv2beta2.HorizontalPodAutoscaler) error { opts := []client.ListOption{ client.InNamespace(params.Instance.Namespace), client.MatchingLabels(map[string]string{ @@ -114,7 +115,7 @@ func deleteHorizontalPodAutoscalers(ctx context.Context, params Params, expected }), } - list := &autoscalingv1.HorizontalPodAutoscalerList{} + list := &autoscalingv2beta2.HorizontalPodAutoscalerList{} if err := params.Client.List(ctx, list, opts...); err != nil { return fmt.Errorf("failed to list: %w", err) } diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index b912899e3f..bca4a7aa46 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -22,7 +22,8 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/apps/v1" - autoscalingv1 "k8s.io/api/autoscaling/v1" + autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -39,10 +40,10 @@ func TestExpectedHPA(t *testing.T) { expectedHPA := collector.HorizontalPodAutoscaler(params.Config, logger, params.Instance) t.Run("should create HPA", func(t *testing.T) { - err := expectedHorizontalPodAutoscalers(context.Background(), params, []autoscalingv1.HorizontalPodAutoscaler{expectedHPA}) + err := expectedHorizontalPodAutoscalers(context.Background(), params, []autoscalingv2beta2.HorizontalPodAutoscaler{expectedHPA}) assert.NoError(t, err) - exists, err := populateObjectIfExists(t, &autoscalingv1.HorizontalPodAutoscaler{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) + exists, err := populateObjectIfExists(t, &autoscalingv2beta2.HorizontalPodAutoscaler{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) assert.NoError(t, err) assert.True(t, exists) }) @@ -57,10 +58,10 @@ func TestExpectedHPA(t *testing.T) { updatedHPA := collector.HorizontalPodAutoscaler(updateParms.Config, logger, updateParms.Instance) createObjectIfNotExists(t, "test-collector", &updatedHPA) - err := expectedHorizontalPodAutoscalers(context.Background(), updateParms, []autoscalingv1.HorizontalPodAutoscaler{updatedHPA}) + err := expectedHorizontalPodAutoscalers(context.Background(), updateParms, []autoscalingv2beta2.HorizontalPodAutoscaler{updatedHPA}) assert.NoError(t, err) - actual := autoscalingv1.HorizontalPodAutoscaler{} + actual := autoscalingv2beta2.HorizontalPodAutoscaler{} exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collector"}) assert.NoError(t, err) @@ -70,7 +71,7 @@ func TestExpectedHPA(t *testing.T) { }) t.Run("should delete HPA", func(t *testing.T) { - err := deleteHorizontalPodAutoscalers(context.Background(), params, []autoscalingv1.HorizontalPodAutoscaler{expectedHPA}) + err := deleteHorizontalPodAutoscalers(context.Background(), params, []autoscalingv2beta2.HorizontalPodAutoscaler{expectedHPA}) assert.NoError(t, err) actual := v1.Deployment{} From 1d45856c615eff3980e33ffe33a9e3d17e408e5a Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Fri, 1 Jul 2022 16:57:22 +0200 Subject: [PATCH 10/28] Install metrics-server on kind clusters for autoscale tests Signed-off-by: Kevin Earls --- Makefile | 6 +++++- hack/install-metrics-server.sh | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100755 hack/install-metrics-server.sh diff --git a/Makefile b/Makefile index a720c4a672..f6a36abd60 100644 --- a/Makefile +++ b/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 diff --git a/hack/install-metrics-server.sh b/hack/install-metrics-server.sh new file mode 100755 index 0000000000..03a11cafdf --- /dev/null +++ b/hack/install-metrics-server.sh @@ -0,0 +1,6 @@ +#!/bin/bash + +# Install metrics-server for autoscale tests +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 \ No newline at end of file From dc55641abb5dd7de98e5f324d7c1809cb2769032 Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Fri, 1 Jul 2022 17:33:27 +0200 Subject: [PATCH 11/28] Fix more lint complaints Signed-off-by: Kevin Earls --- pkg/collector/reconcile/deployment_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index 73a37ad2e7..8cb39c8d37 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -117,9 +117,12 @@ func TestExpectedDeployments(t *testing.T) { UID: instanceUID, }, Spec: v1alpha1.OpenTelemetryCollectorSpec{ - MaxReplicas: &maxReplicas, - Replicas: &replicas, - Mode: v1alpha1.ModeStatefulSet, + AutoScaleSpec: v1alpha1.AutoScaleSpec{ + MinReplicas: &replicas, + MaxReplicas: &maxReplicas, + }, + Replicas: &replicas, + Mode: v1alpha1.ModeStatefulSet, TargetAllocator: v1alpha1.OpenTelemetryTargetAllocator{ Enabled: true, }, From 36d2dda6412217e11038465f6e256fdd4085085a Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Fri, 1 Jul 2022 17:59:20 +0200 Subject: [PATCH 12/28] Only scale up to 5 replicas in test Signed-off-by: Kevin Earls --- tests/e2e/autoscale/00-install.yaml | 2 +- tests/e2e/autoscale/01-assert.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/e2e/autoscale/00-install.yaml b/tests/e2e/autoscale/00-install.yaml index 0ed42b342c..bb44bdb7bd 100644 --- a/tests/e2e/autoscale/00-install.yaml +++ b/tests/e2e/autoscale/00-install.yaml @@ -6,7 +6,7 @@ spec: replicas: 2 autoscale: minReplicas: 2 - maxReplicas: 10 + maxReplicas: 5 resources: limits: cpu: 500m diff --git a/tests/e2e/autoscale/01-assert.yaml b/tests/e2e/autoscale/01-assert.yaml index caedff8257..bf25ef1024 100644 --- a/tests/e2e/autoscale/01-assert.yaml +++ b/tests/e2e/autoscale/01-assert.yaml @@ -15,4 +15,4 @@ metadata: name: simplest status: scale: - replicas: 10 \ No newline at end of file + replicas: 5 \ No newline at end of file From 41b581ab2089f5b1044b9f0d70fbc839130c35bb Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Fri, 1 Jul 2022 18:19:26 +0200 Subject: [PATCH 13/28] Fix test Signed-off-by: Kevin Earls --- tests/e2e/autoscale/00-assert.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/autoscale/00-assert.yaml b/tests/e2e/autoscale/00-assert.yaml index e1397797b5..87a1eabf19 100644 --- a/tests/e2e/autoscale/00-assert.yaml +++ b/tests/e2e/autoscale/00-assert.yaml @@ -12,5 +12,5 @@ metadata: name: simplest-collector spec: minReplicas: 2 - maxReplicas: 10 + maxReplicas: 5 From c22f6b4a64efa2caf30dffeac1bf8af9b8a105fc Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Mon, 4 Jul 2022 14:06:28 +0200 Subject: [PATCH 14/28] Update test so it will work correctly in CI Signed-off-by: Kevin Earls --- tests/e2e/autoscale/00-assert.yaml | 6 +++--- tests/e2e/autoscale/00-install.yaml | 6 +++--- tests/e2e/autoscale/01-assert.yaml | 2 +- tests/e2e/autoscale/01-install.yaml | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/e2e/autoscale/00-assert.yaml b/tests/e2e/autoscale/00-assert.yaml index 87a1eabf19..d43768735b 100644 --- a/tests/e2e/autoscale/00-assert.yaml +++ b/tests/e2e/autoscale/00-assert.yaml @@ -3,7 +3,7 @@ kind: Deployment metadata: name: simplest-collector status: - readyReplicas: 2 + readyReplicas: 1 --- apiVersion: autoscaling/v1 @@ -11,6 +11,6 @@ kind: HorizontalPodAutoscaler metadata: name: simplest-collector spec: - minReplicas: 2 - maxReplicas: 5 + minReplicas: 1 + maxReplicas: 4 diff --git a/tests/e2e/autoscale/00-install.yaml b/tests/e2e/autoscale/00-install.yaml index bb44bdb7bd..e41c6e2f8b 100644 --- a/tests/e2e/autoscale/00-install.yaml +++ b/tests/e2e/autoscale/00-install.yaml @@ -3,10 +3,10 @@ kind: OpenTelemetryCollector metadata: name: simplest spec: - replicas: 2 + replicas: 1 autoscale: - minReplicas: 2 - maxReplicas: 5 + minReplicas: 1 + maxReplicas: 4 resources: limits: cpu: 500m diff --git a/tests/e2e/autoscale/01-assert.yaml b/tests/e2e/autoscale/01-assert.yaml index bf25ef1024..93c918136c 100644 --- a/tests/e2e/autoscale/01-assert.yaml +++ b/tests/e2e/autoscale/01-assert.yaml @@ -15,4 +15,4 @@ metadata: name: simplest status: scale: - replicas: 5 \ No newline at end of file + replicas: 4 \ No newline at end of file diff --git a/tests/e2e/autoscale/01-install.yaml b/tests/e2e/autoscale/01-install.yaml index 833ba51c71..ee860cecd0 100644 --- a/tests/e2e/autoscale/01-install.yaml +++ b/tests/e2e/autoscale/01-install.yaml @@ -14,6 +14,6 @@ spec: - -otlp-endpoint=simplest-collector-headless:4317 - -otlp-insecure - -duration=1m - - -workers=10 + - -workers=20 restartPolicy: Never backoffLimit: 4 \ No newline at end of file From b4a72d153020eb565a813b2d92452c436b384bb5 Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Mon, 4 Jul 2022 18:00:43 +0200 Subject: [PATCH 15/28] Revert to autoscaling v1 Signed-off-by: Kevin Earls --- .../opentelemetrycollector_controller.go | 4 +-- pkg/collector/horizontalpodautoscaler.go | 31 +++++-------------- pkg/collector/horizontalpodautoscaler_test.go | 5 +-- .../reconcile/horizontalpodautoscaler.go | 12 +++---- .../reconcile/horizontalpodautoscaler_test.go | 12 +++---- 5 files changed, 23 insertions(+), 41 deletions(-) diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go index 82ce49679f..20f5324b3f 100644 --- a/controllers/opentelemetrycollector_controller.go +++ b/controllers/opentelemetrycollector_controller.go @@ -21,7 +21,7 @@ import ( "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" - autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" + autoscalingv1 "k8s.io/api/autoscaling/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -180,7 +180,7 @@ func (r *OpenTelemetryCollectorReconciler) SetupWithManager(mgr ctrl.Manager) er Owns(&corev1.ServiceAccount{}). Owns(&corev1.Service{}). Owns(&appsv1.Deployment{}). - Owns(&autoscalingv2beta2.HorizontalPodAutoscaler{}). + Owns(&autoscalingv1.HorizontalPodAutoscaler{}). Owns(&appsv1.DaemonSet{}). Owns(&appsv1.StatefulSet{}). Complete(r) diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go index cc7851cf78..d8000a6526 100644 --- a/pkg/collector/horizontalpodautoscaler.go +++ b/pkg/collector/horizontalpodautoscaler.go @@ -16,8 +16,7 @@ package collector import ( "github.com/go-logr/logr" - autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" - corev1 "k8s.io/api/core/v1" + autoscalingv1 "k8s.io/api/autoscaling/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" @@ -27,44 +26,30 @@ import ( const defaultCPUTarget int32 = 90 -func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) autoscalingv2beta2.HorizontalPodAutoscaler { +func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) autoscalingv1.HorizontalPodAutoscaler { labels := Labels(otelcol, cfg.LabelsFilter()) labels["app.kubernetes.io/name"] = naming.Collector(otelcol) annotations := Annotations(otelcol) cpuTarget := defaultCPUTarget - targetCPUUtilization := autoscalingv2beta2.MetricSpec{ - Type: autoscalingv2beta2.ResourceMetricSourceType, - Resource: &autoscalingv2beta2.ResourceMetricSource{ - Name: corev1.ResourceCPU, - Target: autoscalingv2beta2.MetricTarget{ - Type: autoscalingv2beta2.UtilizationMetricType, - AverageUtilization: &cpuTarget, - }, - }, - ContainerResource: nil, - External: nil, - } - metrics := []autoscalingv2beta2.MetricSpec{targetCPUUtilization} - - return autoscalingv2beta2.HorizontalPodAutoscaler{ + return autoscalingv1.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ Name: naming.HorizontalPodAutoscaler(otelcol), Namespace: otelcol.Namespace, Labels: labels, Annotations: annotations, }, - Spec: autoscalingv2beta2.HorizontalPodAutoscalerSpec{ - ScaleTargetRef: autoscalingv2beta2.CrossVersionObjectReference{ + Spec: autoscalingv1.HorizontalPodAutoscalerSpec{ + ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ APIVersion: v1alpha1.GroupVersion.String(), Kind: "OpenTelemetryCollector", Name: naming.OpenTelemetryCollector(otelcol), }, - MinReplicas: otelcol.Spec.AutoScaleSpec.MinReplicas, - MaxReplicas: *otelcol.Spec.AutoScaleSpec.MaxReplicas, - Metrics: metrics, + MinReplicas: otelcol.Spec.AutoScaleSpec.MinReplicas, + MaxReplicas: *otelcol.Spec.AutoScaleSpec.MaxReplicas, + TargetCPUUtilizationPercentage: &cpuTarget, }, } } diff --git a/pkg/collector/horizontalpodautoscaler_test.go b/pkg/collector/horizontalpodautoscaler_test.go index 8fdb2e1868..efe6e87f61 100644 --- a/pkg/collector/horizontalpodautoscaler_test.go +++ b/pkg/collector/horizontalpodautoscaler_test.go @@ -18,7 +18,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" @@ -52,7 +51,5 @@ func TestHPA(t *testing.T) { assert.Equal(t, "my-instance-collector", hpa.Labels["app.kubernetes.io/name"]) assert.Equal(t, int32(3), *hpa.Spec.MinReplicas) assert.Equal(t, int32(5), hpa.Spec.MaxReplicas) - assert.Equal(t, 1, len(hpa.Spec.Metrics)) - assert.Equal(t, corev1.ResourceCPU, hpa.Spec.Metrics[0].Resource.Name) - assert.Equal(t, int32(90), *hpa.Spec.Metrics[0].Resource.Target.AverageUtilization) + assert.Equal(t, int32(90), *hpa.Spec.TargetCPUUtilizationPercentage) } diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index f56d5883ac..61301b7984 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -18,7 +18,7 @@ import ( "context" "fmt" - autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" + autoscalingv1 "k8s.io/api/autoscaling/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -32,7 +32,7 @@ import ( // HorizontalPodAutoscaler reconciles HorizontalPodAutoscalers if autoscale is true and replicas is nil. func HorizontalPodAutoscalers(ctx context.Context, params Params) error { - desired := []autoscalingv2beta2.HorizontalPodAutoscaler{} + desired := []autoscalingv1.HorizontalPodAutoscaler{} // check if autoscale mode is on, e.g MaxReplicas is not nil if params.Instance.Spec.AutoScaleSpec.MaxReplicas != nil { @@ -52,7 +52,7 @@ func HorizontalPodAutoscalers(ctx context.Context, params Params) error { return nil } -func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv2beta2.HorizontalPodAutoscaler) error { +func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv1.HorizontalPodAutoscaler) error { for _, obj := range expected { desired := obj @@ -60,7 +60,7 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect return fmt.Errorf("failed to set controller reference: %w", err) } - existing := &autoscalingv2beta2.HorizontalPodAutoscaler{} + existing := &autoscalingv1.HorizontalPodAutoscaler{} nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} err := params.Client.Get(ctx, nns, existing) if k8serrors.IsNotFound(err) { @@ -106,7 +106,7 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect return nil } -func deleteHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv2beta2.HorizontalPodAutoscaler) error { +func deleteHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv1.HorizontalPodAutoscaler) error { opts := []client.ListOption{ client.InNamespace(params.Instance.Namespace), client.MatchingLabels(map[string]string{ @@ -115,7 +115,7 @@ func deleteHorizontalPodAutoscalers(ctx context.Context, params Params, expected }), } - list := &autoscalingv2beta2.HorizontalPodAutoscalerList{} + list := &autoscalingv1.HorizontalPodAutoscalerList{} if err := params.Client.List(ctx, list, opts...); err != nil { return fmt.Errorf("failed to list: %w", err) } diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index bca4a7aa46..a10817108f 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -22,7 +22,7 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/apps/v1" - autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" + autoscalingv1 "k8s.io/api/autoscaling/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -40,10 +40,10 @@ func TestExpectedHPA(t *testing.T) { expectedHPA := collector.HorizontalPodAutoscaler(params.Config, logger, params.Instance) t.Run("should create HPA", func(t *testing.T) { - err := expectedHorizontalPodAutoscalers(context.Background(), params, []autoscalingv2beta2.HorizontalPodAutoscaler{expectedHPA}) + err := expectedHorizontalPodAutoscalers(context.Background(), params, []autoscalingv1.HorizontalPodAutoscaler{expectedHPA}) assert.NoError(t, err) - exists, err := populateObjectIfExists(t, &autoscalingv2beta2.HorizontalPodAutoscaler{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) + exists, err := populateObjectIfExists(t, &autoscalingv1.HorizontalPodAutoscaler{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) assert.NoError(t, err) assert.True(t, exists) }) @@ -58,10 +58,10 @@ func TestExpectedHPA(t *testing.T) { updatedHPA := collector.HorizontalPodAutoscaler(updateParms.Config, logger, updateParms.Instance) createObjectIfNotExists(t, "test-collector", &updatedHPA) - err := expectedHorizontalPodAutoscalers(context.Background(), updateParms, []autoscalingv2beta2.HorizontalPodAutoscaler{updatedHPA}) + err := expectedHorizontalPodAutoscalers(context.Background(), updateParms, []autoscalingv1.HorizontalPodAutoscaler{updatedHPA}) assert.NoError(t, err) - actual := autoscalingv2beta2.HorizontalPodAutoscaler{} + actual := autoscalingv1.HorizontalPodAutoscaler{} exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collector"}) assert.NoError(t, err) @@ -71,7 +71,7 @@ func TestExpectedHPA(t *testing.T) { }) t.Run("should delete HPA", func(t *testing.T) { - err := deleteHorizontalPodAutoscalers(context.Background(), params, []autoscalingv2beta2.HorizontalPodAutoscaler{expectedHPA}) + err := deleteHorizontalPodAutoscalers(context.Background(), params, []autoscalingv1.HorizontalPodAutoscaler{expectedHPA}) assert.NoError(t, err) actual := v1.Deployment{} From 33d9c48a8bf3769cb558e8751d179da407cf6dfb Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Wed, 6 Jul 2022 10:15:31 +0200 Subject: [PATCH 16/28] Reduce maxReplicas count to get kuttl tests to work in CI Signed-off-by: Kevin Earls --- tests/e2e/autoscale/00-assert.yaml | 2 +- tests/e2e/autoscale/00-install.yaml | 2 +- tests/e2e/autoscale/01-assert.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/e2e/autoscale/00-assert.yaml b/tests/e2e/autoscale/00-assert.yaml index d43768735b..ef4b828699 100644 --- a/tests/e2e/autoscale/00-assert.yaml +++ b/tests/e2e/autoscale/00-assert.yaml @@ -12,5 +12,5 @@ metadata: name: simplest-collector spec: minReplicas: 1 - maxReplicas: 4 + maxReplicas: 2 diff --git a/tests/e2e/autoscale/00-install.yaml b/tests/e2e/autoscale/00-install.yaml index e41c6e2f8b..be1e3df68a 100644 --- a/tests/e2e/autoscale/00-install.yaml +++ b/tests/e2e/autoscale/00-install.yaml @@ -6,7 +6,7 @@ spec: replicas: 1 autoscale: minReplicas: 1 - maxReplicas: 4 + maxReplicas: 2 resources: limits: cpu: 500m diff --git a/tests/e2e/autoscale/01-assert.yaml b/tests/e2e/autoscale/01-assert.yaml index 93c918136c..6dc44aeb28 100644 --- a/tests/e2e/autoscale/01-assert.yaml +++ b/tests/e2e/autoscale/01-assert.yaml @@ -15,4 +15,4 @@ metadata: name: simplest status: scale: - replicas: 4 \ No newline at end of file + replicas: 2 \ No newline at end of file From 2f5ade53a97629881dd729ab14ac0d4f2ef74e5e Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Wed, 6 Jul 2022 11:08:05 +0200 Subject: [PATCH 17/28] Back out addition of AutoScaleSpec to CRD Signed-off-by: Kevin Earls --- apis/v1alpha1/opentelemetrycollector_types.go | 14 +---- .../opentelemetrycollector_webhook.go | 11 +--- apis/v1alpha1/zz_generated.deepcopy.go | 31 ++-------- ...ntelemetry.io_opentelemetrycollectors.yaml | 20 ++---- ...ntelemetry.io_opentelemetrycollectors.yaml | 20 ++---- pkg/collector/horizontalpodautoscaler.go | 4 +- pkg/collector/horizontalpodautoscaler_test.go | 7 +-- pkg/collector/reconcile/deployment.go | 8 +-- pkg/collector/reconcile/deployment_test.go | 61 +------------------ .../reconcile/horizontalpodautoscaler.go | 6 +- .../reconcile/horizontalpodautoscaler_test.go | 11 ++-- tests/e2e/autoscale/00-install.yaml | 4 +- 12 files changed, 38 insertions(+), 159 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index fbe6e1952e..25a0e60db7 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -19,17 +19,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// AutoScaleSpec defines the common elements used for create HPAs. -type AutoScaleSpec struct { - // MinReplicas sets a lower bound to the autoscaling feature. - // +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"` -} - // OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. type OpenTelemetryCollectorSpec struct { // Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation for details. @@ -48,8 +37,9 @@ type OpenTelemetryCollectorSpec struct { // +optional Replicas *int32 `json:"replicas,omitempty"` + // MaxReplicas sets an upper bound to the autoscaling feature. If MaxReplicas is set autoscaling is enabled. // +optional - AutoScaleSpec `json:"autoscale,omitempty"` + MaxReplicas *int32 `json:"maxReplicas,omitempty"` // ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) // +optional diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index db42d94b7e..f4a6b4f068 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -112,19 +112,14 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error { } // validate autoscale with horizontal pod autoscaler - if r.Spec.AutoScaleSpec.MaxReplicas != nil { - if *r.Spec.AutoScaleSpec.MaxReplicas < int32(1) { + if r.Spec.MaxReplicas != nil { + if *r.Spec.MaxReplicas < int32(1) { return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, maxReplicas should be defined and more than one") } - if r.Spec.Replicas != nil && *r.Spec.Replicas > *r.Spec.AutoScaleSpec.MaxReplicas { + 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.Replicas != nil && *r.Spec.Replicas < *r.Spec.AutoScaleSpec.MinReplicas { - return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, replicas must not be less than minReplicas") - } - } return nil diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index a7c1140ea6..ef246bbdf0 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -24,31 +24,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *AutoScaleSpec) DeepCopyInto(out *AutoScaleSpec) { - *out = *in - if in.MinReplicas != nil { - in, out := &in.MinReplicas, &out.MinReplicas - *out = new(int32) - **out = **in - } - if in.MaxReplicas != nil { - in, out := &in.MaxReplicas, &out.MaxReplicas - *out = new(int32) - **out = **in - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AutoScaleSpec. -func (in *AutoScaleSpec) DeepCopy() *AutoScaleSpec { - if in == nil { - return nil - } - out := new(AutoScaleSpec) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Exporter) DeepCopyInto(out *Exporter) { *out = *in @@ -289,7 +264,11 @@ func (in *OpenTelemetryCollectorSpec) DeepCopyInto(out *OpenTelemetryCollectorSp *out = new(int32) **out = **in } - in.AutoScaleSpec.DeepCopyInto(&out.AutoScaleSpec) + if in.MaxReplicas != nil { + in, out := &in.MaxReplicas, &out.MaxReplicas + *out = new(int32) + **out = **in + } out.TargetAllocator = in.TargetAllocator if in.SecurityContext != nil { in, out := &in.SecurityContext, &out.SecurityContext diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index 8135717960..173ea68836 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -59,21 +59,6 @@ spec: description: Args is the set of arguments to pass to the OpenTelemetry Collector binary type: object - autoscale: - description: AutoScaleSpec defines the common elements used for create - HPAs. - properties: - maxReplicas: - description: MaxReplicas sets an upper bound to the autoscaling - feature. If MaxReplicas is set autoscaling is enabled. - format: int32 - type: integer - minReplicas: - description: MinReplicas sets a lower bound to the autoscaling - feature. - format: int32 - type: integer - type: object config: description: Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation @@ -233,6 +218,11 @@ spec: description: ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) type: string + maxReplicas: + description: MaxReplicas sets an upper bound to the autoscaling feature. + If MaxReplicas is set autoscaling is enabled. + format: int32 + type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 838d6d3dec..60335aeff5 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -57,21 +57,6 @@ spec: description: Args is the set of arguments to pass to the OpenTelemetry Collector binary type: object - autoscale: - description: AutoScaleSpec defines the common elements used for create - HPAs. - properties: - maxReplicas: - description: MaxReplicas sets an upper bound to the autoscaling - feature. If MaxReplicas is set autoscaling is enabled. - format: int32 - type: integer - minReplicas: - description: MinReplicas sets a lower bound to the autoscaling - feature. - format: int32 - type: integer - type: object config: description: Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation @@ -231,6 +216,11 @@ spec: description: ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) type: string + maxReplicas: + description: MaxReplicas sets an upper bound to the autoscaling feature. + If MaxReplicas is set autoscaling is enabled. + format: int32 + type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go index d8000a6526..7c7003fb48 100644 --- a/pkg/collector/horizontalpodautoscaler.go +++ b/pkg/collector/horizontalpodautoscaler.go @@ -47,8 +47,8 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al Name: naming.OpenTelemetryCollector(otelcol), }, - MinReplicas: otelcol.Spec.AutoScaleSpec.MinReplicas, - MaxReplicas: *otelcol.Spec.AutoScaleSpec.MaxReplicas, + MinReplicas: otelcol.Spec.Replicas, + MaxReplicas: *otelcol.Spec.MaxReplicas, TargetCPUUtilizationPercentage: &cpuTarget, }, } diff --git a/pkg/collector/horizontalpodautoscaler_test.go b/pkg/collector/horizontalpodautoscaler_test.go index efe6e87f61..b168816549 100644 --- a/pkg/collector/horizontalpodautoscaler_test.go +++ b/pkg/collector/horizontalpodautoscaler_test.go @@ -35,11 +35,8 @@ func TestHPA(t *testing.T) { Name: "my-instance", }, Spec: v1alpha1.OpenTelemetryCollectorSpec{ - Replicas: &minReplicas, - AutoScaleSpec: v1alpha1.AutoScaleSpec{ - MinReplicas: &minReplicas, - MaxReplicas: &maxReplicas, - }, + Replicas: &minReplicas, + MaxReplicas: &maxReplicas, }, } diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index b0bbd90d93..99aa3a5a31 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/pkg/collector/reconcile/deployment.go @@ -150,12 +150,12 @@ func deleteDeployments(ctx context.Context, params Params, expected []appsv1.Dep // currentReplicasWithHPA calculates deployment replicas if HPA is enabled. func currentReplicasWithHPA(spec v1alpha1.OpenTelemetryCollectorSpec, curr int32) int32 { - if curr < *spec.AutoScaleSpec.MinReplicas { - return *spec.AutoScaleSpec.MinReplicas + if curr < *spec.Replicas { + return *spec.Replicas } - if curr > *spec.AutoScaleSpec.MaxReplicas { - return *spec.AutoScaleSpec.MaxReplicas + if curr > *spec.MaxReplicas { + return *spec.MaxReplicas } return curr diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index 8cb39c8d37..5a513d098b 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -102,61 +102,6 @@ func TestExpectedDeployments(t *testing.T) { assert.Len(t, expected, 0) }) - t.Run("should not update target allocator deployment replicas when collector max replicas is set", func(t *testing.T) { - replicas, maxReplicas := int32(2), int32(10) - param := Params{ - Client: k8sClient, - Instance: v1alpha1.OpenTelemetryCollector{ - TypeMeta: metav1.TypeMeta{ - Kind: "opentelemetry.io", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - UID: instanceUID, - }, - Spec: v1alpha1.OpenTelemetryCollectorSpec{ - AutoScaleSpec: v1alpha1.AutoScaleSpec{ - MinReplicas: &replicas, - MaxReplicas: &maxReplicas, - }, - Replicas: &replicas, - Mode: v1alpha1.ModeStatefulSet, - TargetAllocator: v1alpha1.OpenTelemetryTargetAllocator{ - Enabled: true, - }, - Config: ` - receivers: - jaeger: - protocols: - grpc: - processors: - - exporters: - logging: - - service: - pipelines: - traces: - receivers: [jaeger] - processors: [] - exporters: [logging] - - `, - }, - }, - Scheme: testScheme, - Log: logger, - } - expected := []v1.Deployment{} - allocator := targetallocator.Deployment(param.Config, param.Log, param.Instance) - expected = append(expected, allocator) - - assert.Len(t, expected, 1) - assert.Equal(t, *allocator.Spec.Replicas, int32(1)) - }) - t.Run("should update deployment", func(t *testing.T) { createObjectIfNotExists(t, "test-collector", &expectedDeploy) err := expectedDeployments(context.Background(), param, []v1.Deployment{expectedDeploy}) @@ -294,10 +239,8 @@ func TestCurrentReplicasWithHPA(t *testing.T) { minReplicas := int32(2) maxReplicas := int32(5) spec := v1alpha1.OpenTelemetryCollectorSpec{ - AutoScaleSpec: v1alpha1.AutoScaleSpec{ - MinReplicas: &minReplicas, - MaxReplicas: &maxReplicas, - }, + Replicas: &minReplicas, + MaxReplicas: &maxReplicas, } res := currentReplicasWithHPA(spec, 10) diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 61301b7984..eef5a7e022 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -35,7 +35,7 @@ func HorizontalPodAutoscalers(ctx context.Context, params Params) error { desired := []autoscalingv1.HorizontalPodAutoscaler{} // check if autoscale mode is on, e.g MaxReplicas is not nil - if params.Instance.Spec.AutoScaleSpec.MaxReplicas != nil { + if params.Instance.Spec.MaxReplicas != nil { desired = append(desired, collector.HorizontalPodAutoscaler(params.Config, params.Log, params.Instance)) } @@ -82,9 +82,9 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect } updated.OwnerReferences = desired.OwnerReferences - updated.Spec.MinReplicas = params.Instance.Spec.AutoScaleSpec.MinReplicas + updated.Spec.MinReplicas = params.Instance.Spec.Replicas if params.Instance.Spec.MaxReplicas != nil { - updated.Spec.MaxReplicas = *params.Instance.Spec.AutoScaleSpec.MaxReplicas + updated.Spec.MaxReplicas = *params.Instance.Spec.MaxReplicas } for k, v := range desired.Annotations { diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index a10817108f..4ca6b2d286 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -53,7 +53,7 @@ func TestExpectedHPA(t *testing.T) { maxReplicas := int32(3) updateParms := paramsWithHPA() updateParms.Instance.Spec.Replicas = &minReplicas - updateParms.Instance.Spec.MinReplicas = &minReplicas + updateParms.Instance.Spec.Replicas = &minReplicas updateParms.Instance.Spec.MaxReplicas = &maxReplicas updatedHPA := collector.HorizontalPodAutoscaler(updateParms.Config, logger, updateParms.Instance) @@ -112,12 +112,9 @@ func paramsWithHPA() Params { }, NodePort: 0, }}, - Config: string(configYAML), - Replicas: &minReplicas, - AutoScaleSpec: v1alpha1.AutoScaleSpec{ - MinReplicas: &minReplicas, - MaxReplicas: &maxReplicas, - }, + Config: string(configYAML), + Replicas: &minReplicas, + MaxReplicas: &maxReplicas, }, }, Scheme: testScheme, diff --git a/tests/e2e/autoscale/00-install.yaml b/tests/e2e/autoscale/00-install.yaml index be1e3df68a..feec3b6771 100644 --- a/tests/e2e/autoscale/00-install.yaml +++ b/tests/e2e/autoscale/00-install.yaml @@ -4,9 +4,7 @@ metadata: name: simplest spec: replicas: 1 - autoscale: - minReplicas: 1 - maxReplicas: 2 + maxReplicas: 2 resources: limits: cpu: 500m From 0bc348bda025961c84a3e53a3bcce5e01d01046f Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Wed, 6 Jul 2022 11:22:13 +0200 Subject: [PATCH 18/28] Fixed api-docs Signed-off-by: Kevin Earls --- docs/api.md | 54 +++++++++-------------------------------------------- 1 file changed, 9 insertions(+), 45 deletions(-) diff --git a/docs/api.md b/docs/api.md index f32c53790e..6e947a1127 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1404,13 +1404,6 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. Args is the set of arguments to pass to the OpenTelemetry Collector binary
 - - - - - @@ -1453,6 +1446,15 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent)
+ + + + + @@ -1568,44 +1570,6 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector.
false
autoscaleobject - AutoScaleSpec defines the common elements used for create HPAs.
- 		false
config string false
maxReplicasinteger + MaxReplicas sets an upper bound to the autoscaling feature. If MaxReplicas is set autoscaling is enabled.
+
+ Format: int32
+ 		false
mode enum
-### OpenTelemetryCollector.spec.autoscale -[↩ Parent](#opentelemetrycollectorspec) - - - -AutoScaleSpec defines the common elements used for create HPAs. - - - - - - - - - - - - - - - - - - - - - -
NameTypeDescriptionRequired
maxReplicasinteger - MaxReplicas sets an upper bound to the autoscaling feature. If MaxReplicas is set autoscaling is enabled.
-
- Format: int32
- 		false
minReplicasinteger - MinReplicas sets a lower bound to the autoscaling feature.
-
- Format: int32
- 		false
- - ### OpenTelemetryCollector.spec.env[index] [↩ Parent](#opentelemetrycollectorspec) From 3ce81aa43ae7b2d94e09c2b65e4b0f37a1b54476 Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Wed, 6 Jul 2022 11:43:56 +0200 Subject: [PATCH 19/28] Restore accidentally delete test, clean up whitespace Signed-off-by: Kevin Earls --- .../opentelemetrycollector_controller.go | 1 - pkg/collector/reconcile/deployment_test.go | 52 +++++++++++++++++++ .../reconcile/horizontalpodautoscaler.go | 1 - .../reconcile/horizontalpodautoscaler_test.go | 1 - 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go index 20f5324b3f..5c5f799c55 100644 --- a/controllers/opentelemetrycollector_controller.go +++ b/controllers/opentelemetrycollector_controller.go @@ -22,7 +22,6 @@ import ( "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" autoscalingv1 "k8s.io/api/autoscaling/v1" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index 5a513d098b..f1e2389e2d 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -102,6 +102,58 @@ func TestExpectedDeployments(t *testing.T) { assert.Len(t, expected, 0) }) + t.Run("should not update target allocator deployment replicas when collector max replicas is set", func(t *testing.T) { + replicas, maxReplicas := int32(2), int32(10) + param := Params{ + Client: k8sClient, + Instance: v1alpha1.OpenTelemetryCollector{ + TypeMeta: metav1.TypeMeta{ + Kind: "opentelemetry.io", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + UID: instanceUID, + }, + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + MaxReplicas: &maxReplicas, + Replicas: &replicas, + Mode: v1alpha1.ModeStatefulSet, + TargetAllocator: v1alpha1.OpenTelemetryTargetAllocator{ + Enabled: true, + }, + Config: ` + receivers: + jaeger: + protocols: + grpc: + processors: + + exporters: + logging: + + service: + pipelines: + traces: + receivers: [jaeger] + processors: [] + exporters: [logging] + + `, + }, + }, + Scheme: testScheme, + Log: logger, + } + expected := []v1.Deployment{} + allocator := targetallocator.Deployment(param.Config, param.Log, param.Instance) + expected = append(expected, allocator) + + assert.Len(t, expected, 1) + assert.Equal(t, *allocator.Spec.Replicas, int32(1)) + }) + t.Run("should update deployment", func(t *testing.T) { createObjectIfNotExists(t, "test-collector", &expectedDeploy) err := expectedDeployments(context.Background(), param, []v1.Deployment{expectedDeploy}) diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index eef5a7e022..6d0bca72a2 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -19,7 +19,6 @@ import ( "fmt" autoscalingv1 "k8s.io/api/autoscaling/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index 4ca6b2d286..4c0b9f967e 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -23,7 +23,6 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/apps/v1" autoscalingv1 "k8s.io/api/autoscaling/v1" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" From aeb4a97d200a03db739dd969b0ab4980adc3fc3c Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Thu, 7 Jul 2022 14:34:01 +0200 Subject: [PATCH 20/28] Document that this script is not needed for minikube Signed-off-by: Kevin Earls --- hack/install-metrics-server.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hack/install-metrics-server.sh b/hack/install-metrics-server.sh index 03a11cafdf..fe9c319959 100755 --- a/hack/install-metrics-server.sh +++ b/hack/install-metrics-server.sh @@ -1,6 +1,7 @@ #!/bin/bash -# Install metrics-server for autoscale tests +# 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 \ No newline at end of file From 96917a20fc870040e03ae59a119dbfae8b9aa701 Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Thu, 7 Jul 2022 16:09:36 +0200 Subject: [PATCH 21/28] Fix scaledown Signed-off-by: Kevin Earls --- apis/v1alpha1/opentelemetrycollector_types.go | 4 ++++ apis/v1alpha1/zz_generated.deepcopy.go | 5 +++++ .../opentelemetry.io_opentelemetrycollectors.yaml | 4 ++++ .../bases/opentelemetry.io_opentelemetrycollectors.yaml | 4 ++++ docs/api.md | 9 +++++++++ pkg/collector/reconcile/horizontalpodautoscaler.go | 7 ++++++- tests/e2e/autoscale/00-install.yaml | 1 + 7 files changed, 33 insertions(+), 1 deletion(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 25a0e60db7..5758d241f3 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/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. + // +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"` diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index ef246bbdf0..a1d95abb28 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -264,6 +264,11 @@ func (in *OpenTelemetryCollectorSpec) DeepCopyInto(out *OpenTelemetryCollectorSp *out = new(int32) **out = **in } + if in.MinReplicas != nil { + in, out := &in.MinReplicas, &out.MinReplicas + *out = new(int32) + **out = **in + } if in.MaxReplicas != nil { in, out := &in.MaxReplicas, &out.MaxReplicas *out = new(int32) diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index 173ea68836..c7e4f91952 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -223,6 +223,10 @@ spec: If MaxReplicas is set autoscaling is enabled. format: int32 type: integer + minReplicas: + description: MinReplicas sets a lower bound to the autoscaling feature. + format: int32 + type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 60335aeff5..c8420eb8a2 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -221,6 +221,10 @@ spec: If MaxReplicas is set autoscaling is enabled. format: int32 type: integer + minReplicas: + description: MinReplicas sets a lower bound to the autoscaling feature. + format: int32 + type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/docs/api.md b/docs/api.md index 6e947a1127..85dea45e49 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1455,6 +1455,15 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. Format: int32
false + + minReplicas + integer + + MinReplicas sets a lower bound to the autoscaling feature.
+
+ Format: int32
+ + false mode enum diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 6d0bca72a2..606f4dffce 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/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 { diff --git a/tests/e2e/autoscale/00-install.yaml b/tests/e2e/autoscale/00-install.yaml index feec3b6771..ea822b62e8 100644 --- a/tests/e2e/autoscale/00-install.yaml +++ b/tests/e2e/autoscale/00-install.yaml @@ -4,6 +4,7 @@ metadata: name: simplest spec: replicas: 1 + minReplicas: 1 maxReplicas: 2 resources: limits: From 2ef9467b5d80a05421269dd7535e44b1d7998ab7 Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Mon, 11 Jul 2022 10:42:27 +0200 Subject: [PATCH 22/28] Update webhook to validate minReplicas when needed Signed-off-by: Kevin Earls --- apis/v1alpha1/opentelemetrycollector_webhook.go | 14 ++++++++++++++ pkg/collector/reconcile/horizontalpodautoscaler.go | 3 --- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index f4a6b4f068..16c2f714f1 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -62,6 +62,11 @@ func (r *OpenTelemetryCollector) Default() { one := int32(1) r.Spec.Replicas = &one } + + if r.Spec.MaxReplicas != nil && r.Spec.MinReplicas == nil { + one := int32(1) + r.Spec.MinReplicas = &one + } } // +kubebuilder:webhook:verbs=create;update,path=/validate-opentelemetry-io-v1alpha1-opentelemetrycollector,mutating=false,failurePolicy=fail,groups=opentelemetry.io,resources=opentelemetrycollectors,versions=v1alpha1,name=vopentelemetrycollectorcreateupdate.kb.io,sideEffects=none,admissionReviewVersions=v1 @@ -120,6 +125,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 more than one") + } + } return nil diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 606f4dffce..1fe597790a 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -52,7 +52,6 @@ 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 @@ -86,8 +85,6 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect 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 } } From ba0423bbfabc60fa305552dc7bc87955138fb2b2 Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Mon, 11 Jul 2022 11:49:26 +0200 Subject: [PATCH 23/28] Fix tests Signed-off-by: Kevin Earls --- apis/v1alpha1/opentelemetrycollector_webhook.go | 5 ----- pkg/collector/reconcile/horizontalpodautoscaler.go | 3 +++ 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index 16c2f714f1..9e6b07d018 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -62,11 +62,6 @@ func (r *OpenTelemetryCollector) Default() { one := int32(1) r.Spec.Replicas = &one } - - if r.Spec.MaxReplicas != nil && r.Spec.MinReplicas == nil { - one := int32(1) - r.Spec.MinReplicas = &one - } } // +kubebuilder:webhook:verbs=create;update,path=/validate-opentelemetry-io-v1alpha1-opentelemetrycollector,mutating=false,failurePolicy=fail,groups=opentelemetry.io,resources=opentelemetrycollectors,versions=v1alpha1,name=vopentelemetrycollectorcreateupdate.kb.io,sideEffects=none,admissionReviewVersions=v1 diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 1fe597790a..606f4dffce 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/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 @@ -85,6 +86,8 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect 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 } } From ef318ef8bd39759fcef248c817b58d690f92702c Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Wed, 13 Jul 2022 17:59:46 +0200 Subject: [PATCH 24/28] Initial upgrade code for instances with an HPA Signed-off-by: Kevin Earls --- pkg/collector/upgrade/v0_56_0.go | 32 +++++++++++++++++++++++++++ pkg/collector/upgrade/v0_56_0_test.go | 24 ++++++++++++++++++++ pkg/collector/upgrade/versions.go | 4 ++++ 3 files changed, 60 insertions(+) create mode 100644 pkg/collector/upgrade/v0_56_0.go create mode 100644 pkg/collector/upgrade/v0_56_0_test.go diff --git a/pkg/collector/upgrade/v0_56_0.go b/pkg/collector/upgrade/v0_56_0.go new file mode 100644 index 0000000000..05ce2aa3b7 --- /dev/null +++ b/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 +} diff --git a/pkg/collector/upgrade/v0_56_0_test.go b/pkg/collector/upgrade/v0_56_0_test.go new file mode 100644 index 0000000000..8c52ff8b49 --- /dev/null +++ b/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") +} diff --git a/pkg/collector/upgrade/versions.go b/pkg/collector/upgrade/versions.go index 7aec8a0f50..f78e824e4b 100644 --- a/pkg/collector/upgrade/versions.go +++ b/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. From 27fb26fcbc836b55a5ffb2713fc31c015f2ddb13 Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Fri, 15 Jul 2022 11:32:05 +0200 Subject: [PATCH 25/28] Make sure old Deploment based autoscalers get updated Signed-off-by: Kevin Earls --- pkg/collector/reconcile/horizontalpodautoscaler.go | 12 ++++++++++++ pkg/collector/upgrade/v0_56_0.go | 2 +- pkg/naming/main.go | 5 +++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 606f4dffce..5829a1dfff 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -17,6 +17,7 @@ package reconcile import ( "context" "fmt" + "github.com/open-telemetry/opentelemetry-operator/pkg/naming" autoscalingv1 "k8s.io/api/autoscaling/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -24,6 +25,7 @@ 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" ) @@ -91,6 +93,16 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect } } + // 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 { updated.Annotations[k] = v } diff --git a/pkg/collector/upgrade/v0_56_0.go b/pkg/collector/upgrade/v0_56_0.go index 05ce2aa3b7..808a0487e8 100644 --- a/pkg/collector/upgrade/v0_56_0.go +++ b/pkg/collector/upgrade/v0_56_0.go @@ -24,7 +24,7 @@ func upgrade0_56_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) ( return otelcol, nil } - u.Log.Info(">>>>>> in upgrade0_56_0", "Otel Instance", otelcol.Name, "Upgrade version", u.Version.String()) + u.Log.Info("in upgrade0_56_0", "Otel Instance", otelcol.Name, "Upgrade version", u.Version.String()) one := int32(1) otelcol.Spec.MinReplicas = &one diff --git a/pkg/naming/main.go b/pkg/naming/main.go index 2e2d32c06c..f6b896212f 100644 --- a/pkg/naming/main.go +++ b/pkg/naming/main.go @@ -64,6 +64,11 @@ 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)) From c733b6e392b5d8b6e734be0c66fa100ca7c9660b Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Fri, 15 Jul 2022 12:10:19 +0200 Subject: [PATCH 26/28] Fix lint error Signed-off-by: Kevin Earls --- pkg/collector/reconcile/horizontalpodautoscaler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 5829a1dfff..8c3490673e 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -17,7 +17,6 @@ package reconcile import ( "context" "fmt" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" autoscalingv1 "k8s.io/api/autoscaling/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -27,6 +26,7 @@ import ( "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 From 81aa971a0e82731e4acbd32268066c15931a0faf Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Fri, 15 Jul 2022 14:15:53 +0200 Subject: [PATCH 27/28] Respond to comments Signed-off-by: Kevin Earls --- apis/v1alpha1/opentelemetrycollector_types.go | 2 +- apis/v1alpha1/opentelemetrycollector_webhook.go | 2 +- bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml | 3 ++- config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml | 3 ++- pkg/collector/reconcile/horizontalpodautoscaler_test.go | 1 - tests/e2e/autoscale/00-install.yaml | 1 - 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 5758d241f3..5979695bb9 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -37,7 +37,7 @@ type OpenTelemetryCollectorSpec struct { // +optional Replicas *int32 `json:"replicas,omitempty"` - // MinReplicas sets a lower bound to the autoscaling feature. + // MinReplicas sets a lower bound to the autoscaling feature. It must be at least 1 // +optional MinReplicas *int32 `json:"minReplicas,omitempty"` diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index 9e6b07d018..6a4b1e936f 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -126,7 +126,7 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error { } if r.Spec.MinReplicas != nil && *r.Spec.MinReplicas < int32(1) { - return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, minReplicas should be more than one") + return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, minReplicas should be one or more") } } diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index c7e4f91952..f3b5acbde5 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -224,7 +224,8 @@ spec: format: int32 type: integer minReplicas: - description: MinReplicas sets a lower bound to the autoscaling feature. + description: MinReplicas sets a lower bound to the autoscaling feature. It + must be at least 1 format: int32 type: integer mode: diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index c8420eb8a2..2c16d8bd6f 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -222,7 +222,8 @@ spec: format: int32 type: integer minReplicas: - description: MinReplicas sets a lower bound to the autoscaling feature. + description: MinReplicas sets a lower bound to the autoscaling feature. It + must be at least 1 format: int32 type: integer mode: diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index 4c0b9f967e..62b901da37 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -52,7 +52,6 @@ func TestExpectedHPA(t *testing.T) { maxReplicas := int32(3) updateParms := paramsWithHPA() updateParms.Instance.Spec.Replicas = &minReplicas - updateParms.Instance.Spec.Replicas = &minReplicas updateParms.Instance.Spec.MaxReplicas = &maxReplicas updatedHPA := collector.HorizontalPodAutoscaler(updateParms.Config, logger, updateParms.Instance) diff --git a/tests/e2e/autoscale/00-install.yaml b/tests/e2e/autoscale/00-install.yaml index ea822b62e8..a9b54f4c9a 100644 --- a/tests/e2e/autoscale/00-install.yaml +++ b/tests/e2e/autoscale/00-install.yaml @@ -3,7 +3,6 @@ kind: OpenTelemetryCollector metadata: name: simplest spec: - replicas: 1 minReplicas: 1 maxReplicas: 2 resources: From f3d190b86db0e7a470e12a43088e3786639d019b Mon Sep 17 00:00:00 2001 From: Kevin Earls Date: Fri, 15 Jul 2022 14:26:45 +0200 Subject: [PATCH 28/28] Fixed api-docs Signed-off-by: Kevin Earls --- docs/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api.md b/docs/api.md index 85dea45e49..652f91f998 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1459,7 +1459,7 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. minReplicas integer - MinReplicas sets a lower bound to the autoscaling feature.
+ MinReplicas sets a lower bound to the autoscaling feature. It must be at least 1

Format: int32