wdgzyjx/myxce
1
0
Fork 0
forked from lei.chen/myxce
Code Pull requests Activity

feat(admin-config-tool): task 3.3 - xceadmin.push

Browse source 
push_config()/probe_config(): build/send Request{opcode:upgradeConfig}
and the direct-struct ProbeConfigTagRequest respectively, following
redirects to the primary for the former (ProbeConfigTagReply has no
redirect fields, so probe_config connects directly). Maps controller
status/reply-variant failures to typed results/errors. Tested against
a real local TLS mock controller built from the bundled schema, since
neither opcode has a live controller-side handler today (separately
documented, unrelated to this task).

Unblocked mid-task by bug-1461 (commit e53c086): controller.capnp had
a struct-name collision that miscompiled UpgradeConfigRequest.sha256/
.bytes, making this task's core request impossible to construct via
pycapnp until fixed at the schema root.
This commit is contained in:
Rongsong Shen 2026-07-22 09:30:54 +00:00
parent e53c086ec5
commit c33ef7d479
4 changed files with 455 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

29
.kiro/specs/admin-config-tool/spec.json
View file

@ -1,23 +1,34 @@
{
"feature_name": "admin-config-tool",
"created_at": "2026-07-06T01:30:00Z",
"updated_at": "2026-07-20T00:19:00Z",
"updated_at": "2026-07-22T02:35:00Z",
"language": "en",
"phase": "requirements-generated",
"phase": "tasks-generated",
"approvals": {
"requirements": {
"generated": true,
"approved": false
"approved": true
},
"design": {
"generated": false,
"approved": false
"generated": true,
"approved": true
},
"tasks": {
"generated": false,
"approved": false
"generated": true,
"approved": true
}
},
"ready_for_implementation": false,
"blocked_on": []
"ready_for_implementation": true,
"blocked_on": [
"controller OP_upgradeConfig dispatch defect (cluster.c:667-679 vs ha.c:3926-3951) blocks real-cluster config push E2E validation — see research.md Risks",
"RESOLVED 2026-07-22 (bug-1461, commit e53c086): controller.capnp's struct Data shadowed the capnp builtin blob type, miscompiling UpgradeConfigRequest.sha256/.bytes — fixed at the schema root (renamed to XceData), unblocking push_config()'s request construction. The dispatch defect above is separate and still open — push_config() remains mock-tested only, no live E2E."
],
"gitea": {
"milestone_id": 1,
"issues": {
"requirements_design": 22,
"implementation": 23,
"validation": 24
}
}
}

4
.kiro/specs/admin-config-tool/tasks.md
View file

@ -74,7 +74,7 @@
- _Boundary: xceadmin.discovery_
- _Depends: 2.4_
- [ ] 3.3 Push a cluster binary to the controller, with a dry-run mode
- [x] 3.3 Push a cluster binary to the controller, with a dry-run mode
- A real push reports the assigned configuration version, acknowledgement count, and any error, exiting non-zero on failure
- A dry-run reports whether the controller would accept the config without ever committing it
- Every connection failure, protocol-level rejection, corrupt input, or missing schema maps to a specific, distinct error
@ -152,3 +152,5 @@
- **2.4**: `errors.py`'s exception subclasses (task 1.2) all take `message` as an explicit first constructor argument in addition to their structured fields (e.g. `ConnectionTimeoutError(message, host, port)`, `ReplyDecodeError(message, reason)`) — design.md's abbreviated `Service Interface` type stubs (`class ConnectionTimeoutError(XceAdminError): host: str; port: int`) read as if only the structured fields were needed; always check the real constructor in `errors.py`, not the design doc's shorthand. `xcecommon.wire` is deliberately opcode-agnostic — `send_request`/`send_frame`/`recv_frame` move raw `bytes` only and never import a capnp type; callers own `.to_bytes()`/`.from_bytes()`. Reused `ha/data/xce.crt`/`xce.key` (the repo's existing dev cert pair, same one `deploy-ha.sh` already copies for controller TLS) as the test fixture for both the TLS test server and the client `cert_dir` — no synthetic cert generation needed.
- **3.1**: Requirement 1.7's "unrecognized-but-schema-valid field" warning is real and fires on essentially every real build using `src/configs/resources.yaml` — `resources.yaml`'s `type` key is *required* by `resources.schema.json` (`required: [name, type]`) but has no `ClusterConfig` destination (per `xcecommon.mapping`'s documented decision not to guess the `valtype`/`ordtype`/`flags` integer encoding), so every valid resource entry triggers a "field 'type' ... ignored" warning unconditionally, not just for unusual input. This is a disclosed spec tradeoff, not a bug — flagged here so a future pass over `xceadmin.build`'s warning UX (e.g. only warning once per field-name per run instead of once per item) knows the noise is expected, not a regression. `xceadmin.build`'s unmapped-field table (`_UNMAPPED_BUT_VALID_ITEM_KEYS`) was derived by cross-referencing `xcecommon.mapping`'s per-item builder functions against the real `src/configs/schema/*.schema.json` property lists directly (not from prose alone) to avoid drift.
- **3.2**: design.md's Dependencies list for `xceadmin.discovery` names only `xcecommon.wire`, but decoding a reply to inspect `RequestReply.redirectHost`/`.redirectPort` genuinely requires `xcecommon.schema` too (verified real field names against `controller.capnp:594-595`, which matched design.md exactly this time — unlike task 2.2's `Reply`/`RequestReply` struct-name slip). Treated as a design-doc omission, not license to hand-roll capnp decoding. `follow_redirects`'s `cert_dir` parameter is accepted for signature parity with design.md but unused internally — all I/O, including TLS setup, is delegated to the caller's `send_fn` closure, which is expected to already carry its own `cert_dir`. A connection timeout to any hop (including a non-initial one reached via redirect) raises `NoPrimaryError` naming that specific hop, not the original target.
- **bug-1461 (out-of-band, between 3.2 and 3.3)**: `controller.capnp` had a real, confirmed schema bug — a locally-declared `struct Data` (an `OPCode_get` query-result tagged union) shadowed Cap'n Proto's builtin `Data` blob type for every later `: Data` field in the file, miscompiling `UpgradeConfigRequest.sha256`/`.bytes` (and 5 other fields across `ReplConfigReply`/`ReplConfigPushRequest`/`ReplEnvelope`/`SnapshotTransfer`/`WalCatchupEntry`/`CtrlHelloRequest`, several HA-critical) as a struct-pointer type instead of a byte blob — blocking `push_config()` from constructing a valid request via pycapnp. Fixed at the root (renamed the local struct to `XceData`, `$C.mapname("xce_data_t")` unchanged so the C typedef is stable) across 20 files spanning controller/lib/scheduler/Go — see commit `e53c086` and `.wolf/buglog.json` bug-1461 for the full investigation and fix. This was NOT an admin-config-tool-boundary change; it's a pre-existing, cross-component schema defect that happened to block this task. `push_config()`'s real controller counterpart remains untested live — a *separate*, already-documented dispatch-gate defect (`ha_dispatch_gate()`/`cluster.c`, spec.json `blocked_on`) still blocks that independently of bug-1461.
- **3.3**: `probe_config()` does NOT go through `xceadmin.discovery.follow_redirects` — `ProbeConfigTagReply` is a direct struct with only `configTag`, no `redirectHost`/`.Port` fields exist on it at all, so there is no redirect signal for `follow_redirects` (which unconditionally decodes replies as `RequestReply`) to read; only `push_config()`'s `Request`/`RequestReply`-wrapped exchange supports redirects. `ProbeConfigTagReply` also carries no accept/reject signal — `DryRunResult.accepted` is a client-side inference (true on any successful round-trip), matching the real C reference client (`config_repl.c`'s `xce_probe_config_tag`, which also always sends an empty `name` and returns only a bare tag). pycapnp union field assignment for Void/Text-typed variants (e.g. `reply.data.nodata = None`, `reply.data.debugmsg = "..."`) must use direct assignment, not `.init(name)` — `.init()` without a size is only valid for struct/list/data-typed variants, confirmed via `KjException: init() without a size is only valid for struct and object fields`.

162
src/xceadmin/src/xceadmin/push.py Normal file
View file

@ -0,0 +1,162 @@
"""``config push`` orchestration for xceadmin (design.md, ``xceadmin.push``;
Requirements 2.1, 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, 6.1, 6.2, 6.3, 7.1, 7.2,
7.3, 7.4, 7.5, 7.6).
Two functions, not one boolean-flagged function (Design Review
correction, ``research.md``): ``push_config()`` sends ``Request{opcode:
upgradeConfig, upgradeconfigrequestmsg}`` (``Request``/``RequestReply``
wrapped) and ``probe_config()`` sends ``ProbeConfigTagRequest`` (a direct
struct, no wrapper) -- their replies share no fields, so a shared return
type would misrepresent one of the two.
Both delegate connection + redirect-following to
``xceadmin.discovery.follow_redirects`` before decoding a reply, and both
build/decode using the real ``controller.capnp`` schema
(``xcecommon.schema.load_schema()``) -- never touching the socket
directly except through ``xcecommon.wire``.
``probe_config()`` has no live controller counterpart today
(``research.md``, "ProbeConfigTag liveness": no ``OPCode_probeConfigTag``
dispatch case exists in ``cluster.c``) -- implemented against the
documented wire contract, exercised only by a mock TLS server until the
controller gains a handler. The real C reference client
(``config_repl.c:1907``, ``xce_probe_config_tag``) always sends an empty
``name`` ("anonymous probe; peer logs remote addr") and returns only a
bare config tag with no accept/reject signal -- ``ProbeConfigTagReply``
has no field to carry one. ``probe_config()`` matches this exactly:
``accepted`` is a client-side inference (true on any successful
round-trip), not a value read off the wire, since the wire contract has
nothing else to say. Real SHA/version-compatibility validation
(Requirement 6.1's aspirational description) is future controller-side
work, not something this client can check today.
``probe_config()`` connects directly and does NOT go through
``xceadmin.discovery.follow_redirects``: ``ProbeConfigTagReply`` is a
direct struct with only a ``configTag`` field -- no ``redirectHost``/
``redirectPort`` exist on it at all, so there is no redirect signal for
``follow_redirects`` (which decodes every reply as ``RequestReply``) to
read. Feeding ``ProbeConfigTagReply`` bytes through it would misinterpret
them as a different struct shape. Only ``push_config()``'s
``Request``/``RequestReply``-wrapped exchange supports redirects.
"""
from __future__ import annotations
import hashlib
from dataclasses import dataclass
from pathlib import Path
from xcecommon import wire
from xcecommon.errors import ReplyDecodeError
from xcecommon.schema import load_schema
from xceadmin.discovery import follow_redirects
_DEFAULT_PORT = 9991
@dataclass(frozen=True)
class PushResult:
version_tag: int
acked_count: int
error: str
@dataclass(frozen=True)
class DryRunResult:
current_generation: int
would_be_generation: int
accepted: bool
reason: str
def _split_controller(controller: str) -> tuple[str, int]:
"""``"host:port"`` -> ``(host, port)``; a bare host uses the default
port 9991 (Requirement 2.2's default).
"""
if ":" in controller:
host, port_str = controller.rsplit(":", 1)
return host, int(port_str)
return controller, _DEFAULT_PORT
def _send_via(request_bytes: bytes, cert_dir: str):
def _send(host: str, port: int) -> bytes:
sock = wire.connect(host, port, cert_dir)
try:
return wire.send_request(sock, request_bytes)
finally:
sock.close()
return _send
def push_config(cluster_bin_path: str, controller: str, cert_dir: str) -> PushResult:
"""Push ``cluster_bin_path`` to the controller cluster (2.1). Follows
redirects to the primary (3.1-3.4) before sending.
Postconditions: exit 0 iff ``acked_count > 0`` and ``error == ""``
(2.6); otherwise the caller exits non-zero with ``error`` displayed
(2.7) -- this function only reports the result, it does not exit.
"""
data = Path(cluster_bin_path).read_bytes()
sha256 = hashlib.sha256(data).digest()
mod = load_schema()
request = mod.Request.new_message()
request.opcode = "upgradeConfig"
request.seq = 1
upgrade = request.body.init("upgradeconfigrequestmsg")
upgrade.sha256 = sha256
upgrade.bytes = data
request_bytes = request.to_bytes()
host, port = _split_controller(controller)
_, _, reply_bytes = follow_redirects(host, port, cert_dir, _send_via(request_bytes, cert_dir))
with mod.RequestReply.from_bytes(reply_bytes) as reply:
if reply.status != 0:
return PushResult(
version_tag=0, acked_count=0, error=f"controller returned status {reply.status}"
)
if reply.data.which() != "upgradeconfigreplymsg":
reason = f"unexpected reply variant {reply.data.which()!r}, expected upgradeconfigreplymsg"
raise ReplyDecodeError(f"invalid reply: {reason}", reason)
upgrade_reply = reply.data.upgradeconfigreplymsg
return PushResult(
version_tag=upgrade_reply.versionTag,
acked_count=upgrade_reply.ackedCount,
error=upgrade_reply.error,
)
def probe_config(cluster_bin_path: str, controller: str, cert_dir: str) -> DryRunResult:
"""Probe the controller's current config generation without pushing
anything (6.1). ``cluster_bin_path`` is read only to confirm it
exists and is readable before spending a network round-trip -- its
content plays no role in ``ProbeConfigTagRequest``, which carries no
config data (see module docstring).
Postconditions: exit 0 and "OK: controller will accept this config
(generation N->M)" iff ``accepted`` (6.2); otherwise the caller exits
non-zero with ``reason`` displayed (6.3) -- this function only
reports the result, it does not exit.
"""
Path(cluster_bin_path).read_bytes()
mod = load_schema()
request = mod.ProbeConfigTagRequest.new_message()
request.name = "" # matches config_repl.c's xce_probe_config_tag exactly
request_bytes = request.to_bytes()
host, port = _split_controller(controller)
reply_bytes = _send_via(request_bytes, cert_dir)(host, port)
with mod.ProbeConfigTagReply.from_bytes(reply_bytes) as reply:
current_generation = reply.configTag
return DryRunResult(
current_generation=current_generation,
would_be_generation=current_generation + 1,
accepted=True,
reason="",
)

270
src/xceadmin/tests/test_config_push_mock.py Normal file
View file

@ -0,0 +1,270 @@
"""Mock-controller tests for ``xceadmin.push`` (design.md, ``xceadmin.push``;
Requirements 2.1, 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, 3.1, 3.2, 3.3, 3.4, 6.1,
6.2, 6.3, 7.1, 7.2, 7.3).
``push_config``'s real controller counterpart is blocked on a separate,
already-documented controller-side dispatch-gate defect
(``ha_dispatch_gate()``/``cluster.c``'s inverted primary check --
spec.json ``blocked_on``), and ``probe_config``'s ``OPCode_probeConfigTag``
has no server dispatch case at all (``research.md``, "ProbeConfigTag
liveness"). Both are therefore tested against a real local TLS mock
server built from the actual bundled schema -- not a live cluster --
exactly as design.md's ``xceadmin.push`` Boundary Commitments anticipate.
"""
from __future__ import annotations
import ssl
import socket
import threading
from pathlib import Path
import pytest
from xcecommon import wire
from xcecommon.errors import ReplyDecodeError
from xcecommon.schema import load_schema
from xceadmin import push
REPO_HA_DATA_DIR = Path(__file__).resolve().parents[3] / "ha" / "data"
CERT_FILE = REPO_HA_DATA_DIR / "xce.crt"
KEY_FILE = REPO_HA_DATA_DIR / "xce.key"
CERT_DIR = str(REPO_HA_DATA_DIR)
pytestmark = pytest.mark.skipif(
not (CERT_FILE.is_file() and KEY_FILE.is_file()),
reason="ha/data/xce.crt or ha/data/xce.key not present",
)
def _server_context() -> ssl.SSLContext:
context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
context.load_cert_chain(certfile=str(CERT_FILE), keyfile=str(KEY_FILE))
return context
class _MockController:
"""A one-shot TLS server: accepts one connection, hands the raw
accepted TLS socket to ``handler``, then closes.
"""
def __init__(self, handler):
self._handler = handler
self._listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
self._listener.bind(("127.0.0.1", 0))
self._listener.listen(1)
self.port = self._listener.getsockname()[1]
self._thread = threading.Thread(target=self._run, daemon=True)
self._thread.start()
def _run(self) -> None:
try:
raw_conn, _ = self._listener.accept()
except OSError:
return
try:
tls_conn = _server_context().wrap_socket(raw_conn, server_side=True)
except (ssl.SSLError, OSError):
raw_conn.close()
return
try:
self._handler(tls_conn)
except OSError:
pass
finally:
tls_conn.close()
def close(self) -> None:
self._listener.close()
self._thread.join(timeout=5)
@pytest.fixture
def mock_controller():
servers: list[_MockController] = []
def _make(handler) -> _MockController:
srv = _MockController(handler)
servers.append(srv)
return srv
yield _make
for srv in servers:
srv.close()
@pytest.fixture
def cluster_bin(tmp_path):
path = tmp_path / "cluster.bin"
path.write_bytes(b"fake cluster config binary contents")
return str(path)
# ---------------------------------------------------------------------------
# push_config(): happy path, redirect-following, error mapping
# ---------------------------------------------------------------------------
def test_push_config_successful_commit(mock_controller, cluster_bin):
mod = load_schema()
received = {}
def _handler(conn):
request_bytes = wire.recv_frame(conn)
received["request_bytes"] = request_bytes
with mod.Request.from_bytes(request_bytes) as req:
received["opcode"] = req.opcode
received["sha256"] = bytes(req.body.upgradeconfigrequestmsg.sha256)
received["bytes"] = bytes(req.body.upgradeconfigrequestmsg.bytes)
reply = mod.RequestReply.new_message()
reply.status = 0
upgrade_reply = reply.data.init("upgradeconfigreplymsg")
upgrade_reply.versionTag = 7
upgrade_reply.ackedCount = 3
upgrade_reply.error = ""
wire.send_frame(conn, reply.to_bytes())
srv = mock_controller(_handler)
result = push.push_config(cluster_bin, f"127.0.0.1:{srv.port}", CERT_DIR)
assert result.version_tag == 7
assert result.acked_count == 3
assert result.error == ""
assert received["opcode"] == "upgradeConfig"
assert received["bytes"] == Path(cluster_bin).read_bytes()
import hashlib
assert received["sha256"] == hashlib.sha256(Path(cluster_bin).read_bytes()).digest()
def test_push_config_follows_one_redirect_to_primary(mock_controller, cluster_bin):
mod = load_schema()
def _standby_handler(conn):
wire.recv_frame(conn)
reply = mod.RequestReply.new_message()
reply.status = 0
reply.redirectHost = "127.0.0.1"
# port filled in once the primary server exists, see below
reply.redirectPort = _standby_handler.primary_port
reply.data.nodata = None
wire.send_frame(conn, reply.to_bytes())
def _primary_handler(conn):
wire.recv_frame(conn)
reply = mod.RequestReply.new_message()
reply.status = 0
upgrade_reply = reply.data.init("upgradeconfigreplymsg")
upgrade_reply.versionTag = 9
upgrade_reply.ackedCount = 2
upgrade_reply.error = ""
wire.send_frame(conn, reply.to_bytes())
primary_srv = mock_controller(_primary_handler)
_standby_handler.primary_port = primary_srv.port
standby_srv = mock_controller(_standby_handler)
result = push.push_config(cluster_bin, f"127.0.0.1:{standby_srv.port}", CERT_DIR)
assert result.version_tag == 9
assert result.acked_count == 2
def test_push_config_non_zero_status_reports_error_not_exception(mock_controller, cluster_bin):
mod = load_schema()
def _handler(conn):
wire.recv_frame(conn)
reply = mod.RequestReply.new_message()
reply.status = 1
reply.data.debugmsg = "simulated failure"
wire.send_frame(conn, reply.to_bytes())
srv = mock_controller(_handler)
result = push.push_config(cluster_bin, f"127.0.0.1:{srv.port}", CERT_DIR)
assert result.acked_count == 0
assert "status 1" in result.error
def test_push_config_wrong_reply_variant_raises_reply_decode_error(mock_controller, cluster_bin):
mod = load_schema()
def _handler(conn):
wire.recv_frame(conn)
reply = mod.RequestReply.new_message()
reply.status = 0
reply.data.nodata = None # not upgradeconfigreplymsg
wire.send_frame(conn, reply.to_bytes())
srv = mock_controller(_handler)
with pytest.raises(ReplyDecodeError):
push.push_config(cluster_bin, f"127.0.0.1:{srv.port}", CERT_DIR)
def test_push_config_default_port_used_when_omitted():
host, port = push._split_controller("somehost")
assert host == "somehost"
assert port == 9991
def test_push_config_explicit_port_parsed():
host, port = push._split_controller("somehost:1234")
assert host == "somehost"
assert port == 1234
# ---------------------------------------------------------------------------
# probe_config(): dry-run against the mock controller
# ---------------------------------------------------------------------------
def test_probe_config_reports_current_and_would_be_generation(mock_controller, cluster_bin):
mod = load_schema()
received = {}
def _handler(conn):
request_bytes = wire.recv_frame(conn)
received["request_bytes"] = request_bytes
with mod.ProbeConfigTagRequest.from_bytes(request_bytes) as req:
received["name"] = req.name
reply = mod.ProbeConfigTagReply.new_message()
reply.configTag = 41
wire.send_frame(conn, reply.to_bytes())
srv = mock_controller(_handler)
result = push.probe_config(cluster_bin, f"127.0.0.1:{srv.port}", CERT_DIR)
assert result.current_generation == 41
assert result.would_be_generation == 42
assert result.accepted is True
assert result.reason == ""
assert received["name"] == "" # matches config_repl.c's xce_probe_config_tag exactly
def test_probe_config_zero_generation_still_accepted(mock_controller, cluster_bin):
"""A fresh cluster with no config committed yet (configTag == 0) is
still a successful probe -- there is no rejection signal on this
wire shape (module docstring); the caller decides significance.
"""
mod = load_schema()
def _handler(conn):
wire.recv_frame(conn)
reply = mod.ProbeConfigTagReply.new_message()
reply.configTag = 0
wire.send_frame(conn, reply.to_bytes())
srv = mock_controller(_handler)
result = push.probe_config(cluster_bin, f"127.0.0.1:{srv.port}", CERT_DIR)
assert result.current_generation == 0
assert result.would_be_generation == 1
assert result.accepted is True