Fix patch-release follow-up regressions

Refs #1510

Refs #1501

Refs #1507

Refs #1442

- persist scoped workloads status filters across platform navigation
- derive host memory pressure from available memory
- reapply system settings after every monitor reload path
- bound PBS backup snapshot polling workers during large backup scans
This commit is contained in:
rcourtman 2026-07-04 18:37:59 +01:00
parent 64d58fd161
commit cefc032a37
10 changed files with 283 additions and 58 deletions

View file

@ -27,6 +27,11 @@ Monitoring owns source freshness cadence for Proxmox, PBS, and PMG resources:
the stale threshold is derived from the configured polling interval with a
minimum floor, so API-facing resource status must not degrade merely because a
healthy source is between normal poll cycles.
PBS backup snapshot refresh is a bounded monitoring hot path: group-level
snapshot fetches must run through the fixed worker pool in
`internal/monitoring/monitor_backups.go`, reuse cached snapshots on per-group
fetch failures, and must not allocate one goroutine or buffered result slot per
backup group in large PBS datastores.
Removed host-agent reconnect blocks are identity-scoped: matching may use the
canonical host ID or token-qualified machine/hostname continuity, but must never
block a distinct live host by hostname alone.
@ -205,6 +210,11 @@ threshold value.
guest slices as the primary source of truth. Clustered PVE snapshot polling
must therefore see guests collected earlier in the same cycle before calling
the Proxmox guest snapshot APIs.
PBS backup snapshot refreshes in that same file must stay bounded by the
package worker-pool constant and stream requests through workers instead of
creating one goroutine per backup group; per-group API failures may reuse
cached snapshots, but the polling loop must keep memory proportional to the
worker count rather than datastore cardinality.
12. Add or change agentless availability monitoring only through the
poll-provider path. `internal/monitoring/availability_poller.go` owns ICMP,
TCP, and HTTP probes, provider health, scheduler task construction, and

View file

@ -208,6 +208,10 @@ regression protection.
URL from already-loaded row metadata and must not add per-row metadata
fetches, Discovery probes, or page-local external-link logic on the table
hot path.
Workload status filters remain URL-canonical. Scoped local persistence may
seed a missing `status` parameter when a platform page is revisited, but the
restore path must write the URL once with `replace` and must not force row
filtering through a separate page-local state channel.
4. Keep shared auth gating in `internal/api/router.go` cheap and local: pre-auth quick-setup and recovery routing may short-circuit on loopback/session/token checks, but they must not trigger chart, metrics, or broad persistence fan-out on the protected request hot path.
Reading mutable auth configuration for CSRF bootstrap and login checks must
stay a short in-memory snapshot under `config.Mu.RLock()`: local

View file

@ -42,6 +42,7 @@ describe('useWorkloadsControlsState', () => {
localStorage.clear();
setMockRouterSearch('');
navigateSpy.mockClear();
window.history.replaceState(null, '', '/workloads');
setWideViewport();
});
@ -92,6 +93,48 @@ describe('useWorkloadsControlsState', () => {
});
});
it('persists scoped workload status so platform pages restore it when revisited', async () => {
createRoot((dispose) => {
try {
const [showFilters, setShowFilters] = createSignal(false);
const state = useWorkloadsControlsState({
viewMode: () => 'all' as ViewMode,
showFilters,
setShowFilters,
statusModeStorageScope: 'proxmox',
});
state.setStatusMode('running');
expect(mockRouterSearch()).toBe('?status=running');
expect(localStorage.getItem('workloadsStatusMode:proxmox')).toBe('running');
} finally {
dispose();
}
});
setMockRouterSearch('');
window.history.replaceState(null, '', '/workloads');
navigateSpy.mockClear();
const disposeRestore = createRoot((dispose) => {
const [showFilters, setShowFilters] = createSignal(false);
useWorkloadsControlsState({
viewMode: () => 'all' as ViewMode,
showFilters,
setShowFilters,
statusModeStorageScope: 'proxmox',
});
return dispose;
});
await Promise.resolve();
expect(navigateSpy).toHaveBeenCalledWith('/workloads?status=running', { replace: true });
expect(mockRouterSearch()).toBe('?status=running');
disposeRestore();
});
it('lets Docker scope use a container-native column profile without hiding disk globally', () => {
createRoot((dispose) => {
try {

View file

@ -93,7 +93,7 @@ describe('workloadUrlSyncModel', () => {
expect(
resolveWorkloadsManagedWorkloadsNavigateTarget({
currentPathname: '/proxmox/overview',
currentSearch: '?resource=guest-1&type=vm&agent=node-a',
currentSearch: '?resource=guest-1&status=running&type=vm&agent=node-a',
viewMode: 'pod',
effectiveViewMode: 'pod',
containerRuntime: 'docker',
@ -105,7 +105,7 @@ describe('workloadUrlSyncModel', () => {
selectedHostHint: null,
}),
).toBe(
'/proxmox/overview?resource=guest-1&type=pod&platform=kubernetes&context=prod&namespace=default',
'/proxmox/overview?resource=guest-1&status=running&type=pod&platform=kubernetes&context=prod&namespace=default',
);
expect(

View file

@ -56,6 +56,33 @@ const parseWorkloadsStatusMode = (raw: string | null | undefined): WorkloadsStat
? (raw as WorkloadsStatusMode)
: DEFAULT_WORKLOADS_STATUS_MODE;
const workloadsStatusModeStorageKey = (scope: string | undefined): string => {
const trimmedScope = (scope || '').trim();
return trimmedScope ? `workloadsStatusMode:${trimmedScope}` : 'workloadsStatusMode';
};
const saveWorkloadsStatusMode = (
scope: string | undefined,
value: WorkloadsStatusMode,
): void => {
if (typeof window === 'undefined') return;
try {
window.localStorage.setItem(workloadsStatusModeStorageKey(scope), value);
} catch {
// Ignore storage failures; the URL remains the canonical live state.
}
};
const readSavedWorkloadsStatusMode = (scope: string | undefined): WorkloadsStatusMode => {
if (typeof window === 'undefined') return DEFAULT_WORKLOADS_STATUS_MODE;
try {
const raw = window.localStorage.getItem(workloadsStatusModeStorageKey(scope));
return parseWorkloadsStatusMode(raw);
} catch {
return DEFAULT_WORKLOADS_STATUS_MODE;
}
};
export function useWorkloadsControlsState(options: WorkloadsControlsStateOptions) {
const location = useLocation();
const navigate = useNavigate();
@ -86,6 +113,7 @@ export function useWorkloadsControlsState(options: WorkloadsControlsStateOptions
const statusMode: Accessor<WorkloadsStatusMode> = () =>
parseWorkloadsStatusMode(new URLSearchParams(location.search).get('status'));
const setStatusMode = (value: WorkloadsStatusMode): void => {
saveWorkloadsStatusMode(options.statusModeStorageScope, value);
updateSearchParam((params) => {
if (value === DEFAULT_WORKLOADS_STATUS_MODE) {
params.delete('status');
@ -99,12 +127,9 @@ export function useWorkloadsControlsState(options: WorkloadsControlsStateOptions
if (typeof window === 'undefined') return;
const params = new URLSearchParams(window.location.search);
if (params.has('status')) return;
const scope = (options.statusModeStorageScope || '').trim();
const legacyKey = scope ? `workloadsStatusMode:${scope}` : 'workloadsStatusMode';
const legacy = window.localStorage.getItem(legacyKey);
const parsed = parseWorkloadsStatusMode(legacy);
if (parsed !== DEFAULT_WORKLOADS_STATUS_MODE && legacy === parsed) {
params.set('status', parsed);
const saved = readSavedWorkloadsStatusMode(options.statusModeStorageScope);
if (saved !== DEFAULT_WORKLOADS_STATUS_MODE) {
params.set('status', saved);
navigate(`${window.location.pathname}?${params.toString()}`, { replace: true });
}
});

View file

@ -73,10 +73,15 @@ func Collect(ctx context.Context, diskExclude []string) (Snapshot, error) {
return Snapshot{}, fmt.Errorf("memory stats: %w", err)
}
usedBytes := memStats.Used
freeBytes := memStats.Free
usedBytes := memStats.Used
usedPercent := memStats.UsedPercent
if memStats.Total > 0 && memStats.Available > 0 && memStats.Available <= memStats.Total {
usedBytes = memStats.Total - memStats.Available
usedPercent = memoryUsagePercent(usedBytes, memStats.Total)
}
// Reclaimable page cache: Available counts the pages the kernel would hand
// back under pressure on top of truly-free ones, so the gap is buff/cache.
// Reported separately so the memory bar can show used | cache | free.
@ -94,15 +99,7 @@ func Collect(ctx context.Context, diskExclude []string) (Snapshot, error) {
} else {
usedBytes = 0
}
if memStats.Total > 0 {
usedPercent = float64(usedBytes) / float64(memStats.Total) * 100.0
if usedPercent < 0 {
usedPercent = 0
}
if usedPercent > 100 {
usedPercent = 100
}
}
usedPercent = memoryUsagePercent(usedBytes, memStats.Total)
// Recompute free so used + cache + free still covers the total after
// the ARC pages move out of used.
if memStats.Total >= usedBytes+cacheBytes {
@ -135,6 +132,20 @@ func Collect(ctx context.Context, diskExclude []string) (Snapshot, error) {
return snapshot, nil
}
func memoryUsagePercent(usedBytes, totalBytes uint64) float64 {
if totalBytes == 0 {
return 0
}
percent := float64(usedBytes) / float64(totalBytes) * 100.0
if percent < 0 {
return 0
}
if percent > 100 {
return 100
}
return percent
}
func collectCPUUsage(ctx context.Context) (float64, error) {
percentages, err := cpuPercent(ctx, time.Second, false)
if err != nil {

View file

@ -148,3 +148,43 @@ func TestCollectSplitsReclaimableCache(t *testing.T) {
t.Fatalf("used+cache+free = %d exceeds total %d", sum, snapshot.Memory.TotalBytes)
}
}
func TestCollectDerivesMemoryPressureFromAvailableMemory(t *testing.T) {
origVirtualMemory := virtualMemory
t.Cleanup(func() { virtualMemory = origVirtualMemory })
const gib = uint64(1024 * 1024 * 1024)
virtualMemory = func(ctx context.Context) (*gomem.VirtualMemoryStat, error) {
return &gomem.VirtualMemoryStat{
Total: 16 * gib,
// Some Linux/gopsutil paths can report Used as cache-inclusive. The
// operator-facing pressure value should follow MemAvailable instead.
Used: 15 * gib,
Free: 1 * gib,
Available: 8 * gib,
UsedPercent: 93.75,
}, nil
}
snapshot, err := Collect(context.Background(), nil)
if err != nil {
t.Fatalf("Collect failed: %v", err)
}
if got, want := snapshot.Memory.UsedBytes, int64(8*gib); got != want {
t.Fatalf("UsedBytes = %d, want %d", got, want)
}
if got, want := snapshot.Memory.CacheBytes, int64(7*gib); got != want {
t.Fatalf("CacheBytes = %d, want %d", got, want)
}
if got, want := snapshot.Memory.FreeBytes, int64(1*gib); got != want {
t.Fatalf("FreeBytes = %d, want %d", got, want)
}
if got, want := snapshot.Memory.Usage, 50.0; got != want {
t.Fatalf("Usage = %.2f, want %.2f", got, want)
}
sum := snapshot.Memory.UsedBytes + snapshot.Memory.CacheBytes + snapshot.Memory.FreeBytes
if got, want := sum, snapshot.Memory.TotalBytes; got != want {
t.Fatalf("used+cache+free = %d, want total %d", got, want)
}
}

View file

@ -20,6 +20,8 @@ import (
"github.com/rs/zerolog/log"
)
const pbsBackupSnapshotFetchWorkers = 5
func pveBackupTemplateSubjectKey(instance, guestType, node string, vmid int) string {
return alerts.BuildBackupPVETemplateSubjectKey(instance, guestType, node, vmid)
}
@ -1630,52 +1632,70 @@ func (m *Monitor) fetchPBSBackupSnapshots(ctx context.Context, client *pbs.Clien
return nil
}
results := make(chan []models.PBSBackup, len(requests))
var wg sync.WaitGroup
sem := make(chan struct{}, 5)
workerCount := pbsBackupSnapshotFetchWorkers
if len(requests) < workerCount {
workerCount = len(requests)
}
for _, req := range requests {
req := req
jobs := make(chan pbsBackupFetchRequest)
results := make(chan []models.PBSBackup, workerCount)
var wg sync.WaitGroup
for i := 0; i < workerCount; i++ {
wg.Add(1)
go func() {
defer wg.Done()
select {
case sem <- struct{}{}:
case <-ctx.Done():
return
}
defer func() { <-sem }()
log.Debug().
Str("instance", instanceName).
Str("datastore", req.datastore).
Str("namespace", req.namespace).
Str("type", req.group.BackupType).
Str("id", req.group.BackupID).
Msg("Refreshing PBS backup group")
snapshots, err := client.ListBackupSnapshots(ctx, req.datastore, req.namespace, req.group.BackupType, req.group.BackupID)
if err != nil {
log.Error().
Err(err).
for req := range jobs {
log.Debug().
Str("instance", instanceName).
Str("datastore", req.datastore).
Str("namespace", req.namespace).
Str("type", req.group.BackupType).
Str("id", req.group.BackupID).
Msg("Failed to list PBS backup snapshots")
Msg("Refreshing PBS backup group")
if len(req.cached.snapshots) > 0 {
results <- req.cached.snapshots
snapshots, err := client.ListBackupSnapshots(ctx, req.datastore, req.namespace, req.group.BackupType, req.group.BackupID)
if err != nil {
log.Error().
Err(err).
Str("instance", instanceName).
Str("datastore", req.datastore).
Str("namespace", req.namespace).
Str("type", req.group.BackupType).
Str("id", req.group.BackupID).
Msg("Failed to list PBS backup snapshots")
if len(req.cached.snapshots) > 0 {
select {
case results <- req.cached.snapshots:
case <-ctx.Done():
}
}
continue
}
return
}
results <- convertPBSSnapshots(instanceName, req.datastore, req.namespace, snapshots)
backups := convertPBSSnapshots(instanceName, req.datastore, req.namespace, snapshots)
select {
case results <- backups:
case <-ctx.Done():
return
}
}
}()
}
go func() {
defer close(jobs)
for _, req := range requests {
select {
case jobs <- req:
case <-ctx.Done():
return
}
}
}()
go func() {
wg.Wait()
close(results)

View file

@ -3,12 +3,18 @@ package monitoring
import (
"context"
"fmt"
"net/http"
"net/http/httptest"
"strconv"
"strings"
"sync/atomic"
"testing"
"time"
"github.com/rcourtman/pulse-go-rewrite/internal/models"
proxmoxmapper "github.com/rcourtman/pulse-go-rewrite/internal/recovery/mapper/proxmox"
"github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources"
"github.com/rcourtman/pulse-go-rewrite/pkg/pbs"
pveapi "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox"
)
@ -157,6 +163,73 @@ func TestMonitorCalculateBackupOperationTimeout_UsesCanonicalReadState(t *testin
}
}
func TestFetchPBSBackupSnapshotsUsesBoundedWorkerPool(t *testing.T) {
t.Parallel()
const requestCount = 40
var active int64
var maxActive int64
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if strings.Contains(r.URL.Path, "/admin/datastore/archive/snapshots") {
current := atomic.AddInt64(&active, 1)
defer atomic.AddInt64(&active, -1)
for {
previous := atomic.LoadInt64(&maxActive)
if current <= previous || atomic.CompareAndSwapInt64(&maxActive, previous, current) {
break
}
}
time.Sleep(10 * time.Millisecond)
backupID := r.URL.Query().Get("backup-id")
_, _ = w.Write([]byte(fmt.Sprintf(
`{"data":[{"backup-type":"vm","backup-id":%q,"backup-time":1700000000}]}`,
backupID,
)))
return
}
http.NotFound(w, r)
}))
defer server.Close()
client, err := pbs.NewClient(pbs.ClientConfig{
Host: server.URL,
TokenName: "root@pam!token",
TokenValue: "secret",
})
if err != nil {
t.Fatalf("failed to create PBS client: %v", err)
}
requests := make([]pbsBackupFetchRequest, 0, requestCount)
for i := 0; i < requestCount; i++ {
backupID := strconv.Itoa(1000 + i)
requests = append(requests, pbsBackupFetchRequest{
datastore: "archive",
group: pbs.BackupGroup{
BackupType: "vm",
BackupID: backupID,
LastBackup: 1700000000,
BackupCount: 1,
},
})
}
m := &Monitor{}
backups := m.fetchPBSBackupSnapshots(context.Background(), client, "pbs1", requests)
if len(backups) != requestCount {
t.Fatalf("expected %d fetched backups, got %d", requestCount, len(backups))
}
if got := atomic.LoadInt64(&maxActive); got > int64(pbsBackupSnapshotFetchWorkers) {
t.Fatalf("expected at most %d concurrent PBS snapshot fetches, saw %d", pbsBackupSnapshotFetchWorkers, got)
}
}
func TestMonitorPollGuestSnapshots_UsesCanonicalReadState(t *testing.T) {
m := &Monitor{
state: models.NewState(),

View file

@ -402,10 +402,7 @@ func Run(ctx context.Context, version string) error {
// Initialize API server with reload function
var router *api.Router
reloadFunc := func() error {
if err := reloadableMonitor.Reload(); err != nil {
return err
}
refreshRouterAfterMonitorReload := func() {
if router != nil {
router.SetMonitor(reloadableMonitor.GetMonitor())
router.SetMultiTenantMonitor(reloadableMonitor.GetMultiTenantMonitor())
@ -418,6 +415,12 @@ func Run(ctx context.Context, version string) error {
// are lost on every monitor reload triggered by auto-registration.
router.ReloadSystemSettings()
}
}
reloadFunc := func() error {
if err := reloadableMonitor.Reload(); err != nil {
return err
}
refreshRouterAfterMonitorReload()
return nil
}
router = api.NewRouter(cfg, reloadableMonitor.GetMonitor(), reloadableMonitor.GetMultiTenantMonitor(), wsHub, reloadFunc, version)
@ -627,12 +630,8 @@ func Run(ctx context.Context, version string) error {
log.Info().Msg(".env mock settings changed, reloading monitor")
if err := reloadableMonitor.Reload(); err != nil {
log.Error().Err(err).Msg("Failed to reload monitor after .env mock setting change")
} else if router != nil {
router.SetMonitor(reloadableMonitor.GetMonitor())
router.SetMultiTenantMonitor(reloadableMonitor.GetMultiTenantMonitor())
if cfg := reloadableMonitor.GetConfig(); cfg != nil {
router.SetConfig(cfg)
}
} else {
refreshRouterAfterMonitorReload()
}
})