fix: wait for llama-stack health condition#481
Merged
dbasunag merged 1 commit intoopendatahub-io:mainfrom Aug 1, 2025
Merged
Conversation
Fixes timing issue when the LlamaStackDistribition is not ready, causing the llama-stack service portforwarding to fail Signed-off-by: Jorge Garcia Oncins <jgarciao@redhat.com>
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughA call to Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/lgtm', '/wip', '/build-push-pr-image', '/hold', '/verified', '/cherry-pick'} |
Contributor
Author
|
/verified |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/rag/conftest.py (1)
79-87: Hard-coded timeout; prefer shared constant & double-check the required condition name
wait_for_conditionis invoked with a literaltimeout=240.
For consistency with the rest of the file (Timeout.TIMEOUT_1MIN,Timeout.TIMEOUT_2MIN), use the corresponding constant (e.g.Timeout.TIMEOUT_4MIN) or add one if it doesn’t exist. This avoids magic numbers and keeps all timeouts centrally tunable.Additionally, verify that the CR actually exposes a
"HealthCheck"condition—most OCP-style CRDs use"Ready". An incorrect condition name will cause an unnecessary 4-minute stall followed by a failure.- llama_stack_distribution.wait_for_condition(condition="HealthCheck", status="True", timeout=240) + llama_stack_distribution.wait_for_condition( + condition="HealthCheck", # confirm this is the correct condition + status="True", + timeout=Timeout.TIMEOUT_4MIN, # or introduce a constant equal to 240 s + )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/rag/conftest.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: israel-hdez
PR: opendatahub-io/opendatahub-tests#346
File: tests/model_serving/model_server/inference_graph/conftest.py:85-92
Timestamp: 2025-06-11T16:40:11.593Z
Learning: The helper `create_isvc` (used in tests/model_serving utilities) already waits until the created InferenceService reports Condition READY=True before returning, so additional readiness waits in fixtures are unnecessary.
Learnt from: jiripetrlik
PR: opendatahub-io/opendatahub-tests#335
File: tests/rag/test_rag.py:0-0
Timestamp: 2025-06-19T14:32:17.658Z
Learning: The `wait_for_replicas()` method from ocp_resources.deployment.Deployment raises an exception when timeout is reached rather than returning a boolean value, so it doesn't need explicit return value checking in tests.
Learnt from: jiripetrlik
PR: opendatahub-io/opendatahub-tests#335
File: tests/rag/test_rag.py:0-0
Timestamp: 2025-06-19T14:32:17.658Z
Learning: The `wait_for_replicas()` method from ocp_resources.deployment.Deployment raises an exception when timeout is reached rather than returning a boolean value, so it doesn't need explicit return value checking in tests.
📚 Learning: the helper `create_isvc` (used in tests/model_serving utilities) already waits until the created inf...
Learnt from: israel-hdez
PR: opendatahub-io/opendatahub-tests#346
File: tests/model_serving/model_server/inference_graph/conftest.py:85-92
Timestamp: 2025-06-11T16:40:11.593Z
Learning: The helper `create_isvc` (used in tests/model_serving utilities) already waits until the created InferenceService reports Condition READY=True before returning, so additional readiness waits in fixtures are unnecessary.
Applied to files:
tests/rag/conftest.py
📚 Learning: the `wait_for_replicas()` method from ocp_resources.deployment.deployment raises an exception when t...
Learnt from: jiripetrlik
PR: opendatahub-io/opendatahub-tests#335
File: tests/rag/test_rag.py:0-0
Timestamp: 2025-06-19T14:32:17.658Z
Learning: The `wait_for_replicas()` method from ocp_resources.deployment.Deployment raises an exception when timeout is reached rather than returning a boolean value, so it doesn't need explicit return value checking in tests.
Applied to files:
tests/rag/conftest.py
jiripetrlik
approved these changes
Aug 1, 2025
dbasunag
approved these changes
Aug 1, 2025
|
Status of building tag latest: success. |
Contributor
Author
|
/cherry-pick 2.23 |
rhods-ci-bot
pushed a commit
that referenced
this pull request
Aug 1, 2025
Fixes timing issue when the LlamaStackDistribition is not ready, causing the llama-stack service portforwarding to fail Signed-off-by: Jorge Garcia Oncins <jgarciao@redhat.com>
Contributor
|
Cherry pick action created PR #482 successfully 🎉! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes timing issue when the LlamaStackDistribition is not ready, causing the llama-stack service portforwarding to fail