diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index f1a38739d..b9ec38638 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 25bce8769..42a4f46e5 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/alerts/active_cleanup.go b/internal/alerts/active_cleanup.go index 34d093569..76cf8ef70 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 27202d3d0..73b9fe289 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 1ab242fb0..e3a12331d 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 000000000..87cd7a4b1 --- /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 d26364874..747351f36 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 9c546f044..2628db03c 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/internal/models/issue2292_backup_age_test.go b/internal/models/issue2292_backup_age_test.go new file mode 100644 index 000000000..ad4993e67 --- /dev/null +++ b/internal/models/issue2292_backup_age_test.go @@ -0,0 +1,218 @@ +package models + +import ( + "testing" + "time" +) + +// 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) + 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 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) + 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 { + if guest.VMID == 112 { + got[guest.Instance] = guest.LastBackup + } + } + } else { + for _, guest := range snapshot.Containers { + if guest.VMID == 112 { + 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) + } + }) + } +} + +// 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"}, + {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"}, + }) + } + + 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"}, + // 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"}, + // 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 { + if guest.VMID == 112 { + got[guest.Instance] = guest.LastBackup + } + } + } else { + for _, guest := range snapshot.Containers { + if guest.VMID == 112 { + 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.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 956e051b6..46d9e0e72 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -4717,8 +4717,9 @@ 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. 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). @@ -4730,8 +4731,6 @@ func (s *State) SyncGuestBackupTimes() { } var bestTime time.Time - bestScore := -1 - for _, backup := range backups { score := proxmoxidentity.BackupGuestMatchScore( backup.Namespace, @@ -4742,6 +4741,30 @@ 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. 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 { + 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 score == 0 { if !subjectIsAmbiguous[subjectKey] { score = 1 @@ -4774,8 +4797,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 } } diff --git a/internal/updates/issue2285_update_check_observation_test.go b/internal/updates/issue2285_update_check_observation_test.go index e0ce58c54..5adc15c94 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,11 +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) - // The check only offers an update when the exact server archive exists. - stable := ReleaseInfo{TagName: "v6.4.5", PublishedAt: time.Date(2026, 9, 30, 8, 0, 0, 0, time.UTC), Assets: []ReleaseAsset{runtimeArchiveFixture(t, "v6.4.5")}} - preview := ReleaseInfo{TagName: "v6.4.6-rc.1", Prerelease: true, PublishedAt: time.Date(2026, 10, 2, 8, 0, 0, 0, time.UTC), Assets: []ReleaseAsset{runtimeArchiveFixture(t, "v6.4.6-rc.1")}} + 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 @@ -52,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}, @@ -65,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) @@ -81,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"}) @@ -96,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 05ef73646..68f890469 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 9eaf6b84f..5f70d500d 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 } diff --git a/tests/qualification/docker-update-retention/README.md b/tests/qualification/docker-update-retention/README.md new file mode 100644 index 000000000..d06bf83d3 --- /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.