Skip to content

fixed precedence on ansible.cfg - #6038

Merged
softwarefactory-project-zuul[bot] merged 2 commits into
ansible:develfrom
gamuniz:honor_thy_precedence
Mar 4, 2020
Merged

fixed precedence on ansible.cfg#6038
softwarefactory-project-zuul[bot] merged 2 commits into
ansible:develfrom
gamuniz:honor_thy_precedence

Conversation

@gamuniz

@gamuniz gamuniz commented Feb 23, 2020

Copy link
Copy Markdown
Contributor
SUMMARY

Fix to correctly use project directory before /etc/ansible/ansible.cfg in instances where a setting exists in both.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • API
AWX VERSION
make VERSION
awx: 9.2.0
ADDITIONAL INFORMATION

@softwarefactory-project-zuul

Copy link
Copy Markdown
Contributor

Build succeeded.

@AlanCoding

Copy link
Copy Markdown
Member

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.

Comment thread awx/main/utils/ansible.py Outdated
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'))

Copy link
Copy Markdown
Contributor

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 ?

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor

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.

@softwarefactory-project-zuul

Copy link
Copy Markdown
Contributor

Build succeeded.

@AlanCoding

Copy link
Copy Markdown
Member

Sorry, still trying to get the testing straight.

@AlanCoding

Copy link
Copy Markdown
Member

This time I'm actually getting around to testing.

@AlanCoding AlanCoding left a comment

Copy link
Copy Markdown
Member

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.

@AlanCoding

Copy link
Copy Markdown
Member

This is about the millionth time I've said this, but hold off for a bit, still waiting for final test results.

@AlanCoding

Copy link
Copy Markdown
Member

Okay, now I'm good with merging this.

@kdelee kdelee left a comment

Copy link
Copy Markdown
Member

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

@softwarefactory-project-zuul

Copy link
Copy Markdown
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul
softwarefactory-project-zuul Bot merged commit d08f592 into ansible:devel Mar 4, 2020
@gamuniz
gamuniz deleted the honor_thy_precedence branch March 4, 2020 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants