Skip to content

test(e2e): add test-e2e-from-ci-bundle target to Makefile - #3378

Open
vparfonov wants to merge 1 commit into
openshift:masterfrom
vparfonov:log9674
Open

test(e2e): add test-e2e-from-ci-bundle target to Makefile#3378
vparfonov wants to merge 1 commit into
openshift:masterfrom
vparfonov:log9674

Conversation

@vparfonov

@vparfonov vparfonov commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

/cc @Clee2691
/assign @jcantrill

Links

Summary by CodeRabbit

  • Tests

    • Added a CI-compatible end-to-end test runner with standardized logging, tracing, timeouts, and test configuration.
    • Improved container security checks to handle unavailable mount paths without producing misleading failures.
  • Chores

    • Added a Make target for running end-to-end tests from a CI bundle and generating test reports.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a8522a48-7250-48b4-987d-b48b3b78c2b8

📥 Commits

Reviewing files that changed from the base of the PR and between ef008cd and 70e56d8.

📒 Files selected for processing (3)
  • Makefile
  • hack/test-e2e-from-ci-bundle.sh
  • test/e2e/collection/security/container_security_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • Makefile
  • hack/test-e2e-from-ci-bundle.sh

📝 Walkthrough

Walkthrough

The PR adds a Makefile target and Bash runner for CI E2E tests. It configures namespace labels and test options. Container security checks now skip unavailable mount paths.

Changes

CI E2E execution

Layer / File(s) Summary
CI E2E target and runner
Makefile, hack/test-e2e-from-ci-bundle.sh
The Makefile target provides E2E images and logging settings. The runner resolves the repository root, labels the namespace, and runs the E2E suite with configured Ginkgo options.
Unavailable mount handling
test/e2e/collection/security/container_security_test.go
The mount check logs and skips the read-only assertion when touch reports No such file or directory.

Estimated code review effort: 2 (Simple) | ~15 minutes

Suggested reviewers: jcantrill, clee2691, cahartma

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The mandatory Description section is empty, although reviewer, approver, and JIRA link requirements are satisfied. Add the change intent, problem context, rationale, and high-level implementation summary to the Description section.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding the CI bundle E2E Makefile target.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@qodo-for-rh-openshift

qodo-for-rh-openshift Bot commented Jul 29, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Add CI-bundle e2e test Make target with CI-friendly runner flags

🧪 Tests ✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Add a dedicated Makefile target to run Go/Ginkgo e2e tests from the CI bundle.
• Introduce a lightweight runner script with verbose, trace, and extended timeout settings.
• Harden two e2e assertions to reduce CI flakiness across environment differences.
Diagram

graph TD
  A["CI job / Developer"] --> B["make test-e2e-from-ci-bundle"] --> C["hack/test-e2e-from-ci-bundle.sh"] --> D["oc label openshift-logging"] --> E["go test ./test/e2e/..."] --> F["Ginkgo e2e suites"] --> G[(OpenShift cluster)]
  subgraph Legend
    direction LR
    _usr["Invoker"] ~~~ _tool["Script/Tooling"] ~~~ _db[("Cluster")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Extend existing e2e runner (hack/test-e2e-olm.sh) with a mode/flag
  • ➕ Reduces duplication of e2e entrypoints and keeps behavior centralized
  • ➕ Makes it easier to share artifact collection, cleanup, and common env setup
  • ➖ More invasive change; risk of impacting existing OLM e2e flow
  • ➖ Requires designing a stable interface (flags/env) and updating CI callers
2. Inline the go test invocation directly in the Makefile target
  • ➕ One fewer script to maintain
  • ➕ Keeps the execution path obvious for contributors
  • ➖ Harder to evolve (shell logic in Make recipes is less readable)
  • ➖ Less reusable for CI steps that want to call the runner directly
3. Adopt a common test harness wrapper (single hack/test-e2e.sh) used by all e2e targets
  • ➕ Clear, consistent entrypoint for all e2e styles (OLM, CI bundle, local)
  • ➕ Easier to add shared defaults (timeouts, NO_COLOR, progress flags)
  • ➖ Bigger refactor than needed for the immediate LOG-9674 goal
  • ➖ May require coordinating changes across CI definitions and docs

Recommendation: The current approach (a dedicated target + minimal runner script) is appropriate for quickly enabling CI-bundle e2e execution without perturbing the existing OLM-based runner. If this target becomes the preferred CI path, consider consolidating runners (or factoring shared setup like labeling, timeouts, and output settings) to avoid drift between e2e entrypoints.

Files changed (4) +18 / -2

Tests (3) +11 / -2
test-e2e-from-ci-bundle.shNew CI-friendly e2e runner script (verbose, trace, longer timeout) +8/-0

New CI-friendly e2e runner script (verbose, trace, longer timeout)

• Adds a small bash runner that labels the openshift-logging namespace for cluster monitoring and runs 'go test ./test/e2e/...' with NO_COLOR, verbose output, Ginkgo trace, and poll progress settings tuned for CI visibility.

hack/test-e2e-from-ci-bundle.sh

container_security_test.goLoosen read-only mount assertion to tolerate mount differences +1/-1

Loosen read-only mount assertion to tolerate mount differences

• Updates the expected 'touch' failure regex to accept either 'Read-only file system' or 'No such file or directory', improving robustness across environments where the mount path may be absent instead of read-only.

test/e2e/collection/security/container_security_test.go

input_selection_test.goAdjust excluded-namespace assertion to avoid brittle empty-list check +2/-1

Adjust excluded-namespace assertion to avoid brittle empty-list check

• Changes the verification from expecting zero namespaces to asserting that no collected namespaces match the excluded '^clo-test' pattern, preventing false failures when unrelated namespaces still produce logs.

test/e2e/input_selection/input_selection_test.go

Other (1) +7 / -0
MakefileAdd test-e2e-from-ci-bundle target to run CI-bundle e2e +7/-0

Add test-e2e-from-ci-bundle target to run CI-bundle e2e

• Introduces a new phony target, 'test-e2e-from-ci-bundle', that exports the same RELATED_IMAGE/IMAGE variables as other e2e targets and calls a dedicated runner script with LOG_LEVEL=3.

Makefile

@openshift-ci
openshift-ci Bot requested review from Clee2691 and cahartma July 29, 2026 16:33
@openshift-ci

openshift-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vparfonov
Once this PR has been reviewed and has the lgtm label, please assign alanconway 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:

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@hack/test-e2e-from-ci-bundle.sh`:
- Line 1: Update the executable permission metadata for
hack/test-e2e-from-ci-bundle.sh from 100644 to 100755 so the Makefile target can
invoke it directly in CI.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9813f581-ce36-4793-b830-ee14b65f7461

📥 Commits

Reviewing files that changed from the base of the PR and between d3b6f7a and 1ef526c.

📒 Files selected for processing (2)
  • Makefile
  • hack/test-e2e-from-ci-bundle.sh

Comment thread hack/test-e2e-from-ci-bundle.sh
@qodo-for-rh-openshift

qodo-for-rh-openshift Bot commented Jul 29, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
✅ Compliance rules (platform): 9 rules

Grey Divider


Action required

1. Missing TLS scanner image ✓ Resolved 🐞 Bug ≡ Correctness
Description
test-e2e-from-ci-bundle runs go test ... test/e2e/... but does not set IMAGE_TLS_SCANNER, and
the TLS e2e suite exits when that env var is unset. This makes the new Make target fail whenever the
TLS suite executes without an externally-provided image.
Code

Makefile[R327-331]

+test-e2e-from-ci-bundle: $(JUNITREPORT)
+	RELATED_IMAGE_VECTOR=$(IMAGE_LOGGING_VECTOR) \
+	RELATED_IMAGE_LOG_FILE_METRIC_EXPORTER=$(IMAGE_LOGFILEMETRICEXPORTER) \
+	IMAGE_LOGGING_EVENTROUTER=$(IMAGE_LOGGING_EVENTROUTER) \
+	LOG_LEVEL=3 hack/test-e2e-from-ci-bundle.sh
Relevance

●●● Strong

Deterministic e2e failure: TLS suite requires IMAGE_TLS_SCANNER; missing env breaks new Make target.

PR-#3269

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new target runs all e2e packages but omits the TLS scanner image env var; the TLS scanner code
hard-exits when that env var is missing and the TLS e2e suite invokes the scanner during tests.

Makefile[326-339]
hack/test-e2e-from-ci-bundle.sh[1-7]
test/framework/e2e/tls/scanner.go[54-60]
test/e2e/operator/tls/e2e_test.go[61-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The new `test-e2e-from-ci-bundle` Make target executes all Go e2e suites (`test/e2e/...`) but does not provide the `IMAGE_TLS_SCANNER` environment variable. The TLS e2e suite calls `tlsscanner.GetImage()` which exits the process if `IMAGE_TLS_SCANNER` is unset, causing this target to fail.

### Issue Context
The existing `test-e2e` target already passes `IMAGE_TLS_SCANNER=$(IMAGE_TLS_SCANNER)` for the same overall e2e surface area.

### Fix Focus Areas
- Makefile[326-339]

### Suggested fix
Add the missing env var to `test-e2e-from-ci-bundle`, matching `test-e2e`, e.g.:
```make
  IMAGE_TLS_SCANNER=$(IMAGE_TLS_SCANNER) \
```
Alternatively, if the intent is to not run TLS scanning in this target, adjust the script/package selection to exclude `test/e2e/operator/tls/...` explicitly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Setup errors masked ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New
Description
hack/test-e2e-from-ci-bundle.sh does not enable strict error handling, so a failing oc label ...
can be ignored and the script exit code will reflect only the subsequent go test result. This can
silently skip required cluster setup while still reporting success.
Code

hack/test-e2e-from-ci-bundle.sh[R3-5]

+oc label ns/openshift-logging openshift.io/cluster-monitoring=true --overwrite
+
+GOFLAGS=-mod=mod NO_COLOR=1 go test -v -timeout=90m ./test/e2e/... \
Relevance

●●● Strong

Strict bash mode prevents silently skipping required setup; low-risk reliability fix.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new script runs oc label ... but does not use set -euo pipefail, meaning failures in that
setup step can be ignored; other repo scripts demonstrate the established pattern of strict mode.

hack/test-e2e-from-ci-bundle.sh[1-8]
hack/build-image.sh[1-6]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`hack/test-e2e-from-ci-bundle.sh` runs a required cluster setup step (`oc label ...`) but does not use strict bash error handling. If the label command fails (missing namespace, permissions, transient API error), the script can still continue and exit successfully if `go test` passes.

### Issue Context
Other `hack/*.sh` scripts in this repo consistently enable strict error handling (`set -euo pipefail`) to prevent masking setup failures.

### Fix Focus Areas
- hack/test-e2e-from-ci-bundle.sh[1-8]

### Suggested fix
- Add `set -euo pipefail` after the shebang.
- Optionally add a brief error message around the `oc label` step so failures are obvious in CI logs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Script assumes repo root ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New
Description
hack/test-e2e-from-ci-bundle.sh runs go test ... ./test/e2e/... relative to the current working
directory and never cds to the repository root. Running the script from another directory (common
in CI wrappers or manual use) will fail to resolve the package path.
Code

hack/test-e2e-from-ci-bundle.sh[R5-8]

+GOFLAGS=-mod=mod NO_COLOR=1 go test -v -timeout=90m ./test/e2e/... \
+   -ginkgo.v -ginkgo.trace \
+   -ginkgo.poll-progress-after=300s \
+   -ginkgo.poll-progress-interval=30s
Relevance

●●● Strong

Ensuring scripts run from any CWD via repo-root cd is a low-risk reliability improvement.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script uses a repo-relative Go package path without establishing a repo root CWD; other hack
scripts show the repo-wide convention of deriving a repo directory for robust execution.

hack/test-e2e-from-ci-bundle.sh[5-8]
hack/benchmark-collector.sh[3-6]
hack/test-e2e-olm.sh[1-13]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The script executes `go test ... ./test/e2e/...` using a relative path but does not ensure it is running from the repository root. Invoking it from a different working directory will break.

### Issue Context
Other scripts under `hack/` compute `repo_dir` from `BASH_SOURCE` and then reference repo-relative paths, making them safe to run from any CWD.

### Fix Focus Areas
- hack/test-e2e-from-ci-bundle.sh[1-8]

### Suggested fix
- Compute `repo_dir` (e.g., `repo_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)`), then `cd "$repo_dir"` before running `go test`.
- Keep the package argument as `./test/e2e/...` (after `cd`) or switch to an absolute path based on `repo_dir`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Mount check too permissive 🐞 Bug ≡ Correctness ⭐ New
Description
The read-only mount assertion now accepts "No such file or directory", which allows the test to pass
even when required /var/log/* paths are missing rather than mounted read-only. This weakens the
test’s ability to detect missing/mis-mounted log directories.
Code

test/e2e/collection/security/container_security_test.go[39]

+			Expect(result).To(MatchRegexp("touch:.cannot.*touch.*" + touchFile + ".*(Read-only file system|No such file or directory)"))
Relevance

●● Moderate

May intentionally reduce flakes but weakens correctness; no close historical precedent found.

PR-#3269

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The matcher change broadens acceptable failures for touch, and the same helper is used to validate
/ and multiple declared "needed" /var/log subdirectories.

test/e2e/collection/security/container_security_test.go[36-41]
test/e2e/collection/security/container_security_test.go[135-141]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`checkMountReadOnly()` is meant to verify mounts are read-only, but the updated regex treats missing paths (`No such file or directory`) as a valid outcome. That can hide regressions where expected `/var/log/...` directories are not present/mounted.

### Issue Context
The helper is used for `/` and for multiple `/var/log/*` subdirectories described as "needed".

### Fix Focus Areas
- test/e2e/collection/security/container_security_test.go[36-41]
- test/e2e/collection/security/container_security_test.go[135-141]

### Suggested fix
- For paths that must exist (at least `/` and any "needed" `/var/log/*` dirs), first assert the directory exists (e.g., `test -d <path>`), then assert the touch failure is specifically `Read-only file system`.
- If some `/var/log/*` paths are truly optional, explicitly list those as optional and keep the strict read-only assertion for the required ones.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (1)
5. Exclusion test can pass empty 🐞 Bug ≡ Correctness ⭐ New
Description
The excluded-namespace assertion no longer checks that any logs were collected from allowed
namespaces, so an empty receiver.ListNamespaces() will pass even if collection is completely
broken. This reduces the test’s ability to catch regressions in input selection or collection.
Code

test/e2e/input_selection/input_selection_test.go[R236-237]

+				Expect(receiver.ListNamespaces()).ToNot(ContainElement(MatchRegexp("^clo-test")),
+					"exp no logs from excluded namespaces")
Relevance

●● Moderate

Could be fixing a flaky test or weakening coverage; intent unclear from history.

PR-#3251

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed test only asserts absence of clo-test* namespaces and does not require any logs;
nearby table entries demonstrate the established pattern of asserting non-empty collections first.

test/e2e/input_selection/input_selection_test.go[224-238]
test/e2e/input_selection/input_selection_test.go[190-193]
test/e2e/input_selection/input_selection_test.go[206-209]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The exclusion case only asserts that no namespace matching `^clo-test` appears. If `receiver.ListNamespaces()` is empty (e.g., no logs collected at all), the test passes, masking failures.

### Issue Context
Other entries in the same `DescribeTable` first assert `namespaces` is non-empty before asserting patterns.

### Fix Focus Areas
- test/e2e/input_selection/input_selection_test.go[224-238]
- test/e2e/input_selection/input_selection_test.go[190-193]

### Suggested fix
- Capture `namespaces := receiver.ListNamespaces()`.
- Add `Expect(namespaces).ToNot(BeEmpty(), "Exp. to collect some logs")`.
- Keep the exclusion assertion: `Expect(namespaces).ToNot(ContainElement(MatchRegexp("^clo-test")))` (and optionally assert expected allowed namespaces are present).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Previous review results

Review updated until commit 70e56d8

Results up to commit 1ef526c ⚖️ Balanced


🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. Missing TLS scanner image 🐞 Bug ≡ Correctness
Description
test-e2e-from-ci-bundle runs go test ... test/e2e/... but does not set IMAGE_TLS_SCANNER, and
the TLS e2e suite exits when that env var is unset. This makes the new Make target fail whenever the
TLS suite executes without an externally-provided image.
Code

Makefile[R327-331]

+test-e2e-from-ci-bundle: $(JUNITREPORT)
+	RELATED_IMAGE_VECTOR=$(IMAGE_LOGGING_VECTOR) \
+	RELATED_IMAGE_LOG_FILE_METRIC_EXPORTER=$(IMAGE_LOGFILEMETRICEXPORTER) \
+	IMAGE_LOGGING_EVENTROUTER=$(IMAGE_LOGGING_EVENTROUTER) \
+	LOG_LEVEL=3 hack/test-e2e-from-ci-bundle.sh
Relevance

●●● Strong

Deterministic e2e failure: TLS suite requires IMAGE_TLS_SCANNER; missing env breaks new Make target.

PR-#3269

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new target runs all e2e packages but omits the TLS scanner image env var; the TLS scanner code
hard-exits when that env var is missing and the TLS e2e suite invokes the scanner during tests.

Makefile[326-339]
hack/test-e2e-from-ci-bundle.sh[1-7]
test/framework/e2e/tls/scanner.go[54-60]
test/e2e/operator/tls/e2e_test.go[61-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The new `test-e2e-from-ci-bundle` Make target executes all Go e2e suites (`test/e2e/...`) but does not provide the `IMAGE_TLS_SCANNER` environment variable. The TLS e2e suite calls `tlsscanner.GetImage()` which exits the process if `IMAGE_TLS_SCANNER` is unset, causing this target to fail.

### Issue Context
The existing `test-e2e` target already passes `IMAGE_TLS_SCANNER=$(IMAGE_TLS_SCANNER)` for the same overall e2e surface area.

### Fix Focus Areas
- Makefile[326-339]

### Suggested fix
Add the missing env var to `test-e2e-from-ci-bundle`, matching `test-e2e`, e.g.:
```make
  IMAGE_TLS_SCANNER=$(IMAGE_TLS_SCANNER) \
```
Alternatively, if the intent is to not run TLS scanning in this target, adjust the script/package selection to exclude `test/e2e/operator/tls/...` explicitly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

Comment thread Makefile
@vparfonov

Copy link
Copy Markdown
Contributor Author

/test e2e-using-bundle

@qodo-for-rh-openshift

qodo-for-rh-openshift Bot commented Jul 30, 2026

Copy link
Copy Markdown

Unsupported PR languages

@vparfonov
vparfonov marked this pull request as draft July 30, 2026 11:06
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2026
@vparfonov

Copy link
Copy Markdown
Contributor Author

/test e2e-using-bundle

@qodo-for-rh-openshift

qodo-for-rh-openshift Bot commented Jul 30, 2026

Copy link
Copy Markdown

Unsupported PR languages

@vparfonov

Copy link
Copy Markdown
Contributor Author

/test e2e-using-bundle

@qodo-for-rh-openshift

qodo-for-rh-openshift Bot commented Jul 30, 2026

Copy link
Copy Markdown

Unsupported PR languages

@vparfonov
vparfonov marked this pull request as ready for review July 30, 2026 14:10
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2026
@openshift-ci
openshift-ci Bot requested a review from jcantrill July 30, 2026 14:11
Comment thread hack/test-e2e-from-ci-bundle.sh Outdated
Comment thread hack/test-e2e-from-ci-bundle.sh Outdated
Comment thread test/e2e/collection/security/container_security_test.go Outdated
Comment thread test/e2e/input_selection/input_selection_test.go Outdated
@qodo-for-rh-openshift

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 6d120f9

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@hack/test-e2e-from-ci-bundle.sh`:
- Around line 3-5: Make the namespace-label setup command fail the script when
`oc label ns/openshift-logging` cannot complete, before the `go test` invocation
runs. Update the shell failure handling near the label command in
`hack/test-e2e-from-ci-bundle.sh` while preserving the existing label and test
commands.

In `@test/e2e/collection/security/container_security_test.go`:
- Around line 39-40: Update the security test assertions around touchFile to
first verify that the expected mount path exists, then validate the touch
command failure while allowing the environment-specific read-only filesystem
error. Remove “No such file or directory” from the touch error regexp so a
missing mount cannot satisfy the test.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c45f4dd2-1829-41b7-9a7a-93a440a4ec1d

📥 Commits

Reviewing files that changed from the base of the PR and between a41af4e and 6d120f9.

📒 Files selected for processing (4)
  • Makefile
  • hack/test-e2e-from-ci-bundle.sh
  • test/e2e/collection/security/container_security_test.go
  • test/e2e/input_selection/input_selection_test.go

Comment thread hack/test-e2e-from-ci-bundle.sh Outdated
Comment thread test/e2e/collection/security/container_security_test.go Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@hack/test-e2e-from-ci-bundle.sh`:
- Around line 8-13: Synchronize the namespace used by the script’s
CLUSTER_LOGGING_OPERATOR_NAMESPACE override with the E2E setup in
container_security_test.go: thread that configured namespace into the test
instead of hard-coding constants.OpenshiftNS, or remove the script override.
Ensure labeling and security checks always target the same namespace.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1179a515-ea4d-42ca-92fe-002908dd853c

📥 Commits

Reviewing files that changed from the base of the PR and between a41af4e and 17714c5.

📒 Files selected for processing (4)
  • Makefile
  • hack/test-e2e-from-ci-bundle.sh
  • test/e2e/collection/security/container_security_test.go
  • test/e2e/input_selection/input_selection_test.go

Comment thread hack/test-e2e-from-ci-bundle.sh
@vparfonov

Copy link
Copy Markdown
Contributor Author

/retest-required

@vparfonov

Copy link
Copy Markdown
Contributor Author

/test e2e-target

@qodo-for-rh-openshift

qodo-for-rh-openshift Bot commented Jul 31, 2026

Copy link
Copy Markdown

PR-Agent: could not find a component named e2e-target in a supported language in this PR.

Add Make target and script to run Go e2e tests when the operator is deployed via OLM bundle in CI.

NOTE:
- runs test packages sequentially to avoid resource collisions
- skips FlowControl tests (as in regular e2e test)

Signed-off-by: Vitalii Parfonov <vparfono@redhat.com>
@vparfonov

Copy link
Copy Markdown
Contributor Author

/test e2e-target

@qodo-for-rh-openshift

qodo-for-rh-openshift Bot commented Jul 31, 2026

Copy link
Copy Markdown

PR-Agent: could not find a component named e2e-target in a supported language in this PR.

@openshift-ci

openshift-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

@vparfonov: all tests passed!

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant