Configure OGX with RAG#104
Conversation
|
Skipping CI for Draft Pull Request. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request transitions the OpenStack Lightspeed operator from static ConfigMap-mounted pod configurations to dynamically generated configurations assembled by init containers. New vector database collection and Python-based configuration generation scripts enable flexible metadata injection. The controller pod template, configuration generation logic, and reconciliation are refactored to use generated config volumes instead of pre-defined ConfigMaps. KUTTL tests are updated with pod-based validation assertions and expected configuration files. ChangesInit Container-Driven Configuration Generation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The PR involves substantial refactoring across deployment, configuration generation, and reconciliation logic with new init container orchestration, multiple new scripts, and restructured configuration schemas. The heterogeneous changes across different functional areas and the density of interconnected logic (init container setup, config generation, environment wiring, and test updates) create moderate complexity despite some sections being relatively straightforward (image tag updates, test assertions). Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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)
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 |
|
/test all |
|
/retest |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@internal/controller/lcore_deployment.go`:
- Around line 565-568: The environment variable LIGHTSPEED_STACK_CONFIG_PATH is
being set to VectorDBVolumeMountPath +
"/lightspeed_stack_config/lightspeed-stack.yaml" which mismatches the actual
path used elsewhere (VectorDBVolumeLightspeedStackConfigPath ->
"/vector-db-discovered-values/lightspeed-stack.yaml"); update the assignment to
use the same canonical constant VectorDBVolumeLightspeedStackConfigPath (or
change that constant to match if intended) so the pod/container and init/build
flow reference the identical path; locate the env var creation block where Name
== "LIGHTSPEED_STACK_CONFIG_PATH" and replace the concatenated path with
VectorDBVolumeLightspeedStackConfigPath.
In `@internal/controller/lcore_reconciler.go`:
- Around line 186-190: The current CreateOrPatch handler overwrites cm.Data with
only the new OGXConfigCMKey (in the controllerutil.CreateOrPatch closure), which
drops legacy ConfigMap keys during upgrades; instead update cm.Data by ensuring
it is non-nil, preserving any existing keys, and then set or overwrite the
OGXConfigCMKey entry to yamlData so legacy keys remain available; apply the same
fix to the other identical cm.Data reassignment occurrence in the file where
cm.Data is being replaced.
In `@test/kuttl/common/expected-configs/validate-config.sh`:
- Around line 14-15: The script dereferences CONFIG_TYPE="$1" and
EXPECTED_CONFIG="$2" while `set -u` is enabled, causing an unbound-variable exit
if arguments are missing; add an argument-count check at the top (e.g. test if
$# -lt 2) and print a clear usage message and exit with non-zero before any
access to $1/$2 so CONFIG_TYPE and EXPECTED_CONFIG are never expanded when
absent.
- Around line 57-65: The script currently assigns POD_NAME by taking the first
entry from `oc get pods` which is nondeterministic; update the `POD_NAME`
selection to deterministically pick a Running and Ready pod instead of the
arbitrary first item. Modify the `oc get pods` invocation in validate-config.sh
(the POD_NAME assignment) to filter pods by status.phase=Running (use
--field-selector=status.phase=Running) and/or by Ready condition, and if
multiple remain, sort by creationTimestamp (or choose the oldest Ready pod) via
jsonpath/jq to consistently select a single pod; ensure the rest of the script
still uses the POD_NAME variable and keep the existing error check for an empty
result.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27831f27-15a5-4f3e-825e-18252889defb
📒 Files selected for processing (39)
Makefileapi/v1beta1/openstacklightspeed_types.gobundle/manifests/openstack-lightspeed-operator.clusterserviceversion.yamlconfig/manager/manager.yamlhack/env.shinternal/controller/assets/vector_database_build.pyinternal/controller/assets/vector_database_collect.shinternal/controller/constants.gointernal/controller/lcore_config.gointernal/controller/lcore_deployment.gointernal/controller/lcore_reconciler.gointernal/controller/llama_stack_config.gotest/kuttl/common/expected-configs/lightspeed-stack-update.yamltest/kuttl/common/expected-configs/lightspeed-stack.yamltest/kuttl/common/expected-configs/ogx_config-update.yamltest/kuttl/common/expected-configs/ogx_config.yamltest/kuttl/common/expected-configs/validate-config.shtest/kuttl/common/openstack-lightspeed-instance/assert-lightspeed-stack-config.yamltest/kuttl/common/openstack-lightspeed-instance/assert-llama-stack-config.yamltest/kuttl/common/openstack-lightspeed-instance/assert-openstack-lightspeed-instance.yamltest/kuttl/common/openstack-lightspeed-instance/assert-pod-lightspeed-stack-config.yamltest/kuttl/common/openstack-lightspeed-instance/assert-pod-llama-stack-config.yamltest/kuttl/common/openstack-lightspeed-instance/errors-openstack-lightspeed-instance.yamltest/kuttl/tests/basic-openstack-lightspeed-configuration/03-assert-lightspeed-stack-config.yamltest/kuttl/tests/basic-openstack-lightspeed-configuration/03-assert-openstack-lightspeed-instance.yamltest/kuttl/tests/basic-openstack-lightspeed-configuration/04-assert-lightspeed-stack-config.yamltest/kuttl/tests/basic-openstack-lightspeed-configuration/04-assert-llama-stack-config.yamltest/kuttl/tests/basic-openstack-lightspeed-configuration/05-assert-llama-stack-config.yamltest/kuttl/tests/update-openstacklightspeed/03-assert-lightspeed-stack-config.yamltest/kuttl/tests/update-openstacklightspeed/03-assert-openstack-lightspeed-instance.yamltest/kuttl/tests/update-openstacklightspeed/04-assert-lightspeed-stack-config.yamltest/kuttl/tests/update-openstacklightspeed/04-assert-llama-stack-config.yamltest/kuttl/tests/update-openstacklightspeed/05-assert-llama-stack-config.yamltest/kuttl/tests/update-openstacklightspeed/06-update-openstack-lightspeed-instance.yamltest/kuttl/tests/update-openstacklightspeed/08-assert-lightspeed-stack-config-update.yamltest/kuttl/tests/update-openstacklightspeed/08-assert-openstacklightspeed-update.yamltest/kuttl/tests/update-openstacklightspeed/09-assert-lightspeed-stack-config-update.yamltest/kuttl/tests/update-openstacklightspeed/09-assert-llama-stack-config-update.yamltest/kuttl/tests/update-openstacklightspeed/10-assert-llama-stack-config-update.yaml
💤 Files with no reviewable changes (8)
- test/kuttl/tests/update-openstacklightspeed/03-assert-lightspeed-stack-config.yaml
- test/kuttl/common/openstack-lightspeed-instance/assert-llama-stack-config.yaml
- test/kuttl/tests/basic-openstack-lightspeed-configuration/03-assert-lightspeed-stack-config.yaml
- test/kuttl/tests/basic-openstack-lightspeed-configuration/04-assert-llama-stack-config.yaml
- test/kuttl/tests/update-openstacklightspeed/09-assert-llama-stack-config-update.yaml
- test/kuttl/tests/update-openstacklightspeed/04-assert-llama-stack-config.yaml
- test/kuttl/tests/update-openstacklightspeed/08-assert-lightspeed-stack-config-update.yaml
- test/kuttl/common/openstack-lightspeed-instance/assert-lightspeed-stack-config.yaml
| Runs as the second init container (`vector-database-config-build`), after | ||
| `vector_database_collect.sh`. It loads operator-provided base configs, walks every | ||
| vector DB directory left by the collect step, and writes merged configs back to | ||
| the shared volume. |
There was a problem hiding this comment.
Thanks for writing this. When I saw the name of the script I thought we were "building" the dbs... But we are "building" the config
|
Thanks @lpiwowar most skimmed thru the code but it does look good! Thanks for it |
This commit ensures that the deployed OGX and Lightspeed Stack is configured to run as RAG with an image provided in the OpenStackLightspeed instance or the default one set in the operator code. The main logic is built on top of initContainers and provides easy extension when we decide to introduce BYOK later. The logic works as follows. Before the Lightspeed Stack and OGX main containers are executed, the following initContainers are run: 1. vector-database-collect: Copies all vector database data (faiss_store.db, llama-stack.yaml, embeddings_model) to a shared volume across all containers in the pod. 2. vector-store-build: Generates the final ogx_config.yaml and lightspeed-stack.yaml files with injected RAG configuration based on the data from step 1. After the init containers complete, both Lightspeed Stack and OGX consume the config files produced by the init containers. Later when introducing BYOK support, we should address: - We should prevent copying the same embedding model multiple times into the shared volume when customers provide multiple container images with the same embedding model. - The embedding model configuration logic for OGX needs to be updated to support BYOK images. Currently, when an embedding model with the same name appears in multiple images, it is only introduced once into the final OGX config, which causes errors related to missing embedding models for some vector databases. The default vector database has been changed to: quay.io/openstack-lightspeed/rag-content-openstack:alpha-ogx-os-docs-2025.2 This image was built MANUALLY and contains ONLY data for the nova project. The default image should be replaced once the upstream pipeline is ready to build llamastack-faiss compatible vector databases. Assisted-By: Claude <noreply@anthropic.com>
This commit enhances the KUTTL tests to validate the generated ogx_config.yaml and lightspeed-stack.yaml files using the initContainers approach introduced in commit b2c6719. The implementation uses a workaround strategy: A) TestAssert steps extract the generated config files from the Lightspeed Stack and OGX containers B) The diff command compares the extracted config files against expected reference files prepared in test/kuttl This ensures comprehensive validation of the operator's config generation functionality during testing. Assisted-By: Claude <noreply@anthropic.com>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lpiwowar, umago 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 |
a8abf60
into
openstack-lightspeed:lcore-migration
This PR ensures that the deployed OGX and Lightspeed Stack is configured to run as RAG with an image provided in the OpenStackLightspeed instance or the default one set in the operator code.
The main logic is built on top of initContainers and provides easy extension when we decide to introduce BYOK later. The logic works as follows.
Before the Lightspeed Stack and OGX main containers are executed, the following initContainers are run:
vector-database-collect: Copies all vector database data (
faiss_store.db,llama-stack.yaml,embeddings_model) to a shared volume across all containers in the pod.vector-store-build: Generates the final
ogx_config.yamlandlightspeed-stack.yamlfiles with injected RAG configuration based on the data from step 1.After the init containers complete, both Lightspeed Stack and OGX consume the config files produced by the init containers.
Later when introducing BYOK support, we should address:
We should prevent copying the same embedding model multiple times into the shared volume when customers provide multiple container images with the same embedding model.
The embedding model configuration logic for OGX needs to be updated to support BYOK images. Currently, when an embedding model with the same name appears in multiple images, it is only introduced once into the final OGX config, which causes errors related to missing embedding models for some vector databases.
The default vector database has been changed to:
This image was built MANUALLY and contains ONLY data for the nova project. The default image should be replaced once the upstream pipeline is ready to build llamastack-faiss compatible vector databases.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores