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>

internal/rbd/replicationcontrollerserver.go

@@ -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

internal/rbd/replicationcontrollerserver_test.go

@@ -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)
+			}
+		})
+	}
+}