diff --git a/pkg/cli/admin/mustgather/mustgather.go b/pkg/cli/admin/mustgather/mustgather.go index 491ada11c5..4729a33a96 100644 --- a/pkg/cli/admin/mustgather/mustgather.go +++ b/pkg/cli/admin/mustgather/mustgather.go @@ -41,6 +41,7 @@ import ( "k8s.io/kubectl/pkg/scheme" "k8s.io/kubectl/pkg/util/templates" admissionapi "k8s.io/pod-security-admission/api" + "k8s.io/utils/clock" "k8s.io/utils/exec" utilptr "k8s.io/utils/ptr" @@ -73,7 +74,9 @@ var ( `) mustGatherExample = templates.Examples(` - # Gather information using the default plug-in image and command, writing into ./must-gather.local. + # Gather information using the default plug-in image and command, writing into + # ./must-gather.local... + # or ./must-gather.local.. if the cluster ID is unavailable oc adm must-gather # Gather information with a specific local folder to copy to @@ -230,7 +233,7 @@ func (o *MustGatherOptions) Complete(f kcmdutil.Factory, cmd *cobra.Command, arg } } if len(o.DestDir) == 0 { - o.DestDir = fmt.Sprintf("must-gather.local.%06d", rand.Int63()) + o.DestDir = o.generateDestDir(context.TODO()) } // TODO: this should be in Validate() method, but added here because of the call to o.completeImages() below if o.AllImages { @@ -266,6 +269,46 @@ func (o *MustGatherOptions) Complete(f kcmdutil.Factory, cmd *cobra.Command, arg return nil } +// generateDestDir builds the default destination directory name for must-gather output. +// The format includes a partial cluster ID (last 12 characters), a UTC timestamp, and a +// random ID to help distinguish must-gather archives from different clusters and collection +// times. If the cluster ID cannot be retrieved (e.g. cluster is unreachable), it falls back +// to the timestamp and random ID only. +func (o *MustGatherOptions) getClock() clock.PassiveClock { + if o.clock != nil { + return o.clock + } + return clock.RealClock{} +} + +// See: https://issues.redhat.com/browse/RFE-5434 +func (o *MustGatherOptions) generateDestDir(ctx context.Context) string { + clusterIDSuffix := "" + if o.ConfigClient != nil { + lookupCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + cv, err := o.ConfigClient.ConfigV1().ClusterVersions().Get(lookupCtx, "version", metav1.GetOptions{}) + if err == nil { + id := string(cv.Spec.ClusterID) + if len(id) >= 12 { + clusterIDSuffix = id[len(id)-12:] + } else if len(id) > 0 { + clusterIDSuffix = id + } + } else { + klog.V(4).Infof("unable to retrieve cluster ID for directory name: %v", err) + } + } + + timestamp := o.getClock().Now().UTC().Format("20060102T150405Z") + randID := rand.Int63() + + if clusterIDSuffix != "" { + return fmt.Sprintf("must-gather.local.%s.%s.%06d", clusterIDSuffix, timestamp, randID) + } + return fmt.Sprintf("must-gather.local.%s.%06d", timestamp, randID) +} + func (o *MustGatherOptions) completeImages(ctx context.Context) error { for _, imageStream := range o.ImageStreams { if image, err := o.resolveImageStreamTagString(ctx, imageStream); err == nil { @@ -398,6 +441,7 @@ type MustGatherOptions struct { SinceTime string RsyncRshCmd string + clock clock.PassiveClock PrinterCreated printers.ResourcePrinter PrinterDeleted printers.ResourcePrinter diff --git a/pkg/cli/admin/mustgather/mustgather_test.go b/pkg/cli/admin/mustgather/mustgather_test.go index 358ae03c4a..4023b29ae9 100644 --- a/pkg/cli/admin/mustgather/mustgather_test.go +++ b/pkg/cli/admin/mustgather/mustgather_test.go @@ -5,6 +5,7 @@ import ( "fmt" "path" "reflect" + "strings" "testing" "time" @@ -28,6 +29,80 @@ import ( imageclient "github.com/openshift/client-go/image/clientset/versioned/fake" ) +// fakeClock implements clock.PassiveClock for deterministic tests. +type fakeClock struct { + t time.Time +} + +func (f fakeClock) Now() time.Time { return f.t } +func (f fakeClock) Since(ts time.Time) time.Duration { return f.t.Sub(ts) } + +func TestGenerateDestDir(t *testing.T) { + fixedTime := time.Date(2026, 4, 14, 12, 0, 0, 0, time.UTC) + fc := fakeClock{t: fixedTime} + + tests := []struct { + name string + clusterID string + expectedPrefix string + }{ + { + name: "with full cluster ID includes last 12 chars", + clusterID: "0194fffc-f70a-4776-b00d-76708af6b91c", + expectedPrefix: "must-gather.local.76708af6b91c.20260414T120000Z.", + }, + { + name: "with short cluster ID uses full ID", + clusterID: "abcdef", + expectedPrefix: "must-gather.local.abcdef.20260414T120000Z.", + }, + { + name: "without cluster ID falls back gracefully", + expectedPrefix: "must-gather.local.20260414T120000Z.", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var configObjects []runtime.Object + if tc.clusterID != "" { + configObjects = []runtime.Object{ + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{Name: "version"}, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(tc.clusterID), + }, + }, + } + } + options := MustGatherOptions{ + IOStreams: genericiooptions.NewTestIOStreamsDiscard(), + ConfigClient: configv1fake.NewSimpleClientset(configObjects...), + clock: fc, + } + destDir := options.generateDestDir(context.TODO()) + if !strings.HasPrefix(destDir, tc.expectedPrefix) { + t.Errorf("expected prefix %q, got %q", tc.expectedPrefix, destDir) + } + }) + } +} + +func TestGenerateDestDirNoConfigClient(t *testing.T) { + fixedTime := time.Date(2026, 4, 14, 12, 0, 0, 0, time.UTC) + fc := fakeClock{t: fixedTime} + + options := MustGatherOptions{ + IOStreams: genericiooptions.NewTestIOStreamsDiscard(), + ConfigClient: nil, + clock: fc, + } + destDir := options.generateDestDir(context.TODO()) + if !strings.HasPrefix(destDir, "must-gather.local.20260414T120000Z.") { + t.Errorf("expected prefix 'must-gather.local.20260414T120000Z.', got %q", destDir) + } +} + func TestImagesAndImageStreams(t *testing.T) { testCases := []struct {