CMP-3863: Added new test TestMustGatherImageWorksAsExpected to cover 75670 test from downstream by taimurhafeez · Pull Request #1023 · ComplianceAsCode/compliance-operator

taimurhafeez · 2025-12-08T15:30:34Z

This TestMustGatherImageWorksAsExpected e2e test verifies that the must-gather debugging tool works correctly for the compliance-operator.

To run it:
make e2e-serial E2E_GO_TEST_FLAGS="-v -run TestMustGatherImageWorksAsExpected"

Expected Output on OCP 4.21:

2026/03/02 15:49:31 Must-gather content verification passed
--- PASS: TestMustGatherImageWorksAsExpected (112.26s)
PASS

…t from downstream

openshift-ci-robot · 2025-12-08T15:30:38Z

@taimurhafeez: This pull request references CMP-3846 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

Details

In response to this:

To run it:
make e2e-serial E2E_GO_TEST_FLAGS="-v -run TestMustGatherImageWorksAsExpected"

Expected Output:
Must-gather content verification passed
--- PASS: TestMustGatherImageWorksAsExpected (129.04s)
PASS
Note: Must use PR-960

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2025-12-08T15:30:46Z

Hi @taimurhafeez. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-ci-robot · 2025-12-08T15:31:22Z

@taimurhafeez: This pull request references CMP-3863 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

Details

In response to this:

To run it:
make e2e-serial E2E_GO_TEST_FLAGS="-v -run TestMustGatherImageWorksAsExpected"

Expected Output:
Must-gather content verification passed
--- PASS: TestMustGatherImageWorksAsExpected (129.04s)
PASS
Note: Must use PR-960

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

rhmdnd · 2025-12-08T18:22:31Z

tests/e2e/serial/main_test.go

+
+	// Wait for the ComplianceSuite scans to complete
+	log.Printf("Waiting for ComplianceSuite scans to complete...")
+	err = f.WaitForSuiteScansStatus(f.OperatorNamespace, scanSettingBindingName, compv1alpha1.PhaseDone, compv1alpha1.ResultNonCompliant)


Potential improvement here would be to consider implementing a backwards compatible change to WaitForSuiteScansStatus to accept a list of acceptable statues. That would allow us to say "wait for NonCompliant or Inconsistent - either are fine".

rhmdnd · 2025-12-08T18:29:09Z

tests/e2e/serial/main_test.go

+		crdPath := fmt.Sprintf("%s/%s", complianceNamespaceDir, crd)
+		if _, err := os.Stat(crdPath); err == nil {
+			log.Printf("Found CRD directory: %s", crd)
+			foundCRD = true


This will only get set on the first occurrence, so if we have compliance scans but not a compliance suite, for example. Since we're using a binding, all three should certainly exist.

rhmdnd · 2025-12-08T18:29:43Z

tests/e2e/serial/main_test.go

+	if !foundCRD {
+		// Check if there are CRD directories at the parent level (timestampDir)
+		// The gather script collects CRDs across all namespaces at lines 8-20
+		log.Printf("Checking for CRD directories at the parent level...")
+		for _, crd := range complianceCRDs {
+			crdPath := fmt.Sprintf("%s/%s", timestampDir, crd)
+			if _, err := os.Stat(crdPath); err == nil {
+				log.Printf("Found CRD directory at parent level: %s", crd)
+				foundCRD = true
+			}
+		}
+	}
+
+	if !foundCRD {
+		// Check in other namespace directories (e.g., test namespace osdk-e2e-*)
+		// Scans may be created in the test namespace, not openshift-compliance
+		log.Printf("Checking for CRD directories in other namespace directories...")
+		timestampEntries, err := os.ReadDir(timestampDir)
+		if err == nil {
+			for _, entry := range timestampEntries {
+				if entry.IsDir() && entry.Name() != "openshift-compliance" && entry.Name() != "gather.logs" {
+					for _, crd := range complianceCRDs {
+						crdPath := fmt.Sprintf("%s/%s/%s", timestampDir, entry.Name(), crd)
+						if _, err := os.Stat(crdPath); err == nil {
+							log.Printf("Found CRD directory in %s: %s", entry.Name(), crd)
+							foundCRD = true
+						}
+					}
+				}
+			}
+		}
+	}
+
+	if !foundCRD {
+		// CRD directories are created by gather_compliance script at lines 8-20
+		// They may not exist if the script encounters errors or if CRDs are not found
+		// This is a warning but not a failure since core must-gather functionality works
+		t.Logf("WARNING: No compliance CRD directories found in any namespace")
+		t.Logf("This may indicate the gather script failed to collect CRD data - check gather.logs")
+		log.Printf("WARNING: No compliance CRD directories found, but core must-gather data was collected successfully")
+	}


Could these be consolidated into a single conditional?

I think it has been addressed.

rhmdnd

A few suggestions/questions inline.

xiaojiey · 2025-12-10T02:09:14Z

tests/e2e/framework/common.go

 		})
 	if createErr != nil {
-		return fmt.Errorf("failed to create Machine Config Pool %s: %w", n, createErr)
+		//return fmt.Errorf("failed to create Machine Config Pool %s: %w", n, createErr)


Remove the comment, please

xiaojiey · 2025-12-10T02:15:38Z

tests/e2e/serial/main_test.go

+	// Wait for the ComplianceSuite scans to complete
+	log.Printf("Waiting for ComplianceSuite scans to complete...")
+	err = f.WaitForSuiteScansStatus(f.OperatorNamespace, scanSettingBindingName, compv1alpha1.PhaseDone, compv1alpha1.ResultNonCompliant)
+	if err != nil {


nested if statements here. How about below if statements?

suite := &compv1alpha1.ComplianceSuite{} if suite.Status.Result != compv1alpha1.ResultNonCompliant && suite.Status.Result != compv1alpha1.ResultInconsistent { t.Fatalf("Unexpected scan result: %v", suite.Status.Result) }

xiaojiey · 2025-12-10T02:16:02Z

tests/e2e/serial/main_test.go

+			t.Fatalf("Scan did not complete successfully: %v", err)
+		}
+	}
+


Remove the empty line, please

…de clarity.

taimurhafeez · 2026-02-25T12:19:19Z

/retest

rhmdnd · 2026-02-27T22:14:48Z

This can be rebased now that #960 landed.

rhmdnd

A few suggestions, specifically for handling the CRD failure case.

rhmdnd · 2026-02-27T22:25:53Z

tests/e2e/serial/main_test.go

+
+	// The must-gather data is written directly to the timestamped directory
+	// Verify openshift-compliance namespace directory exists
+	complianceNamespaceDir := fmt.Sprintf("%s/openshift-compliance", timestampDir)


We could use f.OperatorNamespace if we pass the framework in, or at the very least pass in the namespace (especially if we don't need the whole framework object).

rhmdnd · 2026-02-27T22:27:10Z

tests/e2e/serial/main_test.go

+// Returns an error if CSV doesn't exist or doesn't have the must-gather image
+func getMustGatherImageFromCSV(f *framework.Framework) (string, error) {
+	// First, try to get the CSV name
+	csvName, err := runOCandGetOutput([]string{


Could we just use the CSV API directly instead of using oc?

rhmdnd · 2026-02-27T22:29:48Z

tests/e2e/serial/main_test.go

+	// Verify that compliance resources exist before running must-gather
+	log.Printf("Verifying compliance resources exist in namespace %s...", f.OperatorNamespace)
+	scansOutput, err := runOCandGetOutput([]string{
+		"get", "compliancescans,compliancesuites,compliancecheckresults",


We could grab these using the API like we do elsewhere in the tests, right?

rhmdnd · 2026-02-27T22:30:51Z

tests/e2e/serial/main_test.go

+		"-n", f.OperatorNamespace,
+	})
+	if err != nil {
+		log.Printf("Warning: Failed to check for compliance resources: %v", err)


If we can't verify that the CRDs exist, we might want to fail hard. I'm wondering if it would be valuable to continue running the tests in the event the CRDs aren't present, when the must-gather functionality is going to be looking for them anyway, right?

tests/e2e/serial/main_test.go

rhmdnd · 2026-02-27T22:36:55Z

tests/e2e/serial/main_test.go

+
+	// Try to get from CSV if it exists (for OLM deployments)
+	csvImage, err := getMustGatherImageFromCSV(f)
+	if err == nil && csvImage != "" {


Doesn't look like we do anything with this err if it is set to something. We could log it, but in cases where OLM isn't on the cluster, we'd always be emitting a log about that, which could get noisy.

Might be an opportunity to be more deterministic if OLM is present, and short-circuit entirely if it isn't there.

rhmdnd · 2026-02-27T22:40:43Z

tests/e2e/serial/main_test.go

+	// Execute must-gather
+	log.Printf("Executing must-gather with image %s to directory %s...", mustGatherImage, mustGatherDir)
+	_, err = runOCandGetOutput([]string{
+		"adm", "must-gather",


This makes sense for shelling out to oc since I don't think there is a client for must-gather.

openshift-ci · 2026-02-27T22:41:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: taimurhafeez

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [taimurhafeez]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2026-03-02T15:55:53Z

@taimurhafeez: This pull request references CMP-3863 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

This TestMustGatherImageWorksAsExpected e2e test verifies that the must-gather debugging tool works correctly for the compliance-operator.

To run it:
make e2e-serial E2E_GO_TEST_FLAGS="-v -run TestMustGatherImageWorksAsExpected"

Expected Output on OCP 4.21:
2026/03/02 15:49:31 Must-gather content verification passed
--- PASS: TestMustGatherImageWorksAsExpected (112.26s)
PASS

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

…cation, Detect OLM presence, Extract must-gather image from CSV , Support dynamic operator namespace, Fail early if required compliance resources are missing

taimurhafeez · 2026-03-02T16:00:06Z

A few suggestions, specifically for handling the CRD failure case.

Thanks, Lance. I have addressed most comments, except the one where I replied and asked for your suggestion.

github-actions · 2026-03-02T16:07:37Z

🤖 To deploy this PR, run the following command:

make catalog-deploy CATALOG_IMG=ghcr.io/complianceascode/compliance-operator-catalog:1023-2578b283b622ac9ebe22d383f2509df61c697546

rhmdnd

Looks like CI tripped on infrastructure issues that also affected other PRs over the weekend.

/retest

rhmdnd · 2026-03-02T17:23:30Z

tests/e2e/serial/main_test.go

+		csvItem := &csvList.Items[i]
+		if name := csvItem.GetName(); name != "" {
+			// CSVs typically have names like "compliance-operator.vX.Y.Z"
+			if len(name) >= 18 && name[:18] == "compliance-operator" {


Are we off by one here?

https://go.dev/play/p/BttP3b4ZLyE

rhmdnd · 2026-03-02T17:24:34Z

tests/e2e/serial/main_test.go

+				missingCRDs = append(missingCRDs, crd)
+			}
+		}
+		t.Logf("WARNING: Not all compliance CRD directories found. Missing: %v", missingCRDs)


If these directories are missing - don't we have a bug in the must-gather image? Shouldn't we fail the test if we hit this?

rhmdnd · 2026-03-02T17:26:04Z

/retest-required

openshift-ci · 2026-03-02T20:04:19Z

@taimurhafeez: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-rosa	`2578b28`	link	true	`/test e2e-rosa`
ci/prow/e2e-aws-serial-arm	`2578b28`	link	true	`/test e2e-aws-serial-arm`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

taimurhafeez added 2 commits December 2, 2025 10:35

temporary commit, need to revise

580d0eb

Added new test TestMustGatherImageWorksAsExpected to cover 75670 tes…

de5cfb2

…t from downstream

openshift-ci-robot added the jira/valid-reference label Dec 8, 2025

openshift-ci bot requested review from jhrozek and rhmdnd December 8, 2025 15:30

openshift-ci bot added the needs-ok-to-test label Dec 8, 2025

taimurhafeez changed the title ~~CMP-3846: Added new test TestMustGatherImageWorksAsExpected to cover 75670 test from downstream~~ CMP-3863: Added new test TestMustGatherImageWorksAsExpected to cover 75670 test from downstream Dec 8, 2025

rhmdnd added ok-to-test and removed needs-ok-to-test labels Dec 8, 2025

rhmdnd reviewed Dec 8, 2025

View reviewed changes

xiaojiey reviewed Dec 10, 2025

View reviewed changes

Addresses PR review comments by reducing duplication and improving co…

f4f00af

…de clarity.

rhmdnd requested changes Feb 27, 2026

View reviewed changes

openshift-ci bot assigned rhmdnd Feb 27, 2026

openshift-ci bot added the approved label Feb 27, 2026

Merge branch 'master' into CMP-3863

955b3f3

Use Kubernetes API client instead of oc commands for resource verifi…

2578b28

…cation, Detect OLM presence, Extract must-gather image from CSV , Support dynamic operator namespace, Fail early if required compliance resources are missing

rhmdnd requested changes Mar 2, 2026

View reviewed changes

Conversation

taimurhafeez commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Dec 8, 2025

Uh oh!

openshift-ci bot commented Dec 8, 2025

Uh oh!

openshift-ci-robot commented Dec 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhmdnd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taimurhafeez commented Feb 25, 2026

Uh oh!

rhmdnd commented Feb 27, 2026

Uh oh!

rhmdnd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Feb 27, 2026

Uh oh!

openshift-ci-robot commented Mar 2, 2026

Uh oh!

taimurhafeez commented Mar 2, 2026

Uh oh!

github-actions bot commented Mar 2, 2026

Uh oh!

rhmdnd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhmdnd commented Mar 2, 2026

Uh oh!

openshift-ci bot commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

taimurhafeez commented Dec 8, 2025 •

edited

Loading