add missing unit test coverage for new system-reint code

Features: control Required-githooks: true Signed-off-by: Tom Nabarro <[email protected]>
daos-stack · Dec 18, 2024 · 5c5b8ac · 5c5b8ac
1 parent f53f8bd
commit 5c5b8ac
Show file tree

Hide file tree

Showing 3 changed files with 145 additions and 26 deletions.
diff --git a/src/control/cmd/dmg/system_test.go b/src/control/cmd/dmg/system_test.go
@@ -284,6 +284,32 @@ func TestDmg_SystemCommands(t *testing.T) {
 			"",
 			errNoRanks,
 		},
+		{
+			"system reintegrate with multiple hosts",
+			"system reintegrate --rank-hosts foo-[0,1,4]",
+			strings.Join([]string{
+				printRequest(t, withSystem(
+					withHosts(&control.SystemReintReq{}, "foo-[0-1,4]"),
+					"daos_server")),
+			}, " "),
+			nil,
+		},
+		{
+			"system reintegrate with multiple ranks",
+			"system reintegrate --ranks 0,1,4",
+			strings.Join([]string{
+				printRequest(t, withSystem(
+					withRanks(&control.SystemReintReq{}, 0, 1, 4),
+					"daos_server")),
+			}, " "),
+			nil,
+		},
+		{
+			"system reintegrate without ranks",
+			"system reint", // Verify alias is accepted.
+			"",
+			errNoRanks,
+		},
 		{
 			"system cleanup with machine name",
 			"system cleanup foo1",

diff --git a/src/control/lib/control/system.go b/src/control/lib/control/system.go
@@ -574,6 +574,23 @@ type SystemOsaResult struct {
 	Ranks  string `json:"ranks"`   // RankSet of ranks that should be operated on
 }
 
+// SystemOsaResults is an alias for a SystemOsaResult slice.
+type SystemOsaResults []*SystemOsaResult
+
+// Errors returns a single error combining all error messages associated with system-OSA results.
+// Doesn't retrieve errors from sysResponse because missing ranks or hosts will not be populated in
+// system-OSA operation response.
+func (sors SystemOsaResults) Errors() (err error) {
+	for _, r := range sors {
+		if r.Status != int32(daos.Success) {
+			err = concatErrs(err,
+				errors.Errorf("pool %s ranks %s: %s", r.PoolID, r.Ranks, r.Msg))
+		}
+	}
+
+	return
+}
+
 // SystemDrainReq contains the inputs for the system drain request.
 type SystemDrainReq struct {
 	unaryRequest
@@ -586,21 +603,12 @@ type SystemDrainReq struct {
 // in the response so decoding is not required.
 type SystemDrainResp struct {
 	sysResponse `json:"-"`
-	Results     []*SystemOsaResult `json:"results"`
+	Results     SystemOsaResults `json:"results"`
 }
 
-// Errors returns a single error combining all error messages associated with a system drain
-// response. Doesn't retrieve errors from sysResponse because missing ranks or hosts will not be
-// populated in SystemDrainResp.
-func (sdr *SystemDrainResp) Errors() (errOut error) {
-	for _, r := range sdr.Results {
-		if r.Status != int32(daos.Success) {
-			errOut = concatErrs(errOut,
-				errors.Errorf("pool %s ranks %s: %s", r.PoolID, r.Ranks, r.Msg))
-		}
-	}
-
-	return
+// Errors returns error if any of the results indicate a failure.
+func (resp *SystemDrainResp) Errors() error {
+	return resp.Results.Errors()
 }
 
 // SystemDrain will drain either hosts or ranks from all pools that they are members of. When hosts
@@ -642,21 +650,12 @@ type SystemReintReq struct {
 // in the response so decoding is not required.
 type SystemReintResp struct {
 	sysResponse `json:"-"`
-	Results     []*SystemOsaResult `json:"results"`
+	Results     SystemOsaResults `json:"results"`
 }
 
-// Errors returns a single error combining all error messages associated with a system drain
-// response. Doesn't retrieve errors from sysResponse because missing ranks or hosts will not be
-// populated in SystemReintResp.
-func (sdr *SystemReintResp) Errors() (errOut error) {
-	for _, r := range sdr.Results {
-		if r.Status != int32(daos.Success) {
-			errOut = concatErrs(errOut,
-				errors.Errorf("pool %s ranks %s: %s", r.PoolID, r.Ranks, r.Msg))
-		}
-	}
-
-	return
+// Errors returns error if any of the results indicate a failure.
+func (resp *SystemReintResp) Errors() error {
+	return resp.Results.Errors()
 }
 
 // SystemReint will reintegrate either hosts or ranks to all pools that they are members of. When hosts

diff --git a/src/control/lib/control/system_test.go b/src/control/lib/control/system_test.go
@@ -1157,6 +1157,100 @@ func TestControl_SystemDrain(t *testing.T) {
 	}
 }
 
+func TestControl_SystemReint(t *testing.T) {
+	for name, tc := range map[string]struct {
+		req        *SystemReintReq
+		uErr       error
+		uResp      *UnaryResponse
+		expErr     error
+		expResp    *SystemReintResp
+		expRespErr error
+	}{
+		"nil req": {
+			req:    nil,
+			expErr: errors.New("nil *control.SystemReintReq request"),
+		},
+		"local failure": {
+			req:    new(SystemReintReq),
+			uErr:   errors.New("local failed"),
+			expErr: errors.New("local failed"),
+		},
+		"remote failure": {
+			req:    new(SystemReintReq),
+			uResp:  MockMSResponse("host1", errors.New("remote failed"), nil),
+			expErr: errors.New("remote failed"),
+		},
+		"dual pools; single rank": {
+			req: new(SystemReintReq),
+			uResp: MockMSResponse("10.0.0.1:10001", nil, &mgmtpb.SystemReintResp{
+				Results: []*mgmtpb.SystemOsaResult{
+					{PoolId: test.MockUUID(1), Ranks: "1"},
+					{PoolId: test.MockUUID(2), Ranks: "1"},
+				},
+			}),
+			expResp: &SystemReintResp{
+				Results: []*SystemOsaResult{
+					{PoolID: test.MockUUID(1), Ranks: "1"},
+					{PoolID: test.MockUUID(2), Ranks: "1"},
+				},
+			},
+		},
+		"dual pools; single rank; with errors": {
+			req: new(SystemReintReq),
+			uResp: MockMSResponse("10.0.0.1:10001", nil, &mgmtpb.SystemReintResp{
+				Results: []*mgmtpb.SystemOsaResult{
+					{
+						PoolId: test.MockUUID(1), Ranks: "1",
+						Status: -1, Msg: "fail1",
+					},
+					{
+						PoolId: test.MockUUID(2), Ranks: "1",
+						Status: -1, Msg: "fail2",
+					},
+				},
+			}),
+			expResp: &SystemReintResp{
+				Results: []*SystemOsaResult{
+					{
+						PoolID: test.MockUUID(1), Ranks: "1",
+						Status: -1, Msg: "fail1",
+					},
+					{
+						PoolID: test.MockUUID(2), Ranks: "1",
+						Status: -1, Msg: "fail2",
+					},
+				},
+			},
+			expRespErr: errors.New("pool 00000001-0001-0001-0001-000000000001 ranks 1: fail1, pool 00000002-0002-0002-0002-000000000002 ranks 1: fail2"),
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			log, buf := logging.NewTestLogger(t.Name())
+			defer test.ShowBufferOnFailure(t, buf)
+
+			mi := NewMockInvoker(log, &MockInvokerConfig{
+				UnaryError:    tc.uErr,
+				UnaryResponse: tc.uResp,
+			})
+
+			gotResp, gotErr := SystemReint(test.Context(t), mi, tc.req)
+			test.CmpErr(t, tc.expErr, gotErr)
+			if tc.expErr != nil {
+				return
+			}
+
+			cmpOpts := []cmp.Option{
+				cmpopts.IgnoreUnexported(SystemReintResp{}),
+			}
+			if diff := cmp.Diff(tc.expResp, gotResp, cmpOpts...); diff != "" {
+				t.Fatalf("unexpected response (-want, +got):\n%s\n", diff)
+			}
+
+			test.CmpErr(t, tc.expRespErr, gotResp.Errors())
+		})
+	}
+}
+
 func TestControl_SystemCleanup(t *testing.T) {
 	for name, tc := range map[string]struct {
 		req        *SystemCleanupReq