oc adm must-gather should fail when specified image cannot be pulled

oc adm must-gather should fail when specified image cannot be pulled

Problem Statement

When customers run oc adm must-gather --image=<overlay-product-image> and the specified image cannot be pulled, the command currently:

1. ✅ Prints an error message to stderr
2. ⚠️ Falls back to oc adm inspect collecting generic cluster data
3. ⚠️ Creates an output directory with fallback data
4. ⚠️ Exits with error but after creating output

This creates a support workflow problem:

• Customers see the output directory created and assume collection succeeded
• Error messages scroll away in 50+ lines of "Gathering data for ns/..." output
• Customers upload the wrong (generic) must-gather to support cases
• Support engineers have to request the correct overlay product must-gather
• Result: 1-2 wasted case iterations, delayed problem resolution

---

Real-World Impact

Common scenario:

1. Customer needs ODF/Kubevirt/ACM specific must-gather
2. Runs: oc adm must-gather --image=quay.io/ocs-dev/ocs-must-gather:latest
3. Image name typo or auth issue → image pull fails
4. Command falls back and collects generic namespace data
5. Customer sees must-gather.local.XXX/ directory created
6. Customer uploads to support case
7. ODF support engineer: "This is generic cluster data, not ODF must-gather!"
8. Support asks customer to re-collect with correct image
9. Days lost in back-and-forth

This happens regularly with overlay products:

• OpenShift Data Foundation (ODF)
• KubeVirt / OpenShift Virtualization
• Advanced Cluster Management (ACM)
• OpenShift Logging
• OpenShift Service Mesh
• Any operator with custom must-gather image

---

Current Behavior

Test case:

$ oc adm must-gather --image=quay.io/nonexistent/bad-image:latest

Output:

[must-gather] gather did not start: unable to pull image: ImagePullBackOff...

Error running must-gather collection:
    gather did not start for pod must-gather-xxx: unable to pull image...

Falling back to `oc adm inspect clusterversion.v1.config.openshift.io,clusteroperators.v1.config.openshift.io`
[must-gather] Gathering data for ns/openshift-authentication...
[must-gather] Gathering data for ns/openshift-monitoring...
[must-gather] Gathering data for ns/openshift-etcd...
... (50+ namespaces)

Wrote inspect data to must-gather.local.5414330615473311500/

error: gather did not start for pod must-gather-xxx: unable to pull image...

Problems:

• ✅ Error IS shown (twice)
• ❌ But error gets buried in output noise
• ❌ Output directory IS created with fallback data
• ❌ Fallback data is NOT what customer intended to collect
• ❌ Customers miss the error and upload wrong data

---

Proposed Solution

When --image is explicitly specified and image pull fails:

Fail immediately without fallback

$ oc adm must-gather --image=quay.io/nonexistent/bad-image:latest

Using must-gather plug-in image: quay.io/nonexistent/bad-image:latest

ERROR: Unable to pull must-gather image 'quay.io/nonexistent/bad-image:latest'
ERROR: ImagePullBackOff: unauthorized: access to the requested resource is not authorized

Must-gather collection FAILED. No data was collected.

When you specify --image, that specific image is required.
The command will not fall back to collecting generic cluster data.

Please verify:
  1. Image name and tag are correct
  2. Registry is accessible from the cluster
  3. Image pull secrets are configured if needed

To collect general cluster logs instead, run:
  oc adm must-gather

Exit code: 1 (no output directory created)

Benefits:

• ✅ Customer immediately knows collection failed
• ✅ No misleading output directory created
• ✅ Customer fixes the image issue before opening support case
• ✅ Saves support iteration time
• ✅ Clear guidance on what to do next

---

Rationale

Why hard fail is correct:

When a customer specifies --image=X, they have explicit intent to collect data using that specific image. The fallback violates that intent:

Customer Intent	Current Behavior	Correct Behavior
"Collect ODF must-gather"	Collects generic cluster data	❌ Fail - wrong data
"Use this specific diagnostic tool"	Uses different tool silently	❌ Fail - wrong tool
"Run operator-specific scripts"	Runs generic oc get commands	❌ Fail - wrong scripts

Analogy: If you run gcc myfile.c and gcc isn't found, you don't want the system to silently fall back to a different compiler.

When fallback makes sense:

The current fallback behavior could be preserved for:

• ❌ NOT when --image is specified (explicit intent to use that image)
• ✅ Maybe for --all-images flag (best-effort collection from multiple sources)

---

Implementation Suggestion

Location: pkg/cli/admin/mustgather/mustgather.go

Current code (lines 683-690):

runBackCollection := true
defer func() {
    if ctx.Err() != nil || !runBackCollection {
        return
    }
    o.BackupGathering(ctx, errs)  // Runs fallback collection
}()

Proposed change:

// Only run backup collection if no specific images were requested by user
// If user specified --image, they want THAT image's data, not generic fallback
runBackCollection := len(o.ImagesUserSpecified) == 0  // Track user-specified vs auto-detected images

defer func() {
    if ctx.Err() != nil || !runBackCollection {
        return
    }
    
    if len(o.ImagesUserSpecified) > 0 {
        // User specified explicit images - fail hard, don't fall back
        fmt.Fprintf(o.ErrOut, "\nERROR: Must-gather collection failed for specified image(s).\n")
        fmt.Fprintf(o.ErrOut, "ERROR: Cannot collect overlay product must-gather data.\n")
        fmt.Fprintf(o.ErrOut, "\nWhen --image is specified, that specific image is required.\n")
        fmt.Fprintf(o.ErrOut, "The command will not fall back to generic cluster data collection.\n\n")
        fmt.Fprintf(o.ErrOut, "Please verify the image name, registry access, and try again.\n")
        return  // Exit without running BackupGathering()
    }
    
    // No specific images - fallback is acceptable
    o.BackupGathering(ctx, errs)
}()

Note: Would need to track which images came from --image flag vs auto-detected (like from --all-images).

---

Alternative Solutions Considered

Option 1: Add --strict flag

oc adm must-gather --image=X --strict  # Fail on image pull error

Rejected because:

• ❌ Customers won't know to use --strict
• ❌ Requires reading documentation
• ❌ Wrong behavior should not be the default

Option 2: Create WARNING file in output

Create INCOMPLETE_COLLECTION_WARNING.txt in output directory.

Rejected because:

• ❌ Customers don't read files before uploading
• ❌ Still creates misleading output directory
• ❌ Doesn't prevent the workflow problem

Option 3: Rename output directory

must-gather.local.XXX-INCOMPLETE-FALLBACK/

Rejected because:

• ❌ Customers may not notice directory name
• ❌ Still creates output that shouldn't exist
• ❌ Doesn't prevent uploads

---

Backward Compatibility

Breaking change consideration:

This changes behavior when --image is specified and fails. However:

✅ Not a breaking change for correct usage:

• If image pull succeeds → behavior unchanged
• If no --image specified → behavior unchanged

⚠️ Changes behavior for error case:

• Currently: Create fallback data on image pull failure
• Proposed: Exit with error, no data collected

Impact:

• Anyone relying on fallback data when image fails would be affected
• But this is actually incorrect usage - they should fix their image
• Proper fix: Use correct image name or run without --image flag

Migration path for users who relied on fallback:

# Old (relied on fallback):
oc adm must-gather --image=broken-image  # Would fall back

# New (explicit choice):
oc adm must-gather --image=correct-image  # Fix the image
# OR
oc adm must-gather                        # Collect generic data explicitly

---

Testing

Test cases to add:

1. Image pull fails with --image specified:

   • Should: Exit with error, no output directory created
   • Should: Print clear error message with guidance

2. Image pull succeeds:

   • Should: Behavior unchanged (collect data normally)

3. No --image specified, default image fails:

   • Should: Could still fall back (or fail - depends on design decision)

4. Multiple --image specified, one fails:

   • Should: Fail for all if any explicit image fails

---

Related Issues

• Similar fallback confusion exists in other OpenShift tools
• Customers often don't notice stderr messages
• Support case workflows rely on correct data collection

---

Proposed PR Checklist

• Track user-specified vs auto-detected images
• Skip BackupGathering() when user specified --image
• Print clear error message when failing hard
• Add test cases for image pull failure scenarios
• Update oc adm must-gather --help documentation
• Update must-gather.md documentation

---

Additional Context

Reporter: Red Hat Support Engineer
Frequency: Happens regularly with overlay product support cases
Customer Impact: High - causes case resolution delays
Support Impact: High - wastes 1-2 case iterations per occurrence

Example overlay products affected:

• OpenShift Data Foundation (ODF)
• OpenShift Virtualization (KubeVirt)
• Advanced Cluster Management (ACM)
• Red Hat OpenShift Logging
• Red Hat OpenShift Service Mesh
• Migration Toolkit for Containers (MTC)
• Any operator with custom must-gather image

---

Requested Behavior Summary

IF --image flag was used by customer
  AND image pull fails
THEN
  Print clear error
  Do NOT create output directory
  Do NOT run fallback collection
  Exit with non-zero code
  
IF no --image flag (using auto-detected default)
  AND image pull fails  
THEN
  Current fallback behavior is acceptable (or also fail - design decision)

Key principle: Respect customer's explicit intent when --image is specified.

Start in pkg/cli/admin/mustgather/mustgather.go around the runBackCollection defer at lines 683-690, and trace how explicitly supplied images are distinguished from automatic images. Review the existing must-gather tests and add coverage for failed explicit image pulls, successful pulls, and multiple images. Done means explicit pull failures return an error without fallback data or an output directory, while unaffected cases retain their behavior.