diff --git a/.kiro/specs/admin-config-tool/tasks.md b/.kiro/specs/admin-config-tool/tasks.md index 3f099e76..a6366e42 100644 --- a/.kiro/specs/admin-config-tool/tasks.md +++ b/.kiro/specs/admin-config-tool/tasks.md @@ -121,7 +121,7 @@ - _Boundary: xcecommon.schema, xcecommon.validation_ - _Depends: 2.1, 2.2, 1.3_ -- [ ] 5.4 (P) Golden-file test config build determinism +- [x] 5.4 (P) Golden-file test config build determinism - Building from a minimal fixture, a full fixture covering all seven file types, and a set of intentionally invalid fixtures each produce the expected result - Building the same fixture twice produces byte-identical output and an identical checksum - _Requirements: 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 1.10, 1.11, 1.12, 9.1, 9.2, 9.3_ @@ -158,3 +158,4 @@ - **4.2**: `deploy-ha.sh` resolves both `xceadmin` (`$SCRIPT_DIR/../xceadmin/.venv/bin/xceadmin`) and the YAML configs source (`$SCRIPT_DIR/../configs`) relative to the script's own location (`$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)`), not the caller's CWD — matches the project's other relative-path scripts and keeps the deploy working regardless of where it's invoked from. `--build-config` is parsed out of `"$@"` before positional-arg assignment so it can appear anywhere in the argument list, not just a fixed position. Verified live against the real `xceadmin` build and `src/configs/`: missing `cluster.bin` auto-builds (8.4), an existing one with no flag is left byte-for-byte untouched — same mtime, same SHA (8.5), and `--build-config` against an existing `cluster.bin` forces a rebuild that reproduces byte-identical content (8.3, confirming 9.1's determinism guarantee holds through this path too). - **5.1, 5.2**: Already fully satisfied by task 2.3's `test_mapping.py` and task 2.4's `test_wire.py` respectively — same requirement lists, same acceptance bullets (in-memory-only mapper coverage + the explicit two-field case for 5.1; a fixed known byte sequence + real loopback round-trips for 5.2). No new tests written; would have been pure duplication. - **5.3**: Found and fixed a real gap while writing `test_schema_bundling.py` — `xcecommon.validation.validate_yaml()`'s `json.loads(schema_path.read_text())` call (loading the *bundled JSON validation schema itself*, not the operator's YAML) had no error handling, so a corrupted bundled JSON schema crashed with a raw `json.JSONDecodeError` instead of a clean `ValidationError` — the same "crash vs. clean error" class of bug task 2.2 already fixed for `controller.capnp`, just on the JSON-schema side, and not caught by task 2.1 since that task's tests only exercised operator-YAML failure modes, never a corrupted *schema* file. Fixed by wrapping it and raising `ValidationError(..., constraint="schema-corrupted")`, matching the existing missing-schema handling's pattern one function above it. **Gotcha for any future task touching `xcecommon`/`xceadmin` source**: editing a source file does NOT propagate to a running `.venv` until reinstalled — always re-run `xce-build.sh xceadmin` after a source edit before trusting a bare `.venv/bin/python -m pytest` result; a stale venv (especially one left in `release`/pinned mode by a prior profile run) will silently test the *old* code and produce a false failure (or false pass) that looks like a real bug. +- **5.4**: `tests/fixtures/{minimal,full}/configs/*.yaml` + checked-in `expected.bin` golden files added to `test_config_build.py` (not a new file — design.md's File Structure Plan lists the SAME requirement set for `test_config_build.py` under both 3.1 and 5.4). `full/configs/*.yaml` is a byte-for-byte copy of `src/configs/*.yaml`; its golden SHA (`860cfd4f59ef...`) independently matches the tag `deploy-ha.sh` produced in task 4.2's live verification against the same source files — an unplanned but welcome additional confirmation of Requirement 9's cross-context determinism guarantee. Golden-file tests catch representation drift (bytes change even though decoded content is still semantically correct) that purely structural/decode-based assertions elsewhere in the suite cannot detect; regenerate `expected.bin` deliberately (via `build_cluster_config()`, sanity-checked by decoding it back) whenever a real, intentional field-mapping change lands — never hand-edit the binary. diff --git a/src/xceadmin/tests/fixtures/full/configs/hosts.yaml b/src/xceadmin/tests/fixtures/full/configs/hosts.yaml new file mode 100644 index 00000000..d9d57167 --- /dev/null +++ b/src/xceadmin/tests/fixtures/full/configs/hosts.yaml @@ -0,0 +1,39 @@ +--- +hosts: + - hostname: default + type: "x86_64-linux-gnu" + model: "x86_64-linux-gnu" + max_slot: -1 + description: "default host" + resource: + - name: mg + - name: maxswap + value: 10000 + sched_stop: + - resname: r1m + sched: 3.5 + stop: 4.5 + - resname: r15s + sched: 4.0 + stop: 10.0 + dispatch_window: + - start_time: "9:00" + end_time: "18:00" + - start_time: "21:00" + end_time: "7:00" + + - hostname: worker2 + model: "!" + type: "!" + max_slot: 256 + resource: + - name: gstring + value: "This is a G string" + + +hostgroups: + - groupname: hg1 + members: + - all + exclude_members: [h3] + diff --git a/src/xceadmin/tests/fixtures/full/configs/limits.yaml b/src/xceadmin/tests/fixtures/full/configs/limits.yaml new file mode 100644 index 00000000..127312fd --- /dev/null +++ b/src/xceadmin/tests/fixtures/full/configs/limits.yaml @@ -0,0 +1,49 @@ +--- +limits: + # cap total slots for ug1 across the whole cluster + - name: ug1_slots + users: + members: + - ug1 + resname: slots + value: 10 + + # per-user slot cap inside ug2 (each member gets up to 4, not a shared pool) + - name: ug2_slots_per_user + users: + members: + - ug2 + exclude_members: + - user3 + resname: slots + value: 4 + per_user: true + + # queue-scoped memory cap, percentage of total mem resource + - name: night_mem_pct + queues: + members: + - night + resname: mem + value: 50 + percent: true + + # host-scoped cap, applied per host in hg1 + - name: hg1_slots_per_host + hosts: + members: + - hg1 + resname: slots + value: 20 + per_host: true + + # combined scope: ug1 members on the normal queue + - name: ug1_normal_slots + users: + members: + - ug1 + queues: + members: + - normal + resname: slots + value: 8 diff --git a/src/xceadmin/tests/fixtures/full/configs/queues.yaml b/src/xceadmin/tests/fixtures/full/configs/queues.yaml new file mode 100644 index 00000000..9bafd74c --- /dev/null +++ b/src/xceadmin/tests/fixtures/full/configs/queues.yaml @@ -0,0 +1,94 @@ +--- +queues: + - name: xce + description: "xce queue" + priority: 30 + fairshare: + - user: default + share: 100 + hosts: + members: + - all + + - name: normal + description: "normal queue" + priority: 40 + nice: 10 + fairshare: + - user: default + share: 100 + hosts: + members: + - all + + + - name: night + description: "night queue" + priority: 30 + nice: 10 + res_req: "mem>1000 rusage[mem=1000]" + gpu_req: "num=1:mode=shared:gvendor=NVIDIA:gmodel=A100" + fairshare: + - user: ug1 + share: 1000 + - user: others + share: 100 + cpulimit: 100 + memlimit: 1000 + runlimit: 20 + runtimeest: 30 + dispatch_window: + - start_time: "19:00" + end_time: "8:30" + run_window: + - start_time: "5:19:00" + end_time: "1:8:30" + - start_time: "20:00" + end_time: "8:30" + sched_stop: + - resname: tmp + sched: 2000 + stop: 1000 + - resname: res1 + sched: 1.0 + stop: 2.5 + reservation: 100 + preemption: preemptive + backfill: true + exclusive: false + rerunable: true + pre_exec: "/path/to/preexec.sh" + post_exec: "/path/to/postexec.sh" + job_starter: "/path/to/jobstarter.sh" + users: + members: + - ug1 + - ug2 + exclude_members: + - user3 + hosts: + members: + - hgroup1 + - hgroup2 + exclude_members: + - host3 + + - name: short + description: "for short jobs" + priority: 45 + cpulimit: 15 + fairshare: + - user: ug1 + share: 1000 + - user: default + share: 100 + reservation: 10 + preemption: preemptable + backfill: true + users: + members: + - ugroup1 + hosts: + members: + - hgroup1 + diff --git a/src/xceadmin/tests/fixtures/full/configs/resources.yaml b/src/xceadmin/tests/fixtures/full/configs/resources.yaml new file mode 100644 index 00000000..f12b9f82 --- /dev/null +++ b/src/xceadmin/tests/fixtures/full/configs/resources.yaml @@ -0,0 +1,199 @@ +--- +resources: + # built-in: job slot count (consumed by running jobs, decreases as slots fill) + - name: slots + type: numeric + order: decrease + consumable: true + description: "job slot count (built-in)" + + # built-in: dynamic memory load index reported by workers + - name: mem + type: numeric + interval: 60 + consumable: false + order: decrease + description: "available memory in MB (built-in dynamic load index)" + + # built-in host attributes, registered per-worker by + # controller/worker.c add_hostattrs_resvals() + - name: type + type: string + description: "host OS/arch type (built-in host attribute)" + + - name: model + type: string + description: "host hardware model (built-in host attribute)" + + - name: cpuf + type: numeric + description: "CPU speed factor (built-in host attribute)" + + - name: maxcpus + type: numeric + description: "max CPU count (built-in host attribute)" + + - name: maxmem + type: numeric + description: "max memory in MB (built-in host attribute)" + + - name: maxswap + type: numeric + description: "max swap in MB (built-in host attribute)" + + - name: maxtmp + type: numeric + description: "max tmp disk space in MB (built-in host attribute)" + + - name: ndisks + type: numeric + description: "disk count (built-in host attribute)" + + # dynamic hostbased resource, external load index + - name: dynres1 + type: numeric + interval: 60 + order: decrease + consumable: true + release: true + description: "user defined dynamic resource" + resourcemap: + - location: + - default + + + # dynamic share resource, 2 instances + - name: dynres2 + type: numeric + interval: 60 + consumable: false + order: decrease + description: "user defined dynamic resource" + resourcemap: + - location: + - host1 + - host2 + - location: + - host5 + - host6 + + + # static share resource, all host share 100 res. + - name: staticres4 + type: numeric + interval: 0 + consumable: true + order: decrease + resourcemap: + - location: + - all + value: 100 + + # static share resource, host1 and host2 share the 100 res. + - name: staticres1 + type: numeric + interval: 0 + consumable: true + order: decrease + resourcemap: + - value: 100 + location: + - host1 + - host2 + + # static share resource, each of host1 and host2 shares the 100 res. + - name: staticres2 + type: numeric + interval: 0 + consumable: false + order: decrease + resourcemap: + - value: 100 + location: + - host1 + - value: 100 + location: + - host2 + + # static hostbased resource. + - name: staticres3 + type: numeric + interval: 0 + consumable: true + order: decrease + resourcemap: + - value: 100 + location: + - default + + - name: res1 + type: numeric + interval: 0 + consumable: true + order: decrease + resourcemap: + - location: + - all + value: 100 + + - name: sres1 + type: numeric + interval: 0 + consumable: true + order: decrease + + + # boolean resource + - name: bigmem + type: boolean + order: na + resourcemap: + - value: true + location: + - host1 + - value: true + location: + - host2 + - value: true + location: + - host3 + + # dynamic string resource + - name: strres1 + type: string + interval: 60 + consumable: false + order: na + description: "string resources" + resourcemap: + - location: + - default + + # static string resource + - name: strres2 + type: string + description: "string resources" + resourcemap: + - value: v1 + location: + - host1 + - value: v2 + location: + - host2 + - value: v3 + location: + - host3 + - name: host_quota + description: quota on each host + type: numeric + order: decrease + consumable: true + interval: 0 + resourcemap: + - value: 10 + location: + - default + + - name: gstring + description: string resource + type: string diff --git a/src/xceadmin/tests/fixtures/full/configs/scaler.yaml b/src/xceadmin/tests/fixtures/full/configs/scaler.yaml new file mode 100644 index 00000000..9f904ea1 --- /dev/null +++ b/src/xceadmin/tests/fixtures/full/configs/scaler.yaml @@ -0,0 +1,13 @@ +--- +scalers: + # script-based cloud autoscaler provider + - name: aws-asg + type: script + conf_path: /etc/xce/scalers/aws-asg.conf + script_path: /opt/xce/scalers/aws_asg.sh + script_option: "--region us-east-1" + + # minimal provider, no conf file or options + - name: k8s-pool + type: script + script_path: /opt/xce/scalers/k8s_pool.sh diff --git a/src/xceadmin/tests/fixtures/full/configs/users.yaml b/src/xceadmin/tests/fixtures/full/configs/users.yaml new file mode 100644 index 00000000..b8841ee2 --- /dev/null +++ b/src/xceadmin/tests/fixtures/full/configs/users.yaml @@ -0,0 +1,8 @@ +--- +#define usergroup share +usergroups: + - groupname: ug1 + members: + - user1 + - user2 + diff --git a/src/xceadmin/tests/fixtures/full/configs/xce.yaml b/src/xceadmin/tests/fixtures/full/configs/xce.yaml new file mode 100644 index 00000000..05a0fef2 --- /dev/null +++ b/src/xceadmin/tests/fixtures/full/configs/xce.yaml @@ -0,0 +1,6 @@ +--- +cluster_name: eda1 +heartbeat_interval: 30 +inactive_heartbeat_count: 3 +fetch_job_interval: 10 +netio_timeout: 30 diff --git a/src/xceadmin/tests/fixtures/full/expected.bin b/src/xceadmin/tests/fixtures/full/expected.bin new file mode 100644 index 00000000..13aa3878 Binary files /dev/null and b/src/xceadmin/tests/fixtures/full/expected.bin differ diff --git a/src/xceadmin/tests/fixtures/minimal/configs/hosts.yaml b/src/xceadmin/tests/fixtures/minimal/configs/hosts.yaml new file mode 100644 index 00000000..feea3662 --- /dev/null +++ b/src/xceadmin/tests/fixtures/minimal/configs/hosts.yaml @@ -0,0 +1,8 @@ +hosts: + - hostname: worker1 + type: x86_64-linux-gnu + max_slot: 8 +hostgroups: + - groupname: all_hosts + members: + - worker1 diff --git a/src/xceadmin/tests/fixtures/minimal/configs/queues.yaml b/src/xceadmin/tests/fixtures/minimal/configs/queues.yaml new file mode 100644 index 00000000..d091cc35 --- /dev/null +++ b/src/xceadmin/tests/fixtures/minimal/configs/queues.yaml @@ -0,0 +1,3 @@ +queues: + - name: normal + priority: 30 diff --git a/src/xceadmin/tests/fixtures/minimal/expected.bin b/src/xceadmin/tests/fixtures/minimal/expected.bin new file mode 100644 index 00000000..d2ac36de Binary files /dev/null and b/src/xceadmin/tests/fixtures/minimal/expected.bin differ diff --git a/src/xceadmin/tests/test_config_build.py b/src/xceadmin/tests/test_config_build.py index 93f850a4..5610c725 100644 --- a/src/xceadmin/tests/test_config_build.py +++ b/src/xceadmin/tests/test_config_build.py @@ -1,6 +1,15 @@ """Golden-file-style build tests for ``xceadmin.build`` (design.md, -``xceadmin.build``; Requirements 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.9, -8.3, 8.4). +``xceadmin.build``; Requirements 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, +1.9, 1.10, 1.11, 1.12, 8.3, 8.4, 9.1, 9.2, 9.3). + +The ``tests/fixtures/minimal`` and ``tests/fixtures/full`` directories +(task 5.4) each pair a real, schema-valid ``configs/`` input with a +checked-in ``expected.bin`` -- unlike the dynamic build-then-compare +tests elsewhere in this file, these catch *unintended representation +drift*: a future change that is semantically correct (decodes fine, +right field values) but produces different serialized bytes than +before would still be a determinism regression (Requirement 9) that +purely structural assertions would never notice. """ from __future__ import annotations @@ -259,3 +268,52 @@ def test_write_output_does_not_print_the_hash_itself(tmp_path, capsys): captured = capsys.readouterr() assert captured.err == "" + + +# --------------------------------------------------------------------------- +# Golden-file determinism (task 5.4) +# --------------------------------------------------------------------------- + +FIXTURES_DIR = Path(__file__).resolve().parent / "fixtures" + + +@pytest.mark.parametrize("fixture_name", ["minimal", "full"]) +def test_golden_fixture_matches_checked_in_expected_bin(fixture_name): + fixture_dir = FIXTURES_DIR / fixture_name + data = build.build_cluster_config(str(fixture_dir / "configs")) + expected = (fixture_dir / "expected.bin").read_bytes() + + assert data == expected + + +@pytest.mark.parametrize("fixture_name", ["minimal", "full"]) +def test_golden_fixture_builds_twice_identically(fixture_name): + fixture_dir = FIXTURES_DIR / fixture_name / "configs" + + first = build.build_cluster_config(str(fixture_dir)) + second = build.build_cluster_config(str(fixture_dir)) + + assert first == second + assert hashlib.sha256(first).hexdigest() == hashlib.sha256(second).hexdigest() + + +def test_full_golden_fixture_covers_all_seven_file_types(): + fixture_dir = FIXTURES_DIR / "full" / "configs" + present = {p.name for p in fixture_dir.glob("*.yaml")} + + assert present == { + "hosts.yaml", + "queues.yaml", + "resources.yaml", + "users.yaml", + "limits.yaml", + "xce.yaml", + "scaler.yaml", + } + + +def test_minimal_golden_fixture_covers_hosts_and_queues_only(): + fixture_dir = FIXTURES_DIR / "minimal" / "configs" + present = {p.name for p in fixture_dir.glob("*.yaml")} + + assert present == {"hosts.yaml", "queues.yaml"}