feat(component-failover-adoption): fix worker startup ordering, add rejoin retry-on-failure
Thread cluster_cfg by value through xcecd_config_t -> xcecd_context_t -> cc_ctx so it's populated before controller_connector_start() fires the first join, instead of after xcecd_start() returns. checking_connection() now retries a failed post-reconnect xcecli_join() on the next liveness probe instead of leaving needs_rejoin cleared on mere attempt.
This commit is contained in:
parent
ce9f734a38
commit
331fa02423
SSH key fingerprint:
SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
8 changed files with 663 additions and 12 deletions
|
|
@ -34,7 +34,7 @@
|
|||
- _Boundary: WorkerControllerConnector_
|
||||
- _Depends: 1.2_
|
||||
|
||||
- [ ] 2.2 Fix worker's candidate-list startup ordering and add rejoin retry-on-failure
|
||||
- [x] 2.2 Fix worker's candidate-list startup ordering and add rejoin retry-on-failure
|
||||
- Move the candidate-list assignment to before the connector starts, so the very first connection attempt already sees the full configured list instead of only a synthesized single candidate
|
||||
- After each periodic liveness probe returns, check whether a rejoin is pending; if so, issue the rejoin call as a separate step and clear the pending indicator only if that call succeeds — on its own failure, leave the indicator set so the next liveness probe retries the rejoin rather than leaving the worker connected but never actually re-registered
|
||||
- Observable completion: starting the worker with a peer list where the first listed candidate is unreachable succeeds on the very first connect attempt, not only on a later reconnect; a rejoin call that fails once before succeeding is retried on the next liveness-probe cycle rather than abandoned
|
||||
|
|
@ -165,3 +165,4 @@
|
|||
- Task 1.2: its 8 new tests in `test_cluster.c` substantively cover task 1.3's stated checklist already (candidate order, one-candidate-no-branch, truncation-with-warning, NULL/empty rejection) — task 1.3 should scope down to just the constant-equality assertion (`N_MAX_MASTERS == MAX_HA_CANDIDATES`) rather than duplicating this coverage.
|
||||
- Task 1.3: closed with zero new code. Verified fresh (source read + a clean `xce-build.sh lib asan` run) that every item in its checklist was already satisfied by prior tasks: the `_Static_assert(N_MAX_MASTERS == MAX_HA_CANDIDATES, ...)` landed in task 1.1 (`test_libxce_retry.c:44`), and the four behavioral cases (order/no-branch/truncation/NULL-empty) landed in task 1.2's 8 tests (`test_cluster.c`). No implementer/reviewer cycle was dispatched since there was no diff to review — dispatching one would have either duplicated existing test names (build conflict) or been a no-op review of an empty diff.
|
||||
- Task 2.1: `xce_cluster_config_to_client_config()` (task 1.2) aliases `out->masters[i]` directly to `cfg->candidates[i].hostname` — no `strdup`. Any caller building the source `xce_cluster_config_t` on the stack and passing it through the helper then using the resulting client config beyond that stack frame's lifetime has a use-after-scope bug. Worker's fix keeps the source config in a file-scope `static` for this reason; tasks 3.1 (scheduler) and 4.2 (apiserver) hit the identical hazard and must do the same (or otherwise guarantee the source config outlives the client config's use).
|
||||
- Task 2.2: fixed the startup-ordering bug by threading `cluster_cfg` by value through `xcecd_config_t` → `xcecd_context_t` → `cc_ctx` (set in `main.c` before `xcecd_init()`/`xcecd_start()` run, instead of after). Safe as a plain struct copy because `xce_candidate_t.hostname` is a fixed `char[]` array, not a pointer — no aliasing hazard here (unlike the `xce_cluster_config_to_client_config()` output struct noted above). Scheduler (3.1) likely has the same ordering gap (`sched_init` loops candidates correctly already per design.md, so may not need this fix — verify against current source, don't assume). Rejoin-retry pattern established: `checking_connection()` checks `needs_rejoin` after every probe return, calls `xcecli_join()` directly (not full re-entry into the join-cluster function), clears the flag only on success — same pattern scheduler/apiserver rejoin tasks (3.1, 4.3) should follow.
|
||||
|
|
|
|||
|
|
@ -56,6 +56,13 @@ typedef struct {
|
|||
int update_interval;
|
||||
int domain_sock;
|
||||
int debug_level;
|
||||
/* HA candidate list parsed from --peers (component-failover-adoption,
|
||||
* task 2.2, Req 1.4). Threaded through xcecd_init() -> xcecd_context_t
|
||||
* -> controller_connector_init() so cc_ctx->cluster_cfg is populated
|
||||
* before the first join, instead of main.c assigning it after
|
||||
* xcecd_start() already ran. candidates_count == 0 means no --peers was
|
||||
* given (single-host default). */
|
||||
xce_cluster_config_t cluster_cfg;
|
||||
} xcecd_config_t;
|
||||
|
||||
typedef struct {
|
||||
|
|
@ -81,6 +88,11 @@ typedef struct {
|
|||
char* log_dir;
|
||||
char* db_path;
|
||||
char* virtual_name;
|
||||
/* Copied from xcecd_config_t by xcecd_init(); consumed by
|
||||
* controller_connector_init() (xcecd_start()) to populate
|
||||
* cc_ctx->cluster_cfg BEFORE controller_connector_start() fires the
|
||||
* first join (component-failover-adoption, task 2.2, Req 1.4). */
|
||||
xce_cluster_config_t cluster_cfg;
|
||||
} xcecd_context_t;
|
||||
|
||||
extern xcecd_context_t xcecd_ctx;
|
||||
|
|
|
|||
|
|
@ -331,9 +331,34 @@ int xcecd_join_cluster(hostattrs_t* attrs)
|
|||
return 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Issues the actual rejoin call: the same join payload used at
|
||||
* startup, as a separate call once the triggering call (xcecli_hello
|
||||
* or whatever else invoked the reconnect callback) has returned and
|
||||
* the client library's internal failover lock is free.
|
||||
*
|
||||
* Split out from checking_connection() behind the rejoin_call function
|
||||
* pointer below purely for testability: xcecli_join() against a real
|
||||
* candidate goes through XCECLI_FAILOVER_CALL's full retry/backoff budget
|
||||
* (up to ~100s) on failure, and a genuine success requires a live
|
||||
* capnp-speaking controller -- neither is practical to drive from a fast
|
||||
* unit test. Production always runs through this exact function via the
|
||||
* default rejoin_call pointer; only tests override the pointer to a
|
||||
* controllable double.
|
||||
*/
|
||||
static int perform_pending_rejoin(void)
|
||||
{
|
||||
return xcecli_join(xcecli_ctx, HostRole_worker, &host_attrs);
|
||||
}
|
||||
|
||||
/* Defaults to the real production call (see perform_pending_rejoin() above).
|
||||
* Overridden only by tests. */
|
||||
static int (*rejoin_call)(void) = perform_pending_rejoin;
|
||||
|
||||
/**
|
||||
* @brief Periodic liveness probe on the connector thread
|
||||
* (component-failover-adoption, task 2.1, Req 2 AC1-3, Req 3 AC1/AC3).
|
||||
* (component-failover-adoption, task 2.1, Req 2 AC1-3, Req 3 AC1/AC3;
|
||||
* task 2.2, Req 2 AC5, rejoin retry-on-failure).
|
||||
*
|
||||
* A single xcecli_hello() call. xcecli_hello() is XCECLI_FAILOVER_CALL-
|
||||
* wrapped: on a transport failure it already retries indefinitely across
|
||||
|
|
@ -346,21 +371,46 @@ int xcecd_join_cluster(hostattrs_t* attrs)
|
|||
* the connector thread's normal periodic cadence simply gives the library
|
||||
* another chance on the next call.
|
||||
*
|
||||
* Immediately after xcecli_hello() returns (its internal failover lock is
|
||||
* now free), checks needs_rejoin -- set by xcecd_on_reconnect() when the
|
||||
* library transparently reconnected during some earlier call. If set, issues
|
||||
* xcecli_join() as a separate call (the same join payload used at startup).
|
||||
* The flag clears ONLY on that join call's success (Req 2 AC5): if
|
||||
* xcecli_join() itself returns -E_XCE_NOT_READY (its own retry budget
|
||||
* exhausted) or any other failure, needs_rejoin stays set, so the worker is
|
||||
* never left transport-connected but silently un-joined -- the next
|
||||
* checking_connection() tick (this function's own periodic cadence, no new
|
||||
* timer needed) retries the rejoin.
|
||||
*
|
||||
* @return int Always returns 0.
|
||||
*/
|
||||
int checking_connection()
|
||||
{
|
||||
int ret = xcecli_hello(xcecli_ctx, "hello");
|
||||
|
||||
if (ret == 0) {
|
||||
return 0;
|
||||
if (ret != 0) {
|
||||
XCECD_LOG(XCE_LOG_ERROR,
|
||||
"Connection to controller not currently healthy (rc=%d); "
|
||||
"the client library will keep retrying/rotating "
|
||||
"candidates in the background",
|
||||
ret);
|
||||
}
|
||||
|
||||
XCECD_LOG(XCE_LOG_ERROR,
|
||||
"Connection to controller not currently healthy (rc=%d); the "
|
||||
"client library will keep retrying/rotating candidates in "
|
||||
"the background",
|
||||
ret);
|
||||
if (needs_rejoin) {
|
||||
int rc = rejoin_call();
|
||||
if (rc == 0) {
|
||||
needs_rejoin = false;
|
||||
XCECD_LOG(XCE_LOG_INFO,
|
||||
"checking_connection: rejoin after transparent "
|
||||
"reconnect succeeded");
|
||||
} else {
|
||||
XCECD_LOG(XCE_LOG_WARN,
|
||||
"checking_connection: rejoin after transparent "
|
||||
"reconnect failed (rc=%d); will retry on the next "
|
||||
"liveness probe",
|
||||
rc);
|
||||
}
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -427,7 +427,17 @@ int main(int argc, char* argv[])
|
|||
"(host=%s port=%d)",
|
||||
config.remote_host, config.remote_port);
|
||||
}
|
||||
/* use_ha and cluster_cfg are consumed after xcecd_start() sets cc_ctx */
|
||||
/* P2.2 fix (component-failover-adoption, task 2.2, Req 1.4): assign
|
||||
* cluster_cfg onto config BEFORE xcecd_init()/xcecd_start() run, so it
|
||||
* is threaded through to cc_ctx->cluster_cfg (via xcecd_ctx) before
|
||||
* controller_connector_start() fires the very first join. Previously
|
||||
* this was assigned directly onto cc_ctx AFTER xcecd_start() returned,
|
||||
* which was already too late -- the first join had already run against
|
||||
* an empty (zero-initialized) cluster_cfg, silently ignoring --peers on
|
||||
* startup. When no --peers was given, cluster_cfg stays {0}
|
||||
* (candidates_count == 0), which is exactly the "no HA list configured"
|
||||
* case the synthesize-single-candidate path already handles. */
|
||||
config.cluster_cfg = cluster_cfg;
|
||||
|
||||
struct stat st = { 0 };
|
||||
|
||||
|
|
@ -534,8 +544,10 @@ int main(int argc, char* argv[])
|
|||
return 1;
|
||||
}
|
||||
|
||||
if (use_ha && cc_ctx != NULL) {
|
||||
cc_ctx->cluster_cfg = cluster_cfg;
|
||||
if (use_ha) {
|
||||
/* cluster_cfg was already threaded into cc_ctx->cluster_cfg before
|
||||
* xcecd_start() ran (see config.cluster_cfg assignment above) --
|
||||
* this is confirmation logging only, not the point of assignment. */
|
||||
XCECD_LOG(XCE_LOG_INFO,
|
||||
"HA mode active: %d candidates, primary will be auto-discovered",
|
||||
cluster_cfg.candidates_count);
|
||||
|
|
|
|||
|
|
@ -328,6 +328,11 @@ controller_connector_init(xcecd_context_t* ctx)
|
|||
cc_ctx->thread_pool = ctx->thread_pool;
|
||||
cc_ctx->process_manager = ctx->process_manager;
|
||||
cc_ctx->worker_list = (void**)&ctx->worker_list;
|
||||
/* component-failover-adoption task 2.2 (Req 1.4): populate the full
|
||||
* candidate list here, before controller_connector_start() (called
|
||||
* right after this returns, still inside xcecd_start()) fires the very
|
||||
* first join -- so the first connect attempt already sees it. */
|
||||
cc_ctx->cluster_cfg = ctx->cluster_cfg;
|
||||
|
||||
return cc_ctx;
|
||||
}
|
||||
|
|
@ -384,6 +389,11 @@ int xcecd_init(const xcecd_config_t* config)
|
|||
xcecd_ctx.update_interval = config->update_interval;
|
||||
xcecd_ctx.join_interval = config->join_interval;
|
||||
xcecd_ctx.debug_level = config->debug_level;
|
||||
/* component-failover-adoption task 2.2 (Req 1.4): thread the parsed
|
||||
* --peers candidate list through so it reaches cc_ctx before the first
|
||||
* join, instead of main.c assigning cc_ctx->cluster_cfg after
|
||||
* xcecd_start() already ran. */
|
||||
xcecd_ctx.cluster_cfg = config->cluster_cfg;
|
||||
|
||||
/* thread pool initializing */
|
||||
xcecd_ctx.thread_pool = thread_pool_create(config->thread_pool_size);
|
||||
|
|
|
|||
|
|
@ -22,3 +22,21 @@ test('worker controller_connector test', controller_connector_test,
|
|||
suite: 'worker',
|
||||
timeout: 60,
|
||||
env: ['ASAN_OPTIONS=detect_leaks=0'])
|
||||
|
||||
# component-failover-adoption task 2.2: worker startup-ordering white-box
|
||||
# test. #include's xcecd.c directly (same technique as controller_connector_
|
||||
# test above, but a separate binary/file -- xcecd.c and controller_connector.c
|
||||
# cannot both be #include-d into one TU, see test_xcecd_startup_order.c's
|
||||
# header comment). Same -DDBUS_MANAGER + -DVIRTUAL_CLUSTER flag pair the real
|
||||
# `xcecd_virtual` executable already uses (see xcecd_virtual target above).
|
||||
xcecd_startup_order_test = executable('xcecd_startup_order_test',
|
||||
'test_xcecd_startup_order.c',
|
||||
c_args: ['-DDBUS_MANAGER', '-DVIRTUAL_CLUSTER'],
|
||||
include_directories: [worker_test_incdir],
|
||||
dependencies: worker_test_deps,
|
||||
install: false)
|
||||
|
||||
test('worker xcecd_startup_order test', xcecd_startup_order_test,
|
||||
suite: 'worker',
|
||||
timeout: 60,
|
||||
env: ['ASAN_OPTIONS=detect_leaks=0'])
|
||||
|
|
|
|||
|
|
@ -105,6 +105,9 @@ void setUp(void)
|
|||
cc_ctx = NULL;
|
||||
xcecli_ctx = NULL;
|
||||
needs_rejoin = false;
|
||||
/* Restore the real production call by default (task 2.2) -- individual
|
||||
* rejoin-retry tests below override this to a controllable double. */
|
||||
rejoin_call = perform_pending_rejoin;
|
||||
}
|
||||
|
||||
void tearDown(void) { return; }
|
||||
|
|
@ -307,6 +310,171 @@ static void test_checking_connection_never_permanently_disables_worker(void)
|
|||
elapsed_ms);
|
||||
}
|
||||
|
||||
/* ════════════════════════════════════════════════════════════════════════
|
||||
* Rejoin retry-on-failure (task 2.2, Req 2.5): after a transparent
|
||||
* reconnect sets needs_rejoin, checking_connection() must issue the actual
|
||||
* rejoin as a separate call and clear the flag ONLY on that call's success
|
||||
* -- on failure the flag must stay set so the NEXT checking_connection()
|
||||
* tick retries it, rather than leaving the worker transport-connected but
|
||||
* never actually re-registered.
|
||||
*
|
||||
* These tests override the rejoin_call function-pointer seam (controller_
|
||||
* connector.c) with a controllable double instead of exercising the real
|
||||
* xcecli_join(): a real failure goes through XCECLI_FAILOVER_CALL's full
|
||||
* retry/backoff budget (up to ~100s) before returning, and a real success
|
||||
* requires a live capnp-speaking controller -- neither is practical from a
|
||||
* fast unit test. Production always calls through the same seam via its
|
||||
* default (perform_pending_rejoin(), verified wired correctly by
|
||||
* test_rejoin_callback_registered_via_public_api's sibling tests above and
|
||||
* by direct reading of checking_connection()'s body); only these tests
|
||||
* substitute the pointer.
|
||||
* ════════════════════════════════════════════════════════════════════════ */
|
||||
|
||||
static int g_rejoin_call_count = 0;
|
||||
static int g_rejoin_call_results[4];
|
||||
static bool g_rejoin_use_scripted_results = false;
|
||||
|
||||
static int fake_rejoin_always_fails(void)
|
||||
{
|
||||
g_rejoin_call_count++;
|
||||
return -1;
|
||||
}
|
||||
|
||||
static int fake_rejoin_always_succeeds(void)
|
||||
{
|
||||
g_rejoin_call_count++;
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* Returns g_rejoin_call_results[call index] on each successive call --
|
||||
* lets a test script "fails once, then succeeds" or similar sequences. */
|
||||
static int fake_rejoin_scripted(void)
|
||||
{
|
||||
int idx = g_rejoin_call_count;
|
||||
g_rejoin_call_count++;
|
||||
if (!g_rejoin_use_scripted_results
|
||||
|| idx >= (int)(sizeof(g_rejoin_call_results)
|
||||
/ sizeof(g_rejoin_call_results[0]))) {
|
||||
return -1;
|
||||
}
|
||||
return g_rejoin_call_results[idx];
|
||||
}
|
||||
|
||||
static void test_checking_connection_skips_rejoin_when_not_pending(void)
|
||||
{
|
||||
controller_connector_ctx_t fake_cc = { 0 };
|
||||
fake_cc.running = true;
|
||||
cc_ctx = &fake_cc;
|
||||
xcecli_ctx = NULL; /* xcecli_hello fails fast */
|
||||
|
||||
needs_rejoin = false;
|
||||
g_rejoin_call_count = 0;
|
||||
rejoin_call = fake_rejoin_always_succeeds;
|
||||
|
||||
checking_connection();
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
0, g_rejoin_call_count,
|
||||
"checking_connection() must not attempt a rejoin when none is "
|
||||
"pending");
|
||||
TEST_ASSERT_FALSE(needs_rejoin);
|
||||
|
||||
printf("test_checking_connection_skips_rejoin_when_not_pending: PASS\n");
|
||||
}
|
||||
|
||||
static void test_checking_connection_rejoin_failure_keeps_flag_set(void)
|
||||
{
|
||||
controller_connector_ctx_t fake_cc = { 0 };
|
||||
fake_cc.running = true;
|
||||
cc_ctx = &fake_cc;
|
||||
xcecli_ctx = NULL;
|
||||
|
||||
needs_rejoin = true;
|
||||
g_rejoin_call_count = 0;
|
||||
rejoin_call = fake_rejoin_always_fails;
|
||||
|
||||
checking_connection();
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_rejoin_call_count,
|
||||
"rejoin must have been attempted");
|
||||
TEST_ASSERT_TRUE_MESSAGE(
|
||||
needs_rejoin,
|
||||
"a failed rejoin call must leave needs_rejoin set so the next "
|
||||
"liveness probe retries it (Req 2.5) -- it must NOT be cleared "
|
||||
"merely because the call was attempted");
|
||||
|
||||
printf("test_checking_connection_rejoin_failure_keeps_flag_set: PASS\n");
|
||||
}
|
||||
|
||||
static void test_checking_connection_rejoin_success_clears_flag(void)
|
||||
{
|
||||
controller_connector_ctx_t fake_cc = { 0 };
|
||||
fake_cc.running = true;
|
||||
cc_ctx = &fake_cc;
|
||||
xcecli_ctx = NULL;
|
||||
|
||||
needs_rejoin = true;
|
||||
g_rejoin_call_count = 0;
|
||||
rejoin_call = fake_rejoin_always_succeeds;
|
||||
|
||||
checking_connection();
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(1, g_rejoin_call_count);
|
||||
TEST_ASSERT_FALSE_MESSAGE(
|
||||
needs_rejoin,
|
||||
"a successful rejoin call must clear needs_rejoin (Req 2.5)");
|
||||
|
||||
printf("test_checking_connection_rejoin_success_clears_flag: PASS\n");
|
||||
}
|
||||
|
||||
/* The core Req 2.5 / task 2.2 observable-completion scenario: "a rejoin
|
||||
* call that fails once before succeeding is retried on the next
|
||||
* liveness-probe cycle rather than abandoned" -- exercised as TWO separate
|
||||
* checking_connection() calls (two liveness-probe ticks), not two inner
|
||||
* attempts within one call, matching design.md's "checking_connection()'s
|
||||
* existing periodic cadence is what makes this retry naturally". */
|
||||
static void
|
||||
test_checking_connection_rejoin_retried_on_next_probe_after_failure(void)
|
||||
{
|
||||
controller_connector_ctx_t fake_cc = { 0 };
|
||||
fake_cc.running = true;
|
||||
cc_ctx = &fake_cc;
|
||||
xcecli_ctx = NULL;
|
||||
|
||||
needs_rejoin = true;
|
||||
g_rejoin_call_count = 0;
|
||||
g_rejoin_use_scripted_results = true;
|
||||
g_rejoin_call_results[0] = -E_XCE_NOT_READY; /* 1st tick: fails */
|
||||
g_rejoin_call_results[1] = 0; /* 2nd tick: succeeds */
|
||||
rejoin_call = fake_rejoin_scripted;
|
||||
|
||||
/* Liveness-probe tick 1: rejoin attempted, fails, flag must stay set. */
|
||||
checking_connection();
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_rejoin_call_count,
|
||||
"tick 1 must attempt exactly one rejoin");
|
||||
TEST_ASSERT_TRUE_MESSAGE(
|
||||
needs_rejoin,
|
||||
"flag must remain set after tick 1's failed rejoin -- not "
|
||||
"abandoned");
|
||||
|
||||
/* Liveness-probe tick 2 (checking_connection()'s own periodic cadence,
|
||||
* no new timer): rejoin retried automatically because the flag is
|
||||
* still set, this time succeeds, flag clears. */
|
||||
checking_connection();
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
2, g_rejoin_call_count,
|
||||
"tick 2 must retry the rejoin since it is still pending");
|
||||
TEST_ASSERT_FALSE_MESSAGE(
|
||||
needs_rejoin,
|
||||
"flag must clear once the retried rejoin succeeds on tick 2");
|
||||
|
||||
g_rejoin_use_scripted_results = false;
|
||||
|
||||
printf(
|
||||
"test_checking_connection_rejoin_retried_on_next_probe_after_"
|
||||
"failure: PASS\n");
|
||||
}
|
||||
|
||||
/* ════════════════════════════════════════════════════════════════════════
|
||||
* main
|
||||
* ════════════════════════════════════════════════════════════════════════ */
|
||||
|
|
@ -324,6 +492,11 @@ int main(void)
|
|||
RUN_TEST(test_rejoin_callback_sets_flag_and_returns_immediately);
|
||||
RUN_TEST(test_rejoin_callback_registered_via_public_api);
|
||||
RUN_TEST(test_checking_connection_never_permanently_disables_worker);
|
||||
RUN_TEST(test_checking_connection_skips_rejoin_when_not_pending);
|
||||
RUN_TEST(test_checking_connection_rejoin_failure_keeps_flag_set);
|
||||
RUN_TEST(test_checking_connection_rejoin_success_clears_flag);
|
||||
RUN_TEST(
|
||||
test_checking_connection_rejoin_retried_on_next_probe_after_failure);
|
||||
|
||||
return UNITY_END();
|
||||
}
|
||||
|
|
|
|||
375
src/worker/testsuite/test_xcecd_startup_order.c
Normal file
375
src/worker/testsuite/test_xcecd_startup_order.c
Normal file
|
|
@ -0,0 +1,375 @@
|
|||
/*
|
||||
* test_xcecd_startup_order.c — white-box test for the worker daemon's
|
||||
* candidate-list startup ORDERING fix (component-failover-adoption, task
|
||||
* 2.2, boundary: WorkerControllerConnector, Req 1.4).
|
||||
*
|
||||
* Bug fixed: main.c used to assign cc_ctx->cluster_cfg AFTER xcecd_start()
|
||||
* had already returned -- but xcecd_start() is what creates cc_ctx
|
||||
* (controller_connector_init()) AND fires the very first join
|
||||
* (controller_connector_start()). So the first connect attempt always saw
|
||||
* cc_ctx->cluster_cfg in its zero-initialized state (candidates_count == 0)
|
||||
* and silently ignored any real --peers list on that first attempt.
|
||||
*
|
||||
* Fix: cluster_cfg is now threaded through xcecd_config_t (set by main.c
|
||||
* BEFORE calling xcecd_init()) -> xcecd_context_t (copied by xcecd_init())
|
||||
* -> cc_ctx->cluster_cfg (copied by controller_connector_init(), which
|
||||
* xcecd_start() calls before controller_connector_start()).
|
||||
*
|
||||
* This test pulls the REAL xcecd_init()/xcecd_start()/controller_connector_
|
||||
* init() production code into this TU (#include, same white-box pattern as
|
||||
* test_controller_connector.c) and drives the real call sequence main.c ->
|
||||
* xcecd_init() -> xcecd_start() uses, so a regression in the threading (or
|
||||
* in xcecd_start()'s call ordering) would make this test fail exactly the
|
||||
* way the pre-2.2 bug did.
|
||||
*
|
||||
* Why this is a SEPARATE file from test_controller_connector.c rather than
|
||||
* an extension of it: xcecd.c and controller_connector.c cannot both be
|
||||
* #include-d into the same translation unit. Both are white-box-included as
|
||||
* whole files, and both would then try to provide real, conflicting
|
||||
* definitions of shared symbols (xcecd_process_command is a real, non-static
|
||||
* function in xcecd.c but is stubbed by hand in test_controller_connector.c
|
||||
* for controller_connector.c's sake; cc_ctx is a real global controller_
|
||||
* connector.c defines that xcecd.c only declares extern). This file instead
|
||||
* stubs controller_connector.c's small exported surface that xcecd.c
|
||||
* actually calls (start_controller_connector_thread, controller_connector_
|
||||
* start, checking_connection, xcecd_job_start_error) and defines cc_ctx
|
||||
* itself -- the same "link-satisfying stub" convention test_controller_
|
||||
* connector.c already established, just pointed the other direction.
|
||||
*
|
||||
* Build flags: -DDBUS_MANAGER -DVIRTUAL_CLUSTER, the SAME flag pair the real
|
||||
* `xcecd_virtual` production executable already uses (worker/meson.build) --
|
||||
* not a new, never-exercised configuration. VIRTUAL_CLUSTER additionally
|
||||
* skips xcecd_start()'s sqlite/db-recovery block and xcecd_process_command's
|
||||
* dbus worker-launch block entirely (both `#ifndef VIRTUAL_CLUSTER`), which
|
||||
* keeps this test's stub surface to the daemon's subsystem constructors
|
||||
* (thread pool, process/system/hardware monitors, resource sync) rather than
|
||||
* also needing a real or stubbed sqlite3/dbus stack.
|
||||
*/
|
||||
#include <errno.h>
|
||||
#include <stdbool.h>
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
#include <unity/unity.h>
|
||||
|
||||
/* Pull the real xcecd_init()/xcecd_start()/controller_connector_init() logic
|
||||
* under test into this TU. */
|
||||
#include "../src/xcecd.c"
|
||||
|
||||
/* cc_ctx is normally defined in controller_connector.c (not included here --
|
||||
* see file header comment for why). */
|
||||
controller_connector_ctx_t* cc_ctx = NULL;
|
||||
|
||||
/* ════════════════════════════════════════════════════════════════════════
|
||||
* Link-satisfying stubs
|
||||
*
|
||||
* Subsystem constructors (thread pool / process / system / hardware
|
||||
* monitors / resource sync) return a non-NULL dummy pointer so xcecd_init()
|
||||
* /xcecd_start() reach their real success paths -- this test cares about
|
||||
* the cluster_cfg threading through those functions, not about hardware/
|
||||
* process monitoring behavior. Nothing in this test's runtime path
|
||||
* (xcecd_init() + xcecd_start(), never xcecd_cleanup() or
|
||||
* xcecd_process_command()) dereferences these pointers, so a shared dummy
|
||||
* buffer is sufficient and safe under ASAN.
|
||||
* ════════════════════════════════════════════════════════════════════════ */
|
||||
static char g_dummy_subsystem[4096];
|
||||
|
||||
thread_pool_t* thread_pool_create(size_t n)
|
||||
{
|
||||
(void)n;
|
||||
return (thread_pool_t*)(void*)g_dummy_subsystem;
|
||||
}
|
||||
void thread_pool_destroy(thread_pool_t* pool) { (void)pool; }
|
||||
|
||||
process_manager_t* process_manager_create(void)
|
||||
{
|
||||
return (process_manager_t*)(void*)g_dummy_subsystem;
|
||||
}
|
||||
void process_manager_destroy(process_manager_t* manager) { (void)manager; }
|
||||
bool process_manager_update(process_manager_t* manager)
|
||||
{
|
||||
(void)manager;
|
||||
return true;
|
||||
}
|
||||
|
||||
system_monitor_t* system_monitor_create(void)
|
||||
{
|
||||
return (system_monitor_t*)(void*)g_dummy_subsystem;
|
||||
}
|
||||
void system_monitor_destroy(system_monitor_t* monitor) { (void)monitor; }
|
||||
bool system_monitor_update(system_monitor_t* monitor)
|
||||
{
|
||||
(void)monitor;
|
||||
return true;
|
||||
}
|
||||
|
||||
hardware_monitor_t* hardware_monitor_create(const char* acct_file)
|
||||
{
|
||||
(void)acct_file;
|
||||
return (hardware_monitor_t*)(void*)g_dummy_subsystem;
|
||||
}
|
||||
void hardware_monitor_destroy(hardware_monitor_t* monitor) { (void)monitor; }
|
||||
bool hardware_monitor_update(hardware_monitor_t* monitor)
|
||||
{
|
||||
(void)monitor;
|
||||
return true;
|
||||
}
|
||||
int hardware_monitor_get_cpu_cores(hardware_monitor_t* monitor)
|
||||
{
|
||||
(void)monitor;
|
||||
return 4;
|
||||
}
|
||||
double hardware_monitor_get_total_memory(hardware_monitor_t* monitor)
|
||||
{
|
||||
(void)monitor;
|
||||
return 16384.0;
|
||||
}
|
||||
|
||||
network_handler_t* network_handler_create(uint16_t port)
|
||||
{
|
||||
(void)port;
|
||||
return (network_handler_t*)(void*)g_dummy_subsystem;
|
||||
}
|
||||
int network_handler_start(network_handler_t* handler)
|
||||
{
|
||||
(void)handler;
|
||||
return 0;
|
||||
}
|
||||
void network_handler_destroy(network_handler_t* handler) { (void)handler; }
|
||||
|
||||
resource_sync_t* resource_sync_create(void)
|
||||
{
|
||||
return (resource_sync_t*)(void*)g_dummy_subsystem;
|
||||
}
|
||||
void resource_sync_destroy(resource_sync_t* sync) { (void)sync; }
|
||||
bool resource_sync_start(resource_sync_t* sync)
|
||||
{
|
||||
(void)sync;
|
||||
return true;
|
||||
}
|
||||
void resource_sync_stop(resource_sync_t* sync) { (void)sync; }
|
||||
|
||||
/* Referenced by xcecd_process_command()/xcecd_cleanup() -- never invoked at
|
||||
* runtime by this test (only xcecd_init()/xcecd_start() are called), but
|
||||
* their bodies are compiled unconditionally so the symbols must resolve. */
|
||||
void add_worker(xcecd_worker_t** worker_list, int worker_pid,
|
||||
int worker_status, uint64_t job_id, int ntasks,
|
||||
job_startinfo_t* startinfo)
|
||||
{
|
||||
(void)worker_list;
|
||||
(void)worker_pid;
|
||||
(void)worker_status;
|
||||
(void)job_id;
|
||||
(void)ntasks;
|
||||
(void)startinfo;
|
||||
}
|
||||
bool xcecd_db_insert_worker(xcecd_db_t* db, int worker_pid, int worker_status,
|
||||
uint64_t job_id, int job_status)
|
||||
{
|
||||
(void)db;
|
||||
(void)worker_pid;
|
||||
(void)worker_status;
|
||||
(void)job_id;
|
||||
(void)job_status;
|
||||
return true;
|
||||
}
|
||||
bool ipc_master_check_connection(void) { return true; }
|
||||
pid_t dbus_get_worker_pid(const char* unit_name)
|
||||
{
|
||||
(void)unit_name;
|
||||
return 1;
|
||||
}
|
||||
void dbus_stop_all_workers(xcecd_worker_t** worker_list, sqlite3* db)
|
||||
{
|
||||
(void)worker_list;
|
||||
(void)db;
|
||||
}
|
||||
void cleanup_worker_list(xcecd_worker_t** worker_list, sqlite3* db)
|
||||
{
|
||||
(void)worker_list;
|
||||
(void)db;
|
||||
}
|
||||
void xcecd_db_close(xcecd_db_t* db) { (void)db; }
|
||||
void xcecd_db_is_available(xcecd_db_t* db, const char* db_path)
|
||||
{
|
||||
(void)db;
|
||||
(void)db_path;
|
||||
}
|
||||
int submit_fork_task(thread_pool_t* pool, fork_task_t* task)
|
||||
{
|
||||
(void)pool;
|
||||
(void)task;
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* controller_connector.c's small exported surface xcecd.c actually calls
|
||||
* (verified by grep -- start_controller_connector_thread,
|
||||
* controller_connector_start, checking_connection, xcecd_job_start_error
|
||||
* are the only 4). */
|
||||
int xcecd_job_start_error(uint64_t jobid)
|
||||
{
|
||||
(void)jobid;
|
||||
return 0;
|
||||
}
|
||||
int checking_connection(void) { return 0; }
|
||||
|
||||
/* controller_connector_start() is the function that fires the very first
|
||||
* join attempt in production (xcecd_join_cluster() inside its retry loop).
|
||||
* This test double stands in for it and snapshots cc_ctx->cluster_cfg at
|
||||
* the EXACT moment production would fire that first attempt -- the core
|
||||
* Req 1.4 assertion: was the real candidate list already there by then? */
|
||||
static xce_cluster_config_t g_captured_cluster_cfg_at_first_join;
|
||||
static bool g_controller_connector_start_called = false;
|
||||
|
||||
bool controller_connector_start(void)
|
||||
{
|
||||
g_controller_connector_start_called = true;
|
||||
if (cc_ctx) {
|
||||
g_captured_cluster_cfg_at_first_join = cc_ctx->cluster_cfg;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
int start_controller_connector_thread(task_func_t function, void* arg)
|
||||
{
|
||||
/* Deliberately does NOT start a real thread -- connector_to_controller
|
||||
* (which would call the real checking_connection() in a loop) must
|
||||
* never actually run in this unit test. */
|
||||
(void)function;
|
||||
(void)arg;
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* ════════════════════════════════════════════════════════════════════════
|
||||
* Fixtures
|
||||
* ════════════════════════════════════════════════════════════════════════ */
|
||||
|
||||
static xcecd_config_t base_test_config(void)
|
||||
{
|
||||
xcecd_config_t config = { 0 };
|
||||
config.thread_pool_size = 1;
|
||||
config.resource_update_interval = 1;
|
||||
config.job_status_update_interval = 1;
|
||||
config.port = 0; /* skip network_handler entirely */
|
||||
config.acct_file = "/tmp/test_xcecd_startup_order.acct";
|
||||
config.log_file = "/tmp/test_xcecd_startup_order.log";
|
||||
config.db_path = "/tmp/test_xcecd_startup_order.db";
|
||||
config.work_dir = "/tmp";
|
||||
config.domain_sock_file = "/tmp/.test_xcecd_startup_order.sock";
|
||||
config.virutal_name = "workertest";
|
||||
config.remote_host = "localhost";
|
||||
config.remote_port = 9990;
|
||||
config.join_interval = 1;
|
||||
config.update_interval = 1;
|
||||
config.debug_level = 2;
|
||||
return config;
|
||||
}
|
||||
|
||||
void setUp(void)
|
||||
{
|
||||
memset(&xcecd_ctx, 0, sizeof(xcecd_ctx));
|
||||
cc_ctx = NULL;
|
||||
g_controller_connector_start_called = false;
|
||||
memset(&g_captured_cluster_cfg_at_first_join, 0,
|
||||
sizeof(g_captured_cluster_cfg_at_first_join));
|
||||
}
|
||||
|
||||
void tearDown(void) { return; }
|
||||
|
||||
/* ════════════════════════════════════════════════════════════════════════
|
||||
* Req 1.4 — cc_ctx->cluster_cfg is fully populated BEFORE the first join
|
||||
* attempt fires, for a multi-candidate --peers config.
|
||||
* ════════════════════════════════════════════════════════════════════════ */
|
||||
|
||||
static void
|
||||
test_multi_candidate_cluster_cfg_populated_before_first_join(void)
|
||||
{
|
||||
xce_cluster_config_t peers_cfg = { 0 };
|
||||
peers_cfg.candidates_count = 3;
|
||||
strncpy(peers_cfg.candidates[0].hostname, "ctrl-a",
|
||||
XCE_MAXHOSTNAME_LEN - 1);
|
||||
peers_cfg.candidates[0].port = 9991;
|
||||
strncpy(peers_cfg.candidates[1].hostname, "ctrl-b",
|
||||
XCE_MAXHOSTNAME_LEN - 1);
|
||||
peers_cfg.candidates[1].port = 9992;
|
||||
strncpy(peers_cfg.candidates[2].hostname, "ctrl-c",
|
||||
XCE_MAXHOSTNAME_LEN - 1);
|
||||
peers_cfg.candidates[2].port = 9993;
|
||||
strncpy(peers_cfg.identity, "workertest", sizeof(peers_cfg.identity) - 1);
|
||||
|
||||
xcecd_config_t config = base_test_config();
|
||||
/* This is main.c's fix under test: cluster_cfg is set onto the config
|
||||
* BEFORE xcecd_init()/xcecd_start() run (main.c:~430), not assigned onto
|
||||
* cc_ctx afterward. */
|
||||
config.cluster_cfg = peers_cfg;
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, xcecd_init(&config));
|
||||
TEST_ASSERT_EQUAL_INT(0, xcecd_start());
|
||||
|
||||
TEST_ASSERT_TRUE_MESSAGE(
|
||||
g_controller_connector_start_called,
|
||||
"controller_connector_start() (the first join attempt) must have "
|
||||
"been invoked by xcecd_start()");
|
||||
|
||||
/* The core assertion: by the time the first join fires, cc_ctx-> */
|
||||
/* cluster_cfg already holds the REAL 3-candidate --peers list -- not */
|
||||
/* the zero-initialized/synthesized-single-candidate shape the pre-2.2 */
|
||||
/* bug would have produced here. */
|
||||
TEST_ASSERT_EQUAL_INT(
|
||||
3, g_captured_cluster_cfg_at_first_join.candidates_count);
|
||||
TEST_ASSERT_EQUAL_STRING(
|
||||
"ctrl-a", g_captured_cluster_cfg_at_first_join.candidates[0].hostname);
|
||||
TEST_ASSERT_EQUAL_INT(
|
||||
9991, g_captured_cluster_cfg_at_first_join.candidates[0].port);
|
||||
TEST_ASSERT_EQUAL_STRING(
|
||||
"ctrl-b", g_captured_cluster_cfg_at_first_join.candidates[1].hostname);
|
||||
TEST_ASSERT_EQUAL_INT(
|
||||
9992, g_captured_cluster_cfg_at_first_join.candidates[1].port);
|
||||
TEST_ASSERT_EQUAL_STRING(
|
||||
"ctrl-c", g_captured_cluster_cfg_at_first_join.candidates[2].hostname);
|
||||
TEST_ASSERT_EQUAL_INT(
|
||||
9993, g_captured_cluster_cfg_at_first_join.candidates[2].port);
|
||||
|
||||
printf(
|
||||
"test_multi_candidate_cluster_cfg_populated_before_first_join: "
|
||||
"PASS\n");
|
||||
}
|
||||
|
||||
/* Regression guard: the no-`--peers` case (cluster_cfg left {0}) must still
|
||||
* reach the first join attempt with an EMPTY cc_ctx->cluster_cfg (candidates_
|
||||
* count == 0) -- the synthesize-single-candidate path inside build_cluster_
|
||||
* config() (controller_connector.c, task 2.1) owns turning that into a
|
||||
* one-candidate list; this ordering fix must not accidentally force a
|
||||
* candidate in cc_ctx->cluster_cfg when none was configured. */
|
||||
static void test_no_peers_cluster_cfg_stays_empty_before_first_join(void)
|
||||
{
|
||||
xcecd_config_t config = base_test_config();
|
||||
/* config.cluster_cfg left {0} -- no --peers given. */
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, xcecd_init(&config));
|
||||
TEST_ASSERT_EQUAL_INT(0, xcecd_start());
|
||||
|
||||
TEST_ASSERT_TRUE(g_controller_connector_start_called);
|
||||
TEST_ASSERT_EQUAL_INT(
|
||||
0, g_captured_cluster_cfg_at_first_join.candidates_count);
|
||||
|
||||
printf(
|
||||
"test_no_peers_cluster_cfg_stays_empty_before_first_join: PASS\n");
|
||||
}
|
||||
|
||||
/* ════════════════════════════════════════════════════════════════════════
|
||||
* main
|
||||
* ════════════════════════════════════════════════════════════════════════ */
|
||||
|
||||
int main(void)
|
||||
{
|
||||
xce_init_xcecd_log("/tmp/test_xcecd_startup_order.log", 1024 * 1024, 2);
|
||||
|
||||
UNITY_BEGIN();
|
||||
|
||||
RUN_TEST(test_multi_candidate_cluster_cfg_populated_before_first_join);
|
||||
RUN_TEST(test_no_peers_cluster_cfg_stays_empty_before_first_join);
|
||||
|
||||
return UNITY_END();
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue