OSAC-1768: Enable storage controller in CI values - #336
openshift-merge-bot[bot] merged 1 commit into
Conversation
|
@eliorerz: This pull request references OSAC-1768 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eliorerz 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 |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (4)
WalkthroughUpdated the operator checkout, image tag overrides, and enabled ChangesOperator update and controller enablement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ 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 |
e87658e to
c81b24a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Enable storageController in both vmaas-ci and caas-ci values so the operator populates Tenant.Status.StorageClasses, which is required for ComputeInstance provisioning via AAP.
c81b24a to
03789ee
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@values/caas-ci/values.yaml`:
- Around line 20-25: The controllers.storageController flag is only present in
the CI values and is not yet wired into the chart, so it has no effect. Update
the chart schema and any templates that gate controller deployment to consume
controllers.storageController, using the existing controllers settings in the
values structure and the relevant controller rendering logic to enable/disable
the storage controller consistently.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c958dfe1-b4ce-4581-a2eb-b18a930ea706
📒 Files selected for processing (4)
base/kustomization.yamlbase/osac-operatorvalues/caas-ci/values.yamlvalues/vmaas-ci/values.yaml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 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 `@values/caas-ci/values.yaml`:
- Around line 20-25: The controllers.storageController flag is only present in
the CI values and is not yet wired into the chart, so it has no effect. Update
the chart schema and any templates that gate controller deployment to consume
controllers.storageController, using the existing controllers settings in the
values structure and the relevant controller rendering logic to enable/disable
the storage controller consistently.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c958dfe1-b4ce-4581-a2eb-b18a930ea706
📒 Files selected for processing (4)
base/kustomization.yamlbase/osac-operatorvalues/caas-ci/values.yamlvalues/vmaas-ci/values.yaml
🛑 Comments failed to post (1)
values/caas-ci/values.yaml (1)
20-25: 🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail schema_file="$(fd -p 'values.schema.json' charts . | head -n1 || true)" values_file="$(fd -p 'values.yaml' charts/osac charts . | head -n1 || true)" echo '--- storageController references ---' rg -n -C2 'storageController' charts values . || true if [[ -n "${schema_file}" ]]; then echo echo "--- schema hits: ${schema_file} ---" rg -n -C2 '"storageController"|storageController' "${schema_file}" || true fi if [[ -n "${values_file}" ]]; then echo echo "--- chart values hits: ${values_file} ---" rg -n -C2 'storageController' "${values_file}" || true fiRepository: osac-project/osac-installer
Length of output: 1358
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- files containing storageController ---' rg -n -C2 'storageController' charts values base overlays . || true schema='charts/osac/values.schema.json' values='charts/osac/ci/bundled-postgres-values.yaml' echo echo "--- schema excerpt ---" if [[ -f "$schema" ]]; then rg -n -C3 '"controllers"|storageController' "$schema" || true fi echo echo "--- bundled postgres values excerpt ---" if [[ -f "$values" ]]; then rg -n -C3 'storageController|controllers' "$values" || true fiRepository: osac-project/osac-installer
Length of output: 1522
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- files containing storageController ---' rg -n -C2 'storageController' charts values base overlays . || true schema='charts/osac/values.schema.json' values='charts/osac/ci/bundled-postgres-values.yaml' echo echo "--- schema excerpt ---" if [[ -f "$schema" ]]; then rg -n -C3 '"controllers"|storageController' "$schema" || true fi echo echo "--- bundled postgres values excerpt ---" if [[ -f "$values" ]]; then rg -n -C3 'storageController|controllers' "$values" || true fiRepository: osac-project/osac-installer
Length of output: 1522
Wire
controllers.storageControllerthrough the chart schema and templates. It only appears in the CI values files, so the flag has no effect until the chart actually consumes it.🤖 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 `@values/caas-ci/values.yaml` around lines 20 - 25, The controllers.storageController flag is only present in the CI values and is not yet wired into the chart, so it has no effect. Update the chart schema and any templates that gate controller deployment to consume controllers.storageController, using the existing controllers settings in the values structure and the relevant controller rendering logic to enable/disable the storage controller consistently.Source: Coding guidelines
2 participants
Summary
storageController: truein bothvmaas-ciandcaas-civaluesContext
The Storage Controller populates
Tenant.Status.StorageClasses, which the ComputeInstance Controller reads and passes to AAP astenant_storage_classes. Without it, all VM provisioning fails with "no tenant_storage_classes available".Test plan
Summary by CodeRabbit
New Features
Chores