Skip to content

Commit b755098

Browse files
committed
Do not include WarmPool in json for AutoscalingGroup task
This avoids a circular dependency. I previously considered making the field private, but this is roughly equivalent and less disruptive.
1 parent 26effbe commit b755098

File tree

3 files changed

+20
-7
lines changed

3 files changed

+20
-7
lines changed

Diff for: pkg/model/awsmodel/autoscalinggroup.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,12 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.CloudupModelBuilderContext) e
8282

8383
// @step: now lets build the autoscaling group task
8484
if ig.Spec.Manager != "Karpenter" {
85-
tsk, err := b.buildAutoScalingGroupTask(c, name, ig)
85+
asg, err := b.buildAutoScalingGroupTask(c, name, ig)
8686
if err != nil {
8787
return err
8888
}
89-
tsk.LaunchTemplate = task
90-
c.AddTask(tsk)
89+
asg.LaunchTemplate = task
90+
c.AddTask(asg)
9191

9292
warmPool := b.Cluster.Spec.CloudProvider.AWS.WarmPool.ResolveDefaults(ig)
9393

@@ -103,9 +103,9 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.CloudupModelBuilderContext) e
103103
if warmPool.MaxSize != nil {
104104
warmPoolTask.MaxSize = fi.PtrTo(int32(aws.ToInt64(warmPool.MaxSize)))
105105
}
106-
tsk.WarmPool = warmPoolTask
106+
asg.WarmPool = warmPoolTask
107107
} else {
108-
tsk.WarmPool = nil
108+
asg.WarmPool = nil
109109
}
110110
c.AddTask(warmPoolTask)
111111

Diff for: upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go

+14-2
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,10 @@ type AutoscalingGroup struct {
100100
TargetGroups []*TargetGroup
101101
// CapacityRebalance makes ASG proactively replace spot instances when ASG receives a rebalance recommendation
102102
CapacityRebalance *bool
103-
// WarmPool is the WarmPool config for the ASG
104-
WarmPool *WarmPool
103+
104+
// WarmPool is the WarmPool config for the ASG.
105+
// It is marked to be ignored in JSON marshalling to avoid a circular dependency.
106+
WarmPool *WarmPool `json:"-"`
105107

106108
deletions []fi.CloudupDeletion
107109
}
@@ -316,6 +318,16 @@ func (e *AutoscalingGroup) Find(c *fi.CloudupContext) (*AutoscalingGroup, error)
316318
actual.InstanceProtection = g.NewInstancesProtectedFromScaleIn
317319
}
318320

321+
// // Populate the warmPool, by looking at the tasks
322+
// for _, task := range c.AllTasks() {
323+
// switch task := task.(type) {
324+
// case *WarmPool:
325+
// if task.AutoscalingGroup != nil && fi.ValueOf(task.AutoscalingGroup.Name) == fi.ValueOf(e.Name) {
326+
// e.warmPool = task
327+
// }
328+
// }
329+
// }
330+
319331
return actual, nil
320332
}
321333

Diff for: upup/pkg/fi/cloudup/awstasks/warmpool.go

+1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ type WarmPool struct {
4141
// MinSize is the smallest number of nodes in the warm pool.
4242
MinSize int32
4343

44+
// AutoscalingGroup is the AutoscalingGroup on which we configure this warmpool.
4445
AutoscalingGroup *AutoscalingGroup
4546
}
4647

0 commit comments

Comments
 (0)