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.

Sign up for GitHub

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

Jump to bottom

chore(support-bundle): respect using load-cluster-specs=false #1634

Merged
merged 9 commits into from
Oct 11, 2024
Merged

chore(support-bundle): respect using load-cluster-specs=false #1634

merged 9 commits into from
Oct 11, 2024

Conversation

banjoh
Copy link
Member

@banjoh banjoh commented Oct 2, 2024 •
edited
Loading

Description, Motivation and Context

Using ./support-bundle --load-cluster-specs=false still leads to loading support bundle specs from the cluster.

Fixes: #1638

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

@banjoh banjoh requested a review from a team as a code owner October 2, 2024 15:29
@diamonwiggins
Copy link
Member

diamonwiggins commented Oct 3, 2024 •
edited
Loading

@banjoh any context on this one? FYI i think the reason we decided not to just set the flag to be true by default is because we didn't want there to be any hidden magic taking place if a user did specify additional flags.

Today:

./support-bundle https://some.spec.com would just collect the spec you provided, but with this change it would also load cluster specs. I'd imagine the tests are probably failing for the same reason. Things now being collected from the cluster that we didn't expect.

edit. are you saying that ./support-bundle --load-cluster-specs=false doesn't work today? I could see how in the current code and the conditional being based on args that that could be the case. Its sort of confusing how there are some flags we use directly, but others we convert into args. Either way, if this is what you're trying to address, i agree we should fix. However, we'll probably want to make sure that --load-cluster-specs isn't always true though 🤔

@banjoh banjoh marked this pull request as draft October 7, 2024 12:25
@banjoh
Copy link
Member Author

banjoh commented Oct 7, 2024

any context on this one?

As you correctly figured, ./support-bundle --load-cluster-specs=false does not work today, which is what I'm trying to address in this PR. By default, --load-cluster-specs will always be true

banjoh added 6 commits October 7, 2024 13:40
@banjoh
Some more simplification
d72cb2d 
Signed-off-by: Evans Mungai <[email protected]>
@banjoh
Ensure error in loading specs is printed in CLI
a240ddb 
Signed-off-by: Evans Mungai <[email protected]>
@banjoh
Run linter
72ec21f 
Signed-off-by: Evans Mungai <[email protected]>
@banjoh
Fix failing tests
01c16d8 
Signed-off-by: Evans Mungai <[email protected]>
@banjoh
Remove unnecessary test case rename
94be6b9 
Signed-off-by: Evans Mungai <[email protected]>
@banjoh
Fix error wrapping
6ba6eb8 
Signed-off-by: Evans Mungai <[email protected]>
@banjoh banjoh added type::bug Something isn't working bug::regression labels Oct 8, 2024
@banjoh banjoh marked this pull request as ready for review October 8, 2024 15:41
@banjoh banjoh changed the title fix: Allow using load-cluster-specs=false fix: Respect using load-cluster-specs=false Oct 10, 2024
@banjoh
Better wording in comments
0cc8c3e 
Signed-off-by: Evans Mungai <[email protected]>
@banjoh banjoh added type::chore and removed type::bug Something isn't working bug::regression labels Oct 10, 2024
@banjoh banjoh changed the title fix: Respect using load-cluster-specs=false chore(support-bundle): Always respect using load-cluster-specs=false Oct 10, 2024
@banjoh banjoh changed the title chore(support-bundle): Always respect using load-cluster-specs=false chore(support-bundle): respect using load-cluster-specs=false Oct 11, 2024
@diamonwiggins diamonwiggins merged commit 0113624 into replicatedhq:main Oct 11, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diamonwiggins diamonwiggins diamonwiggins approved these changes

Assignees
No one assigned
Labels
type::chore
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

support-bundle --load-cluster-specs=false still loads specs from a cluster
2 participants
@banjoh @diamonwiggins