Keep a backend whose probe path is wrong in service

A backend that 404s on the health probe path but serves queries fine was
marked down and excluded from every fan-out for good, since the fail-open
only triggers when no backend is left healthy.

Classify a probe reply that refuses the request itself - any 4xx, plus 501 -
as evidence about the probe, not the backend. Such a backend keeps serving
queries and reports the distinct probe_unsupported state on /healthz.
Transport failures and 5xx, 503 included, still mark a backend down.
Log the misconfiguration once per transition with the backend, probe path
and status. Track failure runs per outcome kind so a 404 run and a 503 run
never add up to one threshold.

README.md

@@ -228,19 +228,36 @@ not answering.
   or `unknown` counts as a failure. A body that is not in that shape is judged on
   its status code alone, so pointing `health_probe_path` at some other endpoint
   still works.
+- **A refused probe is not a sick backend.** Only evidence about the *backend*
+  takes it out of the pool: a transport failure (connection refused, DNS, TLS,
+  timeout) or a `5xx` — `503` included, since trapperkeeper answers `503` exactly
+  when its services are not nominal, so it stays a real failure. A reply that
+  refuses the *probe request* is evidence about the probe instead: every `4xx` is
+  the backend answering that our request is the problem (`404`/`410` the path is
+  not there, `405` it does not take a `GET`, `401`/`403` we are not allowed to
+  ask, `429` we asked too often), and `501` says it does not implement the
+  endpoint. A backend answering that way is **left in service** — still queried,
+  still contributing records — and reported as `probe_unsupported` rather than
+  `healthy`, so an operator can tell "verified healthy" from "not actually being
+  checked". A probe path that is wrong for one backend alone can therefore never
+  strand a working backend. The misconfiguration is logged once per transition,
+  naming the backend, the probe path and the status.
 - **Thresholds** — a healthy backend goes down after `health_probe_failures`
   (default 3) **consecutive** failures; a down one comes back after
-  `health_probe_successes` (default 2) consecutive successes. One blip cannot
-  flap a backend out, and one lucky reply cannot flap it back in. A down backend
-  keeps being probed, so recovery is automatic.
+  `health_probe_successes` (default 2) consecutive successes, and the same
+  thresholds gate `probe_unsupported` in and out. One blip cannot flap a backend
+  out, and one lucky reply cannot flap it back in. A run only counts while the
+  replies keep their kind, so a stretch of `404`s and a stretch of `503`s never
+  add up to one threshold. A down backend keeps being probed, so recovery is
+  automatic.
 - **Fails open** — if the prober has marked **every** backend down, `pdbmux`
   queries them all anyway. A wrong `health_probe_path`, a broken prober or a
   partition that only the prober sees can therefore never black-hole traffic;
   the worst case is today's behaviour.
 - **Serves immediately** — the listener never waits for a first probe round, and
   a backend nobody has probed yet counts as healthy, so a restart drops nothing.
-- **Quiet** — only *transitions* (up→down, down→up) are logged, never individual
-  probes.
+- **Quiet** — only *transitions* (up→down, down→up, in and out of
+  `probe_unsupported`) are logged, never individual probes.
 - **Partial responses stay partial.** Health state changes which backends are
   asked, never what a merged answer means: a response built from a subset is
   still served, as before. Every merged response carries `X-Backends:
@@ -248,7 +265,8 @@ not answering.
   a client can tell a full answer from a partial one. On a cache hit the header
   describes the stored body, not the current backend count.
 - **`/healthz`** gives each backend a `state` (`healthy`, `unhealthy`,
-  `unprobed`, or `unmonitored` when probing is off), `consecutive_failures`,
+  `probe_unsupported`, `unprobed`, or `unmonitored` when probing is off),
+  `consecutive_failures`,
   `consecutive_successes`, `last_probe` and `last_error`, alongside the
   `reachable` check `/healthz` runs itself — which always asks **every**
   backend, so a backend queries are skipping is still reported. A `query` object

health.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -19,36 +20,53 @@ const probeBodyLimit = 1 << 20
 const runningState = "running"
 
 const (
-	stateHealthy     = "healthy"
-	stateUnhealthy   = "unhealthy"
-	stateUnprobed    = "unprobed"
-	stateUnmonitored = "unmonitored"
+	stateHealthy          = "healthy"
+	stateUnhealthy        = "unhealthy"
+	stateUnprobed         = "unprobed"
+	stateUnmonitored      = "unmonitored"
+	stateProbeUnsupported = "probe_unsupported"
+)
+
+// probeOutcome is what one probe reply said. A rejected probe is deliberately
+// neither a success nor a failure: it is evidence about the probe request, not
+// about the backend behind it.
+type probeOutcome int
+
+const (
+	outcomeOK probeOutcome = iota
+	outcomeFailure
+	outcomeRejected
 )
 
 type probeState struct {
-	healthy   bool
-	probed    bool
-	failures  int
-	successes int
-	lastProbe time.Time
-	lastErr   string
+	healthy     bool
+	unsupported bool
+	probed      bool
+	outcome     probeOutcome
+	failures    int
+	successes   int
+	lastProbe   time.Time
+	lastErr     string
 }
 
 // backendHealth is a copy of one backend's probe state, safe to read outside the
 // prober's lock.
 type backendHealth struct {
-	Healthy   bool
-	Probed    bool
-	Failures  int
-	Successes int
-	LastProbe time.Time
-	LastErr   string
+	Healthy     bool
+	Unsupported bool
+	Probed      bool
+	Failures    int
+	Successes   int
+	LastProbe   time.Time
+	LastErr     string
 }
 
 func (h backendHealth) stateName() string {
 	switch {
 	case !h.Probed:
 		return stateUnprobed
+	case h.Unsupported:
+		return stateProbeUnsupported
 	case h.Healthy:
 		return stateHealthy
 	default:
@@ -56,6 +74,37 @@ func (h backendHealth) stateName() string {
 	}
 }
 
+// probeRejectedError marks a reply that refused the probe request itself rather
+// than reporting the backend unwell.
+type probeRejectedError struct {
+	status int
+	path   string
+}
+
+func (e *probeRejectedError) Error() string {
+	return fmt.Sprintf("HTTP %d: probe path %q is not usable on this backend", e.status, e.path)
+}
+
+// probeRejects reports whether a status code refuses the probe request rather
+// than reporting the service unhealthy. Every 4xx is the backend answering that
+// our request is the problem, and 501 says it does not implement the endpoint.
+// Other 5xx stay real evidence: trapperkeeper answers 503 exactly when its
+// services are not nominal.
+func probeRejects(code int) bool {
+	return (code >= 400 && code < 500) || code == http.StatusNotImplemented
+}
+
+func classifyProbe(err error) probeOutcome {
+	switch {
+	case err == nil:
+		return outcomeOK
+	case errors.As(err, new(*probeRejectedError)):
+		return outcomeRejected
+	default:
+		return outcomeFailure
+	}
+}
+
 // prober polls each backend's health endpoint on an interval and tracks whether
 // it is currently answering. A nil *prober means probing is disabled and every
 // method is a no-op reporting every backend healthy.
@@ -163,12 +212,13 @@ func (p *prober) snapshot() map[string]backendHealth {
 	out := make(map[string]backendHealth, len(p.states))
 	for name, st := range p.states {
 		out[name] = backendHealth{
-			Healthy:   st.healthy,
-			Probed:    st.probed,
-			Failures:  st.failures,
-			Successes: st.successes,
-			LastProbe: st.lastProbe,
-			LastErr:   st.lastErr,
+			Healthy:     st.healthy,
+			Unsupported: st.unsupported,
+			Probed:      st.probed,
+			Failures:    st.failures,
+			Successes:   st.successes,
+			LastProbe:   st.lastProbe,
+			LastErr:     st.lastErr,
 		}
 	}
 	return out
@@ -193,24 +243,49 @@ func (p *prober) record(name string, err error) {
 		p.mu.Unlock()
 		return
 	}
+	outcome := classifyProbe(err)
+	// A run only counts while the outcome keeps its kind, so a rejected-probe run
+	// and a real-failure run never add up to one threshold.
+	if outcome != st.outcome {
+		st.failures, st.successes = 0, 0
+	}
+	st.outcome = outcome
 	st.probed = true
 	st.lastProbe = p.now()
+
 	var transition string
-	if err != nil {
-		st.lastErr = err.Error()
-		st.successes = 0
-		st.failures++
-		if st.healthy && st.failures >= p.failures {
-			st.healthy = false
-			transition = fmt.Sprintf("warning: backend %q is down after %d consecutive probe failures: %v", name, st.failures, err)
-		}
-	} else {
+	switch outcome {
+	case outcomeOK:
 		st.lastErr = ""
-		st.failures = 0
 		st.successes++
-		if !st.healthy && st.successes >= p.successes {
-			st.healthy = true
-			transition = fmt.Sprintf("info: backend %q is up after %d consecutive probe successes", name, st.successes)
+		if st.successes >= p.successes {
+			switch {
+			case st.unsupported:
+				st.unsupported = false
+				transition = fmt.Sprintf("info: backend %q health probe %q is usable again", name, p.path)
+			case !st.healthy:
+				st.healthy = true
+				transition = fmt.Sprintf("info: backend %q is up after %d consecutive probe successes", name, st.successes)
+			}
+		}
+	case outcomeRejected:
+		st.lastErr = err.Error()
+		st.failures++
+		if !st.unsupported && st.failures >= p.failures {
+			// The backend answered and refused the probe request itself, so this says
+			// nothing about its health: it keeps serving queries, unverified.
+			st.unsupported, st.healthy = true, true
+			transition = fmt.Sprintf("warning: backend %q rejected the health probe %d times: %v; leaving it in service but unverified, fix health_probe_path or its authorization", name, st.failures, err)
+		}
+	default:
+		st.lastErr = err.Error()
+		st.failures++
+		if st.failures >= p.failures {
+			st.unsupported = false
+			if st.healthy {
+				st.healthy = false
+				transition = fmt.Sprintf("warning: backend %q is down after %d consecutive probe failures: %v", name, st.failures, err)
+			}
 		}
 	}
 	p.mu.Unlock()
@@ -236,6 +311,9 @@ func (p *prober) probe(ctx context.Context, b Backend) error {
 	if err != nil {
 		return err
 	}
+	if probeRejects(resp.StatusCode) {
+		return &probeRejectedError{status: resp.StatusCode, path: p.path}
+	}
 	if resp.StatusCode != http.StatusOK {
 		return fmt.Errorf("HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(body)))
 	}

health_test.go

@@ -86,6 +86,19 @@ func markDown(t *testing.T, p *prober, name string) {
 	}
 }
 
+func markUnsupported(t *testing.T, p *prober, name string) {
+	t.Helper()
+	for i := 0; i < p.failures; i++ {
+		p.record(name, &probeRejectedError{status: http.StatusNotFound, path: p.path})
+	}
+	if !p.healthy(name) {
+		t.Fatalf("backend %q was excluded by a rejected probe", name)
+	}
+	if state := p.snapshot()[name].stateName(); state != stateProbeUnsupported {
+		t.Fatalf("state = %q, want %q", state, stateProbeUnsupported)
+	}
+}
+
 func TestProber_FailureThresholdDebouncesABlip(t *testing.T) {
 	cfg := healthConfig("http://a.invalid", "http://b.invalid")
 	cfg.HealthProbeFailures = 3
@@ -488,6 +501,215 @@ func TestBackendsHeader_CountsContributors(t *testing.T) {
 	}
 }
 
+// A reply that refuses the probe request says nothing about the backend, so the
+// backend must keep serving queries instead of being stranded out of the pool.
+func TestProber_RejectedProbeKeepsBackendInService(t *testing.T) {
+	codes := []int{
+		http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden,
+		http.StatusNotFound, http.StatusMethodNotAllowed, http.StatusGone,
+		http.StatusTooManyRequests, http.StatusNotImplemented,
+	}
+	for _, code := range codes {
+		sb := newStatusBackend(t)
+		sb.reply(code, "nope")
+		p := newTestProber(t, healthConfig(sb.srv.URL, "http://b.invalid"))
+
+		err := p.probe(t.Context(), Backend{Name: "a", URL: sb.srv.URL})
+		if classifyProbe(err) != outcomeRejected {
+			t.Fatalf("HTTP %d classified as %v, want a rejected probe (err %v)", code, classifyProbe(err), err)
+		}
+		for i := 0; i < p.failures+3; i++ {
+			p.record("a", err)
+			if !p.healthy("a") {
+				t.Fatalf("HTTP %d excluded backend a after %d probes", code, i+1)
+			}
+		}
+		snap := p.snapshot()["a"]
+		if got := snap.stateName(); got != stateProbeUnsupported {
+			t.Errorf("HTTP %d state = %q, want %q", code, got, stateProbeUnsupported)
+		}
+		if !strings.Contains(snap.LastErr, defaultHealthProbePath) {
+			t.Errorf("HTTP %d last_error %q does not name the probe path", code, snap.LastErr)
+		}
+	}
+}
+
+// 503 is trapperkeeper's answer when its services are not nominal, so it stays a
+// real failure; so does anything that never got an HTTP reply at all.
+func TestProber_ServerErrorsAndTransportFailuresStillMarkDown(t *testing.T) {
+	for _, code := range []int{http.StatusInternalServerError, http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout} {
+		sb := newStatusBackend(t)
+		sb.reply(code, degradedStatusBody("error"))
+		p := newTestProber(t, healthConfig(sb.srv.URL, "http://b.invalid"))
+
+		err := p.probe(t.Context(), Backend{Name: "a", URL: sb.srv.URL})
+		if classifyProbe(err) != outcomeFailure {
+			t.Fatalf("HTTP %d classified as %v, want a real failure", code, classifyProbe(err))
+		}
+		for i := 0; i < p.failures; i++ {
+			p.record("a", err)
+		}
+		if p.healthy("a") {
+			t.Errorf("HTTP %d left backend a in service", code)
+		}
+		if got := p.snapshot()["a"].stateName(); got != stateUnhealthy {
+			t.Errorf("HTTP %d state = %q, want %q", code, got, stateUnhealthy)
+		}
+	}
+
+	dead := httptest.NewServer(http.NotFoundHandler())
+	deadURL := dead.URL
+	dead.Close()
+	p := newTestProber(t, healthConfig(deadURL, "http://b.invalid"))
+	err := p.probe(t.Context(), Backend{Name: "a", URL: deadURL})
+	if classifyProbe(err) != outcomeFailure {
+		t.Fatalf("a refused connection classified as %v, want a real failure (err %v)", classifyProbe(err), err)
+	}
+	for i := 0; i < p.failures; i++ {
+		p.record("a", err)
+	}
+	if p.healthy("a") {
+		t.Error("a refused connection left the backend in service")
+	}
+}
+
+func TestProber_RejectedRunAndFailureRunDoNotAddUp(t *testing.T) {
+	p := newTestProber(t, healthConfig("http://a.invalid", "http://b.invalid"))
+	markUnsupported(t, p, "a")
+	for i := 0; i < 20; i++ {
+		p.record("a", &probeRejectedError{status: http.StatusNotFound, path: p.path})
+	}
+
+	// One real failure on top of a long rejected run must not trip the threshold.
+	p.record("a", errors.New("boom"))
+	if !p.healthy("a") {
+		t.Fatal("a single real failure after a rejected run marked the backend down")
+	}
+	for i := 1; i < p.failures; i++ {
+		p.record("a", errors.New("boom"))
+	}
+	if p.healthy("a") {
+		t.Fatal("a full run of real failures should mark the backend down")
+	}
+	if got := p.snapshot()["a"].stateName(); got != stateUnhealthy {
+		t.Errorf("state = %q, want %q", got, stateUnhealthy)
+	}
+}
+
+func TestProber_RecoversFromProbeUnsupported(t *testing.T) {
+	p := newTestProber(t, healthConfig("http://a.invalid", "http://b.invalid"))
+	markUnsupported(t, p, "a")
+
+	for i := 0; i < p.successes; i++ {
+		p.record("a", nil)
+	}
+	if !p.healthy("a") {
+		t.Fatal("backend left the pool while recovering")
+	}
+	if got := p.snapshot()["a"].stateName(); got != stateHealthy {
+		t.Fatalf("state = %q, want %q", got, stateHealthy)
+	}
+}
+
+// A rejected probe re-admits a backend the prober had already marked down: the
+// evidence that condemned it has been replaced by no evidence at all.
+func TestProber_RejectedProbeReadmitsADownBackend(t *testing.T) {
+	p := newTestProber(t, healthConfig("http://a.invalid", "http://b.invalid"))
+	markDown(t, p, "a")
+	markUnsupported(t, p, "a")
+}
+
+func TestProber_LogsProbeRejectionOncePerTransition(t *testing.T) {
+	var buf strings.Builder
+	p := newProber(healthConfig("http://a.invalid", "http://b.invalid"), log.New(&buf, "", 0))
+	if p == nil {
+		t.Fatal("newProber returned nil for an enabled config")
+	}
+
+	rejected := &probeRejectedError{status: http.StatusNotFound, path: p.path}
+	for i := 0; i < 50; i++ {
+		p.record("a", rejected)
+	}
+	if n := strings.Count(buf.String(), "rejected the health probe"); n != 1 {
+		t.Fatalf("logged the misconfiguration %d times, want once:\n%s", n, buf.String())
+	}
+	if !strings.Contains(buf.String(), `backend "a"`) || !strings.Contains(buf.String(), defaultHealthProbePath) {
+		t.Errorf("log line names neither the backend nor the probe path: %s", buf.String())
+	}
+	if !strings.Contains(buf.String(), "404") {
+		t.Errorf("log line does not name the status: %s", buf.String())
+	}
+
+	// Recovery logs its own single line, and a second rejected run logs again.
+	for i := 0; i < p.successes+5; i++ {
+		p.record("a", nil)
+	}
+	if n := strings.Count(buf.String(), "usable again"); n != 1 {
+		t.Fatalf("logged recovery %d times, want once:\n%s", n, buf.String())
+	}
+	for i := 0; i < 50; i++ {
+		p.record("a", rejected)
+	}
+	if n := strings.Count(buf.String(), "rejected the health probe"); n != 2 {
+		t.Fatalf("second rejected run logged %d times in total, want 2:\n%s", n, buf.String())
+	}
+}
+
+// The regression this guards: a backend whose probe path is wrong for it alone
+// still answers queries perfectly, and must keep contributing records.
+func TestFanOut_ProbeUnsupportedBackendStillContributes(t *testing.T) {
+	a := newFakeBackend(t, `[`+node("h1", "2026-07-20T00:00:00Z")+`]`, `[]`)
+	b := newFakeBackend(t, `[`+node("h2", "2026-07-20T00:00:00Z")+`]`, `[]`)
+	srv := newTestServer(healthConfig(a.srv.URL, b.srv.URL))
+	markUnsupported(t, srv.health, "b")
+
+	rec := doGet(t, srv.Handler(), nodesPath, "")
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status %d: %s", rec.Code, rec.Body.String())
+	}
+	if _, asked := b.params(nodesPath); !asked {
+		t.Fatal("a backend with an unusable probe path was not queried")
+	}
+	body := rec.Body.String()
+	if !strings.Contains(body, "h1") || !strings.Contains(body, "h2") {
+		t.Fatalf("both backends should have contributed: %s", body)
+	}
+	if got := rec.Header().Get(backendsHeader); got != "2/2" {
+		t.Errorf("%s = %q, want 2/2", backendsHeader, got)
+	}
+}
+
+func TestHealthz_ReportsProbeUnsupported(t *testing.T) {
+	a := newFakeBackend(t, `[]`, `[]`)
+	b := newFakeBackend(t, `[]`, `[]`)
+	srv := newTestServer(healthConfig(a.srv.URL, b.srv.URL))
+	srv.health.record("a", nil)
+	markUnsupported(t, srv.health, "b")
+
+	rec := doGet(t, srv.Handler(), "/healthz", "")
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status %d: %s", rec.Code, rec.Body.String())
+	}
+	var hr healthReport
+	if err := json.Unmarshal(rec.Body.Bytes(), &hr); err != nil {
+		t.Fatal(err)
+	}
+	if got := hr.Backends["a"].State; got != stateHealthy {
+		t.Errorf("a state = %q, want %q", got, stateHealthy)
+	}
+	// Distinct from both "verified healthy" and "excluded".
+	unsupported := hr.Backends["b"]
+	if unsupported.State != stateProbeUnsupported {
+		t.Errorf("b state = %q, want %q", unsupported.State, stateProbeUnsupported)
+	}
+	if unsupported.Reachable != "ok" {
+		t.Errorf("b reachable = %q, want ok", unsupported.Reachable)
+	}
+	if unsupported.LastError == "" || unsupported.LastProbe == "" {
+		t.Errorf("b is missing last_error/last_probe: %+v", unsupported)
+	}
+}
+
 func TestStatusBodyHealthy_IgnoresBodiesWithoutStates(t *testing.T) {
 	for _, body := range []string{`{}`, `[]`, `null`, `{"svc":{}}`, `not json`} {
 		if err := statusBodyHealthy([]byte(body)); err != nil {

server.go

@@ -676,7 +676,7 @@ type healthReport struct {
 // prober's running state for the same backend.
 type backendReport struct {
 	Reachable string `json:"reachable"` // "ok" | error text
-	State     string `json:"state"`     // healthy | unhealthy | unprobed | unmonitored
+	State     string `json:"state"`     // healthy | unhealthy | probe_unsupported | unprobed | unmonitored
 	Failures  int    `json:"consecutive_failures"`
 	Successes int    `json:"consecutive_successes"`
 	LastProbe string `json:"last_probe,omitempty"`