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

Danj multiple inputs #683

Merged
merged 10 commits into from Sep 9, 2022
Merged

Danj multiple inputs #683

merged 10 commits into from Sep 9, 2022

Conversation

danj-replicated
Copy link
Member

This PR attempts to address issue #665

This is by no means a complete implimentation, and relies on #680 to enable collectors to be able to perform "intelligent merging".

but for now it allows multiple specs to be passed to support-bundle and will aggregate all defined items in the support-bundle spec, with some limited deduplication for cluster-info and cluster-resources collectors.

Signed-off-by: Dan Jones <danj@replicated.com>
Signed-off-by: Dan Jones <danj@replicated.com>
Signed-off-by: Dan Jones <danj@replicated.com>
Signed-off-by: Dan Jones <danj@replicated.com>
Signed-off-by: Dan Jones <danj@replicated.com>
Signed-off-by: Dan Jones <danj@replicated.com>
@xavpaice xavpaice added the type::feature New feature or request label Aug 28, 2022
@xavpaice xavpaice requested a review from divolgin August 29, 2022 20:46
Copy link
Member

@divolgin divolgin left a comment

Choose a reason for hiding this comment

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

This needs formatting (go fmt) and dead code removed.

@danj-replicated
Copy link
Member Author

After talking to @diamonwiggins I'm going to move the new methods from the api package to the support-bundle package

Signed-off-by: Dan Jones <danj@replicated.com>
Signed-off-by: Dan Jones <danj@replicated.com>
Signed-off-by: Dan Jones <danj@replicated.com>
@xavpaice
Copy link
Member

xavpaice commented Sep 8, 2022

I've tested this manually by adding several bundles at the CLI, some with multiple docs, and a redactor spec. All the bundles were merged as expected, and the redactors ran fine.

The changes requested have been taken care of except the go fmt, which I'd like to put into a separate commit.

The task that relates to this shouldn't be closed without adding tests, but given the test coverage in the current implementation, I think that can be a separate PR.

divolgin
divolgin previously approved these changes Sep 8, 2022
Signed-off-by: Dan Jones <danj@replicated.com>
Copy link
Member

@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.

LGTM. Future options for testing is to add something for runTroubleshoot, worth considering for a future PR.

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.

Make Troubleshoot accept multiple specs
3 participants