Update overhaul community release.

[jdeanwallace]

Resolves https://github.com/tiny-pilot/tinypilot/issues/1027

Testing procedure

Test update path from an existing TinyPilot installation

Follow these install instructions.

Bugs

1. The /opt/tinypilot-updater directory is orphaned after an update. https://github.com/tiny-pilot/tinypilot/pull/1060

Test new installation on fresh device

Follow these install instructions.

Bugs

1. Needs sudo priv when running get-tinypilot.sh. https://github.com/tiny-pilot/tinypilot/pull/1045

2. Temp directory gets deleted before it is used. https://github.com/tiny-pilot/tinypilot/pull/1049

   • CircleCI logs: https://app.circleci.com/pipelines/github/tiny-pilot/tinypilot/2311/workflows/a0d23ef4-f2e5-4283-820f-24fb5f9c651c/jobs/8705?invite=true#step-102-310

3. Prevent get-tinypilot.sh from executing before the script is fully downloaded. https://github.com/tiny-pilot/tinypilot/pull/1052

4. We get a HTTP 404 on some local project licenses because the /opt/tinypilot-updater directory no longer exists. Is it still worth referencing these licenses? https://github.com/tiny-pilot/tinypilot/pull/1054

5. HTTP 500 error when running /opt/tinypilot-privileged/update-video-settings because the /opt/tinypilot-updater directory no longer exists. https://github.com/tiny-pilot/ansible-role-tinypilot/issues/206

6. The CircleCI e2e step is testing the previous bundle version. https://github.com/tiny-pilot/tinypilot/pull/1068

   The e2e step basically just runs get-tinypilot.sh and checks if the webserver is returning HTTP 200. However, the e2e step doesn't wait for the LATEST file to be updated with the newest build version before running because that would effectively "release" the version which makes e2e step pointless.

Test reinstallation on already updated device

Follow these install instructions.

Bugs

1. TinyPilot Debian package doesn't overwrite on reinstall of the same version when installing via Ansible.

   Not really an issue because we're installing the exact same version anyway. However, if we're hacking around on the device and then do a reinstall via Ansible it won't "reset to factory settings" because the TinyPilot Debian package Ansible skips installation when versions are the same. This is probably ideal seeing as we don't want to accidentally override settings or configs.

   However, a dev can overwrite the Debian package files by manually reinstalling the Debian package via:

   sudo dpkg --install tinypilot-20220719142135-1-armhf.deb
   

   Further reading:

   • Debian unpack phases: https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html#details-of-unpack-phase-of-installation-or-upgrade

2. TinyPilot Debian package doesn't remove __pycache__ directories and *.pyc files on uninstall. https://github.com/tiny-pilot/tinypilot/pull/1062 https://github.com/tiny-pilot/tinypilot/pull/1063

   From Debian packaging manual:

   The package’s prerm has to make sure that both foo.pyc and foo.pyo are removed.

   Also not much of an issue because tinypilot still gets uninstalled successfully with the following warning:

   $ sudo apt-get remove tinypilot
   Removing tinypilot (20220711124636) ...
   dpkg: warning: while removing tinypilot, directory '/opt/tinypilot' not empty so not removed
   

   This leaves a few files & directories inside /opt/tinypilot, namely:

   • venv directory
   • app_settings.cfg file
   • .pyc files
   • __pycache__ directories

   Additionally, the entire /opt/tinypilot directory gets removed by our preinst script anyway.

3. The /opt/tinypilot-updater directory ends up with different directory owners (i.e. pi or root) depending on install vs update. https://github.com/tiny-pilot/tinypilot/pull/1074

   When installing:

   ++ whoami
   ++ whoami
   + sudo chown pi:pi --recursive /opt/tinypilot-updater
   + pushd /opt/tinypilot-updater
   

   When updating:

   Jul 26 13:30:52 raspberrypi tinypilot-update-svc[698]: ++ whoami
   Jul 26 13:30:52 raspberrypi tinypilot-update-svc[698]: ++ whoami
   Jul 26 13:30:52 raspberrypi tinypilot-update-svc[698]: + sudo chown root:root --recursive /opt/tinypilot-updater
   Jul 26 13:30:52 raspberrypi sudo[781]:     root : TTY=unknown ; PWD=/opt/tinypilot ; USER=root ; COMMAND=/usr/bin/chown root:root --recursive /opt/tinypilot-updater
   Jul 26 13:30:52 raspberrypi sudo[781]: pam_unix(sudo:session): session opened for user root by (uid=0)
   Jul 26 13:30:52 raspberrypi sudo[781]: pam_unix(sudo:session): session closed for user root
   Jul 26 13:30:52 raspberrypi tinypilot-update-svc[698]: + pushd /opt/tinypilot-updater
   

   However, this doesn't cause any issues.

Test downgrade from Pro to Community

Follow these install instructions.

Bugs

1. Can't read $FORCE_DOWNGRADE because the script is being run as root user. https://github.com/tiny-pilot/tinypilot/pull/1053 https://github.com/tiny-pilot/tinypilot/pull/1058

Test upgrade to Pro on an already updated Community device

Follow these install instructions.

Bugs

1. Can't git pull Pro repo with non-empty /opt/tinypilot directory. https://github.com/tiny-pilot/tinypilotkvm.com/pull/769

   Ansible logs:

   TASK [tinypilot.tinypilot-pro : create TinyPilot folder] ***********************
   ok: [localhost]
   
   TASK [tinypilot.tinypilot-pro : get TinyPilot repo] ****************************
   fatal: [localhost]: FAILED! => {"changed": false, "cmd": "/usr/bin/git clone --origin origin REDACTED", "msg": "fatal: destination path '/opt/tinypilot' already exists and is not an empty directory.", "rc": 128, "stderr": "fatal: destination path '/opt/tinypilot' already exists and is not an empty directory.\n", "stderr_lines": ["fatal: destination path '/opt/tinypilot' already exists and is not an empty directory."], "stdout": "", "stdout_lines": []}
   

   Directory contents:

   $ ls -la /opt/tinypilot
   total 52
   drwxr-xr-x  5 tinypilot tinypilot 4096 Jul 26 13:35 .
   drwxr-xr-x  7 root      root      4096 Jul 26 13:30 ..
   -rw-r--r--  1 tinypilot tinypilot   55 Jul 26 13:23 COPYRIGHT
   -rw-r--r--  1 tinypilot tinypilot 1054 Jul 26 13:23 LICENSE
   -rw-r--r--  1 tinypilot tinypilot 7902 Jul 26 13:23 README.md
   -rw-r--r--  1 tinypilot tinypilot    8 Jul 26 13:26 VERSION
   -rw-r--r--  1 tinypilot tinypilot    0 Jul 26 13:23 __init__.py
   drwxr-xr-x 10 tinypilot tinypilot 4096 Jul 26 13:36 app
   -rw-r--r--  1 tinypilot tinypilot  169 Jul 26 13:09 app_settings.cfg
   -rw-r--r--  1 tinypilot tinypilot  404 Jul 26 13:23 requirements.txt
   drwxr-xr-x  2 tinypilot tinypilot 4096 Jul 26 13:35 scripts
   -rw-r--r--  1 tinypilot tinypilot  468 Jul 26 13:23 setup.py
   drwxr-xr-x  6 tinypilot tinypilot 4096 Jul 26 13:35 venv
   

[codeapprove[bot]]

👋 Review this Pull Request on CodeApprove here: https://codeapprove.com/pr/tiny-pilot/tinypilot/1046

[jdeanwallace]

@mtlynch - Just to be sure, I want to resolve these conflicts, is it safe for me to merge master directly into this branch (without a PR)?

[mtlynch]

@jdeanwallace - Yeah a merge from master -> update-overhaul is fine.

[jdeanwallace]

@mtlynch - I've updated the description of this PR to showcase how I've tested the update-overhaul and what bugs I've found (so far). I've fixed most of the bugs as I went. Some of the bugs aren't that critical (or simply aren't bugs at all).

Can you take a look at what I have so far and comment on the "bugs" that don't have a PR attached?

[mtlynch]

@jdeanwallace - Nice finds on all these bugs! Good thing we caught them now.

The /opt/tinypilot-updater directory is orphaned after an update.

It's a pretty large directory, so I think we should deal with this now. I filed #1055.

We get a HTTP 404 on some local project licenses because the /opt/tinypilot-updater directory no longer exists. Is it still worth referencing these licenses?

Nice find!

Yeah, we should fix this. I submitted #1054.

TinyPilot Debian package doesn't overwrite on reinstall of the same version.

This only affects the dev team during testing, right? Is there a workaround where we can just manually remove it before installing a new version?

TinyPilot Debian package doesn't remove pycache directories and *.pyc files on uninstall.

Yeah, this seems worth fixing as well. I filed #1056.

Can't read $FORCE_DOWNGRADE because the script is being run as root user.

Good catch! I think we should fix this, but I'm wondering if there's a cleaner way.

[jdeanwallace]

@mtlynch

The /opt/tinypilot-updater directory is orphaned after an update.

It's a pretty large directory, so I think we should deal with this now. I filed #1055.

Fixed in https://github.com/tiny-pilot/tinypilot/pull/1060

We get a HTTP 404 on some local project licenses because the /opt/tinypilot-updater directory no longer exists. Is it still worth referencing these licenses?

Nice find!

Yeah, we should fix this. I submitted #1054.

Thanks!

TinyPilot Debian package doesn't overwrite on reinstall of the same version.

This only affects the dev team during testing, right? Is there a workaround where we can just manually remove it before installing a new version?

Sorry actually the "doesn't overwrite on reinstall" part is a feature of when Ansible installs the Debian package. However, if we manually run the following on the device:

sudo dpkg --install tinypilot-20220719142135-1-armhf.deb

the Debian package files will overwrite the files already on the device. So false alarm.

TinyPilot Debian package doesn't remove pycache directories and *.pyc files on uninstall.

Yeah, this seems worth fixing as well. I filed #1056.

Fixed in https://github.com/tiny-pilot/tinypilot/pull/1062 & https://github.com/tiny-pilot/tinypilot/pull/1063

Can't read $FORCE_DOWNGRADE because the script is being run as root user.

Good catch! I think we should fix this, but I'm wondering if there's a cleaner way.

Yes, thanks for your moving the sudo to inside get-tinypilot.sh idea. Fixed in https://github.com/tiny-pilot/tinypilot/pull/1058

[jdeanwallace]

Automated comment from CodeApprove ➜

⏳ @mtlynch please review this Pull Request

[jdeanwallace]

Automated comment from CodeApprove ➜

⏳ @jotaen4tinypilot please review this Pull Request

[jotaen4tinypilot]

Follow-up-note to ourselves: don’t forget to revert the branch filter after merging this branch, see https://github.com/tiny-pilot/tinypilot/pull/1087.

[jotaen4tinypilot]

Follow-up-note to ourselves: don’t forget to switch back the ansible version to master, https://github.com/tiny-pilot/tinypilot/pull/1088.

[jdeanwallace]

FYI when it comes to merging this PR the only option available is "Squash and merge", which I assume is okay:

The reason why we might consider using "Create a merge commit" instead is because the update-overhaul branch has been the target branch for multiple PR's and it might be nice to preserve that merge history to master. However, not a blocker.

[jotaen4tinypilot]

I think I’d second the squash-merging, but no strong opinion here either. Due to us merging back master a couple of times, the history unfortunately wouldn’t come out as tidy as it could be. (Which is no critique of us merging back master frequently!)

(Blue is master, red is update-overhaul.)