OSAC-3011: add LVMS local storage provider for dev/CI environments

[zszabo-rh]

Summary

Adds automatic LVMS-based local storage for dev/CI/E2E environments. When lvms.enabled: true, a post-install Helm hook registers LVMS as a StorageBackend + StorageTier in the fulfillment service. From that point, any tenant onboarded by the operator automatically gets a per-tenant StorageClass via AAP — no manual storage configuration. This replaces the tenant=Default shared StorageClass fallback (removed in the operator change).

Migrates from the now-archived standalone repos (osac-aap#454, osac-installer#474, osac-operator#397). Those PRs are reviewed and have received conceptual /lgtm — use them as the primary reference for context and design rationale.

What changed from the standalone PRs

osac-aap — architectural simplification

• No playbook modifications — the old PRs modified 4 playbooks directly (prohibited pattern). The monorepo dispatcher in osac.service.storage_provider routes automatically via the provider field in STORAGE_TIERS. Only the new role is needed.
• Provider renamed local_lvms → lvms — the old name contains an underscore which fails the dispatcher's DNS-label validation regex. lvms follows the same pattern as vast (provider: vast → osac.templates.vast_storage).
• Role renamed local_lvms_storage → lvms_storage (same reason — dispatcher constructs osac.templates.{{ provider }}_storage)
• All role-internal variables renamed from local_lvms_storage_* to lvms_storage_*

osac-operator — wider scope than standalone

• The sentinel removal now spans 3 files (standalone was 1): tenant_names.go (constant), storage_tier_resolution.go (Default SC list query + fallback resolution block), storage_controller.go (Default fallback in AAP path + mapStorageClassToTenant Default branch + allTenantReconcileRequests helper)

osac-installer — one extra file + SCC fix

• lvms: section added to Phase 3 chart values.yaml + values.schema.json — it only existed in the Phase 2 prereqs chart previously; both phases must gate on the same flag
• runAsUser: 1001 removed from the hook's securityContext — the UID falls outside the namespace's SCC-allocated range on OpenShift; let OpenShift assign automatically (matches all other hooks)

Dependency

This PR should be rebased on top of OSAC-3547 (#97) (EDA rename: ansible_eda.event.* → osac_job_vars.*) if that merges first.

Test plan

• register-local-storage hook fires on lvms.enabled: true and creates StorageBackend with provider: lvms (HTTP 200, verified on edge-17)
• StorageTier: local created and STORAGE_TIER_STATE_ACTIVE
• Operator detects backend and triggers osac-create-tenant-storage-backend AAP job (confirmed in operator logs)
• SCC fix: hook pod starts without UID rejection
• Full E2E dispatch to lvms_storage role (AAP project sync issue in test environment, not code-related — same dispatch path as vast_storage which is proven)
• Sentinel removal: no-AAP environments now require per-tenant labeled SCs (documented; tenant=Default fallback removed as designed)

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added optional LVMS storage configuration for deployments.
  • Added automated registration of local storage backends and tiers.
  • Added tenant-specific StorageClass creation and lifecycle management.
  • Added safe setup and teardown workflows for tenant storage resources.

• Bug Fixes

  • Prevented tenant storage from falling back to shared default StorageClasses.
  • Improved handling of existing LVMS installations, discovery failures, and cleanup errors.
  • Added validation and reporting for ambiguous or unavailable tenant storage tiers.

[openshift-ci-robot]

@zszabo-rh: This pull request references OSAC-3011 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 epic to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

Migrates OSAC-3011 implementation from the now-archived standalone repos (osac-aap#454, osac-installer#474, osac-operator#397) to the monorepo. Those PRs were reviewed and have Roy's conceptual /lgtm — use them as the primary reference for context and design rationale.

What changed from the standalone PRs

osac-aap — architectural simplification

• No playbook modifications — the old PRs modified 4 playbooks directly (prohibited pattern). The monorepo dispatcher in osac.service.storage_provider routes automatically via the provider field in STORAGE_TIERS. Only the new role is needed.
• Provider renamed local_lvms → lvms — the old name contains an underscore which fails the dispatcher's DNS-label validation regex. lvms follows the same pattern as vast (provider: vast → osac.templates.vast_storage).
• Role renamed local_lvms_storage → lvms_storage (same reason — dispatcher constructs osac.templates.{{ provider }}_storage)
• All role-internal variables renamed from local_lvms_storage_* to lvms_storage_*

osac-operator — wider scope than standalone

• The sentinel removal now spans 3 files (standalone was 1): tenant_names.go (constant), storage_tier_resolution.go (Default SC list query + fallback resolution block), storage_controller.go (Default fallback in AAP path + mapStorageClassToTenant Default branch + allTenantReconcileRequests helper)

osac-installer — one extra file + SCC fix

• lvms: section added to Phase 3 chart values.yaml + values.schema.json — it only existed in the Phase 2 prereqs chart previously; both phases must gate on the same flag
• runAsUser: 1001 removed from the hook's securityContext — the UID falls outside the namespace's SCC-allocated range on OpenShift; let OpenShift assign automatically (matches all other hooks)

Dependency

This PR should be rebased on top of OSAC-3547 (Roy's EDA rename: ansible_eda.event.* → osac_job_vars.*) if that merges first. Our playbooks are untouched (no direct dispatch), but the role's variable names follow the same conventions and the operator-side aap_provider.go is also modified by #97.

Test plan

• register-local-storage hook fires on lvms.enabled: true and creates StorageBackend with provider: lvms (HTTP 200, verified on edge-17)
• StorageTier: local created and STORAGE_TIER_STATE_ACTIVE
• Operator detects backend and triggers osac-create-tenant-storage-backend AAP job (confirmed in operator logs)
• SCC fix: hook pod starts without UID rejection
• Full E2E dispatch to lvms_storage role (AAP project sync issue in test environment, not code-related — same dispatch path as vast_storage which is proven)
• Sentinel removal: no-AAP environments now require per-tenant labeled SCs (documented; tenant=Default fallback removed as designed)

🤖 Generated with Claude Code

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.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zszabo-rh

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:

• osac-aap/OWNERS [zszabo-rh]
• osac-installer/OWNERS [zszabo-rh]
• osac-operator/OWNERS [zszabo-rh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds LVMS provider automation, tenant StorageClass lifecycle management, local storage registration during installation, and tenant-only StorageClass resolution in the operator.

Changes

LVMS storage integration

Layer / File(s)	Summary
Provider contract and tenant setup osac-aap/.../lvms_storage/defaults/main.yaml, osac-aap/.../lvms_storage/meta/osac.yaml, osac-aap/.../lvms_storage/tasks/setup.yaml	Defines LVMS defaults and metadata. Validates provider tiers and tenant names. Creates the tenant configuration Secret and publishes LVMS configuration.
Tenant StorageClass lifecycle osac-aap/.../lvms_storage/tasks/ensure_storage_class.yaml, osac-aap/.../lvms_storage/tasks/teardown_backend.yaml, osac-aap/.../lvms_storage/tasks/teardown_cluster_storage.yaml	Creates missing tenant StorageClasses with TopoLVM settings and labels. Exposes their names. Removes tenant Secrets and StorageClasses during teardown.
Installer LVMS registration osac-installer/charts/osac-prereqs/files/hooks/configure-lvms.sh, osac-installer/charts/osac/templates/hooks/register-local-storage.yaml, osac-installer/charts/osac/values.yaml, osac-installer/charts/osac/values.schema.json	Adds LVMS configuration and a conditional registration hook. Handles existing LVMS resources, waits for the StorageClass, and registers the local StorageBackend and StorageTier.
Tenant-only storage resolution osac-operator/internal/controller/storage_controller.go, osac-operator/internal/controller/storage_tier_resolution.go, osac-operator/internal/controller/tenant_names.go, osac-operator/internal/controller/storage_controller_test.go	Removes shared Default StorageClass fallback. Resolves only tenant-specific StorageClasses and updates provisioning and readiness tests.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Installer as LVMS installer
  participant Kubernetes as Kubernetes API
  participant StorageAPI as Storage API
  participant Operator as Storage controller
  Installer->>Kubernetes: create LVMS resources and wait for StorageClass
  Installer->>StorageAPI: create or retrieve local StorageBackend
  StorageAPI-->>Installer: backend ID
  Installer->>StorageAPI: create local StorageTier
  Operator->>Kubernetes: list tenant-specific StorageClasses
  Kubernetes-->>Operator: tenant StorageClasses
  Operator->>Operator: resolve tenant tiers without shared fallback

Loading

Suggested labels: approved, jira/valid-reference, lgtm, storage

Suggested reviewers: adriengentil, danmanor, akshaynadkarni

---

Caution

Pre-merge checks failed

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

• Ignore

❌ Failed checks (3 errors, 1 warning)

Check name	Status	Explanation	Resolution
No-Hardcoded-Secrets	❌ Error	The new Helm hook hardcodes credential fields as username/password="n/a" in its StorageBackend POST payload at register-local-storage.yaml:47; this is not the admin/admin exception.	Remove the hardcoded username/password fields or obtain valid credentials through a secret reference; use an API-supported non-credential representation for LVMS.
No-Injection-Vectors	❌ Error	The hook interpolates API-derived BACKEND_ID into a Python -c program at line 89; quotes can alter the program and execute arbitrary Python code.	Pass BACKEND_ID as argv or stdin and compare it as parsed data. Do not concatenate API values into Python source; validate the identifier format as well.
No-Sensitive-Data-In-Logs	❌ Error	New hooks print raw oc/API error bodies, and LVMS teardown logs Ansible error messages; these may expose internal hostnames or other sensitive response data.	Do not log raw response bodies or error messages. Log only sanitized status codes and fixed diagnostics, and add no_log to Secret operations where results can contain secret data.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (7 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the addition of an LVMS local storage provider for development and CI environments.
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.
No-Weak-Crypto	✅ Passed	The full PR diff adds no MD5, SHA1, DES, RC4, Blowfish, ECB, crypto API, or custom crypto; AUTH_TOKEN is only sent as a bearer header and never compared.
Container-Privileges	✅ Passed	The PR adds no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation=true setting; the new hook denies escalation and drops all capabilities.
Ai-Attribution	✅ Passed	The PR mentions Claude Code, and all 8 PR commits contain Assisted-by: Claude Code <noreply@anthropic.com>; no AI Co-Authored-By trailers were found.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

putComment timed out

[coderabbitai]

Actionable comments posted: 3

🤖 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
`@osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml`:
- Around line 19-25: Update the tier.name validation task in
ensure_storage_class.yaml to enforce Kubernetes DNS-compatible StorageClass
names: allow only lowercase alphanumeric characters, hyphens, and dots, with
1–253 characters and alphanumeric characters at both ends. Remove uppercase
letters and underscores from the regex before tier.name is rendered into
StorageClass metadata.name.

In
`@osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/teardown_backend.yaml`:
- Around line 18-27: In teardown_backend.yaml lines 18-27, replace the
failed_when: false path so Secret deletion failures preserve their failed state
and are included in the teardown result while retaining the warning. In
teardown_cluster_storage.yaml lines 31-37, make the summary list only
successfully deleted StorageClasses and mark or return teardown failure when any
discovered resource remains. Verify the result propagates through the teardown
dispatcher without hiding failures during target-cluster destruction.

In `@osac-installer/charts/osac/templates/hooks/register-local-storage.yaml`:
- Around line 54-60: Update the HTTP 409 handling in the local StorageBackend
and tier registration flows to fetch both existing resources and validate their
specifications against the requested provider lvms, BACKEND_ID reference, and
protocol 2. Reconcile mismatches where supported; otherwise fail with a clear
error instead of treating the conflict as success, while preserving success for
matching resources.

🪄 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: Pro Plus

Run ID: fa04fdda-4df8-4f30-85da-e1dc5fbbff58

📥 Commits

Reviewing files that changed from the base of the PR and between c8ef391 and 68eeb72.

📒 Files selected for processing (13)

• osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/defaults/main.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/meta/osac.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/setup.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/teardown_backend.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/teardown_cluster_storage.yaml
• osac-installer/charts/osac-prereqs/files/hooks/configure-lvms.sh
• osac-installer/charts/osac/templates/hooks/register-local-storage.yaml
• osac-installer/charts/osac/values.schema.json
• osac-installer/charts/osac/values.yaml
• osac-operator/internal/controller/storage_controller.go
• osac-operator/internal/controller/storage_tier_resolution.go
• osac-operator/internal/controller/tenant_names.go

💤 Files with no reviewable changes (1)

• osac-operator/internal/controller/tenant_names.go

[zszabo-rh]

E2E test status update

Hook, operator detection, and AAP job dispatch are all verified. The full E2E run (StorageBackend → AAP setup action → hub Secret → StorageClass) is blocked by a dependency on #99 (OSAC-1992).

Root cause: the monorepo's osac-storage-operations-ig InstanceGroup has OSAC_STORAGE_CONFIG_NAMESPACE in its pod spec but no STORAGE_TIERS — that env var is intentionally absent in the monorepo because #99 moves tier definitions from the static env var to the operator event payload (ansible_eda.event.storage_tier_definitions). Without #99, the playbook fails at the Validate STORAGE_TIERS env var is configured check.

The E2E test will pass cleanly once #99 merges. The lvms_storage role itself is not the issue — the dispatch path and role structure are correct.

Summary of what is verified on edge-17:

• ✅ register-local-storage hook fires and creates StorageBackend{provider: lvms} + StorageTier
• ✅ Operator detects backend and triggers osac-create-tenant-storage-backend AAP job
• ✅ SCC fix: hook pod starts with auto-assigned UID
• ✅ osac-publish-templates and osac-config-as-code succeed with monorepo paths
• ❌ Full AAP dispatch: blocked on OSAC-1992: read tier definitions and backend credentials from AAP extra_vars #99 for STORAGE_TIERS

[zszabo-rh]

/coderabbitai review

[rgolangh]

@zszabo-rh it looks like the tier isn't passed

TASK [Fail if tenant storage classes not available] ****************************
  fatal: [localhost]: FAILED! => {"changed": false, "msg": "ComputeInstance 'vm-59lmh' has no
  tenant_storage_classes available. Either the osac-operator CI controller should inject the
  resolved storageClasses list before triggering provisioning, or JIT storage provisioning
  via STORAGE_TIERS must succeed."}

[rgolangh]

/lgtm

[rgolangh]

/retest

[github-actions]

Re-triggered failed runs:

• E2E VMaaS Full Install (#31080145738)

[zszabo-rh]

/retest

[github-actions]

Re-triggered failed runs:

• E2E VMaaS Full Install (#31080145738)