From 0a019dfba85458d305f334f835b78844a56ef69e Mon Sep 17 00:00:00 2001 From: Gerard Nguyen Date: Thu, 2 May 2024 09:05:50 +1000 Subject: [PATCH 1/4] dedupe host collector and analyzer --- cmd/troubleshoot/cli/run.go | 8 ++++++++ pkg/analyze/analyzer.go | 20 ++++++++++++++++++++ pkg/collect/collect.go | 20 ++++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go index 5dc0781bb..c24bdaff4 100644 --- a/cmd/troubleshoot/cli/run.go +++ b/cmd/troubleshoot/cli/run.go @@ -362,6 +362,14 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface) additionalRedactors.Spec.Redactors = util.Append(additionalRedactors.Spec.Redactors, r.Spec.Redactors) } + // dedupe specs + if len(mainBundle.Spec.HostCollectors) > 0 { + mainBundle.Spec.HostCollectors = collect.DedupHostCollectors(mainBundle.Spec.HostCollectors) + } + if len(mainBundle.Spec.HostAnalyzers) > 0 { + mainBundle.Spec.HostAnalyzers = analyzer.DedupHostAnalyzers(mainBundle.Spec.HostAnalyzers) + } + return mainBundle, additionalRedactors, nil } diff --git a/pkg/analyze/analyzer.go b/pkg/analyze/analyzer.go index 01bf8790d..2b93371ea 100644 --- a/pkg/analyze/analyzer.go +++ b/pkg/analyze/analyzer.go @@ -278,6 +278,26 @@ func DedupAnalyzers(allAnalyzers []*troubleshootv1beta2.Analyze) []*troubleshoot return finalAnalyzers } +func DedupHostAnalyzers(allAnalyzers []*troubleshootv1beta2.HostAnalyze) []*troubleshootv1beta2.HostAnalyze { + uniqueAnalyzers := make(map[string]bool) + finalAnalyzers := []*troubleshootv1beta2.HostAnalyze{} + + for _, analyzer := range allAnalyzers { + data, err := json.Marshal(analyzer) + if err != nil { + // return analyzer as is if for whatever reason it can't be marshalled into json + finalAnalyzers = append(finalAnalyzers, analyzer) + } else { + stringData := string(data) + if _, value := uniqueAnalyzers[stringData]; !value { + uniqueAnalyzers[stringData] = true + finalAnalyzers = append(finalAnalyzers, analyzer) + } + } + } + return finalAnalyzers +} + func stripRedactedLines(yaml []byte) []byte { buf := bytes.NewBuffer(yaml) scanner := bufio.NewScanner(buf) diff --git a/pkg/collect/collect.go b/pkg/collect/collect.go index 2ead4cae8..7ad45c9ab 100644 --- a/pkg/collect/collect.go +++ b/pkg/collect/collect.go @@ -192,3 +192,23 @@ func CollectRemote(c *troubleshootv1beta2.RemoteCollector, additionalRedactors * collectResult.AllCollectedData = allCollectedData return collectResult, nil } + +func DedupHostCollectors(allCollectors []*troubleshootv1beta2.HostCollect) []*troubleshootv1beta2.HostCollect { + uniqueCollectors := make(map[string]bool) + finalCollectors := []*troubleshootv1beta2.HostCollect{} + + for _, collector := range allCollectors { + data, err := json.Marshal(collector) + if err != nil { + // return collector as is if for whatever reason it can't be marshalled into json + finalCollectors = append(finalCollectors, collector) + } else { + stringData := string(data) + if _, value := uniqueCollectors[stringData]; !value { + uniqueCollectors[stringData] = true + finalCollectors = append(finalCollectors, collector) + } + } + } + return finalCollectors +} From 80d9d80845d7832d03b62908752091b79dde9323 Mon Sep 17 00:00:00 2001 From: Gerard Nguyen Date: Fri, 3 May 2024 10:20:00 +1000 Subject: [PATCH 2/4] dedup analyzer --- cmd/troubleshoot/cli/run.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go index c24bdaff4..71afa0602 100644 --- a/cmd/troubleshoot/cli/run.go +++ b/cmd/troubleshoot/cli/run.go @@ -363,6 +363,10 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface) } // dedupe specs + if len(mainBundle.Spec.Analyzers) > 0 { + mainBundle.Spec.Analyzers = analyzer.DedupAnalyzers(mainBundle.Spec.Analyzers) + } + if len(mainBundle.Spec.HostCollectors) > 0 { mainBundle.Spec.HostCollectors = collect.DedupHostCollectors(mainBundle.Spec.HostCollectors) } From 7519b0b9110c4f0f0a6337f479a0d81e5dea6749 Mon Sep 17 00:00:00 2001 From: Gerard Nguyen Date: Fri, 3 May 2024 10:43:15 +1000 Subject: [PATCH 3/4] add unit test --- cmd/troubleshoot/cli/run_test.go | 25 +++++++++++++++++++++++++ pkg/analyze/analyzer.go | 21 ++++++++++----------- pkg/collect/collect.go | 21 ++++++++++----------- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/cmd/troubleshoot/cli/run_test.go b/cmd/troubleshoot/cli/run_test.go index dfc3b715d..c6eca9b8b 100644 --- a/cmd/troubleshoot/cli/run_test.go +++ b/cmd/troubleshoot/cli/run_test.go @@ -343,3 +343,28 @@ spec: }) } } + +func Test_loadDuplicatedBundleSpecs(t *testing.T) { + spec := testutils.ServeFromFilePath(t, ` +apiVersion: troubleshoot.sh/v1beta2 +kind: SupportBundle +metadata: + name: sb +spec: + analyzers: + - clusterVersion: {} + hostCollectors: + - cpu: {} + hostAnalyzers: + - cpu: {} +`) + args := []string{spec, spec} + + ctx := context.Background() + client := testclient.NewSimpleClientset() + sb, _, err := loadSpecs(ctx, args, client) + require.NoError(t, err) + assert.Len(t, sb.Spec.Analyzers, 1) + assert.Len(t, sb.Spec.HostCollectors, 1) + assert.Len(t, sb.Spec.HostAnalyzers, 1) +} diff --git a/pkg/analyze/analyzer.go b/pkg/analyze/analyzer.go index 2b93371ea..6a5593dab 100644 --- a/pkg/analyze/analyzer.go +++ b/pkg/analyze/analyzer.go @@ -279,23 +279,22 @@ func DedupAnalyzers(allAnalyzers []*troubleshootv1beta2.Analyze) []*troubleshoot } func DedupHostAnalyzers(allAnalyzers []*troubleshootv1beta2.HostAnalyze) []*troubleshootv1beta2.HostAnalyze { - uniqueAnalyzers := make(map[string]bool) - finalAnalyzers := []*troubleshootv1beta2.HostAnalyze{} + seen := make(map[string]bool) + out := []*troubleshootv1beta2.HostAnalyze{} for _, analyzer := range allAnalyzers { data, err := json.Marshal(analyzer) if err != nil { - // return analyzer as is if for whatever reason it can't be marshalled into json - finalAnalyzers = append(finalAnalyzers, analyzer) - } else { - stringData := string(data) - if _, value := uniqueAnalyzers[stringData]; !value { - uniqueAnalyzers[stringData] = true - finalAnalyzers = append(finalAnalyzers, analyzer) - } + out = append(out, analyzer) + continue + } + key := string(data) + if _, ok := seen[key]; !ok { + out = append(out, analyzer) + seen[key] = true } } - return finalAnalyzers + return out } func stripRedactedLines(yaml []byte) []byte { diff --git a/pkg/collect/collect.go b/pkg/collect/collect.go index 7ad45c9ab..e0713eda7 100644 --- a/pkg/collect/collect.go +++ b/pkg/collect/collect.go @@ -194,21 +194,20 @@ func CollectRemote(c *troubleshootv1beta2.RemoteCollector, additionalRedactors * } func DedupHostCollectors(allCollectors []*troubleshootv1beta2.HostCollect) []*troubleshootv1beta2.HostCollect { - uniqueCollectors := make(map[string]bool) - finalCollectors := []*troubleshootv1beta2.HostCollect{} + seen := make(map[string]bool) + out := []*troubleshootv1beta2.HostCollect{} for _, collector := range allCollectors { data, err := json.Marshal(collector) if err != nil { - // return collector as is if for whatever reason it can't be marshalled into json - finalCollectors = append(finalCollectors, collector) - } else { - stringData := string(data) - if _, value := uniqueCollectors[stringData]; !value { - uniqueCollectors[stringData] = true - finalCollectors = append(finalCollectors, collector) - } + out = append(out, collector) + continue + } + key := string(data) + if _, ok := seen[key]; !ok { + out = append(out, collector) + seen[key] = true } } - return finalCollectors + return out } From a5c78ddba068f34926b42c4a1ee95c754fc7d6c5 Mon Sep 17 00:00:00 2001 From: Gerard Nguyen Date: Thu, 16 May 2024 10:52:21 +1000 Subject: [PATCH 4/4] use generic --- cmd/troubleshoot/cli/run.go | 14 ++++---------- cmd/troubleshoot/cli/run_test.go | 3 +++ internal/util/util.go | 24 ++++++++++++++++++++++++ pkg/analyze/analyzer.go | 19 ------------------- pkg/collect/collect.go | 19 ------------------- 5 files changed, 31 insertions(+), 48 deletions(-) diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go index 71afa0602..de777d5b7 100644 --- a/cmd/troubleshoot/cli/run.go +++ b/cmd/troubleshoot/cli/run.go @@ -363,16 +363,10 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface) } // dedupe specs - if len(mainBundle.Spec.Analyzers) > 0 { - mainBundle.Spec.Analyzers = analyzer.DedupAnalyzers(mainBundle.Spec.Analyzers) - } - - if len(mainBundle.Spec.HostCollectors) > 0 { - mainBundle.Spec.HostCollectors = collect.DedupHostCollectors(mainBundle.Spec.HostCollectors) - } - if len(mainBundle.Spec.HostAnalyzers) > 0 { - mainBundle.Spec.HostAnalyzers = analyzer.DedupHostAnalyzers(mainBundle.Spec.HostAnalyzers) - } + mainBundle.Spec.Collectors = util.Dedup(mainBundle.Spec.Collectors) + mainBundle.Spec.Analyzers = util.Dedup(mainBundle.Spec.Analyzers) + mainBundle.Spec.HostCollectors = util.Dedup(mainBundle.Spec.HostCollectors) + mainBundle.Spec.HostAnalyzers = util.Dedup(mainBundle.Spec.HostAnalyzers) return mainBundle, additionalRedactors, nil } diff --git a/cmd/troubleshoot/cli/run_test.go b/cmd/troubleshoot/cli/run_test.go index c6eca9b8b..6692a906e 100644 --- a/cmd/troubleshoot/cli/run_test.go +++ b/cmd/troubleshoot/cli/run_test.go @@ -351,6 +351,8 @@ kind: SupportBundle metadata: name: sb spec: + collectors: + - helm: {} analyzers: - clusterVersion: {} hostCollectors: @@ -364,6 +366,7 @@ spec: client := testclient.NewSimpleClientset() sb, _, err := loadSpecs(ctx, args, client) require.NoError(t, err) + assert.Len(t, sb.Spec.Collectors, 1+2) // default clusterInfo + clusterResources assert.Len(t, sb.Spec.Analyzers, 1) assert.Len(t, sb.Spec.HostCollectors, 1) assert.Len(t, sb.Spec.HostAnalyzers, 1) diff --git a/internal/util/util.go b/internal/util/util.go index e70e9acee..e10f80856 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -3,6 +3,7 @@ package util import ( "bufio" "bytes" + "encoding/json" "fmt" "net/url" "os" @@ -145,3 +146,26 @@ func PromptYesNo(question string) bool { } } } + +func Dedup[T any](objs []T) []T { + seen := make(map[string]bool) + out := []T{} + + if len(objs) == 0 { + return objs + } + + for _, o := range objs { + data, err := json.Marshal(o) + if err != nil { + out = append(out, o) + continue + } + key := string(data) + if _, ok := seen[key]; !ok { + out = append(out, o) + seen[key] = true + } + } + return out +} diff --git a/pkg/analyze/analyzer.go b/pkg/analyze/analyzer.go index 6a5593dab..01bf8790d 100644 --- a/pkg/analyze/analyzer.go +++ b/pkg/analyze/analyzer.go @@ -278,25 +278,6 @@ func DedupAnalyzers(allAnalyzers []*troubleshootv1beta2.Analyze) []*troubleshoot return finalAnalyzers } -func DedupHostAnalyzers(allAnalyzers []*troubleshootv1beta2.HostAnalyze) []*troubleshootv1beta2.HostAnalyze { - seen := make(map[string]bool) - out := []*troubleshootv1beta2.HostAnalyze{} - - for _, analyzer := range allAnalyzers { - data, err := json.Marshal(analyzer) - if err != nil { - out = append(out, analyzer) - continue - } - key := string(data) - if _, ok := seen[key]; !ok { - out = append(out, analyzer) - seen[key] = true - } - } - return out -} - func stripRedactedLines(yaml []byte) []byte { buf := bytes.NewBuffer(yaml) scanner := bufio.NewScanner(buf) diff --git a/pkg/collect/collect.go b/pkg/collect/collect.go index e0713eda7..2ead4cae8 100644 --- a/pkg/collect/collect.go +++ b/pkg/collect/collect.go @@ -192,22 +192,3 @@ func CollectRemote(c *troubleshootv1beta2.RemoteCollector, additionalRedactors * collectResult.AllCollectedData = allCollectedData return collectResult, nil } - -func DedupHostCollectors(allCollectors []*troubleshootv1beta2.HostCollect) []*troubleshootv1beta2.HostCollect { - seen := make(map[string]bool) - out := []*troubleshootv1beta2.HostCollect{} - - for _, collector := range allCollectors { - data, err := json.Marshal(collector) - if err != nil { - out = append(out, collector) - continue - } - key := string(data) - if _, ok := seen[key]; !ok { - out = append(out, collector) - seen[key] = true - } - } - return out -}