feat(support): improve dedup by using key

replicatedhq · Oct 24, 2024 · 85b2ea1 · 85b2ea1
1 parent eacff71
commit 85b2ea1
Show file tree

Hide file tree

Showing 3 changed files with 209 additions and 5 deletions.
diff --git a/internal/util/util.go b/internal/util/util.go
@@ -17,6 +17,10 @@ import (
 
 const HOST_COLLECTORS_RUN_AS_ROOT_PROMPT = "Some host collectors need to be run as root.\nDo you want to exit and rerun the command using sudo?"
 
+type Keyer interface {
+	UniqKey() string
+}
+
 func HomeDir() string {
 	if h := os.Getenv("HOME"); h != "" {
 		return h
@@ -156,16 +160,25 @@ func Dedup[T any](objs []T) []T {
 	}
 
 	for _, o := range objs {
-		data, err := json.Marshal(o)
-		if err != nil {
-			out = append(out, o)
-			continue
+		var key string
+
+		// Check if the object implements the Keyer interface
+		if k, ok := any(o).(Keyer); ok {
+			key = k.UniqKey()
+		} else {
+			data, err := json.Marshal(o)
+			if err != nil {
+				out = append(out, o)
+				continue
+			}
+			key = string(data)
 		}
-		key := string(data)
+
 		if _, ok := seen[key]; !ok {
 			out = append(out, o)
 			seen[key] = true
 		}
 	}
+
 	return out
 }
diff --git a/internal/util/util_test.go b/internal/util/util_test.go
@@ -5,6 +5,7 @@ import (
 	"reflect"
 	"testing"
 
+	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -302,3 +303,176 @@ func TestRenderTemplate(t *testing.T) {
 		})
 	}
 }
+
+func Test_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"},
+					},
+				},
+			},
+		},
+		{
+			name: "multiple custom metrics",
+			Collectors: []*troubleshootv1beta2.Collect{
+				{
+					CustomMetrics: &troubleshootv1beta2.CustomMetrics{
+						MetricRequests: []troubleshootv1beta2.MetricRequest{
+							{
+								Namespace:          "default",
+								ResourceMetricName: "pods/cpu_usage",
+							},
+						},
+					},
+				},
+				{
+					CustomMetrics: &troubleshootv1beta2.CustomMetrics{
+						MetricRequests: []troubleshootv1beta2.MetricRequest{
+							{
+								Namespace:          "default",
+								ResourceMetricName: "pods/cpu_usage",
+							},
+						},
+					},
+				},
+			},
+			want: []*troubleshootv1beta2.Collect{
+				{
+					CustomMetrics: &troubleshootv1beta2.CustomMetrics{
+						MetricRequests: []troubleshootv1beta2.MetricRequest{
+							{
+								Namespace:          "default",
+								ResourceMetricName: "pods/cpu_usage",
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "multiple secrets",
+			Collectors: []*troubleshootv1beta2.Collect{
+				{
+					Secret: &troubleshootv1beta2.Secret{
+						Name:         "my-app-postgres",
+						Namespace:    "default",
+						Key:          "uri",
+						IncludeValue: false,
+					},
+				},
+				{
+					Secret: &troubleshootv1beta2.Secret{
+						Name:         "my-app-postgres",
+						Namespace:    "default",
+						Key:          "uri",
+						IncludeValue: false,
+					},
+				},
+			},
+			want: []*troubleshootv1beta2.Collect{
+				{
+					Secret: &troubleshootv1beta2.Secret{
+						Name:         "my-app-postgres",
+						Namespace:    "default",
+						Key:          "uri",
+						IncludeValue: false,
+					},
+				},
+			},
+		},
+		{
+			name: "multiple logs",
+			Collectors: []*troubleshootv1beta2.Collect{
+				{
+					ConfigMap: &troubleshootv1beta2.ConfigMap{
+						Name:     "my-app-config",
+						Selector: []string{"app.kubernetes.io/name=nginx"},
+					},
+				},
+				{
+					ConfigMap: &troubleshootv1beta2.ConfigMap{
+						Name:     "my-app-config",
+						Selector: []string{"app.kubernetes.io/name=nginx"},
+					},
+				},
+			},
+			want: []*troubleshootv1beta2.Collect{
+				{
+					ConfigMap: &troubleshootv1beta2.ConfigMap{
+						Name:     "my-app-config",
+						Selector: []string{"app.kubernetes.io/name=nginx"},
+					},
+				},
+			},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := Dedup(tc.Collectors)
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}
diff --git a/pkg/apis/troubleshoot/v1beta2/collector_shared.go b/pkg/apis/troubleshoot/v1beta2/collector_shared.go
@@ -21,12 +21,21 @@ type ClusterInfo struct {
 	CollectorMeta `json:",inline" yaml:",inline"`
 }
 
+func (c *ClusterInfo) UniqKey() string {
+	return c.CollectorName
+}
+
 type ClusterResources struct {
 	CollectorMeta `json:",inline" yaml:",inline"`
 	Namespaces    []string `json:"namespaces,omitempty" yaml:"namespaces,omitempty"`
 	IgnoreRBAC    bool     `json:"ignoreRBAC,omitempty" yaml:"ignoreRBAC"`
 }
 
+func (c *ClusterResources) UniqKey() string {
+	// use namespace and IgnoreRBAC as key
+	return fmt.Sprintf("%s-%t", strings.Join(c.Namespaces, ","), c.IgnoreRBAC)
+}
+
 // MetricRequest the details of the MetricValuesList to be retrieved
 type MetricRequest struct {
 	// Namespace for which to collect the metric values, empty for non-namespaces resources.
@@ -59,6 +68,10 @@ type Secret struct {
 	IncludeValue  bool     `json:"includeValue,omitempty" yaml:"includeValue,omitempty"`
 }
 
+func (s *Secret) UniqKey() string {
+	return fmt.Sprintf("%s-%s-%s-%t", s.Name, strings.Join(s.Selector, ","), s.Namespace, s.IncludeValue)
+}
+
 type ConfigMap struct {
 	CollectorMeta  `json:",inline" yaml:",inline"`
 	Name           string   `json:"name,omitempty" yaml:"name,omitempty"`
@@ -69,6 +82,10 @@ type ConfigMap struct {
 	IncludeAllData bool     `json:"includeAllData,omitempty" yaml:"includeAllData,omitempty"`
 }
 
+func (c *ConfigMap) UniqKey() string {
+	return fmt.Sprintf("%s-%s-%s-%t-%t", c.Name, strings.Join(c.Selector, ","), c.Namespace, c.IncludeValue, c.IncludeAllData)
+}
+
 type LogLimits struct {
 	MaxAge    string      `json:"maxAge,omitempty" yaml:"maxAge,omitempty"`
 	MaxLines  int64       `json:"maxLines,omitempty" yaml:"maxLines,omitempty"`