Skip to content

Commit 6a3447b

Browse files
youngLiuHYclaude
youngLiuHY
and
claude
committed
feat: adapt to V2 strategy API - remove inline strategy, support BatchV2/CanaryV2 on RolloutStrategy
Adapt rollout codebase to kube-api PR #49 which removes CanaryStrategy, BatchStrategy and Webhooks from RolloutSpec and adds CanaryV2/BatchV2 to RolloutStrategy. Key changes: - Remove inline strategy validation from RolloutSpec, require StrategyRef - Add V1/V2 mutual exclusion validation for RolloutStrategy - Add V2 construction path: constructRolloutRunCanaryV2, constructRolloutRunBatchesV2, resolveRolloutTargets - Update OneTimeStrategy: InlineBatch → BatchV2, ConvertFromInline → ConvertFromV2 - Remove inline_strategy.go, merge helpers into utils.go - Update CRD YAML and config samples for new API shape - Simplify constructRolloutRun error return (always succeeds now) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c25733b commit 6a3447b

19 files changed

Lines changed: 9790 additions & 10432 deletions

‎apis/rollout/v1alpha1/validation/rollout.go‎

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -42,44 +42,9 @@ func ValidateRolloutSpec(spec *rolloutv1alpha1.RolloutSpec, fldPath *field.Path,
4242
allErrs = append(allErrs, field.NotSupported(fldPath.Child("triggerPolicy"), spec.TriggerPolicy, []string{string(rolloutv1alpha1.AutoTriggerPolicy), string(rolloutv1alpha1.ManualTriggerPolicy)}))
4343
}
4444

45-
// Validate strategy mutual exclusion: StrategyRef vs inline strategies (CanaryStrategy and BatchStrategy)
46-
hasStrategyRef := len(spec.StrategyRef) > 0
47-
hasCanary := spec.CanaryStrategy != nil
48-
hasBatch := spec.BatchStrategy != nil
49-
50-
// StrategyRef is mutually exclusive with inline strategies
51-
if hasStrategyRef && hasBatch {
52-
allErrs = append(allErrs, field.Invalid(
53-
fldPath,
54-
spec.StrategyRef,
55-
"strategyRef is mutually exclusive with batchStrategy",
56-
))
57-
}
58-
59-
if hasCanary && !hasBatch {
60-
allErrs = append(allErrs, field.Invalid(
61-
fldPath,
62-
spec.StrategyRef,
63-
"batchStrategy is required when canaryStrategy is set",
64-
))
65-
}
66-
67-
// Must specify at least one strategy
68-
if !hasStrategyRef && !hasBatch {
69-
allErrs = append(allErrs, field.Required(
70-
fldPath.Child("strategyRef"),
71-
"must specify either strategyRef, or batchStrategy",
72-
))
73-
}
74-
75-
// Validate CanaryStrategy if present
76-
if hasCanary {
77-
allErrs = append(allErrs, ValidateRolloutRunCanaryStrategy(spec.CanaryStrategy, fldPath.Child("canaryStrategy"))...)
78-
}
79-
80-
// Validate BatchStrategy if present
81-
if hasBatch {
82-
allErrs = append(allErrs, ValidateRolloutRunBatchStrategy(spec.BatchStrategy, fldPath.Child("batchStrategy"))...)
45+
// StrategyRef is required
46+
if len(spec.StrategyRef) == 0 {
47+
allErrs = append(allErrs, field.Required(fldPath.Child("strategyRef"), "strategyRef is required"))
8348
}
8449

8550
allErrs = append(allErrs, ValidateWorkloadRef(&spec.WorkloadRef, fldPath.Child("workloadRef"), isSupportedGVK)...)

‎apis/rollout/v1alpha1/validation/rollout_test.go‎

Lines changed: 1 addition & 306 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121

2222
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2323
"k8s.io/apimachinery/pkg/runtime/schema"
24-
"k8s.io/apimachinery/pkg/util/intstr"
2524
rolloutv1alpha1 "kusionstack.io/kube-api/rollout/v1alpha1"
2625
)
2726

@@ -86,7 +85,7 @@ func TestValidateRollout(t *testing.T) {
8685
wantErr: true,
8786
},
8887
{
89-
name: "invalid strategy ref",
88+
name: "empty strategy ref",
9089
obj: func() *rolloutv1alpha1.Rollout {
9190
ro := validRollout.DeepCopy()
9291
ro.Spec.StrategyRef = ""
@@ -122,307 +121,3 @@ func TestValidateRollout(t *testing.T) {
122121
})
123122
}
124123
}
125-
126-
// Tests for inline batch strategy validation
127-
func TestValidateRollout_InlineBatchStrategy(t *testing.T) {
128-
tests := []struct {
129-
name string
130-
obj *rolloutv1alpha1.Rollout
131-
isSupportedGVK SupportedGVKFunc
132-
wantErr bool
133-
errMsg string
134-
}{
135-
{
136-
name: "valid inline batch strategy",
137-
obj: &rolloutv1alpha1.Rollout{
138-
ObjectMeta: validMetadata,
139-
Spec: rolloutv1alpha1.RolloutSpec{
140-
TriggerPolicy: "Auto",
141-
WorkloadRef: validWorkloadRef,
142-
BatchStrategy: &rolloutv1alpha1.RolloutRunBatchStrategy{
143-
Batches: []rolloutv1alpha1.RolloutRunStep{
144-
{
145-
Targets: []rolloutv1alpha1.RolloutRunStepTarget{
146-
{
147-
CrossClusterObjectNameReference: rolloutv1alpha1.CrossClusterObjectNameReference{
148-
Cluster: "cluster-a",
149-
Name: "test",
150-
},
151-
Replicas: intstr.FromString("20%"),
152-
},
153-
},
154-
},
155-
},
156-
},
157-
},
158-
},
159-
isSupportedGVK: supportAllGVK,
160-
wantErr: false,
161-
},
162-
{
163-
name: "batch strategy with multiple batches",
164-
obj: &rolloutv1alpha1.Rollout{
165-
ObjectMeta: validMetadata,
166-
Spec: rolloutv1alpha1.RolloutSpec{
167-
TriggerPolicy: "Auto",
168-
WorkloadRef: validWorkloadRef,
169-
BatchStrategy: &rolloutv1alpha1.RolloutRunBatchStrategy{
170-
Batches: []rolloutv1alpha1.RolloutRunStep{
171-
{
172-
Breakpoint: true,
173-
Targets: []rolloutv1alpha1.RolloutRunStepTarget{
174-
{
175-
CrossClusterObjectNameReference: rolloutv1alpha1.CrossClusterObjectNameReference{
176-
Cluster: "cluster-a",
177-
Name: "test",
178-
},
179-
Replicas: intstr.FromString("20%"),
180-
},
181-
},
182-
},
183-
{
184-
Targets: []rolloutv1alpha1.RolloutRunStepTarget{
185-
{
186-
CrossClusterObjectNameReference: rolloutv1alpha1.CrossClusterObjectNameReference{
187-
Cluster: "cluster-a",
188-
Name: "test",
189-
},
190-
Replicas: intstr.FromString("100%"),
191-
},
192-
},
193-
},
194-
},
195-
},
196-
},
197-
},
198-
isSupportedGVK: supportAllGVK,
199-
wantErr: false,
200-
},
201-
{
202-
name: "batch strategy with multiple targets in one batch",
203-
obj: &rolloutv1alpha1.Rollout{
204-
ObjectMeta: validMetadata,
205-
Spec: rolloutv1alpha1.RolloutSpec{
206-
TriggerPolicy: "Auto",
207-
WorkloadRef: validWorkloadRef,
208-
BatchStrategy: &rolloutv1alpha1.RolloutRunBatchStrategy{
209-
Batches: []rolloutv1alpha1.RolloutRunStep{
210-
{
211-
Targets: []rolloutv1alpha1.RolloutRunStepTarget{
212-
{
213-
CrossClusterObjectNameReference: rolloutv1alpha1.CrossClusterObjectNameReference{
214-
Cluster: "cluster-a",
215-
Name: "test-1",
216-
},
217-
Replicas: intstr.FromString("20%"),
218-
},
219-
{
220-
CrossClusterObjectNameReference: rolloutv1alpha1.CrossClusterObjectNameReference{
221-
Cluster: "cluster-b",
222-
Name: "test-2",
223-
},
224-
Replicas: intstr.FromString("30%"),
225-
},
226-
},
227-
},
228-
},
229-
},
230-
},
231-
},
232-
isSupportedGVK: supportAllGVK,
233-
wantErr: false,
234-
},
235-
{
236-
name: "empty batch strategy - no batches",
237-
obj: &rolloutv1alpha1.Rollout{
238-
ObjectMeta: validMetadata,
239-
Spec: rolloutv1alpha1.RolloutSpec{
240-
TriggerPolicy: "Auto",
241-
WorkloadRef: validWorkloadRef,
242-
BatchStrategy: &rolloutv1alpha1.RolloutRunBatchStrategy{
243-
Batches: []rolloutv1alpha1.RolloutRunStep{},
244-
},
245-
},
246-
},
247-
isSupportedGVK: supportAllGVK,
248-
wantErr: true,
249-
errMsg: "must specify at least one batch",
250-
},
251-
{
252-
name: "batch with empty targets",
253-
obj: &rolloutv1alpha1.Rollout{
254-
ObjectMeta: validMetadata,
255-
Spec: rolloutv1alpha1.RolloutSpec{
256-
TriggerPolicy: "Auto",
257-
WorkloadRef: validWorkloadRef,
258-
BatchStrategy: &rolloutv1alpha1.RolloutRunBatchStrategy{
259-
Batches: []rolloutv1alpha1.RolloutRunStep{
260-
{
261-
Targets: []rolloutv1alpha1.RolloutRunStepTarget{},
262-
},
263-
},
264-
},
265-
},
266-
},
267-
isSupportedGVK: supportAllGVK,
268-
wantErr: true,
269-
errMsg: "must specify at least one target",
270-
},
271-
{
272-
name: "target missing name",
273-
obj: &rolloutv1alpha1.Rollout{
274-
ObjectMeta: validMetadata,
275-
Spec: rolloutv1alpha1.RolloutSpec{
276-
TriggerPolicy: "Auto",
277-
WorkloadRef: validWorkloadRef,
278-
BatchStrategy: &rolloutv1alpha1.RolloutRunBatchStrategy{
279-
Batches: []rolloutv1alpha1.RolloutRunStep{
280-
{
281-
Targets: []rolloutv1alpha1.RolloutRunStepTarget{
282-
{
283-
CrossClusterObjectNameReference: rolloutv1alpha1.CrossClusterObjectNameReference{
284-
Cluster: "cluster-a",
285-
},
286-
Replicas: intstr.FromString("20%"),
287-
},
288-
},
289-
},
290-
},
291-
},
292-
},
293-
},
294-
isSupportedGVK: supportAllGVK,
295-
wantErr: true,
296-
errMsg: "name is required",
297-
},
298-
{
299-
name: "target with invalid replicas",
300-
obj: &rolloutv1alpha1.Rollout{
301-
ObjectMeta: validMetadata,
302-
Spec: rolloutv1alpha1.RolloutSpec{
303-
TriggerPolicy: "Auto",
304-
WorkloadRef: validWorkloadRef,
305-
BatchStrategy: &rolloutv1alpha1.RolloutRunBatchStrategy{
306-
Batches: []rolloutv1alpha1.RolloutRunStep{
307-
{
308-
Targets: []rolloutv1alpha1.RolloutRunStepTarget{
309-
{
310-
CrossClusterObjectNameReference: rolloutv1alpha1.CrossClusterObjectNameReference{
311-
Cluster: "cluster-a",
312-
Name: "test",
313-
},
314-
Replicas: intstr.FromString("-1"),
315-
},
316-
},
317-
},
318-
},
319-
},
320-
},
321-
},
322-
isSupportedGVK: supportAllGVK,
323-
wantErr: true,
324-
},
325-
}
326-
327-
for _, tt := range tests {
328-
t.Run(tt.name, func(t *testing.T) {
329-
got := ValidateRollout(tt.obj, tt.isSupportedGVK)
330-
hasErr := got.ToAggregate() != nil
331-
if tt.wantErr != hasErr {
332-
t.Errorf("ValidateRollout() error = %v, wantErr %v", got.ToAggregate(), tt.wantErr)
333-
return
334-
}
335-
if tt.wantErr && tt.errMsg != "" {
336-
errStr := got.ToAggregate().Error()
337-
if !containsString(errStr, tt.errMsg) {
338-
t.Errorf("ValidateRollout() error message = %q, should contain %q", errStr, tt.errMsg)
339-
}
340-
}
341-
})
342-
}
343-
}
344-
345-
// Tests for StrategyRef and inline strategy mutual exclusion
346-
func TestValidateRollout_StrategyMutualExclusion(t *testing.T) {
347-
tests := []struct {
348-
name string
349-
obj *rolloutv1alpha1.Rollout
350-
isSupportedGVK SupportedGVKFunc
351-
wantErr bool
352-
errMsg string
353-
}{
354-
{
355-
name: "StrategyRef is mutually exclusive with BatchStrategy",
356-
obj: &rolloutv1alpha1.Rollout{
357-
ObjectMeta: validMetadata,
358-
Spec: rolloutv1alpha1.RolloutSpec{
359-
TriggerPolicy: "Auto",
360-
StrategyRef: "strategy-a",
361-
WorkloadRef: validWorkloadRef,
362-
BatchStrategy: &rolloutv1alpha1.RolloutRunBatchStrategy{
363-
Batches: []rolloutv1alpha1.RolloutRunStep{
364-
{
365-
Targets: []rolloutv1alpha1.RolloutRunStepTarget{
366-
{
367-
CrossClusterObjectNameReference: rolloutv1alpha1.CrossClusterObjectNameReference{
368-
Cluster: "cluster-a",
369-
Name: "test",
370-
},
371-
Replicas: intstr.FromString("20%"),
372-
},
373-
},
374-
},
375-
},
376-
},
377-
},
378-
},
379-
isSupportedGVK: supportAllGVK,
380-
wantErr: true,
381-
errMsg: "strategyRef is mutually exclusive with batchStrategy",
382-
},
383-
{
384-
name: "neither StrategyRef nor BatchStrategy specified",
385-
obj: &rolloutv1alpha1.Rollout{
386-
ObjectMeta: validMetadata,
387-
Spec: rolloutv1alpha1.RolloutSpec{
388-
TriggerPolicy: "Auto",
389-
WorkloadRef: validWorkloadRef,
390-
},
391-
},
392-
isSupportedGVK: supportAllGVK,
393-
wantErr: true,
394-
errMsg: "must specify either strategyRef, or batchStrategy",
395-
},
396-
}
397-
398-
for _, tt := range tests {
399-
t.Run(tt.name, func(t *testing.T) {
400-
got := ValidateRollout(tt.obj, tt.isSupportedGVK)
401-
hasErr := got.ToAggregate() != nil
402-
if tt.wantErr != hasErr {
403-
t.Errorf("ValidateRollout() error = %v, wantErr %v", got.ToAggregate(), tt.wantErr)
404-
return
405-
}
406-
if tt.wantErr && tt.errMsg != "" {
407-
errStr := got.ToAggregate().Error()
408-
if !containsString(errStr, tt.errMsg) {
409-
t.Errorf("ValidateRollout() error message = %q, should contain %q", errStr, tt.errMsg)
410-
}
411-
}
412-
})
413-
}
414-
}
415-
416-
// Helper function
417-
func containsString(s, substr string) bool {
418-
return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsStringSub(s, substr))
419-
}
420-
421-
func containsStringSub(s, substr string) bool {
422-
for i := 0; i <= len(s)-len(substr); i++ {
423-
if s[i:i+len(substr)] == substr {
424-
return true
425-
}
426-
}
427-
return false
428-
}

0 commit comments

Comments
 (0)