ci(nightly): Always display Operator logs at the end of the E2E tests

[rm3l]

Description

Follow-up to #1650 to help the SL team understand what's going on..

Which issue(s) does this PR fix or relate to

• Relates to https://issues.redhat.com/browse/RHDHBUGS-2050

PR acceptance criteria

• Tests
• Documentation

How to test changes / Special notes to the reviewer

Summary by Sourcery

Enhance the E2E test suite to always capture and display operator logs at the end of the run

Enhancements:

• Record test start time to enable time-based log retrieval
• Defer operator uninstallation and print operator logs in the SynchronizedAfterSuite
• Extract getControllerPodName helper and refactor verifyControllerUp to use it
• Refactor getPodLogs to accept optional pod name or label and include a --since-time filter
• Update fetchOperatorLogs and fetchOperandLogs to leverage the new log retrieval logic and handle errors

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign gazarenkov for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 RHDHBUGS-2050 - Partially compliant Compliant requirements: None Non-compliant requirements: E2E tests must use the Sealights-instrumented operator image: quay.io/rhdh/rhdh-rhel9-operator-sealights instead of quay.io/rhdh/rhdh-rhel9-operator Requires further human verification: Ensure Sealights coverage is properly reported on dashboards Confirm with SL team that logs and timing changes aid their investigation

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue In verifyControllerUp, the kubectl/oc command construction appears malformed due to missing arguments in exec.Command; ensure the full argument list (including "get") is passed when querying pod status. // Validate pod status cmd := exec.Command(helper.GetPlatformTool(), "get", "pods", controllerPodName, "-o", "jsonpath={.status.phase}", "-n", _namespace, ) status, err := helper.Run(cmd) g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(string(status)).Should(Equal("Running"), fmt.Sprintf("controller pod in %s status", status)) Flakiness Risk getControllerPodName returns the first pod without ensuring uniqueness or readiness; consider filtering by phase/ready condition or asserting exactly one active controller to avoid race conditions. func getControllerPodName() (string, error) { cmd := exec.Command(helper.GetPlatformTool(), "get", "pods", "-l", managerPodLabel, "-o", "go-template={{ range .items }}{{ if not .metadata.deletionTimestamp }}{{ .metadata.name }}"+ "{{ \"\\n\" }}{{ end }}{{ end }}", "-n", _namespace, ) podOutput, err := helper.Run(cmd) if err != nil { return "", err } podNames := helper.GetNonEmptyLines(string(podOutput)) if len(podNames) == 0 { return "", fmt.Errorf("no pods found") } return podNames[0], nil } Log Collection Robustness fetchOperatorLogs now prefers a specific pod; if the controller pod restarts, since-time uses test start and may miss earlier crashes; consider --since or including previous logs (-p) and fallback to label selector if pod retrieval fails. func fetchOperatorLogs(managerPodLabel string, raw bool) func() string { return func() string { var logs string controllerPodName, err := getControllerPodName() if err != nil { logs = fmt.Sprintf("Failed to get controller pod name: %v", err) } else { logs = getPodLogs(_namespace, controllerPodName, managerPodLabel) } if raw { return logs } return fmt.Sprintf("=== Operator logs ===\n%s\n", logs) } }

📄 References No matching references available

[qodo-code-review]

PR Type

Tests

---

Description

• Add operator logs display at end of E2E tests

• Improve log fetching with time filtering and error handling

• Refactor pod name retrieval for better reliability

---

File Walkthrough

	Relevant files
Tests	e2e_suite_test.go Enhanced E2E test logging and cleanup                                        tests/e2e/e2e_suite_test.go Add global _start variable to track test execution start time Modify SynchronizedAfterSuite to display operator logs before cleanup Refactor getPodLogs to accept pod name and add time filtering Extract getControllerPodName function with proper error handling +41/-12

---

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• Use GinkgoWriter instead of fmt.Println to ensure operator logs are captured in the Ginkgo output.
• Extract the common log-fetching logic in fetchOperatorLogs and fetchOperandLogs into a shared helper to reduce duplication.
• Add a guard in getPodLogs for the case when both podName and label are empty to prevent invalid kubectl/oc invocations.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Use GinkgoWriter instead of fmt.Println to ensure operator logs are captured in the Ginkgo output.
- Extract the common log-fetching logic in fetchOperatorLogs and fetchOperandLogs into a shared helper to reduce duplication.
- Add a guard in getPodLogs for the case when both podName and label are empty to prevent invalid kubectl/oc invocations.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Restore validation for a single pod In getControllerPodName, restore the validation to ensure exactly one controller pod is found, not just at least one, to prevent tests from passing incorrectly when multiple pods are running. tests/e2e/e2e_suite_test.go [237-241] podNames := helper.GetNonEmptyLines(string(podOutput)) -if len(podNames) == 0 { - return "", fmt.Errorf("no pods found") +if len(podNames) != 1 { + return "", fmt.Errorf("expected 1 controller pod, found %d", len(podNames)) } return podNames[0], nil Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a regression where the test no longer validates that exactly one controller pod is running, which could mask deployment issues and lead to incorrect test passes.	Medium
	Reintroduce pod existence check in verification In verifyControllerUp, add a check to ensure the controllerPodName is not empty after calling getControllerPodName. tests/e2e/e2e_suite_test.go [244-256] func verifyControllerUp(g Gomega, managerPodLabel string) { controllerPodName, err := getControllerPodName() g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(controllerPodName).ShouldNot(BeEmpty()) // Validate pod status cmd := exec.Command(helper.GetPlatformTool(), "get", "pods", controllerPodName, "-o", "jsonpath={.status.phase}", "-n", _namespace, ) status, err := helper.Run(cmd) g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(string(status)).Should(Equal("Running"), fmt.Sprintf("controller pod in %s status", status)) } Apply / Chat Suggestion importance[1-10]: 2 __ Why: While the suggestion correctly identifies the missing validation for a single pod, the proposed change to check if controllerPodName is not empty is redundant, as getControllerPodName already returns an error if no pods are found.	Low
More

[rm3l]

Merging to trigger a nightly build and share the output with the SL team.