mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
fix(pbs): backport absent-status recovery protection
Backport of31f1f1933a, adapted to release/v6.4 client tests. Reproduced absent envelopes returning zero metrics and the null-status poller availability failure on candidate base239ee0f9dabefore applying the production fix. Focused client and lifecycle race checks pass three repetitions. No installed qualification claimed. Change-source: pulse-maintainer
This commit is contained in:
@@ -17,6 +17,23 @@
|
||||
|
||||
## Purpose
|
||||
|
||||
PBS node-status collection must reject HTTP-success responses whose `data`
|
||||
is omitted or null (including a null response envelope). Absent status is
|
||||
unavailable telemetry, not measured zero usage: the poller retains independently
|
||||
established connectivity, marks node metrics unavailable, and must not resolve
|
||||
an active metric incident from that response. This does not add per-field
|
||||
validation of populated status objects.
|
||||
|
||||
Verification: `TestClient_GetNodeStatus_MissingData` in
|
||||
`pkg/pbs/client_http_test.go` covers the absent envelopes.
|
||||
`TestPBSPartialMetricsWebhookLifecycle` in
|
||||
`internal/monitoring/monitor_pbs_webhook_test.go` exercises repeated null status
|
||||
through the real poller, alert manager, notification queue and local webhook.
|
||||
It requires online-but-unavailable projection, unchanged incident identity,
|
||||
no false recovery history or delivery, then one identity-preserving recovery
|
||||
after valid low-memory samples. These are local synthetic checks, not
|
||||
installed-artifact or off-host destination qualification.
|
||||
|
||||
Direct PBS backup polling classifies typed API and authentication failures by
|
||||
the response status exposed by `pbs.HTTPStatus`, including wrapped errors.
|
||||
A 5xx gateway or server response remains transient even when its body quotes
|
||||
|
||||
@@ -26,6 +26,7 @@ const (
|
||||
pbsHealthTestNodeGatewayFailure
|
||||
pbsHealthTestUnavailable
|
||||
pbsHealthTestLowMemory
|
||||
pbsHealthTestNullNodeStatus
|
||||
)
|
||||
|
||||
type pbsHealthTestServer struct {
|
||||
@@ -66,6 +67,10 @@ func newPBSHealthTestServer(t *testing.T) *pbsHealthTestServer {
|
||||
"data": map[string]any{"version": "3.4.2"},
|
||||
})
|
||||
case "/api2/json/nodes/localhost/status":
|
||||
if mode == pbsHealthTestNullNodeStatus {
|
||||
_, _ = w.Write([]byte(`{"data":null}`))
|
||||
return
|
||||
}
|
||||
if mode == pbsHealthTestNodeDenied {
|
||||
http.Error(w, "permission denied", http.StatusForbidden)
|
||||
return
|
||||
|
||||
@@ -98,11 +98,15 @@ func TestPBSPartialMetricsWebhookLifecycle(t *testing.T) {
|
||||
}
|
||||
incident := firing.Alerts[0]
|
||||
waitSent(1)
|
||||
for _, mode := range []pbsHealthTestMode{pbsHealthTestNodeDenied, pbsHealthTestNodeGatewayFailure} {
|
||||
for _, mode := range []pbsHealthTestMode{pbsHealthTestNodeDenied, pbsHealthTestNodeGatewayFailure, pbsHealthTestNullNodeStatus} {
|
||||
fixture.setMode(mode)
|
||||
for range 5 {
|
||||
poll()
|
||||
}
|
||||
projection := pbsInstanceByName(t, monitor.state.GetSnapshot(), instance.Name)
|
||||
if projection.Status != "online" || !projection.NodeMetricsUnavailable {
|
||||
t.Fatalf("partial failure must retain connectivity without claiming metrics: %+v", projection)
|
||||
}
|
||||
active := manager.GetActiveAlerts()
|
||||
if len(active) != 1 || active[0].ID != incident.ID || !active[0].StartTime.Equal(incident.StartTime) {
|
||||
t.Fatal("partial failure changed the active incident")
|
||||
|
||||
+8
-2
@@ -771,14 +771,20 @@ func (c *Client) GetNodeStatus(ctx context.Context) (*NodeStatus, error) {
|
||||
log.Debug().Str("response", string(body)).Msg("PBS node status response")
|
||||
|
||||
var statusResult struct {
|
||||
Data NodeStatus `json:"data"`
|
||||
Data *NodeStatus `json:"data"`
|
||||
}
|
||||
|
||||
if err := json.Unmarshal(body, &statusResult); err != nil {
|
||||
return nil, fmt.Errorf("failed to decode status response: %w", err)
|
||||
}
|
||||
|
||||
return &statusResult.Data, nil
|
||||
// A successful HTTP response is not evidence of available metrics.
|
||||
// Missing/null data must not become zero usage and resolve active alerts.
|
||||
if statusResult.Data == nil {
|
||||
return nil, fmt.Errorf("node status response contains no data")
|
||||
}
|
||||
|
||||
return statusResult.Data, nil
|
||||
}
|
||||
|
||||
// GetDatastores returns all datastores with their status.
|
||||
|
||||
@@ -817,3 +817,18 @@ func TestClient_GetNodeName_ConcurrentTransientFailureIsSingleFlight(t *testing.
|
||||
t.Fatalf("/nodes hit %d times after recovery and cache read, want 2", got)
|
||||
}
|
||||
}
|
||||
func TestClient_GetNodeStatus_MissingData(t *testing.T) {
|
||||
for _, body := range []string{`{"data":null}`, `{}`, `null`} {
|
||||
t.Run(body, func(t *testing.T) {
|
||||
client, server := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(body))
|
||||
})
|
||||
defer server.Close()
|
||||
status, err := client.GetNodeStatus(context.Background())
|
||||
if status != nil || err == nil {
|
||||
t.Fatalf("absent metrics = (%+v, %v), want nil status and error", status, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user