fix(resource-value-verification): close bug-1579 for real via xce_test's ! shell-escape
Previously worked around (task 6.2) by removing the one host-attrs resource (cpuf) from the default set - a custom resource=value arg using any host-attrs name would still false-mismatch at the controller layer, since inject_resources() disconnected the worker before the separate controller- check session ran, and the controller resets host-attrs resources on disconnect. Real fix: xce_test's `!<shell-command>` escape is handled in the single top-level REPL loop before context-specific dispatch, so it works in any context - including worker mode - without calling leave_ctx()/disconnecting. inject_resources() now shells out via `!` to run the controller check in-line, between resupdate and quit, while the worker's connection is still open. query_controller_resvals()'s comparison logic is unchanged - this is a pure sequencing fix. cpuf restored to DEFAULT_RESOURCE_ARGS as live proof. Live-verified: cpuf now MATCHes at the controller layer (was always a false MISMATCH before); adversarial shell-injection attempt via a malicious resource name confirmed safe (printf %q quoting); SIGTERM delivered exactly during the nested check confirmed zero orphaned/zombie processes across 4 independent runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
792dfdf4df
commit
b673b7c933
SSH key fingerprint:
SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
2 changed files with 234 additions and 81 deletions
|
|
@ -5,21 +5,31 @@
|
|||
# resource value via xce_test's resupdate, confirm it is visible at the
|
||||
# controller, live scheduler, and Go apiserver).
|
||||
#
|
||||
# resource-value-verification spec, task 5.1+6.1+6.2+6.3 SCOPE: this file
|
||||
# currently implements deployment (task 5.1), worker join + resource
|
||||
# injection (task 6.1), the controller-side per-layer verification check
|
||||
# (task 6.2), AND the scheduler-side per-layer verification check (task 6.3)
|
||||
# -- stand up a fresh controller + scheduler (-vvv) + apiserver (debug
|
||||
# logging) deployment via xce-deploy-ha's deploy-ha.sh + generated
|
||||
# start-all.sh, confirm all three components are actually up, join a
|
||||
# SYNTHETIC worker (no real worker daemon process) through xce_test's worker
|
||||
# context, run resupdate for one or more resource=value pairs, then (a) query
|
||||
# the controller (xce_test client-context `resval`) for every ACCEPTED pair
|
||||
# and report MATCH/MISMATCH against the injected value, and (b) poll the live
|
||||
# scheduler daemon's own log file (task 4.1's trace line) for the same
|
||||
# ACCEPTED pairs, within a bounded wait, and report MATCH/MISMATCH/
|
||||
# UNREACHABLE distinctly from the controller-side result, then clean up
|
||||
# (trap-based, covering a mid-deployment abort, not just a completed run).
|
||||
# resource-value-verification spec, task 5.1+6.1+6.2+6.3 SCOPE (+ bug-1579
|
||||
# follow-up fix): this file currently implements deployment (task 5.1),
|
||||
# worker join + resource injection (task 6.1), the controller-side per-layer
|
||||
# verification check (task 6.2), AND the scheduler-side per-layer
|
||||
# verification check (task 6.3) -- stand up a fresh controller + scheduler
|
||||
# (-vvv) + apiserver (debug logging) deployment via xce-deploy-ha's
|
||||
# deploy-ha.sh + generated start-all.sh, confirm all three components are
|
||||
# actually up, join a SYNTHETIC worker (no real worker daemon process)
|
||||
# through xce_test's worker context, run resupdate for one or more
|
||||
# resource=value pairs, then (a) query the controller (xce_test
|
||||
# client-context `resval`) for every ACCEPTED pair and report
|
||||
# MATCH/MISMATCH against the injected value, and (b) poll the live scheduler
|
||||
# daemon's own log file (task 4.1's trace line) for the same ACCEPTED pairs,
|
||||
# within a bounded wait, and report MATCH/MISMATCH/UNREACHABLE distinctly
|
||||
# from the controller-side result, then clean up (trap-based, covering a
|
||||
# mid-deployment abort, not just a completed run).
|
||||
#
|
||||
# bug-1579 follow-up fix (this revision): the controller-side check (a) above
|
||||
# now runs NESTED inside inject_resources()'s own xce_test worker session --
|
||||
# via a `!` shell-escape line placed between `resupdate` and `quit` -- rather
|
||||
# than as a wholly separate xce_test process run AFTER that session's own
|
||||
# `quit` has already disconnected the synthetic worker. See the
|
||||
# internal_controller_check() function and the top-of-file bug-1579 note
|
||||
# below for why this matters and exactly how it works; DEFAULT_RESOURCE_ARGS
|
||||
# now includes `cpuf` again as proof the fix is real (see that note).
|
||||
# Apiserver per-layer verification (task 6.4) and verdict aggregation /
|
||||
# cleanup-on-every-path (task 7.1) are added by LATER tasks -- see the
|
||||
# "Extension points" markers in main() below for exactly where they hook in.
|
||||
|
|
@ -30,30 +40,59 @@
|
|||
# Task 6.1 deliberate deviation from design.md: the default (no-args)
|
||||
# resource set is Requirement 1's gap-resource set (r15m, pg, ls, it, swp,
|
||||
# io, tmp) plus a couple of already-supported NUMERIC names for regression
|
||||
# coverage (mem, r1m) -- NOT "type"/"model" as design.md's illustrative text
|
||||
# coverage (mem, cpuf) -- NOT "type"/"model" as design.md's illustrative text
|
||||
# suggests. bug-1574 (xce_test can SEGFAULT, non-deterministically, on
|
||||
# `resupdate -n type/-n model ...` -- uninitialized-memory UB in
|
||||
# set_resval()'s Value_u_svalue fallback path) makes including a string-typed
|
||||
# resource in this script's own default live-testing path a real crash risk,
|
||||
# not a theoretical one. Revisit once bug-1574 is fixed.
|
||||
#
|
||||
# Task 6.2 remediation (bug-1579): the regression-coverage name was
|
||||
# originally "cpuf", but cpuf is one of worker.c's N_HOST_ATTRS
|
||||
# (add_hostattrs_resvals()) -- the controller RESETS it to baseline the
|
||||
# moment the synthetic worker disconnects (mark_worker_unavail() ->
|
||||
# apply_worker_dead_mutation() -> reset_hostattrs_unknown() +
|
||||
# add_hostattrs_resvals()), and inject_resources()'s xce_test session ends
|
||||
# with `quit` (disconnecting the worker) before this task's separate
|
||||
# resval-query session runs -- so cpuf always reported a false MISMATCH
|
||||
# (observed=0.000000). Swapped for "r1m": confirmed via
|
||||
# Task 6.2 remediation (bug-1579, original workaround): the
|
||||
# regression-coverage name was originally "cpuf", but cpuf is one of
|
||||
# worker.c's N_HOST_ATTRS (add_hostattrs_resvals()) -- the controller RESETS
|
||||
# it to baseline the moment the synthetic worker disconnects
|
||||
# (mark_worker_unavail() -> apply_worker_dead_mutation() ->
|
||||
# reset_hostattrs_unknown() + add_hostattrs_resvals()), and
|
||||
# inject_resources()'s xce_test session used to end with `quit` (disconnecting
|
||||
# the worker) before a separate, later xce_test process ran the controller
|
||||
# query -- so cpuf always reported a false MISMATCH (observed=0.000000).
|
||||
# Task 6.2's review swapped it for "r1m" (confirmed via
|
||||
# resval_typeresolve.c's resinfo[] table to be a genuinely pre-existing
|
||||
# ("already-supported") resource name, and via
|
||||
# src/worker/src/resource_sync.c to be a periodic/dynamic load-index
|
||||
# resource (not in worker.c's N_HOST_ATTRS list), so it preserves the
|
||||
# original "already-supported name for regression coverage" intent without
|
||||
# the disconnect-reset problem. See bug-1579 for the full analysis and why
|
||||
# the alternative fix (keep the worker session connected throughout) is out
|
||||
# of scope for this remediation.
|
||||
# resource, not in worker.c's N_HOST_ATTRS list) as a workaround, not a fix --
|
||||
# the underlying disconnect-timing problem remained live for any *other*
|
||||
# host-attrs name (including a user-supplied custom resource=value
|
||||
# argument).
|
||||
#
|
||||
# bug-1579 REAL FIX (this revision): xce_test's `!<shell-command>` escape
|
||||
# (preprocess_and_run(), test.c ~line 950-998) is handled in the single
|
||||
# top-level REPL loop (test.c main(), the sole call site) BEFORE any
|
||||
# context-specific command dispatch -- so it works identically in ANY
|
||||
# context, including `worker`, WITHOUT calling leave_ctx()/disconnecting.
|
||||
# inject_resources() now inserts a `!` line between `resupdate` and `quit`
|
||||
# that invokes this very script recursively, in a hidden
|
||||
# `--internal-controller-check` mode (see internal_controller_check() below),
|
||||
# from INSIDE the still-open worker session. That nested invocation runs its
|
||||
# own client-context xce_test session and queries `resval` for every
|
||||
# requested resource name WHILE the synthetic worker is still connected --
|
||||
# so a host-attrs resource is never reset before it's checked. The nested
|
||||
# check writes raw observed values to $TOP/logs/controller-check-raw.tsv;
|
||||
# query_controller_resvals() (task 6.2's original function, still called at
|
||||
# the same point in main(), after inject_resources() returns) now reads that
|
||||
# file instead of running its own live query -- a SEQUENCING change, not a
|
||||
# logic change: the MATCH/MISMATCH comparison, CONTROLLER_OBSERVED/
|
||||
# CONTROLLER_CHECK_RESULTS population, and controller-check-results.tsv
|
||||
# format are all unchanged. DEFAULT_RESOURCE_ARGS now restores "cpuf"
|
||||
# alongside "mem" (design.md's original "a couple of already-supported
|
||||
# names for regression coverage" intent) -- this is the proof the fix is
|
||||
# real: cpuf now correctly MATCHes at the controller layer instead of
|
||||
# false-MISMATCHing. The alternative considered during task 6.2's review
|
||||
# (keep the worker session connected across all per-layer checks by
|
||||
# reordering calls) was already ruled out there as not viable without this
|
||||
# `!`-escape mechanism -- xce_test's REPL has no command path from the
|
||||
# `worker` context back to `client` without leave_ctx() (`..`/cmd_up), which
|
||||
# disconnects exactly like `quit` does. See bug-1579 for the full history.
|
||||
#
|
||||
# Must be invoked under an environment with `nix` on PATH (same as
|
||||
# xce-deploy-ha / xce-test-ha) -- this script shells out to `nix develop`
|
||||
|
|
@ -107,6 +146,12 @@ set -u
|
|||
|
||||
SELF_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
REPO="$(cd "$SELF_DIR/../../.." && pwd)"
|
||||
# Absolute path to this script itself -- used by inject_resources()'s `!`
|
||||
# shell-escape (bug-1579 fix, see the top-of-file note) to invoke this same
|
||||
# script recursively in `--internal-controller-check` mode, from INSIDE the
|
||||
# xce_test worker session's `system()` call, while that session's connection
|
||||
# to the controller is still open (before its `quit`).
|
||||
SELF_SCRIPT="$SELF_DIR/$(basename "${BASH_SOURCE[0]}")"
|
||||
|
||||
DEPLOY_HA="$REPO/src/tools/deploy-ha.sh"
|
||||
BUILD_DIR="${XCE_RESVERIFY_BUILD_DIR:-$REPO/src/xce.build}"
|
||||
|
|
@ -144,7 +189,7 @@ Usage: $0 [--keep-running] [--dir DIR] [resource=value ...]
|
|||
\$XCE_RESVERIFY_TOP).
|
||||
resource=value inject this resource via resupdate (may repeat). Default
|
||||
when none given: r15m=1.5 pg=2.5 ls=3.5 it=4.5 swp=5.5
|
||||
io=6.5 tmp=7.5 mem=8.5 r1m=9.5 (numeric-only -- see
|
||||
io=6.5 tmp=7.5 mem=8.5 cpuf=9.5 (numeric-only -- see
|
||||
bug-1574 note at top of file).
|
||||
EOF
|
||||
}
|
||||
|
|
@ -352,14 +397,18 @@ confirm_deployment_up() {
|
|||
# names for regression coverage -- see the top-of-file bug-1574 note for why
|
||||
# this deliberately excludes type/model (design.md's own illustrative text
|
||||
# names them, but xce_test can SEGFAULT on a string-typed resupdate, so this
|
||||
# is a documented, deliberate deviation, not an oversight). The second
|
||||
# regression-coverage name is "r1m" (a periodic/dynamic load-index resource),
|
||||
# not a worker.c host-attrs name -- see the top-of-file bug-1579 remediation
|
||||
# note for why a host-attrs name (e.g. cpuf) always false-MISMATCHes against
|
||||
# this script's own two-session (inject, then query) structure.
|
||||
# is a documented, deliberate deviation, not an oversight). The two
|
||||
# regression-coverage names are "mem" and "cpuf", matching design.md's
|
||||
# original "a couple of already-supported names" intent -- "cpuf" is a
|
||||
# worker.c N_HOST_ATTRS name and is restored here (bug-1579's real fix, see
|
||||
# the top-of-file note) as live proof that the controller-side check no
|
||||
# longer false-MISMATCHes a host-attrs resource: the nested `!`-escape
|
||||
# controller check (internal_controller_check()) runs while the synthetic
|
||||
# worker's connection is still open, so cpuf is never reset to baseline
|
||||
# before it's checked.
|
||||
DEFAULT_RESOURCE_ARGS=(
|
||||
r15m=1.5 pg=2.5 ls=3.5 it=4.5 swp=5.5 io=6.5 tmp=7.5
|
||||
mem=8.5 r1m=9.5
|
||||
mem=8.5 cpuf=9.5
|
||||
)
|
||||
|
||||
# Fixed synthetic worker identity for this run -- tags every resupdate'd
|
||||
|
|
@ -415,9 +464,23 @@ inject_resources() {
|
|||
|
||||
log "joining synthetic worker '$WORKER_HOST' (masters=$PEERS) and injecting: ${pairs[*]}"
|
||||
|
||||
# bug-1579 fix: run the controller-side check (internal_controller_check(),
|
||||
# below) via a `!` shell-escape BEFORE `quit` -- while this session's
|
||||
# worker connection is still open -- instead of in a separate xce_test
|
||||
# process run after this session has already disconnected. `%q`-quote
|
||||
# every interpolated value defensively (none of TOP/PEERS/WORKER_HOST/
|
||||
# resource names are expected to need it, but this line is re-parsed by
|
||||
# a shell via xce_test's system() call, so quote anyway).
|
||||
local ctrl_check_cmd
|
||||
ctrl_check_cmd="bash $(printf '%q' "$SELF_SCRIPT") --internal-controller-check $(printf '%q' "$TOP") $(printf '%q' "$PEERS") $(printf '%q' "$WORKER_HOST")"
|
||||
for pair in "${pairs[@]}"; do
|
||||
name="${pair%%=*}"
|
||||
ctrl_check_cmd="$ctrl_check_cmd $(printf '%q' "$name")"
|
||||
done
|
||||
|
||||
local script
|
||||
script="$(printf 'set masters %s\nset myname %s\nworker\njoin\nresupdate%s\nquit\n' \
|
||||
"$PEERS" "$WORKER_HOST" "$resupdate_line")"
|
||||
script="$(printf 'set masters %s\nset myname %s\nworker\njoin\nresupdate%s\n!%s\nquit\n' \
|
||||
"$PEERS" "$WORKER_HOST" "$resupdate_line" "$ctrl_check_cmd")"
|
||||
|
||||
out="$(printf '%s' "$script" | xtest)"
|
||||
|
||||
|
|
@ -475,7 +538,7 @@ $out"
|
|||
[ "$failed" -eq 0 ] || fail "injection did not produce a definitive accept/reject outcome for every requested resource (see above)"
|
||||
}
|
||||
|
||||
# ── Step 5: controller-side verification check (task 6.2) ──────────────────
|
||||
# ── Step 5: controller-side verification check (task 6.2 + bug-1579 fix) ───
|
||||
# Requirement 3.1/3.2: for every SUCCESSFULLY-injected (accepted) resource/
|
||||
# worker pair, query the controller via xce_test's client-context `resval`
|
||||
# (the same xcecli_resval() call cmd_resupdate's own task 2.2 follow-up
|
||||
|
|
@ -484,8 +547,105 @@ $out"
|
|||
# resource was never actually applied at the controller in the first place
|
||||
# (task 2.2's own follow-up query inside resupdate already confirmed that at
|
||||
# injection time) -- there is nothing to verify for it here, so
|
||||
# query_controller_resvals() only queries names marked "accepted" in
|
||||
# query_controller_resvals() only REPORTS on names marked "accepted" in
|
||||
# $TOP/logs/injection-results.tsv.
|
||||
#
|
||||
# bug-1579 fix: the live `resval` query itself no longer happens here. It now
|
||||
# happens EARLIER, nested inside inject_resources()'s own xce_test worker
|
||||
# session via a `!` shell-escape (see internal_controller_check() below and
|
||||
# the top-of-file bug-1579 note) -- for EVERY requested name, accepted or
|
||||
# not, since accept/reject isn't known yet at that point in the sequence.
|
||||
# query_controller_resvals() reads that nested query's raw results back from
|
||||
# $TOP/logs/controller-check-raw.tsv and reports only on the names
|
||||
# $TOP/logs/injection-results.tsv (available only now, after
|
||||
# inject_resources() has returned and parsed the whole session's output)
|
||||
# marks "accepted" -- a pure sequencing change: the MATCH/MISMATCH
|
||||
# comparison, CONTROLLER_OBSERVED/CONTROLLER_CHECK_RESULTS population, and
|
||||
# controller-check-results.tsv format below are all unchanged from task 6.2.
|
||||
|
||||
# internal_controller_check TOP PEERS WORKER_HOST NAME...
|
||||
# NOT a normal in-process function call -- invoked as a FRESH
|
||||
# `bash $SELF_SCRIPT --internal-controller-check ...` process via
|
||||
# inject_resources()'s `!` shell-escape line, from INSIDE the xce_test
|
||||
# worker-context session's system() call, while the injecting synthetic
|
||||
# worker's connection to the controller is STILL OPEN (i.e. before that same
|
||||
# session's `quit`). This is what makes the controller-side check immune to
|
||||
# the worker-disconnect host-attrs reset (worker.c's mark_worker_unavail()
|
||||
# -> apply_worker_dead_mutation() -> reset_hostattrs_unknown() +
|
||||
# add_hostattrs_resvals(), N_HOST_ATTRS) -- the worker never disconnects
|
||||
# before this check runs.
|
||||
#
|
||||
# Queries resval for EVERY given name, not just ones that will turn out to be
|
||||
# "accepted" -- cmd_resupdate()'s own accept/reject determination has already
|
||||
# run inside THIS SAME xce_test process by this point and been printed to
|
||||
# its stdout, but that text isn't available to this freshly-spawned process;
|
||||
# accept/reject filtering happens later, in query_controller_resvals(), once
|
||||
# the parent script has parsed the whole session's captured output. Writes
|
||||
# RAW observed values (blank if not found) to
|
||||
# $TOP/logs/controller-check-raw.tsv (name\tobserved) for
|
||||
# query_controller_resvals() to read once inject_resources() returns.
|
||||
#
|
||||
# On a connection failure, records $TOP/logs/controller-check-error.txt
|
||||
# instead (query_controller_resvals() fails the run on this, same failure
|
||||
# semantics as task 6.2's original inline check) and deliberately still
|
||||
# returns 0 either way: a non-zero exit here becomes this xce_test session's
|
||||
# g_last_status and, in non-interactive/batch mode, ABORTS its REPL loop
|
||||
# before `quit` runs (preprocess_and_run()'s shell-escape handling) --
|
||||
# undesirable when the failure can instead be surfaced through the raw-file/
|
||||
# error-file contract and let this session reach `quit` normally.
|
||||
internal_controller_check() {
|
||||
local top="$1" peers="$2" worker_host="$3"
|
||||
shift 3
|
||||
local names=("$@")
|
||||
|
||||
mkdir -p "$top/logs"
|
||||
local raw_tsv="$top/logs/controller-check-raw.tsv"
|
||||
local err_txt="$top/logs/controller-check-error.txt"
|
||||
rm -f "$err_txt"
|
||||
: > "$raw_tsv"
|
||||
|
||||
[ "${#names[@]}" -gt 0 ] || return 0
|
||||
|
||||
local cmds="" name
|
||||
for name in "${names[@]}"; do
|
||||
cmds="${cmds}resval -n $name -m $worker_host"$'\n'
|
||||
done
|
||||
|
||||
local script out
|
||||
script="$(printf 'set masters %s\nclient\n%squit\n' "$peers" "$cmds")"
|
||||
out="$(printf '%s' "$script" | xtest)"
|
||||
|
||||
if printf '%s\n' "$out" | grep -Fq -- "enter_ctx: invalid"; then
|
||||
printf '%s\n' "$out" > "$err_txt"
|
||||
return 0
|
||||
fi
|
||||
|
||||
# Same sequential state-machine parse query_controller_resvals() used to
|
||||
# do inline pre-fix -- see its own historical comment (still applicable
|
||||
# here) for why (bug-1576's short-name substring hazard, no trailing
|
||||
# marker after the last query).
|
||||
local -A parsed_values=()
|
||||
local line cur_name=""
|
||||
while IFS= read -r line; do
|
||||
case "$line" in
|
||||
"resource "*"@${worker_host}:")
|
||||
cur_name="${line#resource }"
|
||||
cur_name="${cur_name%@${worker_host}:}"
|
||||
;;
|
||||
"value: "*)
|
||||
if [ -n "$cur_name" ]; then
|
||||
parsed_values["$cur_name"]="${line#value: }"
|
||||
fi
|
||||
cur_name=""
|
||||
;;
|
||||
esac
|
||||
done <<< "$out"
|
||||
|
||||
for name in "${names[@]}"; do
|
||||
printf '%s\t%s\n' "$name" "${parsed_values[$name]:-}" >> "$raw_tsv"
|
||||
done
|
||||
return 0
|
||||
}
|
||||
|
||||
declare -A CONTROLLER_OBSERVED=() # resource name -> raw value string as rendered by cmd_resval (only set for accepted names actually queried)
|
||||
declare -A CONTROLLER_CHECK_RESULTS=() # resource name -> "MATCH" | "MISMATCH: injected=<X> observed=<Y>" | "MISMATCH: controller returned no value ..."
|
||||
|
|
@ -552,49 +712,28 @@ query_controller_resvals() {
|
|||
return 0
|
||||
fi
|
||||
|
||||
log "controller check: querying resval for ${#accepted_names[@]} accepted resource(s): ${accepted_names[*]}"
|
||||
|
||||
local cmds="" out
|
||||
for name in "${accepted_names[@]}"; do
|
||||
cmds="${cmds}resval -n $name -m $WORKER_HOST"$'\n'
|
||||
done
|
||||
|
||||
local script
|
||||
script="$(printf 'set masters %s\nclient\n%squit\n' "$PEERS" "$cmds")"
|
||||
|
||||
out="$(printf '%s' "$script" | xtest)"
|
||||
|
||||
if printf '%s\n' "$out" | grep -Fq -- "enter_ctx: invalid"; then
|
||||
# The live `resval` query already ran EARLIER, nested inside
|
||||
# inject_resources()'s own xce_test worker session via a `!` shell
|
||||
# escape (bug-1579 fix -- see internal_controller_check() above), while
|
||||
# the synthetic worker's connection to the controller was still open.
|
||||
# This function just reads back what that nested query already
|
||||
# collected -- see the "Step 5" comment block above.
|
||||
local err_txt="$TOP/logs/controller-check-error.txt"
|
||||
if [ -f "$err_txt" ]; then
|
||||
fail "controller check failed -- xce_test client context could not connect (masters=$PEERS); xce_test output:
|
||||
$out"
|
||||
$(cat "$err_txt")"
|
||||
fi
|
||||
|
||||
# Sequential state-machine parse of cmd_resval's own fixed 2-line block
|
||||
# format (test.c's cmd_resval(): "resource <name>@<host>:\nvalue:
|
||||
# <rendered>\n") or its 1-line not-found message ("no resource value
|
||||
# found in given host"). Keyed off the exact header line rather than a
|
||||
# per-name grep -- avoids the short-resource-name substring hazard
|
||||
# (bug-1576/inject_resources()'s own -Fx lesson: names like "it"/"io"/
|
||||
# "ls" can appear as a literal substring of an unrelated line) and
|
||||
# avoids depending on a trailing line always existing after the LAST
|
||||
# query's output (a plain `grep -A1` on the final block would silently
|
||||
# come up empty if `quit` prints nothing further).
|
||||
local raw_tsv="$TOP/logs/controller-check-raw.tsv"
|
||||
[ -f "$raw_tsv" ] || fail "controller check: $raw_tsv not found -- inject_resources()'s nested controller-check (internal_controller_check(), invoked via its xce_test session's \`!\` shell escape) did not run or did not complete"
|
||||
|
||||
log "controller check: reporting on ${#accepted_names[@]} accepted resource(s) (queried during injection, see internal_controller_check()): ${accepted_names[*]}"
|
||||
|
||||
local -A parsed_values=()
|
||||
local line cur_name=""
|
||||
while IFS= read -r line; do
|
||||
case "$line" in
|
||||
"resource "*"@${WORKER_HOST}:")
|
||||
cur_name="${line#resource }"
|
||||
cur_name="${cur_name%@${WORKER_HOST}:}"
|
||||
;;
|
||||
"value: "*)
|
||||
if [ -n "$cur_name" ]; then
|
||||
parsed_values["$cur_name"]="${line#value: }"
|
||||
fi
|
||||
cur_name=""
|
||||
;;
|
||||
esac
|
||||
done <<< "$out"
|
||||
while IFS=$'\t' read -r name value; do
|
||||
[ -n "$name" ] || continue
|
||||
parsed_values["$name"]="$value"
|
||||
done < "$raw_tsv"
|
||||
|
||||
mkdir -p "$TOP/logs"
|
||||
local results_tsv="$TOP/logs/controller-check-results.tsv"
|
||||
|
|
@ -846,4 +985,17 @@ main() {
|
|||
exit 0
|
||||
}
|
||||
|
||||
# Hidden internal entry point (bug-1579 fix): inject_resources()'s `!`
|
||||
# shell-escape line invokes this same script as `bash "$SELF_SCRIPT"
|
||||
# --internal-controller-check TOP PEERS WORKER_HOST NAME...` from inside its
|
||||
# own xce_test worker session. Intercepted here, BEFORE main()/parse_args()
|
||||
# (which has no knowledge of this flag and would otherwise reject it as an
|
||||
# "Unknown argument") -- runs only internal_controller_check(), none of the
|
||||
# normal deploy/inject/trap flow.
|
||||
if [ "${1:-}" = "--internal-controller-check" ]; then
|
||||
shift
|
||||
internal_controller_check "$@"
|
||||
exit $?
|
||||
fi
|
||||
|
||||
main "$@"
|
||||
|
|
|
|||
|
|
@ -116,7 +116,8 @@
|
|||
- _Depends: 7.1_
|
||||
|
||||
## Implementation Notes
|
||||
- Task 6.3 confirmed (live, via a `cpuf` probe: controller MISMATCH + scheduler MATCH simultaneously) that the scheduler check is genuinely immune to bug-1579 — it reads task 4.1's trace log (written once, at apply-time, in the `RESVALS_DELTA` case only), never touched by a later worker-disconnect's `WORKERS_PUSH`-driven reset. **Real fix for bug-1579 identified, not yet applied** (see below) — the `!<shell-command>` escape in `xce_test` (`preprocess_and_run()`, `test.c` ~line 965) works in ANY context including `worker` mode, since it's handled in the single top-level REPL loop before context dispatch, not per-context. This means `inject_resources()` can stay in worker context (connection held open) after injecting, shell out via `!` to a separate `xce_test` client-mode process for the controller query, and only `quit` at the very end — fully closing bug-1579 (not just working around it in the default set) rather than needing a C-level `xce_test` change. Plan: retrofit this into `inject_resources()`/`query_controller_resvals()` as a follow-up before/alongside task 7.1, then restore a host-attrs resource (e.g. `cpuf`) to `DEFAULT_RESOURCE_ARGS` to prove it.
|
||||
- **bug-1579 is now FULLY fixed** (retrofit on top of tasks 6.1/6.2, not a numbered task — user-requested follow-up). `inject_resources()`'s worker-context `xce_test` script now inserts a `!<shell-command>` line between `resupdate` and `quit`: `! bash <script> --internal-controller-check <top> <peers> <worker-host> <names...>` — this runs the controller-check query via `xce_test`'s `!` shell-escape (`preprocess_and_run()`, handled in the single top-level REPL loop before context dispatch, so it works in ANY context including `worker` mode without disconnecting). The controller check now runs WHILE the injecting worker's connection is still open, so host-attrs resources are never reset before being checked. `DEFAULT_RESOURCE_ARGS` now includes `cpuf` again (proof the fix works: `cpuf` now MATCHes at the controller layer, not the old false MISMATCH). `query_controller_resvals()`'s comparison logic (`controller_values_match()`) is confirmed byte-for-byte unchanged — this was a pure sequencing fix. Any future task touching `inject_resources()`/the controller-check flow should be aware of this nested-process structure (`--internal-controller-check` is a hidden internal dispatch flag, gated before `main()`/`parse_args()` runs — do not remove it thinking it's dead code).
|
||||
- Non-blocking notes from the retrofit's review: `internal_controller_check()` queries ALL requested resource names (including ones that will later be rejected), not just accepted ones like the original design — forced by sequencing (accept/reject isn't known until the whole injecting session's output is parsed after it returns); confirmed benign, the final `controller-check-results.tsv` still correctly omits rejected names. Also: invoking `--internal-controller-check` directly with fewer than 3 args hits `set -u` and dies with a raw "unbound variable" message instead of a clean `fail()` — cosmetic only, this path is unreachable from any normal invocation.
|
||||
- Minor non-blocking follow-up from task 6.3's review: `poll_scheduler_log()`'s output isn't filtered to the caller's `pending` list (harmless today since the only caller already filters, but worth tightening).
|
||||
- Task 6.2 (round 1 REJECTED, round 2 fixed) discovered **bug-1579**: any host-attrs-derived resource (`type`/`model`/`cpuf`/`maxcpus`/`maxmem`/`maxswap`/`maxtmp`/`ndisks` — `worker.c`'s `N_HOST_ATTRS`) gets reset to baseline by the controller the instant the synthetic worker disconnects, and `inject_resources()` (6.1) always disconnects (`quit`) before each layer's separate check session runs — so a controller-layer check on any host-attrs name will ALWAYS false-MISMATCH under the current two-session structure. Fixed for the default set by swapping `DEFAULT_RESOURCE_ARGS`'s `cpuf` for `r1m` (periodic/dynamic, confirmed safe). **This risk is NOT fully closed** — a user-supplied custom `resource=value` argument using any host-attrs name will still hit this. A real fix requires either a C-level `xce_test` change (its REPL has no path from the `worker` context to the `client` context without a disconnect-equivalent `leave_ctx()`) or a bash coprocess restructuring to keep the worker connected across all layer checks — out of scope for tasks 6.1-6.4, worth a dedicated follow-up task if host-attrs resources ever need default-set coverage.
|
||||
- Task 6.1 uncovered and got fixed a critical, unrelated, pre-existing controller bug (**bug-1578**, commit `b406dd7`, standalone — a dangling-pointer use-after-free in `sched_worker_change()`/`sched.c` that crashed the controller on worker disconnect once a scheduler had ever joined, 5 months old, unrelated to this feature). It's fixed now with regression coverage (`test_sched_worker_change.c`). If any later task sees a live controller die unexpectedly during a worker-disconnect sequence, confirm you're building from a commit that includes `b406dd7` before assuming it's a new bug. `inject_resources()` also fixed **bug-1577** (in-boundary): `XCE_WORKER_BIN=/bin/true` is exported before invoking `start-all.sh` to suppress the REAL `xcecd_virtual` worker daemon that `deploy-ha.sh`'s pre-existing worker branch would otherwise silently start (Requirement 2.1 violation) whenever the build dir happens to contain one.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue