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

Add a magical function testinfra_hosts_update to update host list in runtime #609

Closed
wants to merge 1 commit into from

Conversation

amarao
Copy link
Contributor

@amarao amarao commented Apr 7, 2021

This adds a support for optional magical function testinfra_hosts_update to each module, which allows to filter host list after it was created by testinfra.get_hosts before doing an actual test parametrization. Practially, it's allow to filter/update/mangle host list after it was become a list of concrete hosts.

The issue it tries to solve is that testinfra_hosts when used in tests for molecule/ansible environment usually contains group names (as host names are volatile in many molecule usecases), and some tests may requires to be "run_once" for a given group.

The practical scenario is like this (mons is a groups for Ceph monitors):

testinfra_hosts = ["ansible://mons?force_ansible=True"]

def testinfra_hosts_update(host_list):
    return testinfra[host_list[0]]

def test_cluster_OK(host):
   assert 'HEALTH_OK' in host.check_output('ceph -s').stdout

@amarao amarao force-pushed the add_testinfra_hosts_update branch 3 times, most recently from b9ee620 to a3e2537 Compare April 7, 2021 17:30
Copy link
Contributor

@philpep philpep left a comment

Choose a reason for hiding this comment

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

Hi thanks for this. But I think it's a bit too magical :)

Maybe It's better to handle ansible patterns no ? See #602

if hasattr(metafunc.module, "testinfra_hosts_update"):
if not callable(metafunc.module.testinfra_update.callable):
pytest.fail("testinfra_hosts_update must be a function")
params = metafunc.module.testinfra_hosts_update(params)
Copy link
Contributor

Choose a reason for hiding this comment

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

This line should also be under the if no ?

Copy link
Contributor Author

@amarao amarao Apr 8, 2021

Choose a reason for hiding this comment

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

oops. Thank you! I've cleanup code and missed that.

Fixed.

@@ -117,6 +117,10 @@ def pytest_generate_tests(metafunc):
ansible_inventory=metafunc.config.option.ansible_inventory,
force_ansible=metafunc.config.option.force_ansible,
)
if hasattr(metafunc.module, "testinfra_hosts_update"):
if not callable(metafunc.module.testinfra_update.callable):
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be callable(metafunc.module.testinfra_hosts_update) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed bad offset from comment above, now it looks like this:

        if hasattr(metafunc.module, "testinfra_hosts_update"):
            if not callable(metafunc.module.testinfra_update.callable):
                pytest.fail("testinfra_hosts_update must be a function")
            params = metafunc.module.testinfra_hosts_update(params)

The logic is that we complains if there is something called testinfra_hosts_update but it's not callable.

list of hosts inside module.

The key useful scenario is filtering out hosts from
automatically generated host lists (f.e. from ansible inventory
based on group names).

This allows to create a tool to update this list, f.e. to
implement 'run_once' policy.
@amarao amarao force-pushed the add_testinfra_hosts_update branch from a3e2537 to 6d7094f Compare April 8, 2021 09:47
@amarao
Copy link
Contributor Author

amarao commented Apr 8, 2021

Hi thanks for this. But I think it's a bit too magical :)

Maybe It's better to handle ansible patterns no ? See #602

I thought about it. I really would like ability to specify the hosts-style host list (something like that: ansible://mons[0],mgrs,!foobar, which says 'host no1 from monitors, all mgrs, but not the stuff in foobar group or named foobar'), but I suspect it's hard to implement.

Actually some kind of hook to process the hostlist after it was parsed and processed by Ansible is really welcomed. Ansible inventory is really tricky thing (given the molecule control over inventory), so ability to tweak results are important, I think.

The magicalily of testinfra_hosts_update is on pair with testinfra_hosts, I think. Better naming may be a good idea, though.

@philpep
Copy link
Contributor

philpep commented Apr 8, 2021

@amarao I added some comment on #602 , seems ansible --list-hosts <pattern> resolve hosts from the pattern. In any cases I think we need to implement ansible patterns. And maybe later on we could add such hook if still needed.

@philpep philpep closed this May 19, 2023
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.

2 participants