From 3a362f0afba30cf2527f27b3f87e2324559a920a Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 00:53:28 +0100 Subject: [PATCH 1/5] fix(updates): report installability-aware update checks Combine the protected schema-v18 sender with the exact-archive updater. Require installable archives in positive fixtures, classify missing archives and unavailable Pro checks without claiming an offer, and avoid caching an unactivated Pro result. Record the contract and exact combined-source browser receipt. Change-source: pulse-maintainer --- .../subsystems/deployment-installability.md | 9 +++ frontend-modern/browser-verification.json | 38 ++++------ ...issue2285_update_check_observation_test.go | 76 +++++++++++++++++-- internal/updates/manager.go | 22 +++++- internal/updates/pro_update.go | 2 + 5 files changed, 115 insertions(+), 32 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index f1a38739df..b9ec38638f 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -4065,6 +4065,15 @@ telemetry exports only that record at schema v18. An update discovery failure that records nothing is a regression, because a failed check never reaches the update history and is otherwise invisible in the fleet (#2285). Proof: `internal/updates/issue2285_update_check_observation_test.go`. +The observation describes installability, not merely version ordering. Positive +release fixtures must include the exact server archive; a newer release that +lacks that archive is not offered and records `metadata_error`, rather than +`up_to_date` or `available`. A compiled Pro install without usable broker +activation credentials returns its existing operator warning but records +`skipped`, never `up_to_date`; this unavailable result must not be cached across +a later activation. A stable-channel Pro check against a prerelease-only broker +pin records `no_release`. No version, URL, credential or warning text enters +the telemetry observation. Those same workflows must also fetch and dispatch the governed release branch derived from release-control metadata instead of hardcoding `pulse/v6`, `pulse/v6-release`, `main`, or any later branch literal inline; when a stable diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index 25bce8769b..42a4f46e5d 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,36 +1,28 @@ { "version": 1, - "base_sha": "b1e75fb9e4693c5706b2242760d6277e5291e7f9", - "verified_at": "2026-09-27T22:55:00Z", + "base_sha": "6d7e3342e56a4f8f2b411acb3df8dc7777d4ec7f", + "verified_at": "2026-09-27T23:52:13Z", "result": "passed", "changed_paths": ["frontend-modern/src/api/settings.ts"], "content_sha256": { "frontend-modern/src/api/settings.ts": "4172b94c268be6a0ea000e73d155e540f11776e1c59325250e13716722be5aeb" }, - "routes": ["/settings/system-general", "/docs/PRIVACY"], + "routes": ["/tmp/telemetry-schema-2285.html"], "viewports": [ - { - "width": 1024, - "height": 768 - }, - { - "width": 390, - "height": 844 - } + {"width": 1280, "height": 800}, + {"width": 390, "height": 844} ], "states": [ - "Local build of this branch signed in with a test account: General settings telemetry card rendered with the Preview payload control", - "Telemetry payload preview opened: schema_version 18 with update_channel stable, update_check_outcome skipped (source build, recorded by the background update checker) and update_available false, all three keys present", - "Preview at 390px: payload block visible and scrolls within its own box with no page-level horizontal overflow", - "Shipped privacy document: schema version 18 disclosure, Update channel, Update check outcome and Update available rows, and the dated schema 18 changelog row present", - "Privacy document at 390px: new rows render in the existing table, which scrolls within its container like the neighbouring rows, with no page-level horizontal overflow", - "Desktop and phone screenshots inspected for placement, clipping and overflow" + "Production GeneralSettingsPanel mounted in an isolated Vite fixture with a synthetic schema-v18 telemetry-preview API response", + "Preview payload displayed stable-channel skipped, metadata_error, and available outcomes with corresponding false, false, and true update_available booleans", + "The payload block stayed inside the card; the metadata_error JSON scrolled within the block at 390px without page-level horizontal overflow", + "Desktop and phone screenshots for all three outcomes were inspected for placement, clipping and scrolling" ], "interactions": [ - "Signed in through the login form", - "Navigated to /settings/system-general and clicked Preview payload", - "Parsed the rendered payload JSON and checked the schema 18 fields and closed values", - "Resized to 390x844 and rechecked the preview and page overflow", - "Navigated to /docs/PRIVACY and scrolled the Update check outcome row into view at desktop and 390px widths" - ] + "Clicked Preview payload and then Refresh payload twice per viewport, replacing the synthetic API outcome on each request", + "Parsed the rendered payload JSON and asserted schema_version, update_channel, update_check_outcome and update_available", + "Checked page and preview scroll widths at both viewports and checked for uncaught page errors" + ], + "command": "pulse-worker-browser tmp/browser-proof/telemetry-schema-2285.cjs (Playwright 1.56.1, Chromium 141.0.7390.37)", + "notes": "Offline production-component fixture with synthetic API data, not a full application login, installed sender, receiver or release acceptance. The browser result and six screenshots are retained under tmp/browser-proof in this assigned workspace." } diff --git a/internal/updates/issue2285_update_check_observation_test.go b/internal/updates/issue2285_update_check_observation_test.go index cc26e2bd4e..5adc15c945 100644 --- a/internal/updates/issue2285_update_check_observation_test.go +++ b/internal/updates/issue2285_update_check_observation_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "os" + "strings" "testing" "time" @@ -33,10 +34,22 @@ func issue2285Releases(t *testing.T, releases ...ReleaseInfo) string { return string(body) } +func issue2285InstallableRelease(t *testing.T, tag string, prerelease bool, published time.Time) ReleaseInfo { + t.Helper() + asset, ok := updateReleaseAssetForRuntime(tag) + if !ok { + t.Skip("no server release archive for this architecture") + } + return ReleaseInfo{ + TagName: tag, Prerelease: prerelease, PublishedAt: published, + Assets: []ReleaseAsset{asset}, + } +} + func TestIssue2285LastUpdateCheckRecordsEffectiveChannelOutcome(t *testing.T) { setRetrySettingsForTest(t, 1, time.Millisecond, time.Millisecond) - stable := ReleaseInfo{TagName: "v6.4.5", PublishedAt: time.Date(2026, 9, 30, 8, 0, 0, 0, time.UTC)} - preview := ReleaseInfo{TagName: "v6.4.6-rc.1", Prerelease: true, PublishedAt: time.Date(2026, 10, 2, 8, 0, 0, 0, time.UTC)} + stable := issue2285InstallableRelease(t, "v6.4.5", false, time.Date(2026, 9, 30, 8, 0, 0, 0, time.UTC)) + preview := issue2285InstallableRelease(t, "v6.4.6-rc.1", true, time.Date(2026, 10, 2, 8, 0, 0, 0, time.UTC)) for _, tc := range []struct { name string @@ -51,6 +64,7 @@ func TestIssue2285LastUpdateCheckRecordsEffectiveChannelOutcome(t *testing.T) { {name: "update offered", current: "6.4.1", channel: "stable", status: http.StatusOK, body: issue2285Releases(t, stable, preview), wantOutcome: UpdateCheckOutcomeAvailable, wantAvailable: true}, {name: "already current", current: "6.4.5", channel: "stable", status: http.StatusOK, body: issue2285Releases(t, stable, preview), wantOutcome: UpdateCheckOutcomeUpToDate}, {name: "preview channel offered newer prerelease", current: "6.4.5", channel: "rc", status: http.StatusOK, body: issue2285Releases(t, stable, preview), wantOutcome: UpdateCheckOutcomeAvailable, wantAvailable: true}, + {name: "newer release missing exact archive", current: "6.4.1", channel: "stable", status: http.StatusOK, body: issue2285Releases(t, ReleaseInfo{TagName: stable.TagName, Assets: []ReleaseAsset{{Name: "pulse-agent-v6.4.5-linux-amd64.tar.gz", BrowserDownloadURL: "https://example.invalid/agent"}}}), wantOutcome: UpdateCheckOutcomeMetadataError}, {name: "no stable release for channel", current: "6.4.5-rc.3", channel: "stable", status: http.StatusOK, body: issue2285Releases(t, preview), wantOutcome: UpdateCheckOutcomeNoRelease}, {name: "malformed metadata", current: "6.4.1", channel: "stable", status: http.StatusOK, body: `{"message":"not a list"}`, wantOutcome: UpdateCheckOutcomeMetadataError, wantErr: true}, {name: "server error", current: "6.4.1", channel: "stable", status: http.StatusBadGateway, body: `bad gateway`, wantOutcome: UpdateCheckOutcomeNetworkError, wantErr: true}, @@ -64,10 +78,13 @@ func TestIssue2285LastUpdateCheckRecordsEffectiveChannelOutcome(t *testing.T) { t.Fatalf("before any check LastUpdateCheck = %+v, want not_checked on %s", got, tc.channel) } - _, err := manager.CheckForUpdates(context.Background()) + info, err := manager.CheckForUpdates(context.Background()) if (err != nil) != tc.wantErr { t.Fatalf("CheckForUpdates error = %v, wantErr %v", err, tc.wantErr) } + if err == nil && (info.Available != tc.wantAvailable || (tc.wantAvailable && info.DownloadURL == "") || (tc.name == "newer release missing exact archive" && info.DownloadURL != "")) { + t.Fatalf("CheckForUpdates result = %+v, want available %v with matching archive only", info, tc.wantAvailable) + } got := manager.LastUpdateCheck() if got.Outcome != tc.wantOutcome || got.Available != tc.wantAvailable || got.Channel != tc.channel || got.CheckedAt.IsZero() { t.Fatalf("LastUpdateCheck = %+v, want outcome %s available %v on %s", got, tc.wantOutcome, tc.wantAvailable, tc.channel) @@ -80,8 +97,8 @@ func TestIssue2285PreviewOfOtherChannelDoesNotReplaceObservation(t *testing.T) { setRetrySettingsForTest(t, 1, time.Millisecond, time.Millisecond) withBuildVersion(t, "6.4.5") issue2285Server(t, http.StatusOK, issue2285Releases(t, - ReleaseInfo{TagName: "v6.4.5"}, - ReleaseInfo{TagName: "v6.4.6-rc.1", Prerelease: true}, + issue2285InstallableRelease(t, "v6.4.5", false, time.Time{}), + issue2285InstallableRelease(t, "v6.4.6-rc.1", true, time.Time{}), )) manager := NewManager(&config.Config{UpdateChannel: "stable"}) @@ -95,14 +112,61 @@ func TestIssue2285PreviewOfOtherChannelDoesNotReplaceObservation(t *testing.T) { // The settings UI can preview the preview channel before saving it. That // offer is not what this install is being offered, so it must not leak // into the observation telemetry reports. - if _, err := manager.CheckForUpdatesWithChannel(context.Background(), "rc"); err != nil { + previewInfo, err := manager.CheckForUpdatesWithChannel(context.Background(), "rc") + if err != nil { t.Fatalf("preview-channel check: %v", err) } + if !previewInfo.Available || previewInfo.DownloadURL == "" { + t.Fatalf("preview-channel check = %+v, want installable preview offer", previewInfo) + } if got := manager.LastUpdateCheck(); got.Outcome != UpdateCheckOutcomeUpToDate || got.Available || got.Channel != "stable" { t.Fatalf("after previewing rc LastUpdateCheck = %+v, want stable up_to_date unchanged", got) } } +func TestIssue2285UnactivatedProCheckIsNotUpToDateOrCached(t *testing.T) { + setupProUpdateTest(t, "6.0.0") + manager := NewManager(&config.Config{UpdateChannel: "stable", DataPath: t.TempDir()}) + manager.SetProUpdateCredentialSource(func() (ProUpdateCredentials, bool) { + return ProUpdateCredentials{}, false + }) + + info, err := manager.CheckForUpdates(context.Background()) + if err != nil { + t.Fatalf("unactivated Pro check: %v", err) + } + if info.Available || !strings.Contains(info.Warning, "Update checks are unavailable") { + t.Fatalf("unactivated Pro result = %+v, want unavailable warning without an offer", info) + } + if got := manager.LastUpdateCheck(); got.Outcome != UpdateCheckOutcomeSkipped || got.Available || got.Channel != "stable" { + t.Fatalf("unactivated Pro observation = %+v, want stable skipped without offer", got) + } + + fixture := newProBrokerFixture(t, "6.0.5", false) + manager.SetProUpdateCredentialSource(fixture.credentialSource()) + info, err = manager.CheckForUpdates(context.Background()) + if err != nil { + t.Fatalf("activated Pro check: %v", err) + } + if fixture.brokerCalls != 1 || !info.Available { + t.Fatalf("activated Pro result = %+v, broker calls = %d; want fresh offer", info, fixture.brokerCalls) + } + if got := manager.LastUpdateCheck(); got.Outcome != UpdateCheckOutcomeAvailable || !got.Available { + t.Fatalf("activated Pro observation = %+v, want available", got) + } + + previewPin := newProBrokerFixture(t, "6.1.0-rc.1", true) + stableManager := NewManager(&config.Config{UpdateChannel: "stable", DataPath: t.TempDir()}) + stableManager.SetProUpdateCredentialSource(previewPin.credentialSource()) + info, err = stableManager.CheckForUpdates(context.Background()) + if err != nil || info.Available || previewPin.brokerCalls != 1 { + t.Fatalf("stable Pro check against preview pin = %+v, err=%v, broker calls=%d", info, err, previewPin.brokerCalls) + } + if got := stableManager.LastUpdateCheck(); got.Outcome != UpdateCheckOutcomeNoRelease || got.Available { + t.Fatalf("stable Pro preview-pin observation = %+v, want no_release", got) + } +} + func TestIssue2285SourceBuildReportsSkipped(t *testing.T) { withBuildVersion(t, "6.4.0") markerPath := "BUILD_FROM_SOURCE" diff --git a/internal/updates/manager.go b/internal/updates/manager.go index 05ef73646e..68f8904697 100644 --- a/internal/updates/manager.go +++ b/internal/updates/manager.go @@ -74,6 +74,10 @@ type UpdateInfo struct { // compiled Pro binary, which cannot self-update in a container and must // never be pointed at the community rcourtman/pulse image. DockerUpdate *DockerUpdateCommands `json:"dockerUpdate,omitempty"` + // checkOutcome is only for the content-free observation. It distinguishes a + // check that could not run from a completed check with no update; it is not + // part of the update API response. + checkOutcome string } var ( @@ -453,8 +457,14 @@ func (m *Manager) CheckForUpdatesWithOptions(ctx context.Context, options Update m.updateStatus("error", 0, "Failed to check for Pulse Pro updates", proErr) return nil, proErr } - m.recordUpdateCheck(channel, effectiveChannel, availabilityOutcome(info.Available), info.Available) - if useCache { + outcome := availabilityOutcome(info.Available) + if info.checkOutcome != "" { + outcome = info.checkOutcome + } + m.recordUpdateCheck(channel, effectiveChannel, outcome, info.Available) + // A missing activation can be repaired without restarting Pulse. Do not + // cache its unavailable result across the next credentialed check. + if useCache && info.checkOutcome != UpdateCheckOutcomeSkipped { m.statusMu.Lock() m.checkCache[channel] = info m.cacheTime[channel] = time.Now() @@ -566,7 +576,13 @@ func (m *Manager) CheckForUpdatesWithOptions(ctx context.Context, options Update } info.Warning = updateWarning(info.Available, isMajorUpgrade, isPrerelease, currentVer.Major, latestVer.Major) - m.recordUpdateCheck(channel, effectiveChannel, availabilityOutcome(info.Available), info.Available) + checkOutcome := availabilityOutcome(info.Available) + if latestVer.IsNewerThan(currentVer) && downloadURL == "" { + // Metadata for a newer release without this binary's exact archive is + // not an offer and is not evidence that the install is up to date. + checkOutcome = UpdateCheckOutcomeMetadataError + } + m.recordUpdateCheck(channel, effectiveChannel, checkOutcome, info.Available) // Cache the result (only if using saved channel) if useCache { diff --git a/internal/updates/pro_update.go b/internal/updates/pro_update.go index 9eaf6b84f1..5f70d500dc 100644 --- a/internal/updates/pro_update.go +++ b/internal/updates/pro_update.go @@ -310,6 +310,7 @@ func (m *Manager) checkProUpdates(ctx context.Context, channel string, currentIn CurrentVersion: currentInfo.Version, LatestVersion: currentInfo.Version, Warning: "Update checks are unavailable: " + errProUpdateNotActivated().Error(), + checkOutcome: UpdateCheckOutcomeSkipped, }, nil } @@ -333,6 +334,7 @@ func (m *Manager) checkProUpdates(ctx context.Context, channel string, currentIn CurrentVersion: currentInfo.Version, LatestVersion: currentInfo.Version, Warning: fmt.Sprintf("The private Pulse Pro release channel currently serves prerelease %s; stable-channel installs skip prereleases.", manifest.Release.Version), + checkOutcome: UpdateCheckOutcomeNoRelease, }, nil } From 6f99faf2689145208e8985f0f21bfe8c2717e0fb Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 04:03:38 +0100 Subject: [PATCH 2/5] Use newest attributable PBS snapshot for guest backup badges A stronger namespace or comment score currently makes an old snapshot beat a newer, still attributable snapshot. Keep the existing typed-VMID and cross-instance eligibility guards, then choose the latest eligible backup for LastBackup and in-flight state. Add VM and CT regressions for unique fallback and colliding IDs. Contract-Neutral: Repair existing backup-age selection without changing API fields, persisted shapes, or PBS attribution eligibility. Change-source: pulse-maintainer --- internal/models/issue2292_backup_age_test.go | 116 +++++++++++++++++++ internal/models/models.go | 11 +- 2 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 internal/models/issue2292_backup_age_test.go diff --git a/internal/models/issue2292_backup_age_test.go b/internal/models/issue2292_backup_age_test.go new file mode 100644 index 0000000000..b252e1d41a --- /dev/null +++ b/internal/models/issue2292_backup_age_test.go @@ -0,0 +1,116 @@ +package models + +import ( + "testing" + "time" +) + +// A match score establishes whether a PBS snapshot can be attributed to a +// guest. Once it can, the badge must reflect the newest completed snapshot, +// even if an older snapshot has a stronger namespace/comment match (#2292). +func TestSyncGuestBackupTimesNewestAttributablePBSBackup(t *testing.T) { + now := time.Date(2026, time.September, 28, 0, 0, 0, 0, time.UTC) + old := now.Add(-65 * 24 * time.Hour) + recent := now.Add(-24 * time.Hour) + + for _, backupType := range []string{"vm", "ct"} { + t.Run(backupType, func(t *testing.T) { + state := NewState() + otherType := "vm" + if backupType == "vm" { + otherType = "ct" + state.UpdateVMs([]VM{{VMID: 112, Name: "guest-a", Instance: "cluster-a", Node: "node-a"}}) + } else { + state.UpdateContainers([]Container{{VMID: 112, Name: "guest-a", Instance: "cluster-a", Node: "node-a"}}) + } + + state.mu.Lock() + state.PBSBackups = []PBSBackup{ + {ID: "old", VMID: "112", BackupType: backupType, BackupTime: old, + Instance: "pbs-main", Namespace: "node-a", Comment: "guest-a"}, + // A unique typed VMID permits root-namespace fallback (score 1). + {ID: "recent", VMID: "112", BackupType: backupType, BackupTime: recent, + Instance: "pbs-main"}, + // A later snapshot for the other subject type is not this guest's. + {ID: "other-type", VMID: "112", BackupType: otherType, BackupTime: now.Add(-time.Hour), + Instance: "pbs-main", Namespace: "node-a"}, + } + state.mu.Unlock() + + state.SyncGuestBackupTimes() + snapshot := state.GetSnapshot() + var got time.Time + if backupType == "vm" { + got = snapshot.VMs[0].LastBackup + } else { + got = snapshot.Containers[0].LastBackup + } + if !got.Equal(recent) { + t.Errorf("LastBackup = %v, want newer attributable backup %v", got, recent) + } + }) + } +} + +// With a colliding typed VMID, the newer snapshot must still carry positive +// attribution for this guest; a newer snapshot for the other PVE connection +// must not advance its badge. VM and CT subjects also remain separate. +func TestSyncGuestBackupTimesNewestPBSBackupPreservesCollisionGuard(t *testing.T) { + now := time.Date(2026, time.September, 28, 0, 0, 0, 0, time.UTC) + oldA := now.Add(-27 * 24 * time.Hour) + recentA := now.Add(-24 * time.Hour) + recentB := now.Add(-2 * time.Hour) + + for _, backupType := range []string{"vm", "ct"} { + t.Run(backupType, func(t *testing.T) { + state := NewState() + if backupType == "vm" { + state.UpdateVMs([]VM{ + {VMID: 112, Name: "guest-a", Instance: "cluster-a", Node: "node-a"}, + {VMID: 112, Name: "guest-b", Instance: "cluster-b", Node: "node-b"}, + }) + } else { + state.UpdateContainers([]Container{ + {VMID: 112, Name: "guest-a", Instance: "cluster-a", Node: "node-a"}, + {VMID: 112, Name: "guest-b", Instance: "cluster-b", Node: "node-b"}, + }) + } + + state.mu.Lock() + state.PBSBackups = []PBSBackup{ + {ID: "old-a", VMID: "112", BackupType: backupType, BackupTime: oldA, + Instance: "pbs-main", Namespace: "node-a", Comment: "guest-a"}, + // The connection namespace is weaker than the actual node's, but + // still positively identifies A despite the VMID collision. + {ID: "recent-a", VMID: "112", BackupType: backupType, BackupTime: recentA, + Instance: "pbs-main", Namespace: "cluster-a"}, + {ID: "recent-b", VMID: "112", BackupType: backupType, BackupTime: recentB, + Instance: "pbs-main", Namespace: "node-b", Comment: "guest-b"}, + // This newest root-namespace snapshot has no source evidence to + // distinguish the two PVE connections and must be ignored. + {ID: "unattributable", VMID: "112", BackupType: backupType, BackupTime: now.Add(-time.Hour), + Instance: "pbs-unattributed"}, + } + state.mu.Unlock() + + state.SyncGuestBackupTimes() + snapshot := state.GetSnapshot() + got := make(map[string]time.Time) + if backupType == "vm" { + for _, guest := range snapshot.VMs { + got[guest.Instance] = guest.LastBackup + } + } else { + for _, guest := range snapshot.Containers { + got[guest.Instance] = guest.LastBackup + } + } + if !got["cluster-a"].Equal(recentA) { + t.Errorf("cluster-a LastBackup = %v, want its newer, weaker match %v", got["cluster-a"], recentA) + } + if !got["cluster-b"].Equal(recentB) { + t.Errorf("cluster-b LastBackup = %v, want its own snapshot %v", got["cluster-b"], recentB) + } + }) + } +} diff --git a/internal/models/models.go b/internal/models/models.go index 956e051b63..d15c91778b 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -4717,8 +4717,10 @@ func (s *State) SyncGuestBackupTimes() { } } - // findBestPBSBackup finds the best PBS backup for a given typed VMID and guest location. - // Placement and guest-name matches are preferred over VMID-only fallback. + // findBestPBSBackup finds the newest attributable PBS backup for a given + // typed VMID and guest location. A positive placement/name score, or a + // guarded VMID-only fallback, establishes attribution; score strength must + // not let an older snapshot hide a newer completed backup (#2292). // Returns zero time if no suitable backup found. The subject map is a // parameter so the same attribution rules apply to completed snapshots // (feeding LastBackup) and to in-flight ones (feeding BackupInProgress). @@ -4730,8 +4732,6 @@ func (s *State) SyncGuestBackupTimes() { } var bestTime time.Time - bestScore := -1 - for _, backup := range backups { score := proxmoxidentity.BackupGuestMatchScore( backup.Namespace, @@ -4774,8 +4774,7 @@ func (s *State) SyncGuestBackupTimes() { if score <= 0 { continue } - if score > bestScore || (score == bestScore && backup.BackupTime.After(bestTime)) { - bestScore = score + if backup.BackupTime.After(bestTime) { bestTime = backup.BackupTime } } From 7d7b1a0b85b9c5ff1047010ccd409ce6a4b67a82 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 04:25:04 +0100 Subject: [PATCH 3/5] Scope PBS backup recency to uniquely attributed guests A newer snapshot from another cluster can share the same typed VMID and node namespace. Compare each positive match with competing guests before selecting by recency; a stronger competitor excludes it and a tie needs independent attribution. Cover VM and CT completed and in-flight snapshots while preserving the newer weaker unique match. Contract-Neutral: Repair existing backup badge attribution and age selection without changing API fields, persisted shapes, or PBS source contracts. Change-source: pulse-maintainer --- internal/models/issue2292_backup_age_test.go | 94 ++++++++++++++++++-- internal/models/models.go | 31 ++++++- 2 files changed, 116 insertions(+), 9 deletions(-) diff --git a/internal/models/issue2292_backup_age_test.go b/internal/models/issue2292_backup_age_test.go index b252e1d41a..56c91d41a0 100644 --- a/internal/models/issue2292_backup_age_test.go +++ b/internal/models/issue2292_backup_age_test.go @@ -5,9 +5,9 @@ import ( "time" ) -// A match score establishes whether a PBS snapshot can be attributed to a -// guest. Once it can, the badge must reflect the newest completed snapshot, -// even if an older snapshot has a stronger namespace/comment match (#2292). +// A unique typed VMID permits the root-namespace fallback. Once attributed, +// the badge must reflect the newest completed snapshot, even if an older +// snapshot has a stronger namespace/comment match (#2292). func TestSyncGuestBackupTimesNewestAttributablePBSBackup(t *testing.T) { now := time.Date(2026, time.September, 28, 0, 0, 0, 0, time.UTC) old := now.Add(-65 * 24 * time.Hour) @@ -52,9 +52,9 @@ func TestSyncGuestBackupTimesNewestAttributablePBSBackup(t *testing.T) { } } -// With a colliding typed VMID, the newer snapshot must still carry positive -// attribution for this guest; a newer snapshot for the other PVE connection -// must not advance its badge. VM and CT subjects also remain separate. +// With a colliding typed VMID, the newer snapshot must be distinguishable +// from the other PVE connection; a newer snapshot for that other connection +// must not advance this guest's badge. VM and CT subjects remain separate. func TestSyncGuestBackupTimesNewestPBSBackupPreservesCollisionGuard(t *testing.T) { now := time.Date(2026, time.September, 28, 0, 0, 0, 0, time.UTC) oldA := now.Add(-27 * 24 * time.Hour) @@ -114,3 +114,85 @@ func TestSyncGuestBackupTimesNewestPBSBackupPreservesCollisionGuard(t *testing.T }) } } + +// Two independent clusters can have the same VMID and node label. In that +// case, a node namespace alone is positive for both guests, but a matching +// guest name makes the backup specific to one. Recency must be considered +// only after deciding which guest each snapshot can actually identify. +func TestSyncGuestBackupTimesNewestPBSBackupSameNodeCollision(t *testing.T) { + now := time.Date(2026, time.September, 28, 0, 0, 0, 0, time.UTC) + oldA := now.Add(-65 * 24 * time.Hour) + recentA := now.Add(-24 * time.Hour) + recentB := now.Add(-2 * time.Hour) + + for _, backupType := range []string{"vm", "ct"} { + t.Run(backupType, func(t *testing.T) { + state := NewState() + if backupType == "vm" { + state.UpdateVMs([]VM{ + {VMID: 112, Name: "guest-a", Instance: "cluster-a", Node: "pve"}, + {VMID: 112, Name: "guest-b", Instance: "cluster-b", Node: "pve"}, + }) + } else { + state.UpdateContainers([]Container{ + {VMID: 112, Name: "guest-a", Instance: "cluster-a", Node: "pve"}, + {VMID: 112, Name: "guest-b", Instance: "cluster-b", Node: "pve"}, + }) + } + + state.mu.Lock() + state.PBSBackups = []PBSBackup{ + {ID: "old-a", VMID: "112", BackupType: backupType, BackupTime: oldA, + Instance: "pbs-main", Namespace: "pve", Comment: "guest-a"}, + // Weaker than old-a, but only cluster-a matches this namespace. + {ID: "recent-a", VMID: "112", BackupType: backupType, BackupTime: recentA, + Instance: "pbs-main", Namespace: "cluster-a"}, + // Both guests match node pve, but guest-b is the stronger match. + {ID: "recent-b", VMID: "112", BackupType: backupType, BackupTime: recentB, + Instance: "pbs-main", Namespace: "pve", Comment: "guest-b"}, + // Neither guest owns an otherwise indistinguishable newer copy. + {ID: "shared-node", VMID: "112", BackupType: backupType, BackupTime: now.Add(-time.Hour), + Instance: "pbs-main", Namespace: "pve"}, + // The same attribution rule also governs a live, incomplete PBS + // snapshot: only B may show a running backup. + {ID: "running-b", VMID: "112", BackupType: backupType, BackupTime: time.Now(), + Instance: "pbs-main", Namespace: "pve", Comment: "guest-b", InProgress: true}, + {ID: "running-shared", VMID: "112", BackupType: backupType, BackupTime: time.Now(), + Instance: "pbs-main", Namespace: "pve", InProgress: true}, + } + state.mu.Unlock() + + state.SyncGuestBackupTimes() + snapshot := state.GetSnapshot() + got := make(map[string]time.Time) + if backupType == "vm" { + for _, guest := range snapshot.VMs { + got[guest.Instance] = guest.LastBackup + } + } else { + for _, guest := range snapshot.Containers { + got[guest.Instance] = guest.LastBackup + } + } + if !got["cluster-a"].Equal(recentA) { + t.Errorf("cluster-a LastBackup = %v, want its newer uniquely attributable backup %v", got["cluster-a"], recentA) + } + if !got["cluster-b"].Equal(recentB) { + t.Errorf("cluster-b LastBackup = %v, want its own backup %v", got["cluster-b"], recentB) + } + if backupType == "vm" { + for _, guest := range snapshot.VMs { + if guest.BackupInProgress != (guest.Instance == "cluster-b") { + t.Errorf("%s BackupInProgress = %v, want only cluster-b running", guest.Instance, guest.BackupInProgress) + } + } + } else { + for _, guest := range snapshot.Containers { + if guest.BackupInProgress != (guest.Instance == "cluster-b") { + t.Errorf("%s BackupInProgress = %v, want only cluster-b running", guest.Instance, guest.BackupInProgress) + } + } + } + }) + } +} diff --git a/internal/models/models.go b/internal/models/models.go index d15c91778b..359f242c1e 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -4718,9 +4718,8 @@ func (s *State) SyncGuestBackupTimes() { } // findBestPBSBackup finds the newest attributable PBS backup for a given - // typed VMID and guest location. A positive placement/name score, or a - // guarded VMID-only fallback, establishes attribution; score strength must - // not let an older snapshot hide a newer completed backup (#2292). + // typed VMID and guest location. Match strength decides which guest owns + // each snapshot, not whether an older owned snapshot outranks a newer one. // Returns zero time if no suitable backup found. The subject map is a // parameter so the same attribution rules apply to completed snapshots // (feeding LastBackup) and to in-flight ones (feeding BackupInProgress). @@ -4742,6 +4741,32 @@ func (s *State) SyncGuestBackupTimes() { node, ) snapshotKey := pbsSnapshotKey{backupType: backupType, vmid: vmid, unixTime: backup.BackupTime.Unix()} + if score > 0 && subjectIsAmbiguous[subjectKey] { + // A namespace such as "pve" can match two clusters with the + // same node label. A positive score is not attribution when a + // different connection matches the same snapshot at least as + // strongly. A stronger competitor rules this guest out; a tie + // needs independent storage/source evidence below (#2292). + strongestOther := 0 + for _, other := range subjectGuests[subjectKey] { + if other.instance == instance { + continue + } + otherScore := proxmoxidentity.BackupGuestMatchScore( + backup.Namespace, backup.Comment, backup.VMID, + other.name, other.instance, other.node, + ) + if otherScore > strongestOther { + strongestOther = otherScore + } + } + if strongestOther > score { + continue + } + if strongestOther == score { + score = 0 + } + } if score == 0 { if !subjectIsAmbiguous[subjectKey] { score = 1 From d704afa68abeca49699f890f9fa81196f55084ab Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 04:32:02 +0100 Subject: [PATCH 4/5] Reject shared-node PBS ties despite learned source A globally observed second cluster can leave a PBS source apparently mapped to the first while both clusters match a shared-node snapshot. Do not promote a positive but tied guest match into the VMID-only inference path. Regression covers both VM and CT completed and in-flight snapshots with the second cluster visible on another PBS instance. Contract-Neutral: Preserve existing PBS backup attribution safety without changing API fields, persisted shapes, or source contracts. Change-source: pulse-maintainer --- internal/models/issue2292_backup_age_test.go | 28 +++++++++++++++++--- internal/models/models.go | 10 +++---- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/internal/models/issue2292_backup_age_test.go b/internal/models/issue2292_backup_age_test.go index 56c91d41a0..ad4993e674 100644 --- a/internal/models/issue2292_backup_age_test.go +++ b/internal/models/issue2292_backup_age_test.go @@ -98,11 +98,15 @@ func TestSyncGuestBackupTimesNewestPBSBackupPreservesCollisionGuard(t *testing.T got := make(map[string]time.Time) if backupType == "vm" { for _, guest := range snapshot.VMs { - got[guest.Instance] = guest.LastBackup + if guest.VMID == 112 { + got[guest.Instance] = guest.LastBackup + } } } else { for _, guest := range snapshot.Containers { - got[guest.Instance] = guest.LastBackup + if guest.VMID == 112 { + got[guest.Instance] = guest.LastBackup + } } } if !got["cluster-a"].Equal(recentA) { @@ -132,11 +136,13 @@ func TestSyncGuestBackupTimesNewestPBSBackupSameNodeCollision(t *testing.T) { state.UpdateVMs([]VM{ {VMID: 112, Name: "guest-a", Instance: "cluster-a", Node: "pve"}, {VMID: 112, Name: "guest-b", Instance: "cluster-b", Node: "pve"}, + {VMID: 113, Name: "other-b", Instance: "cluster-b", Node: "pve"}, }) } else { state.UpdateContainers([]Container{ {VMID: 112, Name: "guest-a", Instance: "cluster-a", Node: "pve"}, {VMID: 112, Name: "guest-b", Instance: "cluster-b", Node: "pve"}, + {VMID: 113, Name: "other-b", Instance: "cluster-b", Node: "pve"}, }) } @@ -150,6 +156,10 @@ func TestSyncGuestBackupTimesNewestPBSBackupSameNodeCollision(t *testing.T) { // Both guests match node pve, but guest-b is the stronger match. {ID: "recent-b", VMID: "112", BackupType: backupType, BackupTime: recentB, Instance: "pbs-main", Namespace: "pve", Comment: "guest-b"}, + // B is visible on a different PBS instance. That is not proof + // that pbs-main's tied snapshot belongs to A rather than B. + {ID: "other-b", VMID: "113", BackupType: backupType, BackupTime: recentB, + Instance: "pbs-other"}, // Neither guest owns an otherwise indistinguishable newer copy. {ID: "shared-node", VMID: "112", BackupType: backupType, BackupTime: now.Add(-time.Hour), Instance: "pbs-main", Namespace: "pve"}, @@ -167,11 +177,15 @@ func TestSyncGuestBackupTimesNewestPBSBackupSameNodeCollision(t *testing.T) { got := make(map[string]time.Time) if backupType == "vm" { for _, guest := range snapshot.VMs { - got[guest.Instance] = guest.LastBackup + if guest.VMID == 112 { + got[guest.Instance] = guest.LastBackup + } } } else { for _, guest := range snapshot.Containers { - got[guest.Instance] = guest.LastBackup + if guest.VMID == 112 { + got[guest.Instance] = guest.LastBackup + } } } if !got["cluster-a"].Equal(recentA) { @@ -182,12 +196,18 @@ func TestSyncGuestBackupTimesNewestPBSBackupSameNodeCollision(t *testing.T) { } if backupType == "vm" { for _, guest := range snapshot.VMs { + if guest.VMID != 112 { + continue + } if guest.BackupInProgress != (guest.Instance == "cluster-b") { t.Errorf("%s BackupInProgress = %v, want only cluster-b running", guest.Instance, guest.BackupInProgress) } } } else { for _, guest := range snapshot.Containers { + if guest.VMID != 112 { + continue + } if guest.BackupInProgress != (guest.Instance == "cluster-b") { t.Errorf("%s BackupInProgress = %v, want only cluster-b running", guest.Instance, guest.BackupInProgress) } diff --git a/internal/models/models.go b/internal/models/models.go index 359f242c1e..46d9e0e728 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -4745,8 +4745,9 @@ func (s *State) SyncGuestBackupTimes() { // A namespace such as "pve" can match two clusters with the // same node label. A positive score is not attribution when a // different connection matches the same snapshot at least as - // strongly. A stronger competitor rules this guest out; a tie - // needs independent storage/source evidence below (#2292). + // strongly. Do not use batch-learned source inference to break a + // positive tie: it can see the other connection on another PBS + // instance without proving which one wrote this snapshot (#2292). strongestOther := 0 for _, other := range subjectGuests[subjectKey] { if other.instance == instance { @@ -4760,12 +4761,9 @@ func (s *State) SyncGuestBackupTimes() { strongestOther = otherScore } } - if strongestOther > score { + if strongestOther >= score { continue } - if strongestOther == score { - score = 0 - } } if score == 0 { if !subjectIsAmbiguous[subjectKey] { From f7a43d27946a48a61b3c84a5bd1ab1fc561eab97 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 05:03:26 +0100 Subject: [PATCH 5/5] Retain observed Docker update alert age through cleanup Hourly cleanup previously aged pending image updates by first detection, so a continuing update older than 24 hours lost its tracking and could restart its delay. Track observation activity separately, keep unknown checks pending, and reclaim state when observations stop or an affirmative clear arrives. Cover 24- and 48-hour delays and cleanup. Contract-Neutral: Preserve the existing Docker update alert policy and public contract while correcting internal tracking expiry. Change-source: pulse-maintainer --- internal/alerts/active_cleanup.go | 1 + internal/alerts/config_runtime.go | 2 + internal/alerts/docker.go | 21 ++++++ .../alerts/docker_update_retention_test.go | 72 +++++++++++++++++++ internal/alerts/manager.go | 2 + internal/alerts/tracking_cleanup.go | 14 +++- .../docker-update-retention/README.md | 22 ++++++ 7 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 internal/alerts/docker_update_retention_test.go create mode 100644 tests/qualification/docker-update-retention/README.md diff --git a/internal/alerts/active_cleanup.go b/internal/alerts/active_cleanup.go index 34d0935692..76cf8ef70c 100644 --- a/internal/alerts/active_cleanup.go +++ b/internal/alerts/active_cleanup.go @@ -349,6 +349,7 @@ func (m *Manager) ClearActiveAlerts() { m.dockerRestartTracking = make(map[string]*dockerRestartRecord) m.dockerUpdateFirstSeen = make(map[string]time.Time) m.dockerUpdateFirstSeenByIdentity = make(map[string]time.Time) + m.dockerUpdateLastObserved = make(map[string]time.Time) m.smartCounterSnapshots = make(map[string]smartCounterSnapshot) m.ackState = make(map[string]ackRecord) m.ackStateByCanonical = make(map[string]ackRecord) diff --git a/internal/alerts/config_runtime.go b/internal/alerts/config_runtime.go index 27202d3d0f..73b9fe2891 100644 --- a/internal/alerts/config_runtime.go +++ b/internal/alerts/config_runtime.go @@ -241,11 +241,13 @@ func (m *Manager) applyGlobalOfflineSettingsLocked() { m.dockerRestartTracking = make(map[string]*dockerRestartRecord) m.dockerUpdateFirstSeen = make(map[string]time.Time) m.dockerUpdateFirstSeenByIdentity = make(map[string]time.Time) + m.dockerUpdateLastObserved = make(map[string]time.Time) } if m.config.DockerDefaults.UpdateAlertDelayHours < 0 && !containersDisabled { m.clearDockerContainerUpdateAlertsLocked() m.dockerUpdateFirstSeen = make(map[string]time.Time) m.dockerUpdateFirstSeenByIdentity = make(map[string]time.Time) + m.dockerUpdateLastObserved = make(map[string]time.Time) } if servicesDisabled { var serviceAlerts []string diff --git a/internal/alerts/docker.go b/internal/alerts/docker.go index 1ab242fb08..e3a12331d5 100644 --- a/internal/alerts/docker.go +++ b/internal/alerts/docker.go @@ -1206,8 +1206,10 @@ func (m *Manager) clearDockerContainerMetricAlerts(resourceID string, metrics .. func (m *Manager) clearDockerContainerUpdateTracking(resourceID, trackingKey string) { m.mu.Lock() delete(m.dockerUpdateFirstSeen, resourceID) + delete(m.dockerUpdateLastObserved, resourceID) if trackingKey != "" { delete(m.dockerUpdateFirstSeenByIdentity, trackingKey) + delete(m.dockerUpdateLastObserved, trackingKey) } m.mu.Unlock() } @@ -1251,9 +1253,11 @@ func (m *Manager) clearDockerContainerUpdateStateLocked(alert *Alert) { if alert.ResourceID != "" { delete(m.dockerUpdateFirstSeen, alert.ResourceID) + delete(m.dockerUpdateLastObserved, alert.ResourceID) } if trackingKey := dockerUpdateTrackingKeyFromAlert(alert); trackingKey != "" { delete(m.dockerUpdateFirstSeenByIdentity, trackingKey) + delete(m.dockerUpdateLastObserved, trackingKey) } } @@ -1339,6 +1343,17 @@ func (m *Manager) checkDockerContainerImageUpdate(host models.DockerHost, contai return } + // Reports refresh tracking activity even when the registry result is cached + // or unavailable. Do not create a pending condition from unknown evidence. + m.mu.Lock() + if _, exists := m.dockerUpdateFirstSeen[resourceID]; exists { + m.dockerUpdateLastObserved[resourceID] = time.Now() + } + if _, exists := m.dockerUpdateFirstSeenByIdentity[updateTrackingKey]; exists { + m.dockerUpdateLastObserved[updateTrackingKey] = time.Now() + } + m.mu.Unlock() + // Check if this container has an update status reported if container.UpdateStatus == nil { // Missing update status means the condition is unknown, not resolved. @@ -1373,6 +1388,8 @@ func (m *Manager) checkDockerContainerImageUpdate(host models.DockerHost, contai } m.dockerUpdateFirstSeen[resourceID] = firstSeen m.dockerUpdateFirstSeenByIdentity[updateTrackingKey] = firstSeen + m.dockerUpdateLastObserved[resourceID] = time.Now() + m.dockerUpdateLastObserved[updateTrackingKey] = time.Now() m.mu.Unlock() // Check if we've exceeded the delay threshold @@ -1475,6 +1492,7 @@ func (m *Manager) cleanupDockerContainerAlertsWithTracking(host models.DockerHos if strings.HasPrefix(resourceID, prefix) { if _, exists := seen[resourceID]; !exists { delete(m.dockerUpdateFirstSeen, resourceID) + delete(m.dockerUpdateLastObserved, resourceID) } } } @@ -1485,6 +1503,7 @@ func (m *Manager) cleanupDockerContainerAlertsWithTracking(host models.DockerHos } if _, exists := seenUpdateTracking[trackingKey]; !exists { delete(m.dockerUpdateFirstSeenByIdentity, trackingKey) + delete(m.dockerUpdateLastObserved, trackingKey) } } } @@ -1520,11 +1539,13 @@ func (m *Manager) clearDockerHostContainerAlerts(host models.DockerHost) { for resourceID := range m.dockerUpdateFirstSeen { if strings.HasPrefix(resourceID, prefix) { delete(m.dockerUpdateFirstSeen, resourceID) + delete(m.dockerUpdateLastObserved, resourceID) } } for trackingKey := range m.dockerUpdateFirstSeenByIdentity { if strings.HasPrefix(trackingKey, updateTrackingPrefix) { delete(m.dockerUpdateFirstSeenByIdentity, trackingKey) + delete(m.dockerUpdateLastObserved, trackingKey) } } m.mu.Unlock() diff --git a/internal/alerts/docker_update_retention_test.go b/internal/alerts/docker_update_retention_test.go new file mode 100644 index 0000000000..87cd7a4b17 --- /dev/null +++ b/internal/alerts/docker_update_retention_test.go @@ -0,0 +1,72 @@ +package alerts + +import ( + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" +) + +// A cached registry observation is still an asserted condition on each report. +// Its first detection time must not be mistaken for tracking inactivity. +func TestDockerUpdateTrackingSurvivesDailyCleanup(t *testing.T) { + for _, delay := range []int{24, 48} { + t.Run((time.Duration(delay) * time.Hour).String(), func(t *testing.T) { + m := newTestManager(t) + m.config.DockerDefaults.UpdateAlertDelayHours = delay + host := models.DockerHost{ID: "host", DisplayName: "host"} + container := models.DockerContainer{ID: "container", Name: "web", Image: "mongo:7", UpdateStatus: &models.DockerContainerUpdateStatus{UpdateAvailable: true, CurrentDigest: "sha256:old", LatestDigest: "sha256:new", LastChecked: time.Now().Add(-6 * time.Hour)}} + resource := "docker:host/container" + key := dockerUpdateTrackingKey(host, container) + first := time.Now().Add(-25 * time.Hour) + m.dockerUpdateFirstSeen[resource] = first + m.dockerUpdateFirstSeenByIdentity[key] = first + check := func() { m.checkDockerContainerImageUpdate(host, container, resource, "web", "host", "host") } + check() + m.cleanupStaleMaps() + if !m.dockerUpdateFirstSeen[resource].Equal(first) || !m.dockerUpdateFirstSeenByIdentity[key].Equal(first) { + t.Fatal("daily cleanup discarded an observed pending update's first detection") + } + check() + if delay == 24 { + alerts := m.GetActiveAlerts() + if len(alerts) != 1 || !alerts[0].StartTime.Equal(first) { + t.Fatalf("continuing update lost its original incident: %+v", alerts) + } + } + // Unknown reports must neither resolve nor restart a pending condition. + container.UpdateStatus = nil + check() + m.cleanupStaleMaps() + if !m.dockerUpdateFirstSeen[resource].Equal(first) { + t.Fatal("missing status reset pending age") + } + container.UpdateStatus = &models.DockerContainerUpdateStatus{Error: "registry unavailable"} + check() + m.cleanupStaleMaps() + if !m.dockerUpdateFirstSeenByIdentity[key].Equal(first) { + t.Fatal("failed check reset pending age") + } + // An affirmative clear remains effective. + container.UpdateStatus = &models.DockerContainerUpdateStatus{UpdateAvailable: false, LastChecked: time.Now()} + check() + if len(m.GetActiveAlerts()) != 0 || len(m.dockerUpdateFirstSeen) != 0 || len(m.dockerUpdateFirstSeenByIdentity) != 0 || len(m.dockerUpdateLastObserved) != 0 { + t.Fatal("affirmative clear did not retire update") + } + }) + } +} + +func TestDockerUpdateTrackingExpiresOnlyAfterObservationStops(t *testing.T) { + m := newTestManager(t) + old := time.Now().Add(-25 * time.Hour) + for _, key := range []string{"docker:host/container", "docker-update:host/id:container"} { + m.dockerUpdateLastObserved[key] = old + } + m.dockerUpdateFirstSeen["docker:host/container"] = old.Add(-48 * time.Hour) + m.dockerUpdateFirstSeenByIdentity["docker-update:host/id:container"] = old.Add(-48 * time.Hour) + m.cleanupStaleMaps() + if len(m.dockerUpdateFirstSeen) != 0 || len(m.dockerUpdateFirstSeenByIdentity) != 0 || len(m.dockerUpdateLastObserved) != 0 { + t.Fatal("unobserved tracking was not reclaimed") + } +} diff --git a/internal/alerts/manager.go b/internal/alerts/manager.go index d263648743..747351f36e 100644 --- a/internal/alerts/manager.go +++ b/internal/alerts/manager.go @@ -82,6 +82,7 @@ type Manager struct { dockerUpdateFirstSeen map[string]time.Time // Track when image updates were first detected for alert delay // Stable identity tracking prevents update-delay resets when host IDs churn. dockerUpdateFirstSeenByIdentity map[string]time.Time + dockerUpdateLastObserved map[string]time.Time // Resource and stable identity keys; activity, not pending age. // PMG quarantine growth tracking pmgQuarantineHistory map[string][]pmgQuarantineSnapshot // Track quarantine snapshots for growth detection // SMART counter snapshots let alert evaluation distinguish historical @@ -227,6 +228,7 @@ func NewManagerWithDataDir(dataDir string, options ...ManagerOption) *Manager { dockerRestartTracking: make(map[string]*dockerRestartRecord), dockerUpdateFirstSeen: make(map[string]time.Time), dockerUpdateFirstSeenByIdentity: make(map[string]time.Time), + dockerUpdateLastObserved: make(map[string]time.Time), pmgQuarantineHistory: make(map[string][]pmgQuarantineSnapshot), smartCounterSnapshots: make(map[string]smartCounterSnapshot), pmgAnomalyTrackers: make(map[string]*pmgAnomalyTracker), diff --git a/internal/alerts/tracking_cleanup.go b/internal/alerts/tracking_cleanup.go index 9c546f0443..2628db03cb 100644 --- a/internal/alerts/tracking_cleanup.go +++ b/internal/alerts/tracking_cleanup.go @@ -70,14 +70,24 @@ func (m *Manager) cleanupStaleMaps() { } for containerID, firstSeen := range m.dockerUpdateFirstSeen { - if now.Sub(firstSeen) > staleThreshold { + lastObserved := m.dockerUpdateLastObserved[containerID] + if lastObserved.IsZero() { + lastObserved = firstSeen + } + if now.Sub(lastObserved) > staleThreshold { delete(m.dockerUpdateFirstSeen, containerID) + delete(m.dockerUpdateLastObserved, containerID) cleaned++ } } for containerID, firstSeen := range m.dockerUpdateFirstSeenByIdentity { - if now.Sub(firstSeen) > staleThreshold { + lastObserved := m.dockerUpdateLastObserved[containerID] + if lastObserved.IsZero() { + lastObserved = firstSeen + } + if now.Sub(lastObserved) > staleThreshold { delete(m.dockerUpdateFirstSeenByIdentity, containerID) + delete(m.dockerUpdateLastObserved, containerID) cleaned++ } } diff --git a/tests/qualification/docker-update-retention/README.md b/tests/qualification/docker-update-retention/README.md new file mode 100644 index 0000000000..d06bf83d3f --- /dev/null +++ b/tests/qualification/docker-update-retention/README.md @@ -0,0 +1,22 @@ +# Docker update tracking retention + +Run `go test -race ./internal/alerts -run 'TestDockerUpdate|TestCheckDockerContainerImageUpdate|TestCleanupStaleMaps|TestCleanupDockerContainerAlerts|TestCleanup$|TestUpdateConfig.*DockerContainerUpdate|TestEvaluateDockerContainerClearsUpdate' -count=1`. + +`TestDockerUpdateTrackingSurvivesDailyCleanup` seeds a first detection 25 hours +ago, with a cached registry result, and runs the real update evaluator and +hourly tracking cleanup. Both 24-hour (already firing) and 48-hour (pending) +delays must retain the detection time. Missing/error results retain it too; +an affirmative no-update result still clears the incident and both tracking +identities. Tracking is reclaimed after observations stop, and existing tests +cover host identity changes, removal and disabled update policy. + +The regression fails against the preceding source in both delay cases: cleanup +uses first detection as inactivity and discards live tracking. Keep pending age +separate from last observation. The new timestamps are ephemeral, bounded by the +same removal/configuration/24-hour inactivity paths as their tracking entries; +they are not registry-check timestamps or persisted evidence. + +This proves a tracking reset defect, not the cause of every open/resolved/open +cycle in issue #2048. The legacy recovery adapter labels an existing clear with +unknown confidence; it does not initiate that clear. No registry requests, +Docker updates, reporter installation or release availability are exercised.