Skip to content
This repository was archived by the owner on Sep 9, 2026. It is now read-only.

OSAC-1123: enable storage role k8s calls to target remote CaaS clusters - #379

Closed
akshaynadkarni wants to merge 1 commit into
osac-project:mainfrom
akshaynadkarni:feat/OSAC-1123-caas-storage-kubeconfig
Closed

akshaynadkarni wants to merge 1 commit into
osac-project:mainfrom
akshaynadkarni:feat/OSAC-1123-caas-storage-kubeconfig

Conversation

@akshaynadkarni

@akshaynadkarni akshaynadkarni commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Add kubeconfig parameter to all kubernetes.core.k8s and kubernetes.core.k8s_info calls in the VAST storage role so that StorageClasses, CSI resources, and cleanup operations target the correct cluster (OSAC-1123).

Why

The VAST storage role creates StorageClasses, CSI Secrets, and VolumeSnapshotClasses using kubernetes.core.k8s calls that default to the local cluster context. For CaaS clusters, these resources must be created on the tenant's hosted cluster, not the hub. Adding kubeconfig: "{{ _remote_kubeconfig | default(omit) }}" routes each call to the correct cluster when a remote kubeconfig is available, while preserving existing VMaaS behavior (where _remote_kubeconfig is unset and default(omit) drops the parameter).

Testing

$ ansible-lint collections/ansible_collections/osac/templates/roles/vast_storage/
Passed with no violations

19 k8s calls updated across 3 files:

  • ensure_storage_class.yaml (6 calls)
  • ensure_csi_operator.yaml (8 calls)
  • teardown_cluster_storage.yaml (5 calls)

Pre-merge ToDos

  1. Coordinate with osac-operator PR fix: use osac.openshift.io for all annotations #324 (storage controller CaaS support)

Post-merge ToDos

  1. Update osac-installer submodule ref for base/osac-aap
  2. Coordinate with PR OSAC-1327: adds hcp_data_plane provisioning target for CaaS StorageClasses #377 (OSAC-1327, hcp_data_plane target) which resolves _remote_kubeconfig at the playbook level

Related PRs

Ticket

OSAC-1123


Signed-off-by: akshaynadkarni 25892229+akshaynadkarni@users.noreply.github.com
Assisted-by: Cursor/Claude

Summary by CodeRabbit

  • Bug Fixes
    • Kubernetes actions now consistently use the provided remote kubeconfig when available, helping cluster setup, storage class management, and teardown run against the intended cluster.
    • Improved reliability for creating, checking, and removing CSI-related resources, including operator components, storage classes, snapshot classes, and tenant secrets.

Add kubeconfig: "{{ _remote_kubeconfig | default(omit) }}" to all
kubernetes.core.k8s and kubernetes.core.k8s_info calls in the VAST
storage role so that StorageClasses, CSI resources, and cleanup
operations target the correct cluster.

For VMaaS, _remote_kubeconfig is not set, so default(omit) omits the
parameter entirely and existing behavior is preserved. For CaaS,
_remote_kubeconfig is resolved by the playbook-level kubeconfig
handling (OSAC-1327) and passed through to each k8s call.

19 k8s calls updated across 3 files:
- ensure_storage_class.yaml (6)
- ensure_csi_operator.yaml (8)
- teardown_cluster_storage.yaml (5)

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
@openshift-ci-robot

openshift-ci-robot commented Jun 27, 2026

Copy link
Copy Markdown

@akshaynadkarni: This pull request references OSAC-1123 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

Add kubeconfig parameter to all kubernetes.core.k8s and kubernetes.core.k8s_info calls in the VAST storage role so that StorageClasses, CSI resources, and cleanup operations target the correct cluster (OSAC-1123).

Why

The VAST storage role creates StorageClasses, CSI Secrets, and VolumeSnapshotClasses using kubernetes.core.k8s calls that default to the local cluster context. For CaaS clusters, these resources must be created on the tenant's hosted cluster, not the hub. Adding kubeconfig: "{{ _remote_kubeconfig | default(omit) }}" routes each call to the correct cluster when a remote kubeconfig is available, while preserving existing VMaaS behavior (where _remote_kubeconfig is unset and default(omit) drops the parameter).

Testing

$ ansible-lint collections/ansible_collections/osac/templates/roles/vast_storage/
Passed with no violations

19 k8s calls updated across 3 files:

  • ensure_storage_class.yaml (6 calls)
  • ensure_csi_operator.yaml (8 calls)
  • teardown_cluster_storage.yaml (5 calls)

Pre-merge ToDos

  1. Coordinate with osac-operator PR fix: use osac.openshift.io for all annotations #324 (storage controller CaaS support)
  2. Coordinate with PR OSAC-1327: adds hcp_data_plane provisioning target for CaaS StorageClasses #377 (OSAC-1327, hcp_data_plane target) which resolves _remote_kubeconfig at the playbook level

Related PRs

Ticket

OSAC-1123


Signed-off-by: akshaynadkarni 25892229+akshaynadkarni@users.noreply.github.com
Assisted-by: Cursor/Claude

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci

openshift-ci Bot commented Jun 27, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci

openshift-ci Bot commented Jun 27, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akshaynadkarni

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Adds kubeconfig: "{{ _remote_kubeconfig | default(omit) }}" to 19 kubernetes.core.k8s_info and kubernetes.core.k8s task invocations across three vast_storage task files: ensure_csi_operator.yaml, ensure_storage_class.yaml, and teardown_cluster_storage.yaml. No control flow, resource definitions, or other logic is changed.

Changes

Remote kubeconfig scoping for vast_storage tasks

Layer / File(s) Summary
kubeconfig added to CSI operator tasks
roles/vast_storage/tasks/ensure_csi_operator.yaml
Eight tasks receive the kubeconfig parameter: CSI driver check, OLM check, namespace/OperatorGroup/Subscription creation, CRD wait, VastCSIDriver CR creation per protocol, and driver registration wait.
kubeconfig added to StorageClass tasks
roles/vast_storage/tasks/ensure_storage_class.yaml
Six tasks receive the kubeconfig parameter: StorageClass existence check, CSI Secret creation, NFS and block StorageClass creation, VolumeSnapshot CRD check, and VolumeSnapshotClass creation.
kubeconfig added to teardown tasks
roles/vast_storage/tasks/teardown_cluster_storage.yaml
Five tasks receive the kubeconfig parameter: StorageClass list/delete, VolumeSnapshotClass list/delete, and CSI Secret deletion.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

  • osac-project/osac-aap#296: Touches the same vast_storage task files (ensure_csi_operator, ensure_storage_class) that this PR further updates with _remote_kubeconfig scoping.
  • osac-project/osac-aap#338: Introduced or refactored teardown_cluster_storage.yaml, the same file this PR updates with remote kubeconfig scoping on all delete tasks.

Suggested labels

lgtm

Suggested reviewers

  • larsks
  • eranco74

Poem

A kubeconfig passed, no cluster confused,
Each task now knows which API to use.
_remote_kubeconfig | default(omit) — neat,
Nineteen lines added, the pattern complete.
🎯 Targeted calls, no accidental delete!

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Hardcoded-Secrets ✅ Passed No hardcoded secrets found: the PR only adds templated kubeconfig fields; password/secret-looking keys are variable references, not string literals, and no base64 or embedded creds appear.
No-Weak-Crypto ✅ Passed Touched tasks only add kubeconfig to k8s/k8s_info calls; search found no MD5/SHA1/DES/RC4/ECB/custom crypto or secret comparisons.
No-Injection-Vectors ✅ Passed Changed YAML only adds kubeconfig to k8s/k8s_info tasks; no shell/eval/pickle/yaml.load/os.system/dangerous HTML patterns found.
Container-Privileges ✅ Passed Touched files are Ansible k8s tasks only; no privileged/hostPID/hostNetwork/hostIPC/allowPrivilegeEscalation/SYS_ADMIN fields were added.
No-Sensitive-Data-In-Logs ✅ Passed PASS: The PR only adds kubeconfig to k8s calls; no new logs expose credentials, tokens, or hostnames, and secret-handling tasks remain no_log.
Ai-Attribution ✅ Passed AI use is disclosed and properly attributed: HEAD commit has an 'Assisted-by: Cursor/Claude' trailer; no 'Co-Authored-By' misuse was found.
Title check ✅ Passed The title clearly matches the main change: adding kubeconfig support so storage role Kubernetes calls target remote CaaS clusters.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@akshaynadkarni

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/templates/roles/vast_storage/tasks/ensure_storage_class.yaml`:
- Line 44: The remote kubeconfig path is still blocked by the provisioning
guard, so the kubeconfig-scoped Kubernetes tasks in the storage-class flow never
run for CaaS/remote-cluster cases. Update the control flow around the affected
task blocks in ensure_storage_class so they are not gated by the current
_provisioning_target != 'vmaas' fail path, and let the kubeconfig-optional calls
execute for non-VMaaS runs. Keep the existing kubeconfig: "{{ _remote_kubeconfig
| default(omit) }}" pattern on the Kubernetes invocations and adjust the
surrounding conditionals/fail logic instead.

In
`@collections/ansible_collections/osac/templates/roles/vast_storage/tasks/teardown_cluster_storage.yaml`:
- Line 24: The teardown path in teardown_cluster_storage is still gated by the
vmaas-only _provisioning_target check, so the kubeconfig-scoped cleanup tasks
for remote-cluster/CaaS never execute. Update the gating logic in the role so
the cleanup block containing the StorageClass, VolumeSnapshotClass, and CSI
Secret removals is reachable for the remote-cluster path as well, while keeping
the existing _remote_kubeconfig and default(omit) usage in those 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: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 6685ed94-7970-40cf-9b5e-5fd04ea57df9

📥 Commits

Reviewing files that changed from the base of the PR and between d4ca8f6 and a0b2426.

📒 Files selected for processing (3)
  • collections/ansible_collections/osac/templates/roles/vast_storage/tasks/ensure_csi_operator.yaml
  • collections/ansible_collections/osac/templates/roles/vast_storage/tasks/ensure_storage_class.yaml
  • collections/ansible_collections/osac/templates/roles/vast_storage/tasks/teardown_cluster_storage.yaml

@akshaynadkarni

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@akshaynadkarni
akshaynadkarni marked this pull request as ready for review June 28, 2026 04:25
@openshift-ci
openshift-ci Bot requested review from eliorerz and eranco74 June 28, 2026 04:25
@akshaynadkarni akshaynadkarni changed the title OSAC-1123: add kubeconfig parameter to VAST storage k8s calls OSAC-1123: enable storage role k8s calls to target remote CaaS clusters Jun 28, 2026
@akshaynadkarni
akshaynadkarni requested review from avishayt, rgolangh, wgordon17 and zszabo-rh and removed request for eliorerz and eranco74 June 28, 2026 04:34
@akshaynadkarni

Copy link
Copy Markdown
Contributor Author

Closing this PR.
Duplicate changes were made to a separate PR (#377):
commit: 06e48c2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants