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

bug: [sc-103753] Host analysers are not deduplicating during multiple spec merges #1485 #1542

Merged
merged 4 commits into from
May 16, 2024

Conversation

nvanthao
Copy link
Member

@nvanthao nvanthao commented May 3, 2024

Description, Motivation and Context

Fixes: #1485

Given spec sb.yaml

apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
  name: sb
spec:
  analyzers:
  - clusterVersion: {}
  hostCollectors:
  - cpu: {}
  hostAnalyzers:
  - cpu: {}

and command

support-bundle --interactive=false sb.yaml sb.yaml --dry-run

Before

apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
  creationTimestamp: null
  name: merged-support-bundle-spec
spec:
  analyzers:
  - clusterVersion:
      outcomes: null
  - clusterVersion:
      outcomes: null
  hostAnalyzers:
  - cpu:
      outcomes: null
  - cpu:
      outcomes: null
  hostCollectors:
  - cpu: {}
  - cpu: {}
status: {}

After

apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
  creationTimestamp: null
  name: merged-support-bundle-spec
spec:
  analyzers:
  - clusterVersion:
      outcomes: null
  hostAnalyzers:
  - cpu:
      outcomes: null
  hostCollectors:
  - cpu: {}
status: {}

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

Duplicated specs with Analyzer, HostCollector and HostAnalayzer will now be merged by their stringified content

@nvanthao nvanthao added type::bug Something isn't working bug::normal labels May 3, 2024
@nvanthao nvanthao requested a review from a team as a code owner May 3, 2024 00:50
@nvanthao nvanthao linked an issue May 3, 2024 that may be closed by this pull request
@xavpaice
Copy link
Member

quick question - how does this interact if there's a url in one of the specs? Just want to confirm that the dedup happens after the url content replaced that particular spec.

@nvanthao nvanthao force-pushed the gerard/sc-103753/b-dedupe-host-analyzer branch from 2529eca to fedd3e5 Compare May 13, 2024 22:01
@nvanthao
Copy link
Member Author

the dedupe happens after loading URI spec here

err := loadSupportBundleSpecsFromURIs(ctx, kinds)

pkg/collect/collect.go Outdated Show resolved Hide resolved
banjoh
banjoh previously approved these changes May 15, 2024
cmd/troubleshoot/cli/run.go Outdated Show resolved Hide resolved
@nvanthao nvanthao force-pushed the gerard/sc-103753/b-dedupe-host-analyzer branch from f349f4c to a5c78dd Compare May 16, 2024 00:59
@nvanthao nvanthao requested review from DexterYan and banjoh May 16, 2024 01:18
Copy link
Member

@banjoh banjoh left a comment

Choose a reason for hiding this comment

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

Ship it

@nvanthao nvanthao merged commit fb0f81d into main May 16, 2024
27 checks passed
@nvanthao nvanthao deleted the gerard/sc-103753/b-dedupe-host-analyzer branch May 16, 2024 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug::normal type::bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Host analysers are not deduplicating during multiple spec merges
4 participants