rbd: Report remote peer readiness if Up and status.Unknown

Current code uses an !A && !B condition incorrectly to
test A:Up and B:status for a remote peer image.

This should be !A || !B as we require both conditions to
be in the specified state (Up: true, and status Unknown).

This is corrected by this commit, and further fixes:
- check and return ready only when a remote site is
found in the status output
- check if all peer sites are ready, if multiple are found
and return ready appropriately

Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
This commit is contained in:
Shyamsundar Ranganathan 2022-08-08 12:53:41 -05:00 committed by mergify[bot]
parent 8d7b6ee59f
commit c2280011d1
2 changed files with 143 additions and 2 deletions

View File

@ -680,6 +680,7 @@ func (rs *ReplicationServer) DemoteVolume(ctx context.Context,
// It returns true if the state of the remote cluster is up and unknown.
func checkRemoteSiteStatus(ctx context.Context, mirrorStatus *librbd.GlobalMirrorImageStatus) bool {
ready := true
found := false
for _, s := range mirrorStatus.SiteStatuses {
log.UsefulLog(
ctx,
@ -690,13 +691,16 @@ func checkRemoteSiteStatus(ctx context.Context, mirrorStatus *librbd.GlobalMirro
s.Description,
s.LastUpdate)
if s.MirrorUUID != "" {
if s.State != librbd.MirrorImageStatusStateUnknown && !s.Up {
found = true
// If ready is already "false" do not flip it based on another remote peer status
if ready && (s.State != librbd.MirrorImageStatusStateUnknown || !s.Up) {
ready = false
}
}
}
return ready
// Return readiness only if at least one remote peer status was processed
return found && ready
}
// ResyncVolume extracts the RBD volume information from the volumeID, If the

View File

@ -295,3 +295,140 @@ func TestCheckVolumeResyncStatus(t *testing.T) {
})
}
}
func TestCheckRemoteSiteStatus(t *testing.T) {
t.Parallel()
tests := []struct {
name string
args librbd.GlobalMirrorImageStatus
wantReady bool
}{
{
name: "Test a single peer in sync",
args: librbd.GlobalMirrorImageStatus{
SiteStatuses: []librbd.SiteMirrorImageStatus{
{
MirrorUUID: "remote",
State: librbd.MirrorImageStatusStateUnknown,
Up: true,
},
},
},
wantReady: true,
},
{
name: "Test a single peer in sync, including a local instance",
args: librbd.GlobalMirrorImageStatus{
SiteStatuses: []librbd.SiteMirrorImageStatus{
{
MirrorUUID: "remote",
State: librbd.MirrorImageStatusStateUnknown,
Up: true,
},
{
MirrorUUID: "",
State: librbd.MirrorImageStatusStateUnknown,
Up: true,
},
},
},
wantReady: true,
},
{
name: "Test a multiple peers in sync",
args: librbd.GlobalMirrorImageStatus{
SiteStatuses: []librbd.SiteMirrorImageStatus{
{
MirrorUUID: "remote1",
State: librbd.MirrorImageStatusStateUnknown,
Up: true,
},
{
MirrorUUID: "remote2",
State: librbd.MirrorImageStatusStateUnknown,
Up: true,
},
},
},
wantReady: true,
},
{
name: "Test no remote peers",
args: librbd.GlobalMirrorImageStatus{
SiteStatuses: []librbd.SiteMirrorImageStatus{},
},
wantReady: false,
},
{
name: "Test single peer not in sync",
args: librbd.GlobalMirrorImageStatus{
SiteStatuses: []librbd.SiteMirrorImageStatus{
{
MirrorUUID: "remote",
State: librbd.MirrorImageStatusStateReplaying,
Up: true,
},
},
},
wantReady: false,
},
{
name: "Test single peer not up",
args: librbd.GlobalMirrorImageStatus{
SiteStatuses: []librbd.SiteMirrorImageStatus{
{
MirrorUUID: "remote",
State: librbd.MirrorImageStatusStateUnknown,
Up: false,
},
},
},
wantReady: false,
},
{
name: "Test multiple peers, when first peer is not in sync",
args: librbd.GlobalMirrorImageStatus{
SiteStatuses: []librbd.SiteMirrorImageStatus{
{
MirrorUUID: "remote1",
State: librbd.MirrorImageStatusStateStoppingReplay,
Up: true,
},
{
MirrorUUID: "remote2",
State: librbd.MirrorImageStatusStateUnknown,
Up: true,
},
},
},
wantReady: false,
},
{
name: "Test multiple peers, when second peer is not up",
args: librbd.GlobalMirrorImageStatus{
SiteStatuses: []librbd.SiteMirrorImageStatus{
{
MirrorUUID: "remote1",
State: librbd.MirrorImageStatusStateUnknown,
Up: true,
},
{
MirrorUUID: "remote2",
State: librbd.MirrorImageStatusStateUnknown,
Up: false,
},
},
},
wantReady: false,
},
}
for _, tt := range tests {
ts := tt
t.Run(ts.name, func(t *testing.T) {
t.Parallel()
if ready := checkRemoteSiteStatus(context.TODO(), &ts.args); ready != ts.wantReady {
t.Errorf("checkRemoteSiteStatus() ready = %v, expect ready = %v", ready, ts.wantReady)
}
})
}
}