-
Notifications
You must be signed in to change notification settings - Fork 467
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
parse inventory using Ansible Python API #215
base: master
Are you sure you want to change the base?
Conversation
Apologies for the extremely long delay in responding. I was dealing with some personal issues. Is PR #214 still relevant given this PR? |
I'd very much like to get rid of the builtin parser. It's been nothing but a headache, but at the time ansible itself wasn't usable as a library. My main concern is that people would have to have ansible itself also installed. Then again, why wouldn't you? I haven't read and reviewed this PR yet. I hope to be able to do so soon and merge it. It'll require some extensive testing. |
Testing this PR, it seems that it is not compatible with vaulted files. I'm running the following, where I'm poiting
Notice that I exported the ansible vault password in |
I added support for ansible-vault in this PR. quale1#1 Warning : Rendering this with host details will render all the vaulted values in clear text into the rendered file. This can make secrets leak. |
I encountered a few issues with this pull request:
At master...dirks:ansible-cmdb:vault I rebased the work from @quale1 and @gaelgatelement on current master (3f3e412) and addressed 1, 2, 3 and partly 5.i (if you use the I do not know what causes 4. |
Some changes to optionally use the Ansible Python API to parse the inventory files. This could replace the builtin parser which would lead to some simplification, but I took the more conservative approach of leaving the builtin parse intact and adding a
--use-ansible-api
flag to select this as an alternative parser.There are a few things that the builtin ansible-cmdb parser doesn't do the same as Ansible does, especially in parsing :var sections and in the order of reading inventory files. (When multiple inventory files are read from a directory including the group_vars and host_vars cases, the files should be read in sorted order. Then there's
ansible_group_priority
, which I don't use and wouldn't want to implement.)An earlier experiment with running the
ansible-inventory
command to parse the inventory revealed that the mixed static and dynamic inventory test has a bug that makes ansible-cmdb parse it differently than ansible does. Ansible requires that every host be in some group, andtest/f_inventory/mixeddir/dyninv.py
generates json that has a couple hosts that aren't in any group. It looks like Ansible uses a dummy "ungrouped" group in that case, so that's how I fixed the test data. The test of the builtin ansible-cmdb inventory parser passes without modification and this lets parsers based on Ansible code or commands pass as well. (The ansible-inventory experiment was successful, but it seems better to use the Ansible Python API directly rather than spawning another program and parsing its output.)It isn't easy to import the Ansible libraries without renaming
src/ansiblecmdb/ansible.py
because a file in the directory namedansible.py
will confound the import. I renamed the file toansible_cmdb.py
so thatfrom ansible.parsing.dataloader import DataLoader
, etc. work as expected. I also made minimal changes to ansible_cmdb.py (formerly ansible.py) to make it a more useful base class by moving some code out of__init__
to expose key methods to override, specificallyload_inventories
.The tests are duplicated because I was too lazy to use dependency injection to specify whether to use
Ansible
orAnsibleViaInventory
.A random nit: The docstring for the
get_hosts
method says "Return a list of parsed hosts info, with the limit applied if required", but the method returns a dict of hosts info rather than a list.