argoproj / argo-workflows

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

Add WorkflowTemplate name as label when using `workflowTemplateRef` #12670

Open eduardodbr opened 7 months ago

eduardodbr commented 7 months ago

Summary

What change needs making?

When a Workflow or a CronWorkflow is submitted from a WorkflowTemplate,( i.e. using the workflowTemplateRef) it would be nice to have the WorkflowTemplate name as a label for easier filtering using the UI.

Use Cases

When would you use this?

A workflow like:

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  name: workflow-template-hello-world
spec:
  workflowTemplateRef:
    name: workflow-template-submittable

Or workflows that are children of a CronWorkflow like:

apiVersion: argoproj.io/v1alpha1
kind: CronWorkflow
metadata:
  name: cron-from-template
spec:
  schedule: "* * * * *"
  workflowSpec:
    workflowTemplateRef:
      name: workflow-template-submittable

would have labels:

labels:
    workflows.argoproj.io/workflow-template: workflow-template-submittable

I am available to contribute to this new feature if accepted.


Message from the maintainers:

Love this enhancement proposal? Give it a πŸ‘. We prioritize the proposals with the most πŸ‘.

agilgur5 commented 7 months ago

or a CronWorkflow

Implementation-wise, I don't think this part matters, since a CronWorkflow creates Workflows, so the end result would be functionally equivalent to a Workflow with a workflowTemplateRef

Also I simplified the title a good bit for conciseness and better readability (long titles are not easy on the eyes)

sarabala1979 commented 7 months ago

@eduardodbr I would like to understand the usecase for this. Because workflowTemplateRef is a static value it is known during spec creation. Label can be added to the workflow spec itself.

Let me know if I am missing anything

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  labels:
     workflows.argoproj.io/workflow-template: workflow-template-submittable    
  name: workflow-template-hello-world
spec:
  workflowTemplateRef:
    name: workflow-template-submittable
apiVersion: argoproj.io/v1alpha1
kind: CronWorkflow
metadata:
  name: cron-from-template
spec:
  schedule: "* * * * *"
  workflowMetadata:
     labels:
        workflows.argoproj.io/workflow-template: workflow-template-submittable    
  workflowSpec:
    workflowTemplateRef:
      name: workflow-template-submittable
eduardodbr commented 7 months ago

@sarabala1979 from my experience, people forget to add the label and then complain when the workflows don’t appear on the UI when they filter by workflow template. If the label gets added automatically, no need to remember that a label must be configured

sarabala1979 commented 7 months ago

@eduardodbr looks like a specific user usage/env problem, it can be solved through mutating webhook. It is out of the scope of the workflow controller functionality. A controller can add labels if it is required for controller execution or if the controller is creating an object for the execution.

agilgur5 commented 7 months ago

@sarabala1979 so we do already have workflows.argoproj.io/cron-workflow and workflows.argoproj.io/workflow-template that are used as label filters already.

I can't seem to find where these are set in the back-end, but some existing use-cases include the CronWorkflow history of executions from #11811. The same feature could be/is requested for WorkflowTemplates.

sarabala1979 commented 7 months ago

This label will be used/set when cli creates workflow from cronworkflow or workflowtemplate

# Submit a single workflow from an existing resource

  argo submit --from cronwf/my-cron-wf
agilgur5 commented 2 months ago

So it turns out their is indeed a good bit of precedent for this, as I thought.

This whole issue actually half duplicates #7611, which was an old feature request by Alex C. The PR, #12677, is half duplicated by #12776 as well as such. I'm inclined to take the PR for this issue as-is, as such, as it seems a bit nicer style than the other one, but TBD.

This scenario is also addressed in the upstream k8s Controller conventions as well:

[...] we should support conveniences for common cases by default. For example, what we now do in ReplicationController is automatically set the RC's selector and labels to the labels in the pod template by default, if they are not already set. [...]

Unprefixed keys are reserved for end-users. All other labels and annotations must be prefixed.

Pods having some labels automatically set by their parent resource is both convenient and quite common in Controllers, enough so to be referenced in the upstream conventions.

eduardodbr commented 2 months ago

you can reopen my PR and I'll work on requested changes, if any