Skip to content

Commit 5537bb6

Browse files
authored
Merge pull request #17321 from justinsb/verify_can_marshal
Fix JSON circular dependency in ASG / WarmPool
2 parents 9fab628 + 7dc29d2 commit 5537bb6

File tree

4 files changed

+21
-8
lines changed

4 files changed

+21
-8
lines changed

cmd/kops/integration_test.go

+11-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"crypto/rand"
2323
"crypto/rsa"
2424
"crypto/x509"
25+
"encoding/json"
2526
"encoding/pem"
2627
"fmt"
2728
"os"
@@ -1203,10 +1204,19 @@ func (i *integrationTest) runTest(t *testing.T, ctx context.Context, h *testutil
12031204
options.ClusterName = i.clusterName
12041205
options.LifecycleOverrides = i.lifecycleOverrides
12051206

1206-
_, err := RunUpdateCluster(ctx, factory, &stdout, options)
1207+
updateClusterResults, err := RunUpdateCluster(ctx, factory, &stdout, options)
12071208
if err != nil {
12081209
t.Fatalf("error running update cluster %q: %v", i.clusterName, err)
12091210
}
1211+
1212+
// Verify that we can print all the tasks
1213+
// Catches bugs like https://github.com/kubernetes/kops/issues/17316
1214+
for key, task := range updateClusterResults.TaskMap {
1215+
if _, err := json.Marshal(task); err != nil {
1216+
t.Errorf("unable to marshal task %q of type %T to json: %v", key, task, err)
1217+
}
1218+
}
1219+
12101220
}
12111221

12121222
// Compare main files

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

upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go

+4-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
}

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)