-
Notifications
You must be signed in to change notification settings - Fork 522
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.
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
bs target #2976
bs target #2976
Conversation
tools-v2/internal/error/error.go
Outdated
ErrTargetClient = func() *CmdError { | ||
return NewInternalCmdError(82, "get client target error: %s") | ||
} |
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.
duplicate
) | ||
|
||
const ( | ||
targetexample = "$curve bs target" |
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.
Maybe curve bs export target
is more meaningful.
clusterCommand := cluster.NewStatusClusterCommand() | ||
// cluster command no output | ||
clusterCommand.Cmd.SetArgs([]string{ | ||
fmt.Sprintf("--%s", config.FORMAT), config.FORMAT_NOOUT,}) | ||
clusterCommand.Cmd.SilenceErrors = true | ||
err := clusterCommand.Cmd.Execute() | ||
if err != nil { | ||
retErr := cmderror.ErrTargetCluster() | ||
retErr.Format(err.Error()) | ||
return retErr.ToError() | ||
} | ||
clusterResult := clusterCommand.Result.(map[string]interface{}) |
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.
we can use a function in cluster.go to handle. Like other command.
tcmd.Result = append(tcmd.Result.([]ResultStruct), ExtractClusterResult(clusterResult, "etcd")) | ||
tcmd.Result = append(tcmd.Result.([]ResultStruct), ExtractClusterResult(clusterResult, "mds")) | ||
tcmd.Result = append(tcmd.Result.([]ResultStruct), ExtractClusterResult(clusterResult, "chunkserver")) | ||
tcmd.Result = append(tcmd.Result.([]ResultStruct), ExtractClusterResult(clusterResult, "snapshot")) |
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.
we can unmarsal the result using json package?
clientStruct["label"] = map[string]string{ | ||
"job" : "client", | ||
} | ||
clientStruct["targets"] = make([]string, 0) | ||
for _, result := range clientResult { | ||
clientStruct["targets"] = append(clientStruct["targets"].([]string), result["addr"]) | ||
} |
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.
Ditto.
@wsehjk you should squash to one commit and modify the commit message is meaningful. |
cicheck |
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.
good job
func ExtractClusterResult(clusterResult map[string]interface{}, service string) ResultStruct{ | ||
result := ResultStruct{} | ||
arrs := clusterResult[service].([]map[string]string) | ||
result["label"] = map[string]string{ |
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.
Instead of hardcoding, define the constants in "github.com/opencurve/curve/tools-v2/internal/utils"
.
) | ||
|
||
const ( | ||
targetexample = "$curve bs export target" |
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.
targetExample
is better
return retErr.ToError() | ||
} | ||
tcmd.Result = []ResultStruct{} | ||
tcmd.Result = append(tcmd.Result.([]ResultStruct), ExtractClusterResult(clusterResult.(map[string]interface{}), "etcd")) |
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.
ditto
clientCommand.Cmd.SilenceErrors = true | ||
err := clientCommand.Cmd.Execute() | ||
|
||
//clientResult := clientCommand.Result.([]map[string]string) |
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.
Remove unnecessary code.
It is better to show the results of your test. |
cicheck |
export services and clients in the bs cluster | ||
|
||
Usage: | ||
|
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.
Add specific commands.
curve export or curve export target or curve bs export target?
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.
My mistake. I have fixed it
The commit message suggestion: |
The command can generate a target.json file is better. And the default path is current directory. |
cicheck |
8 similar comments
cicheck |
cicheck |
cicheck |
cicheck |
cicheck |
cicheck |
cicheck |
cicheck |
Signed-off-by: wsehjk <[email protected]>
cicheck |
2 similar comments
cicheck |
cicheck |
What problem does this PR solve?
Issue Number: #2564
Problem Summary:
What is changed and how it works?
What's Changed:
add target/target.go,
How it Works:
this command is dependent on two command: curve bs status cluster and curve bs status client
Side effects(Breaking backward compatibility? Performance regression?):
No side effects found
command output:
Check List