From 21b16c1c77f732e2c763138ffe03ac80a3897214 Mon Sep 17 00:00:00 2001 From: lyrandy <42095565+lyrandy@users.noreply.github.com> Date: Sun, 14 Jul 2024 06:06:37 -0400 Subject: [PATCH] Update VirtualBox Group parsing to align with documentation. (#8510) * Update VirtualBox Group parsing to align with documentation. Previously, we could separate the group string on the `/` char and consider each element to be distinct, top-level groups. This change implements the notion of nested groups and the use of the `,` char to split multiple groups. * Address code review comments. Changed the implementation from a breaking change to a minor change by introducing a new parameter to configure the behaviour. Keep the default values to maintain the existing behaviour, and allow consumers an option to opt-in. * Fix line length. The long lines were tripping CI. Reduce the length. * Apply suggestions from code review Update documentation to match expected conventions and correct the final rendered formatting. Set the initial parent_group to `None` instead of `all` and rely on the parent class' inventory reconciliation logic to ensure consistent behaviour across different inventory plugins. Co-authored-by: Felix Fontein * Reword module arg description to avoid issues with CI. One of the lines ended with a colon character which made the CI tests fail since it would interpret it as a YAML key. Reworded the description altogether to avoid that issue. * Apply suggestions from code review Co-authored-by: Felix Fontein --------- Co-authored-by: Felix Fontein --- .../fragments/8508-virtualbox-inventory.yml | 3 + plugins/inventory/virtualbox.py | 85 +++++++++++++++++-- 2 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/8508-virtualbox-inventory.yml diff --git a/changelogs/fragments/8508-virtualbox-inventory.yml b/changelogs/fragments/8508-virtualbox-inventory.yml new file mode 100644 index 0000000000..dd14818331 --- /dev/null +++ b/changelogs/fragments/8508-virtualbox-inventory.yml @@ -0,0 +1,3 @@ +minor_changes: + - >- + virtualbox inventory plugin - expose a new parameter ``enable_advanced_group_parsing`` to change how the VirtualBox dynamic inventory parses VM groups (https://github.com/ansible-collections/community.general/issues/8508, https://github.com/ansible-collections/community.general/pull/8510). \ No newline at end of file diff --git a/plugins/inventory/virtualbox.py b/plugins/inventory/virtualbox.py index 79b04ec722..425ed91642 100644 --- a/plugins/inventory/virtualbox.py +++ b/plugins/inventory/virtualbox.py @@ -14,6 +14,8 @@ DOCUMENTATION = ''' - Get inventory hosts from the local virtualbox installation. - Uses a YAML configuration file that ends with virtualbox.(yml|yaml) or vbox.(yml|yaml). - The inventory_hostname is always the 'Name' of the virtualbox instance. + - Groups can be assigned to the VMs using C(VBoxManage). Multiple groups can be assigned by using V(/) as a delimeter. + - A separate parameter, O(enable_advanced_group_parsing) is exposed to change grouping behaviour. See the parameter documentation for details. extends_documentation_fragment: - constructed - inventory_cache @@ -35,6 +37,19 @@ DOCUMENTATION = ''' description: create vars from virtualbox properties type: dictionary default: {} + enable_advanced_group_parsing: + description: + - The default group parsing rule (when this setting is set to V(false)) is to split the VirtualBox VM's group based on the V(/) character and + assign the resulting list elements as an Ansible Group. + - Setting O(enable_advanced_group_parsing=true) changes this behaviour to match VirtualBox's interpretation of groups according to + U(https://www.virtualbox.org/manual/UserManual.html#gui-vmgroups). + Groups are now split using the V(,) character, and the V(/) character indicates nested groups. + - When enabled, a VM that's been configured using V(VBoxManage modifyvm "vm01" --groups "/TestGroup/TestGroup2,/TestGroup3") will result in + the group C(TestGroup2) being a child group of C(TestGroup); and + the VM being a part of C(TestGroup2) and C(TestGroup3). + default: false + type: bool + version_added: 9.2.0 ''' EXAMPLES = ''' @@ -177,14 +192,10 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable): # found groups elif k == 'Groups': - for group in v.split('/'): - if group: - group = make_unsafe(group) - group = self.inventory.add_group(group) - self.inventory.add_child(group, current_host) - if group not in cacheable_results: - cacheable_results[group] = {'hosts': []} - cacheable_results[group]['hosts'].append(current_host) + if self.get_option('enable_advanced_group_parsing'): + self._handle_vboxmanage_group_string(v, current_host, cacheable_results) + else: + self._handle_group_string(v, current_host, cacheable_results) continue else: @@ -227,6 +238,64 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable): return all(find_host(host, inventory)) + def _handle_group_string(self, vboxmanage_group, current_host, cacheable_results): + '''Handles parsing the VM's Group assignment from VBoxManage according to this inventory's initial implementation.''' + # The original implementation of this inventory plugin treated `/` as + # a delimeter to split and use as Ansible Groups. + for group in vboxmanage_group.split('/'): + if group: + group = make_unsafe(group) + group = self.inventory.add_group(group) + self.inventory.add_child(group, current_host) + if group not in cacheable_results: + cacheable_results[group] = {'hosts': []} + cacheable_results[group]['hosts'].append(current_host) + + def _handle_vboxmanage_group_string(self, vboxmanage_group, current_host, cacheable_results): + '''Handles parsing the VM's Group assignment from VBoxManage according to VirtualBox documentation.''' + # Per the VirtualBox documentation, a VM can be part of many groups, + # and it's possible to have nested groups. + # Many groups are separated by commas ",", and nested groups use + # slash "/". + # https://www.virtualbox.org/manual/UserManual.html#gui-vmgroups + # Multi groups: VBoxManage modifyvm "vm01" --groups "/TestGroup,/TestGroup2" + # Nested groups: VBoxManage modifyvm "vm01" --groups "/TestGroup/TestGroup2" + + for group in vboxmanage_group.split(','): + if not group: + # We could get an empty element due how to split works, and + # possible assignments from VirtualBox. e.g. ,/Group1 + continue + + if group == "/": + # This is the "root" group. We get here if the VM was not + # assigned to a particular group. Consider the host to be + # unassigned to a group. + continue + + parent_group = None + for subgroup in group.split('/'): + if not subgroup: + # Similarly to above, we could get an empty element. + # e.g //Group1 + continue + + if subgroup == '/': + # "root" group. + # Consider the host to be unassigned + continue + + subgroup = make_unsafe(subgroup) + subgroup = self.inventory.add_group(subgroup) + if parent_group is not None: + self.inventory.add_child(parent_group, subgroup) + self.inventory.add_child(subgroup, current_host) + if subgroup not in cacheable_results: + cacheable_results[subgroup] = {'hosts': []} + cacheable_results[subgroup]['hosts'].append(current_host) + + parent_group = subgroup + def verify_file(self, path): valid = False