feat: delete asg hooks in the cloud if they're not defined in the crd

Author: sebltm

exp/api/v1beta2/conditions_consts.go

@@ -65,6 +65,8 @@ const (
 	LifecycleHookCreationFailedReason = "LifecycleHookCreationFailed"
 	// LifecycleHookUpdateFailedReason used for failures during lifecycle hook update.
 	LifecycleHookUpdateFailedReason = "LifecycleHookUpdateFailed"
+	// LifecycleHookDeletionFailedReason used for failures during lifecycle hook deletion.
+	LifecycleHookDeletionFailedReason = "LifecycleHookDeletionFailed"
 )
 
 const (

exp/controllers/awsmachinepool_controller.go

@@ -299,7 +299,18 @@ func (r *AWSMachinePoolReconciler) reconcileNormal(ctx context.Context, machineP
 		return nil
 	}
 
-	if err := reconSvc.ReconcileLifecycleHooks(machinePoolScope); err != nil {
+	lifecycleHookScope, err := scope.NewLifecycleHookScope(scope.LifecycleHookScopeParams{
+		Client:         r.Client,
+		Logger:         &machinePoolScope.Logger,
+		MachinePool:    machinePoolScope.MachinePool,
+		LifecycleHooks: machinePoolScope.AWSMachinePool.Spec.AWSLifecycleHooks,
+		AWSMachinePool: machinePoolScope.AWSMachinePool,
+	})
+	if err != nil {
+		return errors.Wrap(err, "failed to create lifecycle hook scope")
+	}
+
+	if err := reconSvc.ReconcileLifecycleHooks(*lifecycleHookScope); err != nil {
 		r.Recorder.Eventf(machinePoolScope.AWSMachinePool, corev1.EventTypeWarning, "FaileLifecycleHooksReconcile", "Failed to reconcile lifecycle hooks: %v", err)
 		return errors.Wrap(err, "failed to reconcile lifecycle hooks")
 	}

pkg/cloud/scope/lifecyclehook.go

@@ -17,29 +17,71 @@ limitations under the License.
 package scope
 
 import (
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"github.com/pkg/errors"
+	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
 	expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
 	expclusterv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
-	"sigs.k8s.io/cluster-api/util/conditions"
 )
 
 // LaunchTemplateScope defines a scope defined around a launch template.
-type LifecycleHookScope interface {
-	GetMachinePool() *expclusterv1.MachinePool
-	GetLifecycleHooks() []expinfrav1.AWSLifecycleHook
+type LifecycleHookScope struct {
+	client.Client
+	logger.Logger
 
-	IsEKSManaged() bool
-	AdditionalTags() infrav1.Tags
+	MachinePool    *expclusterv1.MachinePool
+	LifecycleHooks []expinfrav1.AWSLifecycleHook
+	AWSMachinePool *expinfrav1.AWSMachinePool
+}
 
-	GetObjectMeta() *metav1.ObjectMeta
-	GetSetter() conditions.Setter
-	PatchObject() error
-	GetEC2Scope() EC2Scope
+type LifecycleHookScopeParams struct {
+	Client client.Client
+	Logger *logger.Logger
 
-	client.Client
-	logger.Wrapper
+	MachinePool    *expclusterv1.MachinePool
+	LifecycleHooks []expinfrav1.AWSLifecycleHook
+	AWSMachinePool *expinfrav1.AWSMachinePool
+}
+
+// NewLifecycleHookScope creates a new LifecycleHookScope.
+func NewLifecycleHookScope(params LifecycleHookScopeParams) (*LifecycleHookScope, error) {
+	if params.Client == nil {
+		return nil, errors.New("client is required when creating a LifecycleHookScope")
+	}
+	if params.MachinePool == nil {
+		return nil, errors.New("machine pool is required when creating a LifecycleHookScope")
+	}
+	if params.LifecycleHooks == nil {
+		params.LifecycleHooks = make([]expinfrav1.AWSLifecycleHook, 0)
+	}
+	if params.AWSMachinePool == nil {
+		return nil, errors.New("aws machine pool is required when creating a LifecycleHookScope")
+	}
+
+	if params.Logger == nil {
+		log := klog.Background()
+		params.Logger = logger.NewLogger(log)
+	}
+
+	return &LifecycleHookScope{
+		Client:         params.Client,
+		Logger:         *params.Logger,
+		MachinePool:    params.MachinePool,
+		LifecycleHooks: params.LifecycleHooks,
+		AWSMachinePool: params.AWSMachinePool,
+	}, nil
+}
+
+func (s *LifecycleHookScope) GetASGName() string {
+	return s.AWSMachinePool.Name
+}
+
+func (s *LifecycleHookScope) GetLifecycleHooks() []expinfrav1.AWSLifecycleHook {
+	return s.LifecycleHooks
+}
+
+func (s *LifecycleHookScope) GetMachinePool() *expclusterv1.MachinePool {
+	return s.MachinePool
 }

pkg/cloud/services/autoscaling/lifecyclehook.go

@@ -21,7 +21,6 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/autoscaling"
-	"github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface"
 	"github.com/pkg/errors"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
 
@@ -36,16 +35,35 @@ func (s *Service) LifecycleHookNeedsUpdate(scope scope.LifecycleHookScope, exist
 		existing.NotificationMetadata != expected.NotificationMetadata
 }
 
+func (s *Service) GetLifecycleHooks(scope scope.LifecycleHookScope) ([]*expinfrav1.AWSLifecycleHook, error) {
+	asgName := scope.GetASGName()
+	input := &autoscaling.DescribeLifecycleHooksInput{
+		AutoScalingGroupName: aws.String(asgName),
+	}
+
+	out, err := s.ASGClient.DescribeLifecycleHooksWithContext(context.TODO(), input)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to describe lifecycle hooks for AutoScalingGroup: %q", scope.GetASGName())
+	}
+
+	hooks := make([]*expinfrav1.AWSLifecycleHook, len(out.LifecycleHooks))
+	for i, hook := range out.LifecycleHooks {
+		hooks[i] = s.SDKToLifecycleHook(hook)
+	}
+
+	return hooks, nil
+}
+
 func (s *Service) GetLifecycleHook(scope scope.LifecycleHookScope, hook *expinfrav1.AWSLifecycleHook) (*expinfrav1.AWSLifecycleHook, error) {
-	asgName := scope.GetMachinePool().Name
+	asgName := scope.GetASGName()
 	input := &autoscaling.DescribeLifecycleHooksInput{
 		AutoScalingGroupName: aws.String(asgName),
 		LifecycleHookNames:   []*string{aws.String(hook.Name)},
 	}
 
 	out, err := s.ASGClient.DescribeLifecycleHooksWithContext(context.TODO(), input)
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed to describe lifecycle hook %q for AutoScalingGroup: %q", hook.Name, scope.GetMachinePool().Name)
+		return nil, errors.Wrapf(err, "failed to describe lifecycle hook %q for AutoScalingGroup: %q", hook.Name, scope.GetASGName())
 	}
 
 	if len(out.LifecycleHooks) == 0 {
@@ -56,7 +74,7 @@ func (s *Service) GetLifecycleHook(scope scope.LifecycleHookScope, hook *expinfr
 }
 
 func (s *Service) CreateLifecycleHook(scope scope.LifecycleHookScope, hook *expinfrav1.AWSLifecycleHook) error {
-	asgName := scope.GetMachinePool().Name
+	asgName := scope.GetASGName()
 
 	lifecycleHookName := hook.Name
 	if lifecycleHookName == "" {
@@ -96,14 +114,14 @@ func (s *Service) CreateLifecycleHook(scope scope.LifecycleHookScope, hook *expi
 	}
 
 	if _, err := s.ASGClient.PutLifecycleHookWithContext(context.TODO(), input); err != nil {
-		return errors.Wrapf(err, "failed to create lifecycle hook %q for AutoScalingGroup: %q", lifecycleHookName, scope.GetMachinePool().Name)
+		return errors.Wrapf(err, "failed to create lifecycle hook %q for AutoScalingGroup: %q", lifecycleHookName, scope.GetASGName())
 	}
 
 	return nil
 }
 
 func (s *Service) UpdateLifecycleHook(scope scope.LifecycleHookScope, hook *expinfrav1.AWSLifecycleHook) error {
-	asgName := scope.GetMachinePool().Name
+	asgName := scope.GetASGName()
 
 	lifecycleHookName := hook.Name
 	if lifecycleHookName == "" {
@@ -143,23 +161,22 @@ func (s *Service) UpdateLifecycleHook(scope scope.LifecycleHookScope, hook *expi
 	}
 
 	if _, err := s.ASGClient.PutLifecycleHookWithContext(context.TODO(), input); err != nil {
-		return errors.Wrapf(err, "failed to update lifecycle hook %q for AutoScalingGroup: %q", lifecycleHookName, scope.GetMachinePool().Name)
+		return errors.Wrapf(err, "failed to update lifecycle hook %q for AutoScalingGroup: %q", lifecycleHookName, scope.GetASGName())
 	}
 
 	return nil
 }
 
 func (s *Service) DeleteLifecycleHook(
-	scope *scope.MachinePoolScope,
-	asgClient autoscalingiface.AutoScalingAPI,
-	hook expinfrav1.AWSLifecycleHook) error {
+	scope scope.LifecycleHookScope,
+	hook *expinfrav1.AWSLifecycleHook) error {
 	input := &autoscaling.DeleteLifecycleHookInput{
-		AutoScalingGroupName: aws.String(scope.Name()),
+		AutoScalingGroupName: aws.String(scope.GetASGName()),
 		LifecycleHookName:    aws.String(hook.Name),
 	}
 
-	if _, err := asgClient.DeleteLifecycleHookWithContext(context.TODO(), input); err != nil {
-		return errors.Wrapf(err, "failed to delete lifecycle hook %q for AutoScalingGroup: %q", hook.Name, scope.Name())
+	if _, err := s.ASGClient.DeleteLifecycleHookWithContext(context.TODO(), input); err != nil {
+		return errors.Wrapf(err, "failed to delete lifecycle hook %q for AutoScalingGroup: %q", hook.Name, scope.GetASGName())
 	}
 
 	return nil

pkg/cloud/services/interfaces.go

@@ -50,9 +50,11 @@ type ASGInterface interface {
 	SuspendProcesses(name string, processes []string) error
 	ResumeProcesses(name string, processes []string) error
 	SubnetIDs(scope *scope.MachinePoolScope) ([]string, error)
+	GetLifecycleHooks(scope scope.LifecycleHookScope) ([]*expinfrav1.AWSLifecycleHook, error)
 	GetLifecycleHook(scope scope.LifecycleHookScope, hook *expinfrav1.AWSLifecycleHook) (*expinfrav1.AWSLifecycleHook, error)
 	CreateLifecycleHook(scope scope.LifecycleHookScope, hook *expinfrav1.AWSLifecycleHook) error
 	UpdateLifecycleHook(scope scope.LifecycleHookScope, hook *expinfrav1.AWSLifecycleHook) error
+	DeleteLifecycleHook(scope scope.LifecycleHookScope, hook *expinfrav1.AWSLifecycleHook) error
 	LifecycleHookNeedsUpdate(scope scope.LifecycleHookScope, incoming *expinfrav1.AWSLifecycleHook, existing *expinfrav1.AWSLifecycleHook) bool
 }
 

pkg/cloud/services/machinepool/machinepool.go

@@ -35,6 +35,29 @@ func (s *Service) ReconcileLifecycleHooks(scope scope.LifecycleHookScope) error
 			return err
 		}
 	}
+
+	// Get a list of lifecycle hooks that are registered with the ASG but not defined in the MachinePool and delete them.
+	hooks, err := s.ASGInterface.GetLifecycleHooks(scope)
+	if err != nil {
+		return err
+	}
+	for _, hook := range hooks {
+		found := false
+		for _, definedHook := range scope.GetLifecycleHooks() {
+			if hook.Name == definedHook.Name {
+				found = true
+				break
+			}
+		}
+		if !found {
+			scope.Info("Deleting lifecycle hook", "hook", hook.Name)
+			if err := s.ASGInterface.DeleteLifecycleHook(scope, hook); err != nil {
+				conditions.MarkFalse(scope.GetMachinePool(), expinfrav1.LifecycleHookExistsCondition, expinfrav1.LifecycleHookDeletionFailedReason, clusterv1.ConditionSeverityError, err.Error())
+				return err
+			}
+		}
+	}
+
 	return nil
 }