diff --git a/backend/internal/services/admin_factory.go b/backend/internal/services/admin_factory.go index 960567f..043ff90 100644 --- a/backend/internal/services/admin_factory.go +++ b/backend/internal/services/admin_factory.go @@ -39,8 +39,14 @@ type AdminServiceResult struct { APIVersion string } +// errProbeNotFound means the probed route returned 404. Garage v2.x also serves +// /v1/health, so a 404 on /v2 is the only reliable "this is a v1 server" signal. +var errProbeNotFound = errors.New("probe endpoint not found") + func NewAdminService(cfg *config.GarageConfig, logLevel string) (*AdminServiceResult, error) { - if err := probeEndpoint(cfg, "/v2/GetClusterHealth"); err == nil { + // retry so a startup fails doesn't lock us to a v1 client. + err := probeEndpointWithRetry(cfg, "/v2/GetClusterHealth") + if err == nil { logger.Info().Str("api_version", "v2").Msg("Detected Garage admin API v2") svc := NewGarageV2AdminService(cfg, logLevel) return &AdminServiceResult{ @@ -50,7 +56,17 @@ func NewAdminService(cfg *config.GarageConfig, logLevel string) (*AdminServiceRe }, nil } - if err := probeEndpoint(cfg, "/v1/health"); err == nil { + // only fall back to v1 on a real 404 + // other errors mean the server is up but the probe failed transiently; picking v1 against + // a v2.x server breaks /v1/status with "v1/ endpoint is no longer supported" (issue #78). + if !errors.Is(err, errProbeNotFound) { + return nil, fmt.Errorf( + "could not detect Garage admin API version at %s: %w. Ensure Garage v1.1+ is running and the admin API is reachable", + cfg.AdminEndpoint, err, + ) + } + + if err := probeEndpointWithRetry(cfg, "/v1/health"); err == nil { logger.Info(). Str("api_version", "v1"). Msg("Detected Garage admin API v1 — cluster statistics and per-node details will be unavailable") @@ -68,6 +84,26 @@ func NewAdminService(cfg *config.GarageConfig, logLevel string) (*AdminServiceRe ) } +const probeAttempts = 4 + +// probeEndpointWithRetry retries transient failures with backoff, but returns a +// 404 immediately, a missing route won't appear on a retry. +func probeEndpointWithRetry(cfg *config.GarageConfig, path string) error { + var err error + backoff := 250 * time.Millisecond + for attempt := range probeAttempts { + err = probeEndpoint(cfg, path) + if err == nil || errors.Is(err, errProbeNotFound) { + return err + } + if attempt < probeAttempts-1 { + time.Sleep(backoff) + backoff *= 2 + } + } + return err +} + func probeEndpoint(cfg *config.GarageConfig, path string) error { session := azuretls.NewSession() ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) @@ -86,6 +122,9 @@ func probeEndpoint(cfg *config.GarageConfig, path string) error { } defer resp.RawBody.Close() + if resp.StatusCode == http.StatusNotFound { + return fmt.Errorf("probe %s returned status 404: %w", path, errProbeNotFound) + } if resp.StatusCode < 200 || resp.StatusCode >= 300 { return fmt.Errorf("probe %s returned status %d", path, resp.StatusCode) } diff --git a/backend/internal/services/admin_factory_test.go b/backend/internal/services/admin_factory_test.go index c0bdc02..0c288f1 100644 --- a/backend/internal/services/admin_factory_test.go +++ b/backend/internal/services/admin_factory_test.go @@ -89,3 +89,61 @@ func TestDetectVersion_Unreachable(t *testing.T) { t.Fatal("expected error for unreachable server") } } + +// Garage v2.x serves /v1/health too, so a transient failure of the /v2 probe +// must not cause a permanent downgrade to the (broken on v2.x) v1 client. +// Regression test for https://github.com/Noooste/garage-ui/issues/78 +func TestDetectVersion_V2_TransientProbeFailure(t *testing.T) { + var v2Hits int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v2/GetClusterHealth": + v2Hits++ + if v2Hits < 3 { // fail the first two attempts, then recover + w.WriteHeader(http.StatusServiceUnavailable) + return + } + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"status":"healthy"}`)) + case "/v1/health": // v2.x still answers this + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"status":"healthy"}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + + cfg := &config.GarageConfig{AdminEndpoint: srv.URL, AdminToken: "tok"} + result, err := NewAdminService(cfg, "") + if err != nil { + t.Fatal(err) + } + if result.APIVersion != "v2" { + t.Fatalf("expected v2 after transient probe failure, got %s", result.APIVersion) + } +} + +// A server that answers /v1/health but returns a server error (not 404) for +// /v2/GetClusterHealth must NOT be detected as v1, because v2.x servers also +// answer /v1/health. Falling through to v1 here is the issue #78 misdetection. +func TestDetectVersion_DoesNotDowngradeOnV2ServerError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v2/GetClusterHealth": + w.WriteHeader(http.StatusServiceUnavailable) + case "/v1/health": + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"status":"healthy"}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + + cfg := &config.GarageConfig{AdminEndpoint: srv.URL, AdminToken: "tok"} + result, err := NewAdminService(cfg, "") + if err == nil && result.APIVersion == "v1" { + t.Fatal("must not downgrade to v1 when /v2 returns a server error; v2.x also serves /v1/health") + } +}