-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
fixed precedence on ansible.cfg #6038
fixed precedence on ansible.cfg #6038
Conversation
Build succeeded.
|
Based on the description of the issue and what you're doing here, this seems like it's probably what we want. But I want to get proper testing for it. I hope I can get around to that myself this afternoon tomorrow, unless someone else can first. |
awx/main/utils/ansible.py
Outdated
@@ -106,7 +106,7 @@ def could_be_inventory(project_path, dir_path, filename): | |||
def read_ansible_config(project_path, variables_of_interest): | |||
fnames = ['/etc/ansible/ansible.cfg'] | |||
if project_path: | |||
fnames.insert(0, os.path.join(project_path, 'ansible.cfg')) | |||
fnames.insert(1, os.path.join(project_path, 'ansible.cfg')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we just want fnames.append
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure if it was done this way for another reason so I fixed it with as little changing as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I'm just saying that based on what you're doing here, names.append(os.path.join(project_path, 'ansible.cfg'))
is more succinct and accomplishes the same thing.
Build succeeded.
|
Sorry, still trying to get the testing straight. |
This time I'm actually getting around to testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found this quite hard to test, but eventually got it. The content for testing is at ansible/test-playbooks#124
make a JT that runs the debug playbook. Find that fails in devel, with role not found. Then use this branch, and find that it is successful.
This is about the millionth time I've said this, but hold off for a bit, still waiting for final test results. |
Okay, now I'm good with merging this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alan's tests passed w/ this branch -- I say ship it
Build succeeded (gate pipeline).
|
SUMMARY
Fix to correctly use project directory before /etc/ansible/ansible.cfg in instances where a setting exists in both.
ISSUE TYPE
COMPONENT NAME
AWX VERSION
ADDITIONAL INFORMATION