feat(worker-rejoin-resync): wire config fields into xceadmin build (task 1.5)
Add workerResyncWindowMs/workerResyncTimeoutMs to xce.schema.json (minimum:0, matching the optional-scalar-field precedent -- these aren't `required` with a floor like heartbeatInterval, so minimum:1 would be wrong) and to mapping.py's YAML->capnp field table. Regenerate both golden config-build fixtures (task 1.1 grew ClusterConfig's wire layout by 2 fields; full/expected.bin and minimal/expected.bin each grew by exactly 8 bytes). full's fixture YAML now sets real values (900000ms/15000ms); minimal deliberately left untouched (no xce.yaml) per its own locked-down "hosts+queues only" boundary test -- its expected.bin still regenerates to reflect the new default-0 fields. Two new tests assert specific decoded values, not just "build succeeds". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
This commit is contained in:
parent
9f3dfe7903
commit
ce3d391e5e
8 changed files with 81 additions and 9 deletions
|
|
@ -28,7 +28,7 @@
|
||||||
- _Boundary: wire codec_
|
- _Boundary: wire codec_
|
||||||
- _Depends: 1.1_
|
- _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
|
- 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)
|
- 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
|
- 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
|
## 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 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 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.
|
||||||
|
|
|
||||||
|
|
@ -29,6 +29,16 @@
|
||||||
"type": "integer",
|
"type": "integer",
|
||||||
"minimum": 0,
|
"minimum": 0,
|
||||||
"description": "network I/O timeout in seconds"
|
"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"]
|
"required": ["cluster_name", "heartbeat_interval", "inactive_heartbeat_count"]
|
||||||
|
|
|
||||||
|
|
@ -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
|
fields, ``hgroups`` and ``hostConfigs``; see the Data Models table in
|
||||||
design.md). Two mapper shapes cover every section:
|
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.
|
top-level ``ClusterConfig`` scalar fields.
|
||||||
- :func:`map_list_section` -- the 7 remaining sections, each a YAML list
|
- :func:`map_list_section` -- the 7 remaining sections, each a YAML list
|
||||||
mapped to a capnp ``List(Struct)`` field, preserving YAML source order
|
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"),
|
("inactive_heartbeat_count", "inactiveHeartbeatCount"),
|
||||||
("fetch_job_interval", "fetchJobInterval"),
|
("fetch_job_interval", "fetchJobInterval"),
|
||||||
("netio_timeout", "netioTimeout"),
|
("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
|
# resources.yaml ``type``/``order`` -> Resource.valtype/ordtype, mirroring
|
||||||
|
|
@ -500,7 +506,7 @@ def map_list_section(
|
||||||
def map_scalar_section(
|
def map_scalar_section(
|
||||||
spec: SectionSpec, values: dict | None, target: "capnp._DynamicStructBuilder" # noqa: ARG001
|
spec: SectionSpec, values: dict | None, target: "capnp._DynamicStructBuilder" # noqa: ARG001
|
||||||
) -> None:
|
) -> 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`).
|
fields directly from ``values`` (:data:`_XCE_SCALAR_FIELDS`).
|
||||||
|
|
||||||
``spec`` is accepted for interface symmetry with :func:`map_list_section`
|
``spec`` is accepted for interface symmetry with :func:`map_list_section`
|
||||||
|
|
|
||||||
|
|
@ -4,3 +4,5 @@ heartbeat_interval: 30
|
||||||
inactive_heartbeat_count: 3
|
inactive_heartbeat_count: 3
|
||||||
fetch_job_interval: 10
|
fetch_job_interval: 10
|
||||||
netio_timeout: 30
|
netio_timeout: 30
|
||||||
|
worker_resync_window_ms: 900000
|
||||||
|
worker_resync_timeout_ms: 15000
|
||||||
|
|
|
||||||
BIN
src/xceadmin/tests/fixtures/full/expected.bin
vendored
BIN
src/xceadmin/tests/fixtures/full/expected.bin
vendored
Binary file not shown.
BIN
src/xceadmin/tests/fixtures/minimal/expected.bin
vendored
BIN
src/xceadmin/tests/fixtures/minimal/expected.bin
vendored
Binary file not shown.
|
|
@ -83,6 +83,50 @@ def test_build_same_input_twice_is_byte_identical(tmp_path):
|
||||||
assert first == second
|
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)
|
# Hard failures propagate (schema violation, reserved name, duplicates)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
|
|
@ -60,11 +60,12 @@ def test_section_map_hosts_yaml_feeds_two_distinct_capnp_fields():
|
||||||
assert yaml_keys == {"hostgroups", "hosts"}
|
assert yaml_keys == {"hostgroups", "hosts"}
|
||||||
|
|
||||||
|
|
||||||
def test_section_map_covers_all_13_clusterconfig_fields_or_documents_exclusion():
|
def test_section_map_covers_all_14_clusterconfig_fields_or_documents_exclusion():
|
||||||
# 5 scalar (xce.yaml) + 7 list capnp_fields covers 12 of the 13
|
# 7 scalar (xce.yaml, including worker-rejoin-resync's workerResyncWindowMs/
|
||||||
# ClusterConfig fields; configGeneration (@12) is the one deliberately
|
# workerResyncTimeoutMs, task 1.5) + 7 list capnp_fields covers 14 of the
|
||||||
# excluded field (Requirement 1.9), documented in the module docstring,
|
# 15 ClusterConfig fields (@0-@14); configGeneration (@12) is the one
|
||||||
# not present in SECTION_MAP at all.
|
# 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"}
|
list_fields = {s["capnp_field"] for s in mapping.SECTION_MAP if s["kind"] == "list"}
|
||||||
assert list_fields == {
|
assert list_fields == {
|
||||||
"resources",
|
"resources",
|
||||||
|
|
@ -83,6 +84,8 @@ def test_section_map_covers_all_13_clusterconfig_fields_or_documents_exclusion()
|
||||||
"inactiveHeartbeatCount",
|
"inactiveHeartbeatCount",
|
||||||
"fetchJobInterval",
|
"fetchJobInterval",
|
||||||
"netioTimeout",
|
"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]
|
spec = mapping.SECTION_MAP[0]
|
||||||
values = {
|
values = {
|
||||||
"cluster_name": "eda1",
|
"cluster_name": "eda1",
|
||||||
|
|
@ -99,6 +102,8 @@ def test_map_scalar_section_sets_all_5_fields(cluster_config):
|
||||||
"inactive_heartbeat_count": 3,
|
"inactive_heartbeat_count": 3,
|
||||||
"fetch_job_interval": 10,
|
"fetch_job_interval": 10,
|
||||||
"netio_timeout": 30,
|
"netio_timeout": 30,
|
||||||
|
"worker_resync_window_ms": 900000,
|
||||||
|
"worker_resync_timeout_ms": 15000,
|
||||||
}
|
}
|
||||||
mapping.map_scalar_section(spec, values, cluster_config)
|
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.inactiveHeartbeatCount == 3
|
||||||
assert cluster_config.fetchJobInterval == 10
|
assert cluster_config.fetchJobInterval == 10
|
||||||
assert cluster_config.netioTimeout == 30
|
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):
|
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.clusterName == "eda1"
|
||||||
assert cluster_config.fetchJobInterval == 0
|
assert cluster_config.fetchJobInterval == 0
|
||||||
assert cluster_config.netioTimeout == 0
|
assert cluster_config.netioTimeout == 0
|
||||||
|
assert cluster_config.workerResyncWindowMs == 0
|
||||||
|
assert cluster_config.workerResyncTimeoutMs == 0
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize("empty", [None, {}])
|
@pytest.mark.parametrize("empty", [None, {}])
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue