Skip to content

Commit 02d57c6

Browse files
Fix Argo workflows error-msg-capture-hook nodeSelector and tolerations propagation
Fixes #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. Co-authored-by: Shriprasad R Patil <Shriprasad-P@users.noreply.github.com>
1 parent b4f8973 commit 02d57c6

3 files changed

Lines changed: 154 additions & 1 deletion

File tree

metaflow/plugins/argo/argo_workflows.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3163,6 +3163,8 @@ def _container(cmds):
31633163
name=f"success-{success_fn_name.replace('_', '-')}",
31643164
container=_container(cmds=_cmd(success_fn_name)),
31653165
service_account_name=resources["service_account"],
3166+
node_selectors=resources.get("node_selector"),
3167+
tolerations=resources.get("tolerations"),
31663168
on_success=True,
31673169
)
31683170
hooks.append(hook)
@@ -3172,6 +3174,8 @@ def _container(cmds):
31723174
name=f"error-{error_fn_name.replace('_', '-')}",
31733175
service_account_name=resources["service_account"],
31743176
container=_container(cmds=_cmd(error_fn_name)),
3177+
node_selectors=resources.get("node_selector"),
3178+
tolerations=resources.get("tolerations"),
31753179
on_error=True,
31763180
)
31773181
hooks.append(hook)
@@ -3320,7 +3324,9 @@ def _error_msg_capture_hook_templates(self):
33203324
),
33213325
).to_dict()
33223326
)
3323-
),
3327+
)
3328+
.node_selectors(resources.get("node_selector"))
3329+
.tolerations(resources.get("tolerations")),
33243330
Template("capture-error-hook-fn-preflight").steps(
33253331
[
33263332
WorkflowStep()

metaflow/plugins/argo/exit_hooks.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,17 @@ def service_account_name(self, service_account_name):
5353
self.payload["serviceAccountName"] = service_account_name
5454
return self
5555

56+
def node_selectors(self, node_selectors):
57+
if "nodeSelector" not in self.payload:
58+
self.payload["nodeSelector"] = {}
59+
if node_selectors:
60+
self.payload["nodeSelector"].update(node_selectors)
61+
return self
62+
63+
def tolerations(self, tolerations):
64+
self.payload["tolerations"] = tolerations
65+
return self
66+
5667

5768
class Hook(object):
5869
"""
@@ -175,6 +186,8 @@ def __init__(
175186
name: str,
176187
container: Dict,
177188
service_account_name: str = None,
189+
node_selectors: Optional[Dict] = None,
190+
tolerations: Optional[List] = None,
178191
on_success: bool = False,
179192
on_error: bool = False,
180193
):
@@ -185,6 +198,12 @@ def __init__(
185198

186199
self.template.container(container)
187200

201+
if node_selectors is not None:
202+
self.template.node_selectors(node_selectors)
203+
204+
if tolerations is not None:
205+
self.template.tolerations(tolerations)
206+
188207
self.lifecycle_hooks = []
189208

190209
if on_success and on_error:

test/ux/core/test_argo_compilation.py

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,3 +138,131 @@ def test_late_attached_kubernetes_mutator_is_reflected_in_argo_template(
138138

139139
assert end_resources["requests"]["cpu"] == "1"
140140
assert end_resources["requests"]["memory"] == "4096M"
141+
142+
143+
def test_argo_error_hook_inherits_node_selector_and_tolerations(
144+
exec_mode, tag, scheduler_config, monkeypatch
145+
):
146+
if exec_mode != "deployer":
147+
pytest.skip("Argo compilation tests require deployer mode")
148+
if scheduler_config.scheduler_type != "argo-workflows":
149+
pytest.skip("Argo compilation tests require the argo-workflows scheduler")
150+
151+
from metaflow import Deployer
152+
153+
from .test_utils import _resolve_flow_path, prepare_runner_deployer_args
154+
155+
# Set environment variables for nodeSelector and tolerations
156+
monkeypatch.setenv(
157+
"METAFLOW_KUBERNETES_NODE_SELECTOR", '{"disktype": "ssd", "role": "compute"}'
158+
)
159+
monkeypatch.setenv(
160+
"METAFLOW_KUBERNETES_TOLERATIONS",
161+
'[{"key": "dedicated", "operator": "Equal", "value": "ml", "effect": "NoSchedule"}]',
162+
)
163+
164+
deployed_flow = (
165+
Deployer(
166+
flow_file=_resolve_flow_path("basic/helloworld.py"),
167+
show_output=False,
168+
**prepare_runner_deployer_args({}),
169+
)
170+
.argo_workflows()
171+
.create(
172+
only_json=True,
173+
tags=tag + ["test_argo_error_hook_node_selector_tolerations"],
174+
**(scheduler_config.deploy_args or {}),
175+
)
176+
)
177+
178+
workflow_template = deployed_flow.workflow_template
179+
assert workflow_template is not None
180+
181+
# Find the error-msg-capture-hook template
182+
error_hook_template = None
183+
start_template = None
184+
for template in workflow_template.get("spec", {}).get("templates", []):
185+
if template.get("name") == "error-msg-capture-hook":
186+
error_hook_template = template
187+
annotations = template.get("metadata", {}).get("annotations", {})
188+
if annotations.get("metaflow/step_name") == "start":
189+
start_template = template
190+
191+
assert error_hook_template is not None, "error-msg-capture-hook template not found"
192+
assert start_template is not None, "start step template not found"
193+
194+
# Verify nodeSelector is present in both error hook and step template
195+
assert "nodeSelector" in error_hook_template
196+
assert error_hook_template["nodeSelector"] == {
197+
"disktype": "ssd",
198+
"role": "compute",
199+
}
200+
201+
assert "nodeSelector" in start_template
202+
assert start_template["nodeSelector"] == {"disktype": "ssd", "role": "compute"}
203+
204+
# Verify tolerations are present in both error hook and step template
205+
assert "tolerations" in error_hook_template
206+
assert error_hook_template["tolerations"] == [
207+
{
208+
"key": "dedicated",
209+
"operator": "Equal",
210+
"value": "ml",
211+
"effect": "NoSchedule",
212+
}
213+
]
214+
215+
assert "tolerations" in start_template
216+
assert start_template["tolerations"] == [
217+
{"key": "dedicated", "operator": "Equal", "value": "ml", "effect": "NoSchedule"}
218+
]
219+
220+
221+
def test_argo_error_hook_without_node_selector_and_tolerations(
222+
exec_mode, tag, scheduler_config, monkeypatch
223+
):
224+
if exec_mode != "deployer":
225+
pytest.skip("Argo compilation tests require deployer mode")
226+
if scheduler_config.scheduler_type != "argo-workflows":
227+
pytest.skip("Argo compilation tests require the argo-workflows scheduler")
228+
229+
from metaflow import Deployer
230+
231+
from .test_utils import _resolve_flow_path, prepare_runner_deployer_args
232+
233+
# Ensure the env vars are not set
234+
monkeypatch.delenv("METAFLOW_KUBERNETES_NODE_SELECTOR", raising=False)
235+
monkeypatch.delenv("METAFLOW_KUBERNETES_TOLERATIONS", raising=False)
236+
237+
deployed_flow = (
238+
Deployer(
239+
flow_file=_resolve_flow_path("basic/helloworld.py"),
240+
show_output=False,
241+
**prepare_runner_deployer_args({}),
242+
)
243+
.argo_workflows()
244+
.create(
245+
only_json=True,
246+
tags=tag + ["test_argo_error_hook_no_node_selector_tolerations"],
247+
**(scheduler_config.deploy_args or {}),
248+
)
249+
)
250+
251+
workflow_template = deployed_flow.workflow_template
252+
assert workflow_template is not None
253+
254+
# Find the error-msg-capture-hook template
255+
error_hook_template = None
256+
for template in workflow_template.get("spec", {}).get("templates", []):
257+
if template.get("name") == "error-msg-capture-hook":
258+
error_hook_template = template
259+
260+
assert error_hook_template is not None, "error-msg-capture-hook template not found"
261+
262+
# Verify nodeSelector is either empty or not present
263+
node_selector = error_hook_template.get("nodeSelector", {})
264+
assert node_selector == {}, f"Expected empty nodeSelector, got {node_selector}"
265+
266+
# Verify tolerations are not present or None
267+
tolerations = error_hook_template.get("tolerations")
268+
assert tolerations is None, f"Expected no tolerations, got {tolerations}"

0 commit comments

Comments
 (0)