Skip to content

OLS-3670 - adding lseval presubmits evaluation to ols - #84192

Open
sriroopar wants to merge 1 commit into
openshift:mainfrom
sriroopar:ols-lseval-presubmit
Open

OLS-3670 - adding lseval presubmits evaluation to ols#84192
sriroopar wants to merge 1 commit into
openshift:mainfrom
sriroopar:ols-lseval-presubmit

Conversation

@sriroopar

@sriroopar sriroopar commented Aug 27, 2026

Copy link
Copy Markdown

Adds a new ols-lseval-presubmit presubmit job for openshift/lightspeed-service that runs the lseval short dataset (10 questions) across 6 providers sequentially.

  • Trigger: run_if_changed on eval configs, test files, provider code, prompts, RAG, and tools
  • Cluster: AWS us-east-1 FIPS 4.22, 1h30m timeout
  • Credentials: openai, azure_openai, watsonx, vertex, bedrock-iam (all from test-credentials namespace)
  • Script: tests/scripts/test-lseval-presubmit.sh

Summary by CodeRabbit

  • Adds the ols-lseval-presubmit job to the openshift/lightspeed-service CI configuration.
  • Runs the 10-question lseval short dataset sequentially across six providers.
  • Uses an AWS us-east-1 FIPS OpenShift 4.22 cluster with a 1 hour 30 minute timeout.
  • Mounts provider credentials from the test-credentials namespace.
  • Runs tests/scripts/test-lseval-presubmit.sh when evaluation, test, provider, prompt, RAG, or tool changes occur.
  • Collects must-gather data and depends on the lightspeed-service-api image.

@sriroopar

Copy link
Copy Markdown
Author

/pj-rehearse pull-ci-openshift-lightspeed-service-main-ols-lseval-presubmit

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 382b8be3-6f64-428c-902b-c2fd2dace0f3

📥 Commits

Reviewing files that changed from the base of the PR and between 24fab75 and 274f84f.

⛔ Files ignored due to path filters (1)
  • ci-operator/jobs/openshift/lightspeed-service/openshift-lightspeed-service-main-presubmits.yaml is excluded by !ci-operator/jobs/**
📒 Files selected for processing (1)
  • ci-operator/config/openshift/lightspeed-service/openshift-lightspeed-service-main.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • ci-operator/config/openshift/lightspeed-service/openshift-lightspeed-service-main.yaml

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.


Walkthrough

The OpenShift Lightspeed CI configuration adds an ols-lseval-presubmit step. The step runs the evaluation script with multiple provider credentials, uses the Lightspeed API image, gathers diagnostics, and runs when relevant files change.

Changes

Lightspeed evaluation CI

Layer / File(s) Summary
Evaluation presubmit step
ci-operator/config/openshift/lightspeed-service/openshift-lightspeed-service-main.yaml
Adds ols-lseval-presubmit with AWS FIPS OCP 4.22 cluster claims, provider credential mounts, the test-lseval-presubmit.sh script, the lightspeed-service-api image dependency, conditional execution, and must-gather collection.

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

Merge Risk: ⚪ Minimal · up to 274f8

This PR adds a localized presubmit job for lseval evaluation, and no actionable merge-blocking risk remains based on the supplied evidence.

Suggested reviewers: blublinsky, raptorsun


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
No-Sensitive-Data-In-Logs ❌ Error The new job activates logging of the cluster-specific OLS route hostname. The changed config runs tests/scripts/test-lseval-presubmit.sh with the claimed cluster. That script runs the E2E pytest set… Redact the OLS route hostname before logging it in the E2E setup and evaluator pipeline. Sanitize captured evaluator stdout/stderr and must-gather output before publishing CI artifacts. Keep credential values out of command arguments and di…
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the addition of the lseval presubmit evaluation, which is the main change in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
Stable And Deterministic Test Names ✅ Passed PASS — The pull request changes only CI configuration and generated Prow job YAML. The added content defines ols-lseval-presubmit, credentials, a shell command, and CI metadata. It adds no Ginkgo so…
Test Structure And Quality ✅ Passed PASS: The pull request changes only two CI YAML files. It adds a job that invokes tests/scripts/test-lseval-presubmit.sh, but it does not add or modify Ginkgo test code. No changed It blocks, reso…
Microshift Test Compatibility ✅ Passed PASS: The pull request changes only CI configuration and generated presubmit job data. The diff adds no Ginkgo test declarations or test source, and it contains no references to MicroShift-unavailable…
Single Node Openshift (Sno) Test Compatibility ✅ Passed PASS: The pull request adds only CI configuration and a generated presubmit job. The exact diff contains no new Ginkgo declarations such as It(), Describe(), Context(), or When(), and it adds …
Topology-Aware Scheduling Compatibility ✅ Passed PASS: The pull request changes only ci-operator configuration and the generated Prow presubmit job. The added job claims an AWS OCP 4.22 cluster and runs a test container, but it adds no deployment ma…
Ote Binary Stdout Contract ✅ Passed PASS. The pull request changes only two YAML CI configuration files. The added job runs a shell script in the src container and does not add or modify an OTE binary, main(), suite setup, logging c…
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The pull request changes only CI configuration and generated Prow jobs. It adds no Ginkgo test code or Ginkgo declarations such as It(), Describe(), Context(), or When(). The referenced LSEval…
No-Weak-Crypto ✅ Passed PASS: The commit adds only CI configuration and generated Prow job YAML. The added commands export provider credential paths and read Bedrock credential files, then invoke an existing script. No MD5, …
Container-Privileges ✅ Passed PASS: The added presubmit and generated Kubernetes job contain no privileged: true, host PID/network/IPC settings, SYS_ADMIN, allowPrivilegeEscalation, security context, capability, or root-user…
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

Full details: Stable And Deterministic Test Names

Explanation

PASS — The pull request changes only CI configuration and generated Prow job YAML. The added content defines ols-lseval-presubmit, credentials, a shell command, and CI metadata. It adds no Ginkgo source or test-title calls such as It(), Describe(), Context(), or When(). Therefore, the stable and deterministic Ginkgo test-name check is not applicable.

Full details: Test Structure And Quality

Explanation

PASS: The pull request changes only two CI YAML files. It adds a job that invokes tests/scripts/test-lseval-presubmit.sh, but it does not add or modify Ginkgo test code. No changed It blocks, resource setup or cleanup, cluster waits, or assertions exist for this check to review.

Full details: Microshift Test Compatibility

Explanation

PASS: The pull request changes only CI configuration and generated presubmit job data. The diff adds no Ginkgo test declarations or test source, and it contains no references to MicroShift-unavailable OpenShift APIs, namespaces, or unsupported platform assumptions. The check applies only when new Ginkgo e2e tests are added.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The pull request adds only CI configuration and a generated presubmit job. The exact diff contains no new Ginkgo declarations such as It(), Describe(), Context(), or When(), and it adds no test source or topology-dependent behavior. The referenced evaluation script is not present in this repository, so this change does not introduce a SNO-incompatible Ginkgo test under this check.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request changes only ci-operator configuration and the generated Prow presubmit job. The added job claims an AWS OCP 4.22 cluster and runs a test container, but it adds no deployment manifest, operator, or controller scheduling constraint. The diff contains no pod anti-affinity, topology spread, replica-count logic, node selectors or affinity, tolerations, PDBs, or arbiter/control-plane assumptions.

Full details: Ote Binary Stdout Contract

Explanation

PASS. The pull request changes only two YAML CI configuration files. The added job runs a shell script in the src container and does not add or modify an OTE binary, main(), suite setup, logging configuration, or any process-level stdout write. The cat commands are inside shell command substitutions, and no changed line writes non-JSON data from an OTE process to stdout.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

The pull request changes only CI configuration and generated Prow jobs. It adds no Ginkgo test code or Ginkgo declarations such as It(), Describe(), Context(), or When(). The referenced LSEval suite is a Python pytest suite, so the custom check is not applicable. The changed YAML also contains no IPv4 literals or URL construction.

Full details: No-Weak-Crypto

Explanation

PASS: The commit adds only CI configuration and generated Prow job YAML. The added commands export provider credential paths and read Bedrock credential files, then invoke an existing script. No MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, custom crypto, or secret/token comparison appears in the added lines. The referenced test script is not added or modified by this commit.

Full details: Container-Privileges

Explanation

PASS: The added presubmit and generated Kubernetes job contain no privileged: true, host PID/network/IPC settings, SYS_ADMIN, allowPrivilegeEscalation, security context, capability, or root-user declaration. The only USER 0 declaration is pre-existing in both HEAD^ and HEAD, and the new job only reuses the existing src image. No listed privilege condition was introduced by this pull request.

Full details: No-Sensitive-Data-In-Logs

Explanation

The new job activates logging of the cluster-specific OLS route hostname. The changed config runs tests/scripts/test-lseval-presubmit.sh with the claimed cluster. That script runs the E2E pytest setup, which prints Waiting for OLS to be ready at url: {ols_url} and the pinned evaluator also logs Setting up API client: {api_config.api_base}. The test then prints the evaluator stdout and stderr to the Prow log. The URL is obtained from the cluster Route host, so the new job can expose an internal CI hostname. The added shell exports pass credential paths or values to the test, but the reviewed changed lines do not directly print API keys or tokens.

Resolution

Redact the OLS route hostname before logging it in the E2E setup and evaluator pipeline. Sanitize captured evaluator stdout/stderr and must-gather output before publishing CI artifacts. Keep credential values out of command arguments and diagnostic exceptions, then regenerate the affected Prow job configuration.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@JoaoFula

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci
openshift-ci Bot requested review from blublinsky and raptorsun August 27, 2026 14:31
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 27, 2026
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2026
Adds a new ols-lseval-presubmit presubmit job for openshift/lightspeed-service
that runs the lseval short dataset (10 questions) across 6 providers sequentially.

- Trigger: run_if_changed on eval configs, test files, provider code, prompts, RAG, and tools
- Cluster: AWS us-east-1 FIPS 4.22, 1h30m timeout
- Credentials: openai, azure_openai, watsonx, vertex, bedrock-iam (all from test-credentials namespace)
- Script: tests/scripts/test-lseval-presubmit.sh

Signed-off-by: Joao Fula <jfula@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@sriroopar
sriroopar force-pushed the ols-lseval-presubmit branch from 1b4a7b8 to 274f84f Compare August 27, 2026 14:32
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@sriroopar: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@JoaoFula

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2026
@openshift-ci

openshift-ci Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoaoFula, sriroopar

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

The pull request process is described 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 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

[REHEARSALNOTIFIER]
@sriroopar: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
pull-ci-openshift-lightspeed-service-main-ols-lseval-presubmit openshift/lightspeed-service presubmit Presubmit changed
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse list to get an up-to-date list of affected jobs
Comment: /pj-rehearse abort to abort all active rehearsals
Comment: /pj-rehearse network-access-allowed to allow rehearsals of tests that have the restrict_network_access field set to false. This must be executed by an openshift org member who is not the PR author

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@sriroopar

Copy link
Copy Markdown
Author

/retest

@sriroopar

Copy link
Copy Markdown
Author

/pj-rehearse pull-ci-openshift-lightspeed-service-main-ols-lseval-presubmit

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@sriroopar: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@sriroopar

Copy link
Copy Markdown
Author

/retest

@openshift-ci

openshift-ci Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

@sriroopar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/rehearse/openshift/lightspeed-service/main/ols-lseval-presubmit 274f84f link unknown /pj-rehearse pull-ci-openshift-lightspeed-service-main-ols-lseval-presubmit

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

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants