Skip to content

Commit 2fda86a

Browse files
author
cskh
authored
fix(analysis): surface analysis validation failure to rollout status (argoproj#1833)
Signed-off-by: Hui Kang <[email protected]>
1 parent 34b4375 commit 2fda86a

File tree

2 files changed

+45
-0
lines changed

2 files changed

+45
-0
lines changed

pkg/apis/rollouts/validation/validation_references.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,18 @@ func ValidateAnalysisTemplatesWithType(rollout *v1alpha1.Rollout, templates Anal
133133
return allErrs
134134
}
135135

136+
if rollout.Spec.Strategy.Canary != nil {
137+
for _, step := range rollout.Spec.Strategy.Canary.Steps {
138+
if step.Analysis != nil {
139+
_, err := analysisutil.NewAnalysisRunFromTemplates(templates.AnalysisTemplates, templates.ClusterAnalysisTemplates, buildAnalysisArgs(templates.Args, rollout), step.Analysis.DryRun, step.Analysis.MeasurementRetention, "", "", "")
140+
if err != nil {
141+
allErrs = append(allErrs, field.Invalid(fldPath, value, err.Error()))
142+
return allErrs
143+
}
144+
}
145+
}
146+
}
147+
136148
for _, template := range templates.AnalysisTemplates {
137149
allErrs = append(allErrs, ValidateAnalysisTemplateWithType(rollout, template, nil, templates.TemplateType, fldPath)...)
138150
}

pkg/apis/rollouts/validation/validation_references_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,39 @@ func TestValidateAnalysisTemplatesWithType(t *testing.T) {
256256
assert.Empty(t, allErrs)
257257
})
258258

259+
t.Run("failure - duplicate MeasurementRetention", func(t *testing.T) {
260+
rollout := getRollout()
261+
rollout.Spec.Strategy.Canary.Steps = append(rollout.Spec.Strategy.Canary.Steps, v1alpha1.CanaryStep{
262+
Analysis: &v1alpha1.RolloutAnalysis{
263+
Templates: []v1alpha1.RolloutAnalysisTemplate{
264+
{
265+
TemplateName: "analysis-template-name",
266+
},
267+
},
268+
MeasurementRetention: []v1alpha1.MeasurementRetention{
269+
{
270+
MetricName: "example",
271+
Limit: 2,
272+
},
273+
},
274+
},
275+
})
276+
templates := getAnalysisTemplatesWithType()
277+
templates.AnalysisTemplates[0].Spec.Args = append(templates.AnalysisTemplates[0].Spec.Args, v1alpha1.Argument{Name: "valid"})
278+
templates.AnalysisTemplates[0].Spec.MeasurementRetention = []v1alpha1.MeasurementRetention{
279+
{
280+
MetricName: "example",
281+
Limit: 5,
282+
},
283+
}
284+
templates.Args = []v1alpha1.AnalysisRunArgument{{Name: "valid", Value: "true"}}
285+
286+
allErrs := ValidateAnalysisTemplatesWithType(rollout, templates)
287+
assert.Len(t, allErrs, 1)
288+
msg := fmt.Sprintf("spec.strategy.canary.steps[0].analysis.templates: Invalid value: \"templateNames: [analysis-template-name cluster-analysis-template-name]\": two Measurement Retention metric rules have the same name 'example'")
289+
assert.Equal(t, msg, allErrs[0].Error())
290+
})
291+
259292
}
260293

261294
func TestValidateAnalysisTemplateWithType(t *testing.T) {

0 commit comments

Comments
 (0)