chore(admin-cli): Implement detailed switch show#1814
Open
inf0rmatiker wants to merge 4 commits into
Open
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
inf0rmatiker
force-pushed
the
chore/1782_switch-show-output
branch
from
a2fa186 to
aa9771e
Compare
inf0rmatiker
added
admin tools
Contributor
|
looks good to me. But would be nice to also get the JSON output working. It should usually be the most simple thing, because we only have to call |
inf0rmatiker
force-pushed
the
chore/1782_switch-show-output
branch
from
db995e0 to
d0adee8
Compare
The `switch show` command previously had a stub implementation that did
not compile. This change fully implements both the list view (all
switches) and the single-switch detail view, bringing it to parity with
the existing `machine show` command.
Additionally: Move switch ID to Switch details section
Changes by file:
`switch/show/args.rs`:
- Replace the untyped `identifier: Option<String>` field with a typed
`switch_id: Option<SwitchId>`, matching the machine show pattern and
enabling parse-time validation of the switch ID format.
`switch/show/cmd.rs`:
- Add `to_table` to convert a `SwitchList` into a prettytable `Table`
(columns: ID, Name, Metadata Name, Slot, Tray, Primary, Power State,
Health, FabricManager(nmxc), State).
- Add `show_switches` to fetch all switches with pagination, sort by
primary ID or state, and render as ASCII table or CSV.
- Add `show_switch_information` to fetch a single switch by ID and
render it in the requested format.
- Add `switch_details_text` to produce a human-readable key-value
representation of all Switch RPC fields (config, status/lifecycle,
fabric manager details, health sources, BMC info, metadata).
- Rewrite `handle_show` to dispatch to the appropriate function and
remove dead commented-out code.
- Add a `switch_details_text_smoke` async unit test that constructs a
representative Switch and prints the formatted output to stdout.
`switch/show/mod.rs`:
- Thread `page_size` and `sort_by` through to `handle_show`, matching
the signature expected by the new implementation.
`switch/tests.rs`:
- Update `parse_show_no_args` and `parse_show_with_identifier` to
reference the new `switch_id` field instead of `identifier`.
`rpc/src/admin_cli.rs`:
- Add `CarbideCliError::SwitchNotFound(SwitchId)` variant.
- Add `OutputFormat::unimplemented_output_format_err()` helper to
reduce boilerplate at call sites.
`managed_switch/show/cmd.rs`:
- Move the Switch ID from a standalone header line into the key-value
data table so it is formatted consistently with other fields.
`machine/show/cmd.rs`:
- Use `output_format.unimplemented_output_format_err()` for the YAML
case instead of an inline `CarbideCliError::NotImplemented`.
`async_write.rs` / `dpu/{status,versions}/cmd.rs`:
- Fix fully-qualified path for `CarbideCliError` in the
`async_write_table_as_csv!` macro and remove unused imports.
Signed-off-by: Caleb Carlson <cacarlson@nvidia.com>
Signed-off-by: Caleb Carlson <cacarlson@nvidia.com>
Signed-off-by: Caleb Carlson <cacarlson@nvidia.com>
Signed-off-by: Caleb Carlson <cacarlson@nvidia.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
This change fully implements both the list view (all switches) and the single-switch detail view, bringing it to parity with the existing
machine showcommand.Additionally: Moves switch ID to Switch details section for
managed-switch show <id>.Output format of
switch show <id>before:Output format after:
This roughly follows the structure of the gRPC
Switchmessage - note that rearranging much more than this makes it a maintenance nightmare when the gRPC message undergoes changes.For context, this is the output format of
machine show <id>:We want to move away from the ALL CAPS key/value representation for the "nice" human-readable text format - no need to scream at the user - but that's outside the scope of this PR.
Changes by file:
switch/show/args.rs:identifier: Option<String>field with a typedswitch_id: Option<SwitchId>, matching the machine show pattern and enabling parse-time validation of the switch ID format.switch/show/cmd.rs:to_tableto convert aSwitchListinto a prettytableTable(columns: ID, Name, Metadata Name, Slot, Tray, Primary, Power State, Health, FabricManager(nmxc), State).show_switchesto fetch all switches with pagination, sort by primary ID or state, and render as ASCII table or CSV.show_switch_informationto fetch a single switch by ID and render it in the requested format.switch_details_textto produce a human-readable key-value representation of all Switch RPC fields (config, status/lifecycle, fabric manager details, health sources, BMC info, metadata).handle_showto dispatch to the appropriate function and remove dead commented-out code.switch_details_text_smokeasync unit test that constructs a representative Switch and prints the formatted output to stdout.switch/show/mod.rs:page_sizeandsort_bythrough tohandle_show, matching the signature expected by the new implementation.switch/tests.rs:parse_show_no_argsandparse_show_with_identifierto reference the newswitch_idfield instead ofidentifier.rpc/src/admin_cli.rs:CarbideCliError::SwitchNotFound(SwitchId)variant.OutputFormat::unimplemented_output_format_err()helper to reduce boilerplate at call sites.managed_switch/show/cmd.rs:machine/show/cmd.rs:output_format.unimplemented_output_format_err()for the YAML case instead of an inlineCarbideCliError::NotImplemented.async_write.rs/dpu/{status,versions}/cmd.rs:CarbideCliErrorin theasync_write_table_as_csv!macro and remove unused imports.Type of Change
Related Issues (Optional)
Closes #1782
Breaking Changes
These changes only affects the CLI. Existing tooling built around expected CLI output formats may break.
switch showcommand will no longer be allowed to input an empty string for the switch ID to display all switches. Instead, they must omit the arg altogether. This arg must be a valid switch ID - it will be immediately parsed and validated into aSwitchIdso invalid formats will fail.managed-switch showcommand has been moved into the coreSwitch:body, instead of the header. This is to maintain parity with the output format ofmanaged-host show.switch show <id>no longer prints an ASCII table row, in the same format asswitch show. Instead, it prints a more detailed ASCII text output similar to howmachine show <id>does it.All other changes are added functionality and do not change existing formats.
Testing
Manual CLI Tests
Switch ID format is validated - no longer allowing any
Stringvalue:Omitting
switch_idpositional argument retains same behavior of listing all switches:Additional Notes
N/A.