fix: resolve stale predecessor misidentification during self-update by nicholas-fedor · Pull Request #1713 · nicholas-fedor/watchtower

nicholas-fedor · 2026-06-02T18:59:17Z

This PR addresses issues that intermittently materialize during self-updates.

Problem

When Watchtower updates itself, the old instance is renamed to watchtower-old-<id> and continues running with the expectation of the new container performing the cleanup of excess Watchtower containers. Multiple code paths failed to distinguish this stale predecessor from the running successor, causing intermittent failures. This manifests at several points:

Hostname self-detection matches both predecessor and successor, returning whichever the daemon lists first. The predecessor being selected causes Watchtower to lose track of itself on the next run.
Excess instance cleanup uses the detected container's ID as the exclusion target. When the predecessor is selected, the running successor's ID doesn't match and is removed as excess.
Chained container resolution reads the container-chain label from the predecessor, which lacks the updated chain, so parent containers are not cleaned up.
The rename step attempts to rename a container that already has the watchtower-old- prefix, causing a same-name error.

Solution

Added IsOldNamedContainer(name) to detect the watchtower-old- prefix convention, then resolved to the successor at each affected decision point:

Hostname fallback prefers the Watchtower container without the predecessor prefix.
Excess cleanup and chained container resolution find the non-predecessor successor when the detected container has the prefix; chained resolution falls back to empty when no successor exists.
Rename step skips containers that already carry the predecessor name.

Changes

pkg/container/container_id.go — added IsOldNamedContainer; hostname fallback prefers non-predecessor matches
internal/actions/cleanup.go — getFilteredContainers and getChainedContainers resolve successors when current is a predecessor; getChainedContainers returns empty when no successor found
internal/actions/update.go — restartStaleContainer skips redundant rename
Tests for hostname preference, successor selection (scoped and unscoped), and rename skip

Summary by CodeRabbit

Bug Fixes
- Improved cleanup to correctly identify and remove predecessor instances when names were rotated.
- Selection logic now prefers current (non-old-named) instances when multiple versions share a scope or hostname.
- Avoids redundant rename operations by detecting already-renamed instances and skipping renames.
- Added tests covering rename-skip and multi-instance selection edge cases.

- Add IsOldNamedContainer helper to identify predecessor containers by name prefix - Fix hostname-based detection to prefer non-old-named Watchtower over lingering predecessor - Resolve old-named current container to same-scope successor in excess cleanup and chain logic - Skip redundant rename when source container already has the target old name

- Fix `getChainedContainers` to clear `effectiveCurrent` when no non-renamed successor container is found, preventing stale references from being used - Set `renamed = true` in `restartStaleContainer` when skipping rename of an already-renamed container, ensuring correct tracking of rename state - Update test expectation to reflect that the container is correctly identified as renamed

coderabbitai · 2026-06-02T18:59:28Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3ded1d2a-ac11-494f-b650-108d8d5f685e

📥 Commits

Reviewing files that changed from the base of the PR and between 302d366 and 2d129cd.

📒 Files selected for processing (3)

internal/actions/cleanup.go
internal/actions/update.go
pkg/container/container_id.go

🚧 Files skipped from review as they are similar to previous changes (2)

internal/actions/update.go
internal/actions/cleanup.go

📝 Walkthrough

Walkthrough

Detects Watchtower "old-named" containers (prefix watchtower-old-), prefers non-old-named successors during hostname-based selection, skips redundant rename when restarting an already old-named instance, and resolves an effective current container in cleanup flows to select proper chain successors by scope and lineage.

Changes

Old-Named Container Handling

Layer / File(s)	Summary
Container old-name detection and hostname selection `pkg/container/container_id.go`, `pkg/container/container_id_test.go`	Adds `IsOldNamedContainer()` to detect the `watchtower-old-` prefix and updates `GetContainerIDFromHostname()` to prefer non-old-named Watchtower candidates when multiple containers share a hostname.
Conditional rename in restartStaleContainer `internal/actions/update.go`, `internal/actions/actions_internal_test.go`	`restartStaleContainer()` skips Docker rename if the source is already old-named (sets `renamed=true`) and otherwise calls `client.RenameContainer`; adds a test asserting no rename call when already old-named.
Cleanup flows with effective current container resolution `internal/actions/cleanup.go`, `internal/actions/cleanup_test.go`	`getFilteredContainers()` and `getChainedContainers()` derive an effective current ID when the provided current is old-named by finding a non-old-named successor in the same (normalized) scope, preferring chain-linked candidates; chained-container filtering excludes `effectiveCurrent.ID()`; tests validate predecessor removal and scope-aware successor selection.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

nicholas-fedor/watchtower#1491: Overlaps restartStaleContainer rename control flow changes (EphemeralSelfUpdate branch).
nicholas-fedor/watchtower#1480: Related changes to GetContainerIDFromHostname() selection logic when multiple Watchtower candidates exist.
nicholas-fedor/watchtower#1657: Related modifications to restartStaleContainer() gating and restart behavior.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main fix: resolving misidentification of stale predecessors during Watchtower self-updates, which aligns with the core changes across all modified files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codacy-production · 2026-06-02T19:03:12Z

Not up to standards ⛔

🔴 Issues 1 medium

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category Results

Complexity 1 medium

View in Codacy

🟢 Metrics 35 complexity · 39 duplication

Metric Results

Complexity 35

Duplication 39

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

codecov · 2026-06-02T19:04:20Z

Codecov Report

❌ Patch coverage is 83.63636% with 18 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/actions/cleanup.go	77.77%	8 Missing and 6 partials ⚠️
pkg/container/container_id.go	71.42%	2 Missing and 2 partials ⚠️

@@            Coverage Diff             @@
##             main    #1713      +/-   ##
==========================================
- Coverage   74.59%   74.41%   -0.18%     
==========================================
  Files          60       61       +1     
  Lines       10052    10199     +147     
==========================================
+ Hits         7498     7590      +92     
- Misses       2286     2330      +44     
- Partials      268      279      +11

Files with missing lines	Coverage Δ
internal/actions/update.go	`83.78% <100.00%> (+0.11%)`	⬆️
pkg/container/container_id.go	`97.26% <71.42%> (-2.74%)`	⬇️
internal/actions/cleanup.go	`94.15% <77.77%> (-4.24%)`	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/actions/update.go (1)
1676-1719: ⚡ Quick win

Centralize the old-name prefix contract.

"watchtower-old-" is now a cross-file protocol between rename generation here and detection in pkg/container/container_id.go. Keeping it duplicated makes future drift easy and would silently break successor detection again. Please move the prefix into a shared constant/helper and build targetOldName from that single source of truth.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/actions/update.go` around lines 1676 - 1719, The hard-coded prefix
"watchtower-old-" must be centralized: introduce a shared constant (e.g.,
OldWatchtowerOldPrefix or WatchtowerOldPrefix) in the common package used by
container ID logic, update the detection function IsOldNamedContainer to
reference that constant, and replace the inline string here when building
targetOldName (symbol targetOldName) so targetOldName is constructed from the
new constant before calling client.RenameContainer; ensure both rename
generation and IsOldNamedContainer use the single source-of-truth
constant/helper so the cross-file protocol cannot drift.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/actions/cleanup.go`:
- Around line 188-199: When currentContainer is old-named, do not pick the first
non-old-named container; instead resolve the true successor by matching lineage:
iterate filteredContainers and prefer a container whose chain/lineage label
(e.g., the Watchtower chain label accessed via c.Labels()[<chainLabelKey>] or
similar) contains currentContainer.ID() (or exact predecessor identifier) and
use that container's ID as currentID; only if no explicit lineage match is found
fall back to the existing scope-only selection. Apply this same lineage-first
logic to the other similar block handling (the section around the 234-268
range).

---

Nitpick comments:
In `@internal/actions/update.go`:
- Around line 1676-1719: The hard-coded prefix "watchtower-old-" must be
centralized: introduce a shared constant (e.g., OldWatchtowerOldPrefix or
WatchtowerOldPrefix) in the common package used by container ID logic, update
the detection function IsOldNamedContainer to reference that constant, and
replace the inline string here when building targetOldName (symbol
targetOldName) so targetOldName is constructed from the new constant before
calling client.RenameContainer; ensure both rename generation and
IsOldNamedContainer use the single source-of-truth constant/helper so the
cross-file protocol cannot drift.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 24c1d184-6a8f-4b52-8f76-5edc130a6121

📥 Commits

Reviewing files that changed from the base of the PR and between e3ac1e7 and 302d366.

📒 Files selected for processing (6)

internal/actions/actions_internal_test.go
internal/actions/cleanup.go
internal/actions/cleanup_test.go
internal/actions/update.go
pkg/container/container_id.go
pkg/container/container_id_test.go

- Replace naive non-old-named fallback in `getFilteredContainers` with chain-label matching: prefer a successor whose container chain contains the old container's ID, falling back to scope-only match - Update `getChainedContainers` to use explicit lineage link via chain label before accepting scope-only match, and reset `effectiveCurrent` when no valid successor exists - Extract `WatchtowerOldPrefix` constant in `pkg/container/container_id.go` and use it in `restartStaleContainer` to eliminate hardcoded string literal

nicholas-fedor added 2 commits June 2, 2026 09:37

This was linked to issues Jun 2, 2026

[Bug]: container updates not complete when watchtower itself updates #1527

Closed

[Bug]: Watchtower got lost during scheduled start after socket-proxy container was updated #1318

Closed

coderabbitai Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread internal/actions/cleanup.go

nicholas-fedor merged commit 7f447b7 into main Jun 2, 2026
19 of 20 checks passed

nicholas-fedor deleted the fix/cleanup-old-named-successor-selection branch June 2, 2026 21:24

coderabbitai Bot mentioned this pull request Jun 7, 2026

fix: prevent orphaned old Watchtower containers from persisting #1743

Merged

coderabbitai Bot mentioned this pull request Jul 22, 2026

fix(actions): harden self-update failure and cancel recovery #2005

Merged

Navigation Menu

Uh oh!

fix: resolve stale predecessor misidentification during self-update - #1713

fix: resolve stale predecessor misidentification during self-update#1713
nicholas-fedor merged 3 commits into
mainfrom
fix/cleanup-old-named-successor-selection

nicholas-fedor commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

codacy-production Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

nicholas-fedor commented Jun 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Changes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

codacy-production Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Not up to standards ⛔

Uh oh!

codecov Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

nicholas-fedor commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading

codacy-production Bot commented Jun 2, 2026 •

edited

Loading

codecov Bot commented Jun 2, 2026 •

edited

Loading