From 67af4f0afa015451e34021d27d6acf59ae3d580c Mon Sep 17 00:00:00 2001
From: Gerard Nguyen <gerard@replicated.com>
Date: Mon, 11 Nov 2024 14:26:43 +1100
Subject: [PATCH 1/2] ensure Copy collector run last

---
 pkg/collect/collector.go     | 17 +++++++++++++++++
 pkg/supportbundle/collect.go |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/pkg/collect/collector.go b/pkg/collect/collector.go
index 310f84ff0..b8ccbf63d 100644
--- a/pkg/collect/collector.go
+++ b/pkg/collect/collector.go
@@ -286,3 +286,20 @@ func DedupCollectors(allCollectors []*troubleshootv1beta2.Collect) []*troublesho
 	}
 	return finalCollectors
 }
+
+// Ensure Copy collectors are last in the list
+// This is because copy collectors are expected to copy files from other collectors such as Exec, RunPod, RunDaemonSet
+func EnsureCopyLast(allCollectors []Collector) []Collector {
+	otherCollectors := make([]Collector, 0)
+	copyCollectors := make([]Collector, 0)
+
+	for _, collector := range allCollectors {
+		if _, ok := collector.(*CollectCopy); ok {
+			copyCollectors = append(copyCollectors, collector)
+		} else {
+			otherCollectors = append(otherCollectors, collector)
+		}
+	}
+
+	return append(otherCollectors, copyCollectors...)
+}
diff --git a/pkg/supportbundle/collect.go b/pkg/supportbundle/collect.go
index cb3f508a2..3866b2859 100644
--- a/pkg/supportbundle/collect.go
+++ b/pkg/supportbundle/collect.go
@@ -139,6 +139,9 @@ func runCollectors(ctx context.Context, collectors []*troubleshootv1beta2.Collec
 		return nil, collect.ErrInsufficientPermissionsToRun
 	}
 
+	// move Copy Collectors if any to the end of the execution list
+	allCollectors = collect.EnsureCopyLast(allCollectors)
+
 	for _, collector := range allCollectors {
 		_, span := otel.Tracer(constants.LIB_TRACER_NAME).Start(ctx, collector.Title())
 		span.SetAttributes(attribute.String("type", reflect.TypeOf(collector).String()))

From 1121e8358558a83ef7cd99b640ebdab5918437f4 Mon Sep 17 00:00:00 2001
From: Gerard Nguyen <gerard@replicated.com>
Date: Wed, 13 Nov 2024 13:39:34 +1100
Subject: [PATCH 2/2] * add unit test * reorder in Preflight as well

---
 pkg/collect/collector_test.go | 81 +++++++++++++++++++++++++++++++++++
 pkg/preflight/collect.go      |  3 ++
 2 files changed, 84 insertions(+)

diff --git a/pkg/collect/collector_test.go b/pkg/collect/collector_test.go
index eb3f33fb1..9362d4b39 100644
--- a/pkg/collect/collector_test.go
+++ b/pkg/collect/collector_test.go
@@ -445,3 +445,84 @@ func TestCollector_DedupCollectors(t *testing.T) {
 		})
 	}
 }
+func TestEnsureCopyLast(t *testing.T) {
+	tests := []struct {
+		name          string
+		allCollectors []Collector
+		want          []Collector
+	}{
+		{
+			name: "no copy collectors",
+			allCollectors: []Collector{
+				&CollectClusterInfo{},
+				&CollectClusterResources{},
+			},
+			want: []Collector{
+				&CollectClusterInfo{},
+				&CollectClusterResources{},
+			},
+		},
+		{
+			name: "only copy collectors",
+			allCollectors: []Collector{
+				&CollectCopy{},
+				&CollectCopy{},
+			},
+			want: []Collector{
+				&CollectCopy{},
+				&CollectCopy{},
+			},
+		},
+		{
+			name: "mixed collectors",
+			allCollectors: []Collector{
+				&CollectClusterInfo{},
+				&CollectCopy{},
+				&CollectClusterResources{},
+				&CollectCopy{},
+			},
+			want: []Collector{
+				&CollectClusterInfo{},
+				&CollectClusterResources{},
+				&CollectCopy{},
+				&CollectCopy{},
+			},
+		},
+		{
+			name: "copy collectors at the beginning",
+			allCollectors: []Collector{
+				&CollectCopy{},
+				&CollectCopy{},
+				&CollectClusterInfo{},
+				&CollectClusterResources{},
+			},
+			want: []Collector{
+				&CollectClusterInfo{},
+				&CollectClusterResources{},
+				&CollectCopy{},
+				&CollectCopy{},
+			},
+		},
+		{
+			name: "copy collectors at the end",
+			allCollectors: []Collector{
+				&CollectClusterInfo{},
+				&CollectClusterResources{},
+				&CollectCopy{},
+				&CollectCopy{},
+			},
+			want: []Collector{
+				&CollectClusterInfo{},
+				&CollectClusterResources{},
+				&CollectCopy{},
+				&CollectCopy{},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := EnsureCopyLast(tt.allCollectors)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
diff --git a/pkg/preflight/collect.go b/pkg/preflight/collect.go
index 115c8bf35..d6ffeefc2 100644
--- a/pkg/preflight/collect.go
+++ b/pkg/preflight/collect.go
@@ -234,6 +234,9 @@ func CollectWithContext(ctx context.Context, opts CollectOpts, p *troubleshootv1
 		return collectResult, collect.ErrInsufficientPermissionsToRun
 	}
 
+	// move Copy Collectors if any to the end of the execution list
+	allCollectors = collect.EnsureCopyLast(allCollectors)
+
 	for i, collector := range allCollectors {
 		_, span := otel.Tracer(constants.LIB_TRACER_NAME).Start(ctx, collector.Title())
 		span.SetAttributes(attribute.String("type", reflect.TypeOf(collector).String()))