From b0cd2bcc5f3b5e8ba27fae244f362e7d0e5e1d90 Mon Sep 17 00:00:00 2001 From: Guilherme Vicentin Date: Mon, 25 Mar 2024 17:53:09 -0300 Subject: [PATCH 1/7] Enable instance shuting down using Shutdown flag --- api/v1alpha1/rpaasinstance_types.go | 6 +++ api/v1alpha1/zz_generated.deepcopy.go | 45 ++++++++++++++++++- .../extensions.tsuru.io_rpaasflavors.yaml | 6 +++ .../extensions.tsuru.io_rpaasinstances.yaml | 6 +++ controllers/controller.go | 25 ++++++++--- 5 files changed, 81 insertions(+), 7 deletions(-) diff --git a/api/v1alpha1/rpaasinstance_types.go b/api/v1alpha1/rpaasinstance_types.go index 09d9a4659..f13fedb36 100644 --- a/api/v1alpha1/rpaasinstance_types.go +++ b/api/v1alpha1/rpaasinstance_types.go @@ -134,6 +134,12 @@ type RpaasInstanceSpec struct { // +optional // +kubebuilder:default:=false Suspend *bool `json:"suspend,omitempty"` + + // Shutdown flag tells whether controller should scale down Nginx instances. + // And any assosciated HorizontalPodAutoscaler is remove/created when this flag is toggled. + // +optional + // +kubebuilder:default:=false + Shutdown *bool `json:"shutdown,omitempty"` } type DynamicCertificates struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 0e83bf711..42f9c2d7c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -285,6 +285,13 @@ func (in *NginxConfig) DeepCopyInto(out *NginxConfig) { *out = new(bool) **out = **in } + if in.TemplateExtraVars != nil { + in, out := &in.TemplateExtraVars, &out.TemplateExtraVars + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxConfig. @@ -363,6 +370,11 @@ func (in *RpaasFlavorSpec) DeepCopyInto(out *RpaasFlavorSpec) { *out = new(RpaasInstanceSpec) (*in).DeepCopyInto(*out) } + if in.IncompatibleFlavors != nil { + in, out := &in.IncompatibleFlavors, &out.IncompatibleFlavors + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RpaasFlavorSpec. @@ -380,7 +392,7 @@ func (in *RpaasInstance) DeepCopyInto(out *RpaasInstance) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) in.Spec.DeepCopyInto(&out.Spec) } @@ -447,6 +459,31 @@ func (in *RpaasInstanceAutoscaleSpec) DeepCopy() *RpaasInstanceAutoscaleSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RpaasInstanceExternalAddressesStatus) DeepCopyInto(out *RpaasInstanceExternalAddressesStatus) { + *out = *in + if in.IPs != nil { + in, out := &in.IPs, &out.IPs + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Hostnames != nil { + in, out := &in.Hostnames, &out.Hostnames + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RpaasInstanceExternalAddressesStatus. +func (in *RpaasInstanceExternalAddressesStatus) DeepCopy() *RpaasInstanceExternalAddressesStatus { + if in == nil { + return nil + } + out := new(RpaasInstanceExternalAddressesStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RpaasInstanceList) DeepCopyInto(out *RpaasInstanceList) { *out = *in @@ -591,6 +628,11 @@ func (in *RpaasInstanceSpec) DeepCopyInto(out *RpaasInstanceSpec) { *out = new(bool) **out = **in } + if in.Shutdown != nil { + in, out := &in.Shutdown, &out.Shutdown + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RpaasInstanceSpec. @@ -606,6 +648,7 @@ func (in *RpaasInstanceSpec) DeepCopy() *RpaasInstanceSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RpaasInstanceStatus) DeepCopyInto(out *RpaasInstanceStatus) { *out = *in + in.ExternalAddresses.DeepCopyInto(&out.ExternalAddresses) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RpaasInstanceStatus. diff --git a/config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml b/config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml index 4ea9e3c3c..53016764a 100644 --- a/config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml +++ b/config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml @@ -6633,6 +6633,12 @@ spec: Defaults to true. type: boolean type: object + shutdown: + default: false + description: Shutdown flag tells whether controller should scale + down Nginx instances. And any assosciated HorizontalPodAutoscaler + is remove/created when this flag is toggled. + type: boolean suspend: default: false description: Suspend flag tells whether controller should suspend diff --git a/config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml b/config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml index 967964832..18abcb400 100644 --- a/config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml +++ b/config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml @@ -6381,6 +6381,12 @@ spec: true. type: boolean type: object + shutdown: + default: false + description: Shutdown flag tells whether controller should scale down + Nginx instances. And any assosciated HorizontalPodAutoscaler is + remove/created when this flag is toggled. + type: boolean suspend: default: false description: Suspend flag tells whether controller should suspend diff --git a/controllers/controller.go b/controllers/controller.go index ee4e7e18c..75dcdc325 100644 --- a/controllers/controller.go +++ b/controllers/controller.go @@ -583,7 +583,7 @@ func (r *RpaasInstanceReconciler) reconcileHPA(ctx context.Context, instance *v1 var observed autoscalingv2.HorizontalPodAutoscaler err = r.Client.Get(ctx, types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, &observed) if k8sErrors.IsNotFound(err) { - if !isAutoscaleEnabled(instance.Spec.Autoscale) { + if !isAutoscaleEnabled(instance) { logger.V(4).Info("Skipping HorizontalPodAutoscaler reconciliation: both HPA resource and desired RpaasAutoscaleSpec not found") return cleanedKeda, nil } @@ -605,7 +605,7 @@ func (r *RpaasInstanceReconciler) reconcileHPA(ctx context.Context, instance *v1 logger = logger.WithValues("HorizontalPodAutoscaler", types.NamespacedName{Name: observed.Name, Namespace: observed.Namespace}) - if !isAutoscaleEnabled(instance.Spec.Autoscale) { + if !isAutoscaleEnabled(instance) { logger.V(4).Info("Deleting HorizontalPodAutoscaler resource") if err = r.Client.Delete(ctx, &observed); err != nil { logger.Error(err, "Unable to delete the HorizontalPodAutoscaler resource") @@ -664,7 +664,7 @@ func (r *RpaasInstanceReconciler) reconcileKEDA(ctx context.Context, instance *v var observed kedav1alpha1.ScaledObject err = r.Client.Get(ctx, types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, &observed) if k8sErrors.IsNotFound(err) { - if !isAutoscaleEnabled(instance.Spec.Autoscale) { + if !isAutoscaleEnabled(instance) { return false, nil // nothing to do } @@ -679,7 +679,7 @@ func (r *RpaasInstanceReconciler) reconcileKEDA(ctx context.Context, instance *v return false, err } - if !isAutoscaleEnabled(instance.Spec.Autoscale) { + if !isAutoscaleEnabled(instance) { err = r.Client.Delete(ctx, &observed) if err != nil { return false, err @@ -699,12 +699,20 @@ func (r *RpaasInstanceReconciler) reconcileKEDA(ctx context.Context, instance *v return true, nil } -func isAutoscaleEnabled(a *v1alpha1.RpaasInstanceAutoscaleSpec) bool { +func shouldShutdownInstance(instance *v1alpha1.RpaasInstance) bool { + return instance.Spec.Shutdown != nil && *instance.Spec.Shutdown +} + +func isAutoscaleValid(a *v1alpha1.RpaasInstanceAutoscaleSpec) bool { return a != nil && (a.MinReplicas != nil && a.MaxReplicas > 0) && (a.TargetCPUUtilizationPercentage != nil || a.TargetMemoryUtilizationPercentage != nil || a.TargetRequestsPerSecond != nil || len(a.Schedules) > 0) } +func isAutoscaleEnabled(instance *v1alpha1.RpaasInstance) bool { + return !shouldShutdownInstance(instance) && isAutoscaleValid(instance.Spec.Autoscale) +} + func newKEDAScaledObject(instance *v1alpha1.RpaasInstance, nginx *nginxv1alpha1.Nginx) (*kedav1alpha1.ScaledObject, error) { var triggers []kedav1alpha1.ScaleTriggers if instance.Spec.Autoscale != nil && instance.Spec.Autoscale.TargetCPUUtilizationPercentage != nil { @@ -1186,7 +1194,12 @@ func newNginx(instanceMergedWithFlavors *v1alpha1.RpaasInstance, plan *v1alpha1. } replicas := instanceMergedWithFlavors.Spec.Replicas - if isAutoscaleEnabled(instanceMergedWithFlavors.Spec.Autoscale) { + shouldShutdown := shouldShutdownInstance(instanceMergedWithFlavors) + if shouldShutdown { + *replicas = 0 + } + + if isAutoscaleEnabled(instanceMergedWithFlavors) { // NOTE: we should avoid changing the number of replicas as it's managed by HPA. replicas = nil } From e7f6ba5528ef57fa1dec3aba5e7e602928ebe9e8 Mon Sep 17 00:00:00 2001 From: Guilherme Vicentin Date: Mon, 25 Mar 2024 17:56:48 -0300 Subject: [PATCH 2/7] Fix doc string --- api/v1alpha1/rpaasinstance_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1alpha1/rpaasinstance_types.go b/api/v1alpha1/rpaasinstance_types.go index f13fedb36..b37890dac 100644 --- a/api/v1alpha1/rpaasinstance_types.go +++ b/api/v1alpha1/rpaasinstance_types.go @@ -136,7 +136,7 @@ type RpaasInstanceSpec struct { Suspend *bool `json:"suspend,omitempty"` // Shutdown flag tells whether controller should scale down Nginx instances. - // And any assosciated HorizontalPodAutoscaler is remove/created when this flag is toggled. + // Any assosciated HorizontalPodAutoscaler is remove/created when this flag is toggled. // +optional // +kubebuilder:default:=false Shutdown *bool `json:"shutdown,omitempty"` From 2ef510f68195c937dff78d02249eb0c314f9534b Mon Sep 17 00:00:00 2001 From: Guilherme Vicentin Date: Tue, 26 Mar 2024 08:59:34 -0300 Subject: [PATCH 3/7] Remove unnecessary pointer --- api/v1alpha1/rpaasinstance_types.go | 2 +- controllers/controller.go | 9 ++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/api/v1alpha1/rpaasinstance_types.go b/api/v1alpha1/rpaasinstance_types.go index b37890dac..7cb43c671 100644 --- a/api/v1alpha1/rpaasinstance_types.go +++ b/api/v1alpha1/rpaasinstance_types.go @@ -139,7 +139,7 @@ type RpaasInstanceSpec struct { // Any assosciated HorizontalPodAutoscaler is remove/created when this flag is toggled. // +optional // +kubebuilder:default:=false - Shutdown *bool `json:"shutdown,omitempty"` + Shutdown bool `json:"shutdown,omitempty"` } type DynamicCertificates struct { diff --git a/controllers/controller.go b/controllers/controller.go index 75dcdc325..04374a2ec 100644 --- a/controllers/controller.go +++ b/controllers/controller.go @@ -699,10 +699,6 @@ func (r *RpaasInstanceReconciler) reconcileKEDA(ctx context.Context, instance *v return true, nil } -func shouldShutdownInstance(instance *v1alpha1.RpaasInstance) bool { - return instance.Spec.Shutdown != nil && *instance.Spec.Shutdown -} - func isAutoscaleValid(a *v1alpha1.RpaasInstanceAutoscaleSpec) bool { return a != nil && (a.MinReplicas != nil && a.MaxReplicas > 0) && @@ -710,7 +706,7 @@ func isAutoscaleValid(a *v1alpha1.RpaasInstanceAutoscaleSpec) bool { } func isAutoscaleEnabled(instance *v1alpha1.RpaasInstance) bool { - return !shouldShutdownInstance(instance) && isAutoscaleValid(instance.Spec.Autoscale) + return !instance.Spec.Shutdown && isAutoscaleValid(instance.Spec.Autoscale) } func newKEDAScaledObject(instance *v1alpha1.RpaasInstance, nginx *nginxv1alpha1.Nginx) (*kedav1alpha1.ScaledObject, error) { @@ -1194,8 +1190,7 @@ func newNginx(instanceMergedWithFlavors *v1alpha1.RpaasInstance, plan *v1alpha1. } replicas := instanceMergedWithFlavors.Spec.Replicas - shouldShutdown := shouldShutdownInstance(instanceMergedWithFlavors) - if shouldShutdown { + if shutdown := instanceMergedWithFlavors.Spec.Shutdown; shutdown { *replicas = 0 } From e93cf037090098b7f8bbc6b83b1eef0aad30b648 Mon Sep 17 00:00:00 2001 From: Guilherme Vicentin Date: Tue, 26 Mar 2024 09:49:18 -0300 Subject: [PATCH 4/7] Remove pointer in manifest files --- api/v1alpha1/zz_generated.deepcopy.go | 5 ----- config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml | 2 +- config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml | 4 ++-- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 42f9c2d7c..e99e4e95c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -628,11 +628,6 @@ func (in *RpaasInstanceSpec) DeepCopyInto(out *RpaasInstanceSpec) { *out = new(bool) **out = **in } - if in.Shutdown != nil { - in, out := &in.Shutdown, &out.Shutdown - *out = new(bool) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RpaasInstanceSpec. diff --git a/config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml b/config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml index 53016764a..676d59a43 100644 --- a/config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml +++ b/config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml @@ -6636,7 +6636,7 @@ spec: shutdown: default: false description: Shutdown flag tells whether controller should scale - down Nginx instances. And any assosciated HorizontalPodAutoscaler + down Nginx instances. Any assosciated HorizontalPodAutoscaler is remove/created when this flag is toggled. type: boolean suspend: diff --git a/config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml b/config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml index 18abcb400..22b87130f 100644 --- a/config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml +++ b/config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml @@ -6384,8 +6384,8 @@ spec: shutdown: default: false description: Shutdown flag tells whether controller should scale down - Nginx instances. And any assosciated HorizontalPodAutoscaler is - remove/created when this flag is toggled. + Nginx instances. Any assosciated HorizontalPodAutoscaler is remove/created + when this flag is toggled. type: boolean suspend: default: false From cca24ab194a1adca6e16692a8c0447d4fd31c131 Mon Sep 17 00:00:00 2001 From: Guilherme Vicentin Date: Tue, 26 Mar 2024 10:42:12 -0300 Subject: [PATCH 5/7] Changing isAutoscaleEnabled param --- controllers/controller.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/controllers/controller.go b/controllers/controller.go index 04374a2ec..99c2599e3 100644 --- a/controllers/controller.go +++ b/controllers/controller.go @@ -583,7 +583,7 @@ func (r *RpaasInstanceReconciler) reconcileHPA(ctx context.Context, instance *v1 var observed autoscalingv2.HorizontalPodAutoscaler err = r.Client.Get(ctx, types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, &observed) if k8sErrors.IsNotFound(err) { - if !isAutoscaleEnabled(instance) { + if !isAutoscaleEnabled(&instance.Spec) { logger.V(4).Info("Skipping HorizontalPodAutoscaler reconciliation: both HPA resource and desired RpaasAutoscaleSpec not found") return cleanedKeda, nil } @@ -605,7 +605,7 @@ func (r *RpaasInstanceReconciler) reconcileHPA(ctx context.Context, instance *v1 logger = logger.WithValues("HorizontalPodAutoscaler", types.NamespacedName{Name: observed.Name, Namespace: observed.Namespace}) - if !isAutoscaleEnabled(instance) { + if !isAutoscaleEnabled(&instance.Spec) { logger.V(4).Info("Deleting HorizontalPodAutoscaler resource") if err = r.Client.Delete(ctx, &observed); err != nil { logger.Error(err, "Unable to delete the HorizontalPodAutoscaler resource") @@ -664,7 +664,7 @@ func (r *RpaasInstanceReconciler) reconcileKEDA(ctx context.Context, instance *v var observed kedav1alpha1.ScaledObject err = r.Client.Get(ctx, types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, &observed) if k8sErrors.IsNotFound(err) { - if !isAutoscaleEnabled(instance) { + if !isAutoscaleEnabled(&instance.Spec) { return false, nil // nothing to do } @@ -679,7 +679,7 @@ func (r *RpaasInstanceReconciler) reconcileKEDA(ctx context.Context, instance *v return false, err } - if !isAutoscaleEnabled(instance) { + if !isAutoscaleEnabled(&instance.Spec) { err = r.Client.Delete(ctx, &observed) if err != nil { return false, err @@ -705,8 +705,8 @@ func isAutoscaleValid(a *v1alpha1.RpaasInstanceAutoscaleSpec) bool { (a.TargetCPUUtilizationPercentage != nil || a.TargetMemoryUtilizationPercentage != nil || a.TargetRequestsPerSecond != nil || len(a.Schedules) > 0) } -func isAutoscaleEnabled(instance *v1alpha1.RpaasInstance) bool { - return !instance.Spec.Shutdown && isAutoscaleValid(instance.Spec.Autoscale) +func isAutoscaleEnabled(instance *v1alpha1.RpaasInstanceSpec) bool { + return !instance.Shutdown && isAutoscaleValid(instance.Autoscale) } func newKEDAScaledObject(instance *v1alpha1.RpaasInstance, nginx *nginxv1alpha1.Nginx) (*kedav1alpha1.ScaledObject, error) { @@ -1194,7 +1194,7 @@ func newNginx(instanceMergedWithFlavors *v1alpha1.RpaasInstance, plan *v1alpha1. *replicas = 0 } - if isAutoscaleEnabled(instanceMergedWithFlavors) { + if isAutoscaleEnabled(&instanceMergedWithFlavors.Spec) { // NOTE: we should avoid changing the number of replicas as it's managed by HPA. replicas = nil } From 546a6771aaa8d581bf5eade6ae597174f83d15c7 Mon Sep 17 00:00:00 2001 From: Guilherme Vicentin Date: Tue, 26 Mar 2024 10:42:38 -0300 Subject: [PATCH 6/7] Testing Shutdown flag --- controllers/controller_test.go | 94 ++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/controllers/controller_test.go b/controllers/controller_test.go index bd2c9bcdb..7cfc952fc 100644 --- a/controllers/controller_test.go +++ b/controllers/controller_test.go @@ -114,6 +114,18 @@ func Test_newNginx(t *testing.T) { }, }, + "with Shutdown enabled": { + instance: func(i *v1alpha1.RpaasInstance) *v1alpha1.RpaasInstance { + i.Spec.Replicas = func(n int32) *int32 { return &n }(8) + i.Spec.Shutdown = true + return i + }, + expected: func(n *nginxv1alpha1.Nginx) *nginxv1alpha1.Nginx { + n.Spec.Replicas = func(n int32) *int32 { return &n }(0) + return n + }, + }, + "with load balancer": { instance: func(i *v1alpha1.RpaasInstance) *v1alpha1.RpaasInstance { i.Spec.Service = &nginxv1alpha1.NginxService{ @@ -249,6 +261,88 @@ func Test_newNginx(t *testing.T) { } } +func Test_isAutoscaleValid(t *testing.T) { + tests := map[string]struct { + isValid bool + autoscale v1alpha1.RpaasInstanceAutoscaleSpec + }{ + "Invalid null autoscale": { + isValid: false, + autoscale: v1alpha1.RpaasInstanceAutoscaleSpec{}, + }, + + "Invalid minReplicas is greater than maxReplicas": { + isValid: false, + autoscale: v1alpha1.RpaasInstanceAutoscaleSpec{ + MinReplicas: func(n int32) *int32 { return &n }(5), + MaxReplicas: 1, + }, + }, + + "Valid autoscale": { + isValid: true, + autoscale: v1alpha1.RpaasInstanceAutoscaleSpec{ + MaxReplicas: 8, + MinReplicas: func(n int32) *int32 { return &n }(2), + TargetCPUUtilizationPercentage: func(n int32) *int32 { return &n }(90), + }, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got := isAutoscaleValid(&tt.autoscale) + assert.Equal(t, tt.isValid, got) + }) + } +} + +func Test_isAutoscaleEnabled(t *testing.T) { + tests := map[string]struct { + isEnabled bool + instanceSpec v1alpha1.RpaasInstanceSpec + }{ + "Disabled autoscale by invalid spec": { + isEnabled: false, + instanceSpec: v1alpha1.RpaasInstanceSpec{ + Shutdown: false, + Autoscale: &v1alpha1.RpaasInstanceAutoscaleSpec{}, + }, + }, + + "Disabled autoscale by shutdown": { + isEnabled: false, + instanceSpec: v1alpha1.RpaasInstanceSpec{ + Shutdown: true, + Autoscale: &v1alpha1.RpaasInstanceAutoscaleSpec{ + MaxReplicas: 8, + MinReplicas: func(n int32) *int32 { return &n }(2), + TargetCPUUtilizationPercentage: func(n int32) *int32 { return &n }(90), + }, + }, + }, + + "Enabled autoscale": { + isEnabled: true, + instanceSpec: v1alpha1.RpaasInstanceSpec{ + Shutdown: false, + Autoscale: &v1alpha1.RpaasInstanceAutoscaleSpec{ + MaxReplicas: 4, + MinReplicas: func(n int32) *int32 { return &n }(2), + TargetCPUUtilizationPercentage: func(n int32) *int32 { return &n }(70), + }, + }, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got := isAutoscaleEnabled(&tt.instanceSpec) + assert.Equal(t, tt.isEnabled, got) + }) + } +} + func Test_mergePlans(t *testing.T) { tests := []struct { base v1alpha1.RpaasPlanSpec From 717ebfe675f0ea194f3b46d45997f7aa8c340382 Mon Sep 17 00:00:00 2001 From: Guilherme Vicentin Date: Wed, 27 Mar 2024 12:03:52 -0300 Subject: [PATCH 7/7] Code review --- controllers/controller.go | 3 ++- controllers/controller_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/controllers/controller.go b/controllers/controller.go index 99c2599e3..39b5bd7b2 100644 --- a/controllers/controller.go +++ b/controllers/controller.go @@ -38,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/tsuru/rpaas-operator/api/v1alpha1" @@ -1191,7 +1192,7 @@ func newNginx(instanceMergedWithFlavors *v1alpha1.RpaasInstance, plan *v1alpha1. replicas := instanceMergedWithFlavors.Spec.Replicas if shutdown := instanceMergedWithFlavors.Spec.Shutdown; shutdown { - *replicas = 0 + replicas = pointer.Int32(0) } if isAutoscaleEnabled(&instanceMergedWithFlavors.Spec) { diff --git a/controllers/controller_test.go b/controllers/controller_test.go index 7cfc952fc..769e50789 100644 --- a/controllers/controller_test.go +++ b/controllers/controller_test.go @@ -1172,6 +1172,22 @@ func Test_reconcileHPA(t *testing.T) { expectedChanged: true, }, + "(native HPA controller) removing autoscale with shutdown flag": { + resources: []runtime.Object{ + baseExpectedHPA.DeepCopy(), + }, + instance: func(ri *v1alpha1.RpaasInstance) *v1alpha1.RpaasInstance { + ri.Spec.Shutdown = true + return ri + }, + customAssert: func(t *testing.T, r *RpaasInstanceReconciler) bool { + var hpa autoscalingv2.HorizontalPodAutoscaler + err := r.Client.Get(context.TODO(), types.NamespacedName{Name: "my-instance", Namespace: "default"}, &hpa) + return assert.True(t, k8sErrors.IsNotFound(err)) + }, + expectedChanged: true, + }, + "(native HPA controller) with RPS enabled": { instance: func(ri *v1alpha1.RpaasInstance) *v1alpha1.RpaasInstance { ri.Spec.Autoscale = &v1alpha1.RpaasInstanceAutoscaleSpec{ @@ -1413,6 +1429,22 @@ func Test_reconcileHPA(t *testing.T) { expectedChanged: true, }, + "(KEDA controller) removing autoscaling with shutdown flag": { + resources: []runtime.Object{ + baseExpectedScaledObject.DeepCopy(), + }, + instance: func(ri *v1alpha1.RpaasInstance) *v1alpha1.RpaasInstance { + ri.Spec.Shutdown = true + return ri + }, + customAssert: func(t *testing.T, r *RpaasInstanceReconciler) bool { + var so kedav1alpha1.ScaledObject + err := r.Client.Get(context.TODO(), types.NamespacedName{Name: baseExpectedScaledObject.Name, Namespace: baseExpectedScaledObject.Namespace}, &so) + return assert.True(t, k8sErrors.IsNotFound(err), "ScaledObject resource should not exist") + }, + expectedChanged: true, + }, + "(KEDA controller) KEDA controller enabled, but instance does not have RPS trigger": { instance: func(ri *v1alpha1.RpaasInstance) *v1alpha1.RpaasInstance { ri.Spec.Autoscale = &v1alpha1.RpaasInstanceAutoscaleSpec{