diff --git a/.kiro/specs/scheduler-sim-live-allocation/tasks.md b/.kiro/specs/scheduler-sim-live-allocation/tasks.md index e3c4f7cd..94579245 100644 --- a/.kiro/specs/scheduler-sim-live-allocation/tasks.md +++ b/.kiro/specs/scheduler-sim-live-allocation/tasks.md @@ -45,7 +45,7 @@ - _Requirements: 1.1, 1.3, 3.1, 3.2, 5.2_ - _Boundary: Core-Safe Allocation Chain, Unsatisfiable-Cache Callback Seam_ -- [ ] 2.3 (P) Make global and host-scoped resource-limit matching driver-agnostic +- [x] 2.3 (P) Make global and host-scoped resource-limit matching driver-agnostic - Change the limit plugin's global-limit and host-scoped-limit matching functions to read the new honest core-state reference from 1.2 instead of deriving anything from the plugin's normal-mode-only field - Also address the confirmed-dead, type-confused write at plugins/limit.c:319 (`ctx.sched = plugin->sched;`, flagged during task 2.1's review — never read back today, but stores an `xce_scheduler_t*`-shaped value into a field that is now genuinely `xce_sched_core_t*`-typed): either populate it honestly from the plugin's new core-state reference, or remove the dead write entirely if nothing in the migrated code path needs it - Keep the existing plugin-hook-registered entry points' external signature unchanged; have them resolve core state from the new field and delegate to the now-driver-agnostic matching logic @@ -115,5 +115,6 @@ ## Implementation Notes - **Task 1.1**: initial submission implemented before tests were written (TDD-mandate violation, caught in review round 1) — the real RED phase must be captured by reverting production files to HEAD via a saved patch + `git checkout --`, rebuilding to see the genuine failure, then reapplying. Retroactively-written tests plus a passing GREEN build alone do not satisfy this project's TDD discipline; the reviewer independently re-derives RED by reverting your changes, so skipping this is always caught. +- **Task 2.3**: while migrating a plugin's matching functions, writing the mandated TDD test for a code path that had genuinely never been exercised before (zero prior test call sites for `match_job_host_limit_()`/`limit_prepare_allocation()`) surfaced a real pre-existing bug (bug-1828) blocking that path entirely — the same defect class as bug-1786, fixed the same way, never applied to this sibling function. Fixing a blocking, in-boundary, minimally-scoped pre-existing bug discovered this way is acceptable within a task rather than escalating, but its buglog entry must state the REAL impact (here: host-scoped limits were never enforced in any mode, not merely a display gap) rather than the narrower symptom that surfaced it — the first-written impact description undersold this until review caught it. - **Task 1.3**: grep for ALL call sites of a function before migrating it, not just the ones named in the task brief — `host_iter.c`'s `sched_host_iter_next()` and `parjob_allocator.c`/`seqjob_allocator.c`'s `my_alloc()` all call the capacity-check functions directly, not only via `sched_try_alloc_on_host_()`. A private `static` helper with no other callers besides the functions being migrated should be migrated alongside them (leaving it stale forces an ugly cast, which this codebase's own design explicitly forbids in shared code). - **Task 1.2**: retyping a widely-called function's signature (e.g. `plugin_job_election`, `plugin_prepare_allocation`, `plugin_commit_decision`) breaks ALL of its call sites as hard compile errors in one step (this toolchain treats `-Wincompatible-pointer-types` as an error, not a warning) — there is no way to migrate a signature and keep the build green without touching every call site in the same commit, even when those call sites live in files nominally owned by a later task. The correct scope is the *minimal* fix (wrap the existing argument, e.g. `sched` → `&(sched->core)`; touch nothing else in that function), not a rewrite of the caller. Also: design.md's Implementation Notes said `plugin_prepare_allocation()` has "one normal-mode call site" — it actually has two (`resource.c`'s `sched_try_alloc_on_host_()` and `schedobj.c`'s `sched_ctx_set_primary()`). Grep for ALL call sites before assuming a design doc's count is exhaustive. diff --git a/.wolf/buglog.json b/.wolf/buglog.json index b42361f6..107efd52 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -22465,6 +22465,50 @@ ], "occurrences": 1, "last_seen": "2026-09-17T21:50:00Z" + }, + { + "id": "bug-1827", + "timestamp": "2026-09-18T07:15:00Z", + "error_message": "NOT A CRASH -- coverage gap found during an /xce-audit-invariants scheduler run (dimension 2: core module I/O isolation), while investigating driver state-equivalence for bug-1826. test_core_isolation.c's own header comment claims core_clock.c joins ONLY the nm(1) undefined-symbol enforcement layer, not the wall-clock text-scan check -- implying it is still covered by the OTHER two text-scan checks (forbidden #include list, forbidden symbol substrings). In reality, core_clock.c is absent from all three text-scan functions (test_core_never_includes_forbidden_headers(), test_core_never_calls_forbidden_symbols(), and the wall-clock scan), not just the wall-clock one -- no XCE_CORE_CLOCK_C_PATH macro is defined anywhere in test_core_isolation.c or testsuite/meson.build's c_args for that test.", + "file": "src/scheduler/testsuite/test_core_isolation.c", + "root_cause": "core_clock.c was added to the mechanically-enforced sched_core_isolation_lib archive (meson.build) and given a documented single exception (the wall-clock text scan, for its one sanctioned clock_gettime() call), but the two OTHER text-scan functions (forbidden-headers, forbidden-symbols) were never extended to also read core_clock.c's source text -- only core.h/core.c/core_events.c/core_session.c are read by those two checks (via XCE_CORE_H_PATH/_C_PATH/_EVENTS_C_PATH/_SESSION_C_PATH macros; no clock-file variant exists). The nm(1) undefined-symbol layer (which DOES include core_clock.c in the archive) only catches an actual undefined-symbol reference at link time -- it cannot catch a bare forbidden #include (e.g. client.h/netio.h/xceapi.h/delay.h/idgen.h/xce/cluster.h) that resolves to no undefined symbol (a macro-only or inline-only header), so the doc comment's implied coverage is narrower in practice than stated. No evidence found that core_clock.c's actual current content violates isolation today -- this is a gap in what the mechanism would CATCH if it did, not a confirmed live violation.", + "fix": "NOT FIXED -- audit-only finding, reported to the user for triage, not bundled into the concurrent scheduler-sim-limit-matching spec work. Straightforward fix when picked up: add a XCE_CORE_CLOCK_C_PATH macro (testsuite/meson.build's c_args for test_core_isolation) and extend test_core_never_includes_forbidden_headers()/test_core_never_calls_forbidden_symbols() to also scan core_clock.c's source text, matching the coverage the file's own doc comment already claims it has.", + "tags": [ + "scheduler", + "core-isolation", + "test-coverage-gap", + "test_core_isolation", + "core_clock", + "audit-finding", + "unfixed", + "discovered-during-xce-audit-invariants" + ], + "occurrences": 1, + "last_seen": "2026-09-18T07:15:00Z" + }, + { + "id": "bug-1828", + "error_message": "IMPACT CORRECTED 2026-09-18 (independent review of scheduler-sim-live-allocation task 2.3): this is NOT just a display/accounting gap -- it meant host-scoped resource limits were NEVER ACTUALLY ENFORCED, in ANY mode (normal or simulation), for this function's entire history. Originally discovered while writing scheduler-sim-live-allocation spec task 2.3's own required TDD test (a host-scoped limit match, via the simulation-mode-registered limit plugin, must create a real usage instance verifiable through limit_get_usage_instances()). limit_prepare_allocation()/match_job_host_limit_() consistently returned rc=-2 (-ENOENT) instead of >= 0, and no usage instance was ever created, even though match_limit() itself genuinely matched (rc=1) and is_host_based_limit() correctly identified the limit as host-based. Tracing the call chain (plugin.c's plugin_prepare_allocation() hook loop -> sched_fill_limit_requests()) shows the missing usage instance meant a host-scoped limit's vres was never added to obj->hlimits, so sched_fill_limit_requests() always iterated an empty list -- the limit's numeric cap was silently never applied to any job's resource request, in production, since this code existed. This does not corrupt the dispatch/pend decision machinery itself (plugin_prepare_allocation()'s hook-calling loop discards the hook's return value, so the bug never fed into a control-flow branch) -- it is a dormant-limit-never-binds defect, but a real enforcement gap, not merely a cosmetic one.", + "root_cause": "match_job_host_limit_() (src/scheduler/src/plugins/limit.c) is the host-scoped sibling of match_job_global_limit_() and has the same defect CLASS bug-1786 fixed in that function alone (misinterpreting key_find()'s negative \"not found\" return as fatal, rather than the correct not-yet-instantiated signal) -- not byte-for-byte identical wording (bug-1786's own guard was `if (rc <= 0) { return rc; }`; this sibling's was `if (rc < 0) { return rc; }`), same underlying mistake. Right after a successful match_limit(), it does `rc = key_find(privdata->instances, vresname, &vres); if (rc < 0) { return rc; }` before the `if (vres == NULL) { instantiate_limit(...) }` block below it. On the FIRST-EVER lookup of a given vresname (always true the first time a host-scoped limit ever matches), key_find() signals \"not found\" as a NEGATIVE rc (-ENOENT), not 0 -- this codebase's established kvmgr convention (get_group(), is_host_based_limit(), and bug-1786/bug-1495's own root causes) -- so this early-return unconditionally caught the expected miss and returned before instantiate_limit() could ever run. bug-1786 (2026-09-07) fixed this exact defect class in match_job_global_limit_() but never touched its host-scoped sibling, which retained the bug because no test exercised match_job_host_limit_()/limit_prepare_allocation() with real matching until this task added one (verified via grep: zero prior test call sites for either function anywhere in the test suite).", + "fix": "FIXED (2026-09-18, scheduler-sim-live-allocation task 2.3). Removed the erroneous `if (rc < 0) { return rc; }` guard in match_job_host_limit_() entirely, mirroring match_job_global_limit_()'s own bug-1786 fix exactly: `vres` (calloc'd NULL, only ever reassigned by a successful key_find) is already the correct, sufficient signal for \"needs instantiation\" via the existing `if (vres == NULL)` block immediately below. TDD: new test test_limit_prepare_allocation_matches_host_limit_via_sim_registered_plugin (test_repl_limit_table.c) drives the real limit_prepare_allocation()/match_job_host_limit_() path against a plugin registered via plugin_register_core() and a real percent-based host-scoped limit; RED (rc=-2, confirmed via temporary debug instrumentation tracing is_host_based_limit()=1, match_limit()=1, but limit_prepare_allocation()=-2) before this fix, GREEN (rc>=0, usage instance created and visible via limit_get_usage_instances()) after. Full scheduler suite: 79/79, zero regressions.", + "files": [ + "src/scheduler/src/plugins/limit.c (match_job_host_limit_)", + "src/scheduler/testsuite/test_repl_limit_table.c (new test)" + ], + "tags": [ + "scheduler", + "limit-plugin", + "kvmgr", + "error-code-convention", + "not-found-vs-error", + "match_job_host_limit_", + "sibling-of-bug-1786", + "scheduler-sim-live-allocation", + "task-2.3", + "fixed" + ], + "occurrences": 1, + "last_seen": "2026-09-18T15:14:00Z" } ] } diff --git a/src/scheduler/src/plugins/limit.c b/src/scheduler/src/plugins/limit.c index a283495a..f48c792e 100644 --- a/src/scheduler/src/plugins/limit.c +++ b/src/scheduler/src/plugins/limit.c @@ -203,7 +203,14 @@ static int match_job_global_limit_(xce_sched_plugin_t* plugin, char* vresname = NULL; xce_vres_t* vres = NULL; limit_plugin_data_t* privdata = NULL; - xce_scheduler_t* sched = NULL; + /* scheduler-sim-live-allocation spec, task 2.3 (Limit Plugin Core-Safe + * Matching): reads plugin->core (task 1.2's honestly-typed reference, + * set by BOTH plugin_register() and plugin_register_core()) instead of + * plugin->sched (a bare void*, NULL for a plugin registered via + * plugin_register_core() -- i.e. every simulation-mode plugin). This is + * what makes global-limit matching work identically regardless of + * which driver registered the plugin. */ + xce_sched_core_t* core = NULL; if ((limit == NULL) || (plugin == NULL) || (ctx == NULL)) { return -EINVAL; @@ -214,9 +221,9 @@ static int match_job_global_limit_(xce_sched_plugin_t* plugin, return -EINVAL; } - sched = plugin->sched; + core = plugin->core; privdata = (limit_plugin_data_t*)plugin->data; - if ((privdata == NULL) || (sched == NULL)) { + if ((privdata == NULL) || (core == NULL)) { return -EINVAL; } @@ -224,17 +231,17 @@ static int match_job_global_limit_(xce_sched_plugin_t* plugin, * plain `var resmgr` (core.h: `var resmgr;` -- already a handle, not * a var*), matching how every other resmgr/ugmgr/etc. consumer in * this codebase (get_group(), is_in_group(), key_find() itself) takes - * its manager by value. Passing &(sched->core.resmgr) here instead - * (as this and the instantiate_limit() call below previously did) - * handed those functions the ADDRESS of the field instead of its - * value -- one level of indirection too many. RTTI's key_find() - * dispatch then rejected that as an invalid object handle: silently - * inside is_host_based_limit() (its own `rc<=0` guard turns ANY - * key_find() failure, including this one, into a uniform -ENOENT -- - * see bug-1786's comment above for why that's not itself a bug), but + * its manager by value. Passing &(core->resmgr) here instead (as this + * and the instantiate_limit() call below previously did) handed those + * functions the ADDRESS of the field instead of its value -- one + * level of indirection too many. RTTI's key_find() dispatch then + * rejected that as an invalid object handle: silently inside + * is_host_based_limit() (its own `rc<=0` guard turns ANY key_find() + * failure, including this one, into a uniform -ENOENT -- see + * bug-1786's comment above for why that's not itself a bug), but * fatally inside instantiate_limit()'s own key_find() call, which * returned -EINVAL and aborted vres instantiation outright. */ - if (is_host_based_limit(sched->core.resmgr, limit) > 0) { + if (is_host_based_limit(core->resmgr, limit) > 0) { return 0; } @@ -273,7 +280,7 @@ static int match_job_global_limit_(xce_sched_plugin_t* plugin, FREEUP(vresname); if (vres == NULL) { - rc = instantiate_limit(&vres, sched->core.resmgr, limit, obj->user, + rc = instantiate_limit(&vres, core->resmgr, limit, obj->user, obj->queue, obj->project); if (rc < 0) { return rc; @@ -288,7 +295,7 @@ static int match_job_global_limit_(xce_sched_plugin_t* plugin, } } - rc = update_limit_res_value(&(sched->core.sharedInst), vres); + rc = update_limit_res_value(&(core->sharedInst), vres); if (rc >= 0) { rc = xlist_store(&(obj->glimits), vres); } @@ -300,7 +307,11 @@ static int limit_job_election(void* arg, xce_sched_job_t* job) { xce_sched_plugin_t* plugin = (xce_sched_plugin_t*)arg; limit_plugin_data_t* privdata = (limit_plugin_data_t*)plugin->data; - xce_scheduler_t* sched = (xce_scheduler_t*)plugin->sched; + /* scheduler-sim-live-allocation spec, task 2.3: plugin->core (task 1.2), + * not plugin->sched -- see match_job_global_limit_()'s identical + * comment above for the full rationale. Already the right type + * (xce_sched_core_t*), no cast needed. */ + xce_sched_core_t* core = plugin->core; xce_sched_obj_t* obj = job->schedobj; int rc = 0; iterator_t it; @@ -310,13 +321,23 @@ static int limit_job_election(void* arg, xce_sched_job_t* job) fmt_jobid(jobidString, sizeof(jobidString), job->jobid); TRACE("%s job %s plugin %s", __func__, jobidString, plugin->name); - if (obj->configver >= sched->core.configver) { + if (obj->configver >= core->configver) { return 0; } reset_sched_obj(obj); - ctx.sched = plugin->sched; + /* scheduler-sim-live-allocation spec, task 2.3: the dead, type-confused + * write this line used to be (`ctx.sched = plugin->sched;`, flagged + * during task 2.1's review) is removed rather than repointed at `core`. + * This `ctx` is a throwaway, function-local value used only to shuttle + * `schedobj` into match_job_global_limit_() below (never passed to + * sched_ctx_set_primary() or any other consumer of ctx->sched) -- + * match_job_global_limit_() itself reads ctx->schedobj only and never + * touches ctx->sched, so there is nothing downstream of this ctx that + * would ever read the field either way. Leaving it unset (zero- + * initialized to NULL by `= { 0 }` above) is honest about that, rather + * than assigning a value for a reader that doesn't exist. */ ctx.worker = NULL; ctx.schedobj = job->schedobj; @@ -345,7 +366,7 @@ static int limit_job_election(void* arg, xce_sched_job_t* job) iter_destroy(&it); - obj->configver = sched->core.configver; + obj->configver = core->configver; return rc; } @@ -358,7 +379,10 @@ static int match_job_host_limit_(xce_sched_plugin_t* plugin, char* vresname = NULL; xce_vres_t* vres = NULL; limit_plugin_data_t* privdata = NULL; - xce_scheduler_t* sched = NULL; + /* scheduler-sim-live-allocation spec, task 2.3: plugin->core, not + * plugin->sched -- see match_job_global_limit_()'s identical comment + * above for the full rationale. */ + xce_sched_core_t* core = NULL; xce_sched_worker_t* worker = NULL; xlist_t* hlimits = NULL; @@ -371,11 +395,11 @@ static int match_job_host_limit_(xce_sched_plugin_t* plugin, return -EINVAL; } - sched = plugin->sched; + core = plugin->core; privdata = (limit_plugin_data_t*)plugin->data; worker = ctx->worker; - if ((sched == NULL) || (privdata == NULL)) { + if ((core == NULL) || (privdata == NULL)) { return -EINVAL; } @@ -383,7 +407,7 @@ static int match_job_host_limit_(xce_sched_plugin_t* plugin, return 0; } - if (is_host_based_limit(sched->core.resmgr, limit) <= 0) { + if (is_host_based_limit(core->resmgr, limit) <= 0) { return 0; } @@ -412,17 +436,26 @@ static int match_job_host_limit_(xce_sched_plugin_t* plugin, return rc; } + /* bug-1828 (scheduler-sim-live-allocation spec, task 2.3, discovered + * while writing this task's own required TDD test -- the host-scoped + * sibling of bug-1786, never fixed alongside it): key_find() signals + * "not found" as a NEGATIVE rc (-ENOENT), not 0, on the FIRST-EVER + * lookup of a given vresname -- exactly the case here, since no vres + * has been instantiated for it yet. The `if (rc < 0) { return rc; }` + * this replaces unconditionally caught that expected miss and returned + * before the `if (vres == NULL)` instantiate_limit() block below could + * ever run, so no host-scoped limit's usage instance was ever created. + * `vres` (calloc'd NULL, only ever reassigned by a successful + * key_find) is already the correct, sufficient signal -- mirrors + * match_job_global_limit_()'s own identical fix (bug-1786) exactly. */ lock(privdata->instances, RTTI_LOCK_READ); rc = key_find(privdata->instances, (void*)vresname, (void*)&vres); unlock(privdata->instances); - if (rc < 0) { - return rc; - } FREEUP(vresname); if (vres == NULL) { - rc = instantiate_limit(&vres, sched->core.resmgr, limit, obj->user, + rc = instantiate_limit(&vres, core->resmgr, limit, obj->user, obj->queue, obj->project); if (rc < 0) { return rc; diff --git a/src/scheduler/testsuite/test_repl_limit_table.c b/src/scheduler/testsuite/test_repl_limit_table.c index b120c428..c289a090 100644 --- a/src/scheduler/testsuite/test_repl_limit_table.c +++ b/src/scheduler/testsuite/test_repl_limit_table.c @@ -2570,6 +2570,244 @@ test_sim_bootstrap_limit_plugin_registers_without_crash_when_config_null( "config_null: PASS\n"); } +/* ---------------------------------------------------------------------- */ +/* scheduler-sim-live-allocation spec, task 2.3 (Limit Plugin Core-Safe */ +/* Matching, Req 2.1/2.2): the sim_bootstrap_limit_plugin() tests above */ +/* prove REGISTRATION and CONFIG-SEEDING work against a plugin_register_ */ +/* core()-registered plugin (plugin->sched left NULL by design). Neither */ +/* proves MATCHING works -- limit_job_election()/match_job_global_limit_()*/ +/* and limit_prepare_allocation()/match_job_host_limit_() have always */ +/* derived their own `xce_scheduler_t* sched` from `plugin->sched`, so a */ +/* sim-registered plugin (sched == NULL) either NULL-derefs (global path, */ +/* limit_job_election()'s own `sched->core.configver` read, no prior NULL */ +/* guard) or returns -EINVAL without matching anything (host-scoped path, */ +/* match_job_host_limit_()'s own `sched == NULL` guard) -- both are the */ +/* "genuinely never invoked/no-op in sim mode" gap bug-1826/this task's own*/ +/* brief describe. These two tests drive the real hook-registered entry */ +/* points directly (the same (void* arg, ...) shape plugin_job_election()/ */ +/* plugin_prepare_allocation() -- both already core-safe, task 1.2/2.2 -- */ +/* dispatch through in production) against a plugin registered ONLY via */ +/* plugin_register_core(), proving matching now reads plugin->core (task */ +/* 1.2's honest core-state reference) instead of plugin->sched. */ +/* ---------------------------------------------------------------------- */ + +/* + * (1) Global limit match (Req 2.1): a limit configured on a plugin + * registered via plugin_register_core() must still match a real job and + * register its usage instance, verifiable through limit_get_usage_ + * instances() -- the exact "Observable" this task's brief specifies. + * `obj.user`/`.queue`/`.project` are non-NULL empty strings (never NULL -- + * match_limit()/is_in_group() both reject NULL outright), matching against + * the limit's default "all"-group fallback (add_configured_limit()'s own + * config shape, seeded here via limit_seed_config() through sim_bootstrap_ + * limit_plugin() instead, since plugin->sched is NULL for this plugin). + */ +static void +test_limit_job_election_matches_global_limit_via_sim_registered_plugin(void) +{ + xce_sched_sim_driver_t sim; + xce_sched_plugin_t* plugin = NULL; + xce_limit_config_t* limits[1]; + xce_conf4sched_t config; + xce_sched_obj_t obj; + xce_sched_job_t job; + var instances_handle; + iterator_t it; + void* item = NULL; + int rc; + + memset(&sim, 0, sizeof(sim)); + rc = xce_sched_core_init(&(sim.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* instantiate_limit() (xcelimit.c) requires the limit's underlying + * resource (val.resname, "mem" -- make_limit_config()'s own hardcoded + * shape) to genuinely exist in core->resmgr (matching production, where + * sched_init_resources() registers it at config load) -- every other + * test in this file that drives real matching (not just registration/ + * config-seeding) does the same via register_per_host_resource(). Its + * non-GLOBAL/non-SHARED flags are harmless here: this limit's own + * percent=0/per_host=0 shape (make_limit_config()) still resolves as a + * GLOBAL limit via is_host_based_limit()'s own fallback rule, matching + * instantiate_and_register_global_vres()'s identical comment above. */ + register_per_host_resource(&sim, "mem"); + + limits[0] = make_limit_config("sim_global_election_limit"); + memset(&config, 0, sizeof(config)); + config.nlimits = 1; + config.limits = limits; + sim.core.config = &config; + + /* Real production bootstrap: plugin_register_core() + limit_seed_ + * config(), exactly as sched_sim_driver_run() calls it (sim_limit.c). */ + sim_bootstrap_limit_plugin(&sim); + + rc = key_find(sim.core.plugins, "xce_sched_limit", (void**)&plugin); + TEST_ASSERT_TRUE(rc >= 0); + TEST_ASSERT_NOT_NULL(plugin); + TEST_ASSERT_NULL_MESSAGE( + plugin->sched, + "sanity: plugin_register_core() must leave plugin->sched NULL -- " + "this test proves matching works WITHOUT it, via plugin->core"); + TEST_ASSERT_EQUAL_PTR_MESSAGE( + &(sim.core), plugin->core, + "sanity: plugin->core (task 1.2) must reference this real sim " + "core"); + + /* limit_job_election() skips any schedobj whose own configver is + * already >= core->configver (the "already evaluated against this + * config" guard, core_events.c's own config-push bump being the + * production trigger) -- both default to 0 (xce_sched_core_init() / + * this memset()), so bump core->configver past the fresh obj's 0 here, + * exactly as a real CONFIG_PUSH event would before this job is ever + * offered to the picker. */ + sim.core.configver = 1; + + memset(&obj, 0, sizeof(obj)); + obj.user = (char*)""; + obj.queue = (char*)""; + obj.project = (char*)""; + xlist_init(&(obj.glimits), 8); + zarray_init(&(obj.hlimits), 8, NULL); + + memset(&job, 0, sizeof(job)); + job.jobid = 1; + job.schedobj = &obj; + + /* The real job_election_hook-registered entry point, called exactly + * the way the now-core-safe plugin_job_election() dispatcher (plugin.c, + * task 1.2) calls it in production -- `arg` is the plugin instance. */ + rc = limit_job_election((void*)plugin, &job); + TEST_ASSERT_TRUE_MESSAGE( + rc >= 0, + "limit_job_election() must succeed for a plugin registered via " + "plugin_register_core(), reading plugin->core instead of the " + "always-NULL-here plugin->sched"); + + instances_handle = limit_get_usage_instances(&(sim.core)); + TEST_ASSERT_NOT_NULL(instances_handle); + + rc = iter_make(&it, instances_handle); + TEST_ASSERT_TRUE(rc >= 0); + + rc = iter_next(&it, &item); + TEST_ASSERT_TRUE(rc >= 0); + TEST_ASSERT_NOT_EQUAL(RTTI_ITER_NO_DATA, rc); + TEST_ASSERT_NOT_NULL_MESSAGE( + item, + "a global limit match via the simulation-mode-registered plugin " + "must create a real usage instance, where today it silently never " + "matches at all (plugin->sched is NULL for a plugin_register_" + "core()-registered plugin)"); + + iter_destroy(&it); + + free_limit_config(limits[0]); + + printf( + "test_limit_job_election_matches_global_limit_via_sim_registered_" + "plugin: PASS\n"); +} + +/* + * (2) Host-scoped limit match (Req 2.2): same proof as (1), but for the + * prepare_allocation_hook path -- a percent-based limit on a real, + * non-GLOBAL/non-SHARED per-host resource (register_per_host_resource(), + * the exact shape add_configured_percent_limit() uses for the normal-mode + * equivalent of this scenario), matched via limit_prepare_allocation() + * against a real xce_sched_ctx_t/xce_sched_worker_t, on a plugin registered + * ONLY via plugin_register_core() (plugin->sched NULL). + */ +static void +test_limit_prepare_allocation_matches_host_limit_via_sim_registered_plugin( + void) +{ + xce_sched_sim_driver_t sim; + xce_sched_plugin_t* plugin = NULL; + xce_limit_val_t val = { .resname = (char*)"sim_host_res", + .percent = 1, + .val = 50.0 }; + xce_limit_config_t limit_cfg = { .name = (char*)"sim_host_limit", + .val = &val }; + xce_limit_config_t* limits[1] = { &limit_cfg }; + xce_conf4sched_t config; + xce_sched_obj_t obj; + xce_sched_worker_t worker; + xce_sched_ctx_t ctx; + var instances_handle; + iterator_t it; + void* item = NULL; + int rc; + + memset(&sim, 0, sizeof(sim)); + rc = xce_sched_core_init(&(sim.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + register_per_host_resource(&sim, "sim_host_res"); + + memset(&config, 0, sizeof(config)); + config.nlimits = 1; + config.limits = limits; + sim.core.config = &config; + + sim_bootstrap_limit_plugin(&sim); + + rc = key_find(sim.core.plugins, "xce_sched_limit", (void**)&plugin); + TEST_ASSERT_TRUE(rc >= 0); + TEST_ASSERT_NOT_NULL(plugin); + TEST_ASSERT_NULL_MESSAGE( + plugin->sched, + "sanity: plugin_register_core() must leave plugin->sched NULL"); + + memset(&obj, 0, sizeof(obj)); + obj.user = (char*)""; + obj.queue = (char*)""; + obj.project = (char*)""; + xlist_init(&(obj.glimits), 8); + zarray_init(&(obj.hlimits), 8, NULL); + + memset(&worker, 0, sizeof(worker)); + worker.name = (char*)"sim_host1"; + worker.idx = 0; + + memset(&ctx, 0, sizeof(ctx)); + ctx.worker = &worker; + ctx.schedobj = &obj; + + /* The real prepare_allocation_hook-registered entry point, called + * exactly the way the now-core-safe plugin_prepare_allocation() + * dispatcher (plugin.c, task 1.2) and sched_ctx_set_primary() + * (schedobj.c, task 2.1) call it in production. */ + rc = limit_prepare_allocation((void*)plugin, &ctx); + TEST_ASSERT_TRUE_MESSAGE( + rc >= 0, + "limit_prepare_allocation() must succeed for a plugin registered " + "via plugin_register_core(), reading plugin->core instead of the " + "always-NULL-here plugin->sched"); + + instances_handle = limit_get_usage_instances(&(sim.core)); + TEST_ASSERT_NOT_NULL(instances_handle); + + rc = iter_make(&it, instances_handle); + TEST_ASSERT_TRUE(rc >= 0); + + rc = iter_next(&it, &item); + TEST_ASSERT_TRUE(rc >= 0); + TEST_ASSERT_NOT_EQUAL(RTTI_ITER_NO_DATA, rc); + TEST_ASSERT_NOT_NULL_MESSAGE( + item, + "a host-scoped limit match via the simulation-mode-registered " + "plugin must create a real usage instance, where today it either " + "no-ops (-EINVAL from match_job_host_limit_()'s own sched==NULL " + "guard) or never matches at all"); + + iter_destroy(&it); + + printf( + "test_limit_prepare_allocation_matches_host_limit_via_sim_" + "registered_plugin: PASS\n"); +} + int main(void) { UNITY_BEGIN(); @@ -2624,5 +2862,9 @@ int main(void) test_sim_bootstrap_limit_plugin_seeds_real_config_limit_when_config_present); RUN_TEST( test_sim_bootstrap_limit_plugin_registers_without_crash_when_config_null); + RUN_TEST( + test_limit_job_election_matches_global_limit_via_sim_registered_plugin); + RUN_TEST( + test_limit_prepare_allocation_matches_host_limit_via_sim_registered_plugin); return UNITY_END(); }