Skip to content

fix(smd): document iface commands and accept multiple network pairs - #225

Merged
synackd merged 3 commits into
mainfrom
fix/smd-iface-man
Oct 29, 2026
Merged

synackd merged 3 commits into
mainfrom
fix/smd-iface-man

Conversation

@synackd

@synackd synackd commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Document smd iface add and smd iface delete in ochami-smd(1), including positional and payload input forms, options, and delete ID validation. Carry forward the documentation from #217.

Accept multiple network,IPv4 arguments in iface add to match its existing help and examples. Reject malformed pairs with a usage error instead of panicking, and validate all pairs before sending a request. Add regression tests for the complete multi-address request and invalid input using the conventions from test/organize.

Fixes #206
Supersedes #217

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
  • I have updated the relevant documentation (CLI examples, man pages, README, other docs, etc.)
  • 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 (not applicable; none added or modified)
    • All referenced licenses are present in the LICENSES/ directory

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Dependency update
  • Build system/CI

@synackd
synackd marked this pull request as ready for review October 28, 2026 16:36
@synackd synackd modified the milestone: v0.11.0 Oct 28, 2026
anyingiit and others added 2 commits October 28, 2026 12:40
The iface section of ochami-smd(1) only described "get". Add entries for
"add" and "delete", following the compep and rfe sections: their forms,
the payload input options, --all and --no-confirm for delete, and the
rule that delete only uses each payload item's ID (blank or path-like IDs
are a usage error, and no ID is derived from MACAddress).

Fixes #206

Signed-off-by: anyingiit <49945850+anyingiit@users.noreply.github.com>
Accept at least three positional arguments so iface add can consume every
network,IPv4 pair advertised by its help text and examples. Keep all pairs
on one Ethernet interface in the request rather than rejecting commands
that specify more than one address.

Reject pairs without a comma or network name before indexing their fields
to report a usage error instead of panicking. Preserve IPv4 validation and
validate every pair before sending the interface to SMD.

Update the man page to describe repeated pair arguments. Add command tests
that verify the complete multi-address request and reject malformed pairs,
including invalid later pairs, without sending a request. Follow the
test/organize conventions for test names, files, comments, and runtimes.

Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
@synackd
synackd requested a review from a team October 28, 2026 19:18
Comment thread man/ochami-smd.1.sc Resolved
Comment thread man/ochami-smd.1.sc Resolved
State that iface add and delete payloads must be an array of
EthernetInterface objects, even for a single interface. Both commands
unmarshal -d data directly into a slice, so a bare object is rejected,
and the data structure example already showed an array while calling
it a single EthernetInterface.

Correct the delete validation rule to cover only the IDs used to select
interfaces. With --all or -d, positional arguments are ignored with a
warning and never validated, so the previous wording promised a usage
error that --all never raised. Document that --all takes precedence over
-d after the payload is validated, and that add also ignores positional
arguments when -d is passed.

Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
@synackd
synackd added this pull request to stack #227 October 29, 2026 19:32
@synackd
synackd requested a review from davidallendj October 29, 2026 19:42
@davidallendj

davidallendj commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

I don't think this is directly related to this PR, but I noticed that the config is checked before input validation. For example, if I intentionally run the following command to produce an error:

ochami smd --ignore-config iface add node de:ca:fc:0f:fe:ee external

I believe external should be something like external,10.1.0.55 including an IP address, but this returns a configuration error instead of one related to the input.

2026-10-29T14:33:55-06:00 ERR root.go:247 > failed to execute command error="failed to get base URI for SMD: could not get smd base URI: base URI for smd not found (neither cluster.uri nor smd.uri specified)"
2026-10-29T14:33:55-06:00 ERR cli.go:59 > see 'ochami smd iface add --help' for long command help
exit status 3

However, if you include the --url flag here, it does the input validation first, and returns a more appropriate error.

ochami smd --ignore-config iface add --uri http://127.0.0.1:18080 --no-token node de:ca:fc:0f:fe:ee external

And here's the corresponding error message:

2026-10-29T14:37:28-06:00 ERR root.go:247 > failed to execute command error="invalid network,ip pair \"external\": expected a network name and IPv4 address separated by a comma"
2026-10-29T14:37:28-06:00 ERR cli.go:59 > see 'ochami smd iface add --help' for long command help
exit status 2

If I'm including the --ignore-config flag, it would seem more logical to me to do the input validation before checking the configuration here, but I don't know if the order was set that way intentionally.

@synackd

synackd commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

However, if you include the --url flag here, it does the input validation first, and returns a more appropriate error.

This is intentional. If you pass --ignore-config, ochami completely ignores the config file and thus has no cluster configuration read in, which has to be passed via e.g. --uri. It doesn't make sense to do any cluster operation without a way to reach OpenCHAMI services.

@davidallendj davidallendj left a comment

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.

LGTM

@synackd
synackd merged commit 1cd3168 into main Oct 29, 2026
19 checks passed
@synackd
synackd deleted the fix/smd-iface-man branch October 29, 2026 21:21
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.

[DOCS] ochami-smd(1) documents only smd iface get

3 participants