Skip to content

Commit 82b1ec9

Browse files
authored
Merge pull request #2592 from DeusData/fix/cli-activation-fixture-no-deadlines
test(cli): the activation-maintenance fixture and the drained probe end on definite events
2 parents 1691be7 + 1aa0d74 commit 82b1ec9

1 file changed

Lines changed: 35 additions & 39 deletions

File tree

‎tests/test_cli.c‎

Lines changed: 35 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,9 +1309,19 @@ TEST(cli_activation_quiesce_does_not_wait_on_bootstrap_startup) {
13091309
test_rmdir_r(tmpdir);
13101310
FAIL("pipe failed");
13111311
}
1312+
/* The parent closes the write end once its install has returned, so the
1313+
* child's watch for the maintenance request ends on a definite event. */
1314+
int install_done_pipe[2] = {-1, -1};
1315+
if (pipe(install_done_pipe) != 0) {
1316+
close(ready_pipe[0]);
1317+
close(ready_pipe[1]);
1318+
test_rmdir_r(tmpdir);
1319+
FAIL("pipe failed");
1320+
}
13121321
pid_t child = fork();
13131322
if (child == 0) {
13141323
close(ready_pipe[0]);
1324+
close(install_done_pipe[1]);
13151325
cbm_daemon_ipc_endpoint_t *endpoint = cbm_daemon_bootstrap_endpoint_new(runtime_parent);
13161326
cbm_version_cohort_manager_t *manager =
13171327
endpoint ? cbm_version_cohort_manager_new(endpoint) : NULL;
@@ -1330,21 +1340,21 @@ TEST(cli_activation_quiesce_does_not_wait_on_bootstrap_startup) {
13301340
bool setup =
13311341
endpoint && manager &&
13321342
cbm_daemon_runtime_process_build_fingerprint((uint64_t)getpid(), fingerprint) &&
1333-
cbm_version_cohort_acquire(manager, &identity, cbm_now_ms() + 45000U, &lease,
1334-
&conflict) == CBM_VERSION_COHORT_OK &&
1343+
/* UINT64_MAX: wait as long as a lock is held, never for a
1344+
* conflicting holder, so no deadline decides the join. */
1345+
cbm_version_cohort_acquire(manager, &identity, UINT64_MAX, &lease, &conflict) ==
1346+
CBM_VERSION_COHORT_OK &&
13351347
cbm_daemon_ipc_startup_lock_try_acquire(endpoint, &startup) == 1 && startup;
13361348
char ready = setup ? 'R' : 'E';
13371349
(void)write(ready_pipe[1], &ready, 1);
13381350
bool saw_maintenance = false;
13391351
/* The installer only reaches the cohort barrier after STAGING the
1340-
* candidate — three tamper-defense SHA-256 passes over the whole
1341-
* binary. The candidate here is this test runner itself (~1 GB with
1342-
* sanitizers), which costs ~40 s on container/CI I/O, so the
1343-
* observation window must cover worst-case staging. On success the
1344-
* child exits the moment it observes the request, so the fast path
1345-
* stays fast. */
1346-
uint64_t maintenance_deadline = cbm_now_ms() + 120000U;
1347-
while (setup && cbm_now_ms() < maintenance_deadline) {
1352+
* candidate, three tamper-defense SHA-256 passes over this test runner
1353+
* itself (~1 GB with sanitizers), which can take long on loaded I/O.
1354+
* No deadline decides the watch (USER RULE: no timeout decides a test).
1355+
* It ends on the request, on a definite UNSAFE/IO answer, or when the
1356+
* parent's install has returned without requesting maintenance. */
1357+
while (setup) {
13481358
cbm_version_cohort_maintenance_presence_t presence =
13491359
cbm_version_cohort_maintenance_presence(manager);
13501360
if (presence == CBM_VERSION_COHORT_MAINTENANCE_REQUESTED) {
@@ -1355,6 +1365,11 @@ TEST(cli_activation_quiesce_does_not_wait_on_bootstrap_startup) {
13551365
presence == CBM_VERSION_COHORT_MAINTENANCE_IO) {
13561366
break;
13571367
}
1368+
struct pollfd install_done = {
1369+
.fd = install_done_pipe[0], .events = POLLIN, .revents = 0};
1370+
if (poll(&install_done, 1, 0) > 0) {
1371+
break;
1372+
}
13581373
/* REQUESTED is level-triggered: the installer holds maintenance EX
13591374
* for the whole mutation window, which cannot complete before this
13601375
* child releases its lease. A realistic probe cadence therefore
@@ -1377,9 +1392,11 @@ TEST(cli_activation_quiesce_does_not_wait_on_bootstrap_startup) {
13771392
}
13781393
cbm_daemon_ipc_endpoint_free(endpoint);
13791394
close(ready_pipe[1]);
1395+
close(install_done_pipe[0]);
13801396
_exit(setup && saw_maintenance ? 0 : 1);
13811397
}
13821398
close(ready_pipe[1]);
1399+
close(install_done_pipe[0]);
13831400
char ready = 0;
13841401
bool child_ready = child > 0 && read(ready_pipe[0], &ready, 1) == 1 && ready == 'R';
13851402
close(ready_pipe[0]);
@@ -1405,6 +1422,7 @@ TEST(cli_activation_quiesce_does_not_wait_on_bootstrap_startup) {
14051422
snprintf(dir_arg, sizeof(dir_arg), "--dir=%s", install_dir);
14061423
char *install_argv[] = {"--force", "--skip-config", "--yes", dir_arg};
14071424
int install_rc = child_ready ? cli_test_cmd_install(4, install_argv) : -1;
1425+
close(install_done_pipe[1]); /* the child's watch ends here at the latest */
14081426
cbm_cli_set_activation_runtime_parent_for_test(g_cli_suite_runtime_parent);
14091427
cbm_set_auto_answer_for_test(0);
14101428

@@ -1590,23 +1608,6 @@ TEST(cli_install_recovers_markerless_stale_rendezvous) {
15901608
* give-up point if one unexpectedly does not. The undrained path keeps the
15911609
* full 2 x CLI_SCOPE_TIMEOUT_MS, which is where a wedged teardown is real. */
15921610
#define CLI_SCOPE_CLEANUP_DRAINED_MS 1000U
1593-
/* Budget for probing that an ALREADY-drained host has stopped serving.
1594-
*
1595-
* The positive question -- "is this daemon still up?" -- repeats a mute
1596-
* attempt until the daemon answers (cli_scope_host_serving), because a slow
1597-
* reply on a loaded runner must not be misread as drained. Asked in the
1598-
* negative it inverts:
1599-
* there is no reply coming, so the whole budget is spent proving silence, and
1600-
* the assertion is decided by a timeout expiring rather than by the system's
1601-
* own behaviour. That was 15 s of the drain test's wall clock and the largest
1602-
* single idle block in the cli suite.
1603-
*
1604-
* The drain is proven POSITIVELY elsewhere in that test: install returns 0
1605-
* only after the activation completed, and the host child exits with
1606-
* CLI_SCOPE_HOST_DRAINED. By the time this probe runs the daemon is already
1607-
* gone, so a short budget confirms the same fact a long one would -- it just
1608-
* stops charging the suite for the wait. */
1609-
#define CLI_SCOPE_HOST_DRAINED_PROBE_MS 2000U
16101611

16111612
typedef struct {
16121613
char tmpdir[256];
@@ -1903,14 +1904,6 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta
19031904
return true;
19041905
}
19051906

1906-
static bool cli_scope_host_serving_within(const cli_scope_fixture_t *fixture, uint32_t timeout_ms) {
1907-
cbm_daemon_runtime_status_t status = {0};
1908-
return fixture->endpoint &&
1909-
cbm_daemon_runtime_request_status(fixture->endpoint, &fixture->identity, timeout_ms,
1910-
&status) &&
1911-
!status.stopping && status.committed_clients == 1;
1912-
}
1913-
19141907
typedef struct {
19151908
const cli_scope_fixture_t *fixture;
19161909
cbm_daemon_runtime_status_t status;
@@ -2081,10 +2074,13 @@ TEST(cli_install_into_host_namespace_still_drains_host_cohort) {
20812074
int install_rc =
20822075
ready ? cli_scope_install(&fixture, fixture.host_home, fixture.host_cache, host_bin, false)
20832076
: -1;
2084-
/* Negative probe: see CLI_SCOPE_HOST_DRAINED_PROBE_MS. The drain itself is
2085-
* asserted positively below via host_exit == CLI_SCOPE_HOST_DRAINED. */
2086-
bool host_serving =
2087-
ready && cli_scope_host_serving_within(&fixture, CLI_SCOPE_HOST_DRAINED_PROBE_MS);
2077+
/* Negative probe, decided by a definite answer like the positive one: a
2078+
* drained host is unreachable or reports stopping at once, while a slow
2079+
* host that still serves is asked again until it answers, so it can no
2080+
* longer pass as drained when its reply misses a short budget. The drain
2081+
* itself is asserted positively below via host_exit ==
2082+
* CLI_SCOPE_HOST_DRAINED. */
2083+
bool host_serving = ready && cli_scope_host_serving(&fixture);
20882084
const char *events = read_test_file(activation_log);
20892085
bool drained_in_log = events && strstr(events, "cohort drained") != NULL &&
20902086
strstr(events, "\"daemon_active_clients\":1") != NULL;

0 commit comments

Comments
 (0)