Skip to content

DAOS-17832 control: Add dmg system rebuild start|stop cmds #16664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daltonbohning merged 5 commits into from
Sep 24, 2025
Merged

DAOS-17832 control: Add dmg system rebuild start|stop cmds #16664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9558126
DAOS-17832 control: Add dmg system rebuild start|stop cmds
tanabarr Sep 12, 2025
1fe4cd9
add mgmt_system.go unit tests for pool rebuild manage
tanabarr Sep 17, 2025
1589977
update ui output
tanabarr Sep 17, 2025
63a8007
Merge remote-tracking branch 'origin/master' into tanabarr/control-sy…
tanabarr Sep 17, 2025
cbc17b3
Merge branch 'master' into tanabarr/control-sysrbld-startstop
kjacque Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/control/cmd/dmg/command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,14 @@ func (bci *bridgeConnInvoker) InvokeUnaryRPC(ctx context.Context, uReq control.U
resp = control.MockMSResponse("", nil, &mgmtpb.PoolRanksResp{})
case *control.PoolExtendReq:
resp = control.MockMSResponse("", nil, &mgmtpb.PoolExtendResp{})
case *control.PoolRebuildReq:
if req.Op == control.PoolRebuildOpCodeStart {
case *control.PoolRebuildManageReq:
if req.OpCode == control.PoolRebuildOpCodeStart {
resp = control.MockMSResponse("", nil, &mgmtpb.DaosResp{})
} else if req.Op == control.PoolRebuildOpCodeStop {
} else if req.OpCode == control.PoolRebuildOpCodeStop {
resp = control.MockMSResponse("", nil, &mgmtpb.DaosResp{})
}
case *control.SystemRebuildManageReq:
resp = control.MockMSResponse("", nil, &mgmtpb.SystemRebuildManageResp{})
case *control.SystemCheckEnableReq:
resp = control.MockMSResponse("", nil, &mgmtpb.DaosResp{})
case *control.SystemCheckDisableReq:
Expand Down
12 changes: 6 additions & 6 deletions src/control/cmd/dmg/pool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1067,13 +1067,13 @@ type poolRebuildOpCmd struct {
}

func (cmd *poolRebuildOpCmd) execute(opCode control.PoolRebuildOpCode, force bool) (errOut error) {
req := &control.PoolRebuildReq{
ID: cmd.PoolID().String(),
Op: opCode,
Force: force,
req := &control.PoolRebuildManageReq{
ID: cmd.PoolID().String(),
OpCode: opCode,
Force: force,
}

err := control.PoolRebuild(cmd.MustLogCtx(), cmd.ctlInvoker, req)
err := control.PoolRebuildManage(cmd.MustLogCtx(), cmd.ctlInvoker, req)
if err != nil || cmd.JSONOutputEnabled() {
if err != nil {
cmd.ctlInvoker.Debug(err.Error())
Expand All @@ -1082,7 +1082,7 @@ func (cmd *poolRebuildOpCmd) execute(opCode control.PoolRebuildOpCode, force boo
return err
}

msg := fmt.Sprintf("Pool-rebuild %s request succeeded", opCode.String())
msg := fmt.Sprintf("Pool-rebuild %s request succeeded", opCode)
cmd.ctlInvoker.Debug(msg)
cmd.Info(msg)

Expand Down
24 changes: 12 additions & 12 deletions src/control/cmd/dmg/pool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1213,9 +1213,9 @@ func TestPoolCommands(t *testing.T) {
"Start interactive rebuild on pool with ID",
"pool rebuild start 031bcaf8-f0f5-42ef-b3c5-ee048676dceb",
strings.Join([]string{
printRequest(t, &control.PoolRebuildReq{
ID: "031bcaf8-f0f5-42ef-b3c5-ee048676dceb",
Op: control.PoolRebuildOpCodeStart,
printRequest(t, &control.PoolRebuildManageReq{
ID: "031bcaf8-f0f5-42ef-b3c5-ee048676dceb",
OpCode: control.PoolRebuildOpCodeStart,
}),
}, " "),
nil,
Expand All @@ -1224,9 +1224,9 @@ func TestPoolCommands(t *testing.T) {
"Start interactive rebuild on pool with label",
"pool rebuild start test_label",
strings.Join([]string{
printRequest(t, &control.PoolRebuildReq{
ID: "test_label",
Op: control.PoolRebuildOpCodeStart,
printRequest(t, &control.PoolRebuildManageReq{
ID: "test_label",
OpCode: control.PoolRebuildOpCodeStart,
}),
}, " "),
nil,
Expand All @@ -1247,9 +1247,9 @@ func TestPoolCommands(t *testing.T) {
"Stop interactive rebuild on pool with ID",
"pool rebuild stop 031bcaf8-f0f5-42ef-b3c5-ee048676dceb",
strings.Join([]string{
printRequest(t, &control.PoolRebuildReq{
ID: "031bcaf8-f0f5-42ef-b3c5-ee048676dceb",
Op: control.PoolRebuildOpCodeStop,
printRequest(t, &control.PoolRebuildManageReq{
ID: "031bcaf8-f0f5-42ef-b3c5-ee048676dceb",
OpCode: control.PoolRebuildOpCodeStop,
}),
}, " "),
nil,
Expand All @@ -1258,9 +1258,9 @@ func TestPoolCommands(t *testing.T) {
"Stop interactive rebuild on pool with label",
"pool rebuild stop test_label",
strings.Join([]string{
printRequest(t, &control.PoolRebuildReq{
ID: "test_label",
Op: control.PoolRebuildOpCodeStop,
printRequest(t, &control.PoolRebuildManageReq{
ID: "test_label",
OpCode: control.PoolRebuildOpCodeStop,
}),
}, " "),
nil,
Expand Down
80 changes: 78 additions & 2 deletions src/control/cmd/dmg/system.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/pkg/errors"

"github.com/daos-stack/daos/src/control/cmd/dmg/pretty"
"github.com/daos-stack/daos/src/control/common"
"github.com/daos-stack/daos/src/control/common/cmdutil"
"github.com/daos-stack/daos/src/control/lib/control"
"github.com/daos-stack/daos/src/control/lib/daos"
Expand Down Expand Up @@ -44,6 +45,7 @@ type SystemCmd struct {
DelAttr systemDelAttrCmd `command:"del-attr" description:"Delete system attributes"`
SetProp systemSetPropCmd `command:"set-prop" description:"Set system properties"`
GetProp systemGetPropCmd `command:"get-prop" description:"Get system properties"`
Rebuild systemRebuildCmd `command:"rebuild" description:"Interactive rebuild commands"`
}

type baseCtlCmd struct {
Expand Down Expand Up @@ -494,7 +496,7 @@ func (cmd *systemGetAttrCmd) Execute(_ []string) error {

var bld strings.Builder
prettyPrintAttrs(&bld, resp.Attributes)
cmd.Infof("%s", bld.String())
cmd.Infof("%s", bld)

return nil
}
Expand Down Expand Up @@ -688,7 +690,81 @@ func (cmd *systemGetPropCmd) Execute(_ []string) error {

var bld strings.Builder
prettyPrintSysProps(&bld, resp.Properties)
cmd.Infof("%s", bld.String())
cmd.Infof("%s", bld)

return nil
}

// systemRebuildCmd represents the system rebuild subcommand.
type systemRebuildCmd struct {
Start systemRebuildStartCmd `command:"start" description:"System rebuild start requests submitted to pools"`
Stop systemRebuildStopCmd `command:"stop" description:"System rebuild stop requests submitted to pools"`
}

type systemRebuildOpCmd struct {
baseCtlCmd
Verbose bool `short:"v" long:"verbose" description:"Print pool identifiers"`
}

func (cmd *systemRebuildOpCmd) execute(opCode control.PoolRebuildOpCode, force bool) (errOut error) {
defer func() {
errOut = errors.Wrapf(errOut, "system rebuild %s failed", opCode)
}()

if cmd.config == nil {
return errors.New("no configuration loaded")
}

req := &control.SystemRebuildManageReq{
OpCode: opCode,
Force: force,
}

resp, err := control.SystemRebuildManage(cmd.MustLogCtx(), cmd.ctlInvoker, req)
if err != nil {
return err // control api returned an error, disregard response
}

if cmd.JSONOutputEnabled() {
return cmd.OutputJSON(resp, resp.Errors())
}

// Print successful results before returning any error.
respPoolsSuccess := []string{}
for _, res := range resp.Results {
if !res.Errored {
respPoolsSuccess = append(respPoolsSuccess, res.ID)
}
}
msg := fmt.Sprintf("System-rebuild %s request succeeded", opCode)
pStr := common.Pluralise("pool", len(respPoolsSuccess))
if cmd.Verbose {
cmd.Infof("%s on %s %v", msg, pStr, respPoolsSuccess)

@kccain kccain Sep 18, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor suggestion (change only if need to change something else in the PR I suppose): Suggest still printing out the number of pools in addition to the list of pool identifiers. Useful to the admin reading the output if the system has many 10s of pools and doesn't want to manually count or otherwise process the command output to derive the number.

Suggested change
cmd.Infof("%s on %s %v", msg, pStr, respPoolsSuccess)
cmd.Infof("%s on %d %s %v", msg, len(respPoolsSuccess), pStr, respPoolsSuccess)

@tanabarr tanabarr Sep 18, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will address either on repush or on subsequent PR

@tanabarr tanabarr Sep 25, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fix applied in #16875

} else {
cmd.Infof("%s on %d %s", msg, len(respPoolsSuccess), pStr)
}

if resp.Errors() != nil {
return errors.Wrapf(resp.Errors(), "System-rebuild %s request failed",
opCode)
}

return nil
}

type systemRebuildStartCmd struct {
systemRebuildOpCmd
}

func (cmd *systemRebuildStartCmd) Execute(_ []string) error {
return cmd.execute(control.PoolRebuildOpCodeStart, false)
}

type systemRebuildStopCmd struct {
systemRebuildOpCmd
Force bool `short:"f" long:"force" description:"Forcibly stop interactive rebuild"`
}

func (cmd *systemRebuildStopCmd) Execute(_ []string) error {
return cmd.execute(control.PoolRebuildOpCodeStop, cmd.Force)
}
150 changes: 150 additions & 0 deletions src/control/cmd/dmg/system_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,43 @@ func TestDmg_SystemCommands(t *testing.T) {
"",
errors.New("not provided"),
},
{
"system rebuild start",
"system rebuild start",
strings.Join([]string{
printRequest(t, &control.SystemRebuildManageReq{
OpCode: control.PoolRebuildOpCodeStart,
}),
}, " "),
nil,
},
{
"system rebuild start with force",
"system rebuild start --force",
"",
errors.New("unknown flag"),
},
{
"system rebuild stop",
"system rebuild stop",
strings.Join([]string{
printRequest(t, &control.SystemRebuildManageReq{
OpCode: control.PoolRebuildOpCodeStop,
}),
}, " "),
nil,
},
{
"system rebuild stop with force",
"system rebuild stop --force",
strings.Join([]string{
printRequest(t, &control.SystemRebuildManageReq{
OpCode: control.PoolRebuildOpCodeStop,
Force: true,
}),
}, " "),
nil,
},
{
"leader query",
"system leader-query",
Expand Down Expand Up @@ -555,3 +592,116 @@ func TestDmg_systemStartCmd(t *testing.T) {
})
}
}

func TestDmg_systemRebuildOpCmd_execute(t *testing.T) {
for name, tc := range map[string]struct {
ctlCfg *control.Config
opCode control.PoolRebuildOpCode
force bool
verbose bool
resp *mgmtpb.SystemRebuildManageResp
msErr error
expErr error
expInfo string
}{
"no config": {
opCode: control.PoolRebuildOpCodeStop,
expErr: errors.New("system rebuild stop failed: no configuration loaded"),
},
"ms failures": {
ctlCfg: &control.Config{},
resp: &mgmtpb.SystemRebuildManageResp{},
msErr: errors.New("failed"),
expErr: errors.New("failed"),
},
"no pools": {
ctlCfg: &control.Config{},
opCode: control.PoolRebuildOpCodeStop,
resp: &mgmtpb.SystemRebuildManageResp{},
expInfo: "System-rebuild stop request succeeded on 0 pools",
},
"no pools; verbose": {
ctlCfg: &control.Config{},
opCode: control.PoolRebuildOpCodeStart,
verbose: true,
resp: &mgmtpb.SystemRebuildManageResp{},
expInfo: "System-rebuild start request succeeded on pools []",
},
"pool stop failed": {
ctlCfg: &control.Config{},
opCode: control.PoolRebuildOpCodeStop,
resp: &mgmtpb.SystemRebuildManageResp{
Results: []*mgmtpb.PoolRebuildManageResult{
{
Errored: true,
Msg: "failed",
Id: "foo",
OpCode: uint32(control.PoolRebuildOpCodeStop),
},
{
Errored: true,
Msg: "failed",
Id: "bar",
OpCode: uint32(control.PoolRebuildOpCodeStop),
},
{
Id: "baz",
OpCode: uint32(control.PoolRebuildOpCodeStop),
},
},
},
expErr: errors.New("failed on pool foo: failed, pool-rebuild stop failed on pool bar"),

@kccain kccain Sep 18, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

no change requested, just not understanding why the ": failed" (Msg) would not be expected to be seen for the failure associated with pool 'bar'. And not sure where "pool-rebuild stop" comes from for the pool 'bar' but does not show up for the pool 'foo' portion of the expected errors.

@tanabarr tanabarr Sep 18, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is just a subset of the Error string expected, validates that the substring is contained in the error message

expInfo: "System-rebuild stop request succeeded on 1 pool",
},
"pool start succeeded; verbose": {
ctlCfg: &control.Config{},
opCode: control.PoolRebuildOpCodeStart,
verbose: true,
resp: &mgmtpb.SystemRebuildManageResp{
Results: []*mgmtpb.PoolRebuildManageResult{
{
Id: "foo",
OpCode: uint32(control.PoolRebuildOpCodeStop),
},
{
Id: "bar",
OpCode: uint32(control.PoolRebuildOpCodeStop),
},
{
Id: "baz",
OpCode: uint32(control.PoolRebuildOpCodeStop),
},
},
},
expInfo: "System-rebuild start request succeeded on pools [foo bar baz]",
},
} {
t.Run(name, func(t *testing.T) {
log, buf := logging.NewTestLogger(t.Name())
defer test.ShowBufferOnFailure(t, buf)

mi := control.NewMockInvoker(log, &control.MockInvokerConfig{
UnaryResponse: control.MockMSResponse("10.0.0.1:10001",
tc.msErr, tc.resp),
})

rbldCmd := new(systemRebuildOpCmd)
rbldCmd.setInvoker(mi)
rbldCmd.SetLog(log)
rbldCmd.setConfig(tc.ctlCfg)
rbldCmd.Verbose = tc.verbose

gotErr := rbldCmd.execute(tc.opCode, tc.force)
test.CmpErr(t, tc.expErr, gotErr)

if !strings.Contains(buf.String(), tc.expInfo) {
t.Fatalf("expected info log output to contain %s, got %s\n",
tc.expInfo, buf.String())
}
if tc.expInfo == "" && strings.Contains(buf.String(), "INFO") {
t.Fatalf("unexpected info log output printed, got %s\n",
buf.String())
}
})
}
}
Loading
Loading