Skip to content

Commit

Permalink
Enable RpaasInstance shutting down using Shutdown flag (#149)
Browse files Browse the repository at this point in the history 
* Enable instance shuting down using Shutdown flag

* Fix doc string

* Remove unnecessary pointer

* Remove pointer in manifest files

* Changing isAutoscaleEnabled param

* Testing Shutdown flag

* Code review

---------

Co-authored-by: Guilherme Vicentin <guilherme.vicentin@g.globo>
  • Loading branch information
@gvicentin
gvicentin and Guilherme Vicentin authored Mar 27, 2024
1 parent 420caaa commit 27db984
Show file tree
Hide file tree
Showing 6 changed files with 198 additions and 7 deletions.
6 changes: 6 additions & 0 deletions api/v1alpha1/rpaasinstance_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
// Any assosciated HorizontalPodAutoscaler is remove/created when this flag is toggled.
// +optional
// +kubebuilder:default:=false
Shutdown bool `json:"shutdown,omitempty"`
}

type DynamicCertificates struct {
Expand Down
40 changes: 39 additions & 1 deletion api/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. Any assosciated HorizontalPodAutoscaler
is remove/created when this flag is toggled.
type: boolean
suspend:
default: false
description: Suspend flag tells whether controller should suspend
Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6381,6 +6381,12 @@ spec:
true.
type: boolean
type: object
shutdown:
default: false
description: Shutdown flag tells whether controller should scale down
Nginx instances. Any assosciated HorizontalPodAutoscaler is remove/created
when this flag is toggled.
type: boolean
suspend:
default: false
description: Suspend flag tells whether controller should suspend
Expand Down
21 changes: 15 additions & 6 deletions controllers/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -583,7 +584,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.Spec) {
logger.V(4).Info("Skipping HorizontalPodAutoscaler reconciliation: both HPA resource and desired RpaasAutoscaleSpec not found")
return cleanedKeda, nil
}
Expand All @@ -605,7 +606,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.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")
Expand Down Expand Up @@ -664,7 +665,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.Spec) {
return false, nil // nothing to do
}

Expand All @@ -679,7 +680,7 @@ func (r *RpaasInstanceReconciler) reconcileKEDA(ctx context.Context, instance *v
return false, err
}

if !isAutoscaleEnabled(instance.Spec.Autoscale) {
if !isAutoscaleEnabled(&instance.Spec) {
err = r.Client.Delete(ctx, &observed)
if err != nil {
return false, err
Expand All @@ -699,12 +700,16 @@ func (r *RpaasInstanceReconciler) reconcileKEDA(ctx context.Context, instance *v
return true, nil
}

func isAutoscaleEnabled(a *v1alpha1.RpaasInstanceAutoscaleSpec) bool {
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.RpaasInstanceSpec) bool {
return !instance.Shutdown && isAutoscaleValid(instance.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 {
Expand Down Expand Up @@ -1186,7 +1191,11 @@ func newNginx(instanceMergedWithFlavors *v1alpha1.RpaasInstance, plan *v1alpha1.
}

replicas := instanceMergedWithFlavors.Spec.Replicas
if isAutoscaleEnabled(instanceMergedWithFlavors.Spec.Autoscale) {
if shutdown := instanceMergedWithFlavors.Spec.Shutdown; shutdown {
replicas = pointer.Int32(0)
}

if isAutoscaleEnabled(&instanceMergedWithFlavors.Spec) {
// NOTE: we should avoid changing the number of replicas as it's managed by HPA.
replicas = nil
}
Expand Down
126 changes: 126 additions & 0 deletions controllers/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1078,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{
Expand Down Expand Up @@ -1319,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{
Expand Down

0 comments on commit 27db984

Please sign in to comment.