[codex] fix backup dump handling#41
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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_BACKUP_DIR=/backups | ||
| NEXTCLOUD_UPGRADE_MIN_FREE_MB=2048 |
There was a problem hiding this comment.
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.
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 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e1777e29d
ℹ️ 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".
| rm -f "$backup_tmp_file" | ||
| } | ||
|
|
||
| trap cleanup EXIT |
There was a problem hiding this comment.
When the pre-upgrade dump succeeds, this EXIT trap still runs cleanup; because maintenance_enabled was set after maintenance:mode --on, the hook immediately turns maintenance mode back off before the Docker entrypoint proceeds to occ upgrade and the post-upgrade hook. In production upgrades with live traffic, requests can then hit the instance while files/schema are being upgraded, so the trap should only disable maintenance on failure or be cleared before a successful exit.
Useful? React with 👍 / 👎.
1 participant
Summary
Keeps a single PostgreSQL dump at
/backups/nextcloud-db.sql.gzduring upgrade, writes it atomically, and makes sure maintenance mode is turned off on failure.Why
This avoids timestamped dump sprawl and reduces the chance of leaving a partial dump or a stuck maintenance state.
Validation
bash tests/test-hooks.shmake test-hooks