feat: add status.lastRunAt field to WorkflowTemplates. Fixes #1915

[qingfeng777]

Fixes #1915

Motivation

It would be useful to know when the last time a WorkflowTemplate was run

Modifications

• add WorkflowTemplateStatus.LastRunAt filed to workflow template filed
• enable workflowTemplate support record last run time

Verification

run locally, submit workflow from template and check

[Joibel]

The implementation of the feature doesn't do what I expected.

Currently, it is only recording uses of WorkflowTemplates when invoked through the user interface or CLI, and will not record uses in many other circumstances:

• Uses from CronWorkflows
• Uses when a workflow is injected via the kubernetes API, or other system which uses this mechanism such as argo-events
• Uses of a template when it is not used through workflowTemplateRef.

I am happy, if we document it as so, that this only records workflowTemplateRef uses of a WorkflowTemplate. It must record all uses via workflowTemplateRef though. Take a look at fetchWorkflowSpec.

I was also expecting this new value to be visible in the user interface, although I might be persuaded that that should be in a separate PR if you have a good reason.

[Joibel]

This should be performed by the controller to catch all uses, not by the server.

[qingfeng777]

Thank you for being so helpful. You were absolutely right. I will make a separate PR to add the Status field and then see how to perform this task with the controller to catch all uses.

[Joibel]

By itself, adding the field but not setting it isn't useful, and is possibly misleading as someone might reasonably expect it to get set.

Could you add the code to set this value from the controller to this same PR please.

[qingfeng777]

Thank you for your advice, I will add the code to set this value from the controller to the same PR later.

[qingfeng777]

Hi,

In my first attempt, I updated the Getter in context.go to an Operator (supporting both get and update operations), using the clientSet as a parameter. However, the requeue and get workflow did not function as expected.

In my second attempt, I used NewContextFromClientSet instead of NewContext, to hold both the informer getter and clientSet. Unfortunately, I was unable to retrieve clientSet in some callers of the ValidateWorkflow function.

Ultimately, I implemented the feature like this. I am unsure if there is a better approach. Could you please review it again?

The main updates to support recording LastRunAt are in the second commit.

Thank you.

[Joibel]

This is a hard problem to solve correctly. The main objection here is how hard this is on the kubernetes API, and how it will slow down execution.

[Joibel]

I am away for 2 weeks, I will take a look when I can.

[qingfeng777]

Happy holidays!

[qingfeng777]

Hi，@Joibel, could you take a look at this PR? thank you.