feat(controller): Allow to configure main container resources #4656
Conversation
|
podSpecPatch? |
|
Thanks for the suggestion. It's definitely an option. However, podSpecPatch requires all of our existing users to modify their workflow spec which is not trivial (especially when we have long-running workflows and established usage patterns). Configuring this in the config of controller seems to be a better option for now. |
| @@ -151,6 +151,9 @@ func (woc *wfOperationCtx) createWorkflowPod(nodeName string, mainCtr apiv1.Cont | |||
| wfSpec := woc.execWf.Spec.DeepCopy() | |||
|
|
|||
| mainCtr.Name = common.MainContainerName | |||
| if isResourcesSpecified(woc.controller.Config.MainContainer) { | |||
| mainCtr.Resources = woc.controller.Config.MainContainer.Resources | |||
There was a problem hiding this comment.
- The workflow spec should take priority over the config.
- Test?
There was a problem hiding this comment.
Thanks for the feedback. Just updated the PR.
There was a problem hiding this comment.
I think you can ignore podSpePatch, but you should check tmpl.Container
There was a problem hiding this comment.
Could you clarify why we need to check tmpl.Container? The goal of this PR is to customize main container no matter what's in the template (similar to what's currently done for executor).
There was a problem hiding this comment.
that would be contrary to how we do it for other item - the user should be able to override a global setting like this
There was a problem hiding this comment.
Actually another goal of this PR is to set reasonable default resources for the main container.
I've added a check on tmpl.Container. Let me know if I've missed anything here.
Signed-off-by: terrytangyuan <terrytangyuan@gmail.com>
Signed-off-by: terrytangyuan <terrytangyuan@gmail.com>
Signed-off-by: terrytangyuan <terrytangyuan@gmail.com>
Signed-off-by: terrytangyuan <terrytangyuan@gmail.com>
Signed-off-by: terrytangyuan <terrytangyuan@gmail.com>
Signed-off-by: terrytangyuan <terrytangyuan@gmail.com>
Some clusters require explicit resources in all containers. This would allow us to configure the resources for the main container.
Signed-off-by: terrytangyuan terrytangyuan@gmail.com
Checklist: