From 4327aa883c1fe750f45bb2ce28b7d78660e6cba3 Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Wed, 30 Oct 2019 11:16:15 -0700 Subject: [PATCH] Allow multiple analyses in experiments created by rollouts --- manifests/crds/rollout-crd.yaml | 57 +++++++++++-------- manifests/install.yaml | 57 +++++++++++-------- manifests/namespace-install.yaml | 57 +++++++++++-------- .../rollouts/v1alpha1/openapi_generated.go | 22 +++++-- pkg/apis/rollouts/v1alpha1/types.go | 6 +- .../v1alpha1/zz_generated.deepcopy.go | 10 ++-- rollout/analysis.go | 2 +- rollout/analysis_test.go | 3 + rollout/experiment.go | 8 +-- rollout/experiment_test.go | 6 +- 10 files changed, 142 insertions(+), 86 deletions(-) diff --git a/manifests/crds/rollout-crd.yaml b/manifests/crds/rollout-crd.yaml index 98a768f528..0f5317e063 100644 --- a/manifests/crds/rollout-crd.yaml +++ b/manifests/crds/rollout-crd.yaml @@ -129,9 +129,12 @@ spec: - name type: object type: array + name: + type: string templateName: type: string required: + - name - templateName type: object canaryService: @@ -165,36 +168,44 @@ spec: - name type: object type: array + name: + type: string templateName: type: string required: + - name - templateName type: object experiment: properties: - analysis: - properties: - arguments: - items: - properties: - name: - type: string - value: - type: string - valueFrom: - properties: - podTemplateHashValue: - type: string - type: object - required: - - name - type: object - type: array - templateName: - type: string - required: - - templateName - type: object + analyses: + items: + properties: + arguments: + items: + properties: + name: + type: string + value: + type: string + valueFrom: + properties: + podTemplateHashValue: + type: string + type: object + required: + - name + type: object + type: array + name: + type: string + templateName: + type: string + required: + - name + - templateName + type: object + type: array duration: format: int32 type: integer diff --git a/manifests/install.yaml b/manifests/install.yaml index b81b3b52c5..b27236f9ca 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -7856,9 +7856,12 @@ spec: - name type: object type: array + name: + type: string templateName: type: string required: + - name - templateName type: object canaryService: @@ -7892,36 +7895,44 @@ spec: - name type: object type: array + name: + type: string templateName: type: string required: + - name - templateName type: object experiment: properties: - analysis: - properties: - arguments: - items: - properties: - name: - type: string - value: - type: string - valueFrom: - properties: - podTemplateHashValue: - type: string - type: object - required: - - name - type: object - type: array - templateName: - type: string - required: - - templateName - type: object + analyses: + items: + properties: + arguments: + items: + properties: + name: + type: string + value: + type: string + valueFrom: + properties: + podTemplateHashValue: + type: string + type: object + required: + - name + type: object + type: array + name: + type: string + templateName: + type: string + required: + - name + - templateName + type: object + type: array duration: format: int32 type: integer diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index 2e00b40b62..8b305b81a7 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -7856,9 +7856,12 @@ spec: - name type: object type: array + name: + type: string templateName: type: string required: + - name - templateName type: object canaryService: @@ -7892,36 +7895,44 @@ spec: - name type: object type: array + name: + type: string templateName: type: string required: + - name - templateName type: object experiment: properties: - analysis: - properties: - arguments: - items: - properties: - name: - type: string - value: - type: string - valueFrom: - properties: - podTemplateHashValue: - type: string - type: object - required: - - name - type: object - type: array - templateName: - type: string - required: - - templateName - type: object + analyses: + items: + properties: + arguments: + items: + properties: + name: + type: string + value: + type: string + valueFrom: + properties: + podTemplateHashValue: + type: string + type: object + required: + - name + type: object + type: array + name: + type: string + templateName: + type: string + required: + - name + - templateName + type: object + type: array duration: format: int32 type: integer diff --git a/pkg/apis/rollouts/v1alpha1/openapi_generated.go b/pkg/apis/rollouts/v1alpha1/openapi_generated.go index b448e4ae56..266c23d02b 100644 --- a/pkg/apis/rollouts/v1alpha1/openapi_generated.go +++ b/pkg/apis/rollouts/v1alpha1/openapi_generated.go @@ -1507,6 +1507,13 @@ func schema_pkg_apis_rollouts_v1alpha1_RolloutAnalysisStep(ref common.ReferenceC SchemaProps: spec.SchemaProps{ Description: "RolloutAnalysisStep defines a template that is used to create a analysisRun", Properties: map[string]spec.Schema{ + "name": { + SchemaProps: spec.SchemaProps{ + Description: "Name the name for the analysisRun", + Type: []string{"string"}, + Format: "", + }, + }, "templateName": { SchemaProps: spec.SchemaProps{ Description: "TemplateName reference of the AnalysisTemplate name used by the Rollout to create the run", @@ -1528,7 +1535,7 @@ func schema_pkg_apis_rollouts_v1alpha1_RolloutAnalysisStep(ref common.ReferenceC }, }, }, - Required: []string{"templateName"}, + Required: []string{"name", "templateName"}, }, }, Dependencies: []string{ @@ -1617,10 +1624,17 @@ func schema_pkg_apis_rollouts_v1alpha1_RolloutExperimentStep(ref common.Referenc Format: "int32", }, }, - "analysis": { + "analyses": { SchemaProps: spec.SchemaProps{ - Description: "AnalysisTemplate what analysis to run with the experiment", - Ref: ref("github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisStep"), + Description: "Analyses what analyses to run with the experiment", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Ref: ref("github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisStep"), + }, + }, + }, }, }, }, diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go index e831a0943f..e27a4a98bc 100644 --- a/pkg/apis/rollouts/v1alpha1/types.go +++ b/pkg/apis/rollouts/v1alpha1/types.go @@ -153,8 +153,8 @@ type RolloutExperimentStep struct { // Duration is the duration in seconds that the experiment should run for // +optional Duration *int32 `json:"duration,omitempty"` - //AnalysisTemplate what analysis to run with the experiment - Analysis *RolloutAnalysisStep `json:"analysis,omitempty"` + //Analyses what analyses to run with the experiment + Analyses []RolloutAnalysisStep `json:"analyses,omitempty"` } // RolloutExperimentTemplate defines the template used to create experiments for the Rollout's experiment canary step @@ -207,6 +207,8 @@ type CanaryStep struct { // RolloutAnalysisStep defines a template that is used to create a analysisRun type RolloutAnalysisStep struct { + // Name the name for the analysisRun + Name string `json:"name"` // TemplateName reference of the AnalysisTemplate name used by the Rollout to create the run TemplateName string `json:"templateName"` // Arguments the arguments that will be added to the AnalysisRuns diff --git a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go index 536cc13e7b..6be4f5343d 100644 --- a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go @@ -904,10 +904,12 @@ func (in *RolloutExperimentStep) DeepCopyInto(out *RolloutExperimentStep) { *out = new(int32) **out = **in } - if in.Analysis != nil { - in, out := &in.Analysis, &out.Analysis - *out = new(RolloutAnalysisStep) - (*in).DeepCopyInto(*out) + if in.Analyses != nil { + in, out := &in.Analyses, &out.Analyses + *out = make([]RolloutAnalysisStep, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } diff --git a/rollout/analysis.go b/rollout/analysis.go index 1815251cc3..aa1de51823 100644 --- a/rollout/analysis.go +++ b/rollout/analysis.go @@ -205,7 +205,7 @@ func (c *RolloutController) getAnalysisRunFromRollout(roCtx *canaryContext, roll ar := v1alpha1.AnalysisRun{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: fmt.Sprintf("%s-%s-%s-", r.Name, rolloutAnalysisStep.TemplateName, podHash), + GenerateName: fmt.Sprintf("%s-%s-%s-", r.Name, rolloutAnalysisStep.Name, podHash), Namespace: r.Namespace, Labels: labels, Annotations: map[string]string{ diff --git a/rollout/analysis_test.go b/rollout/analysis_test.go index a9985611ff..22b435cebd 100644 --- a/rollout/analysis_test.go +++ b/rollout/analysis_test.go @@ -65,6 +65,7 @@ func TestCreateBackgroundAnalysisRun(t *testing.T) { ar := analysisRun(at, v1alpha1.RolloutTypeBackgroundRunLabel, r2) r2.Spec.Strategy.Canary.Analysis = &v1alpha1.RolloutAnalysisStep{ TemplateName: at.Name, + Name: at.Name, } rs1 := newReplicaSetWithStatus(r1, 10, 10) @@ -113,6 +114,7 @@ func TestCreateAnalysisRunOnAnalysisStep(t *testing.T) { steps := []v1alpha1.CanaryStep{{ Analysis: &v1alpha1.RolloutAnalysisStep{ TemplateName: at.Name, + Name: at.Name, }, }} @@ -164,6 +166,7 @@ func TestFailCreateStepAnalysisRunIfInvalidTemplateRef(t *testing.T) { steps := []v1alpha1.CanaryStep{{ Analysis: &v1alpha1.RolloutAnalysisStep{ TemplateName: "invalid-template-ref", + Name: "invalid-template-ref", }, }} diff --git a/rollout/experiment.go b/rollout/experiment.go index 637c25f068..820bae23d9 100644 --- a/rollout/experiment.go +++ b/rollout/experiment.go @@ -76,11 +76,11 @@ func GetExperimentFromTemplate(r *v1alpha1.Rollout, stableRS, newRS *appsv1.Repl experiment.Spec.Templates = append(experiment.Spec.Templates, template) } - if step.Analysis != nil { - args := analysisutil.BuildArgumentsForRolloutAnalysisRun(step.Analysis, stableRS, newRS) - analysis := step.Analysis + for i := range step.Analyses { + analysis := step.Analyses[i] + args := analysisutil.BuildArgumentsForRolloutAnalysisRun(&analysis, stableRS, newRS) analysisTemplate := v1alpha1.ExperimentAnalysisTemplateRef{ - Name: analysis.TemplateName, + Name: analysis.Name, TemplateName: analysis.TemplateName, Arguments: args, } diff --git a/rollout/experiment_test.go b/rollout/experiment_test.go index e52e19913e..0c6f63d478 100644 --- a/rollout/experiment_test.go +++ b/rollout/experiment_test.go @@ -26,9 +26,10 @@ func TestRolloutCreateExperiment(t *testing.T) { SpecRef: v1alpha1.StableSpecRef, Replicas: pointer.Int32Ptr(1), }}, - Analysis: &v1alpha1.RolloutAnalysisStep{ + Analyses: []v1alpha1.RolloutAnalysisStep{{ + Name: "test", TemplateName: at.Name, - }, + }}, }, }} @@ -54,6 +55,7 @@ func TestRolloutCreateExperiment(t *testing.T) { createdEx := f.getCreatedExperiment(createExIndex) assert.Equal(t, createdEx.GenerateName, ex.GenerateName) assert.Equal(t, createdEx.Spec.Analyses[0].TemplateName, at.Name) + assert.Equal(t, createdEx.Spec.Analyses[0].Name, "test") patch := f.getPatchedRollout(patchIndex) expectedPatch := `{ "status": {