"Inventory is broken" message when processing group_vars files.

[dflock]

If I let ansible-review process my group_vars files, I get messages like this in the output:

WARN: Best practice "Same variable defined in siblings" not met:
ansible/group_vars/test.yml:Inventory is broken:
Attempted to read "/home/duncan/dev/tools/provisioning/ansible/ansible.log" as ini file:
/home/duncan/dev/tools/provisioning/ansible/ansible.log:1:
Expected key=value host variable assignment, got: 16:47:41,708

I'm running ansible-review like this: git ls-files ansible/. | xargs ansible-review.

Two things puzzle me about this:

1. It's complaining about ansible.log - which is .gitignored and thus not in the output from git ls-files ansible/. - so I guess ansible-review isn't using that list as a list of files to process, but reading the filesystem itself? Is there a way to make ansible-review ignore some files?
2. I'm not sure what this has to do with the group_vars, or with the inventory, for that matter? Is it assuming ansible.log is my inventory file?

If I exclude the group_vars folder, this problem goes away: git ls-files ansible/. | grep -v group_vars | xargs ansible-review - which indicates that maybe ansible-review does use the list of files?

[willthames]

You might need to turn on some extra verbose logging by running with -v, this doesn't make much sense.

What version of ansible are you using?

We expect ansible-review to process inventory/group_vars. What is not clear is why it then decides to open ansible.log, as that seems nowhere near ansible. It might be that ansible's inventory processing is finding ansible.log, but as you're not experiencing this issue running Ansible itself, this shouldn't be happening with ansible-review

Can you see if you can boil this down to a test case that you're able to share. At the moment there is nothing I can do about this as I simply don't understand why this is happening, and can't reproduce it to diagnose further.

[dflock]

I tried running with -v and the relevant section of the output looks like this:

INFO: Standard "bare words are deprecated for with_items" met
INFO: Best practice "octal file permissions should start with a leading zero" met
INFO: Best practice "Use connection: local rather than hosts: localhost" met
INFO: Best practice "Use connection: local rather than delegate_to: localhost" met
INFO: Best practice "Don't use tabs in almost anything that isn't a Makefile" met
INFO: Best practice "Use handlers rather than when: changed in tasks" met
INFO: Best practice "become_user should be accompanied by become" met
INFO: Best practice "Don't compare to "" - use `when: var` or `when: not var`" met
INFO: Best practice "Don't compare to True or False - use `when: var` or `when: not var`" met
INFO: Reviewing all of GroupVars (ansible/group_vars/test_group.yml)
INFO: GroupVars ansible/group_vars/test_group.yml declares standards version 0.1
INFO: Best practice "Variable uses should contain whitespace" met
INFO: Best practice "Vars should only occur once per file" met
WARN: Best practice "Same variable defined in siblings" not met:
ansible/group_vars/test_group.yml:Inventory is broken: Attempted to read "/home/duncan/dev/tools/provisioning/ansible/ansible.log" as ini file: /home/duncan/dev/tools/provisioning/ansible/ansible.log:1: Expected key=value host variable assignment, got: 16:47:41,708 

The test_group.yml file is empty, but it does the same thing with ones that aren't.

I'll see if I can come up with a reduced test case.

[dflock]

OK, so here's the reduced test case: https://github.com/dflock/ansible-review-testcase

It looks like this:

➜ tree
.
├── ansible
│   ├── ansible.cfg
│   ├── ansible.log
│   └── group_vars
│       └── test_group.yml
├── ansible-review
│   ├── ansible-review.ini
│   ├── ansible-review-post-process.py
│   ├── ansible-review-rules
│   │   ├── lint-rules
│   │   │   ├── ...
│   │   ├── standards.py
│   │   └── standards.pyc
│   └── requirements.txt
└── ar

Running this: find ./ansible | xargs ansible-review -v (which I'm doing via the ar script, to update the paths) produces this output:

INFO: Using configuration file: /home/duncan/.config/ansible-review/config.ini
WARN: Couldn't classify file ./ansible
WARN: Couldn't classify file ./ansible/ansible.cfg
WARN: Couldn't classify file ./ansible/group_vars
INFO: Reviewing all of GroupVars (./ansible/group_vars/test_group.yml)
INFO: GroupVars ./ansible/group_vars/test_group.yml declares standards version 0.1
INFO: Best practice "Variable uses should contain whitespace" met
INFO: Best practice "Vars should only occur once per file" met
WARN: Best practice "Same variable defined in siblings" not met:
./ansible/group_vars/test_group.yml:Inventory is broken: Attempted to read "/home/duncan/tmp/ansible-review-testcase/ansible/ansible.log" as ini file: /home/duncan/tmp/ansible-review-testcase/ansible/ansible.log:1: Expected key=value host variable assignment, got: test 
INFO: Best practice "Don't use tabs in almost anything that isn't a Makefile" met
WARN: Couldn't classify file ./ansible/ansible.log

The current contents of the ansible.log file are:

test test

if I change it to this:

test

Then the Inventory is broken message goes away!?:

INFO: Using configuration file: /home/duncan/.config/ansible-review/config.ini
WARN: Couldn't classify file ./ansible
WARN: Couldn't classify file ./ansible/ansible.cfg
WARN: Couldn't classify file ./ansible/group_vars
INFO: Reviewing all of GroupVars (./ansible/group_vars/test_group.yml)
INFO: GroupVars ./ansible/group_vars/test_group.yml declares standards version 0.1
INFO: Best practice "Variable uses should contain whitespace" met
INFO: Best practice "Vars should only occur once per file" met
INFO: Best practice "Same variable defined in siblings" met
INFO: Best practice "Don't use tabs in almost anything that isn't a Makefile" met
WARN: Couldn't classify file ./ansible/ansible.log

I'm not actually using a virtualenv for this, but the requirements.txt contains this:

ansible==2.3.0.0
ansible-lint==3.4.13
ansible-review==0.13.0

which are the versions that I have installed.

[willthames]

Looking at this structure, I think it's because it expects group_vars to be in an inventory directory. Your ansible.cfg suggests that the inventory directory is elsewhere, so not quite sure what you're trying to do with the group_vars.

I usually use an inventory directory and then have group_vars underneath. I haven't done much digging into what ansible.inventory.Inventory does in your case, but as I'm not sure what you're trying to achieve, it's difficult to know how best to fix finding group_vars in an unexpected (not necessarily wrong) location

[dflock]

We're just using the Standard Ansible directory layout. I think you're talking about/expecting the Alternate Directory Layout?

Our inventory file, at least for our dev env, is created by vagrant, and put into .vagrant/provisioners/ansible/inventory/vagrant_ansible_inventory - which we reference on the command line when we run ansible, with -i .... This works fine - the inventory and the group/host vars files don't have to be co-located for Ansible to work.

I haven't thought much about our directory layout since I set it up ~3yrs ago; I'll give the alternate layout some thought, because I'd like to split up our gigantic group_vars/all.yml - but I think we still want to share most of our variables across environments?

[willthames]

I'm not really talking about the alternate directory layout

We just use a directory (in our case called inventory) that has a bunch of hosts files and host_vars and group_vars as children. Our ansible.cfg points to that inventory directory.

However, I'm not trying to make you change how you use Ansible, more understand how you use it, I didn't even know group_vars could be under the root directory, even though it's perfectly well documented.

The problem is here: https://github.com/willthames/ansible-review/blob/master/lib/ansiblereview/groupvars.py#L49-L50

It's impossible for that check to work if group_vars isn't within an inventory directory containing the host files (it can't check if two groups are competing if it doesn't know the group structure), so I'd just recommend removing the test_matching_groupvar check in standards.py.

Patches accepted that fix the exception at the end of that test to give a more helpful error message