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

Replicate most of the ansible style host matching logic #753

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Code0x58
Copy link

@Code0x58 Code0x58 commented Jan 21, 2024

This expands on the ansible pattern matching so you can use multiple parts, intersection, negation, and regex patterns, on top of the already implemented globbing.

I'd like to backport this change into an 8.2 release if you'll take it.

Copy link
Author

@Code0x58 Code0x58 left a comment

Choose a reason for hiding this comment

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

looks ok when used in testinfra_hosts on my setup. Should apply the change to easily reduce the minimum working python3 version (tested on 3.8)

testinfra/utils/ansible_runner.py Outdated Show resolved Hide resolved
testinfra/utils/ansible_runner.py Outdated Show resolved Hide resolved
else:
positive.update(expand_pattern(requirement, inventory))

result = positive
Copy link
Author

Choose a reason for hiding this comment

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

Comment about something that I don't see as worth changing right now as there hasn't been any response on this yet, and a significant improvement is much better than nothing.

I suspect a perfect copy of the logic from ansible would use something like this here:

Suggested change
result = positive
if positive:
result = positive
else:
result = expand_pattern('all')

Then you imitate behaviour like !right which is equivalent to all:!right, but this would also be best with additions to manual testing and adding of tests to this codebase for the confirmed functionality.

Given that adding all: to the start of a host pattern/query is equivalent, it's a fine enough workaround. If this change gets merged/released, I'll follow up with this change.

@Code0x58 Code0x58 changed the title Draft: Replicate more of the ansible style host matching logic Replicate more of the ansible style host matching logic May 15, 2024
@Code0x58 Code0x58 changed the title Replicate more of the ansible style host matching logic Replicate most of the ansible style host matching logic May 15, 2024
@Code0x58
Copy link
Author

I'm happy with the improvements on here, so have un-drafted the PR

@CarstenGrohmann
Copy link
Contributor

What do you think about renaming test_ansible.py to test_ansible_inventory.py or similar? That would be more meaningful.

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