From effe50a6baa38d9c7aa9031380bca79082e4b490 Mon Sep 17 00:00:00 2001 From: "lei.chen" Date: Sun, 25 Oct 2026 02:58:14 +0000 Subject: [PATCH] cluster info: fill xce id cluster_name/status from live ctrlstatus `xce id` printed empty name/status: two breaks in the chain. 1. CLI decode: the clusters API returns store-backed rows in the OrderMap wire format ({"Fields":[...],"Data":{...}}), but ClusterResponse.Rows ([]ClusterInfo) had no custom UnmarshalJSON, so every row landed all-empty. Same gap array_stats hit; fixed the same way (UnmarshalOrderMapRow). 2. Server data: the cluster row only comes from OnClusterUpdated (config push), which never writes Status or ControllerName -- both stayed at the store zero value (status 0 = DISCONNECT) forever even on a healthy cluster. The controller's real identity/health snapshot already exists on the wire (CtrlStatus reply, xce_ctrlstatus_t); the apiserver just never asked for it. Fix: - backend.FetchCtrlStatus(): CGO wrapper of xcecli_ctrlstatus through callBackend (failover + circuit breaker like every other read), decoded into ClusterCtrlStatus and freed via free_CtrlStatus_ptr (bug-1401 leak contract); no-CGO stub added alongside the others. - GetClustersInfo merges the snapshot into each row before rendering: cluster_name must match the responder's own (a stale-named row never claims another cluster's identity), controller_name <- primaryName, status <- CONNECTED. A backend that doesn't answer keeps the store's last-known state instead of failing the query. - CLI ClusterInfo gains the OrderMap-aware UnmarshalJSON. t1 live: `xce id` now shows name / status / controller t1; submit->run->DONE job verified with all daemons healthy. Co-Authored-By: Claude Code --- src/frontend/apiserver/backend/clib.go | 30 +++++ src/frontend/apiserver/backend/config.go | 16 +++ src/frontend/apiserver/backend/stub.go | 3 + .../apiserver/controllers/clusters.go | 44 +++++++ .../controllers/clusters_status_test.go | 120 ++++++++++++++++++ src/frontend/apiserver/models/clusters.go | 5 + src/frontend/xcecli/models/cluster.go | 13 ++ 7 files changed, 231 insertions(+) create mode 100644 src/frontend/apiserver/controllers/clusters_status_test.go diff --git a/src/frontend/apiserver/backend/clib.go b/src/frontend/apiserver/backend/clib.go index 70a38169..d40f5569 100644 --- a/src/frontend/apiserver/backend/clib.go +++ b/src/frontend/apiserver/backend/clib.go @@ -838,3 +838,33 @@ func FetchFile(jobid uint64, filepath string, offset uint64, maxBytes uint32) (* } return result, nil } + +// FetchCtrlStatus issues the any-node controller-status read through the +// shared backend connection (xcecli_ctrlstatus, failover-wrapped like every +// other read). Callers merge the result into the cluster-info row for +// `xce id`; a backend that never answers returns the callBackend error and +// the caller keeps the store's last-known state. +func FetchCtrlStatus() (*ClusterCtrlStatus, error) { + var status *C.xce_ctrlstatus_t + _, err := callBackend("xcecli_ctrlstatus", func() int { + return int(C.xcecli_ctrlstatus(gCtx.ctx, &status)) + }) + if err != nil { + return nil, err + } + if status == nil { + return nil, fmt.Errorf("xcecli_ctrlstatus returned nil reply") + } + // free_CtrlStatus_ptr frees the fields INSIDE the struct and the struct + // itself, nulling the pointer (bug-1401's leak contract). + defer C.free_CtrlStatus_ptr(&status) + + return &ClusterCtrlStatus{ + ClusterName: goString(status.clusterName), + PrimaryName: goString(status.primaryName), + ResponderName: goString(status.responderName), + ResponderRole: uint8(status.responderRole), + PeerCount: uint32(status.peerCount), + Fenced: status.fenced != 0, + }, nil +} diff --git a/src/frontend/apiserver/backend/config.go b/src/frontend/apiserver/backend/config.go index 41b51c07..ac50e1c5 100644 --- a/src/frontend/apiserver/backend/config.go +++ b/src/frontend/apiserver/backend/config.go @@ -24,3 +24,19 @@ type FetchFileResult struct { FileSize uint64 BytesRead uint32 } + +// ClusterCtrlStatus is the Go view of the controller's identity/health +// snapshot (CtrlStatus reply, xce_ctrlstatus_t) that the cluster-info query +// merges into the otherwise config-static cluster row. Only the fields the +// display needs travel across; term/LSN details stay available to the +// harness `ctrlstatus` command. +type ClusterCtrlStatus struct { + ClusterName string + PrimaryName string + ResponderName string + ResponderRole uint8 + PeerCount uint32 + // Fenced mirrors the reply's fenced flag: true means the responder + // answers but cannot serve writes (HA fence up). + Fenced bool +} diff --git a/src/frontend/apiserver/backend/stub.go b/src/frontend/apiserver/backend/stub.go index 1f295d83..98e2e5be 100644 --- a/src/frontend/apiserver/backend/stub.go +++ b/src/frontend/apiserver/backend/stub.go @@ -52,6 +52,9 @@ func FetchFile(_ uint64, _ string, _ uint64, _ uint32) (*FetchFileResult, error) return nil, errNoCGO } +// FetchCtrlStatus stub — real implementation in clib.go (requires CGO). +func FetchCtrlStatus() (*ClusterCtrlStatus, error) { return nil, errNoCGO } + // ValidateResreqSyntax (bug-1744) needs libxce's grammar parser; without CGO // no job can be submitted anyway (SubmitJob returns errNoCGO), so there is // nothing to check. diff --git a/src/frontend/apiserver/controllers/clusters.go b/src/frontend/apiserver/controllers/clusters.go index 34d94af2..c2ba6ff7 100644 --- a/src/frontend/apiserver/controllers/clusters.go +++ b/src/frontend/apiserver/controllers/clusters.go @@ -1,6 +1,7 @@ package controllers import ( + "apiserver/backend" "apiserver/common" "apiserver/models" "apiserver/services" @@ -176,6 +177,14 @@ func (c ClusterController) GetClustersInfo(ctx *gin.Context) { return } logger.Debug("the cluster info query reply:", zap.Any("clqueryReply", clqueryReply), zap.String("pagetoken", pagetoken)) + + // The store row is config-push static (OnClusterUpdated never writes + // Status/ControllerName), so the live controller identity/health snapshot + // is fetched per query and merged in. A backend that doesn't answer + // (degraded circuit, no reachable controller) keeps the store's + // last-known state instead of failing the whole query. + mergeCtrlStatus(clqueryReply) + //convert to readable string. queryResult := processClusterData(clqueryReply, pagetoken) @@ -183,10 +192,45 @@ func (c ClusterController) GetClustersInfo(ctx *gin.Context) { } +// fetchCtrlStatus is the backend call, a package var so tests can stub it +// (mirrors output.go's fetchFile seam). +var fetchCtrlStatus = backend.FetchCtrlStatus + +// mergeCtrlStatus overwrites the live fields on each cluster row from the +// controller's ctrlstatus reply: cluster_name (the responder knows its own +// cluster), controller_name (current primary) and status (CONNECTED when the +// backend answered; the reply IS the liveness signal — a row whose store +// entry predates the join shows DISCONNECT until this merge). Rows the +// snapshot names differently are left untouched, so a stale-named row can +// never claim another cluster's identity. +func mergeCtrlStatus(rows []utils.OrderMap) { + st, err := fetchCtrlStatus() + if err != nil { + logger.Debug("cluster status live refresh skipped", + zap.Error(err)) + return + } + for _, row := range rows { + if name, ok := row.Data["cluster_name"].(string); !ok || name != st.ClusterName { + continue + } + if _, exist := row.Data["controller_name"]; exist { + row.Data["controller_name"] = st.PrimaryName + } + if _, exist := row.Data["status"]; exist { + row.Data["status"] = models.CLUSTER_CONNECTED + } + } +} + func processClusterData(rows []utils.OrderMap, page_token string) utils.QueryResult { for _, row := range rows { if v, exist := row.Data["status"]; exist { if v != nil { + if strVal, ok := v.(string); ok && strVal != "" { + // already a rendered status string (live merge ran) + continue + } statusVal, _ := utils.IntOrInt64(v) status, err := utils.GetKeyFromValue(models.CLUSTERSTAT, statusVal) if err != nil { diff --git a/src/frontend/apiserver/controllers/clusters_status_test.go b/src/frontend/apiserver/controllers/clusters_status_test.go new file mode 100644 index 00000000..459aff78 --- /dev/null +++ b/src/frontend/apiserver/controllers/clusters_status_test.go @@ -0,0 +1,120 @@ +package controllers + +import ( + "testing" + + "apiserver/backend" + "apiserver/models" + "apiserver/store" + "apiserver/utils" + + "go.uber.org/zap" +) + +// The cluster-info row is config-push static (OnClusterUpdated never writes +// Status/ControllerName), so the query merges the controller's live +// ctrlstatus snapshot in. These tests pin that merge: the live fields land +// on the matching row, a differently-named row is untouched, and a backend +// that doesn't answer keeps the store's last-known state. + +func init() { + utils.Log = zap.NewNop() +} + +func mkClusterRow(name string) utils.OrderMap { + return utils.OrderMap{ + Fields: []string{"cluster_name", "status", "controller_name"}, + Data: map[string]interface{}{ + "cluster_name": name, + "status": 0, // store zero value = DISCONNECT + "controller_name": "", + }, + } +} + +func TestMergeCtrlStatus_LiveFieldsLand(t *testing.T) { + store.InitStores() + orig := fetchCtrlStatus + defer func() { fetchCtrlStatus = orig }() + + fetchCtrlStatus = func() (*backend.ClusterCtrlStatus, error) { + return &backend.ClusterCtrlStatus{ + ClusterName: "eda1", + PrimaryName: "ctrl-a", + }, nil + } + + rows := []utils.OrderMap{mkClusterRow("eda1")} + mergeCtrlStatus(rows) + + if got := rows[0].Data["status"]; got != models.CLUSTER_CONNECTED { + t.Errorf("status: got %v, want %d (CONNECTED)", got, models.CLUSTER_CONNECTED) + } + if got := rows[0].Data["controller_name"]; got != "ctrl-a" { + t.Errorf("controller_name: got %v, want ctrl-a", got) + } +} + +func TestMergeCtrlStatus_DifferentNameRowUntouched(t *testing.T) { + orig := fetchCtrlStatus + defer func() { fetchCtrlStatus = orig }() + + fetchCtrlStatus = func() (*backend.ClusterCtrlStatus, error) { + return &backend.ClusterCtrlStatus{ + ClusterName: "other", + PrimaryName: "ctrl-b", + }, nil + } + + rows := []utils.OrderMap{mkClusterRow("eda1")} + mergeCtrlStatus(rows) + + if got := rows[0].Data["status"]; got != 0 { + t.Errorf("status: got %v, want 0 (untouched — snapshot names another cluster)", got) + } + if got := rows[0].Data["controller_name"]; got != "" { + t.Errorf("controller_name: got %v, want \"\" (untouched)", got) + } +} + +func TestMergeCtrlStatus_BackendDownKeepsStoreState(t *testing.T) { + orig := fetchCtrlStatus + defer func() { fetchCtrlStatus = orig }() + + fetchCtrlStatus = func() (*backend.ClusterCtrlStatus, error) { + return nil, errBackendDownTest{} + } + + rows := []utils.OrderMap{mkClusterRow("eda1")} + mergeCtrlStatus(rows) + + if got := rows[0].Data["status"]; got != 0 { + t.Errorf("status: got %v, want 0 (store state kept when backend down)", got) + } +} + +type errBackendDownTest struct{} + +func (errBackendDownTest) Error() string { return "backend unavailable" } + +func TestProcessClusterData_RendersMergedStatus(t *testing.T) { + orig := fetchCtrlStatus + defer func() { fetchCtrlStatus = orig }() + + fetchCtrlStatus = func() (*backend.ClusterCtrlStatus, error) { + return &backend.ClusterCtrlStatus{ClusterName: "eda1", PrimaryName: "ctrl-a"}, nil + } + + rows := []utils.OrderMap{mkClusterRow("eda1")} + mergeCtrlStatus(rows) + result := processClusterData(rows, "") + + rendered, ok := result.Rows[0].Data["status"].(string) + if !ok { + t.Fatalf("status should render as string after merge, got %T %v", + result.Rows[0].Data["status"], result.Rows[0].Data["status"]) + } + if rendered != "CONNECTED" { + t.Errorf("status: got %q, want CONNECTED", rendered) + } +} diff --git a/src/frontend/apiserver/models/clusters.go b/src/frontend/apiserver/models/clusters.go index 27cc2198..2d4de02b 100644 --- a/src/frontend/apiserver/models/clusters.go +++ b/src/frontend/apiserver/models/clusters.go @@ -4,6 +4,11 @@ const ( CLUSTER_RECONFIG = 1 CLUSTER_RESTART = 2 CLUSTER_SHUTDOWN = 3 + + // CLUSTER_CONNECTED is the store-level status value the cluster-info + // query stamps when the controller's ctrlstatus reply came back live. + // Its string rendering comes from CLUSTERSTAT below. + CLUSTER_CONNECTED = 0x01 ) var CLUSTERSTAT = map[string]int{ diff --git a/src/frontend/xcecli/models/cluster.go b/src/frontend/xcecli/models/cluster.go index 5d808290..fa34090f 100755 --- a/src/frontend/xcecli/models/cluster.go +++ b/src/frontend/xcecli/models/cluster.go @@ -20,3 +20,16 @@ type ClusterInfo struct { HostModels interface{} `json:"host_models"` ClusterAdmins interface{} `json:"cluster_admins"` } + +// UnmarshalJSON decodes both the OrderMap wire format the server sends for +// store-backed rows ({"Fields":[...],"Data":{...}}) and a bare object, so +// `xce id` decodes past the wrapper instead of landing all-empty. +func (c *ClusterInfo) UnmarshalJSON(data []byte) error { + type alias ClusterInfo + var a alias + if err := UnmarshalOrderMapRow(data, &a); err != nil { + return err + } + *c = ClusterInfo(a) + return nil +}