Skip to content

feat: add radosgw-admin multisite read wrappers - #809

Open
johnramsden wants to merge 9 commits into
canonical:mainfrom
johnramsden:feat/rgw-multisite-read-wrappers
Open

feat: add radosgw-admin multisite read wrappers#809
johnramsden wants to merge 9 commits into
canonical:mainfrom
johnramsden:feat/rgw-multisite-read-wrappers

Conversation

@johnramsden

Copy link
Copy Markdown
Member

Description

Add plumbing for RGW multisite replication

  • 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.

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

  • New feature (non-breaking change which adds functionality)

How has this been tested?

Mocked unit tests exercise new functions

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
  • added tests to verify effectiveness of this change

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 sabaini left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for this @johnramsden left a few nits / questions

Comment thread microceph/ceph/test_assets/rgw_zone_get.json
Comment thread microceph/ceph/rgw_multisite.go
Comment thread microceph/ceph/rgw_multisite.go
Comment thread microceph/ceph/rgw_multisite.go
Comment thread microceph/ceph/rgw_multisite.go
@johnramsden
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
johnramsden force-pushed the feat/rgw-multisite-read-wrappers branch 2 times, most recently from 9f0a4d0 to 4543bd9 Compare August 14, 2026 01:00
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
johnramsden force-pushed the feat/rgw-multisite-read-wrappers branch from 4543bd9 to 2f078db Compare August 14, 2026 01:15
@johnramsden
johnramsden marked this pull request as ready for review August 14, 2026 06:28
@johnramsden
johnramsden requested a review from sabaini August 14, 2026 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants