Skip to content

Commit ebb9bad

Browse files
committed
Fix: Set spec.MaxNew for CLI
1 parent 401576c commit ebb9bad

2 files changed

Lines changed: 128 additions & 5 deletions

File tree

internal/cmd/cli/target.go

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,7 @@ func (t *Target) Run(cmd *cobra.Command, args []string) error {
152152
cmd.Println("---")
153153
cmd.Println(string(b))
154154

155-
// Needs to be set to print all targets. UpdatePartitions will only
156-
// create this many deployments if the bundle is new.
157-
bundle.Status.MaxNew = len(matchedTargets)
158-
159-
if err := target.UpdatePartitions(&bundle.Status, matchedTargets); err != nil {
155+
if err := stageAllTargets(bundle, matchedTargets); err != nil {
160156
return err
161157
}
162158
for _, target := range matchedTargets {
@@ -177,3 +173,20 @@ func (t *Target) Run(cmd *cobra.Command, args []string) error {
177173

178174
return nil
179175
}
176+
177+
// stageAllTargets overrides maxNew to the total number of matched targets so
178+
// that UpdatePartitions stages a deployment for every target, not just the
179+
// first target.DefaultMaxNew.
180+
//
181+
// NOTE: mutating bundle.Spec.RolloutStrategy is visible to UpdatePartitions
182+
// because target.Manager.Targets() stores the same bundle pointer in every
183+
// target's Bundle field, and UpdatePartitions reads MaxNew from
184+
// targets[0].Bundle.Spec.RolloutStrategy.
185+
func stageAllTargets(bundle *v1alpha1.Bundle, matchedTargets []*target.Target) error {
186+
count := len(matchedTargets)
187+
if bundle.Spec.RolloutStrategy == nil {
188+
bundle.Spec.RolloutStrategy = &v1alpha1.RolloutStrategy{}
189+
}
190+
bundle.Spec.RolloutStrategy.MaxNew = &count
191+
return target.UpdatePartitions(&bundle.Status, matchedTargets)
192+
}

internal/cmd/cli/target_test.go

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
package cli
2+
3+
import (
4+
"strconv"
5+
"testing"
6+
7+
"github.com/rancher/fleet/internal/cmd/controller/target"
8+
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10+
)
11+
12+
// newTargetsWithoutDeployments creates targets that share a single bundle
13+
// pointer, matching what the real target builder produces.
14+
func newTargetsWithoutDeployments(bundle *fleet.Bundle, count int) []*target.Target {
15+
targets := make([]*target.Target, count)
16+
for i := range count {
17+
targets[i] = &target.Target{
18+
Cluster: &fleet.Cluster{
19+
ObjectMeta: metav1.ObjectMeta{
20+
Namespace: "default",
21+
Name: "cluster-" + strconv.Itoa(i+1),
22+
},
23+
},
24+
Bundle: bundle,
25+
DeploymentID: "deployment-" + strconv.Itoa(i+1),
26+
}
27+
}
28+
return targets
29+
}
30+
31+
func Test_stageAllTargets(t *testing.T) {
32+
tests := []struct {
33+
name string
34+
targetCount int
35+
}{
36+
{
37+
name: "stages all targets when count exceeds default maxNew",
38+
targetCount: 100,
39+
},
40+
{
41+
name: "stages all targets when count is below default maxNew",
42+
targetCount: 10,
43+
},
44+
{
45+
name: "no targets produces no deployments",
46+
targetCount: 0,
47+
},
48+
}
49+
for _, tt := range tests {
50+
t.Run(tt.name, func(t *testing.T) {
51+
bundle := &fleet.Bundle{ObjectMeta: metav1.ObjectMeta{Name: "bundle-1"}}
52+
targets := newTargetsWithoutDeployments(bundle, tt.targetCount)
53+
54+
if err := stageAllTargets(bundle, targets); err != nil {
55+
t.Fatalf("stageAllTargets() failed: %v", err)
56+
}
57+
58+
deploymentCount := 0
59+
for _, tgt := range targets {
60+
if tgt.Deployment != nil {
61+
deploymentCount++
62+
}
63+
}
64+
if deploymentCount != tt.targetCount {
65+
t.Errorf("staged %d deployments, want %d", deploymentCount, tt.targetCount)
66+
}
67+
})
68+
}
69+
}
70+
71+
func Test_stageAllTargets_overwritesExistingMaxNew(t *testing.T) {
72+
// MaxNew=2 is less than the target count (5); if stageAllTargets does not
73+
// overwrite it, UpdatePartitions would only stage 2 deployments.
74+
two := 2
75+
bundle := &fleet.Bundle{ObjectMeta: metav1.ObjectMeta{Name: "bundle-1"}}
76+
bundle.Spec.RolloutStrategy = &fleet.RolloutStrategy{MaxNew: &two}
77+
targets := newTargetsWithoutDeployments(bundle, 5)
78+
79+
if err := stageAllTargets(bundle, targets); err != nil {
80+
t.Fatalf("stageAllTargets() failed: %v", err)
81+
}
82+
83+
deploymentCount := 0
84+
for _, tgt := range targets {
85+
if tgt.Deployment != nil {
86+
deploymentCount++
87+
}
88+
}
89+
if deploymentCount != 5 {
90+
t.Errorf("staged %d deployments, want 5", deploymentCount)
91+
}
92+
}
93+
94+
func Test_stageAllTargets_preservesExistingRolloutStrategy(t *testing.T) {
95+
ten := 10
96+
existing := &fleet.RolloutStrategy{
97+
AutoPartitionThreshold: &ten,
98+
}
99+
bundle := &fleet.Bundle{ObjectMeta: metav1.ObjectMeta{Name: "bundle-1"}}
100+
bundle.Spec.RolloutStrategy = existing
101+
targets := newTargetsWithoutDeployments(bundle, 5)
102+
103+
if err := stageAllTargets(bundle, targets); err != nil {
104+
t.Fatalf("stageAllTargets() failed: %v", err)
105+
}
106+
107+
if bundle.Spec.RolloutStrategy.AutoPartitionThreshold == nil || *bundle.Spec.RolloutStrategy.AutoPartitionThreshold != 10 {
108+
t.Error("existing rollout strategy fields were overwritten")
109+
}
110+
}

0 commit comments

Comments
 (0)