John Agombar [Mon, 28 Apr 2025 12:29:46 +0000 (13:29 +0100)]
qa/workunits/rbd: update to mirror group snapshot tests
Updated tests:
• test_odf_failover_failback - Update retry_promote screnario to limit retries to
10000 attempts
Disabled tests:
• test_create_group_with_images_then_mirror_with_regular_snapshots scenario 1
test sometimes fails as removed regular snapshot remains on secondary cluster
after new mirror group snapshot is sync complete
Helpers:
- get_pool_image_count() $XMLSTARLET variable shouldn't be used anymore
* do not group/image demote on undo (promote failure)
* wait for demote snapshot sync to complete locally before accepting and continue
to actually perform group promote.
N Balachandran [Wed, 23 Apr 2025 15:52:34 +0000 (21:22 +0530)]
rbd-mirror: fix remove_mirror_peer_uuid
A race between the remove_mirror_peer_uuid() function in the group Replayer,
which called group_snap_set without checking if there were any
peer_uuids on the remote snap, and a snapshot cleanup on the primary
could lead to a case where the group snapshot is recreated on the
primary without the snap_order key. This causes all mirroring operations
on the group to fail on the primary as ListSnapshotsRequest fails when
it cannot find the snap_order key.
This has been fixed to update the group snapshot only if it contains the
peer_uuid.
Signed-off-by: N Balachandran <nithya.balachandran@ibm.com>
N Balachandran [Sat, 19 Apr 2025 11:26:49 +0000 (16:56 +0530)]
rbd-mirror: group replayer bootstrap changes
The group replayer BootstrapRequest has been refactored to make
it easier to handle various scenarios. It now mimics the ImageReplayer
Bootstrap sequence to a great degree.
Credit to Ilya Dryomov <idryomov@gmail.com> for the help with
integrating GroupMirrorStateUpdateRequest into the Replayer.
Signed-off-by: N Balachandran <nithya.balachandran@ibm.com>
Ramana Raja [Tue, 15 Apr 2025 19:16:34 +0000 (15:16 -0400)]
librbd/api: propagate ENOENT error in Mirror::group_get_info() API
The ENOENT error was ignored by the librbd API that retrieves mirror
group information. This manifests as a bug in the mirror group
snapshot scheduler, where the scheduler utilized this API to prevent
scheduler commands on groups that are not enabled for snapshot-based
mirroring. Since the librbd API masked the ENOENT error, the
scheduler's check for mirror group mode was compromised, resulting
in scheduler commands unexpectedly succeeding on groups that were not
enabled for snapshot-based mirroring. Therefore, allow the ENOENT error
from the API that retrieves mirror group information to surface. This
fixes the spurious mirror group mode check in the mirror snapshot
scheduler. Additionally, add a test to ensure that group snapshot
schedule commands fail on a group not enabled for mirroring.
John Agombar [Tue, 15 Apr 2025 15:37:24 +0000 (16:37 +0100)]
qa/workunits/rbd: update to rbd_mirror_group_simple
Added -p option to rbd_mirror_group_simple.sh. This can be used to print the enabled tests
and scenarios (for use by Jenkins) when producing a list of tests to run
Changed order of tests so that long running tests are run later
Updated tests:
• test_group_rename - workaround for intermittent failure
New tests:
• test_invalid_actions - test is currently disabled as it is not complete
N Balachandran [Tue, 15 Apr 2025 11:34:41 +0000 (17:04 +0530)]
rbd-mirror: fix crash in group Replayer
The rbd-mirror daemon crashed if the Replayer was destroyed while
an on-going remote group snap listing operation was in progress. The
callback would attempt to access members of the Replayer instance which
no longer existed.
Signed-off-by: N Balachandran <nithya.balachandran@ibm.com>
John Agombar [Thu, 10 Apr 2025 18:29:48 +0000 (19:29 +0100)]
qa/workunits/rbd: update to mirror group snapshot tests
Added -d <filename> option to rbd_mirror_group_simple.sh. This can be used to
save the stdout and stderr from commands run during a test+scenario into $TEMPDIR/filename.
After a successful test completion the contents of this file are deleted prior to running
the next test to prevent the file from becoming too large.
Updated tests:
• various - Added step after a resync request to wait for the group id to change before
continuing. This ensures that the group delete/recreate step has been completed and
prevents later commands from failing with group doesn't exist type errors.
• test_enable_mirroring_when_duplicate_group_exists - Added checks of the "state" and
"description" fields for the peer site in the group status output. Test is disabled
as it currently fails
• test_enable_mirroring_when_duplicate_image_exists_scenarios - simplified test to only
have a single duplicate named image. Test fails still and is disabled.
• test_remote_namespace - added steps to take new snapshot on primary after failover
and check that this syncs successfully.
• test_group_and_standalone_images_do_io - merged 2 scenarios to remove duplication
New tests:
Updated tests:
• test_demote_snap_sync - Checks that a demote snap is correctly synced to the secondary
after the deamon is restarted
rbd: don't fail "rbd group snap ls" if namespace details aren't available
Treat namespace details as an optional set of details, the same as "rbd
snap ls --all" command does. This avoids sporadic failures with ENOENT
when snapshot listing races snapshot removal which "rbd group snap ls"
command is actually more vulnerable to than "rbd snap ls --all" because
namespace details for group snapshots aren't cached internally.
Credit to N Balachandran <nibalach@redhat.com> for root causing this.
rbd-mirror: don't reset m_on_start_finish in GroupReplayer::restart()
If restart() is called while a previous start() is still in progress,
resetting m_on_start_finish before calling stop() from inside restart()
causes on_finish that was passed to start() to be leaked. One of the
ways this manifests in is a hang in InstanceReplayer::stop() on any
kind of shutdown -- due to InstanceReplayer::start_group_replayer()
keeping track of its start() invocations through C_TrackedOp
Ramana Raja [Thu, 27 Mar 2025 17:09:47 +0000 (13:09 -0400)]
rbd_mirror: add fields to mirror group status's description str
Add the following fields to the mirror group status's description string:
- last_snasphot_bytes
- last_snasphot_complete_seconds
- local_snapshot_timestamp
- remote_snapshot_timestamp
John Agombar [Thu, 20 Mar 2025 20:48:57 +0000 (20:48 +0000)]
qa/workunits/rbd: update to mirror group snapshot tests
Added new environment variable RBD_MIRROR_GLOBAL_DEBUG to allow additional debug to be turned on
after a cluster has been created.
Updated tests:
• test_group_rename - disabled some checks that are failing as the fix is deferred
• test_empty_group and test_empty_groups - added some snapshot, demote, promote steps
New tests:
• test_remote_namespace - Added new scenarios that test default and non-default
namespaces on the local and remote clusters.
rbd_mirror: fix cross namespace group snapshot mirroring
In case if group belongs to non-default namespace, the mirror daemon on
secondary is setting an empty mirror peer uuid on non primary demote snapshot.
And when previous secondary turns into primary (by an explicit promote request)
the previous i.e. non primary demote snapshot is getting unlinked as part of
the promote request as there is no mirror peer uuid set on it. Because the
previous dependent snapshot is removed, there are split-brain errors leading
to recent test failures.
This fix makesure to set the right mirror peer uuid on the non primary demote
snapshot even if the group belong to non-default namespace.
Ilya Dryomov [Sun, 30 Mar 2025 09:09:33 +0000 (11:09 +0200)]
qa/workunits/rbd: stick to RBD_MIRROR_MODE for mirror image mode
MIRROR_IMAGE_MODE was dropped in favor of RBD_MIRROR_MODE in commit 9b773eec4a8c ("qa/suites/rbd: Cleanup of MIRROR_IMAGE_MODE").
The check in mirror_group_snapshot_and_wait_for_sync_complete() is
redundant because "rbd mirror group snapshot" command would fail anyway
and mirror_group_internal() isn't used.
Ilya Dryomov [Sun, 30 Mar 2025 09:09:33 +0000 (11:09 +0200)]
qa/workunits/rbd: use xmlstarlet directly in rbd_mirror_helpers.sh
Commit e09f04669053 ("qa/workunits/rbd: mirror group functional tests")
mistakenly resurrected a redundant variable which was dropped in commit 4f309603caa3 ("qa: drop XMLSTARLET variable, use xmlstarlet directly").
Ilya Dryomov [Sun, 30 Mar 2025 09:09:33 +0000 (11:09 +0200)]
qa/workunits/rbd: stop_mirror() should send a given signal just once
Commit e09f04669053 ("qa/workunits/rbd: mirror group functional tests")
moved the kill invocation in stop_mirror() from outside of the wait loop
to inside. This is wrong because the signal should be sent just once:
rbd-mirror daemon installs a oneshot handler for SIGINT and SIGTERM and
a subsequent signal immediately kills the process.
The same commit also erroneously changed RBD_MIRROR_INSTANCES default
for all rbd_mirror_helpers.sh users instead of just group tests.
This fixes sporadic failures in "TEST: no blocklists".
librbd/api: don't mask images in group with read-only as part of image_demote()
if the images are part of a group wait until group_demote() is finally done
with GroupUnlinkPeerRequest() and then mask the images part of the group with
IMAGE_READ_ONLY_FLAG_NON_PRIMARY.
Thanks to Nithya for working along for a better fix here.
librbd/api: finalize the API's about skip-quiesce and ignore-quiesce-error flags
* leave --skip-quiesce and --ignore-quiesce-error options only on
rbd mirror group snapshot command
* drop flags argument from mirror_group_enable(), mirror_group_promote() and
mirror_group_demote() APIs, it will remain only on
mirror_group_create_snapshot() and aio_mirror_group_create_snapshot() APIs
* mirror_group_promote() and mirror_group_demote() should behave as if
RBD_SNAP_CREATE_SKIP_QUIESCE flag was passed
* mirror_group_enable() should use get_default_snap_create_flags() to get flags
-- it will be governed by rbd_default_snapshot_quiesce_mode config option
* make each of the mentioned APIs explicitly do either
a) snap_create_flags_api_to_internal(<flags passed by the user>, &snap_create_flags),
b) snap_create_flags_api_to_internal(get_default_snap_create_flags(), &snap_create_flags)
Credits to Ilya Dryomov <idryomov@gmail.com> for the above finalisation.
rbd-mirror: don't call group_snap_set for every image snap for regular group snap
It looks like we fixed avoiding of calling group_snap_set() for each image
snapshot update for mirror group snapshot, but for regular group snapshot,
it is still happening. This commit will fix it.
For 1 & 2 cases, we can simply delete the so far created group snapshot
in the respective callback handler which is basically an empty INCOMPLETE
group snapshot and let the state machine recreate it again later.
For 3 & 4 cases, we are cannot delete the created snapshot, because the
image snapshots whould have synced/syncing by now, deleting the group
snapshot will bring additional comlications (if there is a failover at
the same time). Hence setting m_retry_validate_snap flag in this case,
this would all the rescan even for regular group snapshots, if the
snapshot is yet INCOMPLETE on disk the validate_image_snaps_sync_complete()
will be called again.
For case 5, added logic to retry remove_mirror_peer_uuid() again.
rbd-mirror: fix m_stop_requested leading to a race
* if m_stop_requested is set then is_replay_interrupted return true.
* also shut_down should set m_stop_requested to false, it is instead
setting it to true this will lead to race and a possible crash accessing GR
b/w shut_down() and notify_group_listener_stop()
librbd/api: fail group promote when there is no previous snapshot
If the group enable time initial snapshot didn't sync to the secondary and
is in incomplete state, but then there happens a force promote on secondary,
there is no previous snapshot for that force promote to rollback to.
John Agombar [Thu, 20 Mar 2025 20:48:57 +0000 (20:48 +0000)]
qa/workunits/rbd: update to mirror group snapshot tests
Updated status() helper function to dump contents of stderr and stdout for last command
New helper functions to check for image snaps existence
Added new environment variable RBD_MIRROR_HIDE_BASH_DEBUGGING to turn off set -x output.
Previously RBD_MIRROR_SHOW_CLI_CMD was being used for this and controlling the display of cli output.
New tests:
• test_group_rename - test that a group rename is only mirrored to the remote after a mirror group
snapshot command. Also test that a group rename is not inadvertantly mirrored or undone
(test commented out as it is failing)
• test_enable_mirroring_when_duplicate_group_exists - various scenarios that check an empty group
and approaches to fixing the duplicate names on either site.
(test is commented out as it is not yet finished)
• test_enable_mirroring_when_duplicate_group_and_images_exists - builds on the previous test
but has duplicate named images too (test is commented out as it is failing)
• test_image_snapshots_with_group - test regular image snapshots along with mirror group snapshots
Enabled tests:
- test_force_promote scenarios 1,2,3 and 5 pass
rbd-mirror: group-replayer check for remote demote state
I'm seeing a possibility for 3 situations here for resync flagging and
rbd-mirror daemon working on it:
1. No Demotion on Primary while/just-before resync is play'ed
there is no demote snap along side resync, we can cancel syncing other
snaps, and start resync as soon as resync is flagged, because there is
no point syncing snaps that we are anyway going to delete the whole
group and resync fresh.
2. first Demote + immediately Resync
demote came first, this mean before proceeding with resync, we should
always see if the last remote snap is PRIMARY (validate if the remote
is still primary, which is on point) and only proceed
3. first Resync + immediately Demote
resync Came first, so we head straight to resync.
Ilya Dryomov [Sun, 16 Mar 2025 16:23:22 +0000 (17:23 +0100)]
qa/workunits/rbd: fix looping in remove_image_retry()
remove_image_retry() is to be called on images that may still have
a watcher (i.e. considered to be open), in which case either of "rbd
snap purge" and "rbd rm" commands can fail.
This unbreaks "TEST: delete images during bootstrap".
Ilya Dryomov [Sat, 15 Mar 2025 19:42:02 +0000 (20:42 +0100)]
qa/workunits/rbd: fix positional argument expansion in create_image()
Sticking $@ into a string that is supposed to form a command isn't
right because the string would be broken apart when $@ has more than
one argument:
If the double-quoted expansion occurs within a word, the expansion of
the first parameter is joined with the beginning part of the original
word, and the expansion of the last parameter is joined with the last
part of the original word.
Resort to $* despite its shortcomings -- given run_cmd() signature it's
the only practical fixup. A wrapper such as run_cmd() should really be
variadic or take an array instead of insisting on a single string.
Ilya Dryomov [Sat, 15 Mar 2025 17:02:49 +0000 (18:02 +0100)]
qa/workunits/rbd: fix looping in wait_for_snapshot_sync_complete()
get_primary_snap_id_for_newest_mirror_snapshot_on_secondary() may be
called on a freshly created image where the only non-primary snapshot
is still incomplete. In this scenario it fails because a snapshot ID
can't be produced, but wait_for_snapshot_sync_complete() should keep
retrying the same as in the regular case.
This fixes sporadic failures in "TEST: add image and test replay",
"TEST: stop mirror, add image, start mirror and test replay" and other
tests that use wait_for_snapshot_sync_complete().
Ilya Dryomov [Sat, 15 Mar 2025 15:42:28 +0000 (16:42 +0100)]
qa/workunits/rbd: make wait_for_omap_keys() work on a non-existent object
wait_for_image_in_omap() may be called on a cluster with mirroring
configured but rbd-mirror daemon never started. rbd_mirror_leader
object isn't expected to exist in that case.
This unbreaks "TEST: check if removed images' OMAP are removed (with
rbd-mirror on one cluster)".
Ilya Dryomov [Sat, 15 Mar 2025 12:56:45 +0000 (13:56 +0100)]
librbd: tolerate image not existing in ImageRemoveRequest
ImageRemoveRequest may be called when the image no longer exists (or
even never existed in case of a clone image whose creation is pending
on its parent showing up on the secondary) to clean up leftover mirror
metadata. Upon failure to get the group spec on a non-existing image
header, the state machine should be advanced to remove_mirror_image().
This fixes sporadic failures in "TEST: cloned images" and "TEST: check
if removed images' OMAP are removed".
John Agombar [Thu, 13 Mar 2025 14:37:57 +0000 (14:37 +0000)]
qa/workunits/rbd: update to mirror group snapshot tests
Update run_test_secnarios function to support a non-contiguous sequence of scenario numbers
Remove assert that checked empty omap keys between tests - now just logs to testlog
New tests:
- test_odf_failover_failback - new scenario with resync request on test_odf_failover_failback
Disabled tests:
- test_force_promote all scenarios fail since test is now checking group
consistency during rollback
Issue II:
src/tools/rbd_mirror/GroupReplayer.h:178:10:
error: lambda capture 'this' is not used [-Werror,-Wunused-lambda-capture]
178 | [this](int r) {
| ^
Issue III:
src/test/rbd_mirror/test_mock_ImageSync.cc:258:16:
error: no matching constructor for initialization of 'MockImageSync'
(aka 'ImageSync<librbd::MockTestImageCtx>')
258 | return new MockImageSync(
* group_snap_set() currently is called per image snapshot ack in group
snapshot, with this change, now on it is called
1. locally, on empty group snap creation with state INCOMPLETE
2. locally when, group snap move to COMPLETE with all image snap details
3. on remote snapshot when remove peer uuid on a previous COMEPLETE snap
* group_snap_set() revert conditioning and return value around check for
"snap key already exists"
* fix user snapshot removal need two succeeding snapshots
John Agombar [Tue, 4 Mar 2025 14:24:43 +0000 (14:24 +0000)]
qa/workunits/rbd: update to mirror group snapshot tests
New tests:
- force promote test with daemon running on both clusters
- test_enable_mirroring_when_duplicate_group_exists
- test_odf_failover_failback test
- test_resync_marker test
- test_force_promote_before_initial_sync test
- scenarios in test_create_group_with_images_then_mirror_with_regular_snapshots
Renamed tests:
- test_multiple_user_snapshot_time to test_multiple_mirror_group_snapshot_unlink_time
- test_multiple_user_snapshot_whilst_stopped to test_multiple_mirror_group_snapshot_whilst_stopped
* moved the internal functions scope to private
* use m_on_start_finish to save the init time Context and use later,
instead of passing it in various functions
N Balachandran [Tue, 11 Mar 2025 10:19:16 +0000 (15:49 +0530)]
rbd-mirror: reuse the ImageReplayers in the GroupReplayer
This fix will start image replayers even if the group replayer
is primary so as to have the correctmirror pool status.
The group replayer will also attempt to reuse the image replayers where
possible on restart.
Signed-off-by: N Balachandran <nithya.balachandran@ibm.com>
N Balachandran [Fri, 7 Mar 2025 07:26:35 +0000 (12:56 +0530)]
rbd-mirror: fix image map notifications for groups
The Group replayer Bootstrap now sends mirroring notifications
when creating or deleting the local group. The ImageMap will only
send the acquire_group notifications once for each group.
Signed-off-by: N Balachandran <nithya.balachandran@ibm.com>
Ramana Raja [Wed, 5 Mar 2025 19:29:16 +0000 (14:29 -0500)]
librbd/mirror: cleanup redundant parameters in CreatePrimaryRequest and
... CreateNonPrimaryRequest constructors. The objects can figure out
the image's group ID and group pool ID from the group_spec stored in
their image_ctx data member. No need to pass in group ID and
group pool ID into the constructors.
Ramana Raja [Tue, 4 Mar 2025 17:43:19 +0000 (12:43 -0500)]
librbd/mirror: change naming format of member image snap
... of primary and non-primary mirror group snaps.
Set the naming format of member image snap of a mirror group snap to be,
mirror.primary.<global_image_id>.<global_group_id>.<group_pool_id>_<group_id>_<group_snap_id>,
or
mirror.non_primary.<global_image_id>.<global_group_id>.<group_pool_id>_<group_id>_<group_snap_id>
Ramana Raja [Fri, 28 Feb 2025 21:49:27 +0000 (16:49 -0500)]
librbd/api: set `image_snap_name` as empty string for mirror gp snap
The member image snapshots of a mirror group snap do not share a
common name unlike those of a user group snap. So set the
`image_snap_name` to an empty string.
The following steps leaves stale group on seondary left undeleted,
1. Create and mirror enable a group with 2 images.
2. Let it sync to the secondary
3. Demote on the primary and promote on the secondary
4. Wait until it starts replaying on the original primary
5. Delete the group on the new primary
Credits to Nithya Balachandran for highlighting the issue with detailed steps.
Following issues are fixed:
* Allow to flag resync, but if remote is not primary do not resync or
even delete the local group.
- Wait for remote to turn primary, if it turns primary, then continue
to resync.
- Just in case if the same site is made primary right after issuing
resync, then clear that flag immediatly.
* Revert some old code in PoolWatcher, unintentional edits/changes.
* Do not send MIRROR_GROUP_STATE_DISABLED notification from group_resync
API, this will lead to release_group(). Credits to Nithya Balachandran
for pointing about this notification deatils.
rbd_mirror: avoid rescans in busy loop to detect new snapshots
Instead move the state to STATE_IDLE once the snapshot limits cannot be met
and move back to STATE_REPLAYING on a call from group_replayer to
set_remote_snap_id_end_limit()
Issue I:
As part of the remove_local_mirror_group if local mirror group global_group_id
doesn't match with GroupReplayer instance (m_global_group_id), then
remove_local_group()
In a case where the daemon is down then group is disabled then removed/added
images then re-enabled groups and then brought the daemon back to life,
the Groupreplayer instances belonging to same group name will mess
leading to path of create_local_mirror_group(), which is wrong.
Issue II:
Also, in cases where local mirror group global_group_id doesn't match with
GroupReplayer instance, if there are ENOENT errors in the bootstrapping
then retry the bootstrapping.
With out this fix, this will lead to group_replayer destroy of a valid instance.
Ramana Raja [Thu, 20 Feb 2025 00:09:32 +0000 (19:09 -0500)]
librbd/api: disallow mirror image operations on a group's member image
Disallow the following mirror image APIs when called directly on an
image that is member of a group:
- mirror image demote
- mirror image disable
- mirror image enable
- mirror image promote
- mirror image resync
- mirror image snapshot
Only allow mirror operations on a group's member image via the mirror
group APIs.
John Agombar [Thu, 13 Feb 2025 17:20:01 +0000 (17:20 +0000)]
qa/workunits/rbd: add new tests and improve existing
Change admin socket mirror group status checks to query status on normal CLI too
Disable test_stopped_daemon test whichhas intermittent failures.
Remove sleep 5 which is no longer needed in RBD mirror group tests
Fix test_image_replay_state() helper function to work without SHOW_CLI_CMD env variable set
rbd_mirror_group_group.sh:
+ testlog "TEST: add a large image to group and test replay"
Also this fix replace the set_finished(), which was removed in the previous
commit, which will cause a regression in the GroupReplayer destroy code path.
N Balachandran [Fri, 21 Feb 2025 06:17:19 +0000 (11:47 +0530)]
rbd-mirror: fixes multiple issues in the group replayer
The commit includes the following:
- Fixed crashes in the start/stop in GroupReplayer
- Fixed crashes in the shut_down sequence in group_replayer::Replayer
- ImageMap will now send release_group notifications for non-empty
groups.
- InstanceReplayer no longer checks if the GroupReplayer needs to be
restarted. The GroupReplayer will stop itself if it determines that it
needs to be restarted.
Signed-off-by: N Balachandran <nithya.balachandran@ibm.com>
Ramana Raja [Tue, 4 Feb 2025 00:55:08 +0000 (19:55 -0500)]
librbd: remove mirror APIs that change mirror group membership
Remove mirror APIs, group_image_add() and group_image_remove() that
are never called as we don't allow adding/removing images to/from a
mirrored group.
rbd-mirror: do not move the images to trash while the disabling is in progress
Images cannot be moved to trash if the state is disabling because its a
transient state where some of the images might have got the oportunity to
disable and some of them part of the group might still be enabled
waiting for the oportunity while a group disable is in progress.
So we wait until the state DISABLING moves to next state, and see if there are
any stale image to move into a trash queue later.