diff --git a/pkg/cloud/services/autoscaling/lifecyclehook.go b/pkg/cloud/services/autoscaling/lifecyclehook.go index 6922eaa32a..a494a4a242 100644 --- a/pkg/cloud/services/autoscaling/lifecyclehook.go +++ b/pkg/cloud/services/autoscaling/lifecyclehook.go @@ -204,8 +204,9 @@ func lifecycleHookNeedsUpdate(existing *expinfrav1.AWSLifecycleHook, expected *e return ptr.Deref(existing.DefaultResult, expinfrav1.LifecycleHookDefaultResultAbandon) != ptr.Deref(expected.DefaultResult, expinfrav1.LifecycleHookDefaultResultAbandon) || ptr.Deref(existing.HeartbeatTimeout, metav1.Duration{Duration: 3600 * time.Second}) != ptr.Deref(expected.HeartbeatTimeout, metav1.Duration{Duration: 3600 * time.Second}) || existing.LifecycleTransition != expected.LifecycleTransition || - existing.NotificationTargetARN != expected.NotificationTargetARN || - existing.NotificationMetadata != expected.NotificationMetadata + ptr.Deref(existing.NotificationTargetARN, "") != ptr.Deref(expected.NotificationTargetARN, "") || + ptr.Deref(existing.RoleARN, "") != ptr.Deref(expected.RoleARN, "") || + ptr.Deref(existing.NotificationMetadata, "") != ptr.Deref(expected.NotificationMetadata, "") } func reconcileLifecycleHook(ctx context.Context, asgService services.ASGInterface, asgName string, wantedHook *expinfrav1.AWSLifecycleHook, existingHooks []*expinfrav1.AWSLifecycleHook, storeConditionsOnObject deprecatedv1beta1conditions.Setter, log logger.Wrapper) error { diff --git a/pkg/cloud/services/autoscaling/lifecyclehook_test.go b/pkg/cloud/services/autoscaling/lifecyclehook_test.go index 5aba913f64..865d3007b1 100644 --- a/pkg/cloud/services/autoscaling/lifecyclehook_test.go +++ b/pkg/cloud/services/autoscaling/lifecyclehook_test.go @@ -22,6 +22,7 @@ import ( . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2" ) @@ -53,6 +54,29 @@ func TestLifecycleHookNeedsUpdate(t *testing.T) { wantUpdate: false, }, + { + name: "exactly equal (all fields filled)", + existing: expinfrav1.AWSLifecycleHook{ + Name: "test", + NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth"), + RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification"), + LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING", + HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second}, + DefaultResult: &defaultResultAbandon, + NotificationMetadata: nil, + }, + expected: expinfrav1.AWSLifecycleHook{ + Name: "test", + NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth"), + RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification"), + LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING", + HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second}, + DefaultResult: &defaultResultAbandon, + NotificationMetadata: nil, + }, + wantUpdate: false, + }, + { name: "heartbeatTimeout and defaultResult not set in manifest, but set to defaults by AWS", existing: expinfrav1.AWSLifecycleHook{ @@ -119,6 +143,90 @@ func TestLifecycleHookNeedsUpdate(t *testing.T) { }, wantUpdate: true, }, + + { + name: "role ARN differs", + existing: expinfrav1.AWSLifecycleHook{ + Name: "test", + NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth"), + RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification"), + LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING", + HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second}, + DefaultResult: &defaultResultAbandon, + NotificationMetadata: nil, + }, + expected: expinfrav1.AWSLifecycleHook{ + Name: "test", + NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth"), + RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification2"), + LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING", + HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second}, + DefaultResult: &defaultResultAbandon, + NotificationMetadata: nil, + }, + wantUpdate: true, + }, + + { + name: "notification target ARN differs", + existing: expinfrav1.AWSLifecycleHook{ + Name: "test", + NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth"), + RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification"), + LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING", + HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second}, + DefaultResult: &defaultResultAbandon, + NotificationMetadata: nil, + }, + expected: expinfrav1.AWSLifecycleHook{ + Name: "test", + NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth2"), + RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification"), + LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING", + HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second}, + DefaultResult: &defaultResultAbandon, + NotificationMetadata: nil, + }, + wantUpdate: true, + }, + + { + name: "notification metadata both empty", + existing: expinfrav1.AWSLifecycleHook{ + Name: "test", + LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING", + HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second}, + DefaultResult: &defaultResultAbandon, + NotificationMetadata: nil, + }, + expected: expinfrav1.AWSLifecycleHook{ + Name: "test", + LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING", + HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second}, + DefaultResult: &defaultResultAbandon, + NotificationMetadata: ptr.To(""), + }, + wantUpdate: false, + }, + + { + name: "notification metadata differs", + existing: expinfrav1.AWSLifecycleHook{ + Name: "test", + LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING", + HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second}, + DefaultResult: &defaultResultAbandon, + NotificationMetadata: ptr.To("abc"), + }, + expected: expinfrav1.AWSLifecycleHook{ + Name: "test", + LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING", + HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second}, + DefaultResult: &defaultResultAbandon, + NotificationMetadata: ptr.To("xyz"), + }, + wantUpdate: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {