Add RolloutAnalysis check on presence of metrics before trying to create AnalysisRuns

[kevinqian-db]

Summary

Today we have the following when reconciling AnalysisRuns (ARs)

argo-rollouts/rollout/analysis.go

Lines 314 to 319 in 46f1427

	func (c *rolloutContext) reconcileBackgroundAnalysisRun() (*v1alpha1.AnalysisRun, error) {
	currentAr := c.currentArs.CanaryBackground
	if c.rollout.Spec.Strategy.Canary.Analysis == nil {
	err := c.cancelAnalysisRuns([]*v1alpha1.AnalysisRun{currentAr})
	return nil, err
	}

However, this check will not catch the case if the analysis field is an empty object vs a nil. If an empty object is placed here, the subsequent AR creation code will run, and eventually result in the failure spec.metrics is empty and errors out. Argo today will keep retry this failure and it will never realize that it can never create this AR.

Right now we did encounter that some entity (we are not sure which one, might even been Argo itself) setting it to empty when trying to skip health checks during an emergency fix. Nevertheless, we believe having an extra check that the configuration here actually yields any metrics should still be beneficial to avoid unintended infinite retry.

Use Cases

When user passed an empty object instead of nil.

---

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.

[zachaller]

Yea, we actually get hit by by this as well, would be open to a fix.

[zachaller]

Having a rollout object with args but no templates I think also recreates this?

      analysis:
        args:
          - name: stable-hash
            valueFrom:
              podTemplateHashValue: Stable
        startingStep: 1

[kevinqian-db]

Having a rollout object with args but no templates I think also recreates this?

Yeah. My idea is to probably check that if the manifest does not provide any templates

[kevinqian-db]

Create PR #3446, PTAL