diff --git a/server/service-operation/monitoring/checker.go b/server/service-operation/monitoring/checker.go index a7c4a7a7..d1252484 100644 --- a/server/service-operation/monitoring/checker.go +++ b/server/service-operation/monitoring/checker.go @@ -2,6 +2,7 @@ package monitoring import ( + "errors" "log" "strings" "time" @@ -12,6 +13,25 @@ import ( "service-operation/types" ) +const ( + // defaultCheckTimeout is the per-attempt timeout for a service check. + defaultCheckTimeout = 10 * time.Second + + // defaultMaxRetries matches the UI default ("3 attempts") and the + // legacy frontend checker behavior (service.max_retries || 3). + defaultMaxRetries = 3 + + // maxRetriesCap bounds user-provided values so one check cycle can + // never block a service monitor loop for an excessive time. + maxRetriesCap = 10 + + // retryDelay is the pause between consecutive attempts. + retryDelay = 2 * time.Second +) + +// errUnsupportedServiceType marks service types the checker cannot handle. +var errUnsupportedServiceType = errors.New("unsupported service type") + func (ms *MonitoringService) performCheck(service pocketbase.Service) { // First, fetch the latest service status from PocketBase to ensure we have current data latestService, err := ms.pbClient.GetService(service.ID) @@ -25,63 +45,22 @@ func (ms *MonitoringService) performCheck(service pocketbase.Service) { return // Silently skip paused services } - timeout := 10 * time.Second // Default timeout - var result *types.OperationResult - - serviceType := strings.ToLower(latestService.ServiceType) - - // Single log message for check start - //log.Printf("Checking %s (%s)", latestService.Name, serviceType) - - switch serviceType { - case "ping", "icmp": - pingOp := operations.NewPingOperation(timeout) - host := latestService.Host - if host == "" { - host = latestService.URL - } - result, err = pingOp.Execute(host, 1) // Single ping for monitoring - - case "dns": - dnsOp := operations.NewDNSOperation(timeout) - host := latestService.Host - if host == "" { - host = latestService.Domain - } - // Default to A record, but could be made configurable - queryType := "A" - result, err = dnsOp.Execute(host, queryType) - - case "tcp": - tcpOp := operations.NewTCPOperation(timeout) - host := latestService.Host - if host == "" { - host = latestService.URL - } - port := latestService.Port - if port <= 0 { - port = 80 // Default port - } - result, err = tcpOp.Execute(host, port) - - case "http", "https": - httpOp := operations.NewHTTPOperation(timeout) - url := latestService.URL - if url == "" { - url = latestService.Host - } - result, err = httpOp.Execute(url, "GET") - - default: + // Honor the per-service retry configuration (services.max_retries). + // The service is only marked down after ALL attempts fail, so one + // slow or dropped probe no longer flaps the service to "down". + maxRetries := normalizeMaxRetries(latestService.MaxRetries) + + result, err := runCheckAttempts(latestService, maxRetries, defaultCheckTimeout, retryDelay) + if errors.Is(err, errUnsupportedServiceType) { log.Printf("Unknown service type: %s for service %s", latestService.ServiceType, latestService.Name) return } - // Determine status based on result + // Determine status based on the final attempt status := "down" errorMessage := "" responseTime := int64(0) - + if err != nil { errorMessage = err.Error() log.Printf("❌ %s failed: %v", latestService.Name, err) @@ -104,7 +83,7 @@ func (ms *MonitoringService) performCheck(service pocketbase.Service) { log.Printf("Failed to verify service status before update for %s: %v", latestService.Name, err) return } - + if currentService.Status == "paused" { return // Silently skip status update for paused services } @@ -120,4 +99,91 @@ func (ms *MonitoringService) performCheck(service pocketbase.Service) { metricsSaver := savers.NewMetricsSaverWithRegion(ms.pbClient, regionName, agentID) metricsSaver.SaveMetricsForService(*latestService, result) } -} \ No newline at end of file +} + +// normalizeMaxRetries clamps the configured max_retries to a sane range, +// falling back to the default when the field is unset (0) or invalid. +func normalizeMaxRetries(configured int) int { + if configured <= 0 { + return defaultMaxRetries + } + if configured > maxRetriesCap { + return maxRetriesCap + } + return configured +} + +// runCheckAttempts executes up to maxRetries check attempts for the service, +// waiting delay between attempts. It returns as soon as one attempt succeeds; +// otherwise it returns the outcome of the final attempt. +func runCheckAttempts(service *pocketbase.Service, maxRetries int, timeout, delay time.Duration) (*types.OperationResult, error) { + var result *types.OperationResult + var err error + + for attempt := 1; attempt <= maxRetries; attempt++ { + result, err = executeCheck(service, timeout) + if errors.Is(err, errUnsupportedServiceType) { + return nil, err + } + + if err == nil && result != nil && result.Success { + if attempt > 1 { + log.Printf("✅ %s succeeded on attempt %d/%d", service.Name, attempt, maxRetries) + } + return result, nil + } + + if attempt < maxRetries { + log.Printf("⚠️ %s check failed (attempt %d/%d), retrying in %s", service.Name, attempt, maxRetries, delay) + time.Sleep(delay) + } + } + + return result, err +} + +// executeCheck runs a single check attempt for the service. It is a plain +// function (it needs no MonitoringService state) so tests can call it directly. +func executeCheck(service *pocketbase.Service, timeout time.Duration) (*types.OperationResult, error) { + switch strings.ToLower(service.ServiceType) { + case "ping", "icmp": + pingOp := operations.NewPingOperation(timeout) + host := service.Host + if host == "" { + host = service.URL + } + return pingOp.Execute(host, 1) // Single ping for monitoring + + case "dns": + dnsOp := operations.NewDNSOperation(timeout) + host := service.Host + if host == "" { + host = service.Domain + } + // Default to A record, but could be made configurable + queryType := "A" + return dnsOp.Execute(host, queryType) + + case "tcp": + tcpOp := operations.NewTCPOperation(timeout) + host := service.Host + if host == "" { + host = service.URL + } + port := service.Port + if port <= 0 { + port = 80 // Default port + } + return tcpOp.Execute(host, port) + + case "http", "https": + httpOp := operations.NewHTTPOperation(timeout) + url := service.URL + if url == "" { + url = service.Host + } + return httpOp.Execute(url, "GET") + } + + return nil, errUnsupportedServiceType +} diff --git a/server/service-operation/monitoring/checker_test.go b/server/service-operation/monitoring/checker_test.go new file mode 100644 index 00000000..9b2b832f --- /dev/null +++ b/server/service-operation/monitoring/checker_test.go @@ -0,0 +1,130 @@ +package monitoring + +import ( + "errors" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "service-operation/pocketbase" +) + +func TestNormalizeMaxRetries(t *testing.T) { + tests := []struct { + name string + configured int + want int + }{ + {"unset falls back to default", 0, defaultMaxRetries}, + {"negative falls back to default", -2, defaultMaxRetries}, + {"single attempt kept as-is", 1, 1}, + {"ui maximum kept as-is", 5, 5}, + {"excessive value clamped to cap", 100, maxRetriesCap}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := normalizeMaxRetries(tt.configured); got != tt.want { + t.Errorf("normalizeMaxRetries(%d) = %d, want %d", tt.configured, got, tt.want) + } + }) + } +} + +// flakyServer returns a test server that fails with HTTP 500 for the first +// failures requests and responds 200 afterwards, plus a hit counter. +func flakyServer(failures int64) (*httptest.Server, *int64) { + var hits int64 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + n := atomic.AddInt64(&hits, 1) + if n <= failures { + w.WriteHeader(http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusOK) + })) + return server, &hits +} + +func TestRunCheckAttemptsRecoversWithinRetryBudget(t *testing.T) { + server, hits := flakyServer(2) + defer server.Close() + + service := &pocketbase.Service{ + Name: "flaky", + ServiceType: "http", + URL: server.URL, + } + + result, err := runCheckAttempts(service, 3, 5*time.Second, 10*time.Millisecond) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result == nil || !result.Success { + t.Fatalf("expected success after retries, got result=%+v", result) + } + if got := atomic.LoadInt64(hits); got != 3 { + t.Errorf("expected 3 attempts, server saw %d", got) + } +} + +func TestRunCheckAttemptsSingleAttemptStaysSingleShot(t *testing.T) { + server, hits := flakyServer(1) + defer server.Close() + + service := &pocketbase.Service{ + Name: "single-shot", + ServiceType: "http", + URL: server.URL, + } + + result, err := runCheckAttempts(service, 1, 5*time.Second, 10*time.Millisecond) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result == nil || result.Success { + t.Fatalf("expected failure with a single attempt, got result=%+v", result) + } + if got := atomic.LoadInt64(hits); got != 1 { + t.Errorf("expected exactly 1 attempt, server saw %d", got) + } +} + +func TestRunCheckAttemptsMarksDownAfterAllAttemptsFail(t *testing.T) { + server, hits := flakyServer(1000) + defer server.Close() + + service := &pocketbase.Service{ + Name: "always-down", + ServiceType: "http", + URL: server.URL, + } + + result, err := runCheckAttempts(service, 3, 5*time.Second, 10*time.Millisecond) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result == nil || result.Success { + t.Fatalf("expected failure after exhausting retries, got result=%+v", result) + } + if got := atomic.LoadInt64(hits); got != 3 { + t.Errorf("expected exactly 3 attempts, server saw %d", got) + } +} + +func TestExecuteCheckUnsupportedServiceType(t *testing.T) { + service := &pocketbase.Service{ + Name: "mystery", + ServiceType: "carrier-pigeon", + } + + if _, err := executeCheck(service, time.Second); !errors.Is(err, errUnsupportedServiceType) { + t.Fatalf("expected errUnsupportedServiceType, got %v", err) + } + + if _, err := runCheckAttempts(service, 3, time.Second, time.Millisecond); !errors.Is(err, errUnsupportedServiceType) { + t.Fatalf("expected errUnsupportedServiceType from runCheckAttempts, got %v", err) + } +}