Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable RpaasInstance shutting down using Shutdown flag #149

Merged
merged 7 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions api/v1alpha1/rpaasinstance_types.go
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

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
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
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
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
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
Loading