Skip to content
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

Consolidated CLI design proposal #1144

Closed
wants to merge 80 commits into from
Closed

Conversation

xavpaice
Copy link
Member

@xavpaice xavpaice commented May 4, 2023

Description, Motivation and Context

Please include a summary of the change or what problem it solves. Please also include relevant motivation and context.

Checklist

  • New and existing tests pass locally with introduced changes.
  • Tests for the changes have been added (for bug fixes / features)
  • The commit message(s) are informative and highlight any breaking changes
  • Any documentation required has been added/updated. For changes to https://troubleshoot.sh/ create a PR here

Does this PR introduce a breaking change?

  • Yes
  • No

The intention is for troubleshoot to run as a kubectl/krew plugin
but for our design I think it's simpler of we focus on the troubleshoot
binary alone.
This fits nicer with the other subcommand names
@replicatedhq replicatedhq deleted a comment from CLAassistant May 10, 2023
Copy link
Member Author

@xavpaice xavpaice left a comment

Choose a reason for hiding this comment

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

Some minor comments, let's flesh this out a bit now with some technical detail on what functions need to move to where and how.

Particularly want to know about breaking changes (i.e. changes to functions that other projects are known to import) so that we can ensure these are agreed up-front.

Also; the existing CLI programs need to continue to work, let's identify what needs to happen to achieve that.

The goal here is that we agree on the desired outcome, the breaking changes that introduces, and the path to get to that point.

Copy link
Member Author

@xavpaice xavpaice left a comment

Choose a reason for hiding this comment

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

Some minor comments, let's flesh this out a bit now with some technical detail on what functions need to move to where and how.

Particularly want to know about breaking changes (i.e. changes to functions that other projects are known to import) so that we can ensure these are agreed up-front.

Also; the existing CLI programs need to continue to work, let's identify what needs to happen to achieve that.

The goal here is that we agree on the desired outcome, the breaking changes that introduces, and the path to get to that point.

banjoh and others added 4 commits June 9, 2023 14:22
…hq/troubleshoot into ada/proposal-troubleshoot-cli
Removed imported flags and made things a bit easier to read
@danj-replicated danj-replicated requested a review from a team as a code owner June 12, 2023 10:56
banjoh
banjoh previously approved these changes Jun 26, 2023
banjoh
banjoh previously approved these changes Jun 28, 2023
Copy link
Member Author

@xavpaice xavpaice left a comment

Choose a reason for hiding this comment

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

couple of notes, based on some discussion today

All top level commands (`support-bundle`, `preflight`, `analyze`, `redact`, and `sbctl`) of the Troubleshoot project will now be subcommands e.g `sbctl` is planned to be `troubleshoot inspect`. Some flags and subcommands names may change to best fit completeness of the CLI interface e.g `--interactive=false` will be `--no-input`. Putting the CLI interface together should aim to follow best practises from well known guidelines such as https://clig.dev/ and other mature projects.

- `troubleshoot inspect` - This subcommand will be equivalent to the `sbctl` command.
- `troubleshoot collect` - This subcommand will be equivalent to the `support-bundle` command.
Copy link
Member Author

Choose a reason for hiding this comment

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

some discussion around this - let's keep it consistent and have this troubleshoot support-bundle

docs/design/proposal-troubleshoot-consolidated-cli.md Outdated Show resolved Hide resolved
docs/design/proposal-troubleshoot-consolidated-cli.md Outdated Show resolved Hide resolved
docs/design/proposal-troubleshoot-consolidated-cli.md Outdated Show resolved Hide resolved
docs/design/proposal-troubleshoot-consolidated-cli.md Outdated Show resolved Hide resolved

## Non-Goals

- Maintain backward compatibility of the CLI interface and the public APIs (new interfaces will be created)
Copy link
Member Author

Choose a reason for hiding this comment

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

let's add a note to the effect that the existing CLI will remain supported until such time as the new CLI is in common use at least by the Replicated product set.

### Public APIs

The existing package entrypoint we use from the support-bundle command is `supportbundle.CollectSupportBundleFromSpec()`
which is an end-to-end function that runs collectors, redactors and then analysis.
Copy link
Member Author

Choose a reason for hiding this comment

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

let's be sure to mention that we do not intend to break this entrypoint immediately, but that it is intended that we use this API to allow future versions to make breaking changes to that function.

@xavpaice
Copy link
Member Author

xavpaice commented Nov 8, 2023

Closing until we revisit the proposal

@xavpaice xavpaice closed this Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type::feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Design proposal for a consolidated Troubleshoot CLI
6 participants