This repository was archived by the owner on Sep 9, 2026. It is now read-only.
Conversation
|
@zszabo-rh: This pull request references MGMT-23826 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 task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zszabo-rh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR implements end-to-end tenant storage provisioning via Ansible Automation Platform (AAP). It introduces an Ansible role ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
zszabo-rh
force-pushed
the
feature/mgmt-23826-tenant-storage-provision
branch
from
April 28, 2026 13:26
caeedcb to
ffa29f1
Compare
| pod_spec_override: "{{ lookup('ansible.builtin.template', 'compute-instance-operations-ig.j2') }}" | ||
| - name: "{{ aap_prefix }}-tenant-operations-ig" | ||
| is_container_group: true | ||
| pod_spec_override: "{{ lookup('ansible.builtin.template', 'compute-instance-operations-ig.j2') }}" |
Contributor
There was a problem hiding this comment.
we probably want to have a dedicated template? The idea of different pod definitions is to limit the RBAC rights allocated, and inject specific configuration for that job.
Contributor
Author
There was a problem hiding this comment.
Yes you're right..
Added a dedicated tenant-operations-ig.j2 template, it only has SA token + kube API access (no cluster-fulfillment-ig secret, no remote cluster kubeconfig, no compute-instance env vars).
zszabo-rh
force-pushed
the
feature/mgmt-23826-tenant-storage-provision
branch
from
April 28, 2026 16:16
491b3f9 to
ae7d34a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration/test-runner-uid (1)
1-2:⚠️ Potential issue | 🔴 CriticalThis
test-runner-uidfile is dead code and should be removed.The
POD_UIDenvironment variable is set directly intests/integration/run_tests.sh(line 16) with the placeholder value00000000-0000-0000-0000-000000000000, not from this file. The test runner never references or consumestests/integration/test-runner-uidanywhere in the codebase. The UUID in this file (9697bd78-1346-4e2a-9b4b-c44d5940c175) is not used. Delete this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/test-runner-uid` around lines 1 - 2, Remove the dead test-runner-uid file (tests/integration/test-runner-uid) because its UUID value (9697bd78-1346-4e2a-9b4b-c44d5940c175) is unused; delete the file from the repo so only the POD_UID value set in tests/integration/run_tests.sh is used and no orphaned UID file remains.
🧹 Nitpick comments (3)
playbook_osac_create_org_storage.yml (1)
6-21: Consider centralizing shared event parsing/validation with the delete playbook.This pre-task block is effectively duplicated in
playbook_osac_delete_org_storage.yml. Extracting it into one shared task include (with explicit payload shape validation) will reduce drift and make failures easier to diagnose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playbook_osac_create_org_storage.yml` around lines 6 - 21, Extract the duplicated pre_tasks (the vars parsing tenant/tenant_name/tenant_namespace and the set_fact for remote_cluster_kubeconfig plus the debug display) into a single included task file and replace the duplicated block in both playbook_osac_create_org_storage.yml and playbook_osac_delete_org_storage.yml with an import_tasks/include_tasks referencing that file; in the shared task file add explicit payload shape validation (e.g., check ansible_eda.event.payload and required metadata keys) and fail fast with a clear message if missing, keep the same variable names (tenant, tenant_name, tenant_namespace, remote_cluster_kubeconfig) so downstream tasks need no changes, and ensure both playbooks call the new include at the same point in pre_tasks.collections/ansible_collections/osac/service/roles/tenant_storage_provision/tests/test.yml (1)
21-171: Add guaranteed teardown withalwaysto keep test runs isolated.If any mid-test assertion fails, cleanup steps may be skipped and leave tenant StorageClasses behind for the next run. Wrapping core tests in a
blockwith analwaysdelete improves repeatability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@collections/ansible_collections/osac/service/roles/tenant_storage_provision/tests/test.yml` around lines 21 - 171, Wrap the core test sequence (Tests 1–5) in an Ansible block and add an always handler that runs the delete role to guarantee teardown; specifically, enclose the include_role tasks that call osac.service.tenant_storage_provision with tasks_from: create (the "Test 1: Create...", "Test 4: Create..." and the idempotency re-run) and the verification/assertion tasks inside a block, and add an always: section that calls the include_role with tasks_from: delete (the same include_role used in "Test 3: Delete tenant StorageClasses" / "Test 5: Delete all multi-tier SCs") to ensure tenant_storage_provision_tenant_name is deleted even if assertions fail. Ensure the always block passes the same tenant_storage_provision_tenant_name and any other vars (tenant_storage_provision_reference_sc, tenant_storage_provision_tiers) as needed so cleanup is deterministic.collections/ansible_collections/osac/service/roles/tenant_storage_provision/meta/argument_specs.yaml (1)
18-27: Add nested tier option schema for fail-fast validation.
tenant_storage_provision_tierscurrently accepts arbitrary dicts. Defining nested options (with requiredname) prevents late failures intasks/create.yamlwhen fields are missing or mistyped.💡 Suggested schema enhancement
tenant_storage_provision_tiers: type: list elements: dict required: false + options: + name: + type: str + required: true + provisioner: + type: str + required: false + parameters: + type: dict + required: false + reclaim_policy: + type: str + required: false + volume_binding_mode: + type: str + required: false + allow_volume_expansion: + type: bool + required: false + mount_options: + type: list + elements: str + required: false description: >- List of storage tiers to provision. Each entry creates one StorageClass named <tenant>-<tier>. Defaults to [{name: default}].🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@collections/ansible_collections/osac/service/roles/tenant_storage_provision/meta/argument_specs.yaml` around lines 18 - 27, The tenant_storage_provision_tiers argument currently accepts arbitrary dicts which causes late failures; update its schema in argument_specs.yaml to define a nested options spec for each tier (use elements: dict with an options block) requiring name and specifying types and constraints for known keys (e.g., name: required string, provisioner: string, parameters: dict, reclaim_policy: string with allowed values, volume_binding_mode: string with allowed values, allow_volume_expansion: bool, mount_options: list of strings) so validation fails fast before tasks/create.yaml runs; ensure the key names match the usage in tasks/create.yaml and any defaults remain compatible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@collections/ansible_collections/osac/config_as_code/roles/aap/vars/controller.yml`:
- Around line 683-684: Replace the reused secretRef named cluster-fulfillment-ig
in the hostpool-operations-ig instance group with a dedicated secret for
hostpool operations: create a new secret (e.g., hostpool-operations-secret)
containing only the credentials needed by hostpool-operations-ig, update the
hostpool-operations-ig entry to reference secretRef: name:
hostpool-operations-secret instead of cluster-fulfillment-ig, and ensure the new
secret is provisioned and RBAC/variable scopes are adjusted so only
hostpool-operations-ig can access it.
In
`@collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/create.yaml`:
- Around line 19-34: Collect all StorageClass names marked as default into a
list instead of overwriting _tsp_reference_sc_name: create a fact (e.g.
_tsp_default_candidates) by appending item.metadata.name in the loop that checks
annotations, then after the loop validate the list length is exactly 1; if
length == 1 set _tsp_reference_sc_name to the single candidate, otherwise fail
with an explicit message about multiple or no default StorageClasses. Update the
tasks that currently reference _tsp_reference_sc_name (the "Select default
StorageClass" block and the fail check) to use the new candidate-list logic and
to set _tsp_reference_sc_name from the validated candidate.
In
`@collections/ansible_collections/osac/service/roles/tenant_storage_provision/tests/test.yml`:
- Around line 47-54: The test indexes into _test1_sc.resources[0] before
asserting the list exists, risking an index error; change the assertion to avoid
direct indexing by either (a) compute _sc_labels using the safe Jinja2 first
filter like "_sc_labels: \"{{ (_test1_sc.resources | first).metadata.labels
}}\"" or (b) move the "_sc_labels" vars assignment to after you assert
"_test1_sc.resources | length == 1"; update the assertions to reference
_sc_labels and the existing symbols _test1_sc.resources, _sc_labels and
test_tenant_name accordingly so the length check always runs before accessing
the first item.
---
Outside diff comments:
In `@tests/integration/test-runner-uid`:
- Around line 1-2: Remove the dead test-runner-uid file
(tests/integration/test-runner-uid) because its UUID value
(9697bd78-1346-4e2a-9b4b-c44d5940c175) is unused; delete the file from the repo
so only the POD_UID value set in tests/integration/run_tests.sh is used and no
orphaned UID file remains.
---
Nitpick comments:
In
`@collections/ansible_collections/osac/service/roles/tenant_storage_provision/meta/argument_specs.yaml`:
- Around line 18-27: The tenant_storage_provision_tiers argument currently
accepts arbitrary dicts which causes late failures; update its schema in
argument_specs.yaml to define a nested options spec for each tier (use elements:
dict with an options block) requiring name and specifying types and constraints
for known keys (e.g., name: required string, provisioner: string, parameters:
dict, reclaim_policy: string with allowed values, volume_binding_mode: string
with allowed values, allow_volume_expansion: bool, mount_options: list of
strings) so validation fails fast before tasks/create.yaml runs; ensure the key
names match the usage in tasks/create.yaml and any defaults remain compatible.
In
`@collections/ansible_collections/osac/service/roles/tenant_storage_provision/tests/test.yml`:
- Around line 21-171: Wrap the core test sequence (Tests 1–5) in an Ansible
block and add an always handler that runs the delete role to guarantee teardown;
specifically, enclose the include_role tasks that call
osac.service.tenant_storage_provision with tasks_from: create (the "Test 1:
Create...", "Test 4: Create..." and the idempotency re-run) and the
verification/assertion tasks inside a block, and add an always: section that
calls the include_role with tasks_from: delete (the same include_role used in
"Test 3: Delete tenant StorageClasses" / "Test 5: Delete all multi-tier SCs") to
ensure tenant_storage_provision_tenant_name is deleted even if assertions fail.
Ensure the always block passes the same tenant_storage_provision_tenant_name and
any other vars (tenant_storage_provision_reference_sc,
tenant_storage_provision_tiers) as needed so cleanup is deterministic.
In `@playbook_osac_create_org_storage.yml`:
- Around line 6-21: Extract the duplicated pre_tasks (the vars parsing
tenant/tenant_name/tenant_namespace and the set_fact for
remote_cluster_kubeconfig plus the debug display) into a single included task
file and replace the duplicated block in both
playbook_osac_create_org_storage.yml and playbook_osac_delete_org_storage.yml
with an import_tasks/include_tasks referencing that file; in the shared task
file add explicit payload shape validation (e.g., check
ansible_eda.event.payload and required metadata keys) and fail fast with a clear
message if missing, keep the same variable names (tenant, tenant_name,
tenant_namespace, remote_cluster_kubeconfig) so downstream tasks need no
changes, and ensure both playbooks call the new include at the same point in
pre_tasks.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13de51ca-feb0-43cb-9cd0-dac7c0d296ad
📒 Files selected for processing (12)
collections/ansible_collections/osac/config_as_code/roles/aap/vars/controller.ymlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/defaults/main.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/meta/argument_specs.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/cleanup_backend.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/configure_backend.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/create.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/delete.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tests/test.ymlplaybook_osac_create_org_storage.ymlplaybook_osac_delete_org_storage.ymlrulebooks/cluster_fulfillment.ymltests/integration/test-runner-uid
zszabo-rh
force-pushed
the
feature/mgmt-23826-tenant-storage-provision
branch
2 times, most recently
from
April 29, 2026 09:13
f9c4c40 to
a7a9cd6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/create.yaml (1)
58-79: Add preflight validation for tier names before provisioning.The loop will fail mid-execution if tier names are invalid or duplicated, potentially leaving partial provisioning. Add upfront assertions to validate Kubernetes-safe naming and uniqueness.
💡 Suggested patch
+- name: Validate tier names before provisioning + ansible.builtin.assert: + that: + - (tenant_storage_provision_tiers | map(attribute='name') | list | length) > 0 + - (tenant_storage_provision_tiers | map(attribute='name') | list | unique | length) == + (tenant_storage_provision_tiers | length) + - item.name is match('^[a-z0-9]([-a-z0-9]*[a-z0-9])?$') + - (item.name | length) <= 63 + fail_msg: "Invalid or duplicate tier name '{{ item.name }}'." + loop: "{{ tenant_storage_provision_tiers }}" + loop_control: + label: "{{ item.name }}" + - name: Create tenant-specific StorageClass per tier kubernetes.core.k8s:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/create.yaml` around lines 58 - 79, Before the StorageClass creation loop, add a preflight validation task that inspects tenant_storage_provision_tiers to ensure each item.name is a valid Kubernetes DNS-1123 label (e.g. matches regex ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ and <=63 chars) and that all names are unique; if validation fails, fail early to avoid partial provisioning. Place this check immediately before the task that loops over tenant_storage_provision_tiers (the task that registers _tsp_create_results) and use Ansible's assert or fail with a computed list of invalid names and a uniqueness check (compare the list to its unique version) so the play aborts with a clear error when names are invalid or duplicated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/create.yaml`:
- Line 16: Change the kubeconfig default to treat empty strings as unset by
replacing occurrences of kubeconfig: "{{ kubeconfig | default(omit) }}" with
kubeconfig: "{{ kubeconfig | default(omit, true) }}" in this role; specifically
update the first k8s_info call, the k8s_info SC fetch reference, the k8s SC
creation loop in create.yaml, and the two analogous kubeconfig entries in
delete.yaml so empty-string kubeconfig values are omitted.
---
Nitpick comments:
In
`@collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/create.yaml`:
- Around line 58-79: Before the StorageClass creation loop, add a preflight
validation task that inspects tenant_storage_provision_tiers to ensure each
item.name is a valid Kubernetes DNS-1123 label (e.g. matches regex
^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ and <=63 chars) and that all names are unique;
if validation fails, fail early to avoid partial provisioning. Place this check
immediately before the task that loops over tenant_storage_provision_tiers (the
task that registers _tsp_create_results) and use Ansible's assert or fail with a
computed list of invalid names and a uniqueness check (compare the list to its
unique version) so the play aborts with a clear error when names are invalid or
duplicated.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5905c6e0-82d8-4cb4-8664-5415cac881fd
📒 Files selected for processing (13)
collections/ansible_collections/osac/config_as_code/roles/aap/templates/tenant-operations-ig.j2collections/ansible_collections/osac/config_as_code/roles/aap/vars/controller.ymlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/defaults/main.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/meta/argument_specs.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/cleanup_backend.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/configure_backend.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/create.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/delete.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tests/test.ymlplaybook_osac_create_org_storage.ymlplaybook_osac_delete_org_storage.ymlrulebooks/cluster_fulfillment.ymltests/integration/test-runner-uid
✅ Files skipped from review due to trivial changes (10)
- tests/integration/test-runner-uid
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/configure_backend.yaml
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/cleanup_backend.yaml
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/defaults/main.yaml
- rulebooks/cluster_fulfillment.yml
- playbook_osac_delete_org_storage.yml
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/meta/argument_specs.yaml
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/delete.yaml
- collections/ansible_collections/osac/config_as_code/roles/aap/templates/tenant-operations-ig.j2
- playbook_osac_create_org_storage.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/tests/test.yml
- collections/ansible_collections/osac/config_as_code/roles/aap/vars/controller.yml
zszabo-rh
force-pushed
the
feature/mgmt-23826-tenant-storage-provision
branch
3 times, most recently
from
May 5, 2026 11:49
0de23bb to
68112bb
Compare
Merged
4 tasks
zszabo-rh
force-pushed
the
feature/mgmt-23826-tenant-storage-provision
branch
from
May 8, 2026 07:10
68112bb to
6ef9454
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@collections/ansible_collections/osac/config_as_code/roles/aap/vars/controller.yml`:
- Line 809: Remove the unresolved merge-conflict marker present in
controller.yml (the line starting with ">>>>>>> 02f5b8c") so the YAML can parse;
locate any matching conflict block (look for "<<<<<<<" and "=======" around the
same section), decide which of the conflicting sections to keep (or merge their
contents correctly), delete the conflict markers (<<<<<<<, =======, >>>>>>>) and
ensure the resulting YAML node under the surrounding keys is syntactically valid
and indented correctly before committing.
In `@playbook_osac_delete_org_storage.yml`:
- Around line 18-20: The playbook currently sets remote_cluster_kubeconfig
directly using the environment lookup (variable remote_cluster_kubeconfig)
instead of sourcing it from the shared role; remove the manual
ansible.builtin.set_fact, add the role osac.service.common to pre_tasks, and
consume the remote_cluster_kubeconfig variable provided by that role before the
tenant storage deletion tasks so the playbook uses the centralized resolution
logic.
In
`@vendor/ansible_collections/osac/massopencloud/roles/ocp_4_17_small/meta/argument_specs.yaml`:
- Around line 28-32: The ssh_public_key argument spec is missing required: true
so callers can omit it; update the ssh_public_key entry in argument_specs.yaml
(the ssh_public_key argument) to include required: true (keeping type: str and
the existing description) so Ansible validates its presence before runtime.
- Around line 20-32: The argument spec currently defines template_parameters as
a dict but doesn't mark it required, so pull_secret's required: true is never
enforced; update the template_parameters option (the dict named
template_parameters) to include required: true so Ansible validates presence of
the dict and then enforces pull_secret's required constraint (ensure you modify
the template_parameters entry in the argument_specs.yaml near the
template_parameters block and keep pull_secret and ssh_public_key as its
options).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a7d942f-f952-4359-a702-914441041df7
📒 Files selected for processing (14)
collections/ansible_collections/osac/config_as_code/roles/aap/templates/tenant-operations-ig.j2collections/ansible_collections/osac/config_as_code/roles/aap/vars/controller.ymlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/defaults/main.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/meta/argument_specs.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/cleanup_backend.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/configure_backend.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/create.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/delete.yamlcollections/ansible_collections/osac/service/roles/tenant_storage_provision/tests/test.ymlplaybook_osac_create_org_storage.ymlplaybook_osac_delete_org_storage.ymlrulebooks/cluster_fulfillment.ymltests/integration/test-runner-uidvendor/ansible_collections/osac/massopencloud/roles/ocp_4_17_small/meta/argument_specs.yaml
✅ Files skipped from review due to trivial changes (8)
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/cleanup_backend.yaml
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/configure_backend.yaml
- collections/ansible_collections/osac/config_as_code/roles/aap/templates/tenant-operations-ig.j2
- tests/integration/test-runner-uid
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/meta/argument_specs.yaml
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/defaults/main.yaml
- playbook_osac_create_org_storage.yml
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/tests/test.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- rulebooks/cluster_fulfillment.yml
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/create.yaml
- collections/ansible_collections/osac/service/roles/tenant_storage_provision/tasks/delete.yaml
zszabo-rh
force-pushed
the
feature/mgmt-23826-tenant-storage-provision
branch
from
May 8, 2026 08:01
9be595d to
a12b035
Compare
Ansible role and playbooks for automated tenant StorageClass lifecycle. When triggered by the OSAC operator (via AAP), creates tenant-specific StorageClasses cloned from a reference SC with proper labels. Key changes: - New role: osac.service.tenant_storage_provision (create/delete tasks) - Both osac.openshift.io/tenant and osac.openshift.io/storage-tier labels - Multi-tier support via tenant_storage_provision_tiers parameter - Per-tier provisioner/parameter overrides from reference SC - CSP extension point (configure_backend.yaml) for backend-specific setup - Playbooks: playbook_osac_create_tenant_storage.yml / delete - Config-as-code: job templates, inventory, instance group registration - EDA rulebook entries for tenant events - Test suite: 6 test cases (create, idempotency, delete, multi-tier, multi-tier delete, delete-on-empty) Companion PR: osac-project/osac-operator (controller integration) Signed-off-by: Zoltan Szabo <zszabo@redhat.com> Generated-By: Claude Code (Anthropic) Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
Per Avishay's feedback: align with the tenant → organization rename. - AAP template names: osac-create-tenant → osac-create-org, osac-delete-tenant → osac-delete-org - Playbook files: playbook_osac_create_tenant_storage → playbook_osac_create_org_storage - Rulebook entries updated to match Internal role name (tenant_storage_provision) unchanged — the Ansible role operates on Tenant CRs and the internal naming is an implementation detail. Signed-off-by: Zoltan Szabo <zszabo@redhat.com> Assisted-By: Claude Code (Anthropic)
- tenant-operations-ig: new dedicated Jinja template with minimal permissions (SA token + kube API access only, no fulfillment secrets or remote cluster config) - hostpool-operations-ig: fix secret reference from cluster-fulfillment-ig to hostpool-operations-ig for credential isolation Signed-off-by: Zoltan Szabo <zszabo@redhat.com> Assisted-By: Claude Code (Anthropic)
Backends like VAST need to provision resources (tenant project, VIP pool, credentials secret) before the StorageClass can be created, because SC parameters reference those resources. The previous "after" ordering worked for shared Ceph but is wrong for any backend where the SC depends on hook-created resources. The hook now runs after the reference SC is fetched but before the SC creation loop, so it can modify tenant_storage_provision_tiers via set_fact to dynamically set provisioner and parameters. Cleanup ordering (cleanup_backend → delete SCs) is unchanged — the hook needs access to SC objects to discover what to clean up. Signed-off-by: Zoltan Szabo <zszabo@redhat.com> Assisted-By: Claude Code (Anthropic)
Signed-off-by: Zoltan Szabo <zszabo@redhat.com> Assisted-By: Claude Code (Anthropic)
zszabo-rh
force-pushed
the
feature/mgmt-23826-tenant-storage-provision
branch
from
May 26, 2026 07:55
ef000be to
50bf6cf
Compare
|
@zszabo-rh: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Contributor
Author
|
Closing this PR — superseded by #296 (merged May 27). PR #296 implements the What carries forward from this PR:
See the storage architecture overview and Akshay's storage planning doc for the full context on the phase-based design. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
osac.service.tenant_storage_provisionfor automated tenant StorageClass lifecycleplaybook_osac_create_org_storage.yml/playbook_osac_delete_org_storage.ymlosac-create-org/osac-delete-orgtenant-operations-iginstance group template (minimal RBAC, no fulfillment secrets)configure_backend.yaml(post-create) andcleanup_backend.yaml(pre-delete)What it does
When the operator creates a Tenant without a pre-existing StorageClass, it triggers the
osac-create-orgAAP job template. The playbook:tenant_storage_provision_reference_sc)tenant_storage_provision_tierswithosac.openshift.io/tenantandosac.openshift.io/storage-tierlabelsconfigure_backend.yaml— empty by default, CSPs override for backend-specific setup (VAST, Ceph quotas, etc.)On Tenant deletion,
osac-delete-orgcallscleanup_backend.yaml(reverse cleanup), then removes all labeled SCs.CSP customization
Three extension points, all configured via AAP extra variables or inventory — no code changes needed:
tenant_storage_provision_reference_sctenant_storage_provision_tiersconfigure_backend.yaml/cleanup_backend.yamlMulti-tier example
Test plan
Assisted-By: Claude Code (Anthropic)
Summary by CodeRabbit
New Features
Tests