From 03813bf5609de69ec4af9feef246f4ff1e8b7f3d Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 20 Jun 2026 05:32:18 +0000 Subject: [PATCH] feat(controller-status): xcecli_ctrlstatus_primary authoritative reach-the-primary read (task 3.2) --- .kiro/specs/controller-status/tasks.md | 3 +- src/include/client.h | 25 +++++ src/lib/client.c | 128 +++++++++++++++++++++++ src/lib/testsuite/test_ctrlstatus.c | 137 ++++++++++++++++++++++++- 4 files changed, 291 insertions(+), 2 deletions(-) diff --git a/.kiro/specs/controller-status/tasks.md b/.kiro/specs/controller-status/tasks.md index add6387..36a4dcb 100644 --- a/.kiro/specs/controller-status/tasks.md +++ b/.kiro/specs/controller-status/tasks.md @@ -34,7 +34,7 @@ - _Requirements: 1.1, 7.1, 7.2, 7.3_ - _Boundary: libxce client_ - _Depends: 1.1, 2.1, 2.2_ -- [ ] 3.2 Add the authoritative reach-the-primary status call +- [x] 3.2 Add the authoritative reach-the-primary status call - Add a client call that returns only an authoritative reply (responder role == primary). When the first answer is from a non-primary, follow the redirect — target the reported primary via the reply's peer list, otherwise rotate the configured controller list — until the primary answers or the failover budget is exhausted. - TDD: a test where the first reachable node is a standby confirms the call follows the hint to the primary and returns a primary-role reply; a no-serving-primary test returns failure (never a standby hint as the authoritative result). - Observable: the authoritative call returns a reply whose responder role is primary even when a standby answered first, and fails (does not return a hint) when no primary serves. @@ -70,3 +70,4 @@ - 1.1 LEARNING (affects later tasks): the `$C.mapname(...)` capnp types are NOT auto-generated in this schema — the generated `controller.capnp.c` references HAND-AUTHORED C typedefs in `src/include/typemap.h` (same convention as `xce_clusterconf_t`). So `xce_ctrlstatus_t`/`xce_ctrlpeer_t` (and the `xce_reply_t.data.ctrlstatus` union member) live in `typemap.h`, verified field-for-field against the regenerated codec. `OPCode_ctrlstatus = 40`, `RequestReply_data_ctrlstatus = 12`. `configChecksum` is `xce_data_t*`. The handler/libxce tasks build/encode against these typemap.h types. - 2.1 LEARNING + SCHEMA AMENDMENT: capnp `Data` in THIS schema maps to the project's STRUCTURED `xce_data_t` union (table-switched), NOT raw bytes — `encode_Data_ptr` can't serialize a SHA-256 blob. So `configChecksum @6` was changed `Data → Text` (a 64-char lowercase-hex string of the full 32-byte `config_sha256`); typemap member is `char*`; freed in `ctrlstatus_free_fields`. Ordinal @6 unchanged, no released decoder yet → safe. Handler lives in NEW `src/controller/ctrlstatus.c` (`ctrlstatus_fill`/`ctrlstatus_free_fields`/`handle_ctrlstatus`), proto in `src/controller/include/cluster.h`, routed at `cluster.c` opcode switch, test `src/controller/testsuite/test_ctrlstatus.c` (registered in `controller_test.c`, built via `controller/meson.build`). `primary_host` read under `ha->lock`; peer name/host strings are BORROWED from myconf (not freed). The libxce client (3.x) decodes `configChecksum` as Text. - 3.1 LEARNING: the ctx-side reply union `xce_ctx_t.reply.u` (in `src/include/xcectx.h`) is SEPARATE from `xce_reply_t.data` (typemap.h) — the decode path needs BOTH a `ctrlstatus` member there AND a `case OPCode_ctrlstatus` in `xcectx.c` handle_reply (mirror clusterconf). Free: clusterconf's `xce_free_clusterconf` macro is a LATENT-BROKEN alias (undefined symbol, never expanded); use the REAL generated free — `xce_free_ctrlstatus(s) → free_CtrlStatus(s)`, `xcecli_free_ctrlstatus(s) → xce_free_ctrlstatus(s)` (any TU expanding it must include the generated `controller.capnp.h`). `xcecli_ctrlstatus` is is_write=false; NULL ctx → `-EINVAL`. +- 3.2: `xcecli_ctrlstatus_primary` = bounded loop (cap `failover.retry.max_retries`) reusing `xcecli_ctrlstatus` + `xce_ctx_capture_redirect_hint` (map `primaryName`→addr via `peers[]`, else master rotation); authoritative iff `responderRole == CTRL_PRIMARY` (== 0; `ctrl_role_t` PRIMARY=0/STANDBY=1/INIT=2/ELECTING=3 in `ha.h` — REVALIDATION TRIGGER: a role-enum reorder breaks the pinned wire ordinal `XCECLI_CTRLSTATUS_ROLE_PRIMARY=0`). On exhaustion → `-E_XCE_NOT_READY`, `*status` untouched (never a hint). Pure helpers `xcecli_ctrlstatus_is_authoritative`/`_primary_addr` unit-tested; live standby→primary redirect is 5.1. diff --git a/src/include/client.h b/src/include/client.h index 190567c..e7eb065 100644 --- a/src/include/client.h +++ b/src/include/client.h @@ -2,6 +2,8 @@ #define _CLIENT_H_ 1 +#include +#include #include #include "netio.h" @@ -54,6 +56,29 @@ extern int xcecli_jobdispatch(xcecli_ctx_t, xce_job_dispatch_t*); extern int xcecli_joblist(xcecli_ctx_t, xce_job_t***); extern int xcecli_clusterconf(xcecli_ctx_t, xce_clusterconf_t**); extern int xcecli_ctrlstatus(xcecli_ctx_t, xce_ctrlstatus_t**); +/* + * xcecli_ctrlstatus_primary (controller-status task 3.2): AUTHORITATIVE + * controller status read. Returns ONLY a reply whose responder role is the + * primary. When the first answer is from a non-primary, it follows the redirect + * (targets the reported primary via the reply's peer list, else rotates the + * configured controller list) until the primary answers or the failover budget + * is exhausted. On budget exhaustion / no reachable primary it returns a typed + * negative and NEVER hands back a non-primary hint reply (Req 9.1, 9.2). The + * any-node hint view is still available via xcecli_ctrlstatus above. + */ +extern int xcecli_ctrlstatus_primary(xcecli_ctx_t, xce_ctrlstatus_t**); +/* + * Pure decision helpers backing the authoritative read (no I/O — unit-testable + * and exposed for tests). xcecli_ctrlstatus_is_authoritative returns true iff + * the reply's responderRole is the primary role (ctrl_role_t PRIMARY == 0). + * xcecli_ctrlstatus_primary_addr maps the reply's primaryName to its host:port + * via the reply's peers list: returns 0 and fills host/port on a hit, -ENOENT + * when the primary is unknown/absent from peers, -EINVAL on a bad argument. + */ +extern bool xcecli_ctrlstatus_is_authoritative(const xce_ctrlstatus_t*); +extern int xcecli_ctrlstatus_primary_addr(const xce_ctrlstatus_t*, + char* host_out, size_t host_sz, + uint16_t* port_out); extern int xcecli_notify_jobfinish(xcecli_ctx_t, xce_jobfinish_t*); extern int xcecli_notify_jobstart(xcecli_ctx_t, job_pidinfo_t*); extern int xcecli_notify_jobfail(xcecli_ctx_t c, job_failinfo_t*); diff --git a/src/lib/client.c b/src/lib/client.c index 01d3c3f..d7eb573 100644 --- a/src/lib/client.c +++ b/src/lib/client.c @@ -1,6 +1,7 @@ #include #include #include +#include #include #include "client.h" @@ -9,6 +10,7 @@ #include "mylock.h" #include "request.h" +#include "xce/errnox.h" /* E_XCE_NOT_READY */ #include "xce/types.h" #include "xceapi.h" #include "xcectx.h" @@ -336,6 +338,132 @@ int xcecli_ctrlstatus(xcecli_ctx_t c, xce_ctrlstatus_t** status) xce_ctrlstatus(&(ctx->ctx), status)); } +/* + * The primary's responder role on the wire is ctrl_role_t PRIMARY. That enum + * lives in the controller-private ha.h (CTRL_PRIMARY == 0) and is intentionally + * not exposed to libxce; the wire ordinal is part of the ctrlstatus reply + * contract (controller/ctrlstatus.c stamps responderRole = ha->role). We pin it + * here as a named constant so the comparison reads intentionally rather than as + * a bare 0. Any reorder of ctrl_role_t is a Revalidation Trigger for this spec. + */ +#define XCECLI_CTRLSTATUS_ROLE_PRIMARY ((uint8_t)0) + +bool xcecli_ctrlstatus_is_authoritative(const xce_ctrlstatus_t* status) +{ + if (status == NULL) { + return false; + } + return status->responderRole == XCECLI_CTRLSTATUS_ROLE_PRIMARY; +} + +int xcecli_ctrlstatus_primary_addr(const xce_ctrlstatus_t* status, + char* host_out, size_t host_sz, + uint16_t* port_out) +{ + int i; + + if ((status == NULL) || (host_out == NULL) || (host_sz == 0) + || (port_out == NULL)) { + return -EINVAL; + } + + /* No known primary (empty/NULL primaryName) ⇒ nothing to target; the caller + * falls back to plain master rotation (Req 2.3 unknown-primary). */ + if ((status->primaryName == NULL) || (status->primaryName[0] == '\0')) { + return -ENOENT; + } + + /* Map the reported primaryName to its configured address via the reply's + * peers list. The list is a small bounded set (configured controllers). */ + for (i = 0; i < status->peers.count; i++) { + xce_ctrlpeer_t* p = status->peers.list[i]; + + if ((p == NULL) || (p->name == NULL) || (p->host == NULL)) { + continue; + } + if (strcmp(p->name, status->primaryName) != 0) { + continue; + } + if ((p->host[0] == '\0') || (p->port == 0)) { + /* named primary has no usable address — treat as absent */ + break; + } + + strncpy(host_out, p->host, host_sz - 1); + host_out[host_sz - 1] = '\0'; + *port_out = (uint16_t)p->port; + return 0; + } + + /* primaryName not present in peers (or no usable address): fall back to + * rotation rather than targeting a bogus address. */ + return -ENOENT; +} + +int xcecli_ctrlstatus_primary(xcecli_ctx_t c, xce_ctrlstatus_t** status) +{ + api_ctx_t* ctx = (api_ctx_t*)c; + xce_ctrlstatus_t* hint; + int rc; + int attempt; + int max_attempts; + + if ((c == NULL) || (status == NULL)) { + return -EINVAL; + } + + /* Bound the redirect loop by the configured failover retry budget so we + * never spin unbounded chasing a never-appearing primary (each inner + * xcecli_ctrlstatus already runs its own full reach-some-node failover). */ + max_attempts = ctx->failover.retry.max_retries; + if (max_attempts < 1) { + max_attempts = 1; + } + + for (attempt = 0; attempt < max_attempts; attempt++) { + hint = NULL; + + rc = xcecli_ctrlstatus(c, &hint); + if (rc < 0) { + /* no reachable controller within the inner budget ⇒ typed failure; + * *status untouched (never a hint reply as the result, Req 9.2). */ + return rc; + } + + if (xcecli_ctrlstatus_is_authoritative(hint)) { + /* the primary itself answered — authoritative result */ + *status = hint; + return 0; + } + + /* Non-authoritative HINT (standby/electing/init). Direct the next query + * at the reported primary if we can map it to an address; otherwise let + * the next xcecli_ctrlstatus rotate the configured master list. The + * redirect hint is consumed one-shot by xce_failover_next_target. */ + { + char phost[XCE_MAXHOSTNAME_LEN]; + uint16_t pport = 0; + + if (xcecli_ctrlstatus_primary_addr(hint, phost, sizeof(phost), + &pport) + == 0) { + mylock_lock(&ctx->failover.lock); + xce_ctx_capture_redirect_hint(&ctx->ctx, phost, pport); + mylock_unlock(&ctx->failover.lock); + } + } + + /* Free the non-authoritative interim reply before the next attempt — no + * leak, and we never return a hint as the authoritative result. */ + xcecli_free_ctrlstatus(hint); + } + + /* Budget exhausted without the primary answering: typed failure, *status + * untouched (Req 9.2 — never return a non-primary reply as authoritative). + */ + return -E_XCE_NOT_READY; +} + int xcecli_notify_jobfinish(xcecli_ctx_t c, xce_jobfinish_t* jfinish) { api_ctx_t* ctx = (api_ctx_t*)c; diff --git a/src/lib/testsuite/test_ctrlstatus.c b/src/lib/testsuite/test_ctrlstatus.c index 3aad11c..d76c02a 100644 --- a/src/lib/testsuite/test_ctrlstatus.c +++ b/src/lib/testsuite/test_ctrlstatus.c @@ -21,6 +21,7 @@ #include #include "client.h" +#include "config.h" /* XCE_MAXHOSTNAME_LEN */ #include "request.h" #include "xce/types.h" @@ -195,12 +196,141 @@ static void test_ctrlstatus_roundtrip_unknown_primary(void) static void test_xcecli_ctrlstatus_null_ctx_typed_failure(void) { xce_ctrlstatus_t* status = (xce_ctrlstatus_t*)0xdeadbeef; - int rc = xcecli_ctrlstatus(NULL, &status); + int rc = xcecli_ctrlstatus(NULL, &status); /* typed negative, not a partial/empty success */ TEST_ASSERT_EQUAL_INT(-EINVAL, rc); printf("test_xcecli_ctrlstatus_null_ctx_typed_failure: PASS\n"); } +/* ── authoritative (primary-only) read: task 3.2 ─────────────────────────── */ + +/* The authoritative call must reject a NULL/unconnected ctx with the same + * typed negative as the any-node read — it must NEVER hand back a hint reply + * as the authoritative result (Req 9.2). The `status` out-param is left + * untouched on failure (it still holds the poisoned sentinel below). */ +static void test_xcecli_ctrlstatus_primary_null_ctx_typed_failure(void) +{ + xce_ctrlstatus_t* status = (xce_ctrlstatus_t*)0xdeadbeef; + int rc = xcecli_ctrlstatus_primary(NULL, &status); + TEST_ASSERT_EQUAL_INT(-EINVAL, rc); + /* never returned a hint reply as the authoritative result */ + TEST_ASSERT_EQUAL_PTR((xce_ctrlstatus_t*)0xdeadbeef, status); + printf("test_xcecli_ctrlstatus_primary_null_ctx_typed_failure: PASS\n"); +} + +/* NULL status out-param is also a typed failure (boundary validation). */ +static void test_xcecli_ctrlstatus_primary_null_out_typed_failure(void) +{ + int rc = xcecli_ctrlstatus_primary(NULL, NULL); + TEST_ASSERT_EQUAL_INT(-EINVAL, rc); + printf("test_xcecli_ctrlstatus_primary_null_out_typed_failure: PASS\n"); +} + +/* The pure authoritative-decision helper: a reply is authoritative iff the + * responder's role is PRIMARY (ctrl_role_t PRIMARY == 0). A standby (1), + * init (2), or electing (3) responder is a non-authoritative HINT — the call + * must keep following the redirect (Req 9.1, 2.3, 3.2). */ +static void test_ctrlstatus_is_authoritative_decision(void) +{ + xce_ctrlstatus_t reply; + memset(&reply, 0, sizeof(reply)); + + reply.responderRole = 0; /* PRIMARY */ + TEST_ASSERT_TRUE(xcecli_ctrlstatus_is_authoritative(&reply)); + + reply.responderRole = 1; /* STANDBY */ + TEST_ASSERT_FALSE(xcecli_ctrlstatus_is_authoritative(&reply)); + + reply.responderRole = 2; /* INIT */ + TEST_ASSERT_FALSE(xcecli_ctrlstatus_is_authoritative(&reply)); + + reply.responderRole = 3; /* ELECTING */ + TEST_ASSERT_FALSE(xcecli_ctrlstatus_is_authoritative(&reply)); + + /* NULL reply is never authoritative (defensive). */ + TEST_ASSERT_FALSE(xcecli_ctrlstatus_is_authoritative(NULL)); + + printf("test_ctrlstatus_is_authoritative_decision: PASS\n"); +} + +/* The pure next-target helper: given a standby's hint reply, map the reported + * primaryName to its host:port via the reply's peers list so the next query + * can target the primary directly (the redirect step of Req 9.1). */ +static void test_ctrlstatus_primary_addr_from_peers(void) +{ + xce_ctrlpeer_t p0 = { .name = "ctrl-a", .host = "10.0.0.1", .port = 9991 }; + xce_ctrlpeer_t p1 = { .name = "ctrl-b", .host = "10.0.0.2", .port = 9992 }; + xce_ctrlpeer_t* peerlist[2] = { &p0, &p1 }; + + xce_ctrlstatus_t hint; + memset(&hint, 0, sizeof(hint)); + hint.responderName = "ctrl-a"; + hint.responderRole = 1; /* STANDBY */ + hint.primaryName = "ctrl-b"; /* the standby knows ctrl-b is primary */ + hint.peers.count = 2; + hint.peers.list = peerlist; + hint.peerCount = 2; + + char host[XCE_MAXHOSTNAME_LEN]; + uint16_t port = 0; + int rc = xcecli_ctrlstatus_primary_addr(&hint, host, sizeof(host), &port); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_STRING("10.0.0.2", host); + TEST_ASSERT_EQUAL_UINT16(9992, port); + + printf("test_ctrlstatus_primary_addr_from_peers: PASS\n"); +} + +/* When the standby reports no known primary (empty primaryName) or the named + * primary is not in the peers list, the address lookup fails (-ENOENT) so the + * caller falls back to plain master rotation rather than targeting a bogus + * address (Req 2.3 unknown-primary, redirect best-effort). */ +static void test_ctrlstatus_primary_addr_unknown_or_missing(void) +{ + xce_ctrlpeer_t p0 = { .name = "ctrl-a", .host = "10.0.0.1", .port = 9991 }; + xce_ctrlpeer_t* peerlist[1] = { &p0 }; + + xce_ctrlstatus_t hint; + memset(&hint, 0, sizeof(hint)); + hint.responderName = "ctrl-a"; + hint.responderRole = 3; /* ELECTING */ + hint.peers.count = 1; + hint.peers.list = peerlist; + hint.peerCount = 1; + + char host[XCE_MAXHOSTNAME_LEN]; + uint16_t port = 7; + + /* unknown primary (empty) */ + hint.primaryName = ""; + TEST_ASSERT_EQUAL_INT(-ENOENT, xcecli_ctrlstatus_primary_addr( + &hint, host, sizeof(host), &port)); + + /* NULL primary */ + hint.primaryName = NULL; + TEST_ASSERT_EQUAL_INT(-ENOENT, xcecli_ctrlstatus_primary_addr( + &hint, host, sizeof(host), &port)); + + /* named primary not present in peers */ + hint.primaryName = "ctrl-z"; + TEST_ASSERT_EQUAL_INT(-ENOENT, xcecli_ctrlstatus_primary_addr( + &hint, host, sizeof(host), &port)); + + /* NULL/zero args are rejected */ + hint.primaryName = "ctrl-a"; + TEST_ASSERT_EQUAL_INT(-EINVAL, xcecli_ctrlstatus_primary_addr( + NULL, host, sizeof(host), &port)); + TEST_ASSERT_EQUAL_INT(-EINVAL, xcecli_ctrlstatus_primary_addr( + &hint, NULL, sizeof(host), &port)); + TEST_ASSERT_EQUAL_INT( + -EINVAL, xcecli_ctrlstatus_primary_addr(&hint, host, 0, &port)); + TEST_ASSERT_EQUAL_INT( + -EINVAL, + xcecli_ctrlstatus_primary_addr(&hint, host, sizeof(host), NULL)); + + printf("test_ctrlstatus_primary_addr_unknown_or_missing: PASS\n"); +} + /* ── Suite entry point ──────────────────────────────────────────────────── */ void test_ctrlstatus(void) @@ -208,4 +338,9 @@ void test_ctrlstatus(void) RUN_TEST(test_ctrlstatus_roundtrip_all_fields); RUN_TEST(test_ctrlstatus_roundtrip_unknown_primary); RUN_TEST(test_xcecli_ctrlstatus_null_ctx_typed_failure); + RUN_TEST(test_xcecli_ctrlstatus_primary_null_ctx_typed_failure); + RUN_TEST(test_xcecli_ctrlstatus_primary_null_out_typed_failure); + RUN_TEST(test_ctrlstatus_is_authoritative_decision); + RUN_TEST(test_ctrlstatus_primary_addr_from_peers); + RUN_TEST(test_ctrlstatus_primary_addr_unknown_or_missing); }