diff --git a/.kiro/specs/worker-rejoin-resync/tasks.md b/.kiro/specs/worker-rejoin-resync/tasks.md index 536ffb0f..c8305de8 100644 --- a/.kiro/specs/worker-rejoin-resync/tasks.md +++ b/.kiro/specs/worker-rejoin-resync/tasks.md @@ -28,7 +28,7 @@ - _Boundary: wire codec_ - _Depends: 1.1_ -- [ ] 1.5 (P) Wire the new config fields into xceadmin's config-build pipeline +- [x] 1.5 (P) Wire the new config fields into xceadmin's config-build pipeline - Add the two new cluster-config fields (the recently-finished reporting window, the reconciliation timeout) to xceadmin's YAML validation schema and its YAML-to-binary field mapping, following the existing `heartbeatInterval`/`inactiveHeartbeatCount` pattern exactly, so an operator can actually set them through `xceadmin config build` — added to the wire schema by task 1.1, but with no configuration path into it until this task - Regenerate the checked-in golden config-build fixtures to match the wire schema's current field set (task 1.1 grew it; the fixtures predate that and no longer match a fresh build) - Observable completion: a YAML config supplying values for both new fields produces a binary that decodes back to those exact values, and the existing golden-fixture regression tests pass again @@ -153,3 +153,4 @@ ## Implementation Notes - Task 1.3: this task's premise (`src/worker` has no `testsuite/`) is FALSE as of implementation time — real, tracked worker test infrastructure already exists (`src/worker/testsuite/{meson.build,test_*.c}`, 5 passing tests, `suite: 'worker'` registered in meson, `/xce-build worker` already runs it), built by `component-failover-adoption` (commits `ce9f734a`/`331fa024`/`619b119e`, 2026-08-11) weeks before this spec was written. `.kiro/steering/structure.md`, `testing.md:55`, and `build-deployment.md:16` are all stale on this point — worth a `/kiro-steering` sync, out of this feature's boundary to fix. Confirmed directly (fresh build + `meson test -C src/xce.build --suite worker`: 5/5 OK) rather than trusting the finding secondhand. No files changed for this task — closed as satisfied by pre-existing infrastructure. - **Task 4.2 must follow the EXISTING worker testsuite convention**, not the controller aggregator-runner pattern the original task 1.3 text pointed at: each test file is its own standalone Unity executable (own `UNITY_BEGIN()`/`RUN_TEST`/`UNITY_END()`, white-box `#include`s the production `.c` file under test — see `test_resource_sync.c`'s shape) with its own explicit `executable()` + `test(..., suite: 'worker', ...)` block added to `src/worker/testsuite/meson.build`, mirroring `test_controller_connector.c`'s registration exactly. Do not introduce a second, competing test-runner shape. +- Task 1.5: `xceadmin config dump`'s `_format_cluster_config` (`build.py`) does not print `workerResyncWindowMs`/`workerResyncTimeoutMs` — an operator can set them via `config build` but can't verify the built value via `config dump`. Confirmed genuinely out of task 1.5's boundary (config-*build*, not config-*dump*) and not required by Requirements 3.7/5.3 as written, but a real, minor observability gap worth a small follow-up task if this becomes a live operator pain point. Not blocking; not added to this plan. diff --git a/src/configs/schema/xce.schema.json b/src/configs/schema/xce.schema.json index f799b200..9cdf7a56 100644 --- a/src/configs/schema/xce.schema.json +++ b/src/configs/schema/xce.schema.json @@ -29,6 +29,16 @@ "type": "integer", "minimum": 0, "description": "network I/O timeout in seconds" + }, + "worker_resync_window_ms": { + "type": "integer", + "minimum": 0, + "description": "worker job-state reconciliation: how far back (in milliseconds) a recently-finished job stays eligible for extra reporting on rejoin" + }, + "worker_resync_timeout_ms": { + "type": "integer", + "minimum": 0, + "description": "worker job-state reconciliation: how long (in milliseconds) the controller waits for a rejoining worker's job-status report before resolving the reconciliation as timed out" } }, "required": ["cluster_name", "heartbeat_interval", "inactive_heartbeat_count"] diff --git a/src/xceadmin/src/xcecommon/mapping.py b/src/xceadmin/src/xcecommon/mapping.py index 1b0089dd..9a53cc2f 100644 --- a/src/xceadmin/src/xcecommon/mapping.py +++ b/src/xceadmin/src/xcecommon/mapping.py @@ -13,7 +13,7 @@ entries -- ``hosts.yaml`` is the one file that feeds two separate capnp fields, ``hgroups`` and ``hostConfigs``; see the Data Models table in design.md). Two mapper shapes cover every section: -- :func:`map_scalar_section` -- the single flat ``xce.yaml`` section, 5 +- :func:`map_scalar_section` -- the single flat ``xce.yaml`` section, 7 top-level ``ClusterConfig`` scalar fields. - :func:`map_list_section` -- the 7 remaining sections, each a YAML list mapped to a capnp ``List(Struct)`` field, preserving YAML source order @@ -180,6 +180,12 @@ _XCE_SCALAR_FIELDS: tuple[tuple[str, str], ...] = ( ("inactive_heartbeat_count", "inactiveHeartbeatCount"), ("fetch_job_interval", "fetchJobInterval"), ("netio_timeout", "netioTimeout"), + # worker-rejoin-resync (Requirements 3.7, 5.3): shared, cluster-wide + # via operational-uniformity override (design.md Configuration + # Model) -- same domain as heartbeat_interval/inactive_heartbeat_count + # above, wired the same way (task 1.5). + ("worker_resync_window_ms", "workerResyncWindowMs"), + ("worker_resync_timeout_ms", "workerResyncTimeoutMs"), ) # resources.yaml ``type``/``order`` -> Resource.valtype/ordtype, mirroring @@ -500,7 +506,7 @@ def map_list_section( def map_scalar_section( spec: SectionSpec, values: dict | None, target: "capnp._DynamicStructBuilder" # noqa: ARG001 ) -> None: - """Set each of the 5 ``xce.yaml``-sourced scalar ``ClusterConfig`` + """Set each of the 7 ``xce.yaml``-sourced scalar ``ClusterConfig`` fields directly from ``values`` (:data:`_XCE_SCALAR_FIELDS`). ``spec`` is accepted for interface symmetry with :func:`map_list_section` diff --git a/src/xceadmin/tests/fixtures/full/configs/xce.yaml b/src/xceadmin/tests/fixtures/full/configs/xce.yaml index 05a0fef2..c88b28e4 100644 --- a/src/xceadmin/tests/fixtures/full/configs/xce.yaml +++ b/src/xceadmin/tests/fixtures/full/configs/xce.yaml @@ -4,3 +4,5 @@ heartbeat_interval: 30 inactive_heartbeat_count: 3 fetch_job_interval: 10 netio_timeout: 30 +worker_resync_window_ms: 900000 +worker_resync_timeout_ms: 15000 diff --git a/src/xceadmin/tests/fixtures/full/expected.bin b/src/xceadmin/tests/fixtures/full/expected.bin index 3da1557f..8fcc5094 100644 Binary files a/src/xceadmin/tests/fixtures/full/expected.bin and b/src/xceadmin/tests/fixtures/full/expected.bin differ diff --git a/src/xceadmin/tests/fixtures/minimal/expected.bin b/src/xceadmin/tests/fixtures/minimal/expected.bin index d2ac36de..4d8e1aa1 100644 Binary files a/src/xceadmin/tests/fixtures/minimal/expected.bin 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 937b89c5..3e279d29 100644 --- a/src/xceadmin/tests/test_config_build.py +++ b/src/xceadmin/tests/test_config_build.py @@ -83,6 +83,50 @@ def test_build_same_input_twice_is_byte_identical(tmp_path): assert first == second +# --------------------------------------------------------------------------- +# worker-rejoin-resync (Requirements 3.7, 5.3): workerResyncWindowMs/ +# workerResyncTimeoutMs are operator-settable through xce.yaml's scalar +# section, following the heartbeat_interval/inactive_heartbeat_count +# pattern exactly (task 1.5). These prove the VALUE round-trips +# (YAML -> binary -> decode) with a specific, checkable pair of numbers -- +# not just that some golden blob happens to match. +# --------------------------------------------------------------------------- + + +def test_build_maps_worker_resync_window_and_timeout_fields(tmp_path): + _write_yaml( + tmp_path / "xce.yaml", + { + "cluster_name": "eda1", + "heartbeat_interval": 30, + "inactive_heartbeat_count": 3, + "worker_resync_window_ms": 900000, + "worker_resync_timeout_ms": 15000, + }, + ) + + data = build.build_cluster_config(str(tmp_path)) + + mod = load_schema() + with mod.ClusterConfig.from_bytes(data) as reader: + assert reader.workerResyncWindowMs == 900000 + assert reader.workerResyncTimeoutMs == 15000 + + +def test_build_leaves_worker_resync_fields_at_default_when_absent(tmp_path): + _write_yaml( + tmp_path / "xce.yaml", + {"cluster_name": "eda1", "heartbeat_interval": 30, "inactive_heartbeat_count": 3}, + ) + + data = build.build_cluster_config(str(tmp_path)) + + mod = load_schema() + with mod.ClusterConfig.from_bytes(data) as reader: + assert reader.workerResyncWindowMs == 0 + assert reader.workerResyncTimeoutMs == 0 + + # --------------------------------------------------------------------------- # Hard failures propagate (schema violation, reserved name, duplicates) # --------------------------------------------------------------------------- diff --git a/src/xceadmin/tests/test_mapping.py b/src/xceadmin/tests/test_mapping.py index b8447f0f..95aa5b10 100644 --- a/src/xceadmin/tests/test_mapping.py +++ b/src/xceadmin/tests/test_mapping.py @@ -60,11 +60,12 @@ def test_section_map_hosts_yaml_feeds_two_distinct_capnp_fields(): assert yaml_keys == {"hostgroups", "hosts"} -def test_section_map_covers_all_13_clusterconfig_fields_or_documents_exclusion(): - # 5 scalar (xce.yaml) + 7 list capnp_fields covers 12 of the 13 - # ClusterConfig fields; configGeneration (@12) is the one deliberately - # excluded field (Requirement 1.9), documented in the module docstring, - # not present in SECTION_MAP at all. +def test_section_map_covers_all_14_clusterconfig_fields_or_documents_exclusion(): + # 7 scalar (xce.yaml, including worker-rejoin-resync's workerResyncWindowMs/ + # workerResyncTimeoutMs, task 1.5) + 7 list capnp_fields covers 14 of the + # 15 ClusterConfig fields (@0-@14); configGeneration (@12) is the one + # deliberately excluded field (Requirement 1.9), documented in the + # module docstring, not present in SECTION_MAP at all. list_fields = {s["capnp_field"] for s in mapping.SECTION_MAP if s["kind"] == "list"} assert list_fields == { "resources", @@ -83,6 +84,8 @@ def test_section_map_covers_all_13_clusterconfig_fields_or_documents_exclusion() "inactiveHeartbeatCount", "fetchJobInterval", "netioTimeout", + "workerResyncWindowMs", + "workerResyncTimeoutMs", } @@ -91,7 +94,7 @@ def test_section_map_covers_all_13_clusterconfig_fields_or_documents_exclusion() # --------------------------------------------------------------------------- -def test_map_scalar_section_sets_all_5_fields(cluster_config): +def test_map_scalar_section_sets_all_7_fields(cluster_config): spec = mapping.SECTION_MAP[0] values = { "cluster_name": "eda1", @@ -99,6 +102,8 @@ def test_map_scalar_section_sets_all_5_fields(cluster_config): "inactive_heartbeat_count": 3, "fetch_job_interval": 10, "netio_timeout": 30, + "worker_resync_window_ms": 900000, + "worker_resync_timeout_ms": 15000, } mapping.map_scalar_section(spec, values, cluster_config) @@ -107,6 +112,8 @@ def test_map_scalar_section_sets_all_5_fields(cluster_config): assert cluster_config.inactiveHeartbeatCount == 3 assert cluster_config.fetchJobInterval == 10 assert cluster_config.netioTimeout == 30 + assert cluster_config.workerResyncWindowMs == 900000 + assert cluster_config.workerResyncTimeoutMs == 15000 def test_map_scalar_section_leaves_missing_optional_keys_at_default(cluster_config): @@ -117,6 +124,8 @@ def test_map_scalar_section_leaves_missing_optional_keys_at_default(cluster_conf assert cluster_config.clusterName == "eda1" assert cluster_config.fetchJobInterval == 0 assert cluster_config.netioTimeout == 0 + assert cluster_config.workerResyncWindowMs == 0 + assert cluster_config.workerResyncTimeoutMs == 0 @pytest.mark.parametrize("empty", [None, {}])