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

hosts defined in two groups in different inventory files each is used multiple times #5749

Closed
g-k-r opened this issue Jan 24, 2014 · 19 comments
Labels
bug This issue/PR relates to a bug.

Comments

@g-k-r
Copy link
Contributor

g-k-r commented Jan 24, 2014

when using multiple inventory files defining different groups for the same hosts, ansible adds the hosts multiple times to the inventory list.

version: 1.4.4+dfsg-1 (from debian jessie)
same problem with current devel 00b3f62

reproduce:

actual output:

$ ansible all --list
    www.google.com
    www.yahoo.com
    www.yahoo.com
$ ansible group2 --list
    www.yahoo.com
$ ansible type2 --list
    www.yahoo.com
    www.yahoo.com

note when the group is defined in both files like [type1] this does not happen.

$ ansible type1 --list
    www.google.com

expected output: www.yahoo.com shows up once only i.e.

$ ansible all --list
    www.google.com
    www.yahoo.com
$ ansible group2 --list
    www.yahoo.com
$ ansible type2 --list
    www.yahoo.com

additional information:

to verify that tasks would actually be executed multiple times use

$ ansible all -m debug -a '"msg=TEST {{inventory_hostname}}"'
www.google.com | success >> {
    "msg": "TEST www.google.com"
}

www.yahoo.com | success >> {
    "msg": "TEST www.yahoo.com"
}

www.yahoo.com | success >> {
    "msg": "TEST www.yahoo.com"
}
@sivel
Copy link
Member

sivel commented Jan 24, 2014

Per contributing.md can you please provide your ansible version? (ansible --version)

If I remember correctly, this has been resolved in devel (1.5)

@jctanner
Copy link
Contributor

Yes, this was resolved by 559e890

Please rebase and try again if you are running an older version.

@g-k-r
Copy link
Contributor Author

g-k-r commented Jan 24, 2014

@sivel: amended the report

@jctanner
Copy link
Contributor

@g-k-r this is resolved in devel, and will be in the 1.5 release.

@g-k-r
Copy link
Contributor Author

g-k-r commented Jan 24, 2014

@jctanner: tested with devel, not solved, will provide unit test in PR based on devel

g-k-r pushed a commit to g-k-r/ansible that referenced this issue Jan 24, 2014
tests issue ansible#5749
 same host defined in different groups which in turn are defined
 in different ini files in an inventory directory
@mpdehaan
Copy link
Contributor

Reopening for testing.

@jctanner
Copy link
Contributor

@g-k-r unable to reproduce ...

jtanner@u1204:~/issues/5631-duplicate-task-runs$ ansible all -i inventory --list
    www.google.com
    www.yahoo.com
jtanner@u1204:~/issues/5631-duplicate-task-runs$ ansible group2 -i inventory --list
    www.yahoo.com
jtanner@u1204:~/issues/5631-duplicate-task-runs$ ansible type2 -i inventory --list
    www.yahoo.com

@jctanner
Copy link
Contributor

@g-k-r how do you install ansible and how are you using the checkout to verify devel's behavior?

@g-k-r
Copy link
Contributor Author

g-k-r commented Jan 31, 2014

I do the following:

git clone https://github.com/g-k-r/ansible.git
. ./hacking/env-setup
ansible --version
make tests

or instead of make tests

PYTHONPATH=./lib ANSIBLE_LIBRARY=./library  nosetests -d -v test/TestInventory.py:TestInventory.test_dir_inventory_multiple_groups

what is strange, is that testing with the original reproduction tests from the first post on this issue do work as you say, while the test started by nosetests doesn't.

PYTHONPATH=./lib ANSIBLE_LIBRARY=./library  nosetests -d -v test/TestInventory.py:TestInventory.test_dir_inventory_multiple_groups
test_dir_inventory_multiple_groups (TestInventory.TestInventory) ... FAIL

======================================================================
FAIL: test_dir_inventory_multiple_groups (TestInventory.TestInventory)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/gkr/workspaces/ansible_issue_5749/ansible/test/TestInventory.py", line 423, in test_dir_inventory_multiple_groups
    assert actual_host_names == ['zeus','morpheus']
AssertionError: 
    <ansible.inventory.group.Group object at 0x208e2c0> = <ansible.inventory.Inventory object at 0x204c510>.get_group('major-god')
    ['zeus', 'morpheus', 'zeus', 'morpheus'] = [<ansible.inventory.host.Host object at 0x208d050>.name for <ansible.inventory.host.Host object at 0x208d050> in <ansible.inventory.group.Group object at 0x208e120>.get_hosts()];
    print "%s : %s " % (<ansible.inventory.group.Group object at 0x208e120>.name, ['zeus', 'morpheus', 'zeus', 'morpheus'])
>>  assert ['zeus', 'morpheus', 'zeus', 'morpheus'] == ['zeus','morpheus'] 

-------------------- >> begin captured stdout << ---------------------
greek : ['zeus', 'morpheus', 'zeus', 'morpheus'] 

--------------------- >> end captured stdout << ----------------------

----------------------------------------------------------------------
Ran 1 test in 0.009s

@g-k-r
Copy link
Contributor Author

g-k-r commented Jan 31, 2014

that is confusing, calling ansible directly succeeds:

cd tests
ansible greek --list -i inventory_dir

what doesn't work anymore is:

$ ansible all -i inventory_dir/ --list-hosts --limit greek
No hosts matched

with ansible 1.4.4 this results in:

$ ansible all -i inventory_dir/ --list -l greek
    zeus
    morpheus
    zeus
    morpheus

g-k-r added a commit to g-k-r/ansible that referenced this issue Jan 31, 2014
tests issue ansible#5749
 same host defined in different groups which in turn are defined
 in different ini files in an inventory directory
g-k-r added a commit to g-k-r/ansible that referenced this issue Jan 31, 2014
@g-k-r
Copy link
Contributor Author

g-k-r commented Jan 31, 2014

sorry about the many commits, i changed the test which now works identical to direct ansible calls, will now create a pull request, if you want to add the test to the code.
otherwise i think this issue is fixed already and I made a mistake during verification.
Maybe you'd like to look into why get_group contains the hosts multiple times and why the --limit doesn't seem to work anymore, I could prepare a test again if needed.

thanks!

@jctanner
Copy link
Contributor

@g-k-r i think you are mixing up installed ansible lib paths with the checkout lib paths. Can you please retest on a clean machine with a fresh checkout and no ansible packages installed?

@g-k-r
Copy link
Contributor Author

g-k-r commented Jan 31, 2014

ok i will test in a cleanrooom VM, hopefully tonight otherwise this needs to wait until the week after next

@g-k-r
Copy link
Contributor Author

g-k-r commented Jan 31, 2014

hi @jctanner i tested in a clean debian wheezy
only installed ansible dependencies, but not ansible itself as it is not available in pure wheezy.

to test the initial implementation with get_group i had to checkout the second commit in my pull request. since the latest commit overwrites the test implementation to use get_hosts instead of get_group.

$ git clone https://github.com/g-k-r/ansible.git
...
$ git checkout --track origin/test_hosts_defined_in_multiple_groups_in_different_files
...
$ git checkout 3fe1341990babde2b3dd5295bdd3072aac88fec2
...
$ make tests
...
======================================================================
FAIL: test_dir_inventory_multiple_groups (TestInventory.TestInventory)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/g-k-r/issue_5749/ansible/test/TestInventory.py", line 423, in test_dir_inventory_multiple_groups
    assert actual_host_names == ['zeus','morpheus']
AssertionError: 
    <ansible.inventory.group.Group object at 0x28d5050> = <ansible.inventory.Inventory object at 0x2736510>.get_group('major-god')
    ['zeus', 'morpheus', 'zeus', 'morpheus'] = [<ansible.inventory.host.Host object at 0x28d2c68>.name for <ansible.inventory.host.Host object at 0x28d2c68> in <ansible.inventory.group.Group object at 0x2848e88>.get_hosts()];
    print "%s : %s " % (<ansible.inventory.group.Group object at 0x2848e88>.name, ['zeus', 'morpheus', 'zeus', 'morpheus'])
>>  assert ['zeus', 'morpheus', 'zeus', 'morpheus'] == ['zeus','morpheus'] 

-------------------- >> begin captured stdout << ---------------------
greek : ['zeus', 'morpheus', 'zeus', 'morpheus'] 

--------------------- >> end captured stdout << ----------------------

inventory.get_group is not behaving like i though it should. anyway the get_hosts version (HEAD of the pull request) works.

@risaacson risaacson self-assigned this Mar 4, 2014
@risaacson
Copy link
Contributor

It looks like there isn't any outstanding issues with this ticket so I am going to close it out based on your final comment here. I don't see the duplication that you are pointing out in the recent branches.

If you are continuing to see problems feel free to ping me @ansible.com and we can sort it out.

@risaacson risaacson reopened this Mar 4, 2014
@mpdehaan
Copy link
Contributor

mpdehaan commented Mar 4, 2014

This sounds like this comment was about a pull request that wasn't applied yet so I'm reopening this one for now, unless we can't reproduce this on head, etc.

@mpdehaan
Copy link
Contributor

mpdehaan commented Mar 4, 2014

ok beat me to it, thanks!

risaacson pushed a commit to risaacson/ansible that referenced this issue Mar 5, 2014
tests issue ansible#5749
 same host defined in different groups which in turn are defined
 in different ini files in an inventory directory

Conflicts:
	test/units/TestInventory.py
risaacson pushed a commit to risaacson/ansible that referenced this issue Mar 5, 2014
closes ansible#5749

Conflicts:
	test/units/TestInventory.py
@risaacson
Copy link
Contributor

Pertaining to the original description of this thread as of 1.5 this is fixed and tests correct.
https://gist.github.com/risaacson/db81aa24b61455bd32e4

After comment 11 you have a few commits that end up being a part of GH-5837. We have made some changes around I cherry picked the changes off into a branch that I am working on un-commenting test_dir_inventory. Your test should be merged as a part of GH-6287. In which you will get credit for the test code.

This issue has wondered a bit but from what I can see there is one remaining question. That question is: Should get_groups return multiple entries?

The answer to this looks to be that based on how it is used internally in inventory/init.py that there is a level of reduplication that ends up happening so it isn’t an issue unless you are calling it directly.

So once my PR goes in it will pull in your PR. I hope that I have shed a little bit of light on this.

risaacson added a commit that referenced this issue Mar 5, 2014
Fix inventory for test_dir_inventory, merge PRs from GH-5749, cleanup some formatting.
@g-k-r
Copy link
Contributor Author

g-k-r commented Mar 6, 2014

@risaacson thanks, information round up appreciated

risaacson pushed a commit to risaacson/ansible that referenced this issue Mar 6, 2014
tests issue ansible#5749
 same host defined in different groups which in turn are defined
 in different ini files in an inventory directory

Conflicts:
	test/units/TestInventory.py
risaacson pushed a commit to risaacson/ansible that referenced this issue Mar 6, 2014
closes ansible#5749

Conflicts:
	test/units/TestInventory.py
risaacson added a commit to risaacson/ansible that referenced this issue Mar 6, 2014
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bug_report labels Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug.
Projects
None yet
Development

No branches or pull requests

6 participants