From 6d6275e78d98a691575c563d70f277ed4e7f2a19 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Wed, 30 Sep 2026 15:43:34 +1000 Subject: [PATCH] refactor: establish cluster health contract --- contracts/cluster/v1/README.md | 11 +++++--- contracts/cluster/v1/discovery.go | 6 ++--- contracts/cluster/v1/health.go | 23 ++++++++++++++++ contracts/cluster/v1/health_test.go | 36 +++++++++++++++++++++++++ spinifex/daemon/daemon.go | 4 +-- spinifex/daemon/daemon_handlers.go | 2 +- spinifex/daemon/daemon_handlers_test.go | 4 +-- spinifex/types/cluster.go | 10 ------- tests/e2e/harness/daemon_client.go | 8 +++--- 9 files changed, 79 insertions(+), 25 deletions(-) create mode 100644 contracts/cluster/v1/health.go create mode 100644 contracts/cluster/v1/health_test.go diff --git a/contracts/cluster/v1/README.md b/contracts/cluster/v1/README.md index 5d21bd654..c58c3d52d 100644 --- a/contracts/cluster/v1/README.md +++ b/contracts/cluster/v1/README.md @@ -1,10 +1,15 @@ # Cluster v1 contracts -This package owns the versioned NATS contracts for cluster-membership control -plane traffic. It is not an AWS tenant-facing API: it lets platform consumers -identify the daemon nodes currently responding to a cluster fan-out. +This package owns the versioned cluster control-plane wire contracts. It is +not an AWS tenant-facing API: it lets platform consumers identify responding +daemon nodes and inspect their health. `NodesDiscoverSubject` retains the deployed `spinifex.nodes.discover` route. `NodeDiscoverResponse.Node` is the node identifier used for responder-set coverage. Do not change either incompatibly in this package; add a new version when a wire change is required. + +`NodeHealthSubject` retains the deployed per-node NATS route, while +`NodeHealthResponse` is also the daemon HTTPS `/health` representation. Do not +change their subject or JSON shape incompatibly in this package; add a new +version when a wire change is required. diff --git a/contracts/cluster/v1/discovery.go b/contracts/cluster/v1/discovery.go index 32696ed27..5879ad02d 100644 --- a/contracts/cluster/v1/discovery.go +++ b/contracts/cluster/v1/discovery.go @@ -1,6 +1,6 @@ -// Package clusterv1 defines version 1 of the cross-process cluster-membership -// contract. These messages establish which daemon nodes are responding to the -// control plane; their subject and JSON fields are wire compatibility. +// Package clusterv1 defines version 1 of the cluster control-plane contract. +// Its NATS subjects and JSON fields are wire compatibility, not implementation +// details of a daemon or consumer. package clusterv1 const ( diff --git a/contracts/cluster/v1/health.go b/contracts/cluster/v1/health.go new file mode 100644 index 000000000..4d7aef33e --- /dev/null +++ b/contracts/cluster/v1/health.go @@ -0,0 +1,23 @@ +package clusterv1 + +import "fmt" + +// NodeHealthSubject returns the NATS route for one daemon's health response. +// It intentionally performs no node-ID validation: callers have always +// supplied the configured node identifier directly to NATS, and this +// structural move must preserve that behaviour. +func NodeHealthSubject(node string) string { + return fmt.Sprintf("spinifex.admin.%s.health", node) +} + +// NodeHealthResponse is the daemon health representation returned by both the +// per-node NATS health route and the daemon HTTPS /health endpoint. +type NodeHealthResponse struct { + Node string `json:"node"` + Status string `json:"status"` + ConfigHash string `json:"config_hash"` + Epoch uint64 `json:"epoch"` + Uptime int64 `json:"uptime"` + Services []string `json:"services"` + ServiceHealth map[string]string `json:"service_health,omitempty"` +} diff --git a/contracts/cluster/v1/health_test.go b/contracts/cluster/v1/health_test.go new file mode 100644 index 000000000..19c899ab9 --- /dev/null +++ b/contracts/cluster/v1/health_test.go @@ -0,0 +1,36 @@ +package clusterv1 + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNodeHealthResponseJSONContract(t *testing.T) { + data, err := json.Marshal(NodeHealthResponse{ + Node: "node-a", + Status: "running", + ConfigHash: "abc123", + Epoch: 7, + Uptime: 42, + Services: []string{"nats", "spinifex"}, + ServiceHealth: map[string]string{ + "nats": "ok", + }, + }) + require.NoError(t, err) + require.JSONEq(t, `{ + "node":"node-a", + "status":"running", + "config_hash":"abc123", + "epoch":7, + "uptime":42, + "services":["nats","spinifex"], + "service_health":{"nats":"ok"} + }`, string(data)) +} + +func TestNodeHealthSubject(t *testing.T) { + require.Equal(t, "spinifex.admin.node-a.health", NodeHealthSubject("node-a")) +} diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index c3cae5a77..c2851a756 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -1017,7 +1017,7 @@ func (d *Daemon) subscribeAll() error { {"ec2.GetSerialConsoleAccessStatus", handleNATSRequest(d.node, d.accountService.GetSerialConsoleAccessStatus), "spinifex-workers"}, {"ec2.EnableSerialConsoleAccess", handleNATSRequest(d.node, d.accountService.EnableSerialConsoleAccess), "spinifex-workers"}, {"ec2.DisableSerialConsoleAccess", handleNATSRequest(d.node, d.accountService.DisableSerialConsoleAccess), "spinifex-workers"}, - {fmt.Sprintf("spinifex.admin.%s.health", d.node), d.handleHealthCheck, ""}, + {clusterv1.NodeHealthSubject(d.node), d.handleHealthCheck, ""}, {clusterv1.NodesDiscoverSubject, d.handleNodeDiscover, ""}, {"spinifex.node.status", d.handleNodeStatus, ""}, {"spinifex.node.vms", d.handleNodeVMs, ""}, @@ -2719,7 +2719,7 @@ func (d *Daemon) ClusterManager() error { status = "starting" } - response := types.NodeHealthResponse{ + response := clusterv1.NodeHealthResponse{ Node: d.node, Status: status, ConfigHash: configHash, diff --git a/spinifex/daemon/daemon_handlers.go b/spinifex/daemon/daemon_handlers.go index 9ce9130d8..a56af0514 100644 --- a/spinifex/daemon/daemon_handlers.go +++ b/spinifex/daemon/daemon_handlers.go @@ -346,7 +346,7 @@ func (d *Daemon) handleHealthCheck(msg *nats.Msg) string { status = "starting" } - response := types.NodeHealthResponse{ + response := clusterv1.NodeHealthResponse{ Node: d.node, Status: status, ConfigHash: configHash, diff --git a/spinifex/daemon/daemon_handlers_test.go b/spinifex/daemon/daemon_handlers_test.go index 79b7f82a9..d47127fbe 100644 --- a/spinifex/daemon/daemon_handlers_test.go +++ b/spinifex/daemon/daemon_handlers_test.go @@ -309,7 +309,7 @@ func TestHandleHealthCheck(t *testing.T) { daemon := createTestDaemon(t, natsURL) - topic := fmt.Sprintf("spinifex.admin.%s.health", daemon.node) + topic := clusterv1.NodeHealthSubject(daemon.node) sub, err := daemon.natsConn.Subscribe(topic, asMsgHandler(daemon.handleHealthCheck)) require.NoError(t, err) defer sub.Unsubscribe() @@ -319,7 +319,7 @@ func TestHandleHealthCheck(t *testing.T) { require.NoError(t, err) require.NotNil(t, reply) - var resp types.NodeHealthResponse + var resp clusterv1.NodeHealthResponse err = json.Unmarshal(reply.Data, &resp) require.NoError(t, err) diff --git a/spinifex/types/cluster.go b/spinifex/types/cluster.go index fb883aecc..3b22bb9ec 100644 --- a/spinifex/types/cluster.go +++ b/spinifex/types/cluster.go @@ -8,13 +8,3 @@ type SharedClusterData struct { Version string `json:"version" toml:"version"` Nodes map[string]config.Config `json:"nodes" toml:"nodes"` } - -type NodeHealthResponse struct { - Node string `json:"node"` - Status string `json:"status"` - ConfigHash string `json:"config_hash"` - Epoch uint64 `json:"epoch"` - Uptime int64 `json:"uptime"` - Services []string `json:"services"` - ServiceHealth map[string]string `json:"service_health,omitempty"` -} diff --git a/tests/e2e/harness/daemon_client.go b/tests/e2e/harness/daemon_client.go index a79e108e5..c50fd928f 100644 --- a/tests/e2e/harness/daemon_client.go +++ b/tests/e2e/harness/daemon_client.go @@ -13,7 +13,7 @@ import ( "strconv" "time" - "github.com/mulgadc/spinifex/spinifex/types" + clusterv1 "github.com/mulgadc/spinifex/contracts/cluster/v1" ) // WaitForMode polls the daemon's /local/status until it reports the expected @@ -145,10 +145,10 @@ func (d *DaemonClient) Instances(ctx context.Context, node Node) ([]LocalInstanc } // Health hits the daemon's /health endpoint to confirm basic reachability. -func (d *DaemonClient) Health(ctx context.Context, node Node) (types.NodeHealthResponse, error) { - var h types.NodeHealthResponse +func (d *DaemonClient) Health(ctx context.Context, node Node) (clusterv1.NodeHealthResponse, error) { + var h clusterv1.NodeHealthResponse if err := d.getJSON(ctx, node, "/health", &h); err != nil { - return types.NodeHealthResponse{}, err + return clusterv1.NodeHealthResponse{}, err } return h, nil }