argoproj / argo-workflows

Workflow Engine for Kubernetes
https://argo-workflows.readthedocs.io/
Apache License 2.0
15.11k stars 3.21k forks source link

feat(controller): show invalid spec on cronwf res #13926

Open carlosrodfern opened 7 hours ago

carlosrodfern commented 7 hours ago

Fixes: #6781

Motivation

Help the user see the validation errors on the cronwf resources early, before they find out later that the workflow didn't trigger.

Modifications

Add the event to the description of the cronwf, e.g.

Events:
  Type     Reason     Age   From                 Message
  ----     ------     ----  ----                 -------
  Warning  Invalid  14s   workflow-controller  cron workflow name "test-cron-wf-very-very-very-very-very-very-very-very-very-very-very-very-long" must not be more than 52 characters long (currently 77)

Verification

I created my own image of workflow-controller and deployed it in a local k3s kubernetes with argo-workflow.

I used this artifact as a test:

apiVersion: argoproj.io/v1alpha1
kind: CronWorkflow
metadata:
  name: test-cron-wf-very-very-very-very-very-very-very-very-very-very-very-very-long
spec:
  schedule: "5 * * * *"
  concurrencyPolicy: "Replace"
  startingDeadlineSeconds: 0
  workflowSpec:
    entrypoint: date
    templates:
    - name: date
      container:
        image: alpine:3.6
        command: [sh, -c]
        args: ["date; sleep 90"]
carlosrodfern commented 7 hours ago

I'm still investigating the possibiilty to actually make it a status, e.g. Ready if the cronworkflow is good, or something else otherwise. The goal is that if I deploy an invalid cronworkflow with ArgoCD, for example, the health status of the cronworkflow would be failed because of invalid spec, and ArgoCD would mark the app syncing as failed.

Joibel commented 1 hour ago

There is already a metric for this.

The best solution is an admission controller which will reject them. This is started in #13879