fix(local): pull the latest default local-mode image on volcano start#82
Conversation
There was a problem hiding this comment.
Pull request overview
Updates local-mode startup to refresh the rolling default server image while preserving custom-image and offline behavior.
Changes:
- Pulls the default server image before Compose startup.
- Skips pulls for custom images.
- Adds startup and restart assertions for pull behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/localmode/compose.go |
Adds best-effort default-image refresh. |
internal/localmode/service.go |
Passes output writer into startup. |
internal/localmode/info.go |
Defines the server Compose service name. |
internal/localmode/start_test.go |
Adds startup pull assertions. |
internal/localmode/restart_test.go |
Accommodates pulls during restart. |
internal/localmode/health_test.go |
Accommodates pulls in health-timeout testing. |
Comments suppressed due to low confidence (1)
internal/localmode/compose.go:145
- The best-effort failure path is a key part of this change, but all new pull stubs return success. Add a test where the pull returns an error and verify that the warning is emitted and Compose
upstill runs, preserving offline startup with a cached image.
if _, err := s.runner.Run(ctx, Command{Name: dockerCommand, Args: args, Env: env}); err != nil {
output.Warning(w, "could not pull latest local-mode image (using cached copy): %v", err)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tkkhq
left a comment
There was a problem hiding this comment.
Code Review — PR #82: fix(local): pull the latest default local-mode image on volcano start
Verification: No baseline run: pr_review_verify reports verification disabled (no user-level verificationBaselines configured), so no PR code was executed (unsandboxed-PR-code risk therefore not incurred). I validated the change from source instead of the shifting/detached working tree by reading the changed files directly from head commit f1fb413 via git show: confirmed io import in compose.go, the new startDockerServices(ctx, w io.Writer, env) signature and its sole caller in Start, serverComposeService = "server" matching the server: service in docker-compose.template.yml, slices imported in all three edited test files, and the output.Warning signature. A read-only go build/go vet I ran happened to hit base-branch content on disk (tree was detached at base), so it does not attest the PR head; head validation was done via head-commit reads and the four heavy lenses.
Overview
This PR makes volcano start proactively refresh the rolling default local-mode image. Before docker compose up, startDockerServices now calls a new refreshDefaultServerImage that runs docker compose pull server for the default tag (kong/volcano:local-nightly), so the stack runs the latest published build instead of a stale cached copy. This is what unblocks credential-free local mode, since the server-side defaults only exist in a freshly pulled image.
The pull is gated: refreshDefaultServerImage re-derives resolveImage() and returns early (no pull) whenever a custom image is selected via --image / VOLCANO_IMAGE / .env.local, matching the CLI's existing "custom images are never pulled, must exist locally" contract. It is best-effort — a pull failure (e.g. offline) is downgraded to a warning and up --force-recreate falls back to the cached image. Plumbing adds a w io.Writer parameter to startDockerServices and a serverComposeService = "server" constant.
Tests in health/restart/start add a fake-runner case for the new docker ... pull command, and start_test.go asserts the default-image start issues a pull while a missing custom-image start does not.
Strengths
- Correct gating:
refreshDefaultServerImageearly-returns for custom images via the sameresolveImage()precedence used elsewhere inStart, so custom/unpublished tags are never pulled. - Graceful offline fallback: pull errors are non-fatal warnings and
upproceeds with the cached image, preserving offline starts. - Correct pull point and env: the pull runs after the custom-image pre-flight and before
up, with the samecomposeEnv(carryingVOLCANO_IMAGE) so the pulled tag matches the started tag;--force-recreateensures the refreshed image is actually used. - Solid, matching test coverage: default-image start asserts
docker pullis called and custom-image-missing start asserts it is not, consistent with the gating logic;serverComposeServicematches theserver:compose service sopull servertargets the right service.
Findings — 0 total (0 inline, 0 summary-only)
No issues found.
Correctness / Security / Performance
- Correctness: Pull runs at the correct point (after custom-image pre-flight, before up); already-running early-return and custom-image paths correctly skip it; no new goroutines/races; ctx cancellation still surfaces via up.
- Security: No injection risk — image name is not in argv (only static
server), execution is shell-freeexec.CommandContext; no new secret sink; credential-free pull stays within the pre-existing trusted default-tag boundary. - Performance: One extra best-effort
docker compose pull serverper default-image start (manifest check when digest unchanged), not looped; temp-file cleanup unchanged; rolling-tag dangling images are standard Docker behavior, low-impact for local dev.
Verdict
Suggested verdict: approve
The change is small, correctly gated to the default image, best-effort on failure, and covered by matching tests; all four heavy lenses and source-grounded validation found no substantiated defects and no blocking issues.
Confidence: 0.90
Tracking
Summary
volcano startnow pulls the rolling default local-mode image (kong/volcano:local-nightly) before bringing the stack up, so it always runs the latest published build instead of a stale cached copy. This is what unblocks credential-free local mode (v0.2.2 / fix(local): send no credential in local mode so all commands work the same way #78): the CLI sends no credential, which requires the server-side defaults (VOL-474 #656, VOL-476 #657) that only exist in a freshly pulled image — Docker Compose otherwise reuses a cachedlocal-nightlyand returns401: authorization header requiredinternal/localmode/compose.go:startDockerServicescalls a newrefreshDefaultServerImagethat runsdocker compose pull serverfor the default image. Best-effort — a pull failure (e.g. offline) is a warning andupfalls back to the cached image, so offline starts still work--image/VOLCANO_IMAGE/.env.localpointing at a non-default tag): the CLI never pulls those and they must exist locally, unchangedlocal-latest/versioned local tag published (the release-image job hasn't run), solocal-nightlyis the only and latest local-mode image; the fix is refreshing that rolling tag, not switching tagsVerification
make lintmake openapi-generated-checkmake teststartissuesdocker pull; custom image does not