-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support dag and steps level scheduling constraints. Fixes: #12568 #12700
Conversation
Signed-off-by: shuangkun <[email protected]>
Signed-off-by: shuangkun <[email protected]>
Signed-off-by: shuangkun <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good stuff.
I'd just like to rework some of the function calls here to allow a bit more code reuse.
@@ -815,6 +826,28 @@ func addSchedulingConstraints(pod *apiv1.Pod, wfSpec *wfv1.WorkflowSpec, tmpl *w | |||
} | |||
} | |||
|
|||
// GetBoundaryTemplate get a template through the node's BoundaryID. | |||
func (woc *wfOperationCtx) GetBoundaryTemplate(nodeName string) (*wfv1.Template, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can something like this function also be used from
argo-workflows/workflow/controller/operator.go
Line 2740 in e00abd1
boundaryNode, err := woc.wf.Status.Nodes.Get(boundaryID) |
The two bits of code are very similar to each other, and it's good to extract common functionality whilst we're doing this kind of work.
This function could take a *NodeStatus instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I will try it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made some reuses, please take a look,Thanks.
Signed-off-by: shuangkun <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @shuangkun
…proj#12568 (argoproj#12700) Signed-off-by: shuangkun <[email protected]>
Fixes #12568
Motivation
I want to Take effect dag and steps level scheduling constraints. Not just workflow level and template level.
Modifications
Find the pod's bounaryID's Template and add the scheduling constraints to pod if exists.
Verification
ut and e2e test