diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go index e5ecce6a..a900f34e 100644 --- a/cmd/troubleshoot/cli/run.go +++ b/cmd/troubleshoot/cli/run.go @@ -328,7 +328,7 @@ the %s Admin Console to begin analysis.` return nil } - fmt.Printf("%s\n", response.ArchivePath) + fmt.Printf("\n%s\n", response.ArchivePath) return nil } diff --git a/pkg/collect/collect.go b/pkg/collect/collect.go index f30c6cd5..9cfdd4e1 100644 --- a/pkg/collect/collect.go +++ b/pkg/collect/collect.go @@ -190,31 +190,3 @@ func CollectRemote(c *troubleshootv1beta2.RemoteCollector, additionalRedactors * collectResult.AllCollectedData = allCollectedData return collectResult, nil } - -// Ensure that the specified collector is in the list of collectors -func EnsureCollectorInList(list []*troubleshootv1beta2.Collect, collector troubleshootv1beta2.Collect) []*troubleshootv1beta2.Collect { - for _, inList := range list { - if collector.ClusterResources != nil && inList.ClusterResources != nil { - return list - } - if collector.ClusterInfo != nil && inList.ClusterInfo != nil { - return list - } - } - - return append(list, &collector) -} - -// collect ClusterResources earliest in the list so the pod list does not include pods started by collectors -func EnsureClusterResourcesFirst(list []*troubleshootv1beta2.Collect) []*troubleshootv1beta2.Collect { - sliceOfClusterResources := []*troubleshootv1beta2.Collect{} - sliceOfOtherCollectors := []*troubleshootv1beta2.Collect{} - for _, collector := range list { - if collector.ClusterResources != nil { - sliceOfClusterResources = append(sliceOfClusterResources, []*troubleshootv1beta2.Collect{collector}...) - } else { - sliceOfOtherCollectors = append(sliceOfOtherCollectors, []*troubleshootv1beta2.Collect{collector}...) - } - } - return append(sliceOfClusterResources, sliceOfOtherCollectors...) -} diff --git a/pkg/collect/collector.go b/pkg/collect/collector.go index 50e5d20b..1124c32b 100644 --- a/pkg/collect/collector.go +++ b/pkg/collect/collector.go @@ -2,6 +2,7 @@ package collect import ( "context" + "encoding/json" "fmt" "strconv" "strings" @@ -189,3 +190,54 @@ func getCollectorName(c interface{}) string { } return collector } + +// Ensure that the specified collector is in the list of collectors +func EnsureCollectorInList(list []*troubleshootv1beta2.Collect, collector troubleshootv1beta2.Collect) []*troubleshootv1beta2.Collect { + for _, inList := range list { + if collector.ClusterResources != nil && inList.ClusterResources != nil { + return list + } + if collector.ClusterInfo != nil && inList.ClusterInfo != nil { + return list + } + } + + return append(list, &collector) +} + +// collect ClusterResources earliest in the list so the pod list does not include pods started by collectors +func EnsureClusterResourcesFirst(list []*troubleshootv1beta2.Collect) []*troubleshootv1beta2.Collect { + sliceOfClusterResources := []*troubleshootv1beta2.Collect{} + sliceOfOtherCollectors := []*troubleshootv1beta2.Collect{} + for _, collector := range list { + if collector.ClusterResources != nil { + sliceOfClusterResources = append(sliceOfClusterResources, []*troubleshootv1beta2.Collect{collector}...) + } else { + sliceOfOtherCollectors = append(sliceOfOtherCollectors, []*troubleshootv1beta2.Collect{collector}...) + } + } + return append(sliceOfClusterResources, sliceOfOtherCollectors...) +} + +// deduplicates a list of troubleshootv1beta2.Collect objects +// marshals object to json and then uses its string value to check for uniqueness +// there is no sorting of the keys in the collect object's spec so if the spec isn't an exact match line for line as written, no dedup will occur +func DedupCollectors(allCollectors []*troubleshootv1beta2.Collect) []*troubleshootv1beta2.Collect { + uniqueCollectors := make(map[string]bool) + finalCollectors := []*troubleshootv1beta2.Collect{} + + 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 +} diff --git a/pkg/collect/collector_test.go b/pkg/collect/collector_test.go index 64d3a764..eb3f33fb 100644 --- a/pkg/collect/collector_test.go +++ b/pkg/collect/collector_test.go @@ -5,6 +5,7 @@ import ( troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" "github.com/replicatedhq/troubleshoot/pkg/multitype" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -364,3 +365,83 @@ pwd=somethinggoeshere;`, }) } } + +func TestCollector_DedupCollectors(t *testing.T) { + tests := []struct { + name string + Collectors []*troubleshootv1beta2.Collect + want []*troubleshootv1beta2.Collect + }{ + { + name: "multiple cluster info", + Collectors: []*troubleshootv1beta2.Collect{ + { + ClusterInfo: &troubleshootv1beta2.ClusterInfo{}, + }, + { + ClusterInfo: &troubleshootv1beta2.ClusterInfo{}, + }, + }, + want: []*troubleshootv1beta2.Collect{ + { + ClusterInfo: &troubleshootv1beta2.ClusterInfo{}, + }, + }, + }, + { + name: "multiple cluster resources with matching namespace lists", + Collectors: []*troubleshootv1beta2.Collect{ + { + ClusterResources: &troubleshootv1beta2.ClusterResources{ + Namespaces: []string{"namespace1", "namespace2"}, + }, + }, + { + ClusterResources: &troubleshootv1beta2.ClusterResources{ + Namespaces: []string{"namespace1", "namespace2"}, + }, + }, + }, + want: []*troubleshootv1beta2.Collect{ + { + ClusterResources: &troubleshootv1beta2.ClusterResources{ + Namespaces: []string{"namespace1", "namespace2"}, + }, + }, + }, + }, + { + name: "multiple cluster resources with unnique namespace lists", + Collectors: []*troubleshootv1beta2.Collect{ + { + ClusterResources: &troubleshootv1beta2.ClusterResources{ + Namespaces: []string{"namespace1", "namespace2"}, + }, + }, + { + ClusterResources: &troubleshootv1beta2.ClusterResources{ + Namespaces: []string{"namespace1000", "namespace2000"}, + }, + }, + }, + want: []*troubleshootv1beta2.Collect{ + { + ClusterResources: &troubleshootv1beta2.ClusterResources{ + Namespaces: []string{"namespace1", "namespace2"}, + }, + }, + { + ClusterResources: &troubleshootv1beta2.ClusterResources{ + Namespaces: []string{"namespace1000", "namespace2000"}, + }, + }, + }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := DedupCollectors(tc.Collectors) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/pkg/preflight/collect.go b/pkg/preflight/collect.go index 73382218..7cf174d8 100644 --- a/pkg/preflight/collect.go +++ b/pkg/preflight/collect.go @@ -135,6 +135,7 @@ func Collect(opts CollectOpts, p *troubleshootv1beta2.Preflight) (CollectResult, } collectSpecs = collect.EnsureCollectorInList(collectSpecs, troubleshootv1beta2.Collect{ClusterInfo: &troubleshootv1beta2.ClusterInfo{}}) collectSpecs = collect.EnsureCollectorInList(collectSpecs, troubleshootv1beta2.Collect{ClusterResources: &troubleshootv1beta2.ClusterResources{}}) + collectSpecs = collect.DedupCollectors(collectSpecs) collectSpecs = collect.EnsureClusterResourcesFirst(collectSpecs) opts.KubernetesRestConfig.QPS = constants.DEFAULT_CLIENT_QPS diff --git a/pkg/supportbundle/collect.go b/pkg/supportbundle/collect.go index cb81b453..63c0adcc 100644 --- a/pkg/supportbundle/collect.go +++ b/pkg/supportbundle/collect.go @@ -76,6 +76,7 @@ func runCollectors(collectors []*troubleshootv1beta2.Collect, additionalRedactor collectSpecs = append(collectSpecs, collectors...) collectSpecs = collect.EnsureCollectorInList(collectSpecs, troubleshootv1beta2.Collect{ClusterInfo: &troubleshootv1beta2.ClusterInfo{}}) collectSpecs = collect.EnsureCollectorInList(collectSpecs, troubleshootv1beta2.Collect{ClusterResources: &troubleshootv1beta2.ClusterResources{}}) + collectSpecs = collect.DedupCollectors(collectSpecs) collectSpecs = collect.EnsureClusterResourcesFirst(collectSpecs) opts.KubernetesRestConfig.QPS = constants.DEFAULT_CLIENT_QPS