[libvirt_manager] Add parent_group key for libvirt layouts

[michburk]

Allow VM types to declare a parent_group key. Hosts are added to the parent group at add_host time so that groups like 'computes' are populated before dependent roles check group membership. A parent group inventory file is also written to reproducer-inventory/ for subsequent playbook runs.

[openshift-ci]

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]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign fultonj 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:

• OWNERS

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

[nemarjan]

LGTM in general, just some questions.

[nemarjan]

(non-blocking): Since selectattr('value.parent_group', 'defined') only checks key existence, would an explicitly empty value like parent_group: "" slip through here? If so, it would produce a file named -group.yml and a nameless group?

[michburk]

That'd be an interesting situation, I'll test it and probably end up adding an explicit check for an empty string and fail in that case. Thanks!

[michburk]

Tested a setup with parent_group: '' and this did indeed create a -group.yml file, added an explicit check: rejectattr('value.parent_group', 'equalto', '') to not create this file. Other parts of this patch treat an empty value as falsey and don't do anything if '' is supplied as a parent group, so rejecting here treats it as if there were no parent group field at all.

If you think parent_group: '' should produce an error early on instead, I wouldn't mind refactoring to handle it that way.

[michburk]

Also added a check to the Craft patch for cifmw_networking_definition task to skip generating group-template entries for child VM types when their parent_group matches an existing VM type in the libvirt layout.

My testing used two child VM types (compute94 and compute96) both setting parent_group: computes, with no compute group defined in the libvirt layout. This worked fine and should be unaffected by this change (re-testing now).

Nemanja's comment prompted me to test a layout with both a regular compute type and a compute96 child with parent_group: computes. This failed because the old logic generated entries for both computes and compute96s with the same networks. Since parent_group adds compute96 nodes to the computes Ansible group, the networking mapper saw those nodes in two groups that both defined the same network and rejected the duplicate.