Analysis Template argument without default value can not be resolved in the rollout yaml

[huikang]

Summary

When AnalysisTemplate has an argument what does not have a default value or valuefrom. The controller's validation failed due to unresolved argument name.

# AnalysisTemplate which sleeps for a specified duration and exits with a specified exit-code
kind: AnalysisTemplate
apiVersion: argoproj.io/v1alpha1
metadata:
  name: sleep-job
spec:
  args:
  - name: duration
  - name: exit-code
    value: "0"
  metrics:
    - name: sleep-job
      count: 1
      provider:
        job:
          spec:
            template:
              spec:
                containers:
                  - name: sleep-job
                    image: nginx:1.19-alpine
                    command: [sh, -c, -x]
                    args: ["sleep {{args.duration}} && exit {{args.exit-code}}"]
                restartPolicy: Never
            backoffLimit: 0
---
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
  name: rollout-bluegreen
spec:
  replicas: 3
  revisionHistoryLimit: 2
  selector:
    matchLabels:
      app: rollout-bluegreen
  template:
    metadata:
      labels:
        app: rollout-bluegreen
    spec:
      containers:
        - name: rollouts-demo
          image: argoproj/rollouts-demo:blue
          imagePullPolicy: Always
          ports:
            - containerPort: 8080
              name: http
              protocol: TCP
  strategy:
    blueGreen:
      activeService: rollout-bluegreen-active
      previewService: rollout-bluegreen-preview
      autoPromotionEnabled: false
      prePromotionAnalysis:
        templates:
          - templateName: sleep-job
        args:
        - name: duration
          value: "5"
---
apiVersion: v1
kind: Service
metadata:
  name: rollout-bluegreen-active
spec:
  ports:
    - port: 80
      targetPort: http
      protocol: TCP
      name: http
  selector:
    app: rollout-bluegreen
---
apiVersion: v1
kind: Service
metadata:
  name: rollout-bluegreen-preview
spec:
  ports:
    - port: 80
      targetPort: http
      protocol: TCP
      name: http
  selector:
    app: rollout-bluegreen

Diagnostics

What version of Argo Rollouts are you running?

commit id: 912d3ac

# Paste the logs from the rollout controller

# Logs for the entire controller:
kubectl logs -n argo-rollouts deployment/argo-rollouts

mespace=default rollout=rollout-bluegreen
INFO[2021-05-20T16:15:19-04:00] Reconciliation completed                      generation=2 namespace=default resourceVersion=33424 rollout=rollout-bluegreen time_ms=1.261676
INFO[2021-05-20T16:15:39-04:00] Started syncing rollout                       generation=2 namespace=default resourceVersion=33424 rollout=rollout-bluegreen
ERRO[2021-05-20T16:15:39-04:00] The Rollout "rollout-bluegreen" is invalid: spec.strategy.blueGreen.prePromotionAnalysis.templates: Invalid value: "templateNames: [sleep-job]": args.duration was not resolved  namespace=default rollout=rollout-bluegreen
INFO[2021-05-20T16:15:39-04:00] Reconciliation completed                      generation=2 namespace=default resourceVersion=33424 rollout=rollout-bluegreen time_ms=1.2937
INFO[2021-05-20T16:15:59-04:00] Started syncing rollout                       generation=2 namespace=default resourceVersion=33424 rollout=rollout-bluegreen
ERRO[2021-05-20T16:15:59-04:00] The Rollout "rollout-bluegreen" is invalid: spec.strategy.blueGreen.prePromotionAnalysis.templates: Invalid value: "templateNames: [sleep-job]": args.duration was not resolved  namespace=default rollout=rollout-bluegreen
INFO[2021-05-20T16:15:59-04:00] Reconciliation completed                      generation=2 namespace=default resourceVersion=33424 rollout=rollout-bluegreen time_ms=1.0176180000000001
INFO[2021-05-20T16:16:19-04:00] Started syncing rollout                       generation=2 namespace=default resourceVersion=33424 rollout=rollout-bluegreen
ERRO[2021-05-20T16:16:19-04:00] The Rollout "rollout-bluegreen" is invalid: spec.strategy.blueGreen.prePromotionAnalysis.templates: Invalid value: "templateNames: [sleep-job]": args.duration was not resolved  namespace=default rollout=rollout-bluegreen
INFO[2021-05-20T16:16:19-04:00] Reconciliation completed                      generation=2 namespace=default resourceVersion=33424 rollout=rollout-bluegreen time_ms=1.3263120000000002
INFO[2021-05-20T16:16:39-04:00] Started syncing rollout                       generation=2 namespace=default resourceVersion=33424 rollout=rollout-bluegreen
ERRO[2021-05-20T16:16:39-04:00] The Rollout "rollout-bluegreen" is invalid: spec.strategy.blueGreen.prePromotionAnalysis.templates: Invalid value: "templateNames: [sleep-job]": args.duration was not resolved  namespace=default rollout=rollout-bluegreen



# Logs for a specific rollout:
kubectl logs -n argo-rollouts deployment/argo-rollouts | grep rollout=<ROLLOUTNAME>

---

Message from the maintainers:

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

[huikang]

@jessesuen , before this line

argo-rollouts/pkg/apis/rollouts/validation/validation_references.go

Line 113 in 912d3ac

err = analysisutil.ResolveArgs(flattenedTemplate.Spec.Args)

The method does not check if the argument name is available in the rollout spec.
Is this an intended behavior?

I think the logic could

if an argument in analysis template does not have a default value {
     found := the argument name is defined in rollout spec
     if found == false {
        return error
    }
}

What do you think?

[jessesuen]

@khhirani this is your code. can you check?

[khhirani]

@jessesuen @huikang This is expected behavior. Each arg in the AnalysisTemplate must have either a value or valueFrom field. We don't give the option to pass arguments for Analysis through the Rollout YAML, at least not at the moment.

[huikang]

@khhirani @jessesuen , thanks for checking. My thought is this is a regress because in this demo application, these two args do not have any value.

https://github.com/argoproj/rollouts-demo/blob/d40152cc2d4964ad10d26a4515b9148b3e990de2/examples/istio/analysis.yaml#L8

Their values comes from the rollout yaml:

https://github.com/argoproj/rollouts-demo/blob/d40152cc2d4964ad10d26a4515b9148b3e990de2/examples/istio/rollout.yaml#L38

Since the rollout-demo was written in 2020, i thought this would be the initial design.

If so, shall we fix the rollout-demo?

[huikang]

I think we also need to fix some missing values in the doc to avoid confusion to the users: https://argoproj.github.io/argo-rollouts/features/analysis/

Please correct me if I am wrong

[iAnomaly]

@khhirani I just upgraded from v0.10 to v1.0.0 and this is a regression. We were using two args defined in a ClusterAnalysisTemplate without values which had the behavior of requiring those args in the Rollout spec instead of making them optional via default values.

In order to fix this bug in v1.0.0 we had to set all args in the ClusterAnalysisTemplate with default values as you say. FWIW, I preferred the old behavior which allow both required and optional args. To @huikang ’s point about the documentation and the fact we were relying on this old behavior and the upgrade broke it, as a user I certainly consider this a regression whether the previous behavior I have described was intended or not. Thank you.

[khhirani]

@huikang @iAnomaly Thank you for the details! Looking into this.