Skip to content

Commit 5de0766

Browse files
michburkopenshift-merge-bot[bot]
authored andcommitted
[libvirt_manager] Add parent_group support for VM type grouping
Allow VM types to declare a parent_group so that multiple subgroups (e.g. compute1, compute2) are independently addressable while also being reachable via a shared parent inventory group (e.g. computes). Generated-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Michael Burke <michburk@redhat.com>
1 parent 2c18e34 commit 5de0766

9 files changed

Lines changed: 271 additions & 21 deletions

File tree

roles/libvirt_manager/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ cifmw_libvirt_manager_configuration:
100100
networkconfig: (dict or list[dict], [network-config](https://cloudinit.readthedocs.io/en/latest/reference/network-config-format-v2.html#network-config-v2) v2 config, needed if a static ip address should be defined at boot time in absence of a dhcp server in special scenarios. Optional)
101101
devices: (dict, optional, defaults to {}. The keys are the VMs of that type that needs devices to be attached, and the values are lists of strings, where each string must contain a valid <hostdev/> libvirt XML element that will be passed to virsh attach-device)
102102
dhcp_options: (list, optional, defaults to []. List of DHCP options to apply to all VMs of this type. Format: ["option_number,value", ...])
103+
parent_ansible_group: (string, optional. The full Ansible inventory group name to use as a parent for this VM type's group. Multiple VM types sharing the same parent_ansible_group are addressable both individually and via the parent group. The value must be the complete group name including any suffix (e.g. "computes", not "compute"). Example: two types "compute1" and "compute2" with parent_ansible_group "computes" creates groups "compute1s", "compute2s", and "computes" containing both. The parent group should be considered abstract and must not match any VM type's own group name. The role will fail if a collision is detected.)
103104
networks:
104105
net_name: <XML definition of the network to create>
105106
```
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
- name: Cleanup
3+
hosts: instance
4+
tasks:
5+
- name: Remove basedir
6+
become: true
7+
ansible.builtin.file:
8+
path: "/opt/basedir"
9+
state: absent
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
---
2+
- name: Parent group inventory test
3+
hosts: instance
4+
gather_facts: true
5+
vars:
6+
cifmw_basedir: "/opt/basedir"
7+
_cifmw_libvirt_manager_layout:
8+
vms:
9+
compute1:
10+
parent_ansible_group: computes
11+
amount: 1
12+
disk_file_name: "blank"
13+
disksize: 5
14+
memory: 1
15+
cpus: 1
16+
nets: []
17+
compute2:
18+
parent_ansible_group: computes
19+
amount: 1
20+
disk_file_name: "blank"
21+
disksize: 5
22+
memory: 1
23+
cpus: 1
24+
nets: []
25+
controller:
26+
amount: 1
27+
disk_file_name: "blank"
28+
disksize: 5
29+
memory: 1
30+
cpus: 1
31+
nets: []
32+
tasks:
33+
- name: Create reproducer-inventory directory
34+
become: true
35+
ansible.builtin.file:
36+
path: "{{ cifmw_basedir }}/reproducer-inventory"
37+
state: directory
38+
owner: "{{ ansible_user_id }}"
39+
group: "{{ ansible_user_id }}"
40+
mode: "0755"
41+
42+
- name: Set all_vms fact
43+
ansible.builtin.set_fact:
44+
cifmw_libvirt_manager_all_vms:
45+
compute1-0: compute1
46+
compute2-0: compute2
47+
controller-0: controller
48+
49+
- name: Add VMs to inventory
50+
vars:
51+
_full_host_name: "{{ item.key }}"
52+
_group: "{{ item.value }}"
53+
_parent_group: >-
54+
{{
55+
_cifmw_libvirt_manager_layout.vms[item.value].parent_ansible_group |
56+
default('')
57+
}}
58+
_ssh_user: "zuul"
59+
_add_ansible_host: false
60+
_ansible_host: ""
61+
_ini_line: "{{ item.key }} ansible_user=zuul vm_type={{ item.value }}"
62+
ansible.builtin.include_role:
63+
name: libvirt_manager
64+
tasks_from: add_vm_to_inventory.yml
65+
loop: "{{ cifmw_libvirt_manager_all_vms | dict2items }}"
66+
67+
- name: Create all group inventory
68+
ansible.builtin.include_role:
69+
name: libvirt_manager
70+
tasks_from: create_all_group_inventory.yml
71+
72+
# --- Assertions ---
73+
74+
- name: Assert runtime inventory has child and parent groups
75+
ansible.builtin.assert:
76+
that:
77+
- "'compute1-0' in groups['compute1s']"
78+
- "'compute2-0' in groups['compute2s']"
79+
- "'compute1-0' in groups['computes']"
80+
- "'compute2-0' in groups['computes']"
81+
- "'controller-0' in groups['controllers']"
82+
- "'controller-0' not in groups.get('computes', [])"
83+
84+
- name: Slurp all-group.yml
85+
ansible.builtin.slurp:
86+
path: "{{ cifmw_basedir }}/reproducer-inventory/all-group.yml"
87+
register: _all_group_content
88+
89+
- name: Assert all-group.yml has parent and child groups
90+
vars:
91+
_ag: "{{ _all_group_content.content | b64decode | from_yaml }}"
92+
ansible.builtin.assert:
93+
that:
94+
- "'compute1s' in _ag.all.children"
95+
- "'compute2s' in _ag.all.children"
96+
- "'controllers' in _ag.all.children"
97+
- "'computes' in _ag.all.children"
98+
- "'compute1s' in _ag.all.children.computes.children"
99+
- "'compute2s' in _ag.all.children.computes.children"
100+
- "'controllers' not in _ag.all.children.computes.children"
101+
102+
- name: Parent group collision validation test
103+
hosts: instance
104+
gather_facts: false
105+
vars:
106+
_cifmw_libvirt_manager_layout:
107+
vms:
108+
compute:
109+
parent_ansible_group: computes
110+
amount: 1
111+
disk_file_name: "blank"
112+
disksize: 5
113+
memory: 1
114+
cpus: 1
115+
nets: []
116+
computes:
117+
parent_ansible_group: bigcomputes
118+
amount: 1
119+
disk_file_name: "blank"
120+
disksize: 5
121+
memory: 1
122+
cpus: 1
123+
nets: []
124+
tasks:
125+
- name: Verify collision detection rejects overlapping names
126+
block:
127+
- name: Run deploy_layout validation with colliding layout
128+
ansible.builtin.include_role:
129+
name: libvirt_manager
130+
tasks_from: deploy_layout.yml
131+
132+
- name: Fail if validation did not catch collision
133+
ansible.builtin.fail:
134+
msg: >-
135+
deploy_layout.yml should have failed due to
136+
'computes' colliding with compute's parent_ansible_group
137+
138+
rescue:
139+
- name: Assert failure message mentions collision
140+
ansible.builtin.assert:
141+
that:
142+
- >-
143+
ansible_failed_result.msg is defined and
144+
'Collisions' in ansible_failed_result.msg
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
log: true
3+
4+
provisioner:
5+
name: ansible
6+
log: true
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
- name: Prepare
3+
hosts: instance
4+
tasks:
5+
- name: Create basedir
6+
become: true
7+
ansible.builtin.file:
8+
path: "/opt/basedir"
9+
state: directory
10+
owner: "{{ ansible_user_id }}"
11+
group: "{{ ansible_user_id }}"
12+
mode: "0755"
Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,51 @@
11
---
2-
- name: Add host to runtime inventory
3-
ansible.builtin.add_host:
4-
name: "{{ _full_host_name }}"
5-
groups: "{{ _group }}s"
6-
ansible_ssh_user: "{{ _ssh_user }}"
7-
ansible_host: "{{ _add_ansible_host | ternary(_ansible_host, omit) }}"
8-
vm_type: "{{ _group }}"
2+
- name: Build group list for host
3+
vars:
4+
_has_parent: "{{ _parent_group | default('') | length > 0 }}"
5+
_groups_list: >-
6+
{{
7+
[_group ~ 's'] +
8+
(_has_parent | ternary([_parent_group], []))
9+
}}
10+
block:
11+
- name: Add host to runtime inventory
12+
ansible.builtin.add_host:
13+
name: "{{ _full_host_name }}"
14+
groups: "{{ _groups_list }}"
15+
ansible_ssh_user: "{{ _ssh_user }}"
16+
ansible_host: "{{ _add_ansible_host | ternary(_ansible_host, omit) }}"
17+
vm_type: "{{ _group }}"
918

10-
- name: Ensure group section exists
11-
ansible.builtin.lineinfile:
12-
path: "{{ cifmw_libvirt_manager_tmp_inv_file }}"
13-
create: true
14-
line: "[{{ _group }}s]"
15-
regexp: "^\\[{{ _group }}s\\]$"
16-
state: present
17-
mode: "0644"
19+
- name: Ensure group section exists in INI inventory
20+
ansible.builtin.lineinfile:
21+
path: "{{ cifmw_libvirt_manager_tmp_inv_file }}"
22+
create: true
23+
line: "[{{ _group }}s]"
24+
regexp: "^\\[{{ _group }}s\\]$"
25+
state: present
26+
mode: "0644"
1827

19-
- name: Append host under proper group
20-
ansible.builtin.lineinfile:
21-
path: "{{ cifmw_libvirt_manager_tmp_inv_file }}"
22-
insertafter: "^\\[{{ _group }}s\\]$"
23-
line: "{{ _ini_line }}"
24-
regexp: "^{{ _full_host_name | regex_escape() }} "
28+
- name: Append host under its group in INI inventory
29+
ansible.builtin.lineinfile:
30+
path: "{{ cifmw_libvirt_manager_tmp_inv_file }}"
31+
insertafter: "^\\[{{ _group }}s\\]$"
32+
line: "{{ _ini_line }}"
33+
regexp: "^{{ _full_host_name | regex_escape() }} "
34+
35+
- name: Ensure parent group children section exists in INI inventory
36+
when: _has_parent
37+
ansible.builtin.lineinfile:
38+
path: "{{ cifmw_libvirt_manager_tmp_inv_file }}"
39+
create: true
40+
line: "[{{ _parent_group }}:children]"
41+
regexp: "^\\[{{ _parent_group }}:children\\]$"
42+
state: present
43+
mode: "0644"
44+
45+
- name: Add child group to parent in INI inventory
46+
when: _has_parent
47+
ansible.builtin.lineinfile:
48+
path: "{{ cifmw_libvirt_manager_tmp_inv_file }}"
49+
insertafter: "^\\[{{ _parent_group }}:children\\]$"
50+
line: "{{ _group }}s"
51+
regexp: "^{{ _group }}s$"

roles/libvirt_manager/tasks/deploy_layout.yml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,34 @@
44
- _cifmw_libvirt_manager_layout is not defined
55
ansible.builtin.include_tasks: generate_layout.yml
66

7+
- name: Validate no VM group name collides with a parent_ansible_group
8+
vars:
9+
_vm_group_names: >-
10+
{% set _names = [] -%}
11+
{% for _k in _cifmw_libvirt_manager_layout.vms.keys() -%}
12+
{% set _ = _names.append(((_k == 'crc') | ternary('ocp', _k)) ~ 's') -%}
13+
{% endfor -%}
14+
{{ _names }}
15+
_reserved_groups:
16+
- hypervisors
17+
- localhosts
18+
_parent_groups: >-
19+
{{
20+
_cifmw_libvirt_manager_layout.vms.values() |
21+
selectattr('parent_ansible_group', 'defined') |
22+
map(attribute='parent_ansible_group') |
23+
unique | list
24+
}}
25+
_disallowed_groups: "{{ _vm_group_names + _reserved_groups }}"
26+
_collisions: "{{ _disallowed_groups | intersect(_parent_groups) }}"
27+
when: _collisions | length > 0
28+
ansible.builtin.fail:
29+
msg: >-
30+
A VM type's own inventory group cannot match a parent_ansible_group value.
31+
Collisions: {{ _collisions | join(', ') }}.
32+
Set parent_ansible_group to an abstract name not used by any VM type
33+
(e.g. computes, not compute1s).
34+
735
# TODO(hjensas): We should have a dedicated role for firewall
836
- name: Enable forwarding in the libvirt zone
937
when: cifmw_libvirt_manager_firewalld_zone_libvirt_forward | default(true) | bool

roles/libvirt_manager/tasks/generate_networking_data.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,11 @@
112112
default(_cifmw_libvirt_manager_layout.vms[_vm_type].user) |
113113
default('zuul')
114114
}}
115+
_parent_group: >-
116+
{{
117+
_cifmw_libvirt_manager_layout.vms[_vm_type].parent_ansible_group |
118+
default('')
119+
}}
115120
_add_ansible_host: >-
116121
{{
117122
_cifmw_libvirt_manager_layout.vms[_vm_type].manage | default(true) and

roles/libvirt_manager/templates/all-inventory.yml.j2

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
11
all:
22
children:
3+
{% set _parent_map = {} -%}
4+
{% for vm in _cifmw_libvirt_manager_layout.vms.keys() if
5+
(_cifmw_libvirt_manager_layout.vms[vm].manage | default(true) and
6+
_cifmw_libvirt_manager_layout.vms[vm].amount | default(1) | int > 0 and
7+
_cifmw_libvirt_manager_layout.vms[vm].parent_ansible_group is defined) -%}
8+
{% set _pg = _cifmw_libvirt_manager_layout.vms[vm].parent_ansible_group -%}
9+
{% if _pg not in _parent_map -%}
10+
{% set _ = _parent_map.update({_pg: []}) -%}
11+
{% endif -%}
12+
{% set _ = _parent_map[_pg].append(vm) -%}
13+
{% endfor -%}
314
{% for vm in _cifmw_libvirt_manager_layout.vms.keys() if
415
(_cifmw_libvirt_manager_layout.vms[vm].manage | default(true) and
516
_cifmw_libvirt_manager_layout.vms[vm].amount | default(1) | int > 0) %}
@@ -20,6 +31,13 @@ virtual machines.
2031
ansible_ssh_common_args: "-o StrictHostKeyChecking=no -J {{ ansible_user | default(ansible_user_id) }}@{{ _hostname }}"
2132
{% endif %}
2233
{% endfor %}
34+
{% for pg, children in _parent_map.items() %}
35+
{{ pg }}:
36+
children:
37+
{% for vm in children %}
38+
{{ (vm == 'crc') | ternary('ocp', vm) }}s: {}
39+
{% endfor %}
40+
{% endfor %}
2341
hypervisors:
2442
hosts:
2543
{% set _hypervisors = (

0 commit comments

Comments
 (0)