Skip to content

Various ansible fixes #112

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

Merged
merged 7 commits into from
Jul 6, 2016
Merged

Various ansible fixes #112

merged 7 commits into from
Jul 6, 2016

Conversation

philpep
Copy link
Contributor

@philpep philpep commented Jun 27, 2016

No description provided.

**kwargs).run()
inventory=self.inventory,
**kwargs)
result = runner.run()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unneeded change

@rabadin
Copy link

rabadin commented Jun 28, 2016

fwiw (I know this is WIP), I tested this branch to see if it would solve the problem with the vault and now all my tests are skipped:

SKIP [4] <path>/tests/integration/test_jenkins.py:34: got empty parameter set [u'_testinfra_backend'], function test_slave_is_online at <path>/tests/integration/test_jenkins.py:34

Reverting to 1.3.1, the tests are not skipped anymore (but blow up with AnsibleError: A vault password must be specified to decrypt [...]).
HTH

@philpep
Copy link
Contributor Author

philpep commented Jun 28, 2016

@rabadin thanks for testing! I think I've broken the get_hosts() function that is responsible to expand host specification to real hosts in ansible inventory...

@retr0h
Copy link
Contributor

retr0h commented Jun 29, 2016

I'm not sure if this belongs here, but I have found testinfra loads variables differently. Ansible.get_variables looks to load host inventory (content specified by --ansible-inventory) vs actual inventory. However the tricky Ansible('debug', "msg={{ $VARIABLE }}")['msg'] works.

Here is my test. Where availability_zone is defined in inventory we bring in through an ansible vars plugin, and cinder_local is defined in "host inventory". We can get to everything through debug, but it's clunky.

def test_variables(Ansible):
    az = Ansible('debug', "msg={{ availability_zone }}")['msg']
    cinder_local = Ansible('debug', "msg={{ cinder_local }}")['msg']
    variables =  Ansible.get_variables()

    assert 'proxmox16-5' == az
    assert 'True' == cinder_local

    assert variables['cinder_local']
    assert 'proxmox16-5' == variables['az']

Failure is:

        assert 'proxmox16-5' == az
        assert 'True' == cinder_local

        assert variables['cinder_local']
>       assert 'proxmox16-5' == variables['az']
E       KeyError: 'az'

@philpep
Copy link
Contributor Author

philpep commented Jun 29, 2016

  • Check host facts gathering (add an option if necessary)
  • Add get_hosts() tests.
  • Fix tests
  • Add more variables tests

@philpep
Copy link
Contributor Author

philpep commented Jun 29, 2016

@retr0h @rabadin updated the patchset.

Now most of ansible objects are created once per backend (instead of once per command run) and I fixed a bug in ansible v2 where get_variables() didn't return group vars (but maybe there is still similar issues).

It would be great if that fix (at least some of) your issues.

@retr0h
Copy link
Contributor

retr0h commented Jun 30, 2016

Now most of ansible objects are created once per backend (instead of once per command run) and I fixed a bug in ansible v2 where get_variables() didn't return group vars (but maybe there is still similar issues).

I should note, I am using ansible 1.9 stable.

@retr0h
Copy link
Contributor

retr0h commented Jun 30, 2016

I still see it gathering facts per host. I have 3 hosts that make up testinfra_hosts = ['mcp']. However, it is much faster. My tests from #56 are now ~9 seconds.

roles/integration_tests/tests/integration/test_keystone.py::test_variables[ansible://mcp1]
roles/integration_tests/tests/integration/test_keystone.py::test_variables[ansible://mcp2]
roles/integration_tests/tests/integration/test_keystone.py::test_variables[ansible://mcp3]

@rabadin
Copy link

rabadin commented Jun 30, 2016

Just tested with f69c802 and I still see all the tests being skipped with the got empty parameter set [u'_testinfra_backend'] message.

@philpep
Copy link
Contributor Author

philpep commented Jul 3, 2016

@rabadin your issue looks weird, and I wouldn't break something while merging this PR. Could you tell me more about your setup and how testinfra is run etc ?

@philpep
Copy link
Contributor Author

philpep commented Jul 3, 2016

@retr0h about:

  assert 'proxmox16-5' == variables['az']

Your variable name is availability_zone, not az. If it's just a typo (in your comment) please open an issue ;)

@retr0h
Copy link
Contributor

retr0h commented Jul 3, 2016

Your variable name is availability_zone, not az. If it's just a typo (in your comment) please open an issue ;)

I'll double check. However, I am looking forward to these performance fixes ;) Could use them.

@rabadin
Copy link

rabadin commented Jul 5, 2016

Hi @philpep, my setup is not easy to reproduce (it includes an openstack cloud) but I've managed to reproduce the "tests being skipped" problem with a vagrant machine. I /think/ the problem comes from using a dynamic inventory (passed to --ansible-inventory). I've seen the tests being skipped using this file: https://gist.github.com/rabadin/cb05f8c39317df23d5f9adb08eaab4f8 (this is a dynamic inventory which contains only my vagrant host -it's obviously dumb but it's only to test with a dynamic inventory).

@rabadin
Copy link

rabadin commented Jul 5, 2016

Note that when I use a static inventory in lieu of my dynamic one (https://gist.github.com/rabadin/a390ebba907de6268dab5a1ee1558248), and with the correct vault_password_file value in ansible.cfg, I'm able to access variables from vaulted files in my tests; so that bit seems to work for me.

@rabadin
Copy link

rabadin commented Jul 5, 2016

After additional investigation I think the problem comes from my dynamic inventory script: the hosts in there were not part of any group and this caused the tests being "skipped". If I add a group with my dynamic hosts (in the dynamic inventory) then it works.

@rabadin
Copy link

rabadin commented Jul 5, 2016

Confirmed: I've done the same test in my openstack config with this pr and when I modify the dynamic inventory script as described above everything works.
👍 ;)

@retr0h
Copy link
Contributor

retr0h commented Jul 5, 2016

Confirmed: I've done the same test in my openstack config with this pr and when I modify the dynamic inventory script as described above everything works.

Awesome, looking forward to this release due to the performance improvements detailed in #56.

@philpep philpep merged commit 8cee538 into master Jul 6, 2016
@philpep
Copy link
Contributor Author

philpep commented Jul 6, 2016

Merged. I'll release a new version soon.

@philpep
Copy link
Contributor Author

philpep commented Jul 6, 2016

Released in 1.4.0

@philpep philpep deleted the ansible-vault branch September 3, 2016 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants