OLS-3218 - Migrating prow integration tests to konflux and preparation of konflux pipelines#70
OLS-3218 - Migrating prow integration tests to konflux and preparation of konflux pipelines#70JoaoFula wants to merge 3 commits into
Conversation
|
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:
📝 WalkthroughWalkthroughThis PR adds sandbox integration test infrastructure for three LLM providers. IntegrationTestScenario resources register test configurations, a Tekton Pipeline orchestrates execution by parsing SNAPSHOT revisions and mounting credentials, and a Bash script handles provider-specific credential setup and test invocation. A minor fix to container runtime detection improves error handling when neither podman nor docker is available. ChangesSandbox Integration Test Infrastructure
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 @.tekton/integration-tests/pipelines/sandbox-integration-test-pipeline.yaml:
- Around line 27-40: The embedded taskSpec is missing the credential param used
by the volume secret, so add a matching param entry to taskSpec.params named
exactly "credential-secret" (type: string) so the substitution
$(params.credential-secret) inside taskSpec.volumes[].secret.secretName
resolves; update the taskSpec.params list (alongside SNAPSHOT and provider) to
include - name: credential-secret and ensure the param name matches the
pipeline-level param declaration.
In @.tekton/integration-tests/scripts/run-sandbox-integration-tests.sh:
- Around line 51-53: The script currently runs a dynamic, unpinned install via
the line "pip install --quiet uv"; change that to install a pinned, reproducible
version (e.g., replace with an exact spec like "uv==<exact-version>") and wrap
the install in a retry/backoff loop (limited attempts, exponential or fixed
backoff and fail fast with a clear error) so transient network failures don't
make CI flaky; update the install invocation in the install-check block that
tests for the "uv" command accordingly.
🪄 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: 3bc8da03-5e86-4343-923a-60e5e81cd2e6
📒 Files selected for processing (3)
.tekton/integration-tests/integration-test-scenarios/sandbox-integration-test-scenarios.yaml.tekton/integration-tests/pipelines/sandbox-integration-test-pipeline.yaml.tekton/integration-tests/scripts/run-sandbox-integration-tests.sh
|
/retest |
JoaoFula
force-pushed
the
implement-ols-3218
branch
from
babff72 to
f468dcc
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
JoaoFula
force-pushed
the
implement-ols-3218
branch
from
f468dcc to
aa9c743
Compare
|
/retest |
JoaoFula
force-pushed
the
implement-ols-3218
branch
from
aa9c743 to
3b25578
Compare
|
/retest |
1 similar comment
|
/retest |
JoaoFula
force-pushed
the
implement-ols-3218
branch
3 times, most recently
from
3e2f33a to
fe4f954
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.tekton/integration-tests/scripts/run-sandbox-integration-tests.sh (1)
40-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrailing whitespace/newline may corrupt the API key.
catpreserves trailing newlines from the file. If the secret has a trailing newline (common withechoor editors), the API key will fail silently.Suggested fix
- OPENAI_API_KEY=$(cat "${CRED_PATH}") + OPENAI_API_KEY=$(tr -d '[:space:]' < "${CRED_PATH}")🤖 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 @.tekton/integration-tests/scripts/run-sandbox-integration-tests.sh around lines 40 - 42, The OPENAI_API_KEY assignment reads from the file using cat, which preserves trailing newlines and whitespace that will corrupt the API key value. In the openai case block where OPENAI_API_KEY is assigned from cat "${CRED_PATH}", strip trailing whitespace and newlines from the file content (using a tool like tr to remove newlines or a similar approach) to ensure the API key is clean before exporting it.
🤖 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.
Outside diff comments:
In @.tekton/integration-tests/scripts/run-sandbox-integration-tests.sh:
- Around line 40-42: The OPENAI_API_KEY assignment reads from the file using
cat, which preserves trailing newlines and whitespace that will corrupt the API
key value. In the openai case block where OPENAI_API_KEY is assigned from cat
"${CRED_PATH}", strip trailing whitespace and newlines from the file content
(using a tool like tr to remove newlines or a similar approach) to ensure the
API key is clean before exporting it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 94bb1461-9774-413d-bece-b59954e7e7b5
📒 Files selected for processing (4)
.tekton/integration-tests/integration-test-scenarios/sandbox-integration-test-scenarios.yaml.tekton/integration-tests/pipelines/sandbox-integration-test-pipeline.yaml.tekton/integration-tests/scripts/run-sandbox-integration-tests.shscripts/e2e-containers.sh
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-operator(manual)
🚧 Files skipped from review as they are similar to previous changes (2)
- .tekton/integration-tests/integration-test-scenarios/sandbox-integration-test-scenarios.yaml
- .tekton/integration-tests/pipelines/sandbox-integration-test-pipeline.yaml
JoaoFula
force-pushed
the
implement-ols-3218
branch
from
fe4f954 to
a4b2727
Compare
openshift-ci
Bot
added
the
needs-rebase
Sets up provider credentials from mounted Konflux workspace secrets and runs BDD tests in host mode via e2e-containers.sh.
Parameterized pipeline that clones the sandbox repo at the SNAPSHOT revision and delegates to run-sandbox-integration-tests.sh. Accepts provider and credential-secret params from ITS CRs.
JoaoFula
force-pushed
the
implement-ols-3218
branch
from
a4b2727 to
cc40675
Compare
openshift-ci
Bot
removed
the
needs-rebase
|
/retest |
Three ITS CRs (claude, gemini, openai) trigger on sandbox component builds. Each passes provider and credential-secret params to the shared sandbox-integration-test-pipeline. removing e2e presubmits adding except for oauth and reforcing logic around filtering for events adding except for oauth and reforcing logic around filtering for events adding except for oauth and reforcing logic around filtering for events adding except for oauth and reforcing logic around filtering for events adding except for oauth and reforcing logic around filtering for events adding except for oauth and reforcing logic around filtering for events adding except for oauth and reforcing logic around filtering for events
JoaoFula
force-pushed
the
implement-ols-3218
branch
from
cc40675 to
7d3f18d
Compare
|
@JoaoFula: 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. |
|
/retest |
1 participant
No description provided.