No error after nginx reload error in subsequent playbook runs

[strarsis]

Submit a feature request or bug report

• I've read the guidelines for Contributing to Roots Projects
• This is a feature request
• This is a bug report
• This request isn't a duplicate of an existing issue
• I've read the docs and followed them (if applicable)
• This is not a personal support request that should be posted on the Roots Discourse forums

What is the current behavior?
When nginx fails to reload after nginx config change the error is reported by ansible.
However, subsequent playbook runs do not attempt to start nginx and no indication or error is logged (all OK).

What is the expected or desired behavior?
Subsequent ansible playbook runs must attempt to start nginx and at least report that it isn't running (as in the first causing run).
Additionally, a config rollback in case of nginx failing to reload using it should be attempted.

---

Bug report

Please provide steps to reproduce, including full log output:

• Intentionally introduce some syntax error in a nginx child config template.
• Run the playbook for the first time, note the error message.
• Run the playbook subsequent times after first time, note the absence of any error messages.
• Log into the server as admin user, try to reload nginx, sudo service nginx reload,
  note that nginx fails to reload.

Please describe your local environment:

Ansible version: 2.2.1.0

OS: Ubuntu 16.04.2 LTS

Vagrant version: Vagrant 1.9.1

Where did the bug happen? Development or remote servers?

Development.

[swalkinshaw]

I confirmed this does happen. The behaviour is "correct" but maybe not ideal.

We could just add

- include: "{{ playbook_dir }}/roles/common/tasks/reload_nginx.yml"

at the end of roles/wordpress-setup/tasks/main.yml

@fullyint thoughts on that?

[fullyint]

tl;dr -- if we decide to change anything, just including reload_nginx.yml and/or "always reloading/restarting all the things" may be most pragmatic, despite being non-idempotent.

---

I like this excerpt discussing how Ansible is "Goal-oriented, Not Scripted"...

Ansible features an state-driven resource model that describes the desired state of computer systems and services, not the paths to get them to this state. No matter what state a system is in, Ansible understands how to transform it to the desired state. -- reference

Trellis performs adequately from a script perspective, warning about the initial Nginx test/reload failure.

However, it could be argued that Trellis fails from the perspective of the "desired state" goal being Nginx running, with the latest configs in effect. With current Trellis, a playbook run (other than the first) would not be aware of a past failure, would not be aware that Nginx needs to be started (if not running), and would not be aware that a reload was needed (if the latest Nginx confs were not yet in effect).

Quoting above, I like this goal: "No matter what state a system is in, [Trellis] understands how to transform it to the desired state." Taking Nginx as an example, we could indeed always include the reload_nginx.yml file to test and reload Nginx. This achieves the goal, the "desired state." The only downside is that it would not be idempotent (state: reloaded always yields changed).

We can maintain idempotence while ensuring that Nginx is running because the service module's state: started is idempotent.

However, idempotence is more challenging while ensuring that Nginx has the latest confs in effect. I don't know of any related option built-in to Ansible. Suppose the playbook managed to template new confs on the server but there was a failure or user-interrupt before Nginx reloaded and applied the new confs. We could compare the Nginx process date (e.g., ps -eo cmd,lstart | grep nginx) with the modified date on the relevant Nginx conf files. If the process is older than the confs, the process needs a restart.

The challenge is that this issue may also apply to other services such as php7.1-fpm, mysql, ssh, ferm, fail2ban, memcached, etc. I haven't dug in enough to see if there are established methods for Ansible to handle a concern such as this (e.g., to detect whether a service is loaded with the latest configs, vs. just running). Perhaps the most effective approach would be to write a module that accepts parameters for processes and their configs, comparing age, then reloading/restarting processes when applicable.

If there were a clean implementation of such a module, it strikes me as worthwhile, but it may be most pragmatic to either instruct users that "If you see an error, you must deal with it, e.g., fix your Nginx conf and reload" (the current implicit expectation), or just always reload/restart services, choosing pragmatism over idempotence.

[strarsis]

Maybe a simple smoke test would already suffice that just tries to connect to each site (HTTP/HTTPS)?
Edit: This would indicate an issue with nginx, but isn't an elegant solution.

[swalkinshaw]

A test that verifies a connection to the main site might be good. Get's a little weirder if you have multiple sites maybe?

[swalkinshaw]

Closing since this hasn't received any activity in over 5 years.