-
Notifications
You must be signed in to change notification settings - Fork 94
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
feat: Add dry run flag to print support bundle specs to std out #1337
Conversation
I've started review this (on my phone) but need to finish on a computer - found a few things I want to verify |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM at a glance but there's a failing test. once the tests are passing I'll approve 👍
cmd/troubleshoot/cli/run.go
Outdated
} | ||
|
||
// Check if we have any collectors to run in the support bundle specs | ||
if len(mainBundle.Spec.Collectors) == 0 && len(mainBundle.Spec.HostCollectors) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by default, we have ClusterInfo
and ClusterResources
even without any collectors, I think maybe we don't need this check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I moved that check to a different place.
support-bundle
should error of there is no spec at all. Providing an empty spec should automatically add the ClusterInfo
and ClusterResources
collectors.
4390e14
to
2b04694
Compare
Changes causing tests to fail have been made. |
Description, Motivation and Context
Introduce
support-bundle --dry-run
flag used to print specs to stdout. The functional change is minor but I took the liberty of refactoring how we load specs from the CLI. This is where the majority of the changes lieFixes #1024
List of changes made
support-bundle --dry-run
cli flagsupport-bundle
binary load specs usingLoadFromCLIArgs
API, similar to thepreflight
binaryTest results
[evans] $ support-bundle --dry-run oci://registry.replicated.com/thanos-reloaded/unstable WARN[0002] unknown type: application/vnd.unknown.config.v1+json WARN[0003] unknown type: application/vnd.unknown.config.v1+json apiVersion: troubleshoot.sh/v1beta2 kind: SupportBundle metadata: creationTimestamp: null name: merged-support-bundle-spec spec: collectors: - clusterInfo: {} - clusterResources: {} - logs: namespace: '{{repl Namespace }}' selector: - app=nginx status: {}
[evans] $ cat sample-troubleshoot.yaml | support-bundle --dry-run - apiVersion: troubleshoot.sh/v1beta2 kind: Redactor metadata: creationTimestamp: null name: merged-redactors-spec spec: redactors: - fileSelector: file: data/my-password-dump name: replace password ....
Checklist
Does this PR introduce a breaking change?