Skip to content

Commit 275b3d6

Browse files
AndiDogAndiDog
committed
Create lifecycle hooks together with ASG
1 parent 1075be8 commit 275b3d6

File tree

3 files changed

+100
-98
lines changed

3 files changed

+100
-98
lines changed

‎exp/controllers/awsmachinepool_controller_test.go‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
304304
asgSvc.EXPECT().CreateASG(gomock.Any()).Return(&expinfrav1.AutoScalingGroup{
305305
Name: "name",
306306
}, nil)
307-
asgSvc.EXPECT().SuspendProcesses("name", []string{"Launch", "Terminate"}).Return(nil).AnyTimes().Times(0)
307+
asgSvc.EXPECT().SuspendProcesses("name", []string{"Launch", "Terminate"}).Return(nil).Times(0)
308308

309309
err := reconciler.reconcileNormal(context.Background(), ms, cs, cs)
310310
g.Expect(err).To(Succeed())
@@ -341,7 +341,7 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
341341
"InstanceRefresh",
342342
"HealthCheck",
343343
"ReplaceUnhealthy",
344-
})).Return(nil).AnyTimes().Times(1)
344+
})).Return(nil).Times(1)
345345

346346
err := reconciler.reconcileNormal(context.Background(), ms, cs, cs)
347347
g.Expect(err).To(Succeed())
@@ -373,8 +373,8 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
373373
}, nil)
374374
asgSvc.EXPECT().SubnetIDs(gomock.Any()).Return([]string{}, nil).Times(1)
375375
asgSvc.EXPECT().UpdateASG(gomock.Any()).Return(nil).AnyTimes()
376-
asgSvc.EXPECT().SuspendProcesses("name", []string{"Terminate"}).Return(nil).AnyTimes().Times(1)
377-
asgSvc.EXPECT().ResumeProcesses("name", []string{"process3"}).Return(nil).AnyTimes().Times(1)
376+
asgSvc.EXPECT().SuspendProcesses("name", []string{"Terminate"}).Return(nil).Times(1)
377+
asgSvc.EXPECT().ResumeProcesses("name", []string{"process3"}).Return(nil).Times(1)
378378

379379
err := reconciler.reconcileNormal(context.Background(), ms, cs, cs)
380380
g.Expect(err).To(Succeed())
@@ -813,6 +813,32 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
813813
})
814814
})
815815
t.Run("Lifecycle Hooks", func(t *testing.T) {
816+
t.Run("ASG created with lifecycle hooks", func(t *testing.T) {
817+
g := NewWithT(t)
818+
setup(t, g)
819+
defer teardown(t, g)
820+
821+
newLifecycleHook := expinfrav1.AWSLifecycleHook{
822+
Name: "new-hook",
823+
LifecycleTransition: "autoscaling:EC2_INSTANCE_LAUNCHING",
824+
}
825+
ms.AWSMachinePool.Spec.AWSLifecycleHooks = append(ms.AWSMachinePool.Spec.AWSLifecycleHooks, newLifecycleHook)
826+
827+
reconSvc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
828+
829+
// New ASG must be created with lifecycle hooks (single AWS SDK call is enough)
830+
//
831+
// TODO: Since GetASGByName and CreateASG are both in the same interface, we can't inspect the actual
832+
// `CreateAutoScalingGroupWithContext` requests parameters here. Make this better testable down to
833+
// AWS SDK level and check `CreateAutoScalingGroupInput.LifecycleHookSpecificationList`.
834+
asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(nil, nil)
835+
asgSvc.EXPECT().CreateASG(gomock.Any()).Return(&expinfrav1.AutoScalingGroup{
836+
Name: "name",
837+
}, nil)
838+
839+
err := reconciler.reconcileNormal(context.Background(), ms, cs, cs)
840+
g.Expect(err).To(Succeed())
841+
})
816842
t.Run("New lifecycle hook is added", func(t *testing.T) {
817843
g := NewWithT(t)
818844
setup(t, g)

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

Lines changed: 29 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -163,24 +163,17 @@ func (s *Service) CreateASG(machinePoolScope *scope.MachinePoolScope) (*expinfra
163163
return nil, fmt.Errorf("getting subnets for ASG: %w", err)
164164
}
165165

166-
input := &expinfrav1.AutoScalingGroup{
167-
Name: machinePoolScope.Name(),
168-
MaxSize: machinePoolScope.AWSMachinePool.Spec.MaxSize,
169-
MinSize: machinePoolScope.AWSMachinePool.Spec.MinSize,
170-
Subnets: subnets,
171-
DefaultCoolDown: machinePoolScope.AWSMachinePool.Spec.DefaultCoolDown,
172-
DefaultInstanceWarmup: machinePoolScope.AWSMachinePool.Spec.DefaultInstanceWarmup,
173-
CapacityRebalance: machinePoolScope.AWSMachinePool.Spec.CapacityRebalance,
174-
MixedInstancesPolicy: machinePoolScope.AWSMachinePool.Spec.MixedInstancesPolicy,
175-
}
166+
name := machinePoolScope.Name()
167+
s.scope.Info("Creating ASG", "name", name)
176168

177169
// Default value of MachinePool replicas set by CAPI is 1.
178170
mpReplicas := *machinePoolScope.MachinePool.Spec.Replicas
171+
var desiredCapacity *int32
179172

180173
// Check that MachinePool replicas number is between the minimum and maximum size of the AWSMachinePool.
181174
// Ignore the problem for externally managed clusters because MachinePool replicas will be updated to the right value automatically.
182175
if mpReplicas >= machinePoolScope.AWSMachinePool.Spec.MinSize && mpReplicas <= machinePoolScope.AWSMachinePool.Spec.MaxSize {
183-
input.DesiredCapacity = &mpReplicas
176+
desiredCapacity = &mpReplicas
184177
} else if !annotations.ReplicasManagedByExternalAutoscaler(machinePoolScope.MachinePool) {
185178
return nil, fmt.Errorf("incorrect number of replicas %d in MachinePool %v", mpReplicas, machinePoolScope.MachinePool.Name)
186179
}
@@ -194,62 +187,46 @@ func (s *Service) CreateASG(machinePoolScope *scope.MachinePoolScope) (*expinfra
194187
// Set the cloud provider tag
195188
additionalTags[infrav1.ClusterAWSCloudProviderTagKey(s.scope.KubernetesClusterName())] = string(infrav1.ResourceLifecycleOwned)
196189

197-
input.Tags = infrav1.Build(infrav1.BuildParams{
198-
ClusterName: s.scope.KubernetesClusterName(),
199-
Lifecycle: infrav1.ResourceLifecycleOwned,
200-
Name: aws.String(machinePoolScope.Name()),
201-
Role: aws.String("node"),
202-
Additional: additionalTags,
203-
})
204-
205-
s.scope.Info("Running instance")
206-
if err := s.runPool(input, machinePoolScope.AWSMachinePool.Status.LaunchTemplateID); err != nil {
207-
// Only record the failure event if the error is not related to failed dependencies.
208-
// This is to avoid spamming failure events since the machine will be requeued by the actuator.
209-
// if !awserrors.IsFailedDependency(errors.Cause(err)) {
210-
// record.Warnf(scope.AWSMachinePool, "FailedCreate", "Failed to create instance: %v", err)
211-
// }
212-
s.scope.Error(err, "unable to create AutoScalingGroup")
213-
return nil, err
214-
}
215-
record.Eventf(machinePoolScope.AWSMachinePool, "SuccessfulCreate", "Created new ASG: %s", machinePoolScope.Name())
216-
217-
return nil, nil
218-
}
219-
220-
func (s *Service) runPool(i *expinfrav1.AutoScalingGroup, launchTemplateID string) error {
221190
input := &autoscaling.CreateAutoScalingGroupInput{
222-
AutoScalingGroupName: aws.String(i.Name),
223-
MaxSize: aws.Int64(int64(i.MaxSize)),
224-
MinSize: aws.Int64(int64(i.MinSize)),
225-
VPCZoneIdentifier: aws.String(strings.Join(i.Subnets, ", ")),
226-
DefaultCooldown: aws.Int64(int64(i.DefaultCoolDown.Duration.Seconds())),
227-
DefaultInstanceWarmup: aws.Int64(int64(i.DefaultInstanceWarmup.Duration.Seconds())),
228-
CapacityRebalance: aws.Bool(i.CapacityRebalance),
191+
AutoScalingGroupName: aws.String(name),
192+
MaxSize: aws.Int64(int64(machinePoolScope.AWSMachinePool.Spec.MaxSize)),
193+
MinSize: aws.Int64(int64(machinePoolScope.AWSMachinePool.Spec.MinSize)),
194+
VPCZoneIdentifier: aws.String(strings.Join(subnets, ", ")),
195+
DefaultCooldown: aws.Int64(int64(machinePoolScope.AWSMachinePool.Spec.DefaultCoolDown.Duration.Seconds())),
196+
DefaultInstanceWarmup: aws.Int64(int64(machinePoolScope.AWSMachinePool.Spec.DefaultInstanceWarmup.Duration.Seconds())),
197+
CapacityRebalance: aws.Bool(machinePoolScope.AWSMachinePool.Spec.CapacityRebalance),
198+
LifecycleHookSpecificationList: getLifecycleHookSpecificationList(machinePoolScope.GetLifecycleHooks()),
229199
}
230200

231-
if i.DesiredCapacity != nil {
232-
input.DesiredCapacity = aws.Int64(int64(aws.Int32Value(i.DesiredCapacity)))
201+
if desiredCapacity != nil {
202+
input.DesiredCapacity = aws.Int64(int64(aws.Int32Value(desiredCapacity)))
233203
}
234204

235-
if i.MixedInstancesPolicy != nil {
236-
input.MixedInstancesPolicy = createSDKMixedInstancesPolicy(i.Name, i.MixedInstancesPolicy)
205+
if machinePoolScope.AWSMachinePool.Spec.MixedInstancesPolicy != nil {
206+
input.MixedInstancesPolicy = createSDKMixedInstancesPolicy(name, machinePoolScope.AWSMachinePool.Spec.MixedInstancesPolicy)
237207
} else {
238208
input.LaunchTemplate = &autoscaling.LaunchTemplateSpecification{
239-
LaunchTemplateId: aws.String(launchTemplateID),
209+
LaunchTemplateId: aws.String(machinePoolScope.AWSMachinePool.Status.LaunchTemplateID),
240210
Version: aws.String(expinfrav1.LaunchTemplateLatestVersion),
241211
}
242212
}
243213

244-
if i.Tags != nil {
245-
input.Tags = BuildTagsFromMap(i.Name, i.Tags)
246-
}
214+
input.Tags = BuildTagsFromMap(name, infrav1.Build(infrav1.BuildParams{
215+
ClusterName: s.scope.KubernetesClusterName(),
216+
Lifecycle: infrav1.ResourceLifecycleOwned,
217+
Name: aws.String(name),
218+
Role: aws.String("node"),
219+
Additional: additionalTags,
220+
}))
247221

248222
if _, err := s.ASGClient.CreateAutoScalingGroupWithContext(context.TODO(), input); err != nil {
249-
return errors.Wrap(err, "failed to create autoscaling group")
223+
s.scope.Error(err, "unable to create AutoScalingGroup")
224+
return nil, errors.Wrap(err, "failed to create autoscaling group")
250225
}
251226

252-
return nil
227+
record.Eventf(machinePoolScope.AWSMachinePool, "SuccessfulCreate", "Created new ASG: %s", machinePoolScope.Name())
228+
229+
return nil, nil
253230
}
254231

255232
// DeleteASGAndWait will delete an ASG and wait until it is deleted.

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

Lines changed: 41 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -51,35 +51,34 @@ func (s *Service) DescribeLifecycleHooks(asgName string) ([]*expinfrav1.AWSLifec
5151
return hooks, nil
5252
}
5353

54-
// CreateLifecycleHook creates a lifecycle hook for the given AutoScalingGroup.
55-
func (s *Service) CreateLifecycleHook(asgName string, hook *expinfrav1.AWSLifecycleHook) error {
56-
input := &autoscaling.PutLifecycleHookInput{
54+
func getPutLifecycleHookInput(asgName string, hook *expinfrav1.AWSLifecycleHook) (ret *autoscaling.PutLifecycleHookInput) {
55+
ret = &autoscaling.PutLifecycleHookInput{
5756
AutoScalingGroupName: aws.String(asgName),
5857
LifecycleHookName: aws.String(hook.Name),
5958
LifecycleTransition: aws.String(hook.LifecycleTransition.String()),
59+
60+
// Optional
61+
RoleARN: hook.RoleARN,
62+
NotificationTargetARN: hook.NotificationTargetARN,
63+
NotificationMetadata: hook.NotificationMetadata,
6064
}
6165

6266
// Optional parameters
6367
if hook.DefaultResult != nil {
64-
input.DefaultResult = aws.String(hook.DefaultResult.String())
68+
ret.DefaultResult = aws.String(hook.DefaultResult.String())
6569
}
6670

6771
if hook.HeartbeatTimeout != nil {
6872
timeoutSeconds := hook.HeartbeatTimeout.Duration.Seconds()
69-
input.HeartbeatTimeout = aws.Int64(int64(timeoutSeconds))
73+
ret.HeartbeatTimeout = aws.Int64(int64(timeoutSeconds))
7074
}
7175

72-
if hook.NotificationTargetARN != nil {
73-
input.NotificationTargetARN = hook.NotificationTargetARN
74-
}
75-
76-
if hook.RoleARN != nil {
77-
input.RoleARN = hook.RoleARN
78-
}
76+
return
77+
}
7978

80-
if hook.NotificationMetadata != nil {
81-
input.NotificationMetadata = hook.NotificationMetadata
82-
}
79+
// CreateLifecycleHook creates a lifecycle hook for the given AutoScalingGroup.
80+
func (s *Service) CreateLifecycleHook(asgName string, hook *expinfrav1.AWSLifecycleHook) error {
81+
input := getPutLifecycleHookInput(asgName, hook)
8382

8483
if _, err := s.ASGClient.PutLifecycleHookWithContext(context.TODO(), input); err != nil {
8584
return errors.Wrapf(err, "failed to create lifecycle hook %q for AutoScalingGroup: %q", hook.Name, asgName)
@@ -90,33 +89,7 @@ func (s *Service) CreateLifecycleHook(asgName string, hook *expinfrav1.AWSLifecy
9089

9190
// UpdateLifecycleHook updates a lifecycle hook for the given AutoScalingGroup.
9291
func (s *Service) UpdateLifecycleHook(asgName string, hook *expinfrav1.AWSLifecycleHook) error {
93-
input := &autoscaling.PutLifecycleHookInput{
94-
AutoScalingGroupName: aws.String(asgName),
95-
LifecycleHookName: aws.String(hook.Name),
96-
LifecycleTransition: aws.String(hook.LifecycleTransition.String()),
97-
}
98-
99-
// Optional parameters
100-
if hook.DefaultResult != nil {
101-
input.DefaultResult = aws.String(hook.DefaultResult.String())
102-
}
103-
104-
if hook.HeartbeatTimeout != nil {
105-
timeoutSeconds := hook.HeartbeatTimeout.Duration.Seconds()
106-
input.HeartbeatTimeout = aws.Int64(int64(timeoutSeconds))
107-
}
108-
109-
if hook.NotificationTargetARN != nil {
110-
input.NotificationTargetARN = hook.NotificationTargetARN
111-
}
112-
113-
if hook.RoleARN != nil {
114-
input.RoleARN = hook.RoleARN
115-
}
116-
117-
if hook.NotificationMetadata != nil {
118-
input.NotificationMetadata = hook.NotificationMetadata
119-
}
92+
input := getPutLifecycleHookInput(asgName, hook)
12093

12194
if _, err := s.ASGClient.PutLifecycleHookWithContext(context.TODO(), input); err != nil {
12295
return errors.Wrapf(err, "failed to update lifecycle hook %q for AutoScalingGroup: %q", hook.Name, asgName)
@@ -160,6 +133,32 @@ func (s *Service) SDKToLifecycleHook(hook *autoscaling.LifecycleHook) *expinfrav
160133
}
161134
}
162135

136+
func getLifecycleHookSpecificationList(lifecycleHooks []expinfrav1.AWSLifecycleHook) (ret []*autoscaling.LifecycleHookSpecification) {
137+
for _, hook := range lifecycleHooks {
138+
spec := &autoscaling.LifecycleHookSpecification{
139+
LifecycleHookName: aws.String(hook.Name),
140+
LifecycleTransition: aws.String(hook.LifecycleTransition.String()),
141+
142+
// Optional
143+
RoleARN: hook.RoleARN,
144+
NotificationTargetARN: hook.NotificationTargetARN,
145+
NotificationMetadata: hook.NotificationMetadata,
146+
}
147+
148+
// Optional parameters
149+
if hook.DefaultResult != nil {
150+
spec.DefaultResult = aws.String(hook.DefaultResult.String())
151+
}
152+
153+
if hook.HeartbeatTimeout != nil {
154+
timeoutSeconds := hook.HeartbeatTimeout.Duration.Seconds()
155+
spec.HeartbeatTimeout = aws.Int64(int64(timeoutSeconds))
156+
}
157+
}
158+
159+
return
160+
}
161+
163162
// ReconcileLifecycleHooks reconciles lifecycle hooks for an ASG
164163
// by creating missing hooks, updating mismatching hooks and
165164
// deleting extraneous hooks (except those specified in

0 commit comments

Comments
 (0)