feat: add radosgw-admin multisite read wrappers - #809
Open
johnramsden wants to merge 9 commits into
Open
Conversation
Add the inert read-only plumbing for RGW multisite replication (CE062): a radosgw-admin exec wrapper with a remote-cluster variant and typed, JSON-backed wrappers for realm get, zonegroup get, zone get, metadata sync status, per-source data sync status, and the mdlog/datalog head markers. Catch-up verdicts are computed deterministically from local sync markers versus the peer's log heads (readable over RADOS through an imported remote), using the same per-shard comparison rule the radosgw-admin sync status text applies internally; that text is not parsed at all since the command ignores --format json on both squid and tentacle. Failing commands yield zero-value results with a nil error, mirroring the RBD wrapper contract, so the upcoming replication handler can treat an unconfigured gateway as ordinary disabled state. Fixtures are captured from a live two-site squid deployment, covering master, secondary and unconfigured views. Nothing calls this code yet; the replication handler arrives in a follow-up. Assisted-by: claude-code:claude-fable-5 Signed-off-by: John Ramsden <john.ramsden@canonical.com>
sabaini
requested changes
Aug 10, 2026
sabaini
left a comment
Collaborator
There was a problem hiding this comment.
Hey, thanks for this @johnramsden left a few nits / questions
johnramsden
marked this pull request as draft
August 11, 2026 01:20
johnramsden
added a commit
to johnramsden/microceph
that referenced
this pull request
Aug 11, 2026
ComputeRgwMetadataSyncVerdict and ComputeRgwDataSyncVerdict now derive FullSyncShards as NumShards minus the count confirmed incremental, matching upstream radosgw-admin's total_behind formula (shards_behind.size() + (num_shards - num_inc)) instead of only counting shards explicitly present with a non-incremental state. A shard missing from the response entirely - not just one explicitly reported full sync - now blocks CaughtUp, instead of being silently ignored. Addresses Peter Sabaini's PR canonical#809 review comment: "the worry is that if partial/initializing shards could be reported as caught up." Assisted-by: claude-code:claude-sonnet-5 Signed-off-by: John Ramsden <john.ramsden@canonical.com>
johnramsden
added a commit
to johnramsden/microceph
that referenced
this pull request
Aug 11, 2026
Address Peter Sabaini's PR canonical#809 review comments: "suggest to check invariants before returning as a safety, later on we do indexing on keys e.g. eg. NumShards >= 0, keys between 0 and NumShards. if NumShards==0 i think there must not be markers?" Signed-off-by: John Ramsden <john.ramsden@canonical.com>
johnramsden
force-pushed
the
feat/rgw-multisite-read-wrappers
branch
2 times, most recently
from
August 14, 2026 01:00
9f0a4d0 to
4543bd9
Compare
The comments restated the same shape three or four sentences at a time. Each wrapper now says what it fetches and how it behaves on failure, in a line or two, with the detail kept on the function it describes rather than gathered somewhere else. No code change. Assisted-by: claude-code:claude-opus-5 Signed-off-by: John Ramsden <john.ramsden@canonical.com>
The metadata sync state arrives as a bare number, so the code compared it to a literal 1. Naming the two values says what the comparison means at the point you read it. Assisted-by: claude-code:claude-opus-5 Signed-off-by: John Ramsden <john.ramsden@canonical.com>
The verdict only looked at shards that appeared in the response. If the zone said it had 64 shards but only listed 3, the other 61 were invisible and the answer came back caught up. Now the count of shards still doing a first full copy is derived from the zone's own shard count minus the ones confirmed to be tailing the log, so a shard that goes unmentioned counts against us. This is the same arithmetic radosgw-admin uses. Assisted-by: claude-code:claude-opus-5 Signed-off-by: John Ramsden <john.ramsden@canonical.com>
Later code indexes the peer's log using the shard keys in these responses, so a response that contradicts itself is worth catching at the point we parse it rather than further along. Three checks: the shard count cannot be negative, a zone claiming zero shards cannot list markers, and every marker's key must fall inside the claimed shard range. A response that fails returns an error, the same as output we cannot parse at all. Assisted-by: claude-code:claude-opus-5 Signed-off-by: John Ramsden <john.ramsden@canonical.com>
When we ask the other site for its log heads and that command fails, the wrapper returns nothing at all. The verdict then had nothing to compare against, found no problems, and said caught up. Losing contact with the other site produced the most reassuring answer we can give. The wrappers did not need changing. They already return nil only on failure, and a real slice on success even when the log is empty. The verdict threw that away by asking how long the slice was instead of whether it existed. It now reports PeerLogUnavailable instead. radosgw-admin does the same thing: it refuses to print a verdict rather than guess. Assisted-by: claude-code:claude-opus-5 Signed-off-by: John Ramsden <john.ramsden@canonical.com>
A gateway that has never started syncing reports status "init", with zero shards and no markers. There was nothing to check, so nothing was found wrong, so we said caught up about a site that had not copied anything. A metadata master reports the same shape, because it syncs from no one. A zone now has to say it is actively syncing before we will call it caught up, which also rules out the setup state in between. For a master the honest answer is "not applicable", and only the caller knows the zone's role, so callers should skip this check for masters rather than read the false as behind. Assisted-by: claude-code:claude-opus-5 Signed-off-by: John Ramsden <john.ramsden@canonical.com>
If the peer reports fewer shards than we have markers for, the extra ones have nothing to compare against and we skip them. We still skip them, as radosgw-admin does, but until now we did it silently, so nobody could tell some shards were never checked. This logs the shard and how many the peer reported. radosgw-admin logs the same case as an error, because it means the two sites disagree about how many shards exist. The verdict is unchanged. Assisted-by: claude-code:claude-opus-5 Signed-off-by: John Ramsden <john.ramsden@canonical.com>
Ceph can mark a data shard as recovering when it is stuck retrying objects that failed to copy, and will not call a zone caught up while any shard is in that state. We do not check it, so we will. We are not fixing that here. The only way to read it is one radosgw-admin call per shard, up to 128 for a single status check, which is too slow for something people run routinely. The obvious cheap substitute does not work either: it reads a different set of shards, and its entries stay until someone trims them, so old failures would look current forever. This writes the gap down and adds a test holding today's behaviour, so changing it later has to be deliberate. Assisted-by: claude-code:claude-opus-5 Signed-off-by: John Ramsden <john.ramsden@canonical.com>
johnramsden
force-pushed
the
feat/rgw-multisite-read-wrappers
branch
from
August 14, 2026 01:15
4543bd9 to
2f078db
Compare
johnramsden
marked this pull request as ready for review
August 14, 2026 06:28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add plumbing for RGW multisite replication
Failing commands yield zero-value results with a nil error, mirroring the RBD wrapper contract, so the upcoming replication handler can treat an unconfigured gateway as ordinary disabled state.
Fixtures are captured from a live two-site squid deployment, covering master, secondary and unconfigured views.
Nothing calls this code yet; the replication handler arrives in a follow-up.
Type of change
How has this been tested?
Mocked unit tests exercise new functions
Contributor checklist
Please check that you have: