feature(smd): add membership subcommand#92
Conversation
|
I'm not 100% sure if my updates to the man page are style-conformant, so I'd appreciate a proofread. |
b06f98b to
2b38b8f
Compare
synackd
left a comment
There was a problem hiding this comment.
This works nicely! Just a few small comments below.
Also, I could see it being useful to get the memberships of multiple nodes, which is probably most efficiently done by using query parameters, e.g:
GET /hsm/v2/memberships?id=x3000c0s0b0n0&id=x3001c1s7b70n0
to get memberships for x3000c0s0b0n0 and x3001c1s7b70n0
Which could be done via:
ochami smd group membership x3000c0s0b0n0 x3001c1s7b70n0
I could also see the use in getting all memberships if no arguments are specified.
The SMD /memberships API has a lot more query parameters that could be used as well. [1] [2] (You can copy the contents of swagger_v2.yaml into an OpenAPI editor such as editor.swagger.io to interactively look through the API spec.)
As a reference/hint for query parameters, take a look at the smd rfe get command which calls GetRedfishEndpoints() which in turn calls GetData(). You essentially craft the query parameters using a url.Values and then pass it as a string to your get function.
| ) | ||
|
|
||
| func newCmdGroupMembership() *cobra.Command { | ||
| // groupGetCmd represents the "smd group get" command |
There was a problem hiding this comment.
Assumedly:
// groupMembershipCmd represents the "smd group membership" command| // groupGetCmd represents the "smd group get" command | ||
| var groupMembershipCmd = &cobra.Command{ | ||
| Use: "membership <node>", | ||
| Args: cobra.MaximumNArgs(1), |
There was a problem hiding this comment.
Could we allow this command to get the membership of multiple nodes?
There was a problem hiding this comment.
Fixed in 3df9405. All options are passed via flags that can be specified multiple times.
| Example: ` ochami smd group get x1000c0s0b0n0`, | ||
| PreRunE: func(cmd *cobra.Command, args []string) error { | ||
| if len(args) != 1 { | ||
| return fmt.Errorf("expected 1 argument (node name), got %d", len(args)) |
There was a problem hiding this comment.
This would technically be the node xname instead of the node name.
There was a problem hiding this comment.
Should be resolved in 3df9405 (This argument got factored out)
|
|
||
| Get the groups a node is a member of. | ||
|
|
||
| *membership* <node> |
There was a problem hiding this comment.
*membership* _xname_
or even:
*membership* _node_xname_
The convention for placeholder args in these manual pages has been to underline them (surround with underscores).
|
Also, there were two new required CI checks that were added to satisfy #91, which has been merged. If you rebase onto the current |
Signed-off-by: Ethan Clark <elclark@lanl.gov>
…ering and multiple nodes Signed-off-by: Ethan Clark <elclark@lanl.gov>
2b38b8f to
3df9405
Compare
Signed-off-by: Ethan Clark <elclark@lanl.gov>
Signed-off-by: Ethan Clark <elclark@lanl.gov>
Signed-off-by: Ethan Clark <elclark@lanl.gov>
Pull Request Template
Thank you for your contribution! Please ensure the following before submitting:
Checklist
make test(or equivalent) locally and all tests passgit commit -s) with my real name and email<filename>.licensesidecarLICENSES/directoryDescription
Adds the
ochami smd membership <node>subcommand, which can be used to query for group memberships of a specific node.Implements #35
Type of Change
For more info, see Contributing Guidelines.