From 091dfbce6180c86b50fcc2a5f730f6aefd7d3e29 Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Tue, 4 Aug 2026 08:37:10 +0200 Subject: [PATCH 01/14] OSAC-3011: add LVMS local storage provider for dev/CI environments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit osac-aap: add lvms_storage role - New storage provider role dispatched as osac.templates.lvms_storage (provider name: lvms, no underscores — passes DNS-label validation) - setup: creates per-tenant hub lifecycle marker Secret - ensure_storage_class: creates topolvm-backed StorageClasses per tier - teardown_cluster_storage / teardown_backend: cleanup actions - No playbook modifications needed — dispatcher routes automatically via STORAGE_TIERS provider field osac-operator: remove defaultStorageClassSentinel - Remove shared Default StorageClass fallback from getTenantStorageClasses - Remove Default branch from mapStorageClassToTenant and allTenantReconcileRequests - No-AAP path now resolves only tenant-specific labeled StorageClasses osac-installer: register-local-storage hook + configure-lvms.sh idempotency - configure-lvms.sh: skip LVMCluster creation and annotation when lvms-vg1 pre-exists (safe on MOC where Ceph is the intended default) - register-local-storage.yaml: post-install/upgrade hook that creates StorageBackend (provider: lvms) + StorageTier when lvms.enabled=true - charts/osac/values.yaml + values.schema.json: add lvms.enabled gate Assisted-by: Claude Code Signed-off-by: Zoltan Szabo --- .../roles/lvms_storage/defaults/main.yaml | 13 ++ .../roles/lvms_storage/meta/osac.yaml | 12 ++ .../tasks/ensure_storage_class.yaml | 84 ++++++++++++ .../roles/lvms_storage/tasks/setup.yaml | 40 ++++++ .../lvms_storage/tasks/teardown_backend.yaml | 27 ++++ .../tasks/teardown_cluster_storage.yaml | 45 +++++++ .../files/hooks/configure-lvms.sh | 30 +++-- .../hooks/register-local-storage.yaml | 122 ++++++++++++++++++ osac-installer/charts/osac/values.schema.json | 17 ++- osac-installer/charts/osac/values.yaml | 6 + .../internal/controller/storage_controller.go | 81 ++---------- .../controller/storage_tier_resolution.go | 34 +---- .../internal/controller/tenant_names.go | 5 - 13 files changed, 400 insertions(+), 116 deletions(-) create mode 100644 osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/defaults/main.yaml create mode 100644 osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/meta/osac.yaml create mode 100644 osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml create mode 100644 osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/setup.yaml create mode 100644 osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/teardown_backend.yaml create mode 100644 osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/teardown_cluster_storage.yaml create mode 100644 osac-installer/charts/osac/templates/hooks/register-local-storage.yaml diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/defaults/main.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/defaults/main.yaml new file mode 100644 index 0000000000..6acb7e2289 --- /dev/null +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/defaults/main.yaml @@ -0,0 +1,13 @@ +--- +# Hub Secret prefix for per-tenant lifecycle markers. +lvms_storage_tenant_config_secret_prefix: "lvms-tenant-config-" + +# Namespace for hub-cluster config Secrets — matches OSAC_STORAGE_CONFIG_NAMESPACE +# set in the storage-operations-ig pod spec (downward API metadata.namespace). +lvms_storage_config_namespace: "{{ lookup('env', 'OSAC_STORAGE_CONFIG_NAMESPACE') | default('osac-system', true) }}" + +# topolvm StorageClass provisioner name. +lvms_storage_provisioner: "topolvm.io" + +# LVMCluster device class name created by osac-installer's configure-lvms.sh hook. +lvms_storage_device_class: "vg1" diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/meta/osac.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/meta/osac.yaml new file mode 100644 index 0000000000..4937f6f407 --- /dev/null +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/meta/osac.yaml @@ -0,0 +1,12 @@ +--- +title: LVMS Storage Provider +description: > + Provisions per-tenant StorageClasses on the hub cluster using the LVMS (LVM Storage) + topolvm provisioner. No external backend or credentials required — LVMS is + installed on the hub by osac-installer when lvms.enabled=true. Hub cluster only; + CaaS guest cluster storage is out of scope. +template_type: storage_provider +implementation_strategy: lvms +capabilities: + provisioning_targets: + - vmaas diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml new file mode 100644 index 0000000000..48e3419573 --- /dev/null +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml @@ -0,0 +1,84 @@ +--- +# LVMS ensure_storage_class: create per-tenant labeled StorageClass on the hub cluster. +# Uses the topolvm provisioner backed by the lvms-vg1 VolumeGroup created by +# osac-installer's configure-lvms.sh hook. One StorageClass per tier. Idempotent. + +- name: Assert _provider_tiers is defined and non-empty + ansible.builtin.assert: + that: + - _provider_tiers is defined + - _provider_tiers | length > 0 + fail_msg: >- + _provider_tiers must be provided by the service role dispatcher. + +- name: Validate tenant_name is a valid DNS label + ansible.builtin.fail: + msg: "tenant_name '{{ tenant_name }}' is not a valid DNS label." + when: tenant_name is not regex('^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$') + +- name: Validate tier names are valid Kubernetes label values + ansible.builtin.fail: + msg: >- + tier.name '{{ item.name }}' is not a valid Kubernetes label value. + Must be 1-63 chars, lowercase alphanumeric, hyphens, underscores, or dots, + starting and ending with an alphanumeric character. + when: item.name is not regex('^[a-z0-9A-Z]([a-z0-9A-Z._-]{0,61}[a-z0-9A-Z])?$') + loop: "{{ _provider_tiers }}" + loop_control: + label: "{{ item.name }}" + +- name: Build expected StorageClass entries + ansible.builtin.set_fact: + _lvms_expected_sc_entries: >- + {% set entries = [] -%} + {% for tier in _provider_tiers -%} + {% set _ = entries.append({'name': 'osac-' ~ tenant_name ~ '-' ~ tier.name, 'tier': tier.name}) -%} + {% endfor -%} + {{ entries }} + _lvms_expected_sc_names: >- + {% set names = [] -%} + {% for tier in _provider_tiers -%} + {% set _ = names.append('osac-' ~ tenant_name ~ '-' ~ tier.name) -%} + {% endfor -%} + {{ names }} + +- name: Check existing tenant StorageClasses + kubernetes.core.k8s_info: + api_version: storage.k8s.io/v1 + kind: StorageClass + label_selectors: + - "osac.openshift.io/tenant={{ tenant_name }}" + - "app.kubernetes.io/managed-by=osac-aap" + register: _lvms_sc_check + +- name: Determine missing StorageClasses + ansible.builtin.set_fact: + _lvms_missing_sc_names: >- + {{ _lvms_expected_sc_names | difference( + _lvms_sc_check.resources | map(attribute='metadata.name') | list + ) }} + +- name: Create missing per-tenant StorageClasses + kubernetes.core.k8s: + state: present + definition: + apiVersion: storage.k8s.io/v1 + kind: StorageClass + metadata: + name: "{{ item.name }}" + labels: + osac.openshift.io/tenant: "{{ tenant_name }}" + osac.openshift.io/storage-tier: "{{ item.tier }}" + app.kubernetes.io/managed-by: osac-aap + provisioner: "{{ lvms_storage_provisioner }}" + parameters: + "topolvm.io/device-class": "{{ lvms_storage_device_class }}" + reclaimPolicy: Delete + volumeBindingMode: WaitForFirstConsumer + loop: "{{ _lvms_expected_sc_entries | selectattr('name', 'in', _lvms_missing_sc_names) | list }}" + loop_control: + label: "{{ item.name }}" + +- name: Set StorageClass names output + ansible.builtin.set_fact: + storage_provider_storage_class_names: "{{ _lvms_expected_sc_entries }}" diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/setup.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/setup.yaml new file mode 100644 index 0000000000..b4f6d4cb25 --- /dev/null +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/setup.yaml @@ -0,0 +1,40 @@ +--- +# LVMS setup: create a per-tenant hub Secret as a lifecycle marker. +# LVMS has no external backend credentials — the Secret exists solely so that +# the storage controller's hubSecretExists() check returns true, allowing it to +# advance from Stage 1 (StorageBackendReady) to Stage 2 (ensure_storage_class). +# Deleted by teardown_backend when the tenant is removed. + +- name: Assert _provider_tiers is defined and non-empty + ansible.builtin.assert: + that: + - _provider_tiers is defined + - _provider_tiers | length > 0 + fail_msg: >- + _provider_tiers must be provided by the service role dispatcher. + This role should not be called directly — use osac.service.storage_provider. + +- name: Validate tenant_name is a valid DNS label + ansible.builtin.fail: + msg: "tenant_name '{{ tenant_name }}' is not a valid DNS label." + when: tenant_name is not regex('^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$') + +- name: Create hub lifecycle marker Secret for tenant + kubernetes.core.k8s: + state: present + definition: + apiVersion: v1 + kind: Secret + metadata: + name: "{{ lvms_storage_tenant_config_secret_prefix }}{{ tenant_name }}" + namespace: "{{ lvms_storage_config_namespace }}" + labels: + osac.openshift.io/tenant: "{{ tenant_name }}" + app.kubernetes.io/managed-by: osac-aap + stringData: + provider: lvms + +- name: Set tenant config output (required by dispatcher) + ansible.builtin.set_fact: + storage_provider_tenant_config: + provider: lvms diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/teardown_backend.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/teardown_backend.yaml new file mode 100644 index 0000000000..0464ae0db7 --- /dev/null +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/teardown_backend.yaml @@ -0,0 +1,27 @@ +--- +# LVMS teardown_backend: remove the per-tenant hub lifecycle marker Secret. +# No external backend resources to clean up — LVMS is cluster-native. + +- name: Validate tenant_name is a valid DNS label + ansible.builtin.fail: + msg: "tenant_name '{{ tenant_name }}' is not a valid DNS label." + when: tenant_name is not regex('^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$') + +- name: Delete hub lifecycle marker Secret + kubernetes.core.k8s: + state: absent + api_version: v1 + kind: Secret + name: "{{ lvms_storage_tenant_config_secret_prefix }}{{ tenant_name }}" + namespace: "{{ lvms_storage_config_namespace }}" + register: _lvms_teardown_secret_result + failed_when: false + +- name: Warn if Secret deletion was unsuccessful + ansible.builtin.debug: + msg: "Warning: Secret deletion for tenant '{{ tenant_name }}' unsuccessful: {{ _lvms_teardown_secret_result.msg | default('unknown') }}" + when: _lvms_teardown_secret_result.failed | default(false) + +- name: Report teardown_backend summary + ansible.builtin.debug: + msg: "Backend teardown for tenant '{{ tenant_name }}' complete." diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/teardown_cluster_storage.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/teardown_cluster_storage.yaml new file mode 100644 index 0000000000..fdd34579f4 --- /dev/null +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/teardown_cluster_storage.yaml @@ -0,0 +1,45 @@ +--- +# LVMS teardown_cluster_storage: remove per-tenant StorageClasses from the hub cluster. +# Finds SCs by label selector. The target cluster may be unreachable (being destroyed) +# so failures are tolerated and reported rather than fatal. + +- name: Validate tenant_name is a valid DNS label + ansible.builtin.fail: + msg: "tenant_name '{{ tenant_name }}' is not a valid DNS label." + when: tenant_name is not regex('^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$') + +- name: Clean up tenant StorageClasses + block: + - name: Find tenant StorageClasses by label + kubernetes.core.k8s_info: + api_version: storage.k8s.io/v1 + kind: StorageClass + label_selectors: + - "osac.openshift.io/tenant={{ tenant_name }}" + - "app.kubernetes.io/managed-by=osac-aap" + register: _lvms_cleanup_sc_list + + - name: Delete tenant StorageClasses + kubernetes.core.k8s: + state: absent + api_version: storage.k8s.io/v1 + kind: StorageClass + name: "{{ item.metadata.name }}" + loop: "{{ _lvms_cleanup_sc_list.resources | default([]) }}" + loop_control: + label: "{{ item.metadata.name }}" + ignore_errors: true # noqa: ignore-errors + + - name: Report teardown_cluster_storage summary + ansible.builtin.debug: + msg: >- + Teardown of cluster-side resources for tenant '{{ tenant_name }}' complete. + {{ _lvms_cleanup_sc_list.resources | default([]) | length }} StorageClass(es) removed. + + rescue: + - name: Warn about teardown_cluster_storage failure + ansible.builtin.debug: + msg: >- + Teardown of cluster-side resources failed for tenant '{{ tenant_name }}'. + This may be expected if the target cluster is being destroyed. + Error: {{ ansible_failed_result.msg | default('unknown') }} diff --git a/osac-installer/charts/osac-prereqs/files/hooks/configure-lvms.sh b/osac-installer/charts/osac-prereqs/files/hooks/configure-lvms.sh index 277e42a5e6..8262210ae4 100644 --- a/osac-installer/charts/osac-prereqs/files/hooks/configure-lvms.sh +++ b/osac-installer/charts/osac-prereqs/files/hooks/configure-lvms.sh @@ -15,15 +15,29 @@ done echo "Waiting for lvms-operator deployment..." oc wait --for=condition=Available deploy/lvms-operator -n openshift-storage --timeout=900s -echo "Applying LVMCluster configuration..." -oc apply -f /config/config.yaml +_sc_output=$(oc get sc lvms-vg1 --ignore-not-found -o name 2>&1) \ + || { echo "ERROR: failed to query StorageClasses: ${_sc_output}" >&2; exit 1; } +if [[ -n "${_sc_output}" ]]; then + # lvms-vg1 pre-exists (e.g. MOC, where it was installed by cluster admins). + # Skip both LVMCluster creation AND the default-class annotation: on shared clusters + # another StorageClass (e.g. Ceph) is already the intended default, and annotating + # lvms-vg1 here would silently override that. + echo "lvms-vg1 already exists, skipping LVMCluster creation and annotation." +else + echo "Applying LVMCluster configuration..." + oc apply -f /config/config.yaml -echo "Waiting for lvms-vg1 StorageClass..." -until [[ -n "$(oc get sc --ignore-not-found lvms-vg1 -o name)" ]]; do - sleep 5 -done + echo "Waiting for lvms-vg1 StorageClass..." + for _attempt in $(seq 1 120); do + _sc_query=$(oc get sc --ignore-not-found lvms-vg1 -o name 2>&1) \ + || { echo "ERROR: oc get StorageClass failed: ${_sc_query}" >&2; exit 1; } + [[ -n "${_sc_query}" ]] && break + (( _attempt < 120 )) || { echo "ERROR: timed out waiting for lvms-vg1 StorageClass" >&2; exit 1; } + sleep 5 + done -echo "Setting lvms-vg1 as default StorageClass..." -oc annotate sc lvms-vg1 storageclass.kubernetes.io/is-default-class=true --overwrite + echo "Setting lvms-vg1 as default StorageClass..." + oc annotate sc lvms-vg1 storageclass.kubernetes.io/is-default-class=true --overwrite +fi echo "LVMS configuration complete." diff --git a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml new file mode 100644 index 0000000000..37d795e576 --- /dev/null +++ b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml @@ -0,0 +1,122 @@ +{{- if .Values.lvms.enabled }} +apiVersion: batch/v1 +kind: Job +metadata: + name: register-local-storage + namespace: {{ .Release.Namespace }} + labels: + {{- include "osac.labels" . | nindent 4 }} + annotations: + "helm.sh/hook": post-install,post-upgrade + "helm.sh/hook-weight": "31" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded +spec: + backoffLimit: 5 + # waitForFulfillment polls up to 60 × (30s curl + 10s sleep) = 2400s worst case. + # This deadline must exceed that to avoid premature termination. + activeDeadlineSeconds: 3000 + template: + metadata: + labels: + {{- include "osac.labels" . | nindent 8 }} + spec: + serviceAccountName: admin + initContainers: + {{- include "osac.waitForFulfillment" . | nindent 6 }} + containers: + - name: register-storage + image: {{ .Values.cliImage }} + command: + - /bin/bash + - -euo + - pipefail + - -c + - | + AUTH_TOKEN=$(< /var/run/secrets/kubernetes.io/serviceaccount/token) + API="https://fulfillment-internal-api:8001/api/private/v1" + + # Step 1: Create the local StorageBackend. Capture the ID whether + # this is the first install (2xx) or a re-install (409 = already exists). + BACKEND_RESP=$(mktemp) + BACKEND_CODE=$(curl -skS \ + --connect-timeout 5 --max-time 30 \ + -o "${BACKEND_RESP}" -w '%{http_code}' \ + -X POST "${API}/storage_backends" \ + -H "Authorization: Bearer ${AUTH_TOKEN}" \ + -H "Content-Type: application/json" \ + -d '{"metadata":{"name":"local"},"spec":{"provider":"lvms","endpoint":"n/a","credentials":{"username":"n/a","password":"n/a"}}}') + + case "${BACKEND_CODE}" in + 2??) + BACKEND_ID=$(python3 -c "import sys,json; print(json.load(open('${BACKEND_RESP}'))['id'])") + echo "StorageBackend 'local' created (id: ${BACKEND_ID})." + ;; + 409) + echo "StorageBackend 'local' already exists, fetching ID..." + BACKEND_ID=$(curl -skS --connect-timeout 5 --max-time 30 \ + "${API}/storage_backends" \ + -H "Authorization: Bearer ${AUTH_TOKEN}" | \ + python3 -c "import sys,json; items=json.load(sys.stdin).get('items',[]); m=next((i for i in items if i.get('metadata',{}).get('name')=='local'),None); sys.exit('local backend not found') if not m else print(m['id'])") + echo "StorageBackend 'local' already registered (id: ${BACKEND_ID})." + ;; + *) + echo "ERROR: Failed to create StorageBackend 'local' (HTTP ${BACKEND_CODE}):" >&2 + cat "${BACKEND_RESP}" >&2 + rm -f "${BACKEND_RESP}" + exit 1 + ;; + esac + rm -f "${BACKEND_RESP}" + + # Step 2: Create the local StorageTier referencing the backend by ID. + # Protocol 2 = STORAGE_PROTOCOL_BLOCK (LVMS provides block volumes). + TIER_BODY=$(printf '{"metadata":{"name":"local"},"spec":{"backends":[{"backend_id":"%s","protocol":2}]}}' "${BACKEND_ID}") + TIER_RESP=$(mktemp) + TIER_CODE=$(curl -skS \ + --connect-timeout 5 --max-time 30 \ + -o "${TIER_RESP}" -w '%{http_code}' \ + -X POST "${API}/storage_tiers" \ + -H "Authorization: Bearer ${AUTH_TOKEN}" \ + -H "Content-Type: application/json" \ + -d "${TIER_BODY}") + + case "${TIER_CODE}" in + 2??) echo "StorageTier 'local' created." ;; + 409) echo "StorageTier 'local' already exists, skipping." ;; + *) + echo "ERROR: Failed to create StorageTier 'local' (HTTP ${TIER_CODE}):" >&2 + cat "${TIER_RESP}" >&2 + rm -f "${TIER_RESP}" + exit 1 + ;; + esac + rm -f "${TIER_RESP}" + + echo "Local storage registration complete." + env: + - name: HOME + value: /tmp + volumeMounts: + - name: tmp + mountPath: /tmp + resources: + requests: + cpu: 50m + memory: 128Mi + limits: + cpu: 200m + memory: 256Mi + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + runAsNonRoot: true + runAsUser: 1001 + seccompProfile: + type: RuntimeDefault + capabilities: + drop: ["ALL"] + volumes: + - name: tmp + emptyDir: {} + restartPolicy: OnFailure +{{- end }} diff --git a/osac-installer/charts/osac/values.schema.json b/osac-installer/charts/osac/values.schema.json index 8546be74fa..bca7d6e7b6 100644 --- a/osac-installer/charts/osac/values.schema.json +++ b/osac-installer/charts/osac/values.schema.json @@ -1302,7 +1302,10 @@ "description": "ClusterVersion records to seed", "items": { "type": "object", - "required": ["version", "image"], + "required": [ + "version", + "image" + ], "properties": { "version": { "type": "string", @@ -1321,6 +1324,18 @@ } } } + }, + "lvms": { + "type": "object", + "description": "Register LVMS as a local StorageBackend + StorageTier in the fulfillment service after install. For CI/development environments only.", + "properties": { + "enabled": { + "type": "boolean", + "description": "Run the register-local-storage hook job when LVMS is deployed", + "default": false + } + }, + "additionalProperties": false } } } diff --git a/osac-installer/charts/osac/values.yaml b/osac-installer/charts/osac/values.yaml index f99692962d..fdfede4889 100644 --- a/osac-installer/charts/osac/values.yaml +++ b/osac-installer/charts/osac/values.yaml @@ -287,3 +287,9 @@ clusterVersions: # - version: "4.22.0" # image: "quay.io/openshift-release-dev/ocp-release:4.22.0-multi" # default: true + +lvms: + # Register LVMS as a local StorageBackend + StorageTier in the fulfillment service + # after install. For CI/development environments only — production deployments + # should use their own storage operators (Ceph, Pure, VAST, etc.). + enabled: false diff --git a/osac-operator/internal/controller/storage_controller.go b/osac-operator/internal/controller/storage_controller.go index 397a8b0684..6f8fbe7b25 100644 --- a/osac-operator/internal/controller/storage_controller.go +++ b/osac-operator/internal/controller/storage_controller.go @@ -322,13 +322,6 @@ func (r *StorageReconciler) handleUpdate(ctx context.Context, instance *v1alpha1 clusterName := string(r.targetCluster) if r.ClusterStorageProvider != nil { - // When AAP is configured, prefer tenant-specific StorageClasses - // (labeled osac.openshift.io/tenant=). If none exist, - // fall back to shared Default SCs so VMs can provision immediately, - // and trigger the AAP cluster storage job to create a proper - // tenant-specific SC. Once the tenant-specific SC appears (via the - // StorageClass watch), the next reconcile picks it up and replaces - // the Default. scResult, err := r.resolveTenantSpecificStorageClasses(ctx, targetClient, tenantName) if err != nil { return ctrl.Result{}, err @@ -352,39 +345,15 @@ func (r *StorageReconciler) handleUpdate(ctx context.Context, instance *v1alpha1 return ctrl.Result{}, nil } - // No tenant-specific SCs. Check for shared Default SCs so VMs - // can provision while the AAP job creates the real one. - defaultFallback, err := getTenantStorageClasses(ctx, targetClient, tenantName) - if err != nil { - return ctrl.Result{}, err - } - - for _, msg := range defaultFallback.duplicateMessages { - r.Recorder.Eventf(instance, nil, corev1.EventTypeWarning, eventReasonDuplicateStorageClass, eventActionDetectDuplicate, "%s", msg) - } - - if len(defaultFallback.resolved) > 0 { - condMsg := r.appendMissingTierWarnings(instance, tierDefinitions, defaultFallback.resolved, defaultFallback.ambiguousTiers, - defaultFallback.conditionMessage()+"; tenant-specific provisioning pending") - instance.SetStatusCondition(v1alpha1.TenantConditionClusterStorageReady, - metav1.ConditionTrue, - v1alpha1.TenantReasonFound, - condMsg) - instance.Status.StorageClasses = defaultFallback.resolved - instance.Status.ClusterStorage = []v1alpha1.ClusterStorageStatus{ - {ClusterName: clusterName, Ready: true, Reason: v1alpha1.TenantReasonFound}, - } - } else { - condMsg := r.appendMissingTierWarnings(instance, tierDefinitions, nil, defaultFallback.ambiguousTiers, - fmt.Sprintf("no StorageClass found for tenant %q", tenantName)) - instance.SetStatusCondition(v1alpha1.TenantConditionClusterStorageReady, - metav1.ConditionFalse, - v1alpha1.TenantReasonNotFound, - condMsg) - instance.Status.StorageClasses = nil - instance.Status.ClusterStorage = []v1alpha1.ClusterStorageStatus{ - {ClusterName: clusterName, Ready: false, Reason: v1alpha1.TenantReasonNotFound}, - } + condMsg := r.appendMissingTierWarnings(instance, tierDefinitions, nil, scResult.ambiguousTiers, + fmt.Sprintf("no StorageClass found for tenant %q", tenantName)) + instance.SetStatusCondition(v1alpha1.TenantConditionClusterStorageReady, + metav1.ConditionFalse, + v1alpha1.TenantReasonNotFound, + condMsg) + instance.Status.StorageClasses = nil + instance.Status.ClusterStorage = []v1alpha1.ClusterStorageStatus{ + {ClusterName: clusterName, Ready: false, Reason: v1alpha1.TenantReasonNotFound}, } return r.handleClusterStorageProvisioning(ctx, instance, hubSecretReady) @@ -405,11 +374,9 @@ func (r *StorageReconciler) handleUpdate(ctx context.Context, instance *v1alpha1 } } else { // When no provisioning provider is configured, resolve StorageClasses - // using the full tier resolution logic: tenant-specific SCs take - // priority, with shared default SCs (labeled tenant=Default) as - // fallback. This serves environments running OSAC without AAP/VAST - // where an admin or prepare-tenant.sh has labeled existing - // StorageClasses manually. + // labeled osac.openshift.io/tenant=. This serves environments + // running OSAC without AAP where an admin has pre-provisioned + // tenant-specific StorageClasses manually. result, err := getTenantStorageClasses(ctx, targetClient, tenantName) if err != nil { return ctrl.Result{}, err @@ -979,12 +946,6 @@ func (r *StorageReconciler) mapStorageClassToTenant(ctx context.Context, obj cli return nil } - if tenantName == defaultStorageClassSentinel { - log.Info("shared Default StorageClass changed, reconciling all tenants", - "storageClass", obj.GetName()) - return r.allTenantReconcileRequests(ctx) - } - tenant := &v1alpha1.Tenant{} if err := r.Get(ctx, client.ObjectKey{Namespace: r.tenantNamespace, Name: tenantName}, tenant); err != nil { if client.IgnoreNotFound(err) != nil { @@ -1011,24 +972,6 @@ func (r *StorageReconciler) mapSecretToTenant(ctx context.Context, obj client.Ob return []reconcile.Request{{NamespacedName: client.ObjectKeyFromObject(tenant)}} } -func (r *StorageReconciler) allTenantReconcileRequests(ctx context.Context) []reconcile.Request { - log := ctrllog.FromContext(ctx) - - tenantList := &v1alpha1.TenantList{} - if err := r.List(ctx, tenantList, client.InNamespace(r.tenantNamespace)); err != nil { - log.Error(err, "unable to list Tenants for Default SC reconciliation") - return nil - } - - requests := make([]reconcile.Request, 0, len(tenantList.Items)) - for i := range tenantList.Items { - requests = append(requests, reconcile.Request{ - NamespacedName: client.ObjectKeyFromObject(&tenantList.Items[i]), - }) - } - return requests -} - // tenantSpecificStorageClasses is the result of resolveTenantSpecificStorageClasses: // resolved StorageClasses per tier, plus the duplicate-SC messages and ambiguous // tier names for tiers excluded from resolved because multiple StorageClasses diff --git a/osac-operator/internal/controller/storage_tier_resolution.go b/osac-operator/internal/controller/storage_tier_resolution.go index df4fc57c6f..e12e5717a7 100644 --- a/osac-operator/internal/controller/storage_tier_resolution.go +++ b/osac-operator/internal/controller/storage_tier_resolution.go @@ -82,21 +82,12 @@ func getTenantStorageClasses(ctx context.Context, targetClient client.Client, te return tierResolutionResult{}, err } - defaultSCList := &storagev1.StorageClassList{} - if err := targetClient.List(ctx, defaultSCList, client.MatchingLabels{osacTenantKey: defaultStorageClassSentinel}); err != nil { - return tierResolutionResult{}, err - } - tenantByTier := groupByTier(tenantSCList.Items) - defaultByTier := groupByTier(defaultSCList.Items) allTiers := make(map[string]struct{}) for t := range tenantByTier { allTiers[t] = struct{}{} } - for t := range defaultByTier { - allTiers[t] = struct{}{} - } sortedTiers := make([]string, 0, len(allTiers)) for t := range allTiers { @@ -108,7 +99,6 @@ func getTenantStorageClasses(ctx context.Context, targetClient client.Client, te for _, tier := range sortedTiers { tenantSCs := tenantByTier[tier] - defaultSCs := defaultByTier[tier] switch len(tenantSCs) { case 1: @@ -119,9 +109,8 @@ func getTenantStorageClasses(ctx context.Context, targetClient client.Client, te }) msg := fmt.Sprintf("tier %q: StorageClass %q (tenant-specific)", tier, scName) result.resolvedMessages = append(result.resolvedMessages, msg) - continue case 0: - // Fall through to Default resolution below. + // Tier not available — no StorageClass labeled for this tenant. default: joined, names := joinStorageClassNames(tenantSCs) msg := fmt.Sprintf("tier %q: multiple tenant StorageClasses [%s]", tier, joined) @@ -129,27 +118,6 @@ func getTenantStorageClasses(ctx context.Context, targetClient client.Client, te result.errorMessages = append(result.errorMessages, msg) result.duplicateMessages = append(result.duplicateMessages, msg) result.ambiguousTiers = append(result.ambiguousTiers, tier) - continue - } - - switch len(defaultSCs) { - case 1: - scName := defaultSCs[0].GetName() - result.resolved = append(result.resolved, v1alpha1.ResolvedStorageClass{ - Name: scName, - Tier: tier, - }) - msg := fmt.Sprintf("tier %q: StorageClass %q (shared Default)", tier, scName) - result.resolvedMessages = append(result.resolvedMessages, msg) - case 0: - // Tier not available. - default: - joined, names := joinStorageClassNames(defaultSCs) - msg := fmt.Sprintf("tier %q: multiple shared Default StorageClasses [%s]", tier, joined) - log.Info(msg, "tenant", tenantName, "tier", tier, "storageClasses", names) - result.errorMessages = append(result.errorMessages, msg) - result.duplicateMessages = append(result.duplicateMessages, msg) - result.ambiguousTiers = append(result.ambiguousTiers, tier) } } diff --git a/osac-operator/internal/controller/tenant_names.go b/osac-operator/internal/controller/tenant_names.go index e9426cf8bc..99bef40a08 100644 --- a/osac-operator/internal/controller/tenant_names.go +++ b/osac-operator/internal/controller/tenant_names.go @@ -25,11 +25,6 @@ import ( ) const ( - // defaultStorageClassSentinel is the label value that marks a shared StorageClass - // available to all tenants. No Tenant CR can be named "Default" because uppercase - // is forbidden in Kubernetes resource names. - defaultStorageClassSentinel = "Default" - // tenantControllerName is the name used when creating the event recorder tenantControllerName = "tenant-controller" From fc9a8ea75b8bd075628e94359b817f39143a5dc3 Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Tue, 4 Aug 2026 09:37:49 +0200 Subject: [PATCH 02/14] OSAC-3011: fix register-local-storage hook SCC violation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove explicit runAsUser: 1001 — the namespace SCC range on OpenShift restricts UIDs to the project-allocated range (e.g. 1000850000+), so a fixed non-root UID is rejected. Let OpenShift assign a UID automatically, matching the pattern used by all other osac-installer hooks. Assisted-by: Claude Code Signed-off-by: Zoltan Szabo --- .../charts/osac/templates/hooks/register-local-storage.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml index 37d795e576..a84f72306c 100644 --- a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml +++ b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml @@ -109,8 +109,6 @@ spec: securityContext: allowPrivilegeEscalation: false readOnlyRootFilesystem: true - runAsNonRoot: true - runAsUser: 1001 seccompProfile: type: RuntimeDefault capabilities: From 352d575483ba35e6677cbafb3954b546c033afb9 Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Tue, 4 Aug 2026 11:21:47 +0200 Subject: [PATCH 03/14] OSAC-3011: address CodeRabbit review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ensure_storage_class.yaml: tighten tier name regex to lowercase-only. The previous pattern allowed uppercase (A-Z) which would produce StorageClass names like osac-- — invalid as DNS labels. register-local-storage.yaml: validate existing resource spec on HTTP 409. A conflict only proves the name exists. After fetching the existing StorageBackend/StorageTier, verify provider=lvms and the tier's backend reference matches, rather than silently trusting whatever is registered. Assisted-by: Claude Code Signed-off-by: Zoltan Szabo --- .../tasks/ensure_storage_class.yaml | 12 +++--- .../hooks/register-local-storage.yaml | 37 ++++++++++++++++--- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml index 48e3419573..efd401122b 100644 --- a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml @@ -16,13 +16,15 @@ msg: "tenant_name '{{ tenant_name }}' is not a valid DNS label." when: tenant_name is not regex('^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$') -- name: Validate tier names are valid Kubernetes label values +- name: Validate tier names are lowercase-safe for StorageClass names ansible.builtin.fail: msg: >- - tier.name '{{ item.name }}' is not a valid Kubernetes label value. - Must be 1-63 chars, lowercase alphanumeric, hyphens, underscores, or dots, - starting and ending with an alphanumeric character. - when: item.name is not regex('^[a-z0-9A-Z]([a-z0-9A-Z._-]{0,61}[a-z0-9A-Z])?$') + tier.name '{{ item.name }}' is not valid: must be 1-63 chars, + lowercase alphanumeric, hyphens, underscores, or dots, + starting and ending with a lowercase alphanumeric character. + Uppercase is rejected because StorageClass names (osac--) + must be valid DNS labels. + when: item.name is not regex('^[a-z0-9]([a-z0-9._-]{0,61}[a-z0-9])?$') loop: "{{ _provider_tiers }}" loop_control: label: "{{ item.name }}" diff --git a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml index a84f72306c..2872355741 100644 --- a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml +++ b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml @@ -52,12 +52,21 @@ spec: echo "StorageBackend 'local' created (id: ${BACKEND_ID})." ;; 409) - echo "StorageBackend 'local' already exists, fetching ID..." - BACKEND_ID=$(curl -skS --connect-timeout 5 --max-time 30 \ + echo "StorageBackend 'local' already exists, verifying spec..." + EXISTING=$(curl -skS --connect-timeout 5 --max-time 30 \ "${API}/storage_backends" \ - -H "Authorization: Bearer ${AUTH_TOKEN}" | \ - python3 -c "import sys,json; items=json.load(sys.stdin).get('items',[]); m=next((i for i in items if i.get('metadata',{}).get('name')=='local'),None); sys.exit('local backend not found') if not m else print(m['id'])") - echo "StorageBackend 'local' already registered (id: ${BACKEND_ID})." + -H "Authorization: Bearer ${AUTH_TOKEN}") + BACKEND_ID=$(echo "${EXISTING}" | python3 -c " +import sys,json +items=json.load(sys.stdin).get('items',[]) +m=next((i for i in items if i.get('metadata',{}).get('name')=='local'),None) +if not m: + print('ERROR: StorageBackend local not found after 409', file=sys.stderr); sys.exit(1) +if m.get('spec',{}).get('provider') != 'lvms': + print(f'ERROR: existing local backend has provider {m[\"spec\"].get(\"provider\")!r}, expected lvms', file=sys.stderr); sys.exit(1) +print(m['id']) +") + echo "StorageBackend 'local' already registered with provider=lvms (id: ${BACKEND_ID})." ;; *) echo "ERROR: Failed to create StorageBackend 'local' (HTTP ${BACKEND_CODE}):" >&2 @@ -82,7 +91,23 @@ spec: case "${TIER_CODE}" in 2??) echo "StorageTier 'local' created." ;; - 409) echo "StorageTier 'local' already exists, skipping." ;; + 409) + echo "StorageTier 'local' already exists, verifying backend reference..." + EXISTING_TIER=$(curl -skS --connect-timeout 5 --max-time 30 \ + "${API}/storage_tiers" -H "Authorization: Bearer ${AUTH_TOKEN}") + echo "${EXISTING_TIER}" | python3 -c " +import sys,json +items=json.load(sys.stdin).get('items',[]) +m=next((t for t in items if t.get('metadata',{}).get('name')=='local'),None) +if not m: + print('ERROR: StorageTier local not found after 409', file=sys.stderr); sys.exit(1) +backends=[b.get('backend_id') for b in m.get('spec',{}).get('backends',[])] +if '${BACKEND_ID}' not in backends: + print(f'WARNING: existing local tier references backends {backends}, not ${BACKEND_ID}') +else: + print('StorageTier local correctly references backend ${BACKEND_ID}.') +" || true + ;; *) echo "ERROR: Failed to create StorageTier 'local' (HTTP ${TIER_CODE}):" >&2 cat "${TIER_RESP}" >&2 From 77fba7306f183392b6474eb96c48fa105da14472 Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Tue, 4 Aug 2026 11:30:47 +0200 Subject: [PATCH 04/14] OSAC-3011: address CodeRabbit follow-up findings ensure_storage_class.yaml: remove underscore from tier name regex. Underscores produce invalid StorageClass names (osac--gold_tier fails Kubernetes DNS label validation). Valid chars: [a-z0-9.-]. register-local-storage.yaml: strengthen StorageTier 409 validation. Previous fix used `|| true` which swallowed failures. Now exits 1 when the existing local tier does not reference the expected backend_id with protocol=2, matching the strictness of the StorageBackend check. Assisted-by: Claude Code Signed-off-by: Zoltan Szabo --- .../lvms_storage/tasks/ensure_storage_class.yaml | 9 +++++---- .../templates/hooks/register-local-storage.yaml | 14 +++++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml index efd401122b..2e9b8057f8 100644 --- a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml @@ -20,11 +20,12 @@ ansible.builtin.fail: msg: >- tier.name '{{ item.name }}' is not valid: must be 1-63 chars, - lowercase alphanumeric, hyphens, underscores, or dots, + lowercase alphanumeric, hyphens, or dots only, starting and ending with a lowercase alphanumeric character. - Uppercase is rejected because StorageClass names (osac--) - must be valid DNS labels. - when: item.name is not regex('^[a-z0-9]([a-z0-9._-]{0,61}[a-z0-9])?$') + Underscores and uppercase are rejected because tier.name is embedded + in the StorageClass name (osac--), which must be a + valid Kubernetes DNS label. + when: item.name is not regex('^[a-z0-9]([a-z0-9.-]{0,61}[a-z0-9])?$') loop: "{{ _provider_tiers }}" loop_control: label: "{{ item.name }}" diff --git a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml index 2872355741..1930b139d0 100644 --- a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml +++ b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml @@ -92,7 +92,7 @@ print(m['id']) case "${TIER_CODE}" in 2??) echo "StorageTier 'local' created." ;; 409) - echo "StorageTier 'local' already exists, verifying backend reference..." + echo "StorageTier 'local' already exists, verifying spec..." EXISTING_TIER=$(curl -skS --connect-timeout 5 --max-time 30 \ "${API}/storage_tiers" -H "Authorization: Bearer ${AUTH_TOKEN}") echo "${EXISTING_TIER}" | python3 -c " @@ -101,12 +101,12 @@ items=json.load(sys.stdin).get('items',[]) m=next((t for t in items if t.get('metadata',{}).get('name')=='local'),None) if not m: print('ERROR: StorageTier local not found after 409', file=sys.stderr); sys.exit(1) -backends=[b.get('backend_id') for b in m.get('spec',{}).get('backends',[])] -if '${BACKEND_ID}' not in backends: - print(f'WARNING: existing local tier references backends {backends}, not ${BACKEND_ID}') -else: - print('StorageTier local correctly references backend ${BACKEND_ID}.') -" || true +backends=m.get('spec',{}).get('backends',[]) +match=next((b for b in backends if b.get('backend_id')=='${BACKEND_ID}' and b.get('protocol')==2),None) +if not match: + print(f'ERROR: existing local tier does not reference backend ${BACKEND_ID} with protocol=2; got: {backends}', file=sys.stderr); sys.exit(1) +print('StorageTier local correctly references backend ${BACKEND_ID} with protocol=2.') +" ;; *) echo "ERROR: Failed to create StorageTier 'local' (HTTP ${TIER_CODE}):" >&2 From 8614eb4fa3951ee4b2c19d1c1cae130bde92ca1b Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Tue, 4 Aug 2026 12:39:31 +0200 Subject: [PATCH 05/14] OSAC-3011: drop tier name validation from lvms_storage role The fulfillment service enforces valid tier names at creation time. A second validation layer in the AAP role adds noise without benefit. Assisted-by: Claude Code Signed-off-by: Zoltan Szabo --- .../lvms_storage/tasks/ensure_storage_class.yaml | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml index 2e9b8057f8..b3ad4e3215 100644 --- a/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/tasks/ensure_storage_class.yaml @@ -16,20 +16,6 @@ msg: "tenant_name '{{ tenant_name }}' is not a valid DNS label." when: tenant_name is not regex('^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$') -- name: Validate tier names are lowercase-safe for StorageClass names - ansible.builtin.fail: - msg: >- - tier.name '{{ item.name }}' is not valid: must be 1-63 chars, - lowercase alphanumeric, hyphens, or dots only, - starting and ending with a lowercase alphanumeric character. - Underscores and uppercase are rejected because tier.name is embedded - in the StorageClass name (osac--), which must be a - valid Kubernetes DNS label. - when: item.name is not regex('^[a-z0-9]([a-z0-9.-]{0,61}[a-z0-9])?$') - loop: "{{ _provider_tiers }}" - loop_control: - label: "{{ item.name }}" - - name: Build expected StorageClass entries ansible.builtin.set_fact: _lvms_expected_sc_entries: >- From 84ea7e62f50ec9514cc1e728df70ebbffaf7cd31 Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Tue, 4 Aug 2026 13:12:38 +0200 Subject: [PATCH 06/14] =?UTF-8?q?OSAC-3011:=20fix=20CI=20failures=20?= =?UTF-8?q?=E2=80=94=20test=20symbol=20and=20hook=20YAML=20syntax?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit storage_controller_test.go: remove all references to deleted defaultStorageClassSentinel constant. Update test assertions to match the new behavior (no Default SC fallback): tests that expected Default SCs to be resolved now expect empty StorageClasses or use tenant-labeled SCs instead. Test names updated to describe the new behavior. register-local-storage.yaml: replace Python f-strings with stderr.write() calls. The f-string pattern `got: {backends}` inside a Helm YAML block scalar caused a YAML parse error ("could not find expected ':'") because the YAML parser misinterpreted {backends} as a flow mapping. Assisted-by: Claude Code Signed-off-by: Zoltan Szabo --- .../hooks/register-local-storage.yaml | 11 ++-- .../controller/storage_controller_test.go | 52 +++++++------------ 2 files changed, 24 insertions(+), 39 deletions(-) diff --git a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml index 1930b139d0..d4375b053e 100644 --- a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml +++ b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml @@ -61,9 +61,10 @@ import sys,json items=json.load(sys.stdin).get('items',[]) m=next((i for i in items if i.get('metadata',{}).get('name')=='local'),None) if not m: - print('ERROR: StorageBackend local not found after 409', file=sys.stderr); sys.exit(1) -if m.get('spec',{}).get('provider') != 'lvms': - print(f'ERROR: existing local backend has provider {m[\"spec\"].get(\"provider\")!r}, expected lvms', file=sys.stderr); sys.exit(1) + sys.stderr.write('ERROR: StorageBackend local not found after 409\n'); sys.exit(1) +provider=m.get('spec',{}).get('provider','') +if provider != 'lvms': + sys.stderr.write('ERROR: existing local backend has provider ' + provider + ', expected lvms\n'); sys.exit(1) print(m['id']) ") echo "StorageBackend 'local' already registered with provider=lvms (id: ${BACKEND_ID})." @@ -100,11 +101,11 @@ import sys,json items=json.load(sys.stdin).get('items',[]) m=next((t for t in items if t.get('metadata',{}).get('name')=='local'),None) if not m: - print('ERROR: StorageTier local not found after 409', file=sys.stderr); sys.exit(1) + sys.stderr.write('ERROR: StorageTier local not found after 409\n'); sys.exit(1) backends=m.get('spec',{}).get('backends',[]) match=next((b for b in backends if b.get('backend_id')=='${BACKEND_ID}' and b.get('protocol')==2),None) if not match: - print(f'ERROR: existing local tier does not reference backend ${BACKEND_ID} with protocol=2; got: {backends}', file=sys.stderr); sys.exit(1) + sys.stderr.write('ERROR: existing local tier does not reference backend ${BACKEND_ID} with protocol=2\n'); sys.exit(1) print('StorageTier local correctly references backend ${BACKEND_ID} with protocol=2.') " ;; diff --git a/osac-operator/internal/controller/storage_controller_test.go b/osac-operator/internal/controller/storage_controller_test.go index 6969d03acb..42f5d2067d 100644 --- a/osac-operator/internal/controller/storage_controller_test.go +++ b/osac-operator/internal/controller/storage_controller_test.go @@ -304,10 +304,9 @@ var _ = Describe("Storage Controller", func() { Expect(cond.Reason).To(Equal(v1alpha1.TenantReasonFound)) }) - It("should set StorageBackendReady=False with NoProvider and continue to Stage 2 when no provider configured", func() { + It("should set StorageBackendReady=False with NoProvider and ClusterStorageReady=False when no tenant SCs exist", func() { name := "storage-test-no-provider" createReadyTenantForStorage(ctx, name, testNamespace) - createLabeledStorageClass(ctx, "default-sc-"+name, defaultStorageClassSentinel, "default") r := NewStorageReconciler( testMcManager, testNamespace, mcmanager.LocalCluster, @@ -329,10 +328,9 @@ var _ = Describe("Storage Controller", func() { clusterCond := tenant.GetStatusCondition(v1alpha1.TenantConditionClusterStorageReady) Expect(clusterCond).NotTo(BeNil()) - Expect(clusterCond.Status).To(Equal(metav1.ConditionTrue)) + Expect(clusterCond.Status).To(Equal(metav1.ConditionFalse)) - Expect(tenant.Status.StorageClasses).To(HaveLen(1)) - Expect(tenant.Status.StorageClasses[0].Name).To(Equal("default-sc-" + name)) + Expect(tenant.Status.StorageClasses).To(BeNil()) }) It("should set ClusterStorageReady=False when no provider and no labeled SCs", func() { @@ -367,7 +365,7 @@ var _ = Describe("Storage Controller", func() { It("should preserve tenant controller fields when patching storage status", func() { name := "storage-test-patch-preserves-phase" createReadyTenantForStorage(ctx, name, testNamespace) - createLabeledStorageClass(ctx, "default-sc-"+name, defaultStorageClassSentinel, "default") + createLabeledStorageClass(ctx, name+"-tenant-sc", name, "default") r := NewStorageReconciler( testMcManager, testNamespace, mcmanager.LocalCluster, @@ -386,7 +384,7 @@ var _ = Describe("Storage Controller", func() { Expect(tenant.Status.Namespace).To(Equal(name)) Expect(tenant.Status.StorageClasses).To(HaveLen(1)) - Expect(tenant.Status.StorageClasses[0].Name).To(Equal("default-sc-" + name)) + Expect(tenant.Status.StorageClasses[0].Name).To(Equal(name + "-tenant-sc")) }) It("should propagate trigger error without creating fake job", func() { @@ -473,11 +471,10 @@ var _ = Describe("Storage Controller", func() { Expect(tenant.Status.StorageClasses[0].Tier).To(Equal("default")) }) - It("should use Default SC fallback and trigger provisioning when only default SC exists and provider is configured", func() { + It("should set ClusterStorageReady=False and trigger provisioning when no tenant SCs exist and provider is configured", func() { name := "storage-test-default-only-with-provider" createReadyTenantForStorage(ctx, name, testNamespace) createHubSecret(ctx, name, secretsNamespace) - createLabeledStorageClass(ctx, "shared-default-sc-"+name, defaultStorageClassSentinel, "default") clusterProvider := &mockProvisioningProvider{name: "cluster-storage-mock"} r := NewStorageReconciler( @@ -495,22 +492,17 @@ var _ = Describe("Storage Controller", func() { clusterCond := tenant.GetStatusCondition(v1alpha1.TenantConditionClusterStorageReady) Expect(clusterCond).NotTo(BeNil()) - Expect(clusterCond.Status).To(Equal(metav1.ConditionTrue)) - Expect(clusterCond.Reason).To(Equal(v1alpha1.TenantReasonFound)) - Expect(clusterCond.Message).To(ContainSubstring("tenant-specific provisioning pending")) + Expect(clusterCond.Status).To(Equal(metav1.ConditionFalse)) + Expect(clusterCond.Reason).To(Equal(v1alpha1.TenantReasonNotFound)) - Expect(tenant.Status.StorageClasses).To(HaveLen(1)) - Expect(tenant.Status.StorageClasses[0].Name).To(Equal("shared-default-sc-" + name)) + Expect(tenant.Status.StorageClasses).To(BeNil()) Expect(tenant.Status.ClusterStorageJobs).To(HaveLen(1)) }) - It("should detect duplicate Default SCs in AAP fallback and set ClusterStorageReady=False", func() { - name := "storage-test-dup-default-aap" + It("should set ClusterStorageReady=False and trigger provisioning when provider configured but no tenant SCs", func() { + name := "storage-test-no-tenant-sc-with-provider" createReadyTenantForStorage(ctx, name, testNamespace) createHubSecret(ctx, name, secretsNamespace) - // Two Default SCs for the same tier: triggers duplicate detection - createLabeledStorageClass(ctx, "shared-default-dup1-"+name, defaultStorageClassSentinel, "default") - createLabeledStorageClass(ctx, "shared-default-dup2-"+name, defaultStorageClassSentinel, "default") clusterProvider := &mockProvisioningProvider{name: "cluster-storage-mock"} r := NewStorageReconciler( @@ -526,17 +518,12 @@ var _ = Describe("Storage Controller", func() { tenant := &v1alpha1.Tenant{} Expect(k8sClient.Get(ctx, nn, tenant)).To(Succeed()) - // Default fallback found duplicates, so no SCs are resolved. - // The condition should reflect NotFound (no usable SCs) and - // provisioning should still be triggered to create the real one. clusterCond := tenant.GetStatusCondition(v1alpha1.TenantConditionClusterStorageReady) Expect(clusterCond).NotTo(BeNil()) Expect(clusterCond.Status).To(Equal(metav1.ConditionFalse)) Expect(clusterCond.Reason).To(Equal(v1alpha1.TenantReasonNotFound)) Expect(tenant.Status.StorageClasses).To(BeNil()) - // Cluster storage provisioning should still run to create a - // tenant-specific SC that supersedes the ambiguous defaults Expect(tenant.Status.ClusterStorageJobs).To(HaveLen(1)) }) @@ -633,11 +620,10 @@ var _ = Describe("Storage Controller", func() { }) Context("Tier resolution", func() { - It("should fall back to Default StorageClass when no tenant-specific SC", func() { - name := "storage-test-default-fallback" + It("should resolve no StorageClasses when only tenant-labeled SCs are absent (no Default fallback)", func() { + name := "storage-test-no-tenant-sc" createReadyTenantForStorage(ctx, name, testNamespace) createHubSecret(ctx, name, secretsNamespace) - createLabeledStorageClass(ctx, "shared-default-sc-"+name, defaultStorageClassSentinel, "default") r := NewStorageReconciler( testMcManager, testNamespace, mcmanager.LocalCluster, @@ -652,15 +638,13 @@ var _ = Describe("Storage Controller", func() { tenant := &v1alpha1.Tenant{} Expect(k8sClient.Get(ctx, nn, tenant)).To(Succeed()) - Expect(tenant.Status.StorageClasses).To(HaveLen(1)) - Expect(tenant.Status.StorageClasses[0].Name).To(Equal("shared-default-sc-" + name)) + Expect(tenant.Status.StorageClasses).To(BeNil()) }) - It("should prefer tenant-specific SC over Default", func() { + It("should resolve only tenant-specific SCs (unlabeled Default SCs ignored)", func() { name := "storage-test-tenant-priority" createReadyTenantForStorage(ctx, name, testNamespace) createHubSecret(ctx, name, secretsNamespace) - createLabeledStorageClass(ctx, "shared-sc-"+name, defaultStorageClassSentinel, "default") createLabeledStorageClass(ctx, name+"-tenant-sc", name, "default") r := NewStorageReconciler( @@ -1768,7 +1752,7 @@ var _ = Describe("Storage Controller", func() { It("should fall through to SC resolution when BackendsClient is nil", func() { name := "storage-test-nil-client" createReadyTenantForStorage(ctx, name, testNamespace) - createLabeledStorageClass(ctx, "default-sc-"+name, defaultStorageClassSentinel, "default") + createLabeledStorageClass(ctx, name+"-sc", name, "default") // BackendProvider set but BackendsClient nil: should behave as no backend registered r := NewStorageReconciler( @@ -1789,7 +1773,7 @@ var _ = Describe("Storage Controller", func() { Expect(backendCond.Status).To(Equal(metav1.ConditionFalse)) Expect(backendCond.Reason).To(Equal(v1alpha1.TenantReasonNoProvider)) Expect(backendCond.Message).To(ContainSubstring("No fulfillment service connection configured")) - // Falls through to Stage 2: default SC resolved, no AAP job triggered + // Falls through to Stage 2: tenant-specific SC resolved, no AAP job triggered Expect(tenant.Status.StorageClasses).NotTo(BeEmpty()) Expect(tenant.Status.StorageBackendJobs).To(BeEmpty()) }) @@ -1797,7 +1781,7 @@ var _ = Describe("Storage Controller", func() { It("should fall through to SC resolution when BackendsClient reports no backends (total=0)", func() { name := "storage-test-zero-backends" createReadyTenantForStorage(ctx, name, testNamespace) - createLabeledStorageClass(ctx, "default-sc-"+name, defaultStorageClassSentinel, "default") + createLabeledStorageClass(ctx, name+"-sc", name, "default") r := NewStorageReconciler( testMcManager, testNamespace, mcmanager.LocalCluster, From 15063880e7a4fe4748d49716a6744ae72781d9db Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Tue, 4 Aug 2026 13:19:38 +0200 Subject: [PATCH 07/14] OSAC-3011: fix YAML parse error in register-local-storage hook Multi-line Python code at zero indentation exits the YAML block scalar context (Helm's YAML parser tries to parse it as regular YAML). Replace multi-line embedded Python with single-line one-liners that stay at the correct indentation within the block scalar. Assisted-by: Claude Code Signed-off-by: Zoltan Szabo --- .../hooks/register-local-storage.yaml | 27 +++---------------- 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml index d4375b053e..e60a60447e 100644 --- a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml +++ b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml @@ -56,17 +56,7 @@ spec: EXISTING=$(curl -skS --connect-timeout 5 --max-time 30 \ "${API}/storage_backends" \ -H "Authorization: Bearer ${AUTH_TOKEN}") - BACKEND_ID=$(echo "${EXISTING}" | python3 -c " -import sys,json -items=json.load(sys.stdin).get('items',[]) -m=next((i for i in items if i.get('metadata',{}).get('name')=='local'),None) -if not m: - sys.stderr.write('ERROR: StorageBackend local not found after 409\n'); sys.exit(1) -provider=m.get('spec',{}).get('provider','') -if provider != 'lvms': - sys.stderr.write('ERROR: existing local backend has provider ' + provider + ', expected lvms\n'); sys.exit(1) -print(m['id']) -") + BACKEND_ID=$(echo "${EXISTING}" | python3 -c "import sys,json; items=json.load(sys.stdin).get('items',[]); m=next((i for i in items if i.get('metadata',{}).get('name')=='local'),None); p=m.get('spec',{}).get('provider','') if m else ''; sys.stderr.write('ERROR: backend not found or wrong provider: '+p+'\n') or sys.exit(1) if not m or p!='lvms' else print(m['id'])") echo "StorageBackend 'local' already registered with provider=lvms (id: ${BACKEND_ID})." ;; *) @@ -93,21 +83,10 @@ print(m['id']) case "${TIER_CODE}" in 2??) echo "StorageTier 'local' created." ;; 409) - echo "StorageTier 'local' already exists, verifying spec..." + echo "StorageTier 'local' already exists, verifying backend reference..." EXISTING_TIER=$(curl -skS --connect-timeout 5 --max-time 30 \ "${API}/storage_tiers" -H "Authorization: Bearer ${AUTH_TOKEN}") - echo "${EXISTING_TIER}" | python3 -c " -import sys,json -items=json.load(sys.stdin).get('items',[]) -m=next((t for t in items if t.get('metadata',{}).get('name')=='local'),None) -if not m: - sys.stderr.write('ERROR: StorageTier local not found after 409\n'); sys.exit(1) -backends=m.get('spec',{}).get('backends',[]) -match=next((b for b in backends if b.get('backend_id')=='${BACKEND_ID}' and b.get('protocol')==2),None) -if not match: - sys.stderr.write('ERROR: existing local tier does not reference backend ${BACKEND_ID} with protocol=2\n'); sys.exit(1) -print('StorageTier local correctly references backend ${BACKEND_ID} with protocol=2.') -" + echo "${EXISTING_TIER}" | python3 -c "import sys,json; items=json.load(sys.stdin).get('items',[]); m=next((t for t in items if t.get('metadata',{}).get('name')=='local'),None); sys.exit(1) if not m else None; ok=next((b for b in m.get('spec',{}).get('backends',[]) if b.get('backend_id')=='${BACKEND_ID}' and b.get('protocol')==2),None); sys.stderr.write('ERROR: tier does not reference backend ${BACKEND_ID}\n') or sys.exit(1) if not ok else print('ok')" ;; *) echo "ERROR: Failed to create StorageTier 'local' (HTTP ${TIER_CODE}):" >&2 From 9c2da5d359a4986cf7d72700a9897ad57b0fd3b1 Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Tue, 4 Aug 2026 14:03:38 +0200 Subject: [PATCH 08/14] OSAC-3011: fix lvms schema rejecting extra properties from other phases The bmaas-ci values file sets lvms.channel (consumed by osac-prereqs Phase 2). The additionalProperties: false on the osac Phase 3 chart schema incorrectly rejected it. Phase 3 only cares about lvms.enabled; other properties belong to Phase 2 and must not be blocked here. Assisted-by: Claude Code Signed-off-by: Zoltan Szabo --- osac-installer/charts/osac/values.schema.json | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/osac-installer/charts/osac/values.schema.json b/osac-installer/charts/osac/values.schema.json index bca7d6e7b6..8ebe111a03 100644 --- a/osac-installer/charts/osac/values.schema.json +++ b/osac-installer/charts/osac/values.schema.json @@ -1007,9 +1007,20 @@ "image": { "type": "object", "properties": { - "repository": { "type": "string" }, - "tag": { "type": "string" }, - "pullPolicy": { "type": "string", "enum": ["Always", "IfNotPresent", "Never"] } + "repository": { + "type": "string" + }, + "tag": { + "type": "string" + }, + "pullPolicy": { + "type": "string", + "enum": [ + "Always", + "IfNotPresent", + "Never" + ] + } } }, "database": { @@ -1334,8 +1345,7 @@ "description": "Run the register-local-storage hook job when LVMS is deployed", "default": false } - }, - "additionalProperties": false + } } } } From 09cae8f918823a0612b31479c91cc42b8aade1e4 Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Wed, 5 Aug 2026 07:17:00 +0200 Subject: [PATCH 09/14] OSAC-3011: add Default SC fixture to tier-resolution regression tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two tests in the "Tier resolution" context claimed to prove the old tenant=Default fallback is gone, but neither created a Default-labelled StorageClass — so the regression could re-appear undetected. Add the fixture to each test so the assertions actually cover the removed path. Assisted-by: Claude Code Signed-off-by: Zoltan Szabo --- osac-operator/internal/controller/storage_controller_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osac-operator/internal/controller/storage_controller_test.go b/osac-operator/internal/controller/storage_controller_test.go index 42f5d2067d..f9d4998f12 100644 --- a/osac-operator/internal/controller/storage_controller_test.go +++ b/osac-operator/internal/controller/storage_controller_test.go @@ -624,6 +624,7 @@ var _ = Describe("Storage Controller", func() { name := "storage-test-no-tenant-sc" createReadyTenantForStorage(ctx, name, testNamespace) createHubSecret(ctx, name, secretsNamespace) + createLabeledStorageClass(ctx, name+"-default-sc", "Default", "default") r := NewStorageReconciler( testMcManager, testNamespace, mcmanager.LocalCluster, @@ -645,6 +646,7 @@ var _ = Describe("Storage Controller", func() { name := "storage-test-tenant-priority" createReadyTenantForStorage(ctx, name, testNamespace) createHubSecret(ctx, name, secretsNamespace) + createLabeledStorageClass(ctx, name+"-default-sc", "Default", "default") createLabeledStorageClass(ctx, name+"-tenant-sc", name, "default") r := NewStorageReconciler( From bce9de019c79ebe50c3c199a106020da63f6fdb2 Mon Sep 17 00:00:00 2001 From: Akshay Nadkarni <25892229+akshaynadkarni@users.noreply.github.com> Date: Wed, 5 Aug 2026 13:26:23 -0400 Subject: [PATCH 10/14] Apply suggestions from code review Remove reference to `Default` StorageClass. Co-authored-by: Akshay Nadkarni <25892229+akshaynadkarni@users.noreply.github.com> --- osac-operator/internal/controller/storage_controller.go | 3 ++- osac-operator/internal/controller/storage_tier_resolution.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/osac-operator/internal/controller/storage_controller.go b/osac-operator/internal/controller/storage_controller.go index 6f8fbe7b25..ef2c4ff91d 100644 --- a/osac-operator/internal/controller/storage_controller.go +++ b/osac-operator/internal/controller/storage_controller.go @@ -984,7 +984,8 @@ type tenantSpecificStorageClasses struct { // resolveTenantSpecificStorageClasses lists only StorageClasses labeled with the // given tenant name, ignoring shared defaults (labeled tenant=Default). Used when -// AAP is configured and the controller should not fall back to shared defaults. +// resolveTenantSpecificStorageClasses lists only StorageClasses labeled with the +// given tenant name. Used when AAP is configured. func (r *StorageReconciler) resolveTenantSpecificStorageClasses( ctx context.Context, targetClient client.Client, tenantName string, ) (tenantSpecificStorageClasses, error) { diff --git a/osac-operator/internal/controller/storage_tier_resolution.go b/osac-operator/internal/controller/storage_tier_resolution.go index e12e5717a7..9655df88ea 100644 --- a/osac-operator/internal/controller/storage_tier_resolution.go +++ b/osac-operator/internal/controller/storage_tier_resolution.go @@ -37,7 +37,9 @@ type tierResolutionResult struct { duplicateMessages []string // ambiguousTiers holds the name of every tier excluded from resolved because // multiple StorageClasses matched it (tenant-specific or Default) — a distinct, - // separately-reported problem from a tier having no StorageClass at all. +// ambiguousTiers holds the name of every tier excluded from resolved because +// multiple StorageClasses matched it — a distinct, separately-reported problem +// from a tier having no StorageClass at all. ambiguousTiers []string } From e591fe6510f682979f809704625495031e857b6a Mon Sep 17 00:00:00 2001 From: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Date: Wed, 5 Aug 2026 15:22:38 -0400 Subject: [PATCH 11/14] fix: remove duplicate comment lines and trailing whitespace The GitHub suggestion commits appended new comment text without removing the old lines, producing duplicate comments. Also fixes trailing whitespace and restores tab indentation for struct field comments in Go. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> --- osac-operator/internal/controller/storage_controller.go | 6 ++---- .../internal/controller/storage_tier_resolution.go | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/osac-operator/internal/controller/storage_controller.go b/osac-operator/internal/controller/storage_controller.go index ef2c4ff91d..75dcf12e0b 100644 --- a/osac-operator/internal/controller/storage_controller.go +++ b/osac-operator/internal/controller/storage_controller.go @@ -982,10 +982,8 @@ type tenantSpecificStorageClasses struct { ambiguousTiers []string } -// resolveTenantSpecificStorageClasses lists only StorageClasses labeled with the -// given tenant name, ignoring shared defaults (labeled tenant=Default). Used when -// resolveTenantSpecificStorageClasses lists only StorageClasses labeled with the -// given tenant name. Used when AAP is configured. +// resolveTenantSpecificStorageClasses lists StorageClasses labeled with the +// given tenant name. Used when AAP is configured. func (r *StorageReconciler) resolveTenantSpecificStorageClasses( ctx context.Context, targetClient client.Client, tenantName string, ) (tenantSpecificStorageClasses, error) { diff --git a/osac-operator/internal/controller/storage_tier_resolution.go b/osac-operator/internal/controller/storage_tier_resolution.go index 9655df88ea..3cdd9a9a2f 100644 --- a/osac-operator/internal/controller/storage_tier_resolution.go +++ b/osac-operator/internal/controller/storage_tier_resolution.go @@ -36,10 +36,8 @@ type tierResolutionResult struct { errorMessages []string duplicateMessages []string // ambiguousTiers holds the name of every tier excluded from resolved because - // multiple StorageClasses matched it (tenant-specific or Default) — a distinct, -// ambiguousTiers holds the name of every tier excluded from resolved because -// multiple StorageClasses matched it — a distinct, separately-reported problem -// from a tier having no StorageClass at all. + // multiple StorageClasses matched it — a distinct, separately-reported problem + // from a tier having no StorageClass at all. ambiguousTiers []string } From e161f96798d11be67338209cf0a131afec4d6458 Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Thu, 6 Aug 2026 08:50:49 +0200 Subject: [PATCH 12/14] OSAC-3011: decouple LVMS operator install from StorageBackend registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lvms.enabled in osac-prereqs installs the LVMS operator and creates the LVMCluster. The register-local-storage Phase 3 hook previously also checked lvms.enabled, causing it to fire in vmaas-ci (which has lvms.enabled=true for VM disk management) and create an unexpected local StorageBackend there. This broke VMaaS E2E CI: the storage controller detected the StorageBackend, switched from the fallback path to the provisioning path, triggered AAP to create per-tenant StorageClasses, and compute instance tests ran before AAP finished — leaving Status.StorageClasses nil and failing the test. Introduce lvms.registerStorageBackend (default: false) in the osac chart. The register-local-storage hook now checks this flag instead. Set it to true only in edge-17 (OSAC-3234 testing) and leave vmaas-ci unchanged. Assisted-by: Claude Code Signed-off-by: Zoltan Szabo --- .../hooks/register-local-storage.yaml | 2 +- osac-installer/charts/osac/values.schema.json | 6 +-- osac-installer/charts/osac/values.yaml | 10 +++-- osac-installer/values/edge-17/values.yaml | 41 +++++++++++++++++++ 4 files changed, 51 insertions(+), 8 deletions(-) create mode 100644 osac-installer/values/edge-17/values.yaml diff --git a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml index e60a60447e..97393541a3 100644 --- a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml +++ b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml @@ -1,4 +1,4 @@ -{{- if .Values.lvms.enabled }} +{{- if .Values.lvms.registerStorageBackend }} apiVersion: batch/v1 kind: Job metadata: diff --git a/osac-installer/charts/osac/values.schema.json b/osac-installer/charts/osac/values.schema.json index 8ebe111a03..4501a15a58 100644 --- a/osac-installer/charts/osac/values.schema.json +++ b/osac-installer/charts/osac/values.schema.json @@ -1338,11 +1338,11 @@ }, "lvms": { "type": "object", - "description": "Register LVMS as a local StorageBackend + StorageTier in the fulfillment service after install. For CI/development environments only.", + "description": "LVMS StorageBackend registration in OSAC. Distinct from lvms.enabled in osac-prereqs (which installs the LVMS operator). Enable registerStorageBackend only in environments where LVMS is installed AND you want it exposed as an OSAC storage backend.", "properties": { - "enabled": { + "registerStorageBackend": { "type": "boolean", - "description": "Run the register-local-storage hook job when LVMS is deployed", + "description": "Register LVMS as a local StorageBackend + StorageTier in the fulfillment service. Do not enable in vmaas-ci or production.", "default": false } } diff --git a/osac-installer/charts/osac/values.yaml b/osac-installer/charts/osac/values.yaml index fdfede4889..116e7d9345 100644 --- a/osac-installer/charts/osac/values.yaml +++ b/osac-installer/charts/osac/values.yaml @@ -289,7 +289,9 @@ clusterVersions: # default: true lvms: - # Register LVMS as a local StorageBackend + StorageTier in the fulfillment service - # after install. For CI/development environments only — production deployments - # should use their own storage operators (Ceph, Pure, VAST, etc.). - enabled: false + # Distinct from lvms.enabled in charts/osac-prereqs (which installs the LVMS operator + # and creates the LVMCluster). This flag registers LVMS as a local StorageBackend + + # StorageTier in the OSAC fulfillment service. Enable only in environments where LVMS + # is installed AND you want it exposed as an OSAC-managed storage backend + # (dev/demo clusters). Do NOT enable in vmaas-ci or production — those use VAST/Ceph. + registerStorageBackend: false diff --git a/osac-installer/values/edge-17/values.yaml b/osac-installer/values/edge-17/values.yaml new file mode 100644 index 0000000000..cfbc2dd7a1 --- /dev/null +++ b/osac-installer/values/edge-17/values.yaml @@ -0,0 +1,41 @@ +# edge-17 dev values — CaaS LVMS testing (OSAC-3234) +# Extends values/caas-ci/values.yaml with edge-17-specific overrides. +# Usage (this file is an overlay — caas-ci is the base): +# make install-operators VALUES_FILE=values/caas-ci/values.yaml +# make install-prereqs VALUES_FILE=values/caas-ci/values.yaml DOMAIN= +# make install-osac VALUES_FILE=values/caas-ci/values.yaml DOMAIN= \ +# EXTRA_HELM_ARGS="--values values/edge-17/values.yaml \ +# --set-string aap.instanceGroups.clusterFulfillment.secret.AWS_ACCESS_KEY_ID= \ +# --set-string aap.instanceGroups.clusterFulfillment.secret.AWS_SECRET_ACCESS_KEY=" +# +# DOMAIN is the apps domain of the edge-17 cluster, e.g.: +# apps.caas-dev.eridani.com (or whatever cluster-tool assigns) +# Get it with: oc get ingresses.config/cluster -o jsonpath='{.spec.domain}' + +# --- Fulfillment Service --- +# externalHostname and internalHostname are set via DOMAIN= make var, no override needed here. + +# --- AAP --- +aap: + instanceGroups: + clusterFulfillment: + enabled: true + config: + NETWORK_STEPS_COLLECTION: "ci.steps" + HOSTED_CLUSTER_CONTROLLER_AVAILABILITY_POLICY: "SingleReplica" + HOSTED_CLUSTER_INFRASTRUCTURE_AVAILABILITY_POLICY: "SingleReplica" + EXTERNAL_ACCESS_BASE_DOMAIN: "ecoeng-osac-ci.devcluster.openshift.com" + EXTERNAL_ACCESS_SUPPORTED_BASE_DOMAINS: "ecoeng-osac-ci.devcluster.openshift.com" + HOSTED_CLUSTER_BASE_DOMAIN: "ecoeng-osac-ci.devcluster.openshift.com" + IMPORT_AGENTS_NAMESPACE: "hardware-inventory" + IMPORT_AGENTS_INFRAENV_NAME: "hardware-inventory" + IMPORT_AGENTS_PULL_SECRET_NAME: "pull-secret" + # AWS credentials for Route53 — passed via EXTRA_HELM_ARGS, not stored here: + # --set-string aap.instanceGroups.clusterFulfillment.secret.AWS_ACCESS_KEY_ID= + # --set-string aap.instanceGroups.clusterFulfillment.secret.AWS_SECRET_ACCESS_KEY= + +# --- LVMS StorageBackend --- +# edge-17 uses sno-4-22 (CaaS) which has LVMS installed via make install-operators. +# Register it as an OSAC StorageBackend for OSAC-3234 testing. +lvms: + registerStorageBackend: true From 2dc2dbfc1ec8a9936bed14263156ffda7783ec73 Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Thu, 6 Aug 2026 08:57:14 +0200 Subject: [PATCH 13/14] Revert "OSAC-3011: decouple LVMS operator install from StorageBackend registration" This reverts commit e161f96798d11be67338209cf0a131afec4d6458. --- .../hooks/register-local-storage.yaml | 2 +- osac-installer/charts/osac/values.schema.json | 6 +-- osac-installer/charts/osac/values.yaml | 10 ++--- osac-installer/values/edge-17/values.yaml | 41 ------------------- 4 files changed, 8 insertions(+), 51 deletions(-) delete mode 100644 osac-installer/values/edge-17/values.yaml diff --git a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml index 97393541a3..e60a60447e 100644 --- a/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml +++ b/osac-installer/charts/osac/templates/hooks/register-local-storage.yaml @@ -1,4 +1,4 @@ -{{- if .Values.lvms.registerStorageBackend }} +{{- if .Values.lvms.enabled }} apiVersion: batch/v1 kind: Job metadata: diff --git a/osac-installer/charts/osac/values.schema.json b/osac-installer/charts/osac/values.schema.json index 4501a15a58..8ebe111a03 100644 --- a/osac-installer/charts/osac/values.schema.json +++ b/osac-installer/charts/osac/values.schema.json @@ -1338,11 +1338,11 @@ }, "lvms": { "type": "object", - "description": "LVMS StorageBackend registration in OSAC. Distinct from lvms.enabled in osac-prereqs (which installs the LVMS operator). Enable registerStorageBackend only in environments where LVMS is installed AND you want it exposed as an OSAC storage backend.", + "description": "Register LVMS as a local StorageBackend + StorageTier in the fulfillment service after install. For CI/development environments only.", "properties": { - "registerStorageBackend": { + "enabled": { "type": "boolean", - "description": "Register LVMS as a local StorageBackend + StorageTier in the fulfillment service. Do not enable in vmaas-ci or production.", + "description": "Run the register-local-storage hook job when LVMS is deployed", "default": false } } diff --git a/osac-installer/charts/osac/values.yaml b/osac-installer/charts/osac/values.yaml index 116e7d9345..fdfede4889 100644 --- a/osac-installer/charts/osac/values.yaml +++ b/osac-installer/charts/osac/values.yaml @@ -289,9 +289,7 @@ clusterVersions: # default: true lvms: - # Distinct from lvms.enabled in charts/osac-prereqs (which installs the LVMS operator - # and creates the LVMCluster). This flag registers LVMS as a local StorageBackend + - # StorageTier in the OSAC fulfillment service. Enable only in environments where LVMS - # is installed AND you want it exposed as an OSAC-managed storage backend - # (dev/demo clusters). Do NOT enable in vmaas-ci or production — those use VAST/Ceph. - registerStorageBackend: false + # Register LVMS as a local StorageBackend + StorageTier in the fulfillment service + # after install. For CI/development environments only — production deployments + # should use their own storage operators (Ceph, Pure, VAST, etc.). + enabled: false diff --git a/osac-installer/values/edge-17/values.yaml b/osac-installer/values/edge-17/values.yaml deleted file mode 100644 index cfbc2dd7a1..0000000000 --- a/osac-installer/values/edge-17/values.yaml +++ /dev/null @@ -1,41 +0,0 @@ -# edge-17 dev values — CaaS LVMS testing (OSAC-3234) -# Extends values/caas-ci/values.yaml with edge-17-specific overrides. -# Usage (this file is an overlay — caas-ci is the base): -# make install-operators VALUES_FILE=values/caas-ci/values.yaml -# make install-prereqs VALUES_FILE=values/caas-ci/values.yaml DOMAIN= -# make install-osac VALUES_FILE=values/caas-ci/values.yaml DOMAIN= \ -# EXTRA_HELM_ARGS="--values values/edge-17/values.yaml \ -# --set-string aap.instanceGroups.clusterFulfillment.secret.AWS_ACCESS_KEY_ID= \ -# --set-string aap.instanceGroups.clusterFulfillment.secret.AWS_SECRET_ACCESS_KEY=" -# -# DOMAIN is the apps domain of the edge-17 cluster, e.g.: -# apps.caas-dev.eridani.com (or whatever cluster-tool assigns) -# Get it with: oc get ingresses.config/cluster -o jsonpath='{.spec.domain}' - -# --- Fulfillment Service --- -# externalHostname and internalHostname are set via DOMAIN= make var, no override needed here. - -# --- AAP --- -aap: - instanceGroups: - clusterFulfillment: - enabled: true - config: - NETWORK_STEPS_COLLECTION: "ci.steps" - HOSTED_CLUSTER_CONTROLLER_AVAILABILITY_POLICY: "SingleReplica" - HOSTED_CLUSTER_INFRASTRUCTURE_AVAILABILITY_POLICY: "SingleReplica" - EXTERNAL_ACCESS_BASE_DOMAIN: "ecoeng-osac-ci.devcluster.openshift.com" - EXTERNAL_ACCESS_SUPPORTED_BASE_DOMAINS: "ecoeng-osac-ci.devcluster.openshift.com" - HOSTED_CLUSTER_BASE_DOMAIN: "ecoeng-osac-ci.devcluster.openshift.com" - IMPORT_AGENTS_NAMESPACE: "hardware-inventory" - IMPORT_AGENTS_INFRAENV_NAME: "hardware-inventory" - IMPORT_AGENTS_PULL_SECRET_NAME: "pull-secret" - # AWS credentials for Route53 — passed via EXTRA_HELM_ARGS, not stored here: - # --set-string aap.instanceGroups.clusterFulfillment.secret.AWS_ACCESS_KEY_ID= - # --set-string aap.instanceGroups.clusterFulfillment.secret.AWS_SECRET_ACCESS_KEY= - -# --- LVMS StorageBackend --- -# edge-17 uses sno-4-22 (CaaS) which has LVMS installed via make install-operators. -# Register it as an OSAC StorageBackend for OSAC-3234 testing. -lvms: - registerStorageBackend: true From 9297982e56a25ea62e2fbca2f97987380443d742 Mon Sep 17 00:00:00 2001 From: Zoltan Szabo Date: Thu, 6 Aug 2026 09:14:00 +0200 Subject: [PATCH 14/14] OSAC-3011: enable storageFulfillment IG in vmaas-ci for LVMS storage testing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The register-local-storage hook (gated on lvms.enabled=true) now registers a local StorageBackend + StorageTier in the fulfillment service. The storage controller detects this and triggers AAP jobs to create per-tenant StorageClasses via the lvms_storage role. Without a storage-operations-ig configured in vmaas-ci, those jobs had no instance group to run on and the compute instance test failed with empty tenant_storage_classes. Enable storageFulfillment with STORAGE_TIERS pointing to the local LVMS tier so the storage IG exists and per-tenant StorageClasses are created before compute instance provisioning runs. STORAGE_TIERS here is temporary — will be replaced by osac_job_vars.storage_tier_definitions once OSAC-3013 AAP side lands. Assisted-by: Claude Code Signed-off-by: Zoltan Szabo --- osac-installer/values/vmaas-ci/values.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osac-installer/values/vmaas-ci/values.yaml b/osac-installer/values/vmaas-ci/values.yaml index 276ff1e966..80dc559de0 100644 --- a/osac-installer/values/vmaas-ci/values.yaml +++ b/osac-installer/values/vmaas-ci/values.yaml @@ -146,6 +146,14 @@ aap: eeImage: "ghcr.io/osac-project/osac-aap:latest" projectGitUri: "https://github.com/osac-project/osac" projectGitBranch: "main" + instanceGroups: + storageFulfillment: + enabled: true + config: + # OSAC-3011: local LVMS tier for VMaaS CI storage testing. + # Temporary: will be sourced from osac_job_vars.storage_tier_definitions + # once OSAC-3013 AAP side lands. + STORAGE_TIERS: '[{"name":"local","protocol":"block","provider":"lvms"}]' # --- Validation --- validation: