fix(argo): propagate nodeSelector/tolerations to error and exit hooks - #3371
Conversation
…s propagation Fixes Netflix#3309 - Add .node_selectors() and .tolerations() calls to error-msg-capture-hook template to match step pod configuration - Add node_selectors and tolerations support to ContainerHook and _Template classes in exit_hooks.py for @exit_hook decorators - Update ContainerHook instantiations to pass nodeSelector and tolerations from resources dictionary - Add comprehensive tests to verify nodeSelector and tolerations propagate correctly to error hook templates, matching step templates - Tests verify both presence and absence of configuration The error-msg-capture-hook and @exit_hook templates now inherit nodeSelector and tolerations consistently with regular step pods, allowing these hooks to run on the same nodes as the workflow steps.
Greptile SummaryPropagates Kubernetes scheduling constraints to Argo-generated hook templates.
Confidence Score: 5/5The implementation appears safe to merge, with only the non-blocking absence of compilation coverage for user-defined exit-hook templates remaining. No new issue was introduced since the previous review. The previous test-coverage finding remains outstanding because the tests still deploy a flow without
|
| Filename | Overview |
|---|---|
| metaflow/plugins/argo/argo_workflows.py | Passes workflow-level node selectors and tolerations into generated success, error, and error-message capture hook templates. |
| metaflow/plugins/argo/exit_hooks.py | Extends container hooks to serialize optional Kubernetes node selectors and tolerations. |
| test/ux/core/test_argo_compilation.py | Tests error-message capture hook scheduling configuration, but the previously reported lack of user-defined exit-hook coverage remains. |
Reviews (2): Last reviewed commit: "Fix Argo workflows error-msg-capture-hoo..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| deployed_flow = ( | ||
| Deployer( | ||
| flow_file=_resolve_flow_path("basic/helloworld.py"), |
There was a problem hiding this comment.
These compilation tests deploy basic/helloworld.py, which has no @exit_hook, and inspect only error-msg-capture-hook. They therefore never exercise the changed ContainerHook paths that generate user-defined success-* and error-* templates. Add an @exit_hook fixture and verify that both templates inherit nodeSelector and tolerations; otherwise this propagation behavior could regress undetected.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
02d57c6 to
845fcd8
Compare
1 participant
Fixes
Fixes #3309
Summary
Propagate
nodeSelectorandtolerationsinto the Argo error-msg-capture-hook and@exit_hooktemplates so hooks can schedule on the same nodes as workflow steps.Test plan