diff --git a/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.py b/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.py index fe9ef52312..43c410457d 100644 --- a/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.py +++ b/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.py @@ -20,6 +20,7 @@ from nemo_deployments_plugin.backends.docker.config import DockerExecutorConfig from nemo_deployments_plugin.backends.docker.containers import ( DeploymentConfigError, + build_docker_plan, build_port_bindings, build_volume_bindings, device_requests_for_gpus, @@ -28,7 +29,6 @@ merged_volume_mounts, parse_docker_backend_config, restart_policy_kwargs, - validate_config_for_docker, ) from nemo_deployments_plugin.backends.docker.gpu import GPUAllocationError, get_shared_gpu_pool from nemo_deployments_plugin.backends.docker.ports import find_available_port @@ -42,10 +42,13 @@ from nemo_deployments_plugin.backends.labels import ( BACKOFF_LIMIT_LABEL, CONFIG_NAME_LABEL, + CONTAINER_ROLE_LABEL, + CONTAINER_ROLE_SERVER, DEPLOYMENT_NAME_LABEL, DEPLOYMENT_WORKSPACE_LABEL, MANAGED_BY_KEY, RESTART_POLICY_LABEL, + companion_container_name, container_name, deployment_identity_labels, deployment_key, @@ -160,7 +163,7 @@ async def create_deployment( try: config = await self._load_deployment_config(workspace, config_name) config = await resolve_deployment_config_secrets(self._sdk, config) - container_spec = validate_config_for_docker(config) + plan = build_docker_plan(config) except DeploymentConfigError as exc: return BackendStatusUpdate(status="FAILED", status_message=str(exc)) except SecretResolutionError as exc: @@ -174,6 +177,7 @@ async def create_deployment( logger.exception("Failed to load deployment config %s/%s", workspace, config_name) return BackendStatusUpdate(status="FAILED", status_message=f"Failed to load deployment config: {exc}") + container_spec = plan.primary docker_cfg = parse_docker_backend_config(backend_config) if config.backend_config.docker is not None: docker_cfg = config.backend_config.docker @@ -209,17 +213,30 @@ async def create_deployment( ) host_ports[port_spec.container_port] = host_port + # Pull all images in the group (init + primary + sidecars) up front. if self._executor_config.pull_images: - pull_error = await self._pull_image( - container_spec.image, - ngc_api_key=env_dict(container_spec).get("NGC_API_KEY"), - ) - if pull_error is not None: - if gpu_ids and gpu_pool is not None: - gpu_pool.release_gpu(dep_key) - return BackendStatusUpdate(status="FAILED", status_message=pull_error) - - all_labels = { + for container in [*plan.init_containers, container_spec, *plan.sidecars]: + pull_error = await self._pull_image( + container.image, + ngc_api_key=env_dict(container).get("NGC_API_KEY"), + ) + if pull_error is not None: + # A pull failure is only fatal if the image is not already + # present locally. Locally-built/loaded images (e.g. the LoRA + # adapters sidecar `nmp-api:local`) are not pullable from a + # registry, so fall back to the local copy when it exists. + # Only a genuine "not found locally" is fatal here; other + # docker client errors from images.get propagate rather than + # being masked as "image present". + try: + await asyncio.to_thread(self._client.images.get, container.image) + logger.info("Image %s not pullable but present locally; using local copy", container.image) + except (self._docker_errors.ImageNotFound, self._docker_errors.NotFound): + if gpu_ids and gpu_pool is not None: + gpu_pool.release_gpu(dep_key) + return BackendStatusUpdate(status="FAILED", status_message=pull_error) + + base_labels = { **labels, **config.labels, **deployment_identity_labels( @@ -230,12 +247,100 @@ async def create_deployment( backoff_limit=config.backoff_limit, ), } + + # 1) Init containers run to completion, in order, before the group starts. + for init in plan.init_containers: + init_status = await self._run_init_container( + workspace=workspace, + name=name, + config=config, + init=init, + base_labels=base_labels, + dep_key=dep_key, + gpu_ids=gpu_ids, + ) + if init_status is not None: + return init_status + + # 2) Primary (server) container: publishes ports, owns GPUs. + server_run_kwargs = self._build_run_kwargs( + workspace=workspace, + config=config, + container=container_spec, + name=c_name, + labels={**base_labels, CONTAINER_ROLE_LABEL: CONTAINER_ROLE_SERVER}, + host_ports=host_ports, + gpu_ids=gpu_ids, + network=docker_cfg.network, + ) + try: + await asyncio.to_thread(self._client.containers.run, **server_run_kwargs) + except Exception as exc: + if gpu_ids and gpu_pool is not None: + gpu_pool.release_gpu(dep_key) + logger.exception("Failed to start container %s", c_name) + return BackendStatusUpdate(status="FAILED", status_message=f"Failed to start container: {exc}") + + # 3) Sidecars share the primary's network namespace + volumes; no ports/GPU. + # + # Known limitation: a sidecar joins ``network=container:``, so it + # is tied to *this* server container instance. Docker's Always restart + # policy restarts the same server container in place (netns preserved), so + # ordinary server crashes are fine. But if the server container is ever + # replaced by a *new* container (a fresh create rather than an in-place + # restart), the sidecar's netns reference dangles and nothing re-runs the + # sidecars. Today a full recreate goes through delete_model_deployment + # (which tears down the whole group) before create, so the sidecars are + # re-run together; a future partial/primary-only recreate would need to + # recreate the whole group to stay consistent. + for sidecar in plan.sidecars: + sidecar_name = companion_container_name(workspace, name, sidecar.name) + sidecar_run_kwargs = self._build_run_kwargs( + workspace=workspace, + config=config, + container=sidecar, + name=sidecar_name, + labels={**base_labels, CONTAINER_ROLE_LABEL: sidecar.name}, + host_ports={}, + gpu_ids=[], + network=f"container:{c_name}", + ) + try: + await asyncio.to_thread(self._client.containers.run, **sidecar_run_kwargs) + except Exception as exc: + logger.exception("Failed to start sidecar container %s", sidecar_name) + # Tear the whole group down so we don't leave a half-started deployment. + await self.delete_deployment(workspace, name) + return BackendStatusUpdate( + status="FAILED", status_message=f"Failed to start sidecar {sidecar.name}: {exc}" + ) + + endpoints = self._build_endpoints(container_spec, host_ports) + return BackendStatusUpdate( + status="STARTING", + status_message=f"Container {c_name} created", + endpoints=endpoints, + ) + + def _build_run_kwargs( + self, + *, + workspace: str, + config: DeploymentConfig, + container: Container, + name: str, + labels: dict[str, str], + host_ports: dict[int, int], + gpu_ids: list[int], + network: str | None, + ) -> dict[str, Any]: + """Build docker ``containers.run`` kwargs for one container in a group.""" run_kwargs: dict[str, Any] = { - "image": container_spec.image, - "name": c_name, + "image": container.image, + "name": name, "detach": True, - "labels": all_labels, - "environment": env_dict(container_spec), + "labels": labels, + "environment": env_dict(container), **restart_policy_kwargs(config.restart_policy, config.backoff_limit), } # Mirror Kubernetes semantics (see the k8s compiler): a container spec's @@ -244,39 +349,100 @@ async def create_deployment( # kwargs so a driven container overrides the image's baked-in ENTRYPOINT # instead of appending to it. Conflating both into docker ``command`` # leaves the image ENTRYPOINT in force and silently drops the spec. - if container_spec.command: - run_kwargs["entrypoint"] = list(container_spec.command) - if container_spec.args: - run_kwargs["command"] = list(container_spec.args) + if container.command: + run_kwargs["entrypoint"] = list(container.command) + if container.args: + run_kwargs["command"] = list(container.args) - volume_bindings = build_volume_bindings(workspace, merged_volume_mounts(config, container_spec)) + volume_bindings = build_volume_bindings(workspace, merged_volume_mounts(config, container)) if volume_bindings: run_kwargs["volumes"] = volume_bindings - if container_spec.ports: - run_kwargs["ports"] = build_port_bindings(container_spec, host_ports) + # When joining another container's network namespace, docker forbids + # publishing ports (they belong to the primary). Only the primary maps + # host ports. + if network is not None and network.startswith("container:"): + run_kwargs["network"] = network + else: + if container.ports: + run_kwargs["ports"] = build_port_bindings(container, host_ports) + if network: + run_kwargs["network"] = network device_requests = device_requests_for_gpus(gpu_ids) if device_requests: run_kwargs["device_requests"] = device_requests - if docker_cfg.network: - run_kwargs["network"] = docker_cfg.network + return run_kwargs + + async def _run_init_container( + self, + *, + workspace: str, + name: str, + config: DeploymentConfig, + init: Container, + base_labels: dict[str, str], + dep_key: str, + gpu_ids: list[int], + ) -> BackendStatusUpdate | None: + """Run one init container to completion. Return an error status or None on success.""" + init_name = companion_container_name(workspace, name, f"init-{init.name}") + # Best-effort clean any stale init container from a prior attempt. + try: + stale = await asyncio.to_thread(self._client.containers.get, init_name) + await asyncio.to_thread(stale.remove, force=True) + except self._docker_errors.NotFound: + pass + except Exception: + logger.warning("Failed to remove stale init container %s", init_name, exc_info=True) + + # Init containers are intentionally CPU-only: no device_requests is set, + # so they never receive a GPU. This suits the current init workload + # (lora-cache-init prepares the scratch dir). If the models compiler ever + # emits a GPU-needing init container, this would need to plumb GPUs + # through here. + run_kwargs: dict[str, Any] = { + "image": init.image, + "name": init_name, + "detach": True, + "labels": {**base_labels, CONTAINER_ROLE_LABEL: f"init-{init.name}"}, + "environment": env_dict(init), + } + if init.command: + run_kwargs["entrypoint"] = list(init.command) + if init.args: + run_kwargs["command"] = list(init.args) + volume_bindings = build_volume_bindings(workspace, merged_volume_mounts(config, init)) + if volume_bindings: + run_kwargs["volumes"] = volume_bindings + + def _run_and_wait() -> int: + container = self._client.containers.run(**run_kwargs) + result = container.wait(timeout=self._executor_config.docker_timeout) + exit_code = int(result.get("StatusCode", 1)) if isinstance(result, dict) else int(result) + try: + container.remove(force=True) + except Exception: + logger.warning("Failed to remove init container %s", init_name, exc_info=True) + return exit_code try: - await asyncio.to_thread(self._client.containers.run, **run_kwargs) + exit_code = await asyncio.to_thread(_run_and_wait) except Exception as exc: - if gpu_ids and gpu_pool is not None: - gpu_pool.release_gpu(dep_key) - logger.exception("Failed to start container %s", c_name) - return BackendStatusUpdate(status="FAILED", status_message=f"Failed to start container: {exc}") + if gpu_ids and self._gpu_pool is not None: + self._gpu_pool.release_gpu(dep_key) + logger.exception("Init container %s failed to run", init_name) + return BackendStatusUpdate(status="FAILED", status_message=f"Init container {init.name} failed: {exc}") - endpoints = self._build_endpoints(container_spec, host_ports) - return BackendStatusUpdate( - status="STARTING", - status_message=f"Container {c_name} created", - endpoints=endpoints, - ) + if exit_code != 0: + if gpu_ids and self._gpu_pool is not None: + self._gpu_pool.release_gpu(dep_key) + return BackendStatusUpdate( + status="FAILED", + status_message=f"Init container {init.name} exited with code {exit_code}", + ) + return None async def read_status(self, *, workspace: str, name: str) -> BackendStatusUpdate: c_name = container_name(workspace, name) @@ -324,6 +490,13 @@ async def read_status(self, *, workspace: str, name: str) -> BackendStatusUpdate host_ports=host_ports, ) if ready and restart_policy == "Always": + sidecar_ok, sidecar_reason = await self._sidecars_healthy(workspace, name, config) + if not sidecar_ok: + return BackendStatusUpdate( + status="STARTING", + status_message=f"Server ready but sidecar not ready ({sidecar_reason})", + endpoints=endpoints, + ) return BackendStatusUpdate( status="READY", status_message=f"Container running and ready ({reason})", @@ -387,17 +560,92 @@ async def read_status(self, *, workspace: str, name: str) -> BackendStatusUpdate return BackendStatusUpdate(status="STARTING", status_message=f"Container state: {state}") + async def _sidecars_healthy(self, workspace: str, name: str, config: DeploymentConfig | None) -> tuple[bool, str]: + """Return (all_healthy, reason) for a deployment's expected sidecar containers. + + The set of expected sidecars is derived from the deployment ``config`` + (every container after the primary), so a sidecar that has been *removed* + entirely — not just exited — is still detected as not-ready. A sidecar is + healthy only when a container for its role is present AND running; an + exited or missing sidecar keeps the deployment out of READY (its Always + restart policy recreates an exited one; a missing one signals a broken + group). + + Deployments with no sidecars (the common single-container case) skip the + Docker enumeration entirely. + """ + expected_roles = {sidecar.name for sidecar in build_docker_plan(config).sidecars} if config else set() + if not expected_roles: + return True, "no sidecars" + + def _list() -> list[Any]: + return self._client.containers.list( + all=True, + filters={ + "label": [ + f"{MANAGED_BY_KEY}={MANAGED_BY_LABEL}", + f"{DEPLOYMENT_WORKSPACE_LABEL}={workspace}", + f"{DEPLOYMENT_NAME_LABEL}={name}", + ] + }, + ) + + try: + containers = await asyncio.to_thread(_list) + except Exception: + # If we cannot enumerate the group, do not block readiness on it. + logger.warning("Failed to list sidecars for %s/%s", workspace, name, exc_info=True) + return True, "sidecar check skipped" + + running_roles: set[str] = set() + for container in containers: + role = (container.labels or {}).get(CONTAINER_ROLE_LABEL, "") + if role not in expected_roles: + continue + if container.status != "running": + return False, f"sidecar '{role}' is {container.status}" + running_roles.add(role) + + missing = expected_roles - running_roles + if missing: + return False, f"sidecar '{sorted(missing)[0]}' is missing" + return True, "sidecars running" + async def delete_deployment(self, workspace: str, name: str) -> BackendStatusUpdate: c_name = container_name(workspace, name) dep_key = deployment_key(workspace, name) def _delete() -> None: + # Remove every container in the deployment group: the primary + # (dep-) plus any companion sidecar/init containers, which are + # discoverable by the shared deployment identity labels. + group: dict[str, Any] = {} try: - container = self._client.containers.get(c_name) - container.stop(timeout=30) - container.remove(force=True) - except self._docker_errors.NotFound: - return + for container in self._client.containers.list( + all=True, + filters={ + "label": [ + f"{MANAGED_BY_KEY}={MANAGED_BY_LABEL}", + f"{DEPLOYMENT_WORKSPACE_LABEL}={workspace}", + f"{DEPLOYMENT_NAME_LABEL}={name}", + ] + }, + ): + group[container.name] = container + except Exception: + logger.warning("Failed to list group containers for %s; falling back to primary", c_name, exc_info=True) + # Ensure the primary is included even if the label list query missed it. + if c_name not in group: + try: + group[c_name] = self._client.containers.get(c_name) + except self._docker_errors.NotFound: + pass + for container in group.values(): + try: + container.stop(timeout=30) + container.remove(force=True) + except self._docker_errors.NotFound: + continue try: await asyncio.to_thread(_delete) diff --git a/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/containers.py b/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/containers.py index 00b50cd6e4..b5575423e5 100644 --- a/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/containers.py +++ b/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/containers.py @@ -5,6 +5,7 @@ from __future__ import annotations +from dataclasses import dataclass, field from typing import Any from nemo_deployments_plugin.backends.labels import docker_volume_name @@ -16,17 +17,70 @@ class DeploymentConfigError(ValueError): """Invalid deployment config for docker backend.""" +@dataclass(frozen=True) +class DockerDeploymentPlan: + """The docker backend's view of a (possibly multi-container) DeploymentConfig. + + - ``init_containers`` run to completion, in order, before the group starts. + - ``primary`` is the server container; it keeps the canonical deployment + container name, publishes host ports, and owns any GPUs. + - ``sidecars`` share the primary's network namespace (``network=container:``) + and volumes; they do not publish host ports or own GPUs. + """ + + primary: Container + init_containers: list[Container] = field(default_factory=list) + sidecars: list[Container] = field(default_factory=list) + + @property + def is_multi_container(self) -> bool: + """True when the plan has any init containers or sidecars beyond the primary.""" + return bool(self.init_containers or self.sidecars) + + def parse_docker_backend_config(backend_config: dict[str, Any]) -> DockerDeploymentConfig: docker_section = backend_config.get("docker") or {} return DockerDeploymentConfig.model_validate(docker_section) +def build_docker_plan(config: DeploymentConfig) -> DockerDeploymentPlan: + """Split a DeploymentConfig into a docker orchestration plan. + + The first container is the primary/server; any additional containers are + sidecars sharing the primary's network namespace and volumes (e.g. the LoRA + adapters sidecar). Init containers run to completion first. + + Raises DeploymentConfigError for shapes the docker backend cannot honor. + """ + if not config.containers: + raise DeploymentConfigError("docker backend requires at least one container") + + primary = config.containers[0] + sidecars = list(config.containers[1:]) + + # Sidecars share the primary's netns, so they cannot publish their own host + # ports. (The primary owns the published ports for the whole group.) + for sidecar in sidecars: + if sidecar.ports: + raise DeploymentConfigError( + f"docker sidecar container '{sidecar.name}' may not declare ports; " + "it shares the primary container's network namespace" + ) + + return DockerDeploymentPlan( + primary=primary, + init_containers=list(config.init_containers), + sidecars=sidecars, + ) + + def validate_config_for_docker(config: DeploymentConfig) -> Container: - if config.init_containers: - raise DeploymentConfigError("init_containers are not supported by the docker backend in v1") - if len(config.containers) != 1: - raise DeploymentConfigError(f"docker backend v1 supports exactly one container; got {len(config.containers)}") - return config.containers[0] + """Back-compat single-container accessor: returns the primary container. + + Retained for callers/tests that only need the primary; use + :func:`build_docker_plan` for multi-container orchestration. + """ + return build_docker_plan(config).primary def restart_policy_kwargs(restart_policy: RestartPolicy, backoff_limit: int) -> dict[str, Any]: diff --git a/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/labels.py b/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/labels.py index 17068e7b4d..99ace6ebe1 100644 --- a/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/labels.py +++ b/plugins/nemo-deployments/src/nemo_deployments_plugin/backends/labels.py @@ -25,6 +25,14 @@ VOLUME_WORKSPACE_LABEL = "nemo.nvidia.com/volume-workspace" VOLUME_NAME_LABEL = "nemo.nvidia.com/volume-name" BACKOFF_LIMIT_LABEL = "nemo.nvidia.com/backoff-limit" +# Role of a docker container within a multi-container deployment group +# (e.g. "server" or a sidecar container name). Used to discover companion +# containers (LoRA adapters sidecar) that share the primary server container. +CONTAINER_ROLE_LABEL = "nemo.nvidia.com/container-role" + +# The primary container in a deployment group keeps the canonical deployment +# name (``container_name``); companions derive their names from it plus role. +CONTAINER_ROLE_SERVER = "server" def deployment_key(workspace: str, name: str) -> str: @@ -33,10 +41,24 @@ def deployment_key(workspace: str, name: str) -> str: def container_name(workspace: str, deployment_name: str) -> str: - """Docker container name for a deployment (``dep-`` prefix, hashed identity).""" + """Docker container name for a deployment (``dep-`` prefix, hashed identity). + + This is the *primary* (server) container of a deployment group. Companion + containers (init/sidecar) derive their names from it via + :func:`companion_container_name`. + """ return k8s_deployment_resource_name(workspace, deployment_name) +def companion_container_name(workspace: str, deployment_name: str, role: str) -> str: + """Docker container name for a companion (sidecar/init) container in a group. + + Derived from the primary container name so the whole group shares the + ``dep-`` stem and is easy to discover/tear down together. + """ + return f"{container_name(workspace, deployment_name)}-{role}" + + def docker_volume_name(workspace: str, volume_name: str) -> str: """Docker volume name for a deployment volume (``dep-vol-`` prefix, hashed identity).""" return k8s_volume_resource_name(workspace, volume_name) diff --git a/plugins/nemo-deployments/tests/unit/backends/docker/docker_helpers.py b/plugins/nemo-deployments/tests/unit/backends/docker/docker_helpers.py index 99e761a669..93d4916d1c 100644 --- a/plugins/nemo-deployments/tests/unit/backends/docker/docker_helpers.py +++ b/plugins/nemo-deployments/tests/unit/backends/docker/docker_helpers.py @@ -7,7 +7,7 @@ from typing import Any -from nemo_deployments_plugin.entities import Container, DeploymentConfig +from nemo_deployments_plugin.entities import Container, ContainerPort, DeploymentConfig, VolumeMount from nemo_deployments_plugin.types import RestartPolicy @@ -27,6 +27,49 @@ def sample_config(*, restart_policy: RestartPolicy = "Always") -> DeploymentConf ) +def lora_config(*, restart_policy: RestartPolicy = "Always") -> DeploymentConfig: + """A LoRA-shaped multi-container config: init + server + adapters sidecar. + + Mirrors what the models compiler emits for docker + LoRA (server publishes + :8000, adapters sidecar shares the server netns and the scratch volume). + """ + return DeploymentConfig( + name="cfg1", + workspace="default", + initContainers=[ + Container( + name="lora-cache-init", + image="docker.io/library/busybox:latest", + command=["sh", "-c", "mkdir -p /scratch/loras && chmod -R 777 /scratch/loras"], + volumeMounts=[VolumeMount(name="scratch", mountPath="/scratch")], + ) + ], + containers=[ + Container( + name="server", + image="vllm/vllm-openai:v0.22.1", + command=["vllm", "serve"], + args=["/model-store"], + ports=[ContainerPort(name="http", containerPort=8000)], + volumeMounts=[ + VolumeMount(name="weights", mountPath="/model-store", readOnly=True), + VolumeMount(name="scratch", mountPath="/scratch"), + ], + ), + Container( + name="lora-adapters", + image="my-registry/nmp-api:local", + command=["nemo", "services", "run", "--sidecars", "adapters"], + volumeMounts=[ + VolumeMount(name="weights", mountPath="/model-store", readOnly=True), + VolumeMount(name="scratch", mountPath="/scratch"), + ], + ), + ], + restart_policy=restart_policy, # ty: ignore[unknown-argument] + ) + + def container_attrs(*, status: str = "running", exit_code: int = 0) -> dict[str, Any]: del status return { diff --git a/plugins/nemo-deployments/tests/unit/backends/docker/test_backend_mocked.py b/plugins/nemo-deployments/tests/unit/backends/docker/test_backend_mocked.py index 4dc699fc55..9be1747f83 100644 --- a/plugins/nemo-deployments/tests/unit/backends/docker/test_backend_mocked.py +++ b/plugins/nemo-deployments/tests/unit/backends/docker/test_backend_mocked.py @@ -8,14 +8,17 @@ from unittest.mock import AsyncMock, MagicMock import pytest -from backends.docker.docker_helpers import container_attrs, sample_config +from backends.docker.docker_helpers import container_attrs, lora_config, sample_config from docker.errors import APIError, NotFound from nemo_deployments_plugin.backends.docker.backend import DockerDeploymentBackend from nemo_deployments_plugin.backends.labels import ( CONFIG_NAME_LABEL, + CONTAINER_ROLE_LABEL, DEPLOYMENT_NAME_LABEL, DEPLOYMENT_WORKSPACE_LABEL, RESTART_POLICY_LABEL, + companion_container_name, + container_name, ) from nemo_deployments_plugin.constants import MANAGED_BY_LABEL from nemo_deployments_plugin.entities import Deployment @@ -73,6 +76,184 @@ async def test_create_deployment_maps_command_to_entrypoint( assert run_kwargs["command"] == ["hello"] +@pytest.mark.asyncio +async def test_create_lora_group_runs_init_server_and_sidecar( + docker_backend: DockerDeploymentBackend, + mock_entities: AsyncMock, + mock_docker_client: MagicMock, +) -> None: + """A LoRA-shaped config runs init-to-completion, then server, then sidecar. + + The sidecar shares the server's network namespace (network=container:) + and publishes no host ports; only the server maps ports. + """ + mock_entities.get.return_value = lora_config() + mock_docker_client.containers.get.side_effect = NotFound("missing") + + # Init container is run+waited: containers.run returns a container whose + # wait() reports success. + init_container = MagicMock() + init_container.wait.return_value = {"StatusCode": 0} + server_container = MagicMock(id="server123") + sidecar_container = MagicMock(id="sidecar123") + mock_docker_client.containers.run.side_effect = [init_container, server_container, sidecar_container] + + update = await docker_backend.create_deployment( + workspace="default", + name="srv", + config_name="cfg1", + labels={"managed-by": MANAGED_BY_LABEL}, + backend_config={}, + ) + + assert update.status == "STARTING" + calls = mock_docker_client.containers.run.call_args_list + assert len(calls) == 3 + + # 1) init container ran to completion (detached then waited + removed) + init_container.wait.assert_called_once() + init_container.remove.assert_called_once() + + # 2) server publishes ports, no shared netns + server_kwargs = calls[1].kwargs + assert server_kwargs["name"] == container_name("default", "srv") + assert server_kwargs["labels"][CONTAINER_ROLE_LABEL] == "server" + assert "ports" in server_kwargs + assert server_kwargs.get("network", "") == "" + + # 3) sidecar shares the server netns, publishes no ports + sidecar_kwargs = calls[2].kwargs + assert sidecar_kwargs["name"] == companion_container_name("default", "srv", "lora-adapters") + assert sidecar_kwargs["labels"][CONTAINER_ROLE_LABEL] == "lora-adapters" + assert sidecar_kwargs["network"] == f"container:{container_name('default', 'srv')}" + assert "ports" not in sidecar_kwargs + + +@pytest.mark.asyncio +async def test_create_lora_group_fails_when_init_nonzero( + docker_backend: DockerDeploymentBackend, + mock_entities: AsyncMock, + mock_docker_client: MagicMock, +) -> None: + """A non-zero init container fails the deployment before the server starts.""" + mock_entities.get.return_value = lora_config() + mock_docker_client.containers.get.side_effect = NotFound("missing") + + init_container = MagicMock() + init_container.wait.return_value = {"StatusCode": 1} + mock_docker_client.containers.run.return_value = init_container + + update = await docker_backend.create_deployment( + workspace="default", + name="srv", + config_name="cfg1", + labels={"managed-by": MANAGED_BY_LABEL}, + backend_config={}, + ) + + assert update.status == "FAILED" + assert "init" in update.status_message.lower() + # only the init container was run (server/sidecar never started) + assert mock_docker_client.containers.run.call_count == 1 + + +@pytest.mark.asyncio +async def test_create_falls_back_to_local_image_when_pull_fails( + mock_sdk: MagicMock, + mock_entities: AsyncMock, + mock_docker_client: MagicMock, +) -> None: + """A pull failure is tolerated when the image is already present locally. + + Local-only images (e.g. the LoRA adapters sidecar ``nmp-api:local``) are not + pullable from a registry; the backend uses the local copy instead of failing. + """ + from unittest.mock import patch + + with ( + patch("nemo_deployments_plugin.backends.docker.backend.AsyncEntitiesResource"), + patch("nemo_deployments_plugin.backends.docker.backend.NemoEntitiesClient", return_value=mock_entities), + patch("nemo_deployments_plugin.backends.docker.backend.get_shared_gpu_pool", return_value=None), + patch("docker.from_env", return_value=mock_docker_client), + ): + # pull_images enabled so the pull path runs + backend = DockerDeploymentBackend(mock_sdk, {"docker_timeout": 60, "pull_images": True}) + backend._client = mock_docker_client + + mock_entities.get.return_value = sample_config() + mock_docker_client.containers.get.side_effect = NotFound("missing") + mock_docker_client.images.pull.side_effect = APIError("404 not found") + mock_docker_client.images.get.return_value = MagicMock() # present locally + mock_docker_client.containers.run.return_value = MagicMock(id="abc123") + + update = await backend.create_deployment( + workspace="default", + name="srv", + config_name="cfg1", + labels={"managed-by": MANAGED_BY_LABEL}, + backend_config={}, + ) + + assert update.status == "STARTING" + mock_docker_client.images.get.assert_called_once() + mock_docker_client.containers.run.assert_called_once() + + +@pytest.mark.asyncio +async def test_create_fails_when_pull_fails_and_no_local_image( + mock_sdk: MagicMock, + mock_entities: AsyncMock, + mock_docker_client: MagicMock, +) -> None: + """If the image cannot be pulled AND is not present locally, create fails.""" + from unittest.mock import patch + + with ( + patch("nemo_deployments_plugin.backends.docker.backend.AsyncEntitiesResource"), + patch("nemo_deployments_plugin.backends.docker.backend.NemoEntitiesClient", return_value=mock_entities), + patch("nemo_deployments_plugin.backends.docker.backend.get_shared_gpu_pool", return_value=None), + patch("docker.from_env", return_value=mock_docker_client), + ): + backend = DockerDeploymentBackend(mock_sdk, {"docker_timeout": 60, "pull_images": True}) + backend._client = mock_docker_client + + mock_entities.get.return_value = sample_config() + mock_docker_client.containers.get.side_effect = NotFound("missing") + mock_docker_client.images.pull.side_effect = APIError("404 not found") + mock_docker_client.images.get.side_effect = NotFound("missing locally") + + update = await backend.create_deployment( + workspace="default", + name="srv", + config_name="cfg1", + labels={"managed-by": MANAGED_BY_LABEL}, + backend_config={}, + ) + + assert update.status == "FAILED" + assert "pull image" in update.status_message.lower() + mock_docker_client.containers.run.assert_not_called() + + +@pytest.mark.asyncio +async def test_delete_removes_whole_group( + docker_backend: DockerDeploymentBackend, + mock_docker_client: MagicMock, +) -> None: + """delete_deployment stops+removes every container in the group.""" + server = MagicMock() + server.name = container_name("default", "srv") + sidecar = MagicMock() + sidecar.name = companion_container_name("default", "srv", "lora-adapters") + mock_docker_client.containers.list.return_value = [server, sidecar] + + update = await docker_backend.delete_deployment("default", "srv") + + assert update.status == "SUCCEEDED" + server.remove.assert_called_once() + sidecar.remove.assert_called_once() + + @pytest.mark.asyncio async def test_create_volume_runs_init_chmod_container( docker_backend: DockerDeploymentBackend, @@ -172,6 +353,91 @@ async def test_read_status_ready_when_running_without_probe( assert update.status == "READY" +def _running_server_container() -> MagicMock: + container = MagicMock() + container.id = "abc123def456" + container.status = "running" + container.labels = { + "managed-by": MANAGED_BY_LABEL, + RESTART_POLICY_LABEL: "Always", + CONFIG_NAME_LABEL: "cfg1", + } + container.ports = {} + container.attrs = container_attrs() + return container + + +def _sidecar_container(role: str, status: str) -> MagicMock: + sidecar = MagicMock() + sidecar.status = status + sidecar.labels = { + "managed-by": MANAGED_BY_LABEL, + DEPLOYMENT_WORKSPACE_LABEL: "default", + DEPLOYMENT_NAME_LABEL: "srv", + CONTAINER_ROLE_LABEL: role, + } + return sidecar + + +@pytest.mark.asyncio +async def test_read_status_ready_when_sidecar_running( + docker_backend: DockerDeploymentBackend, + mock_entities: AsyncMock, + mock_docker_client: MagicMock, +) -> None: + """Server ready + adapters sidecar running => READY.""" + mock_docker_client.containers.get.return_value = _running_server_container() + mock_entities.get.return_value = lora_config() # server + lora-adapters sidecar + mock_docker_client.containers.list.return_value = [_sidecar_container("lora-adapters", "running")] + + update = await docker_backend.read_status(workspace="default", name="srv") + + assert update.status == "READY" + + +@pytest.mark.asyncio +async def test_read_status_starting_when_sidecar_stopped( + docker_backend: DockerDeploymentBackend, + mock_entities: AsyncMock, + mock_docker_client: MagicMock, +) -> None: + """Server ready but the adapters sidecar is stopped => drop back to STARTING. + + Locks in the readiness gating: a present-but-not-running sidecar keeps the + deployment out of READY. + """ + mock_docker_client.containers.get.return_value = _running_server_container() + mock_entities.get.return_value = lora_config() + mock_docker_client.containers.list.return_value = [_sidecar_container("lora-adapters", "exited")] + + update = await docker_backend.read_status(workspace="default", name="srv") + + assert update.status == "STARTING" + assert "sidecar" in update.status_message.lower() + + +@pytest.mark.asyncio +async def test_read_status_starting_when_sidecar_removed( + docker_backend: DockerDeploymentBackend, + mock_entities: AsyncMock, + mock_docker_client: MagicMock, +) -> None: + """Server ready but the expected adapters sidecar is entirely gone => STARTING. + + A removed (not merely exited) sidecar is detected by comparing the expected + sidecar roles from the config against the containers actually present. + """ + mock_docker_client.containers.get.return_value = _running_server_container() + mock_entities.get.return_value = lora_config() + # The sidecar container has been removed: only unrelated containers remain. + mock_docker_client.containers.list.return_value = [] + + update = await docker_backend.read_status(workspace="default", name="srv") + + assert update.status == "STARTING" + assert "sidecar" in update.status_message.lower() + + @pytest.mark.asyncio async def test_read_status_lost_when_missing_always( docker_backend: DockerDeploymentBackend, diff --git a/plugins/nemo-deployments/tests/unit/backends/docker/test_containers.py b/plugins/nemo-deployments/tests/unit/backends/docker/test_containers.py new file mode 100644 index 0000000000..2e2f2e9c7e --- /dev/null +++ b/plugins/nemo-deployments/tests/unit/backends/docker/test_containers.py @@ -0,0 +1,55 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for the docker DeploymentConfig -> orchestration plan builder.""" + +from __future__ import annotations + +import pytest +from backends.docker.docker_helpers import lora_config, sample_config +from nemo_deployments_plugin.backends.docker.containers import ( + DeploymentConfigError, + build_docker_plan, + validate_config_for_docker, +) +from nemo_deployments_plugin.entities import Container, ContainerPort, DeploymentConfig + + +def test_single_container_plan_has_no_init_or_sidecars() -> None: + plan = build_docker_plan(sample_config()) + assert plan.primary.name == "main" + assert plan.init_containers == [] + assert plan.sidecars == [] + assert plan.is_multi_container is False + + +def test_lora_plan_splits_init_primary_and_sidecar() -> None: + plan = build_docker_plan(lora_config()) + assert plan.primary.name == "server" + assert [c.name for c in plan.init_containers] == ["lora-cache-init"] + assert [c.name for c in plan.sidecars] == ["lora-adapters"] + assert plan.is_multi_container is True + + +def test_plan_rejects_sidecar_with_ports() -> None: + config = DeploymentConfig( + name="cfg1", + workspace="default", + containers=[ + Container(name="server", image="img", ports=[ContainerPort(name="http", containerPort=8000)]), + Container(name="sidecar", image="img2", ports=[ContainerPort(name="x", containerPort=9000)]), + ], + ) + with pytest.raises(DeploymentConfigError, match="may not declare ports"): + build_docker_plan(config) + + +def test_plan_rejects_empty_config() -> None: + config = DeploymentConfig(name="cfg1", workspace="default", containers=[]) + with pytest.raises(DeploymentConfigError, match="at least one container"): + build_docker_plan(config) + + +def test_validate_config_returns_primary_for_back_compat() -> None: + assert validate_config_for_docker(sample_config()).name == "main" + assert validate_config_for_docker(lora_config()).name == "server" diff --git a/services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py b/services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py index 2e83db03b1..8b8a33cec6 100644 --- a/services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py +++ b/services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py @@ -25,7 +25,6 @@ apply_deleting_timeout, apply_pending_timeout, ) -from nmp.core.models.controllers.backends.engine import ENGINE_GENERIC, config_engine from nmp.core.models.controllers.context import ModelContext logger = logging.getLogger(__name__) @@ -72,18 +71,6 @@ async def create_model_deployment(self, ctx: ModelContext) -> DeploymentStatusUp return DeploymentStatusUpdate( status="UNKNOWN", status_message="Deployments plugin is unavailable for runtime none." ) - lora_enabled = resolved.view.lora_enabled and config_engine(resolved.config) != ENGINE_GENERIC - if resolved.runtime == Runtime.DOCKER and lora_enabled: - # Fail fast: the plugin docker runtime is single-container today, so a - # LoRA deployment (server + adapters sidecar) cannot run there yet. - return DeploymentStatusUpdate( - status="ERROR", - status_message=( - "LoRA serving is not supported on the docker runtime yet " - "(deployments-plugin docker is single-container). Deploy LoRA " - "models on the kubernetes runtime instead." - ), - ) teardown = await self.delete_model_deployment(resolved.deployment.workspace, resolved.deployment.name) if teardown.status == "DELETING": return DeploymentStatusUpdate( diff --git a/services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py b/services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py index 408e7e5692..cb45f7fad2 100644 --- a/services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py +++ b/services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py @@ -134,7 +134,7 @@ def _resolved_docker_lora() -> ResolvedPluginDeployment: deployment=SimpleNamespace(name="my-dep", workspace="default"), config=SimpleNamespace(engine="vllm"), model_entity=None, - view=DeploymentConfigView(model_namespace="org", model_name="model", lora_enabled=True), + view=DeploymentConfigView(model_namespace="org", model_name="model", lora_enabled=True, gpu=1), weights_type=ModelWeightsType.FILES_SERVICE, model_namespace="org", model_name="model", @@ -146,23 +146,63 @@ def _resolved_docker_lora() -> ResolvedPluginDeployment: @pytest.mark.asyncio -async def test_docker_lora_fails_fast_before_touching_substrate() -> None: +async def test_docker_lora_creates_substrate() -> None: + """Docker + LoRA is now supported: it creates substrate like any other deploy. + + The docker backend runs the LoRA shape as a multi-container group (server + + adapters sidecar) so there is no longer a fast-fail guardrail. + """ backend = DeploymentsPluginServiceBackend(AsyncMock(), {}, "puller:latest") backend.init() backend._entities = AsyncMock() + created: list[object] = [] + + async def _create(entity: object) -> object: + created.append(entity) + return entity + + backend._entities.create = AsyncMock(side_effect=_create) + backend._entities.get = AsyncMock(side_effect=NemoEntityNotFoundError("missing")) + backend._entities.delete = AsyncMock(side_effect=NemoEntityNotFoundError("missing")) with ( patch( "nmp.core.models.controllers.backends.deployments_plugin.backend.resolve_plugin_deployment", return_value=_resolved_docker_lora(), ), - patch.object(backend, "delete_model_deployment", AsyncMock()) as delete_mock, + patch( + "nmp.core.models.controllers.backends.deployments_plugin.backend.executor_for_runtime", + return_value="local-docker", + ), ): result = await backend.create_model_deployment(_ctx()) - assert result.status == "ERROR" - assert "docker" in result.status_message.lower() - assert "lora" in result.status_message.lower() - delete_mock.assert_not_called() - backend._entities.create.assert_not_called() + + assert result.status == "PENDING" + # Substrate is created (volume(s) + puller + server configs/deployments), + # not rejected. The server config carries the multi-container LoRA shape. + assert any(isinstance(item, Deployment) and item.name == "my-dep-server" for item in created) + + # Assert the full multi-container LoRA contract on the server DeploymentConfig, + # so a regression that drops the adapters sidecar / init / its port/GPU + # settings fails here rather than silently passing. + server_config = next( + item for item in created if isinstance(item, DeploymentConfig) and item.name.endswith("-server") + ) + container_names = [c.name for c in server_config.containers] + assert container_names == ["server", "lora-adapters"] + + server_container = server_config.containers[0] + sidecar_container = server_config.containers[1] + + # The adapters sidecar publishes no ports and requests no GPU (it shares the + # server's network namespace and GPU); only the server owns those. + assert not sidecar_container.ports + assert not sidecar_container.resources.limits.get("nvidia.com/gpu") + assert server_container.ports # server exposes the inference port + assert server_container.resources.limits.get("nvidia.com/gpu") == "1" + + # The lora-cache-init init container prepares the shared scratch volume. + init_names = [c.name for c in server_config.init_containers] + assert "lora-cache-init" in init_names @pytest.mark.asyncio diff --git a/services/core/models/tests/unit/controllers/test_deployment_reconciler.py b/services/core/models/tests/unit/controllers/test_deployment_reconciler.py index 9d52834e29..ee97aaf983 100644 --- a/services/core/models/tests/unit/controllers/test_deployment_reconciler.py +++ b/services/core/models/tests/unit/controllers/test_deployment_reconciler.py @@ -233,18 +233,14 @@ async def test_reconcile_individual_deployment_error_fallback_conflict_is_noop( @pytest.mark.asyncio -async def test_reconcile_created_docker_lora_error_persisted(reconciler, mock_backend_registry, make_deployment): - """CREATED + docker LoRA backend ERROR is persisted with a clear single-container message.""" +async def test_reconcile_created_backend_error_persisted(reconciler, mock_backend_registry, make_deployment): + """CREATED + backend ERROR is persisted verbatim to the deployment status.""" deployment = make_deployment(status="CREATED") mock_backend = MagicMock() mock_backend.create_model_deployment = AsyncMock( return_value=DeploymentStatusUpdate( status="ERROR", - status_message=( - "LoRA serving is not supported on the docker runtime yet " - "(deployments-plugin docker is single-container). Deploy LoRA " - "models on the kubernetes runtime instead." - ), + status_message="Backend create failed for some reason", ) ) mock_backend_registry.get_backend.return_value = mock_backend @@ -254,7 +250,7 @@ async def test_reconcile_created_docker_lora_error_persisted(reconciler, mock_ba call_kwargs = reconciler._models_sdk.inference.deployments.update_status.call_args.kwargs assert call_kwargs["status"] == "ERROR" - assert "single-container" in call_kwargs["status_message"] + assert call_kwargs["status_message"] == "Backend create failed for some reason" @pytest.mark.asyncio