nginx restart causes task failure with multiple sources of nginx config

[jmeickle]

I overrode nginx.conf in my own role, dependent on this one, in order to add a new logging format. This starts to run into problems on later runs once I have vhosts deployed using my new logging format. The vhost files already exist, your role overwrites nginx.conf, and then before I can overwrite it again this line triggers a service restart that fails due to the vhosts depending on a missing logging format entry: https://github.com/geerlingguy/ansible-role-nginx/blob/master/tasks/main.yml#L28

IMO that task should just check that service=enabled, with a notify nginx restart to actually ensure it's running. That way all of the nginx configuration, including config from later-executing roles, will have been applied by the time nginx attempts a reload.

[zmoshansky]

@geerlingguy This change would be nice to compose this role with a larger playbook. Furthermore, what's your thought on changing the restart handler to:

---
- name: restart nginx
  service: name=nginx state=restarted
  when: {{ nginx_enable_restart }}

where nginx_enable_restart=true by default?

I believe it would preserve the same behaviour, but allow composing playbooks to make any addition changes, or hand off restart to a daemon like monit.

[FinBoWa]

I did a tweak on my own fork to allow overriding what template is used. See commit Sohova@bde879c

using {{ inventory_dir }} or {{ playbook_dir }} its quite easy to get the path to your own ad hoc template file when you just can't be bothered to fork the project :).

[FinBoWa]

Thought its better just to fork the ansible role and add the missing bits there suit your needs and using the requirements file in yml format.

- src: https://github.com/Focus-Flow/ansible-role-nginx
  version: 1.4.0+0.0.1
  name: geerlingguy.nginx
  path: ansible-roles/ansible-roles-remote

That way you can lock the version what you were using to a specific one. When its forked its quite easy to merge changes from the upstream back if needed to be and give pull requests back to roles creator :).

[geerlingguy]

Note—with the addition of #36, the addition of vhost configs is ordered more correctly, and the restart of Nginx is dependent on a config check. Typically, if I need to add any further specialization to virtual host configuration or other Nginx config, I do that in pre_tasks, prior to running this role.

Also, with the pattern you're using, it might be simplest (if you desire) to use the just-added nginx_extra_http_options to add options directly inside the server http {} block.

I don't know if I want to make this role's start/stop/restarting behavior more complicated than it already is. The proposal in #32 seems much more confusing to me, and I don't see a really simple nor idempotent solution to having this role and a role with it as a dependency both controlling the same config file.