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

Adjust ansible_user and reload_nginx handler for Ansible 2.1 #631

Merged
merged 1 commit into from
Aug 6, 2016

Conversation

fullyint
Copy link
Contributor

@fullyint fullyint commented Aug 6, 2016

This PR updates Trellis to accommodate two changes in Ansible 2.1.1.0

Handlers

The Trellis reload nginx handler is an include file with two tasks. A discourse thread revealed that Ansible 2.1.1.0 was only running the second of these two tasks. The second task fails without the var that should have been registered in the first task.

As far as I can tell, Ansible 2.0.2.0 treated the include task as a single handler task, but 2.1.1.0 reads into the include file, seeing two potential handler tasks. In this latter case, only the second task uses the notified handler name (reload nginx) so only this second task runs.

This change in 2.1.1.0 jeopardizes the strategy of using an include to run multiple tasks via a single handler name. Once Trellis requires Ansible >= 2.2, these two tasks in question can be run under a single name via the coming listen parameter.

In the meantime, this PR gives the first task the handler name that will be notified (reload nginx) and lets that first task notify the second task as another handler.

The challenge is that when the first task in reload_nginx.yml is used as an included task (vs. as a handler) -- always the case in 2.0.2.0 and once in 2.1.1.0 -- it results in the second task being run twice. The second task runs once in its role as a task in the include, then again in its role as a notified handler. This isn't a huge problem but it is unnecessary repetition. The PR avoids the unnecessary repetition by making the first task's notify parameter conditional on when the task is running as a handler (vs. included task).

ansible_user

Previously the ansible_user variable defaulted to undefined, even on local_action tasks. In Ansible 2.1.1.0 (or in some version since 2.0.2.0), ansible_user for local_action tasks defaults to the username active on the control machine.

This causes a problem to the remote-user role's block conditional when: ansible_user is not defined. When Ansible interprets the "Check whether Ansible can connect as root" task, ansible_user is in fact now defined because the task uses a local_action. Thus the task doesn't run and doesn't register the variable needed in the following task, so the playbook fails.

This PR distributes the block's conditional when to the separate tasks. The "Check whether Ansible can connect as root" task is no longer conditional on ansible_user is undefined because then it would never run. There is no problem to have this task always run because of its failed_when: false and changed_when: false.

@swalkinshaw
Copy link
Member

Tested on both 2.0.2.0 and 2.1.1.0. All good 👍

@swalkinshaw swalkinshaw merged commit 0711208 into master Aug 6, 2016
@swalkinshaw swalkinshaw deleted the ansible-2.1 branch August 6, 2016 22:11
@fullyint fullyint mentioned this pull request Aug 8, 2016
5 tasks
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