Skip to content

Feat/nextcloud upgrade hooks#41

Open
henmohr wants to merge 2 commits into
mainfrom
feat/nextcloud-upgrade-hooks
Open

Feat/nextcloud upgrade hooks#41
henmohr wants to merge 2 commits into
mainfrom
feat/nextcloud-upgrade-hooks

Conversation

@henmohr

@henmohr henmohr commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread docker-compose.yml
NEXTCLOUD_VERSION: ${NEXTCLOUD_VERSION:-stable-fpm}
volumes:
- ./volumes/nextcloud:/var/www/html
- ./backups:/backups

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment thread .env.example
Comment on lines +11 to +12
NEXTCLOUD_BACKUP_DIR=/backups
NEXTCLOUD_UPGRADE_MIN_FREE_MB=2048

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant