OLS-3265: Add Konflux integration test pipeline and e2e test improvements#113
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@blublinsky: This pull request references OLS-3265 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 "5.0.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. |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (10)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughPR adds sandbox-mode configuration to the Makefile and manager deployment args, introduces a Tekton pipeline and installer script for ephemeral-cluster e2e runs, updates e2e test helpers to support dynamic test namespaces and explicit verification approval, and enhances execution tests with per-proposal ServiceAccount and RoleBinding assertions plus explicit verification sequencing in the verification flow. ChangesEnd-to-end test infrastructure and CI/CD
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 @.tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yaml:
- Line 148: The pipeline is silently ignoring checkout failures because the
command 'git fetch --depth=1 origin "${COMMIT}" && git checkout "${COMMIT}" ||
true' swallows errors; remove the trailing '|| true' so the shell returns a
non‑zero exit and the Tekton task fails on fetch/checkout errors, or
alternatively replace it with an explicit validation step after checkout that
verifies the current HEAD matches "${COMMIT}" and exits non‑zero if not (update
the existing command string referenced above).
🪄 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: db93b893-0f21-4d09-8d7f-2c94fe7a2e1b
📒 Files selected for processing (6)
.tekton/integration-tests/integration-test-scenarios.yaml.tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yamlMakefiletest/e2e/execution_test.gotest/e2e/helpers_test.gotest/e2e/verification_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
blublinsky
force-pushed
the
e2e-test
branch
2 times, most recently
from
ae22399 to
2500c3f
Compare
There was a problem hiding this comment.
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 `@test/e2e/helpers_test.go`:
- Around line 118-127: The deleteBarePod helper defaults OPERATOR_NAMESPACE to
"openshift-lightspeed", which mismatches the Makefile default of "default";
update deleteBarePod (and its OPERATOR_NAMESPACE fallback logic) to use the same
default used when running locally—either change the fallback to "default" or,
better, use the existing test namespace variable (testNS) used elsewhere in
tests so cleanup targets the same namespace the operator creates bare pods in;
adjust only the fallback assignment inside deleteBarePod to reference testNS (or
"default") so deletion succeeds.
🪄 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: 5811c707-782d-406d-bc3d-609591c934a6
📒 Files selected for processing (6)
.tekton/integration-tests/integration-test-scenarios.yaml.tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yamlMakefiletest/e2e/execution_test.gotest/e2e/helpers_test.gotest/e2e/verification_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
✅ Files skipped from review due to trivial changes (1)
- .tekton/integration-tests/integration-test-scenarios.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- test/e2e/execution_test.go
- .tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yaml
openshift-ci
Bot
added
the
needs-rebase
openshift-ci
Bot
removed
the
needs-rebase
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Makefile (1)
160-163: 💤 Low valueSed delimiter collision risk if
SANDBOX_IMAGEcontains|.The sed command uses
|as a delimiter. IfSANDBOX_IMAGEever contains a literal|(unlikely for image refs, but possible), the substitution will fail silently or produce malformed YAML.Consider escaping or using a less common delimiter if this needs to be more robust.
🤖 Prompt for 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. In `@Makefile` around lines 160 - 163, The sed substitution in the Makefile uses | as the delimiter and can break if $(SANDBOX_IMAGE) contains |; update the sed invocation to use a less-common delimiter (for example @) for all three substitutions so variables OPERATOR_NAMESPACE, SANDBOX_MODE and SANDBOX_IMAGE are substituted safely (e.g. replace -e 's|__OPERATOR_NAMESPACE__|$(OPERATOR_NAMESPACE)|g' with -e 's@__OPERATOR_NAMESPACE__@$(OPERATOR_NAMESPACE)`@g`' and likewise for the other two) or alternatively switch to a quoting/escaping approach (perl -pe or awk) that properly handles arbitrary characters in $(SANDBOX_IMAGE).
🤖 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.
Nitpick comments:
In `@Makefile`:
- Around line 160-163: The sed substitution in the Makefile uses | as the
delimiter and can break if $(SANDBOX_IMAGE) contains |; update the sed
invocation to use a less-common delimiter (for example @) for all three
substitutions so variables OPERATOR_NAMESPACE, SANDBOX_MODE and SANDBOX_IMAGE
are substituted safely (e.g. replace -e
's|__OPERATOR_NAMESPACE__|$(OPERATOR_NAMESPACE)|g' with -e
's@__OPERATOR_NAMESPACE__@$(OPERATOR_NAMESPACE)`@g`' and likewise for the other
two) or alternatively switch to a quoting/escaping approach (perl -pe or awk)
that properly handles arbitrary characters in $(SANDBOX_IMAGE).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c7f9cca9-fe2c-4d0b-b4f6-80684f074eda
📒 Files selected for processing (10)
.tekton/integration-tests/integration-test-scenarios.yaml.tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yaml.tekton/integration-tests/scripts/install-operator.shMakefileconfig/manager/manager.yamltest/e2e/analysis_execution_test.gotest/e2e/denial_test.gotest/e2e/execution_test.gotest/e2e/helpers_test.gotest/e2e/verification_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
✅ Files skipped from review due to trivial changes (1)
- test/e2e/analysis_execution_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- test/e2e/verification_test.go
- .tekton/integration-tests/integration-test-scenarios.yaml
- test/e2e/execution_test.go
- test/e2e/helpers_test.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/e2e/helpers_test.go (1)
118-127:⚠️ Potential issue | 🟡 MinorOPERATOR_NAMESPACE default mismatch across files.
deleteBarePoddefaultsOPERATOR_NAMESPACEto"openshift-lightspeed"(line 123), butMakefileline 50 defaults it to"default". When running the operator locally viamake runwithout explicitly settingOPERATOR_NAMESPACE, the operator creates bare pods in namespace"default", but this cleanup helper attempts to delete them from"openshift-lightspeed", causing cleanup to fail and leaving orphaned pods.🤖 Prompt for 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. In `@test/e2e/helpers_test.go` around lines 118 - 127, The deleteBarePod helper uses OPERATOR_NAMESPACE default "openshift-lightspeed" which mismatches the Makefile default "default"; update the deleteBarePod function (and any similar test helpers) to use the same default namespace as the Makefile (e.g., default to "default" or read from a shared constant/env helper) so the cleanup targets the same namespace the operator creates pods in; specifically, change the fallback value in deleteBarePod (function name: deleteBarePod, variable: operatorNS / OPERATOR_NAMESPACE) to match the Makefile's default or centralize the namespace default into a shared symbol and reference it from the test helper.
🤖 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.
Duplicate comments:
In `@test/e2e/helpers_test.go`:
- Around line 118-127: The deleteBarePod helper uses OPERATOR_NAMESPACE default
"openshift-lightspeed" which mismatches the Makefile default "default"; update
the deleteBarePod function (and any similar test helpers) to use the same
default namespace as the Makefile (e.g., default to "default" or read from a
shared constant/env helper) so the cleanup targets the same namespace the
operator creates pods in; specifically, change the fallback value in
deleteBarePod (function name: deleteBarePod, variable: operatorNS /
OPERATOR_NAMESPACE) to match the Makefile's default or centralize the namespace
default into a shared symbol and reference it from the test helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3a7ed3bc-07ba-4352-bc5b-997af8607f37
📒 Files selected for processing (10)
.tekton/integration-tests/integration-test-scenarios.yaml.tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yaml.tekton/integration-tests/scripts/install-operator.shMakefileconfig/manager/manager.yamltest/e2e/analysis_execution_test.gotest/e2e/denial_test.gotest/e2e/execution_test.gotest/e2e/helpers_test.gotest/e2e/verification_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
🚧 Files skipped from review as they are similar to previous changes (8)
- test/e2e/denial_test.go
- .tekton/integration-tests/integration-test-scenarios.yaml
- config/manager/manager.yaml
- .tekton/integration-tests/scripts/install-operator.sh
- test/e2e/analysis_execution_test.go
- test/e2e/execution_test.go
- test/e2e/verification_test.go
- .tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yaml
|
/retest |
|
@blublinsky: all tests passed! 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. |
| name: cluster-admin | ||
| subjects: | ||
| - kind: ServiceAccount | ||
| name: controller-manager |
There was a problem hiding this comment.
may be unrelated to this PR, but do we want to standardize the SA name here?
this PR uses controller-manager, the quickstart script uses lightspeed-agentic-operator
the bundle PR is going to use agentic-controller-manager
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: blublinsky The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
4 participants
Summary
Add a Konflux integration test pipeline that runs e2e tests against an ephemeral Hypershift cluster on every operator image build. Also improve the e2e tests for bare-pod mode and per-proposal SA verification.
Konflux pipeline
Pipeline deploys the freshly-built operator image from SNAPSHOT with a fixed mock agent (quay.io/openshift-lightspeed/ols-qe:lightspeed-mock-agent). No OLM bundle, no real LLM secrets — tests are self-contained.
Design decisions
Fixed mock agent image: Not built from SNAPSHOT — it's a stable test fixture that changes rarely. Avoids coupling two component builds.
No auto-approval for verification: Each test explicitly approves only the steps it needs. Prevents the execution test from accidentally spawning a verification pod (wasting 60s+ per run).
TEST_NAMESPACE env var: Allows the same tests to run in any namespace — openshift-lightspeed in Konflux/production, overridable for local dev.
validate-agent-sandbox (check, not install): The Sandbox operator install method is transitioning (raw manifests → OLM). The Makefile checks CRDs exist but doesn't install them — that's the user's/OLM's responsibility.