Skip to content

Commit 135c611

Browse files
kaiitunnzclaude
andcommitted
refactor: derive worker SSH capability from executor initialization
Advertise capabilities from the initialized executor map rather than a standalone Docker probe: executors declare an is_available() hook, SSHExecutor returns docker_available(), and init_executor skips an unavailable executor so it is never constructed. The worker initializes executors before registering and reports ssh = "ssh" in executors, so a disabled flag, missing socket, missing package, or init failure all converge to ssh=False. The registry now validates entries are Executor subclasses, which types the is_available gate without a cast. SSHExecutor resolves its container label name lazily so it can still pick up the worker id once registration completes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>
1 parent 507a7cd commit 135c611

6 files changed

Lines changed: 149 additions & 50 deletions

File tree

src/worker/executors/__init__.py

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,60 @@
11
import importlib
22

3+
from .base_executor import Executor
4+
35
_IMPORT_ERRORS: dict[str, str] = {}
46

57

6-
def _safe_import(name: str, module: str) -> type | None:
8+
def _import_executor(name: str, module: str) -> type[Executor] | None:
79
try:
810
pkg = importlib.import_module(module, package=__package__)
9-
return getattr(pkg, name)
11+
if issubclass(cls := getattr(pkg, name), Executor):
12+
return cls
13+
error = f"{name} is not a subclass of Executor"
1014
except Exception as exc:
11-
_IMPORT_ERRORS[name] = str(exc)
12-
return None
15+
error = str(exc)
16+
_IMPORT_ERRORS[name] = error
17+
return None
1318

1419

15-
VLLMExecutor = _safe_import("VLLMExecutor", ".vllm_executor")
16-
VLLMLoRAExecutor = _safe_import("VLLMLoRAExecutor", ".vllm_lora_executor")
17-
PPOExecutor = _safe_import("PPOExecutor", ".ppo_executor")
18-
DPOExecutor = _safe_import("DPOExecutor", ".dpo_executor")
19-
SFTExecutor = _safe_import("SFTExecutor", ".sft_executor")
20-
LoRASFTExecutor = _safe_import("LoRASFTExecutor", ".lora_sft_executor")
21-
ImageClassificationTrainingExecutor = _safe_import(
20+
VLLMExecutor = _import_executor("VLLMExecutor", ".vllm_executor")
21+
VLLMLoRAExecutor = _import_executor("VLLMLoRAExecutor", ".vllm_lora_executor")
22+
PPOExecutor = _import_executor("PPOExecutor", ".ppo_executor")
23+
DPOExecutor = _import_executor("DPOExecutor", ".dpo_executor")
24+
SFTExecutor = _import_executor("SFTExecutor", ".sft_executor")
25+
LoRASFTExecutor = _import_executor("LoRASFTExecutor", ".lora_sft_executor")
26+
ImageClassificationTrainingExecutor = _import_executor(
2227
"ImageClassificationTrainingExecutor", ".image_classification_executor"
2328
)
24-
HFTransformersExecutor = _safe_import(
29+
HFTransformersExecutor = _import_executor(
2530
"HFTransformersExecutor", ".transformers_executor"
2631
)
27-
RAGExecutor = _safe_import("RAGExecutor", ".rag_executor")
28-
AgentExecutor = _safe_import("AgentExecutor", ".agent_executor")
29-
EchoExecutor = _safe_import("EchoExecutor", ".echo_executor")
30-
DataProfilingExecutor = _safe_import(
32+
RAGExecutor = _import_executor("RAGExecutor", ".rag_executor")
33+
AgentExecutor = _import_executor("AgentExecutor", ".agent_executor")
34+
EchoExecutor = _import_executor("EchoExecutor", ".echo_executor")
35+
DataProfilingExecutor = _import_executor(
3136
"DataProfilingExecutor", ".data_profiling_executor"
3237
)
33-
DataRetrievalExecutor = _safe_import(
38+
DataRetrievalExecutor = _import_executor(
3439
"DataRetrievalExecutor", ".data_retrieval_executor"
3540
)
36-
DiffusersExecutor = _safe_import("DiffusersExecutor", ".diffusers_executor")
37-
APIExecutor = _safe_import("APIExecutor", ".api_executor")
38-
SSHExecutor = _safe_import("SSHExecutor", ".ssh_executor")
39-
OmniText2ImageExecutor = _safe_import(
41+
DiffusersExecutor = _import_executor("DiffusersExecutor", ".diffusers_executor")
42+
APIExecutor = _import_executor("APIExecutor", ".api_executor")
43+
SSHExecutor = _import_executor("SSHExecutor", ".ssh_executor")
44+
OmniText2ImageExecutor = _import_executor(
4045
"OmniText2ImageExecutor", ".omni_text2image_executor"
4146
)
42-
OmniText2SpeechExecutor = _safe_import(
47+
OmniText2SpeechExecutor = _import_executor(
4348
"OmniText2SpeechExecutor", ".omni_text2speech_executor"
4449
)
45-
OmniText2AudioExecutor = _safe_import(
50+
OmniText2AudioExecutor = _import_executor(
4651
"OmniText2AudioExecutor", ".omni_text2audio_executor"
4752
)
48-
OmniText2GeneralExecutor = _safe_import(
53+
OmniText2GeneralExecutor = _import_executor(
4954
"OmniText2GeneralExecutor", ".omni_text2general_executor"
5055
)
5156

52-
EXECUTOR_REGISTRY: dict[str, type | None] = {
57+
EXECUTOR_REGISTRY: dict[str, type[Executor] | None] = {
5358
"vllm": VLLMExecutor,
5459
"vllm_lora": VLLMLoRAExecutor,
5560
"ppo": PPOExecutor,

src/worker/executors/base_executor.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,16 @@ def __init__(
8181
self._hardware = hardware
8282
self._lifecycle = lifecycle
8383

84+
@classmethod
85+
def is_available(cls, config: WorkerConfig) -> bool:
86+
"""Whether this executor can run on the worker.
87+
88+
Override to declare a runtime dependency the worker must satisfy. An unavailable
89+
executor is skipped at startup, so it never registers and the worker advertises
90+
no capability for it.
91+
"""
92+
return True
93+
8494
def emit_update(self, task_id: str, payload: dict[str, Any]) -> None:
8595
"""Emit a mid-task TASK_UPDATE event.
8696

src/worker/executors/ssh_executor.py

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@
4848
from shared.utils.manifest import ARTIFACTS_DIR, prepare_output_dir
4949
from worker.config import WorkerConfig
5050
from worker.executors.utils.checkpoints import maybe_upload_artifacts
51-
from worker.executors.utils.docker import DockerUnavailableError, docker_client
51+
from worker.executors.utils.docker import (
52+
DockerUnavailableError,
53+
docker_available,
54+
docker_client,
55+
)
5256

5357
from .base_executor import (
5458
ExecutionError,
@@ -384,19 +388,30 @@ class SSHExecutor(Executor):
384388
def __init__(self, *args: Any, **kwargs: Any) -> None:
385389
super().__init__(*args, **kwargs)
386390
config = self._config
387-
lifecycle = self._lifecycle
388-
self._worker_name = (
389-
config.container_name
390-
or config.alias
391-
or (lifecycle.worker_id if lifecycle else uuid.uuid4().hex[:8])
392-
)
391+
self._worker_name = config.container_name or config.alias or None
393392
self._docker: DockerClient | None = None
394393
self._docker_gpu_runtime: str | None = config.docker_gpu_runtime
395394
self._ssh_network: str | None = None
396395
self._cancel_event = threading.Event()
397396
self._finish_event = threading.Event()
398397
self._current_container: Container | None = None
399398

399+
@classmethod
400+
def is_available(cls, config: WorkerConfig) -> bool:
401+
return docker_available()
402+
403+
@property
404+
def worker_name(self) -> str:
405+
if name := self._worker_name:
406+
return name
407+
name = (
408+
lifecycle.worker_id
409+
if (lifecycle := self._lifecycle)
410+
else uuid.uuid4().hex[:8]
411+
)
412+
self._worker_name = name
413+
return name
414+
400415
# ------------------------------------------------------------------ #
401416
# Lifecycle
402417
# ------------------------------------------------------------------ #
@@ -413,7 +428,7 @@ def teardown(self) -> None:
413428
client = self._get_docker_client()
414429
try:
415430
containers = client.containers.list(
416-
filters={"label": f"{_LABEL_WORKER}={self._worker_name}"}
431+
filters={"label": f"{_LABEL_WORKER}={self.worker_name}"}
417432
)
418433
except Exception as exc:
419434
logger.warning(
@@ -454,7 +469,8 @@ def run(self, task: ExecutorTask, out_dir: Path) -> SSHResult:
454469
container_cmd = self._resolve_noninteractive_command(client, cfg)
455470

456471
session_id = new_ssh_session_id()
457-
container_name = f"{self._worker_name}_ssh-{task.task_id[:8]}-{session_id[:8]}"
472+
worker_name = self.worker_name
473+
container_name = f"{worker_name}_ssh-{task.task_id[:8]}-{session_id[:8]}"
458474

459475
prepare_output_dir(out_dir) # Ensure output dir exists before mounting
460476
resolved_inputs = self._resolve_inputs(task, cfg)
@@ -463,7 +479,7 @@ def run(self, task: ExecutorTask, out_dir: Path) -> SSHResult:
463479
)
464480

465481
labels = {
466-
_LABEL_WORKER: self._worker_name,
482+
_LABEL_WORKER: worker_name,
467483
_LABEL_TASK: task.task_id,
468484
_LABEL_SESSION: session_id,
469485
_LABEL_MANAGED: "true",
@@ -1193,7 +1209,7 @@ def _stage_inputs_in_volume(
11931209
volume = client.volumes.create(
11941210
name=volume_name,
11951211
labels={
1196-
_LABEL_WORKER: self._worker_name,
1212+
_LABEL_WORKER: self.worker_name,
11971213
_LABEL_SESSION: session_id,
11981214
_LABEL_MANAGED: "true",
11991215
},

src/worker/main.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from .executors import EXECUTOR_CLASS_NAMES, EXECUTOR_REGISTRY, IMPORT_ERRORS
1010
from .executors.base_executor import Executor
1111
from .executors.mp_executor import MPExecutor
12-
from .executors.utils.docker import docker_available
1312
from .hw import collect_hw
1413
from .lifecycle import Lifecycle
1514
from .power import PowerMonitor
@@ -59,7 +58,7 @@ def initialize_executors(
5958
hardware: WorkerHardware,
6059
logger: logging.Logger,
6160
lifecycle: Lifecycle,
62-
registry: dict[str, type | None] | None = None,
61+
registry: dict[str, type[Executor] | None] | None = None,
6362
import_errors: dict[str, str] | None = None,
6463
cuda_available: bool | None = None,
6564
enable_mp_executors: bool = True,
@@ -97,6 +96,10 @@ def init_executor(key: str, *, gpu_required: bool = False):
9796
logger.info("Executor %s requires a GPU; unavailable, skipping", key)
9897
return None
9998

99+
if not cls.is_available(config):
100+
logger.info("Executor %s is unavailable; skipping", key)
101+
return None
102+
100103
try:
101104
if key in configured_wrapped:
102105
return MPExecutor(cls, config, hardware)
@@ -197,7 +200,16 @@ def main() -> None:
197200
)
198201
hardware = collect_hw(bandwidth_bytes_per_sec=cfg.network_bandwidth_bytes_per_sec)
199202
logger.info("Collected hardware info: %s", hardware)
200-
capabilities = WorkerCapabilities(ssh=docker_available())
203+
204+
executors, default_executor = initialize_executors(
205+
cfg,
206+
hardware,
207+
logger,
208+
lifecycle,
209+
enable_mp_executors=cfg.enable_mp_executors,
210+
)
211+
212+
capabilities = WorkerCapabilities(ssh="ssh" in executors)
201213
ssh_limits = cfg.ssh_limits
202214
if capabilities.ssh:
203215
if ssh_limits is None:
@@ -207,8 +219,6 @@ def main() -> None:
207219
)
208220
else:
209221
logger.info("SSH resource cap: %s", ssh_limits.model_dump())
210-
else:
211-
logger.info("SSH support disabled; Docker is not reachable from this worker.")
212222
lifecycle.start(
213223
env={},
214224
hardware=hardware,
@@ -217,14 +227,6 @@ def main() -> None:
217227
tags=cfg.tags,
218228
)
219229

220-
executors, default_executor = initialize_executors(
221-
cfg,
222-
hardware,
223-
logger,
224-
lifecycle,
225-
enable_mp_executors=cfg.enable_mp_executors,
226-
)
227-
228230
task_stream = supervisor_client.iter_tasks()
229231
runner = Runner(
230232
lifecycle,

tests/worker/test_executor_bootstrap.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,13 @@
1010
from pathlib import Path
1111
from typing import Any
1212

13+
import pytest
14+
1315
from shared.schemas.result import BaseExecutorResult
1416
from tests.worker.factories import make_live_worker_config, make_worker_hardware
17+
from worker.executors import ssh_executor as ssh_mod
1518
from worker.executors.base_executor import Executor, ExecutorTask
19+
from worker.executors.ssh_executor import SSHExecutor
1620
from worker.main import initialize_executors
1721

1822

@@ -28,6 +32,16 @@ def run(self, task: ExecutorTask, out_dir: Path) -> BaseExecutorResult:
2832
return BaseExecutorResult.model_validate({"ok": True})
2933

3034

35+
class _UnavailableExecutor(_PassthroughExecutor):
36+
"""Executor that declares itself unavailable on this worker."""
37+
38+
name = "unavailable"
39+
40+
@classmethod
41+
def is_available(cls, config: Any) -> bool:
42+
return False
43+
44+
3145
class TestInitializeExecutorsHardware:
3246
def test_executor_receives_hardware_via_passthrough(self, tmp_path: Path) -> None:
3347
cfg = make_live_worker_config(tmp_path)
@@ -48,3 +62,32 @@ def test_executor_receives_hardware_via_passthrough(self, tmp_path: Path) -> Non
4862
assert isinstance(default, _PassthroughExecutor)
4963
assert executors["echo"]._hardware is hw
5064
assert default._hardware is hw
65+
66+
67+
class TestInitializeExecutorsAvailability:
68+
def test_unavailable_executor_is_skipped(self, tmp_path: Path) -> None:
69+
cfg = make_live_worker_config(tmp_path)
70+
executors, _ = initialize_executors(
71+
config=cfg,
72+
hardware=make_worker_hardware(),
73+
logger=logging.getLogger("test"),
74+
lifecycle=None, # type: ignore[arg-type]
75+
registry={
76+
"default": _PassthroughExecutor,
77+
"echo": _UnavailableExecutor,
78+
},
79+
import_errors={},
80+
cuda_available=False,
81+
enable_mp_executors=False,
82+
)
83+
assert "echo" not in executors
84+
assert "default" in executors
85+
86+
def test_ssh_availability_tracks_docker(
87+
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
88+
) -> None:
89+
cfg = make_live_worker_config(tmp_path)
90+
monkeypatch.setattr(ssh_mod, "docker_available", lambda: False)
91+
assert SSHExecutor.is_available(cfg) is False
92+
monkeypatch.setattr(ssh_mod, "docker_available", lambda: True)
93+
assert SSHExecutor.is_available(cfg) is True

tests/worker/test_executor_registry.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
"""Tests for the executor registry and safe import mechanism."""
22

3+
from types import SimpleNamespace
4+
5+
import pytest
6+
7+
import worker.executors as executors_pkg
38
from worker.executors import EXECUTOR_CLASS_NAMES, EXECUTOR_REGISTRY, IMPORT_ERRORS
49

510

@@ -51,8 +56,26 @@ def test_training_executors_are_wrapped_for_isolation(self) -> None:
5156
assert "image_classification_training" in _EXECUTORS_TO_WRAP
5257
assert "image_classification_training" in EXECUTOR_REGISTRY
5358

54-
def test_safe_import_does_not_crash(self) -> None:
59+
def test_import_executor_does_not_crash(self) -> None:
5560
"""The registry should load without raising, even when deps are missing."""
5661
# If we got here, the import at module level already succeeded
5762
assert isinstance(EXECUTOR_REGISTRY, dict)
5863
assert isinstance(IMPORT_ERRORS, dict)
64+
65+
def test_import_executor_rejects_non_executor(
66+
self, monkeypatch: pytest.MonkeyPatch
67+
) -> None:
68+
"""A registered name that resolves to a non-Executor is dropped and recorded."""
69+
fake_module = SimpleNamespace(NotAnExecutor=object)
70+
monkeypatch.setattr(
71+
executors_pkg.importlib, "import_module", lambda *a, **k: fake_module
72+
)
73+
executors_pkg._IMPORT_ERRORS.pop("NotAnExecutor", None)
74+
75+
result = executors_pkg._import_executor("NotAnExecutor", ".does_not_matter")
76+
77+
assert result is None
78+
assert (
79+
"not a subclass of Executor"
80+
in executors_pkg._IMPORT_ERRORS["NotAnExecutor"]
81+
)

0 commit comments

Comments
 (0)