LCORE-2035- Add TLS fixes for konflux run#1929
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
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:
WalkthroughSplits the monolithic ChangesTLS E2E Infrastructure for Prow/Konflux
Sequence Diagram(s)sequenceDiagram
participant BehaveRunner
participant environment_py
participant tls_py
participant e2e_ops_sh
participant k8s_cluster
participant lightspeed_stack
rect rgba(70, 130, 180, 0.5)
Note over BehaveRunner,environment_py: before_feature (TLS feature file)
BehaveRunner->>environment_py: before_feature(context, feature)
environment_py->>tls_py: reset_tls_prow_state()
environment_py->>tls_py: prepare_tls_feature_entry_on_prow(feature_filename)
tls_py->>e2e_ops_sh: deploy-e2e-mock-tls-inference
e2e_ops_sh->>k8s_cluster: apply e2e-mock-tls-inference.yaml
e2e_ops_sh->>k8s_cluster: wait for pod ready
tls_py->>e2e_ops_sh: sync-mock-tls-certs-secret (with retry)
e2e_ops_sh->>k8s_cluster: create secret e2e-mock-tls-certs
tls_py->>e2e_ops_sh: restart-both-services
e2e_ops_sh->>k8s_cluster: restore llama-stack pod
e2e_ops_sh->>lightspeed_stack: restart lightspeed-stack pod
end
rect rgba(34, 139, 34, 0.5)
Note over BehaveRunner,lightspeed_stack: per-scenario TLS step
BehaveRunner->>tls_py: _configure_tls(tls_dict, base_url)
tls_py->>k8s_cluster: update llama run configmap
tls_py->>e2e_ops_sh: restart-llama-stack (with cert mount)
BehaveRunner->>lightspeed_stack: REST query via mock TLS port
lightspeed_stack-->>BehaveRunner: HTTP 200/500 response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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: 5
🤖 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 `@tests/e2e-prow/rhoai/manifests/lightspeed/e2e-mock-tls-inference.yaml`:
- Line 37: The pip install command on line 37 uses open-ended version
constraints (trustme>=1.2.1 and cryptography>=42.0.0) without upper bounds,
causing non-deterministic behavior when upstream releases new versions. Add
upper bound constraints to both packages to make the dependency versions
deterministic. For trustme, add an upper bound like >=1.2.1,<2 (or appropriate
upper bound based on tested compatibility). For cryptography, update the
constraint from >=42.0.0 to a bounded range reflecting currently compatible
versions such as >=42.0.0,<44 (or pin to the specific tested version if 43.x
versions have incompatibilities with your code). Ensure both packages have
explicit upper bounds to prevent non-deterministic e2e test behavior.
In `@tests/e2e-prow/rhoai/manifests/lightspeed/llama-stack-openai.yaml`:
- Around line 44-47: The cache validation logic fails to verify the existence of
a critical startup artifact, allowing partial PVC caches to be considered valid.
In the conditional block that checks for PVC cache hits (the if statement
checking for /opt/app-root/.venv directory and Python version), add an
additional check to ensure /opt/app-root/llama_stack_configuration.py exists
before marking the cache as valid. This required artifact is executed
unconditionally later in the script, so the cache should only be considered a
hit when this file is present.
In `@tests/e2e-prow/rhoai/pipeline-konflux.sh`:
- Around line 150-152: The script deletes the PVC with oc delete on line 150 and
immediately reapplies it on line 151, creating a race condition since PVC
deletion is asynchronous. Add a wait mechanism after the oc delete pvc
llama-stack-app-root command to ensure the PVC is fully removed before
attempting to recreate it with oc apply. Use an oc wait command or retry logic
to verify the PVC is completely deleted and gone from the namespace before
proceeding with the reapplication.
In `@tests/e2e-prow/rhoai/scripts/e2e-ops.sh`:
- Around line 906-938: The `return 1` statement on line 937 that appears after
the for loop (which iterates on pass in 1 2 and closes with done on line 936) is
unreachable because all control paths through the loop either explicitly return
(at lines 922 and 935) or continue in a way that eventually leads to an explicit
return. Remove this unreachable `return 1` statement to clean up the code and
avoid confusion for future readers.
In `@tests/e2e/features/environment.py`:
- Around line 260-261: The comparison at line 260 in the environment.py file
compares scenario.status to the string "failed", but scenario.status is a Status
enum object from behave.model_core, not a string. This comparison will always
evaluate to false, causing the function to return early and skip the intended
log dumping logic. Replace the string literal "failed" with the appropriate
Status enum member from behave.model_core (such as Status.failed). If the Status
enum is not already imported, add the necessary import statement at the top of
the file.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 19c8b685-e337-40d5-a0ff-c152ad35f4fb
📒 Files selected for processing (19)
.tekton/integration-tests/pipeline/lightspeed-stack-integration-test.yamltests/e2e-prow/rhoai/manifests/lightspeed/e2e-mock-tls-inference.yamltests/e2e-prow/rhoai/manifests/lightspeed/llama-stack-openai.yamltests/e2e-prow/rhoai/pipeline-konflux.shtests/e2e-prow/rhoai/pipeline-services-konflux.shtests/e2e-prow/rhoai/scripts/e2e-ops.shtests/e2e/configuration/server-mode/lightspeed-stack-tls.yamltests/e2e/features/environment.pytests/e2e/features/proxy.featuretests/e2e/features/steps/proxy.pytests/e2e/features/steps/tls.pytests/e2e/features/tls-ca.featuretests/e2e/features/tls-mtls.featuretests/e2e/features/tls-tlsv13.featuretests/e2e/features/tls.featuretests/e2e/mock_tls_inference_server/server.pytests/e2e/test_list.txttests/e2e/utils/llama_config_utils.pytests/e2e/utils/prow_utils.py
💤 Files with no reviewable changes (1)
- tests/e2e/features/tls.feature
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: build-pr
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🧰 Additional context used
📓 Path-based instructions (2)
tests/e2e/**/*.{py,feature}
📄 CodeRabbit inference engine (AGENTS.md)
Use behave (BDD) framework for end-to-end testing with Gherkin feature files
Files:
tests/e2e/features/proxy.featuretests/e2e/mock_tls_inference_server/server.pytests/e2e/utils/llama_config_utils.pytests/e2e/features/tls-mtls.featuretests/e2e/features/tls-ca.featuretests/e2e/features/steps/proxy.pytests/e2e/utils/prow_utils.pytests/e2e/features/tls-tlsv13.featuretests/e2e/features/environment.pytests/e2e/features/steps/tls.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Usepytest.mark.asynciomarker for async tests
Files:
tests/e2e/mock_tls_inference_server/server.pytests/e2e/utils/llama_config_utils.pytests/e2e/features/steps/proxy.pytests/e2e/utils/prow_utils.pytests/e2e/features/environment.pytests/e2e/features/steps/tls.py
🧠 Learnings (4)
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.
Applied to files:
tests/e2e/configuration/server-mode/lightspeed-stack-tls.yamltests/e2e-prow/rhoai/manifests/lightspeed/e2e-mock-tls-inference.yamltests/e2e-prow/rhoai/manifests/lightspeed/llama-stack-openai.yaml
📚 Learning: 2026-02-19T10:06:50.647Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 1181
File: tests/e2e-prow/rhoai/manifests/lightspeed/mock-jwks.yaml:32-34
Timestamp: 2026-02-19T10:06:50.647Z
Learning: In the rhoai tests under tests/e2e-prow/rhoai/manifests, avoid static ConfigMap definitions for mock-jwks-script and mcp-mock-server-script since these ConfigMaps are created dynamically by the pipeline.sh deployment script using 'oc create configmap'. Ensure there are no static ConfigMap resources for these names in the manifests. If such ConfigMaps are added in the future, coordinate with the pipeline to reflect dynamic creation or adjust tests to rely on the dynamic provisioning.
Applied to files:
tests/e2e-prow/rhoai/manifests/lightspeed/e2e-mock-tls-inference.yamltests/e2e-prow/rhoai/manifests/lightspeed/llama-stack-openai.yaml
📚 Learning: 2026-04-07T09:20:26.590Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 1467
File: tests/e2e/features/steps/common.py:36-49
Timestamp: 2026-04-07T09:20:26.590Z
Learning: For Behave-based Python tests, rely on Behave’s Context layered stack for attribute lifecycle: Behave pushes a new Context layer when entering feature scope (before_feature) and again for scenario scope (before_scenario). Attributes assigned inside given/when/then steps live on the current scenario layer and are automatically removed when the scenario ends. As a result, step-set attributes should not be expected to persist across scenarios or features, and manual cleanup in after_scenario/after_feature is generally unnecessary for attributes set in step functions. Only perform manual cleanup for attributes that you set explicitly in before_feature/before_scenario, since those live on the respective feature/scenario layers.
Applied to files:
tests/e2e/features/steps/proxy.pytests/e2e/features/environment.pytests/e2e/features/steps/tls.py
📚 Learning: 2026-04-13T13:39:54.963Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 1490
File: tests/e2e/features/environment.py:206-211
Timestamp: 2026-04-13T13:39:54.963Z
Learning: In lightspeed-stack E2E tests under tests/e2e/features, it is intentional to set context.feature_config inside Background/step functions (scenario-scoped Behave layer). The environment.py after_scenario restore logic should only restore configuration when context.scenario_lightspeed_override_active is True; this flag is set by configure_service only when a real config switch occurs (so restore does not run for scenarios without a switch). Additionally, steps/common.py’s module-level _active_lightspeed_stack_config_basename is used to prevent re-applying the same config across subsequent scenarios, ensuring scenario_lightspeed_override_active stays False after the first apply. Therefore, reviewers should not “fix” this flow as if feature_config were incorrectly scoped or if after_scenario restoration is missing—config switching and restoration are meant to happen exactly once per actual switch, not redundantly per scenario.
Applied to files:
tests/e2e/features/steps/proxy.pytests/e2e/features/environment.pytests/e2e/features/steps/tls.py
🪛 Checkov (3.2.530)
tests/e2e-prow/rhoai/manifests/lightspeed/e2e-mock-tls-inference.yaml
[low] 3-87: CPU limits should be set
(CKV_K8S_11)
[low] 3-87: CPU requests should be set
(CKV_K8S_10)
[low] 3-87: The default namespace should not be used
(CKV_K8S_21)
[low] 3-87: Image should use digest
(CKV_K8S_43)
[low] 3-87: Image Pull Policy should be Always
(CKV_K8S_15)
[low] 3-87: Memory limits should be set
(CKV_K8S_13)
[low] 3-87: Memory requests should be set
(CKV_K8S_12)
[low] 3-87: Use read-only filesystem for containers where possible
(CKV_K8S_22)
[low] 3-87: Containers should run as a high UID to avoid host conflict
(CKV_K8S_40)
[low] 3-87: Ensure that Service Account Tokens are only mounted where necessary
(CKV_K8S_38)
[low] 88-104: The default namespace should not be used
(CKV_K8S_21)
tests/e2e-prow/rhoai/manifests/lightspeed/llama-stack-openai.yaml
[medium] 6-250: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[low] 6-250: CPU limits should be set
(CKV_K8S_11)
[low] 6-250: CPU requests should be set
(CKV_K8S_10)
[low] 6-250: The default namespace should not be used
(CKV_K8S_21)
[low] 6-250: Minimize the admission of containers with the NET_RAW capability
(CKV_K8S_28)
[low] 6-250: Image should use digest
(CKV_K8S_43)
[low] 6-250: Image Tag should be fixed - not latest or blank
(CKV_K8S_14)
[low] 6-250: Memory limits should be set
(CKV_K8S_13)
[low] 6-250: Memory requests should be set
(CKV_K8S_12)
[low] 6-250: Minimize the admission of containers with capabilities assigned
(CKV_K8S_37)
[low] 6-250: Use read-only filesystem for containers where possible
(CKV_K8S_22)
[low] 6-250: Containers should run as a high UID to avoid host conflict
(CKV_K8S_40)
[low] 6-250: Prefer using secrets as files over secrets as environment variables
(CKV_K8S_35)
[low] 6-250: Ensure that Service Account Tokens are only mounted where necessary
(CKV_K8S_38)
🪛 Trivy (0.69.3)
tests/e2e-prow/rhoai/manifests/lightspeed/e2e-mock-tls-inference.yaml
[info] 15-80: CPU not limited
Container 'e2e-mock-tls-inference' of Pod 'e2e-mock-tls-inference' should set 'resources.limits.cpu'
Rule: KSV-0011
(IaC/Kubernetes)
[error] 15-80: Root file system is not read-only
Container 'e2e-mock-tls-inference' of Pod 'e2e-mock-tls-inference' should set 'securityContext.readOnlyRootFilesystem' to true
Rule: KSV-0014
(IaC/Kubernetes)
[info] 15-80: CPU requests not specified
Container 'e2e-mock-tls-inference' of Pod 'e2e-mock-tls-inference' should set 'resources.requests.cpu'
Rule: KSV-0015
(IaC/Kubernetes)
[info] 15-80: Memory requests not specified
Container 'e2e-mock-tls-inference' of Pod 'e2e-mock-tls-inference' should set 'resources.requests.memory'
Rule: KSV-0016
(IaC/Kubernetes)
[info] 15-80: Memory not limited
Container 'e2e-mock-tls-inference' of Pod 'e2e-mock-tls-inference' should set 'resources.limits.memory'
Rule: KSV-0018
(IaC/Kubernetes)
[info] 15-80: Runs with UID <= 10000
Container 'e2e-mock-tls-inference' of Pod 'e2e-mock-tls-inference' should set 'securityContext.runAsUser' > 10000
Rule: KSV-0020
(IaC/Kubernetes)
[info] 15-80: Runs with GID <= 10000
Container 'e2e-mock-tls-inference' of Pod 'e2e-mock-tls-inference' should set 'securityContext.runAsGroup' > 10000
Rule: KSV-0021
(IaC/Kubernetes)
[info] 5-8: Workloads in the default namespace
pod e2e-mock-tls-inference in default namespace should set metadata.namespace to a non-default namespace
Rule: KSV-0110
(IaC/Kubernetes)
🔇 Additional comments (28)
tests/e2e-prow/rhoai/scripts/e2e-ops.sh (7)
51-75: LGTM!
77-99: LGTM!
325-397: LGTM!
399-442: LGTM!
444-467: LGTM!
940-968: LGTM!
1001-1057: LGTM!tests/e2e/features/steps/tls.py (6)
39-53: LGTM!
55-74: LGTM!
77-136: LGTM!
138-200: LGTM!
202-257: LGTM!
264-403: LGTM!tests/e2e/utils/llama_config_utils.py (1)
24-39: LGTM!tests/e2e/utils/prow_utils.py (2)
94-131: LGTM!
144-148: LGTM!tests/e2e/configuration/server-mode/lightspeed-stack-tls.yaml (1)
1-23: LGTM!tests/e2e/mock_tls_inference_server/server.py (1)
16-50: LGTM!Also applies to: 283-285
tests/e2e-prow/rhoai/manifests/lightspeed/e2e-mock-tls-inference.yaml (1)
1-36: LGTM!Also applies to: 38-105
tests/e2e-prow/rhoai/manifests/lightspeed/llama-stack-openai.yaml (1)
226-233: LGTM!Also applies to: 247-250
tests/e2e-prow/rhoai/pipeline-services-konflux.sh (1)
29-42: LGTM!Also applies to: 46-47
tests/e2e/features/tls-ca.feature (1)
1-78: LGTM!tests/e2e/features/tls-mtls.feature (1)
1-78: LGTM!tests/e2e/features/tls-tlsv13.feature (1)
1-58: LGTM!tests/e2e/test_list.txt (1)
30-32: LGTM!tests/e2e/features/steps/proxy.py (1)
308-308: LGTM!Also applies to: 322-330
tests/e2e/features/proxy.feature (1)
1-1: LGTM!.tekton/integration-tests/pipeline/lightspeed-stack-integration-test.yaml (1)
170-171: LGTM!
| - -c | ||
| - | | ||
| set -e | ||
| pip install --quiet --no-cache-dir --target /tmp/pydeps 'trustme>=1.2.1' 'cryptography>=42.0.0' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the file
find . -name "e2e-mock-tls-inference.yaml" -type fRepository: lightspeed-core/lightspeed-stack
Length of output: 148
🏁 Script executed:
# Check if the file exists and read around line 37
if [ -f "tests/e2e-prow/rhoai/manifests/lightspeed/e2e-mock-tls-inference.yaml" ]; then
echo "=== File content with line numbers ==="
cat -n "tests/e2e-prow/rhoai/manifests/lightspeed/e2e-mock-tls-inference.yaml" | sed -n '30,45p'
else
echo "File not found at expected path"
fiRepository: lightspeed-core/lightspeed-stack
Length of output: 808
🌐 Web query:
trustme python package latest version 2024
💡 Result:
The latest version of the trustme Python package released in 2024 was version 1.2.0, which was published on October 7, 2024 [1][2]. As of today, June 15, 2026, the project has since released version 1.2.1 (published January 2, 2025) [1][2][3].
Citations:
- 1: https://github.com/python-trio/trustme/blob/main/docs/source/index.rst
- 2: https://trustme.readthedocs.io/
- 3: https://pypi.org/project/trustme/
🌐 Web query:
cryptography python package version 42 43 release
💡 Result:
The cryptography Python package releases for versions 42 and 43 are summarized below based on their official changelogs. Version 42 Series The 42.x series began with version 42.0.0, released on January 22, 2024 [1][2]. The series concluded with version 42.0.8, released on June 4, 2024 [1][3]. - 42.0.0: 2024-01-22 - 42.0.1: 2024-01-24 - 42.0.2: 2024-01-30 - 42.0.3: 2024-02-15 - 42.0.4: 2024-02-20 - 42.0.5: 2024-02-23 - 42.0.6: 2024-05-04 - 42.0.7: 2024-05-06 - 42.0.8: 2024-06-04 Version 43 Series The 43.x series began with version 43.0.0, released on July 20, 2024 [1][2]. Major changes in 43.0.0 included dropping support for OpenSSL versions older than 1.1.1e and LibreSSL versions older than 3.8 [2][4]. - 43.0.0: 2024-07-20 - 43.0.1: 2024-09-03 - 43.0.2: 2024-10-18 - 43.0.3: 2024-10-18 [1][2]
Citations:
- 1: https://cryptography.io/en/43.0.3/changelog/
- 2: https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst
- 3: https://cryptography.io/en/42.0.8/changelog/
- 4: https://cryptography.io/en/stable/changelog/
Pin runtime Python dependencies to deterministic version ranges.
Line 37 installs trustme and cryptography with open-ended >= constraints, causing non-deterministic e2e behavior when upstream releases new versions. The suggested patch adds upper bounds, though the cryptography<43 bound is outdated since version 43.0.0 was released in July 2024 and multiple 43.x versions exist as of today. Update the constraint to reflect currently compatible versions (e.g., cryptography>=42.0.0,<44 if 43.x is compatible, or pin to the tested version explicitly).
Suggested patch
- pip install --quiet --no-cache-dir --target /tmp/pydeps 'trustme>=1.2.1' 'cryptography>=42.0.0'
+ pip install --quiet --no-cache-dir --target /tmp/pydeps \
+ 'trustme>=1.2.1,<2' \
+ 'cryptography>=42.0.0,<44'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pip install --quiet --no-cache-dir --target /tmp/pydeps 'trustme>=1.2.1' 'cryptography>=42.0.0' | |
| pip install --quiet --no-cache-dir --target /tmp/pydeps \ | |
| 'trustme>=1.2.1,<2' \ | |
| 'cryptography>=42.0.0,<44' |
🤖 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 `@tests/e2e-prow/rhoai/manifests/lightspeed/e2e-mock-tls-inference.yaml` at
line 37, The pip install command on line 37 uses open-ended version constraints
(trustme>=1.2.1 and cryptography>=42.0.0) without upper bounds, causing
non-deterministic behavior when upstream releases new versions. Add upper bound
constraints to both packages to make the dependency versions deterministic. For
trustme, add an upper bound like >=1.2.1,<2 (or appropriate upper bound based on
tested compatibility). For cryptography, update the constraint from >=42.0.0 to
a bounded range reflecting currently compatible versions such as >=42.0.0,<44
(or pin to the specific tested version if 43.x versions have incompatibilities
with your code). Ensure both packages have explicit upper bounds to prevent
non-deterministic e2e test behavior.
| if [[ -d /opt/app-root/.venv ]] \ | ||
| && /opt/app-root/.venv/bin/python --version >/dev/null 2>&1 \ | ||
| && [[ -d /opt/app-root/src ]]; then | ||
| echo "PVC cache hit: app-root already provisioned — skipping full install" |
There was a problem hiding this comment.
Fast-path cache validation misses a required startup artifact.
Line 44-47 treats cache as valid without checking /opt/app-root/llama_stack_configuration.py, but Line 182 executes it unconditionally. A partial PVC cache can cause repeat crash loops.
Suggested patch
- if [[ -d /opt/app-root/.venv ]] \
- && /opt/app-root/.venv/bin/python --version >/dev/null 2>&1 \
- && [[ -d /opt/app-root/src ]]; then
+ if [[ -d /opt/app-root/.venv ]] \
+ && /opt/app-root/.venv/bin/python --version >/dev/null 2>&1 \
+ && [[ -d /opt/app-root/src ]] \
+ && [[ -f /opt/app-root/llama_stack_configuration.py ]]; thenAlso applies to: 182-185
🤖 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 `@tests/e2e-prow/rhoai/manifests/lightspeed/llama-stack-openai.yaml` around
lines 44 - 47, The cache validation logic fails to verify the existence of a
critical startup artifact, allowing partial PVC caches to be considered valid.
In the conditional block that checks for PVC cache hits (the if statement
checking for /opt/app-root/.venv directory and Python version), add an
additional check to ensure /opt/app-root/llama_stack_configuration.py exists
before marking the cache as valid. This required artifact is executed
unconditionally later in the script, so the cache should only be considered a
hit when this file is present.
| oc delete pvc llama-stack-app-root -n "$NAMESPACE" --ignore-not-found=true 2>/dev/null || true | ||
| cat <<'EOF' | oc apply -n "$NAMESPACE" -f - | ||
| apiVersion: v1 |
There was a problem hiding this comment.
PVC delete/recreate sequence has a race.
Line 150 deletes the claim and Line 151 reapplies immediately. PVC deletion is asynchronous; recreate can fail while the old claim is still terminating.
Suggested patch
oc delete pvc llama-stack-app-root -n "$NAMESPACE" --ignore-not-found=true 2>/dev/null || true
+for _ in $(seq 1 60); do
+ oc get pvc llama-stack-app-root -n "$NAMESPACE" >/dev/null 2>&1 || break
+ sleep 2
+done
+oc get pvc llama-stack-app-root -n "$NAMESPACE" >/dev/null 2>&1 && {
+ echo "❌ PVC llama-stack-app-root is still terminating"
+ exit 1
+}
cat <<'EOF' | oc apply -n "$NAMESPACE" -f -📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| oc delete pvc llama-stack-app-root -n "$NAMESPACE" --ignore-not-found=true 2>/dev/null || true | |
| cat <<'EOF' | oc apply -n "$NAMESPACE" -f - | |
| apiVersion: v1 | |
| oc delete pvc llama-stack-app-root -n "$NAMESPACE" --ignore-not-found=true 2>/dev/null || true | |
| for _ in $(seq 1 60); do | |
| oc get pvc llama-stack-app-root -n "$NAMESPACE" >/dev/null 2>&1 || break | |
| sleep 2 | |
| done | |
| oc get pvc llama-stack-app-root -n "$NAMESPACE" >/dev/null 2>&1 && { | |
| echo "❌ PVC llama-stack-app-root is still terminating" | |
| exit 1 | |
| } | |
| cat <<'EOF' | oc apply -n "$NAMESPACE" -f - | |
| apiVersion: v1 |
🤖 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 `@tests/e2e-prow/rhoai/pipeline-konflux.sh` around lines 150 - 152, The script
deletes the PVC with oc delete on line 150 and immediately reapplies it on line
151, creating a race condition since PVC deletion is asynchronous. Add a wait
mechanism after the oc delete pvc llama-stack-app-root command to ensure the PVC
is fully removed before attempting to recreate it with oc apply. Use an oc wait
command or retry logic to verify the PVC is completely deleted and gone from the
namespace before proceeding with the reapplication.
| for pass in 1 2; do | ||
| if ! oc get pod "$mock_pod" -n "$NAMESPACE" &>/dev/null; then | ||
| echo "[e2e-ops] WARN: $mock_pod not found — deploying mock TLS inference..." | ||
| if ! _deploy_e2e_mock_tls_inference_pod; then | ||
| return 1 | ||
| fi | ||
| elif ! oc wait pod/"$mock_pod" -n "$NAMESPACE" --for=condition=Ready --timeout=120s; then | ||
| echo "[e2e-ops] WARN: $mock_pod not Ready — redeploying mock TLS inference..." | ||
| cmd_delete_e2e_mock_tls_inference | ||
| if ! _deploy_e2e_mock_tls_inference_pod; then | ||
| return 1 | ||
| fi | ||
| fi | ||
|
|
||
| if _sync_mock_tls_certs_from_running_pod; then | ||
| echo "✓ Secret e2e-mock-tls-certs updated" | ||
| return 0 | ||
| fi | ||
|
|
||
| if [[ $pass -eq 1 ]]; then | ||
| echo "[e2e-ops] WARN: mock TLS cert sync failed (oc exec/kubelet) — redeploying mock pod and retrying..." | ||
| cmd_delete_e2e_mock_tls_inference | ||
| if ! _deploy_e2e_mock_tls_inference_pod; then | ||
| return 1 | ||
| fi | ||
| continue | ||
| fi | ||
|
|
||
| e2e_ops_dump_pod_logs "$mock_pod" 80 | ||
| return 1 | ||
| done | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Unreachable code after for loop.
The return 1 on line 937 is unreachable because the for loop on lines 906-936 always exits via explicit return statements (lines 922, 935) or continue (line 931). While harmless, it may confuse future readers.
Suggested cleanup
e2e_ops_dump_pod_logs "$mock_pod" 80
return 1
done
- return 1
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for pass in 1 2; do | |
| if ! oc get pod "$mock_pod" -n "$NAMESPACE" &>/dev/null; then | |
| echo "[e2e-ops] WARN: $mock_pod not found — deploying mock TLS inference..." | |
| if ! _deploy_e2e_mock_tls_inference_pod; then | |
| return 1 | |
| fi | |
| elif ! oc wait pod/"$mock_pod" -n "$NAMESPACE" --for=condition=Ready --timeout=120s; then | |
| echo "[e2e-ops] WARN: $mock_pod not Ready — redeploying mock TLS inference..." | |
| cmd_delete_e2e_mock_tls_inference | |
| if ! _deploy_e2e_mock_tls_inference_pod; then | |
| return 1 | |
| fi | |
| fi | |
| if _sync_mock_tls_certs_from_running_pod; then | |
| echo "✓ Secret e2e-mock-tls-certs updated" | |
| return 0 | |
| fi | |
| if [[ $pass -eq 1 ]]; then | |
| echo "[e2e-ops] WARN: mock TLS cert sync failed (oc exec/kubelet) — redeploying mock pod and retrying..." | |
| cmd_delete_e2e_mock_tls_inference | |
| if ! _deploy_e2e_mock_tls_inference_pod; then | |
| return 1 | |
| fi | |
| continue | |
| fi | |
| e2e_ops_dump_pod_logs "$mock_pod" 80 | |
| return 1 | |
| done | |
| return 1 | |
| } | |
| for pass in 1 2; do | |
| if ! oc get pod "$mock_pod" -n "$NAMESPACE" &>/dev/null; then | |
| echo "[e2e-ops] WARN: $mock_pod not found — deploying mock TLS inference..." | |
| if ! _deploy_e2e_mock_tls_inference_pod; then | |
| return 1 | |
| fi | |
| elif ! oc wait pod/"$mock_pod" -n "$NAMESPACE" --for=condition=Ready --timeout=120s; then | |
| echo "[e2e-ops] WARN: $mock_pod not Ready — redeploying mock TLS inference..." | |
| cmd_delete_e2e_mock_tls_inference | |
| if ! _deploy_e2e_mock_tls_inference_pod; then | |
| return 1 | |
| fi | |
| fi | |
| if _sync_mock_tls_certs_from_running_pod; then | |
| echo "✓ Secret e2e-mock-tls-certs updated" | |
| return 0 | |
| fi | |
| if [[ $pass -eq 1 ]]; then | |
| echo "[e2e-ops] WARN: mock TLS cert sync failed (oc exec/kubelet) — redeploying mock pod and retrying..." | |
| cmd_delete_e2e_mock_tls_inference | |
| if ! _deploy_e2e_mock_tls_inference_pod; then | |
| return 1 | |
| fi | |
| continue | |
| fi | |
| e2e_ops_dump_pod_logs "$mock_pod" 80 | |
| return 1 | |
| done | |
| } |
🤖 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 `@tests/e2e-prow/rhoai/scripts/e2e-ops.sh` around lines 906 - 938, The `return
1` statement on line 937 that appears after the for loop (which iterates on pass
in 1 2 and closes with done on line 936) is unreachable because all control
paths through the loop either explicitly return (at lines 922 and 935) or
continue in a way that eventually leads to an explicit return. Remove this
unreachable `return 1` statement to clean up the code and avoid confusion for
future readers.
| if scenario.status != "failed": | ||
| return |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For behave version 1.3.0, does Scenario.statusreturn aStatusenum (not a string), and what comparison is recommended inafter_scenario hooks?
💡 Result:
Yes, in behave version 1.3.0, Scenario.status returns a Status enum (specifically from behave.model_core.Status), not a string [1][2]. For comparisons in after_scenario hooks, it is recommended to import the Status class and compare the scenario's status against the enum members [3]. Example usage in your environment.py: from behave.model_core import Status def after_scenario(context, scenario): if scenario.status == Status.failed: # Perform actions for failed scenarios pass elif scenario.status == Status.passed: # Perform actions for passed scenarios pass Note that Scenario.status is a read-only property [4][5]. Attempting to assign a value to it directly will result in an AttributeError [6][7]. If you need to check specific statuses, you should use the enum members provided by Status (e.g., Status.passed, Status.failed, Status.skipped, Status.untested) [8][9].
Citations:
- 1: https://github.com/behave/behave/blob/69cda59c/CHANGES.rst
- 2: https://github.com/behave/behave/blob/master/CHANGES.rst
- 3: https://stackoverflow.com/questions/35284828/how-to-get-the-status-of-the-scenario-ran-in-behave
- 4: https://behave.readthedocs.io/en/stable/api.html?highlight=after_all
- 5: https://behave.readthedocs.io/en/stable/api/
- 6: https://stackoverflow.com/questions/60612313/how-to-change-the-status-of-a-scenario-in-behave-python
- 7: How to change the status of a scenario in behave python behave/behave#816
- 8: https://behave.readthedocs.io/en/stable/appendix.status/
- 9: https://behave.readthedocs.io/en/latest/appendix.status/
🏁 Script executed:
# Inspect the environment.py file around lines 260-261
fd environment.py tests/e2e/features | head -1 | xargs -I {} cat -n {} | sed -n '250,270p'Repository: lightspeed-core/lightspeed-stack
Length of output: 1037
🏁 Script executed:
# Search for Status enum imports and usage in behave documentation/code
rg "Status\." tests/e2e/features/ -A 2 | head -30Repository: lightspeed-core/lightspeed-stack
Length of output: 58
🌐 Web query:
behave Status enum members list - what are all available status values in behave.model_core.Status
💡 Result:
All available status values in behave.model_core.Status are: 1) untested 2) untested_pending 3) untested_undefined 4) skipped 5) passed 6) failed 7) error 8) hook_error 9) pending 10) pending_warn 11) undefined [1][2] These are the members listed in Behave’s “Status Values” documentation for the behave.model_core.Status enum-class. [1][2]
Citations:
- 1: https://behave.readthedocs.io/en/stable/appendix.status/
- 2: https://behave.readthedocs.io/en/latest/appendix.status/
Replace string comparison with Behave Status enum members.
Line 260 compares scenario.status to the string "failed", but scenario.status returns a Status enum object from behave.model_core. This comparison will always be false, causing the function to return early regardless of scenario outcome and skip log dumping entirely.
💡 Suggested fix
from behave.model import Feature, Scenario
+from behave.model_core import Status
@@
- if scenario.status != "failed":
+ if scenario.status not in (Status.failed, Status.error, Status.hook_error):
return🤖 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 `@tests/e2e/features/environment.py` around lines 260 - 261, The comparison at
line 260 in the environment.py file compares scenario.status to the string
"failed", but scenario.status is a Status enum object from behave.model_core,
not a string. This comparison will always evaluate to false, causing the
function to return early and skip the intended log dumping logic. Replace the
string literal "failed" with the appropriate Status enum member from
behave.model_core (such as Status.failed). If the Status enum is not already
imported, add the necessary import statement at the top of the file.
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)
tests/e2e-prow/rhoai/scripts/e2e-ops.sh (1)
358-361: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueRedundant conditional — simplify to direct assignment.
These lines are already inside the
E2E_KONFLUX_E2E == 1block (line 335), so the nested check always evaluates to true. The initial assignmentllama_wait_attempts=90is effectively dead code within this branch.♻️ Suggested simplification
- local llama_wait_attempts=90 - if [[ "${E2E_KONFLUX_E2E:-0}" == "1" ]]; then - llama_wait_attempts=360 - fi + local llama_wait_attempts=360🤖 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 `@tests/e2e-prow/rhoai/scripts/e2e-ops.sh` around lines 358 - 361, The conditional check for `E2E_KONFLUX_E2E` at lines 358-361 is redundant because this code block already exists within an outer `E2E_KONFLUX_E2E == 1` block (mentioned at line 335), making the nested condition always true and the initial `llama_wait_attempts=90` assignment dead code. Remove the redundant if statement and directly assign `llama_wait_attempts=360` without the conditional wrapper, since you are already guaranteed to be in the E2E_KONFLUX_E2E environment at this point in the code.
🤖 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 `@tests/e2e-prow/rhoai/scripts/e2e-ops.sh`:
- Around line 358-361: The conditional check for `E2E_KONFLUX_E2E` at lines
358-361 is redundant because this code block already exists within an outer
`E2E_KONFLUX_E2E == 1` block (mentioned at line 335), making the nested
condition always true and the initial `llama_wait_attempts=90` assignment dead
code. Remove the redundant if statement and directly assign
`llama_wait_attempts=360` without the conditional wrapper, since you are already
guaranteed to be in the E2E_KONFLUX_E2E environment at this point in the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0c156159-6434-4d89-8982-4a66c2900d4b
📒 Files selected for processing (1)
tests/e2e-prow/rhoai/scripts/e2e-ops.sh
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build-pr
- GitHub Check: unit_tests (3.12)
- GitHub Check: unit_tests (3.13)
- GitHub Check: check_dependencies
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🔇 Additional comments (6)
tests/e2e-prow/rhoai/scripts/e2e-ops.sh (6)
942-942: Unreachable code — already flagged.This
return 1after thedoneis unreachable; all loop iterations exit via explicitreturnstatements.
52-75: LGTM!
77-99: LGTM!
429-431: LGTM!
475-475: LGTM!Also applies to: 502-509
448-462: LGTM!Also applies to: 834-905, 945-973, 999-1089
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)
tests/e2e-prow/rhoai/scripts/e2e-ops.sh (1)
358-361: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueRedundant conditional check.
The inner
if [[ "${E2E_KONFLUX_E2E:-0}" == "1" ]]on line 359 is nested inside the block starting at line 335 that already verified the same condition. This check always evaluates to true here.🧹 Suggested simplification
- local llama_wait_attempts=90 - if [[ "${E2E_KONFLUX_E2E:-0}" == "1" ]]; then - llama_wait_attempts=360 - fi + local llama_wait_attempts=360🤖 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 `@tests/e2e-prow/rhoai/scripts/e2e-ops.sh` around lines 358 - 361, The conditional check `if [[ "${E2E_KONFLUX_E2E:-0}" == "1" ]]` at line 359 is redundant because it is already verified in an outer block that precedes it. Remove this inner conditional statement entirely and execute the assignment `llama_wait_attempts=360` unconditionally, since it is guaranteed that E2E_KONFLUX_E2E equals "1" at this point in the code. Keep the `llama_wait_attempts=90` initialization outside, and only perform the adjustment to 360 within the outer already-verified block.
🤖 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 `@tests/e2e-prow/rhoai/scripts/e2e-ops.sh`:
- Around line 358-361: The conditional check `if [[ "${E2E_KONFLUX_E2E:-0}" ==
"1" ]]` at line 359 is redundant because it is already verified in an outer
block that precedes it. Remove this inner conditional statement entirely and
execute the assignment `llama_wait_attempts=360` unconditionally, since it is
guaranteed that E2E_KONFLUX_E2E equals "1" at this point in the code. Keep the
`llama_wait_attempts=90` initialization outside, and only perform the adjustment
to 360 within the outer already-verified block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9f0dfef0-32eb-449b-950d-9a4c65a21c62
📒 Files selected for processing (2)
tests/e2e-prow/rhoai/pipeline-konflux.shtests/e2e-prow/rhoai/scripts/e2e-ops.sh
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: unit_tests (3.13)
- GitHub Check: unit_tests (3.12)
- GitHub Check: build-pr
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🔇 Additional comments (13)
tests/e2e-prow/rhoai/pipeline-konflux.sh (1)
150-151: PVC recreation still races with asynchronous deletion.Line 150 deletes the claim and Line 151 immediately reapplies it; deletion is asynchronous, so recreation can intermittently fail while the old PVC is still
Terminating. Please add an explicit wait-for-gone step before reapplying.tests/e2e-prow/rhoai/scripts/e2e-ops.sh (12)
937-937: Unreachable code after for loop (already flagged).The
return 1on line 937 is unreachable because all control paths through the for loop exit via explicitreturnstatements orcontinue. This was already identified in a previous review.
1-99: LGTM!
101-231: LGTM!
233-319: LGTM!
400-442: LGTM!
444-467: LGTM!
469-532: LGTM!
534-672: LGTM!
674-827: LGTM!
829-878: LGTM!
966-985: LGTM!
987-1084: LGTM!
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)
tests/e2e/utils/prow_utils.py (1)
152-156: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winError detail inconsistent with
restart_podpattern.
restart_pod(lines 120-131) constructs a rich detail string combining stdout+stderr with smart truncation. Here, onlyresult.stderris passed. Per the docstring (line 43), e2e-ops.sh writes most output—including errors—to stdout. Ifrestore_llama_stack_podfails, the exception may lack useful diagnostics.Consider applying the same detail construction:
♻️ Suggested alignment
if result.returncode != 0: print(result.stderr, end="") + combined = f"{result.stdout or ''}\n{result.stderr or ''}".strip() + if "========== failure logs:" in combined: + detail = combined + else: + detail = "\n".join(combined.splitlines()[-40:]) if combined else "" + detail = detail or f"exit {result.returncode}" raise subprocess.CalledProcessError( - result.returncode, "restart-llama-stack", result.stderr + result.returncode, "restart-llama-stack", detail )🤖 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 `@tests/e2e/utils/prow_utils.py` around lines 152 - 156, The error handling for the restart-llama-stack command is inconsistent with the pattern used in the restart_pod function. Instead of passing only result.stderr to the CalledProcessError, apply the same detail construction logic that restart_pod uses (lines 120-131) which combines stdout and stderr with smart truncation. Replace the result.stderr argument with a constructed detail string that merges result.stdout and result.stderr, matching the pattern from restart_pod to ensure complete error diagnostics are captured since e2e-ops.sh writes most output to stdout.
🤖 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 `@tests/e2e/utils/prow_utils.py`:
- Around line 152-156: The error handling for the restart-llama-stack command is
inconsistent with the pattern used in the restart_pod function. Instead of
passing only result.stderr to the CalledProcessError, apply the same detail
construction logic that restart_pod uses (lines 120-131) which combines stdout
and stderr with smart truncation. Replace the result.stderr argument with a
constructed detail string that merges result.stdout and result.stderr, matching
the pattern from restart_pod to ensure complete error diagnostics are captured
since e2e-ops.sh writes most output to stdout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0311d509-cce6-407f-a6a1-2e9c2a3d6535
📒 Files selected for processing (2)
tests/e2e-prow/rhoai/manifests/lightspeed/llama-stack-openai.yamltests/e2e/utils/prow_utils.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: unit_tests (3.13)
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: integration_tests (3.13)
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: integration_tests (3.12)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🧰 Additional context used
📓 Path-based instructions (2)
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Usepytest.mark.asynciomarker for async tests
Files:
tests/e2e/utils/prow_utils.py
tests/e2e/**/*.{py,feature}
📄 CodeRabbit inference engine (AGENTS.md)
Use behave (BDD) framework for end-to-end testing with Gherkin feature files
Files:
tests/e2e/utils/prow_utils.py
🧠 Learnings (2)
📚 Learning: 2026-02-19T10:06:50.647Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 1181
File: tests/e2e-prow/rhoai/manifests/lightspeed/mock-jwks.yaml:32-34
Timestamp: 2026-02-19T10:06:50.647Z
Learning: In the rhoai tests under tests/e2e-prow/rhoai/manifests, avoid static ConfigMap definitions for mock-jwks-script and mcp-mock-server-script since these ConfigMaps are created dynamically by the pipeline.sh deployment script using 'oc create configmap'. Ensure there are no static ConfigMap resources for these names in the manifests. If such ConfigMaps are added in the future, coordinate with the pipeline to reflect dynamic creation or adjust tests to rely on the dynamic provisioning.
Applied to files:
tests/e2e-prow/rhoai/manifests/lightspeed/llama-stack-openai.yaml
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.
Applied to files:
tests/e2e-prow/rhoai/manifests/lightspeed/llama-stack-openai.yaml
🔇 Additional comments (3)
tests/e2e-prow/rhoai/manifests/lightspeed/llama-stack-openai.yaml (2)
44-46: Fast-path cache validation still missingllama_stack_configuration.pycheck.The cache-hit conditional at lines 44-46 does not verify
/opt/app-root/llama_stack_configuration.pyexists, but line 186 executes it unconditionally. A partial PVC cache (e.g., interrupted full provisioning after line 93 but before line 94) will cause crash loops.
57-61: LGTM!tests/e2e/utils/prow_utils.py (1)
145-147: LGTM!
2 participants
Description
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
Improvements
Tests