diff --git a/docs/stack-reference.md b/docs/stack-reference.md index b784a6ba..9f121071 100644 --- a/docs/stack-reference.md +++ b/docs/stack-reference.md @@ -742,6 +742,12 @@ The context object (`ctx`) provides: | `ctx["http_post"]` | `callable` | `http_post(url, body, content_type=..., headers=...)` → parsed JSON. Form-encoded by default. | | `ctx["http_get"]` | `callable` | `http_get(url, headers=...)` → parsed JSON. Pass auth explicitly: `headers={"Authorization": "Bearer ..."}`. | +A Python hook or CLI plugin may put its stacklet's directory on `sys.path` +and import helpers from it by bare name (`from oauth import sync`). The +framework forgets those modules and restores `sys.path` when the hook or +command returns, so two stacklets can each have a helper of the same name +even when one process runs both, as `stack restart all` does. + A hook that raises stops the command and is reported as a failure. A hook that asked the admin and got a no raises `stack.hooks.Cancelled` instead: `stack up` stops with `{"cancelled": }`, prints the diff --git a/lib/stack/cli.py b/lib/stack/cli.py index 7fc1082c..d6f36767 100644 --- a/lib/stack/cli.py +++ b/lib/stack/cli.py @@ -245,12 +245,15 @@ def up(self, stacklet_id: str) -> dict: "STACK_DOMAIN": self.stack._cfg("core", "domain"), } + # On every up, not only the first: a release that moves a + # service to a new image tag leaves that image missing here, + # and `compose up` would download it with its output captured, + # a silent wait of minutes behind the start spinner. + with self.stack.output.spinner("Pulling images"): + docker.compose_pull(compose_file, env=compose_env) if manifest.get("build"): with self.stack.output.spinner("Building service"): docker.compose_build(compose_file, env=compose_env) - elif first_run: - with self.stack.output.spinner("Pulling images"): - docker.compose_pull(compose_file, env=compose_env) with self.stack.output.spinner("Starting containers"): code, err = docker.compose_up(compose_file, env=compose_env) diff --git a/lib/stack/docker.py b/lib/stack/docker.py index f1d34045..ff7199b6 100644 --- a/lib/stack/docker.py +++ b/lib/stack/docker.py @@ -92,10 +92,17 @@ def compose_down(compose_file: str | Path) -> tuple[int, str]: def compose_pull(compose_file: str | Path, env: dict | None = None) -> None: - """Pull images for a compose file. Streams output.""" + """Pull the images a compose file names that are not here yet. Streams output. + + Only missing images: when every image is present this asks no + registry, so it costs nothing on a restart and works offline. Newer + versions of present images are Watchtower's job. Services built from + a Dockerfile are left to `compose_build`. + """ full_env = {**__import__("os").environ, **(env or {})} _docker( "compose", "-f", str(compose_file), "pull", + "--policy", "missing", "--ignore-buildable", timeout=600, env=full_env, ) diff --git a/lib/stack/hooks.py b/lib/stack/hooks.py index 64cc42aa..a5dab3ec 100644 --- a/lib/stack/hooks.py +++ b/lib/stack/hooks.py @@ -23,6 +23,8 @@ import importlib.util import os import subprocess +import sys +from contextlib import contextmanager from pathlib import Path @@ -206,6 +208,30 @@ def build_hook_ctx(stacklet_id: str, env=None, step_fn=None, **kwargs): ) +@contextmanager +def stacklet_imports(stacklet_dir: Path): + """Keep the modules a stacklet's code imports from its own directory + to that stacklet. + + Hooks and CLI plugins put their stacklet's directory on `sys.path` and + import its helpers by bare name (`from oauth import sync`). One + process runs the hooks of many stacklets, as in `stack restart all`, + and Python caches a module by name: the second stacklet with an + `oauth.py` got the first one's. On the way out this forgets every + module loaded from `stacklet_dir` and puts `sys.path` back. + """ + root = Path(stacklet_dir).resolve() + path_before = list(sys.path) + try: + yield + finally: + sys.path[:] = path_before + for name, module in list(sys.modules.items()): + origin = getattr(module, "__file__", None) + if origin and Path(origin).resolve().is_relative_to(root): + del sys.modules[name] + + class HookResolver: """Finds and runs lifecycle hooks for a stacklet. @@ -256,12 +282,13 @@ def run(self, name: str, ctx) -> bool: def _run_python(self, path: Path, ctx) -> bool: """Load and run a Python hook.""" try: - spec = importlib.util.spec_from_file_location( - f"hook.{path.stem}", path) - mod = importlib.util.module_from_spec(spec) - spec.loader.exec_module(mod) - if hasattr(mod, "run"): - mod.run(ctx) + with stacklet_imports(self._stacklet_dir): + spec = importlib.util.spec_from_file_location( + f"hook.{path.stem}", path) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + if hasattr(mod, "run"): + mod.run(ctx) return True except Cancelled: raise diff --git a/lib/stack/stack.py b/lib/stack/stack.py index 70726a14..bf1b3d8a 100644 --- a/lib/stack/stack.py +++ b/lib/stack/stack.py @@ -1387,39 +1387,41 @@ def run_cli_command(self, stacklet_id: str, command: str, args: list | None = No return {"error": f"Command '{command}' not found for {stacklet_id}"} try: - spec = importlib.util.spec_from_file_location( - f"{stacklet_id}.cli.{command}", module_path) - mod = importlib.util.module_from_spec(spec) - spec.loader.exec_module(mod) - if hasattr(mod, "run"): - # TODO: secrets.all() exposes every secret to every plugin. - # Fine for trusted stacklets, but if we ever support community - # plugins, scope this to the stacklet's own secrets only. - from .users import load_users - config = { - "domain": self._cfg("core", "domain"), - # Same base the containers get as LINK_BASE_URL, for - # plugins that print a `/go` link. Derived here rather - # than from `domain` above, because port mode has no - # domain and the LAN fallback lives in _home_url. - "home_url": self._home_url(), - "data_dir": str(self.data), - "repo_root": str(self.root), - "instance_dir": str(self.instance_dir), - "manifest": stacklet.get("manifest", {}), - "stack": self.config, - "secrets": self.secrets.all(), - "users": load_users(self.instance_dir), - # Lazy health probe — plugins call config["is_healthy"]() - # when they need to gate work on the stacklet actually - # responding. Zero-arg closure so we don't pay the HTTP - # round-trip on every plugin invocation. - "is_healthy": lambda: self.is_healthy(stacklet_id), - # Save a setting to stack.toml, the plugin-side twin of - # a hook's ctx.cfg(key, value), for any section. - "set_cfg": self._set_cfg, - } - return mod.run(args or [], stacklet, config) + from .hooks import stacklet_imports + with stacklet_imports(Path(stacklet["path"])): + spec = importlib.util.spec_from_file_location( + f"{stacklet_id}.cli.{command}", module_path) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + if hasattr(mod, "run"): + # TODO: secrets.all() exposes every secret to every plugin. + # Fine for trusted stacklets, but if we ever support community + # plugins, scope this to the stacklet's own secrets only. + from .users import load_users + config = { + "domain": self._cfg("core", "domain"), + # Same base the containers get as LINK_BASE_URL, for + # plugins that print a `/go` link. Derived here rather + # than from `domain` above, because port mode has no + # domain and the LAN fallback lives in _home_url. + "home_url": self._home_url(), + "data_dir": str(self.data), + "repo_root": str(self.root), + "instance_dir": str(self.instance_dir), + "manifest": stacklet.get("manifest", {}), + "stack": self.config, + "secrets": self.secrets.all(), + "users": load_users(self.instance_dir), + # Lazy health probe — plugins call config["is_healthy"]() + # when they need to gate work on the stacklet actually + # responding. Zero-arg closure so we don't pay the HTTP + # round-trip on every plugin invocation. + "is_healthy": lambda: self.is_healthy(stacklet_id), + # Save a setting to stack.toml, the plugin-side twin of + # a hook's ctx.cfg(key, value), for any section. + "set_cfg": self._set_cfg, + } + return mod.run(args or [], stacklet, config) except Exception as e: return {"error": str(e)} diff --git a/tests/unit/framework/test_cli_class.py b/tests/unit/framework/test_cli_class.py index a4ac4d12..88bc7f6b 100644 --- a/tests/unit/framework/test_cli_class.py +++ b/tests/unit/framework/test_cli_class.py @@ -370,6 +370,30 @@ def test_destroy_with_an_unknown_id_destroys_nothing(self, tmp_path, monkeypatch assert cli.stack.is_installed("a") +class TestEveryUpPullsWhatIsMissing: + """After `stack update`, a release can name new image tags for a + stacklet that is already set up. `compose up` fetches a missing image + itself, with its output captured, so `stack restart` sat at "Starting + containers" for minutes with nothing on screen. The visible pull step + runs on every up, not only the first.""" + + def test_a_second_up_pulls_before_it_starts(self, tmp_path, monkeypatch): + cli, _ = _make_cli(tmp_path, {"myapp": {}}) + (tmp_path / "stacklets" / "myapp" / "docker-compose.yml").write_text( + "name: stack-myapp\nservices: {}\n") + steps = [] + monkeypatch.setattr("stack.docker.compose_pull", + lambda *a, **kw: steps.append("pull")) + monkeypatch.setattr("stack.docker.compose_up", + lambda *a, **kw: (steps.append("up"), (0, ""))[1]) + cli.up("myapp") + steps.clear() + + cli.up("myapp") + + assert steps == ["pull", "up"] + + class TestUpFailureNamesTheCause: """When compose cannot start a container, the reason is in its output (a port another process holds, a missing mount), and the admin diff --git a/tests/unit/framework/test_docker_runtime.py b/tests/unit/framework/test_docker_runtime.py index e0dbb5d2..fe990191 100644 --- a/tests/unit/framework/test_docker_runtime.py +++ b/tests/unit/framework/test_docker_runtime.py @@ -211,3 +211,18 @@ def test_offline_it_builds_from_the_cached_base(self): pulled, cached = self._build(1, 0) assert "--pull" in pulled assert cached[cached.index("build"):] == ["build"] + + +class TestComposePullFetchesOnlyWhatIsMissing: + """The pull runs on every `stack up`, so a release that moves a service + to a new image tag downloads it with its progress on screen. It must + cost nothing when every image is here, and leave built images to the + build.""" + + def test_only_missing_images_are_pulled(self): + from stack import docker + docker._context = None + with patch("subprocess.run") as run: + docker.compose_pull("/tmp/compose.yml") + cmd = run.call_args[0][0] + assert cmd[cmd.index("pull"):] == ["pull", "--policy", "missing", "--ignore-buildable"] diff --git a/tests/unit/framework/test_hook_resolver.py b/tests/unit/framework/test_hook_resolver.py index 77d2b711..ca5930af 100644 --- a/tests/unit/framework/test_hook_resolver.py +++ b/tests/unit/framework/test_hook_resolver.py @@ -252,3 +252,45 @@ def _make_ctx(env=None, step_fn=None): """Build a minimal StackContext for testing.""" from stack.hooks import StackContext return StackContext(stack=None, stacklet_id="test", env=env or {}, step_fn=step_fn) + + +class TestEachStackletImportsItsOwnHelpers: + """Hooks import their stacklet's helper modules by bare name. One + process runs many stacklets' hooks (`stack restart all`), and code and + photos both ship an `oauth.py`: photos was handed code's, and its + start failed with `sync() takes 4 positional arguments but 5 were + given`.""" + + HOOK = textwrap.dedent("""\ + import sys + from pathlib import Path + sys.path.insert(0, str(Path(__file__).parent.parent)) + from helper import WHO + + def run(ctx): + ctx["seen"].append(WHO) + """) + + def _stacklet(self, tmp_path, name): + from stack.hooks import HookResolver + root = tmp_path / "stacklets" / name + (root / "hooks").mkdir(parents=True) + (root / "helper.py").write_text(f"WHO = {name!r}\n") + (root / "hooks" / "on_start.py").write_text(self.HOOK) + return HookResolver(root) + + def test_two_stacklets_with_a_helper_of_the_same_name(self, tmp_path): + seen = [] + code, photos = self._stacklet(tmp_path, "code"), self._stacklet(tmp_path, "photos") + + assert code.run("on_start", {"seen": seen}) + assert photos.run("on_start", {"seen": seen}) + + assert seen == ["code", "photos"] + + def test_the_search_path_is_left_as_it_was(self, tmp_path): + before = list(sys.path) + + self._stacklet(tmp_path, "code").run("on_start", {"seen": []}) + + assert sys.path == before