Argo workflows error-msg-capture-hook nodeSelector and tolerations propagation

[queimadus]

It seems like the default nodeSelector and tolerations added to all step pods (via METAFLOW_KUBERNETES_TOLERATIONS and METAFLOW_KUBERNETES_NODE_SELECTOR), aren't added to the argo workflows error-msg-capture-hook pod, as well.

This makes it so this pod can't re-use the same node from the previous steps.

Is this intended since this is a lightweight pod that should run in a slimmer node?

https://github.com/Netflix/metaflow/blob/master/metaflow/plugins/argo/argo_workflows.py#L3272

https://github.com/Netflix/metaflow/blob/master/metaflow/plugins/argo/argo_workflows.py#L2850-L2853

[csmeby]

Hey good catch. The error capture hook and @exit_hook templates are missing the nodeSelector and tolerations wired up for regular steps.

I checked the git history (97a5ea5, c0437dd) and it looks like these just got left behind as attributes were added one-by-one over time rather than an intentional choice to keep them lightweight. Proposed fix: Add .node_selectors(...) and .tolerations(...) to the template in _error_msg_capture_hook_templates (matching step pods). Add support for both to ContainerHook/_Template in exit_hooks.py and wire them through.

I am happy to make a PR for this

[goingforstudying-ctrl]

Is anyone still working on this? It's been about ten days since the last note, so I'd like to take it if not, otherwise happy to leave it.

The read above matches what I see: the error-msg-capture-hook and @exit_hook templates never call .node_selectors(...) / .tolerations(...) on their Template, while the step templates do (around line 2856). The wiring should be the same here since these builders already read the start step's @kubernetes attributes into resources, and the env-level defaults (METAFLOW_KUBERNETES_NODE_SELECTOR / METAFLOW_KUBERNETES_TOLERATIONS) land in those attributes too.

Plan: add .node_selectors(resources.get("node_selector")) and .tolerations(resources.get("tolerations")) to the hook templates, matching the step pods. Then a rendering test that sets the two env vars and asserts the hook template carries the same selectors and tolerations as a step pod, plus a case with them unset to confirm nothing extra renders.

On whether the slim pod should stay off those nodes on purpose: the git history point above (left behind as attributes accumulated one by one) reads right to me. The hook already inherits image, secrets and service account from the start step, so inheriting placement looks consistent.

[saikat709]

I would like to give it a try. I will wait for confirmation from maintainer.