Potential bug/solution with Inventory.groups_list()

Hello list!

I think I have found a bug which causes the magic “groups” variable to contain duplicate host names (same name multiple times in a single group), when using an inventory directory (-i dirname).

I read the code a bit, and it appears the magic “groups” variable contains the output of Inventory.groups_list(). So made a simple test case using the existing inventory dir test fixture to show Inventory.groups_list() returning duplicates. Here is my commit (on top of HEAD as of an hour ago):

https://github.com/taylorbarstow/ansible/commit/ffbae34159eff3cd2e7527ab372a35294a9ed549

And here is the relevant snippet of my (failing) test output:

INVENTORY groups_list[‘all’]=[‘morpheus’, ‘morpheus’, ‘morpheus’, ‘thor’, ‘thor’, ‘thor’, ‘zeus’, ‘zeus’, ‘zeus’]
EXPECTED groups_list[‘all’]=[‘morpheus’, ‘thor’, ‘zeus’]
INVENTORY groups_list[‘major-god’]=[‘thor’, ‘thor’, ‘zeus’, ‘zeus’]
EXPECTED groups_list[‘major-god’]=[‘thor’, ‘zeus’]
INVENTORY groups_list[‘norse’]=[‘thor’, ‘thor’]
EXPECTED groups_list[‘norse’]=[‘thor’]
INVENTORY groups_list[‘minor-god’]=[‘morpheus’, ‘morpheus’]
EXPECTED groups_list[‘minor-god’]=[‘morpheus’]
INVENTORY groups_list[‘ungrouped’]=[‘morpheus’, ‘thor’, ‘zeus’]
EXPECTED groups_list[‘ungrouped’]=
INVENTORY groups_list[‘greek’]=[‘morpheus’, ‘morpheus’, ‘zeus’, ‘zeus’]

EXPECTED groups_list[‘greek’]=[‘morpheus’, ‘zeus’]

I am not 100% sure that I have understood ‘ungrouped’ correctly (should it actually be empty?) – but that is besides the point, I think you will agree that the groups_list output above does not look right with all those duplicate hostnames.

I’m new to ansible. Would anyone be willing to give me a hand validating this issue? Do you agree that it is a problem? Can you reproduce the failure of that test case / output in your environment?

Finally, I have some ideas for a potential resolution, but how long can this email really get? If people agree this is a bug, I’ll open an issue / PR in github.

Thank you in advance! Ansible is awesome!
Taylor

Might be worth checking with Serge whether his PR fixes this or not (https://github.com/ansible/ansible/pull/6734) as he’s doing a lot of work in that area of code (and if it’s not, he might be able to shed some light on this)

Will

Hi Taylor,

“I still need to actually add some additional unit tests to reflect the modifications I made. I was going to address that after the PR would get merged, but maybe better to not wait.”

Please just go ahead and update the PR. Thanks!

Hi all,

Thanks for the responses! FYI, I continued the discussion with Serge on his PR, including a few more test cases:

https://github.com/ansible/ansible/pull/6379

Taylor