fix(xceadmin): probe_config uses Request envelope + follow_redirects (bug-1506, bug-1507)
- probe_config() now wraps probe in Request envelope matching push_config() - Controller OPCode_probeConfigTag handler replies through xce_reply() with redirectHost/redirectPort populated - follow_redirects works identically for both push and probe flows
This commit is contained in:
parent
d70bf6a402
commit
b00e97ffe6
3 changed files with 86 additions and 54 deletions
|
|
@ -30,14 +30,13 @@ 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.
|
||||
``probe_config()`` now follows the same ``Request``-envelope +
|
||||
``follow_redirects`` + ``decode_reply`` path as ``push_config()``
|
||||
(bug-1507). The controller gained a real ``OPCode_probeConfigTag``
|
||||
handler (bug-1506) that replies through the standard ``xce_reply()``
|
||||
→ ``Request{opcode: reply, body.replymsg: RequestReply}`` envelope
|
||||
with ``redirectHost``/``redirectPort`` populated, so ``follow_redirects``
|
||||
works identically for both flows.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -131,28 +130,52 @@ def push_config(cluster_bin_path: str, controller: str) -> PushResult:
|
|||
|
||||
def probe_config(cluster_bin_path: str, controller: 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).
|
||||
anything (6.1, bug-1507). ``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.
|
||||
|
||||
Now goes through the standard ``Request`` envelope →
|
||||
``follow_redirects`` → ``decode_reply`` path (matching
|
||||
``push_config``), using the controller's real ``OPCode_probeConfigTag``
|
||||
handler (bug-1506). Before this fix the request was sent as a bare
|
||||
``ProbeConfigTagRequest`` direct struct with no controller-side
|
||||
handler at all, so ``accepted=True`` was a client-side fabrication.
|
||||
|
||||
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.
|
||||
(generation N->M)" iff the controller returns a successful reply
|
||||
(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 = mod.Request.new_message()
|
||||
request.opcode = "probeConfigTag"
|
||||
request.seq = 1
|
||||
probe = request.body.init("probeconfigtagrequestmsg")
|
||||
probe.name = ""
|
||||
request_bytes = request.to_bytes()
|
||||
|
||||
host, port = _split_controller(controller)
|
||||
reply_bytes = _send_via(request_bytes)(host, port)
|
||||
_, _, reply_bytes = follow_redirects(host, port, _send_via(request_bytes))
|
||||
|
||||
with mod.ProbeConfigTagReply.from_bytes(reply_bytes) as reply:
|
||||
current_generation = reply.configTag
|
||||
with decode_reply(reply_bytes) as reply:
|
||||
if reply.status != 0:
|
||||
return DryRunResult(
|
||||
current_generation=0,
|
||||
would_be_generation=0,
|
||||
accepted=False,
|
||||
reason=f"controller returned status {reply.status}",
|
||||
)
|
||||
if reply.data.which() != "probeconfigtagreplymsg":
|
||||
reason = (
|
||||
f"unexpected reply variant {reply.data.which()!r}, "
|
||||
f"expected probeconfigtagreplymsg"
|
||||
)
|
||||
raise ReplyDecodeError(f"invalid reply: {reason}", reason)
|
||||
probe_reply = reply.data.probeconfigtagreplymsg
|
||||
current_generation = probe_reply.configTag
|
||||
|
||||
return DryRunResult(
|
||||
current_generation=current_generation,
|
||||
|
|
|
|||
|
|
@ -271,16 +271,18 @@ def test_config_push_connection_refused_exits_nonzero_not_raw_traceback(cluster_
|
|||
assert result.output.strip() != ""
|
||||
|
||||
|
||||
def test_config_push_dry_run_reports_generations(mock_controller, cluster_bin):
|
||||
def test_config_push_dry_run_reports_generations(mock_controller, cluster_bin, wrap_reply):
|
||||
from xcecommon import wire
|
||||
|
||||
mod = load_schema()
|
||||
|
||||
def _handler(conn):
|
||||
wire.recv_frame(conn)
|
||||
reply = mod.ProbeConfigTagReply.new_message()
|
||||
reply.configTag = 4
|
||||
wire.send_frame(conn, reply.to_bytes())
|
||||
inner = mod.RequestReply.new_message()
|
||||
inner.status = 0
|
||||
probe_reply = inner.data.init("probeconfigtagreplymsg")
|
||||
probe_reply.configTag = 4
|
||||
wire.send_frame(conn, wrap_reply(inner))
|
||||
|
||||
srv = mock_controller(_handler)
|
||||
|
||||
|
|
|
|||
|
|
@ -244,19 +244,29 @@ def test_push_config_explicit_port_parsed():
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_probe_config_reports_current_and_would_be_generation(mock_controller, cluster_bin):
|
||||
def test_probe_config_reports_current_and_would_be_generation(mock_controller, cluster_bin, wrap_reply):
|
||||
"""bug-1507: probe_config now sends a Request envelope and decodes
|
||||
through follow_redirects → decode_reply, matching push_config's path.
|
||||
The mock server must unwrap the Request envelope on receive and wrap
|
||||
its reply the same way a real controller does. """
|
||||
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
|
||||
# Request{opcode: probeConfigTag, body.probeconfigtagrequestmsg: ...}
|
||||
with mod.Request.from_bytes(request_bytes) as req:
|
||||
assert req.opcode == "probeConfigTag"
|
||||
assert req.body.which() == "probeconfigtagrequestmsg"
|
||||
received["name"] = req.body.probeconfigtagrequestmsg.name
|
||||
|
||||
reply = mod.ProbeConfigTagReply.new_message()
|
||||
reply.configTag = 41
|
||||
wire.send_frame(conn, reply.to_bytes())
|
||||
# Build reply: RequestReply{status:0, data.probeconfigtagreplymsg: {configTag:41}}
|
||||
inner = mod.RequestReply.new_message()
|
||||
inner.status = 0
|
||||
probe_reply = inner.data.init("probeconfigtagreplymsg")
|
||||
probe_reply.configTag = 41
|
||||
wire.send_frame(conn, wrap_reply(inner))
|
||||
|
||||
srv = mock_controller(_handler)
|
||||
|
||||
|
|
@ -266,21 +276,22 @@ def test_probe_config_reports_current_and_would_be_generation(mock_controller, c
|
|||
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
|
||||
assert received["name"] == ""
|
||||
|
||||
|
||||
def test_probe_config_zero_generation_still_accepted(mock_controller, cluster_bin):
|
||||
def test_probe_config_zero_generation_still_accepted(mock_controller, cluster_bin, wrap_reply):
|
||||
"""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.
|
||||
"""
|
||||
still a successful probe -- configTag == 0 is a valid, real response
|
||||
from a real controller handler now (bug-1506). """
|
||||
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())
|
||||
inner = mod.RequestReply.new_message()
|
||||
inner.status = 0
|
||||
probe_reply = inner.data.init("probeconfigtagreplymsg")
|
||||
probe_reply.configTag = 0
|
||||
wire.send_frame(conn, wrap_reply(inner))
|
||||
|
||||
srv = mock_controller(_handler)
|
||||
|
||||
|
|
@ -293,27 +304,23 @@ def test_probe_config_zero_generation_still_accepted(mock_controller, cluster_bi
|
|||
|
||||
@pytest.mark.parametrize("tag_value", [0, 1, 999999])
|
||||
def test_probe_config_has_no_wire_level_rejection_signal_by_design(
|
||||
mock_controller, cluster_bin, tag_value
|
||||
mock_controller, cluster_bin, tag_value, wrap_reply
|
||||
):
|
||||
"""5.6 asks for both an accepted and a rejected dry-run outcome, but
|
||||
``ProbeConfigTagReply`` (module docstring, ``push.py`` module
|
||||
docstring) has only a bare ``configTag`` -- no field exists on the
|
||||
real wire contract to signal rejection, matching the real C
|
||||
reference client (``config_repl.c``'s ``xce_probe_config_tag``, which
|
||||
also returns only a bare tag). This test locks in that limitation
|
||||
explicitly rather than silently having no coverage for it: every
|
||||
successful round-trip, regardless of the returned ``configTag``
|
||||
value, yields ``accepted=True`` -- there is no reachable code path
|
||||
that produces ``accepted=False`` today. A real "would this SHA be
|
||||
accepted" check is future controller-side work (Requirement 6.1).
|
||||
"""
|
||||
"""ProbeConfigTagReply (controller.capnp:909) has only a bare
|
||||
``configTag`` field -- no separate accept/reject flag exists. A
|
||||
real controller handler (bug-1506) always returns status==0 here;
|
||||
every successful round-trip, regardless of the returned configTag
|
||||
value, yields ``accepted=True``. A real "would this SHA be accepted"
|
||||
check is future work (Requirement 6.1). """
|
||||
mod = load_schema()
|
||||
|
||||
def _handler(conn):
|
||||
wire.recv_frame(conn)
|
||||
reply = mod.ProbeConfigTagReply.new_message()
|
||||
reply.configTag = tag_value
|
||||
wire.send_frame(conn, reply.to_bytes())
|
||||
inner = mod.RequestReply.new_message()
|
||||
inner.status = 0
|
||||
probe_reply = inner.data.init("probeconfigtagreplymsg")
|
||||
probe_reply.configTag = tag_value
|
||||
wire.send_frame(conn, wrap_reply(inner))
|
||||
|
||||
srv = mock_controller(_handler)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue