Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vmware_vm_inventory - fixed hostgroups being created automatically when the with_path option is enabled #2248

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

dmnagornyi
Copy link

@dmnagornyi dmnagornyi commented Nov 14, 2024

SUMMARY

Fixes #1688
Plugin should not create hostgroups automatically when with_path is true. There is keyed_groups parameter for that.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

community.vmware.vmware_vm_inventory

ADDITIONAL INFORMATION
I just removed a block of code that performed an undocumented function and generated a lot of unnecessary hostgroups.

Removed the piece that automatically created the host groups from the VM path.
Removed unnecessary ansible.module_utils.six module.
It makes sense to look for rooted 'path' value in variable instead of group_names
Updated Inventory with rooted 'with_path' option test.
@dmnagornyi
Copy link
Author

@mariolenz please take a look when you have a chance. Minimal changes, shouldn't interfere with anything.

@mariolenz
Copy link
Collaborator

recheck

@dmnagornyi
Copy link
Author

recheck

Anything else I need to do?

@mariolenz
Copy link
Collaborator

I'm sorry, I didn't find much time to work on this collection during the last weeks. I hope I'm able to review this PR soon.

@mariolenz
Copy link
Collaborator

Thanks very much for your contribution, but I'm not sure if we merge this because

  1. even if the documentation says otherwise, the plugin has worked like this for ages and your change might break things for users who rely on it working as it does
  2. I've never really understood those integration plugins :-/
  3. there's now a vms inventory plugin in vmware.vmware 1.9.0
  4. because of 2 and 3 I'm about to deprecate vmware_vm_inventory, anyway: [WIP] Deprecate inventory plugins #2283

If this isn't very important for you, I suggest to close this PR and not merge it. If you really want to have this, I'll try to understand more but this might take some time (see 2 above).

@mariolenz mariolenz added the needs_info This issue requires further information. Please answer any outstanding questions label Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs_info This issue requires further information. Please answer any outstanding questions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

community.vmware.vmware_vm_inventory creates groups for each folder in the hosts path
2 participants