Skip to content

Commit a809fcb

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#5762 from AndiDog/avoid-lifecyclehook-constant-updates
🐛 Fix lifecycle hooks being updated constantly, consider changes to `RoleARN` field as well
2 parents ed98c60 + 31e29fe commit a809fcb

File tree

2 files changed

+111
-2
lines changed

2 files changed

+111
-2
lines changed

‎pkg/cloud/services/autoscaling/lifecyclehook.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,9 @@ func lifecycleHookNeedsUpdate(existing *expinfrav1.AWSLifecycleHook, expected *e
204204
return ptr.Deref(existing.DefaultResult, expinfrav1.LifecycleHookDefaultResultAbandon) != ptr.Deref(expected.DefaultResult, expinfrav1.LifecycleHookDefaultResultAbandon) ||
205205
ptr.Deref(existing.HeartbeatTimeout, metav1.Duration{Duration: 3600 * time.Second}) != ptr.Deref(expected.HeartbeatTimeout, metav1.Duration{Duration: 3600 * time.Second}) ||
206206
existing.LifecycleTransition != expected.LifecycleTransition ||
207-
existing.NotificationTargetARN != expected.NotificationTargetARN ||
208-
existing.NotificationMetadata != expected.NotificationMetadata
207+
ptr.Deref(existing.NotificationTargetARN, "") != ptr.Deref(expected.NotificationTargetARN, "") ||
208+
ptr.Deref(existing.RoleARN, "") != ptr.Deref(expected.RoleARN, "") ||
209+
ptr.Deref(existing.NotificationMetadata, "") != ptr.Deref(expected.NotificationMetadata, "")
209210
}
210211

211212
func reconcileLifecycleHook(ctx context.Context, asgService services.ASGInterface, asgName string, wantedHook *expinfrav1.AWSLifecycleHook, existingHooks []*expinfrav1.AWSLifecycleHook, storeConditionsOnObject deprecatedv1beta1conditions.Setter, log logger.Wrapper) error {

‎pkg/cloud/services/autoscaling/lifecyclehook_test.go‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
. "github.com/onsi/gomega"
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25+
"k8s.io/utils/ptr"
2526

2627
expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2"
2728
)
@@ -53,6 +54,29 @@ func TestLifecycleHookNeedsUpdate(t *testing.T) {
5354
wantUpdate: false,
5455
},
5556

57+
{
58+
name: "exactly equal (all fields filled)",
59+
existing: expinfrav1.AWSLifecycleHook{
60+
Name: "test",
61+
NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth"),
62+
RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification"),
63+
LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING",
64+
HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second},
65+
DefaultResult: &defaultResultAbandon,
66+
NotificationMetadata: nil,
67+
},
68+
expected: expinfrav1.AWSLifecycleHook{
69+
Name: "test",
70+
NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth"),
71+
RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification"),
72+
LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING",
73+
HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second},
74+
DefaultResult: &defaultResultAbandon,
75+
NotificationMetadata: nil,
76+
},
77+
wantUpdate: false,
78+
},
79+
5680
{
5781
name: "heartbeatTimeout and defaultResult not set in manifest, but set to defaults by AWS",
5882
existing: expinfrav1.AWSLifecycleHook{
@@ -119,6 +143,90 @@ func TestLifecycleHookNeedsUpdate(t *testing.T) {
119143
},
120144
wantUpdate: true,
121145
},
146+
147+
{
148+
name: "role ARN differs",
149+
existing: expinfrav1.AWSLifecycleHook{
150+
Name: "test",
151+
NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth"),
152+
RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification"),
153+
LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING",
154+
HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second},
155+
DefaultResult: &defaultResultAbandon,
156+
NotificationMetadata: nil,
157+
},
158+
expected: expinfrav1.AWSLifecycleHook{
159+
Name: "test",
160+
NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth"),
161+
RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification2"),
162+
LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING",
163+
HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second},
164+
DefaultResult: &defaultResultAbandon,
165+
NotificationMetadata: nil,
166+
},
167+
wantUpdate: true,
168+
},
169+
170+
{
171+
name: "notification target ARN differs",
172+
existing: expinfrav1.AWSLifecycleHook{
173+
Name: "test",
174+
NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth"),
175+
RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification"),
176+
LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING",
177+
HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second},
178+
DefaultResult: &defaultResultAbandon,
179+
NotificationMetadata: nil,
180+
},
181+
expected: expinfrav1.AWSLifecycleHook{
182+
Name: "test",
183+
NotificationTargetARN: ptr.To("arn:aws:sqs:eu-west-1:123456789012:mycluster-nth2"),
184+
RoleARN: ptr.To("arn:aws:iam::123456789012:role/mycluster-nth-notification"),
185+
LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING",
186+
HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second},
187+
DefaultResult: &defaultResultAbandon,
188+
NotificationMetadata: nil,
189+
},
190+
wantUpdate: true,
191+
},
192+
193+
{
194+
name: "notification metadata both empty",
195+
existing: expinfrav1.AWSLifecycleHook{
196+
Name: "test",
197+
LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING",
198+
HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second},
199+
DefaultResult: &defaultResultAbandon,
200+
NotificationMetadata: nil,
201+
},
202+
expected: expinfrav1.AWSLifecycleHook{
203+
Name: "test",
204+
LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING",
205+
HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second},
206+
DefaultResult: &defaultResultAbandon,
207+
NotificationMetadata: ptr.To(""),
208+
},
209+
wantUpdate: false,
210+
},
211+
212+
{
213+
name: "notification metadata differs",
214+
existing: expinfrav1.AWSLifecycleHook{
215+
Name: "test",
216+
LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING",
217+
HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second},
218+
DefaultResult: &defaultResultAbandon,
219+
NotificationMetadata: ptr.To("abc"),
220+
},
221+
expected: expinfrav1.AWSLifecycleHook{
222+
Name: "test",
223+
LifecycleTransition: "autoscaling:EC2_INSTANCE_TERMINATING",
224+
HeartbeatTimeout: &metav1.Duration{Duration: 3600 * time.Second},
225+
DefaultResult: &defaultResultAbandon,
226+
NotificationMetadata: ptr.To("xyz"),
227+
},
228+
wantUpdate: true,
229+
},
122230
}
123231
for _, tt := range tests {
124232
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)