fix: update com.docker.compose.image label when recreating container#2183
Open
GiulioSavini wants to merge 1 commit intogetarcaneapp:mainfrom
Open
fix: update com.docker.compose.image label when recreating container#2183GiulioSavini wants to merge 1 commit intogetarcaneapp:mainfrom
GiulioSavini wants to merge 1 commit intogetarcaneapp:mainfrom
Conversation
When Arcane updates a container via the standalone path (not through docker compose up), it copies all labels from the old container verbatim. The com.docker.compose.image label retains the old image ID, causing docker compose up -d to detect a mismatch and needlessly recreate the container. After setting the new image ref, inspect the pulled image and update the compose label to match the new image ID. Fixes getarcaneapp#1935
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
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.
Checklist
mainbranchWhat This PR Fixes
When Arcane updates a container via the standalone path (i.e. when the compose project is not registered in the DB), it copies all labels from the old container verbatim — including
com.docker.compose.image, which stores the old image ID.After the update, running
docker compose up -dsees thatcom.docker.compose.imagedoesn't match the current image ID and recreates the container, breaking compatibility withdocker compose.Fixes #1935
Root Cause
In
updateContainer(), the container config (including all labels) is copied from the old container and onlycfg.Imageis updated to the new ref. Thecom.docker.compose.imagelabel is left pointing to the old image ID.Docker Compose uses this label to determine whether a container is up-to-date. A mismatch triggers an unnecessary recreate.
Changes Made
updater_service.go— After settingcfg.Image = newRef, inspect the newly pulled image and updatecom.docker.compose.imageto the new image ID (only when the label already exists on the container). Falls back gracefully with a warning log if the image inspect fails.Why This Is Safe
UpdateProjectServices→docker compose up) is not affectedImageInspectfails for any reason, the old behavior is preserved (label unchanged) with a warning logTesting Done
go vet ./internal/services/— no issuesgo test ./internal/services/ -run TestUpdater— all tests passAI Tool Used (if applicable)
AI Tool: N/A
Assistance Level: N/A
Disclaimer Greptiles Reviews use AI, make sure to check over its work.
To better help train Greptile on our codebase, if the comment is useful and valid Like the comment, if its not helpful or invalid Dislike
To have Greptile Re-Review the changes, mention
greptileai.Greptile Summary
This PR fixes a label-staleness bug in the standalone (non-compose-DB) container update path: after Arcane recreates a container with a new image, the
com.docker.compose.imagelabel was left pointing to the old image ID, causingdocker compose up -dto trigger a redundant recreate. The fix inspects the newly-pulled image immediately after settingcfg.Image = newRefand updates the label to the fresh image ID — only when the label already exists on the container, and with a graceful fallback if the inspect fails.Key changes
updater_service.go: After updatingcfg.Image, callsdcli.ImageInspectto resolve the new image ID and writes it back tocfg.Labels[\"com.docker.compose.image\"]. The update is guarded by a nil-check oncfg.Labelsand an existence-check for the label key, so containers that were never compose-managed are unaffected.updateContainer); the compose-aware path (UpdateProjectServices) is unchanged.Confidence Score: 5/5
Safe to merge — the fix is narrowly scoped, well-guarded, and does not affect the compose-aware update path.
The only finding is a pre-existing P2 naming-convention issue (
updateContainershould beupdateContainerInternal) that this PR touches but did not introduce. The logic itself is correct: it guards against a nil Labels map, only updates the key when it already exists, and falls back gracefully on inspect failure. No new security, correctness, or reliability concerns were introduced.No files require special attention.
Important Files Changed
com.docker.compose.imagelabel insideupdateContainerso that after a standalone image refresh the label reflects the new image ID, preventing Docker Compose from treating the container as stale. Logic is well-guarded (nil-check, existence-check, graceful fallback). Pre-existing naming-convention violation: function should beupdateContainerInternal.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[updateContainer called] --> B[Stop old container] B --> C[Remove old container] C --> D["cfg := inspect.Config\ncfg.Image = newRef"] D --> E{cfg.Labels != nil?} E -- No --> H E -- Yes --> F{"com.docker.compose.image\nexists in labels?"} F -- No --> H F -- Yes --> G["dcli.ImageInspect(ctx, newRef)"] G -- success --> I["cfg.Labels[com.docker.compose.image] = imgInspect.ID"] G -- error --> J["slog.WarnContext: could not inspect new image"] I --> H J --> H[PrepareRecreateHostConfigForEngine] H --> K[ContainerCreate with updated cfg] K --> L[Start new container]Comments Outside Diff (1)
backend/internal/services/updater_service.go, line 748 (link)updateContainermissing "Internal" suffixPer the project's naming convention, all unexported functions must carry an
Internalsuffix.updateContainer(and its two call sites at lines 640 and 1535) should be renamed toupdateContainerInternalfor consistency with the rest of the file (e.g.pruneImageIDsWithInUseSetInternal,collectUsedImagesFromContainersInternal,buildRunningImageIDSetInternal).Rule Used: What: All unexported functions must have the "Inte... (source)
Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix: update com.docker.compose.image lab..." | Re-trigger Greptile
Context used: