From 7a48391172bc1c7abc00732b039d1f6b55443ee3 Mon Sep 17 00:00:00 2001 From: Ole Markus With Date: Thu, 18 Aug 2022 13:29:46 +0200 Subject: [PATCH] Delete disabled lifecycle hooks and implement disable logic for warmpools --- pkg/model/awsmodel/autoscalinggroup.go | 47 +++++++++---------- pkg/model/awsmodel/nodeterminationhandler.go | 1 + ...s_s3_object_cluster-completed.spec_content | 1 + .../minimal-warmpool/in-v1alpha2.yaml | 1 + .../awstasks/autoscalinglifecyclehook.go | 44 +++++++++++++---- 5 files changed, 60 insertions(+), 34 deletions(-) diff --git a/pkg/model/awsmodel/autoscalinggroup.go b/pkg/model/awsmodel/autoscalinggroup.go index 06748bb431..3f7978d4c2 100644 --- a/pkg/model/awsmodel/autoscalinggroup.go +++ b/pkg/model/awsmodel/autoscalinggroup.go @@ -88,10 +88,9 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error { } tsk.LaunchTemplate = task c.AddTask(tsk) - } - warmPool := b.Cluster.Spec.WarmPool.ResolveDefaults(ig) - { + warmPool := b.Cluster.Spec.WarmPool.ResolveDefaults(ig) + enabled := fi.Bool(warmPool.IsEnabled()) warmPoolTask := &awstasks.WarmPool{ Name: &name, @@ -102,29 +101,29 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error { warmPoolTask.MinSize = warmPool.MinSize warmPoolTask.MaxSize = warmPool.MaxSize - if warmPool.EnableLifecycleHook { - hookName := "kops-warmpool" - name := fmt.Sprintf("%s-%s", hookName, ig.GetName()) - - lifecyleTask := &awstasks.AutoscalingLifecycleHook{ - ID: aws.String(name), - Name: aws.String(name), - HookName: aws.String(hookName), - Lifecycle: b.Lifecycle, - AutoscalingGroup: b.LinkToAutoscalingGroup(ig), - DefaultResult: aws.String("ABANDON"), - // We let nodeup have 10 min to complete. Normally this should happen much faster, - // but CP nodes need 5 min or so to start on new clusters, and we need to wait for that. - HeartbeatTimeout: aws.Int64(600), - LifecycleTransition: aws.String("autoscaling:EC2_INSTANCE_LAUNCHING"), - } - - c.AddTask(lifecyleTask) - - } - } c.AddTask(warmPoolTask) + + hookName := "kops-warmpool" + name := fmt.Sprintf("%s-%s", hookName, ig.GetName()) + enableHook := warmPool.IsEnabled() && warmPool.EnableLifecycleHook + + lifecyleTask := &awstasks.AutoscalingLifecycleHook{ + ID: aws.String(name), + Name: aws.String(name), + HookName: aws.String(hookName), + AutoscalingGroup: b.LinkToAutoscalingGroup(ig), + Lifecycle: b.Lifecycle, + DefaultResult: aws.String("ABANDON"), + // We let nodeup have 10 min to complete. Normally this should happen much faster, + // but CP nodes need 5 min or so to start on new clusters, and we need to wait for that. + HeartbeatTimeout: aws.Int64(600), + LifecycleTransition: aws.String("autoscaling:EC2_INSTANCE_LAUNCHING"), + Enabled: &enableHook, + } + + c.AddTask(lifecyleTask) + } } diff --git a/pkg/model/awsmodel/nodeterminationhandler.go b/pkg/model/awsmodel/nodeterminationhandler.go index 96a78e21fc..0d9680a64d 100644 --- a/pkg/model/awsmodel/nodeterminationhandler.go +++ b/pkg/model/awsmodel/nodeterminationhandler.go @@ -112,6 +112,7 @@ func (b *NodeTerminationHandlerBuilder) configureASG(c *fi.ModelBuilderContext, DefaultResult: aws.String("CONTINUE"), HeartbeatTimeout: aws.Int64(DefaultMessageRetentionPeriod), LifecycleTransition: aws.String("autoscaling:EC2_INSTANCE_TERMINATING"), + Enabled: aws.Bool(true), } c.AddTask(lifecyleTask) diff --git a/tests/integration/update_cluster/minimal-warmpool/data/aws_s3_object_cluster-completed.spec_content b/tests/integration/update_cluster/minimal-warmpool/data/aws_s3_object_cluster-completed.spec_content index 27a4943c58..6b07058ec1 100644 --- a/tests/integration/update_cluster/minimal-warmpool/data/aws_s3_object_cluster-completed.spec_content +++ b/tests/integration/update_cluster/minimal-warmpool/data/aws_s3_object_cluster-completed.spec_content @@ -225,3 +225,4 @@ spec: nodes: public warmPool: enableLifecycleHook: true + maxSize: 1 diff --git a/tests/integration/update_cluster/minimal-warmpool/in-v1alpha2.yaml b/tests/integration/update_cluster/minimal-warmpool/in-v1alpha2.yaml index 4fa33ca039..4056e980b4 100644 --- a/tests/integration/update_cluster/minimal-warmpool/in-v1alpha2.yaml +++ b/tests/integration/update_cluster/minimal-warmpool/in-v1alpha2.yaml @@ -43,6 +43,7 @@ spec: zone: us-test-1a warmPool: enableLifecycleHook: true + maxSize: 1 --- diff --git a/upup/pkg/fi/cloudup/awstasks/autoscalinglifecyclehook.go b/upup/pkg/fi/cloudup/awstasks/autoscalinglifecyclehook.go index f49bc22323..4b73448a20 100644 --- a/upup/pkg/fi/cloudup/awstasks/autoscalinglifecyclehook.go +++ b/upup/pkg/fi/cloudup/awstasks/autoscalinglifecyclehook.go @@ -43,6 +43,8 @@ type AutoscalingLifecycleHook struct { DefaultResult *string HeartbeatTimeout *int64 LifecycleTransition *string + + Enabled *bool } var _ fi.CompareWithID = &AutoscalingLifecycleHook{} @@ -64,6 +66,10 @@ func (h *AutoscalingLifecycleHook) Find(c *fi.Context) (*AutoscalingLifecycleHoo return nil, fmt.Errorf("error listing ASG Lifecycle Hooks: %v", err) } if response == nil || len(response.LifecycleHooks) == 0 { + if !fi.BoolValue(h.Enabled) { + return h, nil + } + return nil, nil } if len(response.LifecycleHooks) > 1 { @@ -80,6 +86,7 @@ func (h *AutoscalingLifecycleHook) Find(c *fi.Context) (*AutoscalingLifecycleHoo DefaultResult: hook.DefaultResult, HeartbeatTimeout: hook.HeartbeatTimeout, LifecycleTransition: hook.LifecycleTransition, + Enabled: fi.Bool(true), } return actual, nil @@ -104,16 +111,27 @@ func (_ *AutoscalingLifecycleHook) CheckChanges(a, e, changes *AutoscalingLifecy func (*AutoscalingLifecycleHook) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *AutoscalingLifecycleHook) error { if changes != nil { - request := &autoscaling.PutLifecycleHookInput{ - AutoScalingGroupName: e.AutoscalingGroup.Name, - DefaultResult: e.DefaultResult, - HeartbeatTimeout: e.HeartbeatTimeout, - LifecycleHookName: e.GetHookName(), - LifecycleTransition: e.LifecycleTransition, - } - _, err := t.Cloud.Autoscaling().PutLifecycleHook(request) - if err != nil { - return err + if fi.BoolValue(e.Enabled) { + request := &autoscaling.PutLifecycleHookInput{ + AutoScalingGroupName: e.AutoscalingGroup.Name, + DefaultResult: e.DefaultResult, + HeartbeatTimeout: e.HeartbeatTimeout, + LifecycleHookName: e.GetHookName(), + LifecycleTransition: e.LifecycleTransition, + } + _, err := t.Cloud.Autoscaling().PutLifecycleHook(request) + if err != nil { + return err + } + } else { + request := &autoscaling.DeleteLifecycleHookInput{ + AutoScalingGroupName: e.AutoscalingGroup.Name, + LifecycleHookName: e.GetHookName(), + } + _, err := t.Cloud.Autoscaling().DeleteLifecycleHook(request) + if err != nil { + return err + } } } @@ -129,6 +147,9 @@ type terraformASGLifecycleHook struct { } func (_ *AutoscalingLifecycleHook) RenderTerraform(t *terraform.TerraformTarget, a, e, changes *AutoscalingLifecycleHook) error { + if !fi.BoolValue(e.Enabled) { + return nil + } tf := &terraformASGLifecycleHook{ Name: e.GetHookName(), AutoScalingGroupName: e.AutoscalingGroup.TerraformLink(), @@ -149,6 +170,9 @@ type cloudformationASGLifecycleHook struct { } func (_ *AutoscalingLifecycleHook) RenderCloudformation(t *cloudformation.CloudformationTarget, a, e, changes *AutoscalingLifecycleHook) error { + if !fi.BoolValue(e.Enabled) { + return nil + } tf := &cloudformationASGLifecycleHook{ LifecycleHookName: e.GetHookName(), AutoScalingGroupName: e.AutoscalingGroup.CloudformationLink(),