Skip to content

feature(smd): add membership subcommand#92

Open
Wafffle77 wants to merge 5 commits into
mainfrom
group-membership-query
Open

feature(smd): add membership subcommand#92
Wafffle77 wants to merge 5 commits into
mainfrom
group-membership-query

Conversation

@Wafffle77
Copy link
Copy Markdown
Collaborator

Pull Request Template

Thank you for your contribution! Please ensure the following before submitting:

Checklist

  • My code follows the style guidelines of this project
  • I have added/updated comments where needed
  • I have added tests that prove my fix is effective or my feature works
  • I have run make test (or equivalent) locally and all tests pass
  • DCO Sign-off: All commits are signed off (git commit -s) with my real name and email
  • REUSE Compliance:
    • Each new/modified source file has SPDX copyright and license headers
    • Any non-commentable files include a <filename>.license sidecar
    • All referenced licenses are present in the LICENSES/ directory

Description

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

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

For more info, see Contributing Guidelines.

@Wafffle77
Copy link
Copy Markdown
Collaborator Author

I'm not 100% sure if my updates to the man page are style-conformant, so I'd appreciate a proofread.

@Wafffle77 Wafffle77 force-pushed the group-membership-query branch from b06f98b to 2b38b8f Compare May 27, 2026 18:28
@synackd synackd self-requested a review May 27, 2026 22:41
Copy link
Copy Markdown
Collaborator

@synackd synackd left a comment

Choose a reason for hiding this comment

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

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.

Comment thread cmd/smd/group/membership.go Outdated
)

func newCmdGroupMembership() *cobra.Command {
// groupGetCmd represents the "smd group get" command
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Assumedly:

// groupMembershipCmd represents the "smd group membership" command

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 7675031

Comment thread cmd/smd/group/membership.go Outdated
// groupGetCmd represents the "smd group get" command
var groupMembershipCmd = &cobra.Command{
Use: "membership <node>",
Args: cobra.MaximumNArgs(1),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we allow this command to get the membership of multiple nodes?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 3df9405. All options are passed via flags that can be specified multiple times.

Comment thread cmd/smd/group/membership.go Outdated
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))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This would technically be the node xname instead of the node name.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Should be resolved in 3df9405 (This argument got factored out)

Comment thread man/ochami-smd.1.sc Outdated

Get the groups a node is a member of.

*membership* <node>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

*membership* _xname_

or even:

*membership* _node_xname_

The convention for placeholder args in these manual pages has been to underline them (surround with underscores).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Should be resolved in 77b650e

@synackd
Copy link
Copy Markdown
Collaborator

synackd commented May 28, 2026

Also, there were two new required CI checks that were added to satisfy #91, which has been merged. If you rebase onto the current main, these checks will be satisfied.

Wafffle77 added 2 commits May 28, 2026 14:10
Signed-off-by: Ethan Clark <elclark@lanl.gov>
…ering and multiple nodes

Signed-off-by: Ethan Clark <elclark@lanl.gov>
@Wafffle77 Wafffle77 force-pushed the group-membership-query branch from 2b38b8f to 3df9405 Compare May 28, 2026 21:29
Wafffle77 added 3 commits May 28, 2026 15:44
Signed-off-by: Ethan Clark <elclark@lanl.gov>
Signed-off-by: Ethan Clark <elclark@lanl.gov>
Signed-off-by: Ethan Clark <elclark@lanl.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants