diff --git a/internal/monitoring/docker_commands_test.go b/internal/monitoring/docker_commands_test.go index 75d9704..00b61c4 100644 --- a/internal/monitoring/docker_commands_test.go +++ b/internal/monitoring/docker_commands_test.go @@ -719,6 +719,311 @@ func TestQueueDockerStopCommand(t *testing.T) { }) } +func TestGetDockerCommandPayload(t *testing.T) { + t.Parallel() + + t.Run("empty hostID returns nil", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + + payload, status := monitor.getDockerCommandPayload("") + if payload != nil { + t.Fatalf("expected nil payload for empty hostID, got %v", payload) + } + if status != nil { + t.Fatalf("expected nil status for empty hostID, got %v", status) + } + }) + + t.Run("whitespace-only hostID returns nil", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + + payload, status := monitor.getDockerCommandPayload(" ") + if payload != nil { + t.Fatalf("expected nil payload for whitespace hostID, got %v", payload) + } + if status != nil { + t.Fatalf("expected nil status for whitespace hostID, got %v", status) + } + }) + + t.Run("host not found in dockerCommands returns nil", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + + payload, status := monitor.getDockerCommandPayload("nonexistent-host") + if payload != nil { + t.Fatalf("expected nil payload for nonexistent host, got %v", payload) + } + if status != nil { + t.Fatalf("expected nil status for nonexistent host, got %v", status) + } + }) + + t.Run("expired command is cleaned up and returns nil", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + + host := models.DockerHost{ + ID: "host-expired", + Hostname: "node-expired", + DisplayName: "node-expired", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + // Queue a command + cmdStatus, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("queue stop command: %v", err) + } + + // Manually expire the command + monitor.mu.Lock() + cmd := monitor.dockerCommands[host.ID] + past := time.Now().Add(-2 * dockerCommandDefaultTTL) + cmd.status.ExpiresAt = &past + monitor.mu.Unlock() + + // Fetch should return nil and clean up + payload, status := monitor.getDockerCommandPayload(host.ID) + if payload != nil { + t.Fatalf("expected nil payload for expired command, got %v", payload) + } + if status != nil { + t.Fatalf("expected nil status for expired command, got %v", status) + } + + // Verify cleanup + monitor.mu.Lock() + _, commandExists := monitor.dockerCommands[host.ID] + _, indexExists := monitor.dockerCommandIndex[cmdStatus.ID] + monitor.mu.Unlock() + + if commandExists { + t.Fatal("expected expired command to be removed from dockerCommands") + } + if indexExists { + t.Fatal("expected expired command to be removed from dockerCommandIndex") + } + + // Verify state was updated with expired status + hostState := findDockerHost(t, monitor, host.ID) + if hostState.Command == nil { + t.Fatal("expected host command to be set in state") + } + if hostState.Command.Status != DockerCommandStatusExpired { + t.Fatalf("expected expired status in state, got %q", hostState.Command.Status) + } + if hostState.Command.FailureReason == "" { + t.Fatal("expected failure reason to be set on expired command") + } + }) + + t.Run("queued command is marked as dispatched", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + + host := models.DockerHost{ + ID: "host-queued", + Hostname: "node-queued", + DisplayName: "node-queued", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + // Queue a command + _, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("queue stop command: %v", err) + } + + // Verify command is queued + monitor.mu.Lock() + cmd := monitor.dockerCommands[host.ID] + if cmd.status.Status != DockerCommandStatusQueued { + t.Fatalf("expected queued status before fetch, got %q", cmd.status.Status) + } + monitor.mu.Unlock() + + // Fetch payload - should mark as dispatched + payload, status := monitor.getDockerCommandPayload(host.ID) + + // For stop command, payload is nil but status should be returned + if status == nil { + t.Fatal("expected status to be returned") + } + if status.Status != DockerCommandStatusDispatched { + t.Fatalf("expected dispatched status, got %q", status.Status) + } + if status.DispatchedAt == nil { + t.Fatal("expected DispatchedAt to be set") + } + + // Verify state was updated + hostState := findDockerHost(t, monitor, host.ID) + if hostState.Command == nil { + t.Fatal("expected host command to be set in state") + } + if hostState.Command.Status != DockerCommandStatusDispatched { + t.Fatalf("expected dispatched status in state, got %q", hostState.Command.Status) + } + + // payload is nil for stop commands (no additional data needed) + if payload != nil { + t.Fatalf("expected nil payload for stop command, got %v", payload) + } + }) + + t.Run("already dispatched command is not re-marked", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + + host := models.DockerHost{ + ID: "host-dispatched", + Hostname: "node-dispatched", + DisplayName: "node-dispatched", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + // Queue a command + _, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("queue stop command: %v", err) + } + + // First fetch - marks as dispatched + _, status1 := monitor.getDockerCommandPayload(host.ID) + if status1 == nil { + t.Fatal("expected status from first fetch") + } + firstDispatchedAt := status1.DispatchedAt + firstUpdatedAt := status1.UpdatedAt + + // Small delay to ensure time would change if re-marked + time.Sleep(time.Millisecond) + + // Second fetch - should NOT re-mark + _, status2 := monitor.getDockerCommandPayload(host.ID) + if status2 == nil { + t.Fatal("expected status from second fetch") + } + + // DispatchedAt should be the same + if status2.DispatchedAt == nil { + t.Fatal("expected DispatchedAt to still be set") + } + if !status2.DispatchedAt.Equal(*firstDispatchedAt) { + t.Fatalf("DispatchedAt changed: first=%v, second=%v", *firstDispatchedAt, *status2.DispatchedAt) + } + + // UpdatedAt should be the same (not re-updated) + if !status2.UpdatedAt.Equal(firstUpdatedAt) { + t.Fatalf("UpdatedAt changed: first=%v, second=%v", firstUpdatedAt, status2.UpdatedAt) + } + + // Status should still be dispatched + if status2.Status != DockerCommandStatusDispatched { + t.Fatalf("expected dispatched status, got %q", status2.Status) + } + }) + + t.Run("returns payload and status copy", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + + host := models.DockerHost{ + ID: "host-payload", + Hostname: "node-payload", + DisplayName: "node-payload", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + // Manually create a command with a payload + testPayload := map[string]any{ + "action": "test", + "count": 42, + } + cmd := newDockerHostCommand("test-type", "test message", dockerCommandDefaultTTL, testPayload) + + monitor.mu.Lock() + monitor.dockerCommands[host.ID] = &cmd + monitor.dockerCommandIndex[cmd.status.ID] = host.ID + monitor.mu.Unlock() + + // Fetch payload + payload, status := monitor.getDockerCommandPayload(host.ID) + + if payload == nil { + t.Fatal("expected payload to be returned") + } + if payload["action"] != "test" { + t.Fatalf("expected action 'test', got %v", payload["action"]) + } + if payload["count"] != 42 { + t.Fatalf("expected count 42, got %v", payload["count"]) + } + + if status == nil { + t.Fatal("expected status to be returned") + } + if status.Type != "test-type" { + t.Fatalf("expected type 'test-type', got %q", status.Type) + } + if status.Message != "test message" { + t.Fatalf("expected message 'test message', got %q", status.Message) + } + + // Verify it's a copy by modifying the returned status + status.Message = "modified" + monitor.mu.Lock() + originalCmd := monitor.dockerCommands[host.ID] + if originalCmd.status.Message == "modified" { + t.Fatal("modifying returned status should not affect original") + } + monitor.mu.Unlock() + }) + + t.Run("hostID with whitespace is normalized", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + + host := models.DockerHost{ + ID: "host-whitespace", + Hostname: "node-whitespace", + DisplayName: "node-whitespace", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + // Queue a command + _, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("queue stop command: %v", err) + } + + // Fetch with whitespace-padded ID + _, status := monitor.getDockerCommandPayload(" host-whitespace ") + if status == nil { + t.Fatal("expected status to be returned for whitespace-padded hostID") + } + if status.Status != DockerCommandStatusDispatched { + t.Fatalf("expected dispatched status, got %q", status.Status) + } + }) +} + func TestAcknowledgeDockerCommandErrorPaths(t *testing.T) { t.Parallel() diff --git a/internal/monitoring/metrics_history_test.go b/internal/monitoring/metrics_history_test.go index 9640bd8..afc235d 100644 --- a/internal/monitoring/metrics_history_test.go +++ b/internal/monitoring/metrics_history_test.go @@ -499,6 +499,190 @@ func TestGetGuestMetrics(t *testing.T) { } } +func TestGetGuestMetrics_AllMetricTypes(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + metricType string + value float64 + }{ + {name: "cpu metric", metricType: "cpu", value: 50.5}, + {name: "memory metric", metricType: "memory", value: 70.0}, + {name: "disk metric", metricType: "disk", value: 45.0}, + {name: "diskread metric", metricType: "diskread", value: 1024.0}, + {name: "diskwrite metric", metricType: "diskwrite", value: 512.0}, + {name: "netin metric", metricType: "netin", value: 2048.0}, + {name: "netout metric", metricType: "netout", value: 1536.0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mh := NewMetricsHistory(100, time.Hour) + mh.AddGuestMetric("vm-test", tt.metricType, tt.value, now.Add(-5*time.Minute)) + + result := mh.GetGuestMetrics("vm-test", tt.metricType, time.Hour) + + if len(result) != 1 { + t.Fatalf("len(result) = %d, want 1", len(result)) + } + if result[0].Value != tt.value { + t.Errorf("value = %v, want %v", result[0].Value, tt.value) + } + }) + } +} + +func TestGetGuestMetrics_GuestNotFound(t *testing.T) { + mh := NewMetricsHistory(100, time.Hour) + + // Add data for one guest + mh.AddGuestMetric("vm-100", "cpu", 50.0, time.Now()) + + // Query non-existent guest + result := mh.GetGuestMetrics("vm-nonexistent", "cpu", time.Hour) + + if len(result) != 0 { + t.Errorf("expected empty slice for non-existent guest, got %d elements", len(result)) + } + if result == nil { + t.Error("expected empty slice, got nil") + } +} + +func TestGetGuestMetrics_UnknownMetricType(t *testing.T) { + now := time.Now() + mh := NewMetricsHistory(100, time.Hour) + + // Add data for the guest + mh.AddGuestMetric("vm-100", "cpu", 50.0, now.Add(-5*time.Minute)) + mh.AddGuestMetric("vm-100", "memory", 70.0, now.Add(-5*time.Minute)) + + unknownTypes := []string{"invalid", "unknown", "foo", "bar", "CPU", "Memory", ""} + + for _, metricType := range unknownTypes { + t.Run("type_"+metricType, func(t *testing.T) { + result := mh.GetGuestMetrics("vm-100", metricType, time.Hour) + + if len(result) != 0 { + t.Errorf("expected empty slice for unknown metric type %q, got %d elements", metricType, len(result)) + } + if result == nil { + t.Errorf("expected empty slice for unknown metric type %q, got nil", metricType) + } + }) + } +} + +func TestGetGuestMetrics_DurationFiltering(t *testing.T) { + now := time.Now() + mh := NewMetricsHistory(100, 2*time.Hour) + + // Add points at different times + mh.AddGuestMetric("vm-100", "cpu", 10.0, now.Add(-90*time.Minute)) // old + mh.AddGuestMetric("vm-100", "cpu", 20.0, now.Add(-60*time.Minute)) // old + mh.AddGuestMetric("vm-100", "cpu", 30.0, now.Add(-30*time.Minute)) // recent + mh.AddGuestMetric("vm-100", "cpu", 40.0, now.Add(-15*time.Minute)) // recent + mh.AddGuestMetric("vm-100", "cpu", 50.0, now.Add(-5*time.Minute)) // recent + + tests := []struct { + name string + duration time.Duration + wantLen int + wantFirst float64 + wantLast float64 + }{ + { + name: "all points within 2 hours", + duration: 2 * time.Hour, + wantLen: 5, + wantFirst: 10.0, + wantLast: 50.0, + }, + { + name: "points within 45 minutes", + duration: 45 * time.Minute, + wantLen: 3, + wantFirst: 30.0, + wantLast: 50.0, + }, + { + name: "points within 20 minutes", + duration: 20 * time.Minute, + wantLen: 2, + wantFirst: 40.0, + wantLast: 50.0, + }, + { + name: "points within 10 minutes", + duration: 10 * time.Minute, + wantLen: 1, + wantFirst: 50.0, + wantLast: 50.0, + }, + { + name: "zero duration excludes all", + duration: 0, + wantLen: 0, + }, + { + name: "very short duration excludes all", + duration: 1 * time.Minute, + wantLen: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := mh.GetGuestMetrics("vm-100", "cpu", tt.duration) + + if len(result) != tt.wantLen { + t.Errorf("len(result) = %d, want %d", len(result), tt.wantLen) + } + if len(result) > 0 { + if result[0].Value != tt.wantFirst { + t.Errorf("first value = %v, want %v", result[0].Value, tt.wantFirst) + } + if result[len(result)-1].Value != tt.wantLast { + t.Errorf("last value = %v, want %v", result[len(result)-1].Value, tt.wantLast) + } + } + }) + } +} + +func TestGetGuestMetrics_EmptyMetricsData(t *testing.T) { + mh := NewMetricsHistory(100, time.Hour) + + // Directly populate an empty guest metrics entry + mh.mu.Lock() + mh.guestMetrics["vm-empty"] = &GuestMetrics{ + CPU: []MetricPoint{}, + Memory: []MetricPoint{}, + Disk: []MetricPoint{}, + DiskRead: []MetricPoint{}, + DiskWrite: []MetricPoint{}, + NetworkIn: []MetricPoint{}, + NetworkOut: []MetricPoint{}, + } + mh.mu.Unlock() + + metricTypes := []string{"cpu", "memory", "disk", "diskread", "diskwrite", "netin", "netout"} + + for _, metricType := range metricTypes { + t.Run(metricType, func(t *testing.T) { + result := mh.GetGuestMetrics("vm-empty", metricType, time.Hour) + + if len(result) != 0 { + t.Errorf("expected empty slice for empty %s metrics, got %d elements", metricType, len(result)) + } + if result == nil { + t.Errorf("expected empty slice for empty %s metrics, got nil", metricType) + } + }) + } +} + func TestGetNodeMetrics(t *testing.T) { now := time.Now() mh := NewMetricsHistory(100, time.Hour) diff --git a/internal/monitoring/staleness_tracker_test.go b/internal/monitoring/staleness_tracker_test.go index 082040d..f37a384 100644 --- a/internal/monitoring/staleness_tracker_test.go +++ b/internal/monitoring/staleness_tracker_test.go @@ -491,3 +491,190 @@ func TestTrackerKey(t *testing.T) { } } } + +func TestStalenessTracker_MergeSnapshot_TableDriven(t *testing.T) { + baseTime := time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC) + olderTime := baseTime.Add(-10 * time.Second) + newerTime := baseTime.Add(10 * time.Second) + + tests := []struct { + name string + existing *FreshnessSnapshot // nil means no existing entry + merge FreshnessSnapshot + wantLastSuccess time.Time + wantLastError time.Time + wantLastMutated time.Time + wantChangeHash string + }{ + { + name: "merge into non-existent entry creates new entry", + existing: nil, + merge: FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "new-instance", + LastSuccess: baseTime, + LastError: baseTime, + LastMutated: baseTime, + ChangeHash: "abc123", + }, + wantLastSuccess: baseTime, + wantLastError: baseTime, + wantLastMutated: baseTime, + wantChangeHash: "abc123", + }, + { + name: "newer LastSuccess updates existing", + existing: &FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastSuccess: baseTime, + }, + merge: FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastSuccess: newerTime, + }, + wantLastSuccess: newerTime, + }, + { + name: "older LastSuccess does not update existing", + existing: &FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastSuccess: baseTime, + }, + merge: FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastSuccess: olderTime, + }, + wantLastSuccess: baseTime, + }, + { + name: "newer LastError updates existing", + existing: &FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastError: baseTime, + }, + merge: FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastError: newerTime, + }, + wantLastError: newerTime, + }, + { + name: "older LastError does not update existing", + existing: &FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastError: baseTime, + }, + merge: FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastError: olderTime, + }, + wantLastError: baseTime, + }, + { + name: "newer LastMutated updates existing", + existing: &FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastMutated: baseTime, + }, + merge: FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastMutated: newerTime, + }, + wantLastMutated: newerTime, + }, + { + name: "older LastMutated does not update existing", + existing: &FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastMutated: baseTime, + }, + merge: FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + LastMutated: olderTime, + }, + wantLastMutated: baseTime, + }, + { + name: "non-empty ChangeHash updates existing", + existing: &FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + ChangeHash: "old-hash", + }, + merge: FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + ChangeHash: "new-hash", + }, + wantChangeHash: "new-hash", + }, + { + name: "empty ChangeHash does not overwrite existing", + existing: &FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + ChangeHash: "existing-hash", + }, + merge: FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "test", + ChangeHash: "", + }, + wantChangeHash: "existing-hash", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tracker := NewStalenessTracker(nil) + + // Set up existing entry if specified + if tt.existing != nil { + tracker.setSnapshot(*tt.existing) + } + + // Perform merge + tracker.mergeSnapshot(tt.merge) + + // Get result + snap, ok := tracker.snapshot(tt.merge.InstanceType, tt.merge.Instance) + if !ok { + t.Fatal("snapshot not found after merge") + } + + // Verify instance metadata is always set + if snap.InstanceType != tt.merge.InstanceType { + t.Errorf("InstanceType = %v, want %v", snap.InstanceType, tt.merge.InstanceType) + } + if snap.Instance != tt.merge.Instance { + t.Errorf("Instance = %q, want %q", snap.Instance, tt.merge.Instance) + } + + // Verify timestamps + if !tt.wantLastSuccess.IsZero() && !snap.LastSuccess.Equal(tt.wantLastSuccess) { + t.Errorf("LastSuccess = %v, want %v", snap.LastSuccess, tt.wantLastSuccess) + } + if !tt.wantLastError.IsZero() && !snap.LastError.Equal(tt.wantLastError) { + t.Errorf("LastError = %v, want %v", snap.LastError, tt.wantLastError) + } + if !tt.wantLastMutated.IsZero() && !snap.LastMutated.Equal(tt.wantLastMutated) { + t.Errorf("LastMutated = %v, want %v", snap.LastMutated, tt.wantLastMutated) + } + if tt.wantChangeHash != "" && snap.ChangeHash != tt.wantChangeHash { + t.Errorf("ChangeHash = %q, want %q", snap.ChangeHash, tt.wantChangeHash) + } + }) + } +}