fix(controller): trim config versions by ns mtime; unique test tmpdir

Two pre-existing failures (2026-05-13, commit 491c255) surfaced by the
controller unit-test suite (1070 Tests 2 Failures), both in the config
version storage path:

1. conf_trim_old_versions sorted cluster.bin.* versions by
   second-granularity st_mtime. Versions saved within the same second
   (rapid replication, or the trim test's 15ms-spaced saves) tie, glibc's
   stable mergesort keeps creation order, and the newest (current-pointer)
   entry lands at the protected keep_n index — so nothing is pruned and
   keep_n+1 files remain. Store st_mtim (struct timespec) and compare
   tv_sec then tv_nsec.

2. cr_setup() built its tmpdir path from (pid, time(NULL)) at second
   granularity; a Unity assertion failure longjmps past cr_teardown(),
   leaking the dir, and the next test (same pid+second) silently inherited
   the leftover conf/current — cascading a second failure
   (test_conf_get_saved_tag_no_config read a real tag instead of 0). Add a
   per-invocation counter to the path so no two tests share a dir.

Verified: nix develop ./src --command bash src/tools/xce-build.sh all →
meson 9/9 Ok 0 Fail, controller 1070 Tests 0 Failures, xceadmin 172
passed, frontend Go all ok.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
lei.chen 2026-08-01 03:34:55 +00:00
parent 84fc46b5fe
commit f2993d1efc
2 changed files with 23 additions and 7 deletions

View file

@ -1012,15 +1012,23 @@ int conf_check_stale(const char *workdir, time_t stale_timeout_s)
typedef struct {
char name[NAME_MAX];
time_t mtime;
struct timespec mtime; /* st_mtim — nanosecond resolution. Trimming must
* not tie on second-granularity st_mtime: configs
* saved within the same second (rapid replication
* or unit tests) would sort as equal and leave the
* protected current slot as the only trim
* candidate — which the pointer-protection skips,
* so nothing ever gets pruned. */
} ver_entry_t;
static int ver_entry_cmp_desc(const void *a, const void *b)
{
const ver_entry_t *ea = (const ver_entry_t *)a;
const ver_entry_t *eb = (const ver_entry_t *)b;
if (eb->mtime > ea->mtime) return 1;
if (eb->mtime < ea->mtime) return -1;
if (eb->mtime.tv_sec != ea->mtime.tv_sec)
return (eb->mtime.tv_sec > ea->mtime.tv_sec) ? 1 : -1;
if (eb->mtime.tv_nsec != ea->mtime.tv_nsec)
return (eb->mtime.tv_nsec > ea->mtime.tv_nsec) ? 1 : -1;
return 0;
}
@ -1059,7 +1067,7 @@ void conf_trim_old_versions(const char *workdir, int keep_n)
if (stat(full, &st) == 0) {
strncpy(entries[n].name, e->d_name, NAME_MAX - 1);
entries[n].name[NAME_MAX - 1] = '\0';
entries[n].mtime = st.st_mtime;
entries[n].mtime = st.st_mtim;
n++;
}
}

View file

@ -61,8 +61,16 @@ static char tmpdir[256];
static void cr_setup(void)
{
snprintf(g_tmpdir, sizeof(g_tmpdir), "/tmp/test_cr_%d_%ld",
(int)getpid(), (long)time(NULL));
static unsigned int cr_seq;
/* Unique per invocation, not just per (pid, second): the 1000+ tests in
* this suite routinely run within the same wall-clock second, and a
* Unity assertion failure longjmps out of the test, skipping the
* cr_teardown() that would rm -rf the dir — so a reused path would make
* the next test silently inherit the previous one's cluster.bin.* files
* and conf/current (observed as a cascade: a leaked trim-test dir made
* test_conf_get_saved_tag_no_config read a real tag instead of 0). */
snprintf(g_tmpdir, sizeof(g_tmpdir), "/tmp/test_cr_%d_%u_%ld",
(int)getpid(), cr_seq++, (long)time(NULL));
mkdir(g_tmpdir, 0700);
/* conf_save_config() writes to workdir/name/conf — mirror that here */
snprintf(g_instdir, sizeof(g_instdir), "%s/ctrl0", g_tmpdir);