test(robot): add the single-VM RGW placement suite

[johnramsden]

Description

The unit tests cover the decisions; this suite proves the behaviour on a real snap with a real gateway and real S3 uploads, sending PUT /1.0/placement to the control socket the way a caller would. Its 18 cases cover invalid requests changing nothing, plaintext on a chosen and on the default port, reapply without a restart, a port change, a gateway stopped by hand, TLS enable, reuse, rotation, dual listeners, an explicit downgrade, a failed reuse never serving plaintext, disable after a partial cleanup, and that an omitted rgw entry, an empty policy and DELETE leave a running gateway alone. "Same process" is checked by comparing the systemd InvocationID of snap.microceph.rgw before and after, not just that the port answers.

resources/rgw_placement.py and rgw_placement.resource hold the RGW-specific actions, such as generate_tls_pair_in_vm, which builds a real throwaway CA and pair inside the VM rather than shipping one in from the runner. microceph_api_put_from_file_in_vm sends the request body from a file rather than a command-line argument, so a generated private key never appears in a process list or a shell history. resources/rgw_probe.py's find_material_leaks then checks the API, the dqlite files and both service journals for that same generated key surfacing as raw, base64 or JSON-escaped text.

resources/placement_status.py gains pure parsers, each raising on a malformed API body instead of reading it as "nothing there", which would hide a real failure as an empty result. response_code now falls back to error_code, a pre-existing bug fixed in passing: an error body carries its status in status_code 0, so response_code returned 0 for every error before this. Every parser has a pytest counterpart in test_harness_helpers.py, run with no VM needed.

Adds rgw-placement-tests to the robot-tests matrix in .github/workflows/tests.yml. The suite needs one VM, like the other single-system suites, so it runs on the default runner.

Stack: PR 10 of 13. Merge in order. Each PR is one logical change on top of the one before it.

1. feat(db): add a table for each member's last applied RGW frontend #881 feat(db): add a table for each member's last applied RGW frontend
2. fix(services): keep error classes through the service enable path #882 fix(services): keep error classes through the service enable path
3. feat(api): describe an RGW placement as an object with explicit TLS intent #883 feat(api): describe an RGW placement as an object with explicit TLS intent
4. feat(rgw): make the member-side RGW apply safe to repeat #884 feat(rgw): make the member-side RGW apply safe to repeat
5. feat(rgw): wait until the gateway really serves the requested frontend #885 feat(rgw): wait until the gateway really serves the requested frontend
6. feat(rgw): reuse the member's own pair when TLS is asked for without one #886 feat(rgw): reuse the member's own pair when TLS is asked for without one
7. fix(rgw): make RGW disable converge when repeated or interrupted #887 fix(rgw): make RGW disable converge when repeated or interrupted
8. feat(placement): accept the RGW object in the policy and keep its secrets out #888 feat(placement): accept the RGW object in the policy and keep its secrets out
9. feat(placement): reconcile RGW across the cluster #889 feat(placement): reconcile RGW across the cluster
10. test(robot): add the single-VM RGW placement suite #890 test(robot): add the single-VM RGW placement suite (this PR)
11. test(robot): add the three-VM RGW placement suite #891 test(robot): add the three-VM RGW placement suite
12. docs: keep the RGW placement contract in HACKING.md for now #892 docs: keep the RGW placement contract in HACKING.md for now
13. fix(rgw): do not trust a pending-apply marker the operator completed by hand #893 fix(rgw): do not trust a pending-apply marker the operator completed by hand

Reviewer note: generate_tls_pair_in_vm takes an optional san_ip parameter to add an IP SAN to the generated certificate; no case in this suite passes it, so that argument is currently dead weight (verified with a repo-wide grep — no caller passes a third argument to Generate TLS Pair In VM).

Type of change

• Clean code (code refactor, test updates; does not introduce functional changes)

How has this been tested?

• Unit tests pass at this stack's cumulative tip
• Robot tests pass

Contributor checklist

Please check that you have:

• self-reviewed the code in this PR
• added code comments, particularly in less straightforward areas
• checked and added or updated relevant documentation
• checked and added or updated relevant release notes
• added tests to verify effectiveness of this change

[johnramsden]

I believe the failure:

{"type":"error","status":"","status_code":0,"operation":"","error_code":500,"error":"failed to list cluster members: failed to get cluster members: context canceled","metadata":null}

is #877, will examine

[sabaini]

clanker driven review

P1: Readiness failure makes teardown target the wrong VM
Location: tests/robot/resources/microceph_harness.py:1573–1576 (launch_outer_test_vm).

Evidence: ${OUTER_VM} is now updated only after _launch_vm_instance() returns. That helper can successfully launch the requested VM and then raise during the agent or cloud-init checks at lines 1617–1623. Previously, the variable was updated after successful launch but before those checks. The new suite requests microceph-rgw-placement-vm at rgw_placement_tests.robot:42, while the shared resource defaults ${OUTER_VM} to microceph-test-vm. Teardown resolves that variable for diagnostics and for the force-stop/delete commands in destroy_lxd_instances at lines 1732–1740.

Reproduction: A no-LXD reproduction mocked successful launch and agent readiness, then returned a cloud-init failure. Running the base and head implementations produced:

base teardown: lxc delete microceph-rgw-placement-vm --force
head teardown: lxc delete microceph-test-vm --force
The counterconditions were checked: the suite does not prebind the requested name, and the extracted launcher neither updates ${OUTER_VM} nor cleans up failures in the readiness phase.

Impact: Setup failure leaves the newly launched VM allocated and collects diagnostics from the wrong target. If an instance with the stale/default name exists, teardown force-stops and deletes that other instance. This affects existing custom-named suites as well as the new RGW suite.

Recommendation: Bind the selected outer VM name before readiness can raise, while keeping the generic guest launcher independent of ${OUTER_VM}. Add mocked agent/cloud-init-failure tests asserting that teardown targets only the requested instance.

P2: Endpoint-closed assertions accept execution failures as proof of closure
Location: tests/robot/resources/rgw_placement.py:221–246, particularly rgw_endpoint_reachable_in_vm.
Evidence: The helper returns False for every nonzero execution result. This includes failure to execute inside the guest and the harness's synthetic timeout result, not just a socket connection refusal. The closure poll immediately accepts that value, and rgw_placement.resource:34–35 treats it as a successful closure assertion. A no-LXD reproduction returning either ExecResult(124, "", "Command timed out…") or an LXD agent error made the closure helper return the exact False value required by Robot.
Impact: A transient failed probe can let port-removal or TLS-shutdown checks pass without observing the endpoint at all. Later successful calls do not establish that the old listener closed.
Recommendation: Make the guest socket probe emit an explicit result for expected socket outcomes and exit successfully. Check execution success and parse that result strictly; an LXD error, timeout, or malformed output must fail or retry as unknown, not mean “closed.” Add helper tests covering these outcomes.
P2: The disable case never exercises stopping a live TLS gateway or removing its material
Location: tests/robot/rgw-placement-tests/rgw_placement_tests.robot:372–394; preceding downgrade at lines 277–291.
Evidence: The downgrade case has already removed all TLS material and asserted its absence. The disable case then deletes radosgw.conf and stops RGW itself before submitting enabled:false. Its inactive-unit and empty-material assertions therefore begin with both conditions already satisfied.
Impact: The partial-cleanup/repeated-disable scenario is useful, but it cannot detect a disable implementation that removes database records while failing to stop a running gateway or delete its TLS files. This leaves the suite-level claim that disable closes TLS and removes material unproven. Existing Go tests, including TestDisableRGWConvergent, cover these operations with mocked service control; that is not evidence of broken production behavior, but does not replace the missing snap/systemd transition.
Recommendation: Retain the partial-cleanup case and add a disable transition from a running TLS gateway with generated material present. Assert that its listener closes, its unit becomes inactive, its observed frontend disappears, and its generated TLS files are removed.
P2: Empty-policy and DELETE cases do not verify that the gateway is untouched
Location: tests/robot/rgw-placement-tests/rgw_placement_tests.robot:343–370.
Evidence: These cases check desired state and successful fetching afterward, but neither compares the gateway's InvocationID before and after the operation. A needless restart followed by successful serving would pass, especially because the fetch helper retries. The no-service-operations/untouched contract is explicit in microceph/api/types/placement.go:150–157, and the immediately preceding omission case already demonstrates the appropriate identity assertion.
Impact: The tests establish eventual availability, not the claimed absence of service operations. They would miss a restart regression in these stand-down paths.
Recommendation: Add nonempty before/after InvocationID comparisons around both operations, retaining the state and content assertions. This is a test-quality gap, not a demonstrated production restart bug.

[sabaini]

lgtm, ty