Provide a way to set default Workflow options that will apply to all workflows

[simster7]

Summary

Provide a way to set default Workflow options that will apply to all workflows.

Motivation

Users have asked for a way to set particular Workflow options once in the worfklow-controller-cm that will apply to all new Workflows as a convenience feature.

See: #1827 (comment) #1790

Proposal

We should add a way to allow users to set an arbitrary number of default Workflow options.

[NikeNano]

I would be happy to make a contribution and work on this. I have looked over the code base a bit but don't understand how the workflow-controller-configmap is parsed/ handled by the controller. Could some one point me in the right direction, where I can understand the logic for this?

Thanks for the help!

[rushtehrani]

We also need this for PodGC.

[NikeNano]

I have read more of the code base, and as I understand the workflow-controller-configmap is parsed by the config_controller. As I understand the workflowControllerConfig is parsed here:
https://github.com/argoproj/argo/blob/02022e4bb36977253ff2e362f844b9596e768102/workflow/controller/config_controller.go#L39

I see the following updates needed:

• Updated the workflow-controller-configmap to include the options of interest.
• Pass the workflow-controller-configmap to the controllers of interest such as the ttlcontroller Not relevant since the configmap is set as an attribute of the WorkflowController https://github.com/argoproj/argo/blob/02022e4bb36977253ff2e362f844b9596e768102/workflow/controller/config_controller.go#L47

Alternatives on how to handle the configmap

1. Update the controllers of interest to handle the workflow-controller-configmap which is a attribute of the workflow https://github.com/argoproj/argo/blob/02022e4bb36977253ff2e362f844b9596e768102/workflow/controller/controller.go#L45 and adjust for if the value is not set in the workflow but in the configmap.
2. Update the workflows when they are passed to Argo based upon the configmap. As I understand then this could be incorporated after this step
   https://github.com/argoproj/argo/blob/02022e4bb36977253ff2e362f844b9596e768102/workflow/controller/controller.go#L265
   Potentially as a function setting all values that the configmap supplies that are not set in the workflow

[NikeNano]

@simster7 do you have some feedback on this? :)

[simster7]

We would have to go with option 1. However, we would also need to come up with a spec of how to declare default options on the config map. Do you have any ideas/suggestions for this?

[NikeNano]

Cool, thanks for the feedback @simster7.

• Why option 1? It feels like it should be more general if we updated the workflow in one place instead? Then also it could be handled for other default settings instead of adding in the controllers?

• For the config map I found a similar example for the https://github.com/argoproj/argo/blob/c51c1302f48cec5b9c6009b9b7e50962d338c679/docs/workflow-controller-configmap.yaml#L40

I suggest we add the following:

 # TTLStrategy defines the default time to live for workflows depending on how they complete
 TTLStrategyDefault: 
    secondsAfterCompleted: -1 #  Default time to live after workflow is completed, replaces ttlSecondsAfterFinished
    secondsAfterSuccess: -1    #  Default time to live after workflow is successful
    secondsAfterFailed: -1   # Default time to live after workflow fails

I am not sure if we should add secondsAfterCompleted since it will be deprecated.

What are your thoughts?

[NikeNano]

I it is fine that I start to work on it, could you please assign me to the issue?

[simster7]

@NikeNano Sorry, we've been rather busy with the upcoming 2.5 RC launch. I feel like we will need to have an internal discussion about this feature, which is not likely to happen until we ship the 2.5 RC launch.

However, feel free to make a proof of concept for this however you'd think it's best... if it's good it might drive our discussion in its favor. Just know that if you get started on this we might ask you to change it in the future :)

I will assign you the issue.

[zhujl1991]

We would have to go with option 1. However, we would also need to come up with a spec of how to declare default options on the config map. Do you have any ideas/suggestions for this?

@simster7 Why do we go with option 1 here? Agree with the @NikeNano 's comment below:
"Why option 1? It feels like it should be more general if we updated the workflow in one place instead? Then also it could be handled for other default settings instead of adding in the controllers?"

[simster7]

Fixed in #2331