OSAC-3183: update ocp_virt_vm role to read GPU from CR payload

[Tzif-Morgen]

OSAC-3183: update ocp_virt_vm role to read GPU from CR payload

Jira: OSAC-3183
Story type: [DEV]

Summary

Updates the ocp_virt_vm Ansible role to read GPU configuration from compute_instance.spec.gpu (the CR payload) instead of the old gpu_devices role parameter. The role now loops over spec.gpu.count to create KubeVirt hostDevices entries, each using pciDeviceSelector and resourceName from the GpuSpec struct.

Changes

• argument_specs.yaml: Remove gpu_devices parameter (32 lines)
• create_build_spec.yaml: Loop range(spec.gpu.count) instead of iterating gpu_devices list
• configure_permitted_host_devices.yaml: Build desired_pci_host_devices from single spec.gpu entry; replace all gpu_devices guards with compute_instance.spec.gpu is defined
• create.yaml: Update guard for configure_permitted_host_devices include
• test_overrides/ocp_virt_vm_with_gpu: Delete entire role (no longer needed)
• Integration fixture: Add spec.gpu block, switch templateID to base role
• Integration test: Assertions read from fixture instead of hardcoded values

Testing

• Unit tests: 3 new tests added (GPU single count=1, GPU many count=3, no GPU backward compat)
• Integration tests: Updated to use CR-based GPU fields; requires kind cluster to run
• Output verification: vm_template_spec output verified identical between branch and main for all 3 scenarios

Acceptance Criteria

• AC-1: Role reads GPU fields from compute_instance.spec.gpu
• AC-2: Loop over count creates correct hostDevices entries
• AC-3: Old gpu_devices parameter removed
• AC-4: Non-GPU ComputeInstances work without hostDevices
• AC-5: No playbook changes needed

Dependencies

• OSAC-3162 (GpuSpec CRD struct) — merged
• OSAC-3182 (reconciler stamps GPU from InstanceType onto CR) — merged

Summary by CodeRabbit

• New Features

  • GPU-enabled virtual machines now configure host devices directly from the VM’s GPU settings.
  • Supports selecting GPU resource names and requesting multiple GPUs with sequential device configuration.
  • VMs without GPU settings no longer receive host-device configuration.

• Bug Fixes

  • Improved GPU passthrough handling for single- and multi-GPU virtual machines.

• Tests

  • Added validation coverage for GPU counts, resource names, device naming, and GPU-disabled VMs.

[openshift-ci-robot]

@Tzif-Morgen: This pull request references OSAC-3183 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.

Details

In response to this:

OSAC-3183: update ocp_virt_vm role to read GPU from CR payload

Jira: OSAC-3183
Story type: [DEV]

Summary

Updates the ocp_virt_vm Ansible role to read GPU configuration from compute_instance.spec.gpu (the CR payload) instead of the old gpu_devices role parameter. The role now loops over spec.gpu.count to create KubeVirt hostDevices entries, each using pciDeviceSelector and resourceName from the GpuSpec struct.

Changes

• argument_specs.yaml: Remove gpu_devices parameter (32 lines)
• create_build_spec.yaml: Loop range(spec.gpu.count) instead of iterating gpu_devices list
• configure_permitted_host_devices.yaml: Build desired_pci_host_devices from single spec.gpu entry; replace all gpu_devices guards with compute_instance.spec.gpu is defined
• create.yaml: Update guard for configure_permitted_host_devices include
• test_overrides/ocp_virt_vm_with_gpu: Delete entire role (no longer needed)
• Integration fixture: Add spec.gpu block, switch templateID to base role
• Integration test: Assertions read from fixture instead of hardcoded values

Testing

• Unit tests: 3 new tests added (GPU single count=1, GPU many count=3, no GPU backward compat)
• Integration tests: Updated to use CR-based GPU fields; requires kind cluster to run
• Output verification: vm_template_spec output verified identical between branch and main for all 3 scenarios

Acceptance Criteria

• AC-1: Role reads GPU fields from compute_instance.spec.gpu
• AC-2: Loop over count creates correct hostDevices entries
• AC-3: Old gpu_devices parameter removed
• AC-4: Non-GPU ComputeInstances work without hostDevices
• AC-5: No playbook changes needed

Dependencies

• OSAC-3162 (GpuSpec CRD struct) — merged
• OSAC-3182 (reconciler stamps GPU from InstanceType onto CR) — merged

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

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Tzif-Morgen
Once this PR has been reviewed and has the lgtm label, please assign akshaynadkarni for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• osac-aap/OWNERS

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 10e3bb85-7d0e-41de-80c3-cf8872aaa839

📥 Commits

Reviewing files that changed from the base of the PR and between 069c83e and 989e2d9.

📒 Files selected for processing (13)

• osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/meta/argument_specs.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tasks/configure_permitted_host_devices.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tasks/create.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tasks/create_build_spec.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tests/fixtures/computeinstance-with-gpu-test.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tests/fixtures/computeinstance-with-many-gpu-test.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tests/test.yml
• osac-aap/collections/ansible_collections/osac/test_overrides/roles/ocp_virt_vm_with_gpu/meta/argument_specs.yaml
• osac-aap/collections/ansible_collections/osac/test_overrides/roles/ocp_virt_vm_with_gpu/meta/osac.yaml
• osac-aap/collections/ansible_collections/osac/test_overrides/roles/ocp_virt_vm_with_gpu/tasks/create.yaml
• osac-aap/collections/ansible_collections/osac/test_overrides/roles/ocp_virt_vm_with_gpu/tasks/delete.yaml
• osac-aap/tests/integration/fixtures/computeinstance-with-gpu-test.yaml
• osac-aap/tests/integration/targets/compute_instance_with_gpu_create/tasks/baseline.yml

💤 Files with no reviewable changes (5)

• osac-aap/collections/ansible_collections/osac/test_overrides/roles/ocp_virt_vm_with_gpu/tasks/delete.yaml
• osac-aap/collections/ansible_collections/osac/test_overrides/roles/ocp_virt_vm_with_gpu/tasks/create.yaml
• osac-aap/collections/ansible_collections/osac/test_overrides/roles/ocp_virt_vm_with_gpu/meta/argument_specs.yaml
• osac-aap/collections/ansible_collections/osac/test_overrides/roles/ocp_virt_vm_with_gpu/meta/osac.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/meta/argument_specs.yaml

---

Walkthrough

The VM role replaces gpu_devices with compute_instance.spec.gpu. It generates host devices from the requested GPU count and validates single-GPU, multi-GPU, and no-GPU configurations.

Changes

GPU specification migration

Layer / File(s)	Summary
GPU host-device configuration osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/meta/argument_specs.yaml, osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tasks/create.yaml, osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tasks/configure_permitted_host_devices.yaml	The role removes the gpu_devices argument. Host-device configuration now uses compute_instance.spec.gpu, including its PCI selector and resource name, for HyperConverged reads, merging, and patches.
GPU template generation osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tasks/create_build_spec.yaml	The template builder creates sequential hostDevices entries for the configured GPU count and assigns the configured resource name.
GPU validation and integration migration osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tests/fixtures/*, osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tests/test.yml, osac-aap/tests/integration/fixtures/computeinstance-with-gpu-test.yaml, osac-aap/tests/integration/targets/compute_instance_with_gpu_create/tasks/baseline.yml, osac-aap/collections/ansible_collections/osac/test_overrides/roles/ocp_virt_vm_with_gpu/*	Tests cover one GPU, three GPUs, and an absent GPU specification. The integration fixture uses the standard VM template and derives expected GPU values from the fixture. The GPU-specific test override is removed.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Mergeability Score: ⚪ Minimal · up to 989e2

The change moves GPU configuration to the ComputeInstance payload while preserving non-GPU behavior; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant ComputeInstance
  participant create_yaml
  participant create_build_spec_yaml
  participant configure_permitted_host_devices_yaml
  participant HyperConverged
  ComputeInstance->>create_yaml: provide spec.gpu
  create_yaml->>create_build_spec_yaml: build VM template
  create_build_spec_yaml->>create_build_spec_yaml: create hostDevices for GPU count
  create_yaml->>configure_permitted_host_devices_yaml: configure permitted devices
  configure_permitted_host_devices_yaml->>HyperConverged: read and patch PCI host-device state

Loading

Possibly related PRs

• osac-project/osac#217: Introduces the ComputeInstanceSpec.Gpu fields consumed by this PR.
• osac-project/osac#230: Modifies GPU PCI selector, resource name, and count handling.
• osac-project/osac#281: Relates to consuming ComputeInstanceSpec.Gpu fields.

Suggested reviewers: ygalblum, fullsend-ai-review

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: updating the ocp_virt_vm role to read GPU configuration from the ComputeInstance custom resource payload.
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	The full merge-base diff adds only GPU selectors, resource names, fixtures, and test logic; scans found no API keys, tokens, passwords, private keys, credential URLs, or long encoded blobs.
No-Weak-Crypto	✅ Passed	The PR diff adds no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret-comparison logic; changes only GPU configuration and tests.
No-Injection-Vectors	✅ Passed	The PR changes only Ansible YAML fixtures and tasks; added-line and current-file scans found no SQL concatenation, shell=True, eval/exec, unsafe YAML load, pickle.loads, os.system, or dangerouslySe...
Container-Privileges	✅ Passed	The PR diff adds no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, or container root settings.
No-Sensitive-Data-In-Logs	✅ Passed	The PR adds no debug or secret logging; new failure messages expose only test GPU host-device data, while existing broad object dumps are unchanged.
Ai-Attribution	✅ Passed	The five PR commits identify Claude Code and include Assisted-by trailers; none uses a Co-Authored-By trailer for an AI tool.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Ended 6:58 PM UTC

Commit: 989e2d9 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:59 PM UTC · Completed 7:14 PM UTC

Commit: 989e2d9 · View workflow run →

[fullsend-ai-review]

Review

Findings

Low

• [naming-convention] osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tasks/configure_permitted_host_devices.yaml:2 — Task name says "entry" (singular) but the variable desired_pci_host_devices is a list and downstream tasks (merge logic, rejectattr) treat it as such. The singular/plural mismatch is inconsistent with the prior naming convention.
  Remediation: Rename to "Build desired pciHostDevices entries from compute_instance.spec.gpu" to match the variable type.

• [test-structure-consistency] osac-aap/collections/ansible_collections/osac/templates/roles/ocp_virt_vm/tests/test.yml — The file header comment block (lines 1–12) documents Tests 1–12 with their invocation variables (e.g., -e test_create_missing_cores_fails=true). The three new tests (13, 14, 15) are not listed in this header, breaking the established pattern.
  Remediation: Add entries for Test 13 (test_create_gpu_single), Test 14 (test_create_gpu_many), and Test 15 (test_create_no_gpu) to the header comment block following the same format as Tests 1–12.

[fullsend-ai-review]

[low] naming-convention

Task name says entry (singular) but the variable desired_pci_host_devices is a list and downstream tasks (merge logic, rejectattr) treat it as such. The singular/plural mismatch is inconsistent with the prior naming convention.

Suggested fix: Rename to Build desired pciHostDevices entries from compute_instance.spec.gpu to match the variable type.