From 7a90af089c70730cf081ca1a1437710a8fef4bb1 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 3 Oct 2026 05:54:48 +0000 Subject: [PATCH] feat(controller-peers-env-fallback): route the controller's two identity loops through the shared lookup (task 2.1) Co-Authored-By: Claude Opus 5.5 (1M context) --- .../controller-peers-env-fallback/tasks.md | 2 +- src/controller/cluster.c | 29 ++-- src/controller/testsuite/test_ha_cluster.c | 136 ++++++++++++++++++ 3 files changed, 145 insertions(+), 22 deletions(-) diff --git a/.kiro/specs/controller-peers-env-fallback/tasks.md b/.kiro/specs/controller-peers-env-fallback/tasks.md index 2420b571..b1377df9 100644 --- a/.kiro/specs/controller-peers-env-fallback/tasks.md +++ b/.kiro/specs/controller-peers-env-fallback/tasks.md @@ -22,7 +22,7 @@ - Done when, after `/xce-build` (controller, scheduler, worker, harness) on a machine with no local cluster, the script runs with no cases and exits 0, and exits non-zero with the pre-flight message while a listener is bound to port 9990 or when pointed at a build directory missing a binary - [ ] 2. Controller -- [ ] 2.1 Route the controller's two existing identity loops through the shared lookup +- [x] 2.1 Route the controller's two existing identity loops through the shared lookup - Replace the loop that picks the controller's election priority, keeping the existing "at least one candidate" guard so a standalone controller still gets priority 0, and keeping "no match gives the last index" at that call site - Replace the loop that picks the listen port, keeping the existing "no match with a peers list refuses to start" and "port 0 falls back to the default port" behavior - Pure refactor: no behavior change for any input diff --git a/src/controller/cluster.c b/src/controller/cluster.c index 59600fe9..39671d3b 100644 --- a/src/controller/cluster.c +++ b/src/controller/cluster.c @@ -1780,16 +1780,9 @@ int cluster_init_phase1(cluster_t* c) int my_priority = 0; const xce_cluster_config_t* pcfg = &c->myconf->peers_cfg; if (pcfg->candidates_count > 0) { - my_priority = pcfg->candidates_count - 1; /* default: last = lowest */ - for (int i = 0; i < pcfg->candidates_count; i++) { - const char* key = pcfg->candidates[i].name[0] - ? pcfg->candidates[i].name - : pcfg->candidates[i].hostname; - if (strncmp(key, c->myconf->name, XCE_MAXNAME_LEN - 1) == 0) { - my_priority = i; - break; - } - } + /* No match: last index = lowest priority. */ + int idx = xce_peers_find_identity(pcfg, c->myconf->name); + my_priority = idx >= 0 ? idx : pcfg->candidates_count - 1; INFO("HA peers: %d candidates, my_priority=%d (name=%s)", pcfg->candidates_count, my_priority, c->myconf->name); } @@ -1873,7 +1866,8 @@ int cluster_open_listener(cluster_t* c) return 0; /* Determine listen port: - * 1. Search --peers candidates for the local name. + * 1. Look up the local name among the --peers candidates + * (xce_peers_find_identity()). * - Matched with a nonzero port: bind to it (unchanged happy * path; also how a single-node operator gets a custom port, * via a single self-referencing --peers entry). @@ -1889,16 +1883,9 @@ int cluster_open_listener(cluster_t* c) bool matched = false; { const xce_cluster_config_t* pcfg = &c->myconf->peers_cfg; - for (int j = 0; j < pcfg->candidates_count; j++) { - const char* key = pcfg->candidates[j].name[0] - ? pcfg->candidates[j].name - : pcfg->candidates[j].hostname; - if (strncmp(key, c->myconf->name, XCE_MAXNAME_LEN - 1) == 0) { - my_port = pcfg->candidates[j].port; - matched = true; - break; - } - } + int idx = xce_peers_find_identity(pcfg, c->myconf->name); + matched = idx >= 0; + my_port = matched ? (uint32_t)pcfg->candidates[idx].port : 0; if (!matched && pcfg->candidates_count > 0) { ERROR("cluster_open_listener: local name '%s' not found in " diff --git a/src/controller/testsuite/test_ha_cluster.c b/src/controller/testsuite/test_ha_cluster.c index 74201b90..1051f1be 100644 --- a/src/controller/testsuite/test_ha_cluster.c +++ b/src/controller/testsuite/test_ha_cluster.c @@ -9,11 +9,19 @@ * Instead they manually zero-init cluster_t and call ha_init() on c.ha directly. */ +#include +#include +#include #include +#include +#include #include +#include +#include #include +#include "cluster.h" #include "ha.h" #include "xce/cluster_def.h" @@ -138,6 +146,130 @@ static void test_hooks_registered_guard_prevents_double_register(void) printf("test_hooks_registered_guard_prevents_double_register: PASS\n"); } +/* ── cluster_open_listener(): which --peers candidate is this controller ── + * + * Characterization tests (controller-peers-env-fallback task 2.1): pin the + * listener's candidate match before it is routed through + * xce_peers_find_identity(). The key is the candidate's name, or its + * hostname when unnamed; a match binds that candidate's port, and no match + * with a non-empty --peers list refuses to start without opening a socket. + * The port-0/standalone default-port fallback is left out: it binds a fixed + * port (9990) that another process on this host may hold. */ + +static void listener_set_candidate(xce_cluster_config_t* cfg, int i, + const char* name, const char* hostname, + int port) +{ + snprintf(cfg->candidates[i].name, XCE_MAXNAME_LEN, "%s", name); + snprintf(cfg->candidates[i].hostname, XCE_MAXHOSTNAME_LEN, "%s", + hostname); + cfg->candidates[i].port = port; +} + +/* A zeroed cluster_t with only what cluster_open_listener() reads. */ +static cluster_t* listener_make_cluster(myconf_t* conf, const char* name) +{ + cluster_t* c = calloc(1, sizeof(*c)); + TEST_ASSERT_NOT_NULL(c); + memset(conf, 0, sizeof(*conf)); + conf->name = (char*)name; + c->myconf = conf; + c->myhost = &c->myhost_info; + c->ep.fd = -1; + return c; +} + +/* An unused TCP port: bind port 0, read it back, release it. */ +static int listener_free_port(void) +{ + struct sockaddr_in sa; + socklen_t len = sizeof(sa); + int fd = socket(AF_INET, SOCK_STREAM, 0); + TEST_ASSERT_TRUE(fd >= 0); + memset(&sa, 0, sizeof(sa)); + sa.sin_family = AF_INET; + sa.sin_addr.s_addr = htonl(INADDR_ANY); + sa.sin_port = 0; + TEST_ASSERT_EQUAL_INT(0, bind(fd, (struct sockaddr*)&sa, sizeof(sa))); + TEST_ASSERT_EQUAL_INT(0, getsockname(fd, (struct sockaddr*)&sa, &len)); + close(fd); + return ntohs(sa.sin_port); +} + +static int listener_bound_port(int fd) +{ + struct sockaddr_in sa; + socklen_t len = sizeof(sa); + TEST_ASSERT_EQUAL_INT(0, getsockname(fd, (struct sockaddr*)&sa, &len)); + return ntohs(sa.sin_port); +} + +static void test_open_listener_binds_named_candidate_port(void) +{ + myconf_t conf; + cluster_t* c = listener_make_cluster(&conf, "ctrl-b"); + int port = listener_free_port(); + + conf.peers_cfg.candidates_count = 2; + listener_set_candidate(&conf.peers_cfg, 0, "ctrl-a", "localhost", 1); + listener_set_candidate(&conf.peers_cfg, 1, "ctrl-b", "localhost", port); + + TEST_ASSERT_EQUAL_INT(0, cluster_open_listener(c)); + TEST_ASSERT_TRUE(c->ep.fd >= 0); + TEST_ASSERT_EQUAL_INT(port, listener_bound_port(c->ep.fd)); + + close(c->ep.fd); + free(c); +} + +static void test_open_listener_matches_unnamed_candidate_by_hostname(void) +{ + myconf_t conf; + cluster_t* c = listener_make_cluster(&conf, "node7"); + int port = listener_free_port(); + + conf.peers_cfg.candidates_count = 2; + listener_set_candidate(&conf.peers_cfg, 0, "ctrl-a", "localhost", 1); + listener_set_candidate(&conf.peers_cfg, 1, "", "node7", port); + + TEST_ASSERT_EQUAL_INT(0, cluster_open_listener(c)); + TEST_ASSERT_TRUE(c->ep.fd >= 0); + TEST_ASSERT_EQUAL_INT(port, listener_bound_port(c->ep.fd)); + + close(c->ep.fd); + free(c); +} + +static void test_open_listener_refuses_when_name_not_in_peers(void) +{ + myconf_t conf; + cluster_t* c = listener_make_cluster(&conf, "ctrl-z"); + + conf.peers_cfg.candidates_count = 2; + listener_set_candidate(&conf.peers_cfg, 0, "ctrl-a", "localhost", 1); + listener_set_candidate(&conf.peers_cfg, 1, "ctrl-b", "localhost", 2); + + TEST_ASSERT_EQUAL_INT(-EINVAL, cluster_open_listener(c)); + TEST_ASSERT_EQUAL_INT(-1, c->ep.fd); + + free(c); +} + +/* A named candidate is never matched by its hostname. */ +static void test_open_listener_ignores_hostname_of_named_candidate(void) +{ + myconf_t conf; + cluster_t* c = listener_make_cluster(&conf, "node7"); + + conf.peers_cfg.candidates_count = 1; + listener_set_candidate(&conf.peers_cfg, 0, "ctrl-a", "node7", 1); + + TEST_ASSERT_EQUAL_INT(-EINVAL, cluster_open_listener(c)); + TEST_ASSERT_EQUAL_INT(-1, c->ep.fd); + + free(c); +} + void test_ha_cluster(void) { UnitySetTestFile(__FILE__); @@ -146,4 +278,8 @@ void test_ha_cluster(void) RUN_TEST(test_ha_demote_to_standby_uses_c_ha); RUN_TEST(test_ha_promote_increments_term_via_cluster); RUN_TEST(test_hooks_registered_guard_prevents_double_register); + RUN_TEST(test_open_listener_binds_named_candidate_port); + RUN_TEST(test_open_listener_matches_unnamed_candidate_by_hostname); + RUN_TEST(test_open_listener_refuses_when_name_not_in_peers); + RUN_TEST(test_open_listener_ignores_hostname_of_named_candidate); }