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/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())) 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()))