Skip to content

fix(vllm): validate persistent container configuration - #327

Open
nvddr wants to merge 2 commits into
mainfrom
agent/vllm-config-fingerprint
Open

fix(vllm): validate persistent container configuration#327
nvddr wants to merge 2 commits into
mainfrom
agent/vllm-config-fingerprint

Conversation

@nvddr

@nvddr nvddr commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • label repository-owned vLLM containers with a deterministic launch fingerprint
  • reuse healthy or stopped containers only when their launch contract matches
  • recreate stale or unlabeled containers before model startup

Why

A persistent container could be restarted with model arguments, GPU assignment, or memory limits from an older profile even after the active YAML changed.

Validation

  • uv run --project tests pytest -q tests/test_vllm_docker.py
  • ruff check on changed Python files
  • SPDX header check

Signed-off-by: Devdeep Ray <devdeepr@Nvidia.com>
@nvddr
nvddr force-pushed the agent/vllm-config-fingerprint branch from e34df37 to d3c198e Compare July 31, 2026 20:54
@nvddr

nvddr commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Review outcome: ready for review

The branch is rebased on current main, and the earlier blockers are resolved:

  • Healthy ports are reusable only when the expected named Docker container is running and carries the matching launch fingerprint; an unowned listener now fails without being stopped.
  • The fingerprint includes a versioned launcher contract, so changes to launcher-controlled Docker behavior (entrypoint, bootstrap, fixed flags) invalidate older immutable containers.
  • Matching stopped containers restart, matching containers still starting are monitored instead of receiving a conflicting second docker run, and stale containers are recreated.

Validation:

  • Focused isolated suite: 42 passed (tests/test_vllm_docker.py)
  • Ruff: passed
  • Fresh GitHub CI: all checks passed (Python 3.11/3.12, docs build/publish, Ruff, lock, SPDX, DCO, and CodeQL)

No unresolved review threads remain. The PR is ready for maintainer review and merge.

@nvddr
nvddr marked this pull request as ready for review August 10, 2026 21:48
@nvddr
nvddr requested review from blongs-nv, wenxind-nvidia and yanziz-nvidia and removed request for wenxind-nvidia August 10, 2026 21:48
container_name,
)
sys.exit(1)
existing = False
)
sys.exit(1)
existing = False
running = False
container_name,
)
sys.exit(1)
existing = False
)
sys.exit(1)
existing = False
running = False
@nvddr
nvddr deployed to github-pages August 10, 2026 21:56 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 10, 2026
@wenxind-nvidia

Copy link
Copy Markdown
Collaborator

Follow-up review

Blockers

None. The update at 724efff resolves the previous blockers:

  • Healthy listeners now require the expected running container and matching fingerprint.
  • The fingerprint includes a versioned launcher contract.
  • The branch is current with main, conflict-free, and mergeable.

Suggestion

[P2] Account for credential rotation in the container fingerprint

HF_TOKEN remains outside the fingerprint even though it is stored in the immutable container environment. A matching stopped container can therefore restart with an obsolete token after credential rotation. Consider hashing it into the fingerprint or documenting the intentional exclusion.

Nits

  • Remove the unused existing and running assignments at lines 640–641 and 662–663, already flagged by the code-quality bot.

Validation

  • tests/test_vllm_docker.py: 42 passed
  • Ruff, SPDX, and diff checks passed
  • GitHub CI: 14/14 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants