From 3b6440cec8b3d7ffce9cd9bd13e3353fbedb7ff1 Mon Sep 17 00:00:00 2001 From: binjip978 Date: Fri, 22 Apr 2022 07:09:21 +0000 Subject: [PATCH 1/2] Set replicas to MaxReplicas if HPA is enabled If number of replicas in current deployment status is bigger than MaxReplicas and HPA is enabled set deployment replicas to MaxReplicas. --- pkg/collector/reconcile/deployment.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index 247a932579..e044d0ab7b 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/pkg/collector/reconcile/deployment.go @@ -98,14 +98,20 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D updated.ObjectMeta.Labels[k] = v } - // if autoscale is enabled, use replicas from current Status if params.Instance.Spec.MaxReplicas != nil { currentReplicas := existing.Status.Replicas - // if replicas (minReplicas from HPA perspective) is bigger than - // current status use it. - if params.Instance.Spec.Replicas != nil && *params.Instance.Spec.Replicas > currentReplicas { - currentReplicas = *params.Instance.Spec.Replicas + if params.Instance.Spec.Replicas != nil { + // if replicas (minReplicas from HPA perspective) is bigger than + // current status use it. + if *params.Instance.Spec.Replicas > currentReplicas { + currentReplicas = *params.Instance.Spec.Replicas + } + // honor hpa max replicas parameter + if currentReplicas > *params.Instance.Spec.MaxReplicas { + currentReplicas = *params.Instance.Spec.MaxReplicas + } } + updated.Spec.Replicas = ¤tReplicas } From d9ca106de2f514420c4b4be7f0cdcd734632bd43 Mon Sep 17 00:00:00 2001 From: binjip978 Date: Wed, 27 Apr 2022 06:34:29 +0000 Subject: [PATCH 2/2] Move replicas calculation in seperate function Signed-off-by: binjip978 --- pkg/collector/reconcile/deployment.go | 28 ++++++++++++---------- pkg/collector/reconcile/deployment_test.go | 18 ++++++++++++++ 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index e044d0ab7b..d8bb01d54e 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/pkg/collector/reconcile/deployment.go @@ -24,6 +24,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" "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator" ) @@ -99,19 +100,7 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D } if params.Instance.Spec.MaxReplicas != nil { - currentReplicas := existing.Status.Replicas - if params.Instance.Spec.Replicas != nil { - // if replicas (minReplicas from HPA perspective) is bigger than - // current status use it. - if *params.Instance.Spec.Replicas > currentReplicas { - currentReplicas = *params.Instance.Spec.Replicas - } - // honor hpa max replicas parameter - if currentReplicas > *params.Instance.Spec.MaxReplicas { - currentReplicas = *params.Instance.Spec.MaxReplicas - } - } - + currentReplicas := currentReplicasWithHPA(params.Instance.Spec, existing.Status.Replicas) updated.Spec.Replicas = ¤tReplicas } @@ -160,3 +149,16 @@ func deleteDeployments(ctx context.Context, params Params, expected []appsv1.Dep return nil } + +// 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.MaxReplicas { + return *spec.MaxReplicas + } + + return curr +} diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index aa955af3a8..5a513d098b 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -234,3 +234,21 @@ func TestExpectedDeployments(t *testing.T) { }) } + +func TestCurrentReplicasWithHPA(t *testing.T) { + minReplicas := int32(2) + maxReplicas := int32(5) + spec := v1alpha1.OpenTelemetryCollectorSpec{ + Replicas: &minReplicas, + MaxReplicas: &maxReplicas, + } + + res := currentReplicasWithHPA(spec, 10) + assert.Equal(t, int32(5), res) + + res = currentReplicasWithHPA(spec, 1) + assert.Equal(t, int32(2), res) + + res = currentReplicasWithHPA(spec, 3) + assert.Equal(t, int32(3), res) +}