CMP-3863: Added new test TestMustGatherImageWorksAsExpected to cover 75670 test from downstream #1023
Conversation
|
@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. DetailsIn response to this:
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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: taimurhafeez The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
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 Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
taimurhafeez
changed the title
|
@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. DetailsIn response to this:
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. |
tests/e2e/serial/main_test.go
Outdated
|
|
||
| // 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) |
There was a problem hiding this comment.
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".
tests/e2e/serial/main_test.go
Outdated
| crdPath := fmt.Sprintf("%s/%s", complianceNamespaceDir, crd) | ||
| if _, err := os.Stat(crdPath); err == nil { | ||
| log.Printf("Found CRD directory: %s", crd) | ||
| foundCRD = true |
There was a problem hiding this comment.
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.
tests/e2e/serial/main_test.go
Outdated
| 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") | ||
| } |
There was a problem hiding this comment.
Could these be consolidated into a single conditional?
rhmdnd
left a comment
rhmdnd
left a comment
There was a problem hiding this comment.
A few suggestions/questions inline.
tests/e2e/framework/common.go
Outdated
| }) | ||
| 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) |
There was a problem hiding this comment.
Remove the comment, please
| // 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 { |
There was a problem hiding this comment.
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)
}
tests/e2e/serial/main_test.go
Outdated
| t.Fatalf("Scan did not complete successfully: %v", err) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Remove the empty line, please
|
@taimurhafeez: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
4 participants
To run it:
make e2e-serial E2E_GO_TEST_FLAGS="-v -run TestMustGatherImageWorksAsExpected"Expected Output:
Note: Must use PR-960