Configure NGINX without Ansible

[jdeanwallace]

Resolves #1429

This PR configures TinyPilot's NGINX server without Ansible and drops ansible-role-nginx as a dependency.

Notes

1. We moved TinyPilot's inline NGINX config to its own Jinja2 template file that is stored on the device at /usr/share/tinypilot/templates/tinypilot.conf.j2. The postinst script then populates the template variables and installs TinyPilot's NGINX config to /etc/nginx/conf.d/tinypilot.conf.

   • We store the template file on device because its variables only get populated at install time (with variables set by the YAML data in /home/tinypilot/settings.yml and TinyPilot's constants and defaults).
   • As part of this move to a Jinja2 template file, we've inlined the value of tinypilot_dir (i.e., /opt/tinypilot) and dropped the tinypilot_external_port Ansible variable (because it's no longer used by Ansible).
   • As Linux recommends, /usr/share/ is intended for "Architecture-independent (shared) data".
   • As Michael pointed out, we can also split TinyPilot's NGINX config into templated and static files to avoid wasting cpu cycles on trying to render boilerplate config.

2. A problem I encountered with the default NGINX config at /etc/nginx/nginx.conf is that previously ansible-role-nginx has been modifying this file and splitting TinyPilot's NGINX config across both /etc/nginx/nginx.conf (e.g., to define upstreams) and /etc/nginx/sites-enabled/tinypilot.conf (e.g., to define servers). So in order to restore NGINX's default config without using ansible-role-nginx, we package our own version of nginx.conf (based on the default values set by ansible-role-nginx) and replacing /etc/nginx/nginx.conf at install time.

3. Seeing as the TinyPilot systemd service relies on the NGINX service, we now bind thetinypilot.service state to the state of nginx.service. However, this relationship is only one-way. For example, if nginx stops then tinypilot will stop, but if tinypilot stops then nginx won't necessarily stop.

   According to systemd docs:

   BindsTo=
   When used in conjunction with After= on the same unit the behaviour of BindsTo= is even stronger. In this case, the unit bound to strictly has to be in active state for this unit to also be in active state.

4. (FYI) According to an interesting Lintian error that came up, we shouldn't use systemctl in the Debian postinst script and instead we should be using deb-systemd-invoke. Apparently, this is to accommodate systems that both do and do-not use systemd.

You can test this PR by running the following command on the device:

sudo /opt/tinypilot/scripts/install-bundle \
  https://output.circle-artifacts.com/output/job/51c8f653-51fe-4c52-a07d-220b950a7d72/artifacts/0/bundler/dist/tinypilot-community-20230711T1506Z-1.9.0-77+1412aca.tgz

Useful links:

• Michael's WIP branch: https://github.com/tiny-pilot/tinypilot-pro/compare/no-nginx-role?expand=1

[jdeanwallace]

Automated comment from CodeApprove ➜

⏳ @jotaen4tinypilot please review this Pull Request

[jdeanwallace]

@jotaen4tinypilot - Sorry, please hold off on the review. I found an issue.

[codeapprove]

Automated comment from CodeApprove ➜

👀 @jdeanwallace it's your turn, please take a look

[jdeanwallace]

Automated comment from CodeApprove ➜

⏳ @jotaen4tinypilot please review this Pull Request

[jdeanwallace]

This PR is ready to be reviewed 👍

[mtlynch]

@jdeanwallace - Can you share more detail about this part:

However, it doesn't drop it as a dependency just yet because we still need the role to be able to restore NGINX's original config (i.e., /etc/nginx/nginx.conf

Why do we need to restore this file? Are there alternative approaches that don't require a checkpoint release?

[jdeanwallace]

Automated comment from CodeApprove ➜

Thanks! I've resolved the outstanding comments and updated the PR description.

---

In: Discussion
Yeah the Pro version of this PR would require some extra work. I'll create some follow-up Pro-only PRs that would merge into the Pro version of this PR (before merging with master).

---

In: app/update/settings.py:
Thanks, done.

---

In: debian-pkg/debian/tinypilot.postinst:
Done.

---

In: debian-pkg/debian/tinypilot.postinst:

> Line 89
# checkpoint release.

Good idea! I've referenced https://github.com/tiny-pilot/tinypilot-pro/issues/978 in all the workarounds.

---

In: debian-pkg/usr/share/tinypilot/nginx.conf:
Based on your previous comment about differing config parameters, I've maintained the default values set by ansible-role-nginx and added a comment to the top of the nginx.conf file that explains where it came from. Thanks!

---

In: debian-pkg/usr/share/tinypilot/nginx.conf:

> Line 6
worker_processes auto;

Ah good catch! Thanks I have gone through each directive and preserved the value chosen by ansible-role-nginx to maintain the current config.

worker_processes is now auto, but it used to be "4"

ansible-role-nginx seemed to be basing its value on the number of CPUs present, which is the same as auto. So I went with auto.

---

👀 @jotaen4tinypilot, @mtlynch it's your turn please take a look

[mtlynch]

Automated comment from CodeApprove ➜

⏳ Approval Pending (3 unresolved comments)
Approval will be granted automatically when all comments are resolved

LGTM, thanks!

---

In: Discussion

However, it doesn't drop it as a dependency just yet. We'll do so in a follow-up PR. For now, we just skip running the nginx Ansible tasks.

(optional) I'm in favor of just dropping it now. It doesn't seem like there's an advantage to leaving it around as dead code until the follow-up.

---

In: debian-pkg/debian/tinypilot.postinst:

> Line 82
  <<< "${TINYPILOT_NGINX_TEMPLATE}" \

Do we need to read the template into a string? Is it possible to do something like this?

. venv/bin/activate && \
  PYTHONPATH=/opt/tinypilot/app \
    ./scripts/render-template \
  < /usr/share/tinypilot/templates/tinypilot.conf.j2 \
  > /etc/nginx/conf.d/tinypilot.conf

If we're only using the paths once, I'd rather inline them instead of defining variables for them since it's clear enough what the paths mean.

---

In: debian-pkg/debian/tinypilot.postinst:

> Line 92
  install \

If we're not using the permissions/ownership features of install, would it make more sense to use the simpler cp?

---

👀 @jdeanwallace, @jotaen4tinypilot it's your turn please take a look

[jdeanwallace]

Automated comment from CodeApprove ➜

---

In: Discussion
Okay cool, I've merged the follow-up PR into this PR and updated the PR description.

---

In: debian-pkg/debian/tinypilot.postinst:
Sure, done.

---

In: debian-pkg/debian/tinypilot.postinst:
Thanks, I'll keep that in mind for next time too.

---

👀 @jotaen4tinypilot it's your turn please take a look

[mtlynch]

Automated comment from CodeApprove ➜

Approved: I have approved this change on CodeApprove and all of my comments have been resolved.

[jdeanwallace]

@mtlynch - I've hit that codeapprove bug again where the PR approval has been granted but the PR is still blocked. I left a comment on their GitHub issue.

Jan is off this week, so can I just dismiss his review and merge?

[mtlynch]

Jan is off this week, so can I just dismiss his review and merge?

Sure

Jan is off this week and we need to unblock the PR