Feat/nextcloud upgrade hooks#41
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be81860e7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| NEXTCLOUD_VERSION: ${NEXTCLOUD_VERSION:-stable-fpm} | ||
| volumes: | ||
| - ./volumes/nextcloud:/var/www/html | ||
| - ./backups:/backups |
There was a problem hiding this comment.
Create a writable backup mount before running hooks
On a fresh checkout where ./backups does not already exist, this bind mount is created by Docker as a root-owned host directory, but the official Nextcloud entrypoint executes hook scripts as www-data. The first upgrade then fails at the pre-upgrade redirect to /backups/app_list.old (or the dump output) before the database backup is made, so the container cannot complete an image upgrade unless the operator manually pre-creates and chowns the directory. Please make this mount writable by default, for example by committing a managed directory/setup step or using a named volume with suitable ownership.
Useful? React with 👍 / 👎.
| NEXTCLOUD_BACKUP_DIR=/backups | ||
| NEXTCLOUD_UPGRADE_MIN_FREE_MB=2048 |
There was a problem hiding this comment.
Pass upgrade hook settings into the container
These new values are only added to .env.example, but the app service environment lists in the compose files never pass NEXTCLOUD_BACKUP_DIR or NEXTCLOUD_UPGRADE_MIN_FREE_MB through to the container. In deployments that edit .env to change the backup path or free-space threshold as documented, the pre-upgrade hook still falls back to /backups and 2048, so the safety check and backup location cannot actually be configured from the sample env file. Add these variables to the app environment in both compose files.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| echo "Running post-upgrade Nextcloud commands" | ||
| php occ app:list > /backups/app_list.new |
There was a problem hiding this comment.
Honor the configured backup directory after upgrade
When NEXTCLOUD_BACKUP_DIR is customized for the pre-upgrade hook, this post-upgrade hook still writes and reads /backups directly. On setups that mount only the configured backup path, the redirect here fails after occ upgrade but before maintenance:mode --off, leaving the upgraded instance stuck in maintenance; even when /backups exists, the app-list diff compares the wrong files. Use the same backup directory variable here as the pre-upgrade script.
Useful? React with 👍 / 👎.
No description provided.