Track image-content directories in build-sim-image trigger#34
Merged
Conversation
scripts/sim.Dockerfile does ``COPY . /workspace``, so the image's /workspace tree IS a snapshot of the repo at build time. The previous trigger only fired on build-pipeline files (install-sim.sh, the Dockerfile, .dockerignore, the astra-sim submodule pin, the workflow itself), which left ``:latest`` stale whenever real simulator changes landed: a fix to serving/__main__.py or a new cluster config in configs/cluster/ would not rebuild the image, so pulled containers ran old code while only the registry tag advanced via main branch moves. Add the four top-level directories baked into /workspace (serving/, configs/, workloads/, profiler/) to the path filter. They mirror the docker-vllm.sh mount surface, so any change a contributor would expect to see inside the sim container now triggers a rebuild. Cost: a few extra CI runs per week on serving/ + configs/ edits. Worth it to keep the image honest.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes the sim image trigger fire on every change to repo content that ends up baked into
/workspace, not just on build-pipeline changes.Why
scripts/sim.DockerfiledoesCOPY . /workspace, so the image's/workspacetree IS a snapshot of the repo at build time. The previous trigger only fired on the build pipeline:So a fix to
serving/__main__.pyor a new cluster config inconfigs/cluster/would not rebuild the image — pulled containers ran old code while:latestonly advanced on the merge commit's path-filter match. PR #33 (single-mode dispatch inserving/spec_compression_stress.sh) is a concrete example: the new behaviour would never reach users viadocker pulluntil something unrelated pushed an image rebuild.Change
Add the four top-level directories that are part of the image's
/workspacesnapshot:These mirror the mount surface that
scripts/docker-vllm.shexposes today, so anything a contributor would expect to see inside the container now triggers a rebuild.Cost
A few extra CI runs per week when
serving/orconfigs/change. Per-platform native runners + GHA buildx cache keep each one to ~2-3 min on cache hit, so the budget impact is small. Worth it to keep:latesthonest.Test plan
serving/__main__.pytriggers a build run.docs/(already in.dockerignore) does not trigger a build run.:latestincludes the updatedserving/spec_compression_stress.shfrom PR Add single-mode dispatch to spec_compression_stress.sh #33 (verifiable withdocker run --rm ghcr.io/psal-postech/llmservingsimspec/sim:latest bash /workspace/serving/spec_compression_stress.sh nonsense— the new error message proves the new script is in there).Generated by Claude Code