Do not report a healthy Trino cell as unreachable

controlplane/admin/trino.go

@@ -96,8 +96,12 @@ type TrinoStatus struct {
 	// than one that is busy.
 	BlockedQueries int `json:"blocked_queries"`
 
-	Nodes       int `json:"nodes"`
-	FailedNodes int `json:"failed_nodes"`
+	// NodeStats reports whether Nodes and FailedNodes mean anything. A cell
+	// using Trino's default discovery.type=ANNOUNCE does not serve /v1/node,
+	// so the console must show "not reported here" rather than zero nodes.
+	NodeStats   bool `json:"node_stats"`
+	Nodes       int  `json:"nodes"`
+	FailedNodes int  `json:"failed_nodes"`
 
 	// OrgsByState counts Trino-enabled orgs by provisioning state, so a
 	// stuck tenant is visible without opening the org list.
@@ -307,13 +311,20 @@ func (a *TrinoAPI) handleStatus(c *gin.Context) {
 		}
 	}
 
+	// Node stats are best-effort. A cell built with the default
+	// discovery.type=ANNOUNCE does not serve /v1/node at all, and a healthy
+	// cell must not be reported as down because an endpoint it never had is
+	// missing. Any other error still counts against the cell.
 	if nodes, nodeErr := a.nodes.get(ctx, a.client.Nodes); nodeErr == nil {
+		status.NodeStats = true
 		status.Nodes = len(nodes)
 		for _, n := range nodes {
 			if n.Failed {
 				status.FailedNodes++
 			}
 		}
+	} else if isTrinoEndpointUnavailable(nodeErr) {
+		status.NodeStats = false
 	} else if status.Error == "" {
 		status.Available = false
 		status.Error = nodeErr.Error()
@@ -477,6 +488,17 @@ func (a *TrinoAPI) handleKillQuery(c *gin.Context) {
 
 func (a *TrinoAPI) handleNodes(c *gin.Context) {
 	nodes, err := a.nodes.get(c.Request.Context(), a.client.Nodes)
+	if isTrinoEndpointUnavailable(err) {
+		// Not a gateway failure: the coordinator answered, and it does not
+		// serve this route. 501 says the console asked for something this
+		// cell cannot provide, which is what an operator needs to know.
+		c.JSON(http.StatusNotImplemented, gin.H{
+			"cell":      a.cell,
+			"available": false,
+			"reason":    "this cell does not serve /v1/node; Trino binds it only under discovery.type=AIRLIFT_DISCOVERY",
+		})
+		return
+	}
 	if err != nil {
 		c.JSON(http.StatusBadGateway, gin.H{"error": err.Error(), "available": false})
 		return

controlplane/admin/trino_client.go

@@ -50,6 +50,36 @@ var errTrinoNotFound = errors.New("trino: query not found")
 
 func isTrinoNotFound(err error) bool { return errors.Is(err, errTrinoNotFound) }
 
+// errTrinoEndpointUnavailable is returned when the coordinator answers but
+// does not serve the route at all. That is a statement about how the cell is
+// built, not about its health, and it is not the same incident as a
+// coordinator that never answered.
+//
+// /v1/node is the case this exists for. Trino binds NodeResource only under
+// discovery.type=AIRLIFT_DISCOVERY; the default is ANNOUNCE, which these
+// cells use, so the route is absent and the coordinator correctly answers
+// 404. Reporting that as a dead cell hides the real state of a healthy one.
+var errTrinoEndpointUnavailable = errors.New("trino: endpoint not served by this coordinator")
+
+func isTrinoEndpointUnavailable(err error) bool {
+	return errors.Is(err, errTrinoEndpointUnavailable)
+}
+
+// queryRouteNotFound re-reads a 404 for the per-query routes as a missing
+// query rather than a missing route.
+//
+// /v1/query/{id} is always served, so a 404 there is about the id, not the
+// route: JAX-RS answers 404 when it cannot convert the path parameter into a
+// QueryId. Only the aged-out case answers 410, so without this an operator
+// following a malformed link would be told the coordinator does not serve
+// query lookups at all.
+func queryRouteNotFound(err error) error {
+	if isTrinoEndpointUnavailable(err) {
+		return fmt.Errorf("%w", errTrinoNotFound)
+	}
+	return err
+}
+
 // TrinoQuery is one query as the console shows it: identity, lifecycle and
 // the cost counters an operator triages on.
 //
@@ -194,9 +224,16 @@ func NewTrinoCoordinatorClient(baseURL, tlsServerName string, creds TrinoCredent
 }
 
 // do issues one authenticated request and returns the body. Non-2xx is an
-// error carrying the status, except 410 Gone, which maps to
-// errTrinoNotFound so a stale query link reads as "gone" rather than as a
-// broken cell.
+// error carrying the status, except two cases that are not cell failures:
+//
+//	410 Gone      the query is no longer held -> errTrinoNotFound, so a
+//	              stale query link reads as "gone" rather than as a broken
+//	              cell. QueryResource throws GoneException for this, and
+//	              only for this.
+//	404 Not Found the coordinator does not serve the route at all ->
+//	              errTrinoEndpointUnavailable, so an endpoint this cell was
+//	              never built with reads as unavailable rather than as
+//	              silence from the coordinator.
 func (c *trinoCoordinatorHTTPClient) do(ctx context.Context, method, path string, body io.Reader) ([]byte, error) {
 	req, err := http.NewRequestWithContext(ctx, method, c.baseURL+path, body)
 	if err != nil {
@@ -215,8 +252,10 @@ func (c *trinoCoordinatorHTTPClient) do(ctx context.Context, method, path string
 	defer func() { _ = resp.Body.Close() }()
 	raw, readErr := io.ReadAll(resp.Body)
 	switch {
-	case resp.StatusCode == http.StatusGone, resp.StatusCode == http.StatusNotFound:
+	case resp.StatusCode == http.StatusGone:
 		return nil, fmt.Errorf("%s %s: %w", method, path, errTrinoNotFound)
+	case resp.StatusCode == http.StatusNotFound:
+		return nil, fmt.Errorf("%s %s: %w", method, path, errTrinoEndpointUnavailable)
 	case resp.StatusCode == http.StatusForbidden:
 		// Worth its own message: this is what a missing observer grant or
 		// an un-rolled-out OPA bundle looks like, and it is fixed in a
@@ -352,7 +391,7 @@ func (c *trinoCoordinatorHTTPClient) Queries(ctx context.Context) ([]TrinoQuery,
 func (c *trinoCoordinatorHTTPClient) Query(ctx context.Context, queryID string) (*TrinoQuery, error) {
 	raw, err := c.do(ctx, http.MethodGet, "/v1/query/"+url.PathEscape(queryID)+"?pruned=true", nil)
 	if err != nil {
-		return nil, err
+		return nil, queryRouteNotFound(err)
 	}
 	var wire trinoBasicQueryInfo
 	if err := json.Unmarshal(raw, &wire); err != nil {
@@ -372,7 +411,7 @@ func (c *trinoCoordinatorHTTPClient) KillQuery(ctx context.Context, queryID, mes
 	ctx, cancel := context.WithTimeout(ctx, trinoKillTimeout)
 	defer cancel()
 	_, err := c.do(ctx, http.MethodPut, "/v1/query/"+url.PathEscape(queryID)+"/killed", strings.NewReader(message))
-	return err
+	return queryRouteNotFound(err)
 }
 
 type trinoNodeStats struct {

controlplane/admin/trino_client_test.go

@@ -291,6 +291,48 @@ func TestCoordinatorClientGoneQueryIsNotFound(t *testing.T) {
 	if !isTrinoNotFound(err) {
 		t.Errorf("410 Gone should map to a not-found error, got %v", err)
 	}
+	if isTrinoEndpointUnavailable(err) {
+		t.Error("410 Gone is a missing query, not a missing endpoint")
+	}
+}
+
+// TestCoordinatorClientMissingRouteIsEndpointUnavailable: a coordinator
+// built with Trino's default discovery.type=ANNOUNCE does not bind
+// NodeResource, so /v1/node answers 404. The cell is healthy and must not be
+// reported as one that never answered.
+func TestCoordinatorClientMissingRouteIsEndpointUnavailable(t *testing.T) {
+	c, _ := newTrinoTestCoordinator(t, func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+	})
+	_, err := c.Nodes(context.Background())
+	if !isTrinoEndpointUnavailable(err) {
+		t.Errorf("404 should map to an endpoint-unavailable error, got %v", err)
+	}
+	if isTrinoNotFound(err) {
+		t.Error("a missing route must not read as a missing query")
+	}
+}
+
+// TestCoordinatorClientQueryRoute404StaysNotFound guards the per-query
+// routes. /v1/query/{id} is always served, so a 404 there is about the id —
+// JAX-RS answers 404 when it cannot parse one into a QueryId. An operator
+// following a malformed link must be told the query is missing, not that the
+// coordinator does not serve query lookups.
+func TestCoordinatorClientQueryRoute404StaysNotFound(t *testing.T) {
+	c, _ := newTrinoTestCoordinator(t, func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+	})
+	_, err := c.Query(context.Background(), "not a query id")
+	if !isTrinoNotFound(err) {
+		t.Errorf("404 on the query route should read as a missing query, got %v", err)
+	}
+	if isTrinoEndpointUnavailable(err) {
+		t.Error("the query route exists; a 404 there is not a missing endpoint")
+	}
+
+	if killErr := c.KillQuery(context.Background(), "not a query id", "why"); !isTrinoNotFound(killErr) {
+		t.Errorf("404 on kill should read as a missing query, got %v", killErr)
+	}
 }
 
 // TestCoordinatorClientForbiddenIsSurfaced: a 403 means the observer's OPA

controlplane/admin/trino_test.go

@@ -313,6 +313,40 @@ func TestStatusReportsProvisioningWhenTheCoordinatorIsDown(t *testing.T) {
 	}
 }
 
+// TestStatusStaysAvailableWhenTheCellDoesNotServeNodes is the bug this
+// distinction exists for. Trino binds NodeResource only under
+// discovery.type=AIRLIFT_DISCOVERY, and these cells run the default
+// ANNOUNCE, so /v1/node answers 404 on a perfectly healthy coordinator.
+// Before this, that 404 set available=false and the console reported a
+// working cell as one that never answered.
+func TestStatusStaysAvailableWhenTheCellDoesNotServeNodes(t *testing.T) {
+	coord := &fakeTrinoCoordinator{
+		info:     &TrinoServerInfo{Version: "484", Environment: "production"},
+		queries:  []TrinoQuery{{QueryID: "q1", State: "RUNNING", Principal: "db_a"}},
+		nodesErr: fmt.Errorf("GET /v1/node: %w", errTrinoEndpointUnavailable),
+	}
+	r := trinoTestRouter(testTrinoAPI(t, coord, twoOrgTrinoStore()), RoleViewer)
+
+	code, body := doTrinoJSON(t, r, http.MethodGet, "/api/v1/trino/status", "")
+	if code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", code)
+	}
+	if body["available"] != true {
+		t.Errorf("a cell that does not serve /v1/node is still available, body: %v", body)
+	}
+	if s, _ := body["error"].(string); s != "" {
+		t.Errorf("a missing endpoint is not a cell error, got %q", s)
+	}
+	if body["node_stats"] != false {
+		t.Error("node_stats must be false so the console shows 'not reported' rather than zero nodes")
+	}
+	// The rest of the cell must still be reported.
+	states := body["queries_by_state"].(map[string]any)
+	if states["RUNNING"] != float64(1) {
+		t.Errorf("queries must still be counted, got %v", states)
+	}
+}
+
 func TestStatusCountsQueriesAndNodes(t *testing.T) {
 	coord := &fakeTrinoCoordinator{
 		info: &TrinoServerInfo{Version: "484", Environment: "production", UptimeMS: 3600000},

controlplane/admin/ui/src/hooks/useApi.ts

@@ -787,6 +787,7 @@ export function useTrinoStatus() {
         available: false,
         queries_by_state: {},
         blocked_queries: 0,
+        node_stats: true,
         nodes: 0,
         failed_nodes: 0,
         orgs_by_state: {},

controlplane/admin/ui/src/lib/trino.test.ts

@@ -59,6 +59,7 @@ function status(over: Partial<TrinoStatus> = {}): TrinoStatus {
     available: true,
     queries_by_state: {},
     blocked_queries: 0,
+    node_stats: true,
     nodes: 0,
     failed_nodes: 0,
     orgs_by_state: {},

controlplane/admin/ui/src/pages/TrinoCluster.tsx

@@ -181,6 +181,14 @@ export function TrinoCluster() {
           <CardContent>
             {nodes.isLoading ? (
               <TableSkeleton cols={5} />
+            ) : status.data && !status.data.node_stats ? (
+              // Not the same as an empty fleet. Trino serves /v1/node only
+              // under discovery.type=AIRLIFT_DISCOVERY, and a cell on the
+              // default ANNOUNCE has no such endpoint to answer with.
+              <EmptyState
+                title="Nodes are not reported by this cell"
+                description="This coordinator does not serve /v1/node. Trino registers that endpoint only under discovery.type=AIRLIFT_DISCOVERY, and this cell uses the default. The cell is healthy; its fleet is simply not visible here."
+              />
             ) : (nodes.data?.nodes ?? []).length === 0 ? (
               <EmptyState
                 title="No nodes reported"

controlplane/admin/ui/src/pages/TrinoQueries.test.tsx

@@ -58,6 +58,7 @@ function status(over: Partial<TrinoStatus> = {}): TrinoStatus {
     available: true,
     queries_by_state: {},
     blocked_queries: 0,
+    node_stats: true,
     nodes: 2,
     failed_nodes: 0,
     orgs_by_state: {},

controlplane/admin/ui/src/types/api.ts

@@ -836,6 +836,11 @@ export interface TrinoStatus {
   server?: TrinoServerInfo;
   queries_by_state: Record<string, number>;
   blocked_queries: number;
+  // node_stats=false means this cell does not report nodes at all, so nodes
+  // and failed_nodes are both zero and mean nothing. Trino serves /v1/node
+  // only under discovery.type=AIRLIFT_DISCOVERY; these cells run the default
+  // ANNOUNCE. Render "not reported" rather than zero nodes.
+  node_stats: boolean;
   nodes: number;
   failed_nodes: number;
   orgs_by_state: Record<string, number>;