DAOS-16304 tools: Add daos health net-test command #14980
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,12 +14,16 @@ import ( | |
|
|
||
| "github.com/daos-stack/daos/src/control/build" | ||
| "github.com/daos-stack/daos/src/control/cmd/daos/pretty" | ||
| "github.com/daos-stack/daos/src/control/common/cmdutil" | ||
| "github.com/daos-stack/daos/src/control/lib/daos" | ||
| "github.com/daos-stack/daos/src/control/lib/ranklist" | ||
| "github.com/daos-stack/daos/src/control/lib/ui" | ||
| "github.com/daos-stack/daos/src/control/logging" | ||
| ) | ||
|
|
||
| type healthCmds struct { | ||
| Check healthCheckCmd `command:"check" description:"Perform DAOS system health checks"` | ||
| NetTest netTestCmd `command:"net-test" description:"Perform non-destructive DAOS networking tests"` | ||
| } | ||
|
|
||
| type healthCheckCmd struct { | ||
|
|
@@ -68,7 +72,7 @@ func (cmd *healthCheckCmd) Execute([]string) error { | |
| return err | ||
| } | ||
|
|
||
| sysInfo, err := cmd.apiProvider.GetSystemInfo(cmd.MustLogCtx()) | ||
|
There was a problem hiding this comment. Curious why this change? seems a bit cumbersome to pass a thing from the same object (cmd) into a submethos onto itself. Should we consider adding a method to cmd instead? cmd.GetSysInfor() to take care of all other call sites? There was a problem hiding this comment. As discussed in another conversation with @kjacque, I'm experimenting with a couple of different approaches to implementing the bindings, hence the provider thing here. This change specifically was just to establish a precedent that the API calls should have a context. As for why we're getting a context from the command object and giving it to the API call, that's a whole other conversation that boils down to patterns encouraged by the use of go-flags, which is what we settled on years ago as the CLI framework for these commands. |
||
| if err != nil { | ||
| cmd.Errorf("failed to query system information: %v", err) | ||
| } | ||
|
|
@@ -166,3 +170,66 @@ func (cmd *healthCheckCmd) Execute([]string) error { | |
|
|
||
| return nil | ||
| } | ||
|
|
||
| type netTestCmd struct { | ||
| cmdutil.JSONOutputCmd | ||
| cmdutil.LogCmd | ||
| sysCmd | ||
| Ranks ui.RankSetFlag `short:"r" long:"ranks" description:"Use the specified ranks as test endpoints (default: all)"` | ||
| Tags ui.RankSetFlag `short:"t" long:"tags" description:"Use the specified tags on ranks" default:"0"` | ||
| XferSize ui.ByteSizeFlag `short:"s" long:"size" description:"Per-RPC transfer size (send/reply)"` | ||
| MaxInflight uint `short:"m" long:"max-inflight" description:"Maximum number of inflight RPCs"` | ||
| RepCount uint `short:"c" long:"rep-count" description:"Number of times to repeat the RPCs, per endpoint"` | ||
| TpsBytes bool `short:"y" long:"bytes" description:"Show throughput values in bytes per second"` | ||
| Verbose bool `short:"v" long:"verbose" description:"Display more detailed DAOS network testing information"` | ||
| } | ||
|
|
||
| func (cmd *netTestCmd) Execute(_ []string) error { | ||
| cfg := &daos.SelfTestConfig{ | ||
| GroupName: cmd.SysName, | ||
| EndpointRanks: cmd.Ranks.Ranks(), | ||
| EndpointTags: ranklist.RanksToUint32(cmd.Tags.Ranks()), | ||
| MaxInflightRPCs: cmd.MaxInflight, | ||
| Repetitions: cmd.RepCount, | ||
| } | ||
| if cmd.XferSize.IsSet() { | ||
| // If set, use that size, otherwise use the zero value. | ||
| cfg.SendSizes = []uint64{cmd.XferSize.Bytes} | ||
| cfg.ReplySizes = cfg.SendSizes | ||
| } | ||
| if err := cfg.SetDefaults(); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if !cmd.JSONOutputEnabled() { | ||
| var cfgBuf strings.Builder | ||
| if err := pretty.PrintSelfTestConfig(&cfgBuf, cfg, cmd.Verbose); err != nil { | ||
| return err | ||
| } | ||
| cmd.Info(cfgBuf.String()) | ||
| cmd.Info("Starting non-destructive network test (duration depends on performance)...\n\n") | ||
knard-intel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| res, err := RunSelfTest(cmd.MustLogCtx(), cfg) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if cmd.JSONOutputEnabled() { | ||
| return cmd.OutputJSON(struct { | ||
| Cfg *daos.SelfTestConfig `json:"configuration"` | ||
| Res []*daos.SelfTestResult `json:"results"` | ||
| }{ | ||
| Cfg: cfg, | ||
| Res: res, | ||
| }, nil) | ||
| } | ||
|
|
||
| var resBuf strings.Builder | ||
| if err := pretty.PrintSelfTestResults(&resBuf, res, cmd.Verbose, cmd.TpsBytes); err != nil { | ||
| return err | ||
| } | ||
| cmd.Info(resBuf.String()) | ||
|
|
||
| return nil | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| // | ||
| // (C) Copyright 2024 Intel Corporation. | ||
| // | ||
| // SPDX-License-Identifier: BSD-2-Clause-Patent | ||
| // | ||
|
|
||
| package main | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/dustin/go-humanize" | ||
| "github.com/google/go-cmp/cmp" | ||
| "github.com/google/go-cmp/cmp/cmpopts" | ||
|
|
||
| "github.com/daos-stack/daos/src/control/common/cmdutil" | ||
| "github.com/daos-stack/daos/src/control/common/test" | ||
| "github.com/daos-stack/daos/src/control/lib/daos" | ||
| "github.com/daos-stack/daos/src/control/lib/ranklist" | ||
| "github.com/daos-stack/daos/src/control/lib/ui" | ||
| "github.com/daos-stack/daos/src/control/logging" | ||
| ) | ||
|
|
||
| func RunSelfTest(ctx context.Context, cfg *daos.SelfTestConfig) ([]*daos.SelfTestResult, error) { | ||
| return []*daos.SelfTestResult{}, nil | ||
| } | ||
|
|
||
| func TestDaos_netTestCmdExecute(t *testing.T) { | ||
| // Quickie smoke test for the UI -- will flesh out later. | ||
| var opts cliOptions | ||
| log, buf := logging.NewTestLogger(t.Name()) | ||
| defer test.ShowBufferOnFailure(t, buf) | ||
| args := []string{ | ||
| "health", "net-test", | ||
| "--ranks", "0-3", | ||
| "--tags", "4-9", | ||
| "--size", "20 MiB", | ||
| "--rep-count", "2222", | ||
| "--bytes", "--verbose", | ||
| } | ||
| expArgs := netTestCmd{} | ||
| expArgs.Ranks.Replace(ranklist.MustCreateRankSet("0-3")) | ||
| expArgs.Tags.Replace(ranklist.MustCreateRankSet("4-9")) | ||
| expArgs.XferSize.Bytes = 20 * humanize.MiByte | ||
| expArgs.RepCount = 2222 | ||
| expArgs.Verbose = true | ||
| expArgs.TpsBytes = true | ||
|
|
||
| if err := parseOpts(args, &opts, log); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| cmpOpts := cmp.Options{ | ||
| cmpopts.IgnoreUnexported(netTestCmd{}), | ||
| cmp.Comparer(func(a, b ranklist.RankSet) bool { | ||
| return a.String() == b.String() | ||
| }), | ||
| cmp.Comparer(func(a, b ui.ByteSizeFlag) bool { | ||
| return a.String() == b.String() | ||
| }), | ||
| cmpopts.IgnoreTypes(cmdutil.LogCmd{}, cmdutil.JSONOutputCmd{}), | ||
| } | ||
| test.CmpAny(t, "health net-test args", expArgs, opts.Health.NetTest, cmpOpts...) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "non-destructive" necessary here? we don't qualify the above health check command at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to convey to the user that the test won't overwrite any data, even though it's doing writes. Simpler to just write "non-destructive" to allay any concerns, otherwise the concerned user has to go read documentation to figure out whether or not they should be worried.