Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make sure to fail rather than continue if rsync fails #2361

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joshtrichards
Copy link
Member

@joshtrichards joshtrichards commented Jan 13, 2025

Ran across an in-the-wild environment where rsync was failing due to I/O disk errors, but initializing continued on anyhow. We should check the exit values and fail hard since it'll result in a broken installation (and may get overlooked until later depending on what files the errors occur on).

2025-01-13T15:13:04.250546270Z Initializing nextcloud 30.0.4.1 ...
2025-01-13T15:13:13.543136088Z rsync: [sender] read errors mapping "/usr/src/nextcloud/apps/text/js/pgsql-6nMyK_Wd.chunk.mjs.map": I/O error (5)
[...]
2025-01-13T15:13:13.791311797Z ERROR: apps/text/js/pgsql-6nMyK_Wd.chunk.mjs.map failed verification -- update discarded.
[...]
2025-01-13T15:13:18.615971825Z rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1338) [sender=3.3.0]
2025-01-13T15:13:18.903862977Z Initializing finished
2025-01-13T15:13:18.903916218Z New Nextcloud instance.
2025-01-13T15:13:18.913472340Z Installing with pgsql database
2025-01-13T15:13:18.913679471Z Starting Nextcloud installation...

Follow-up ideas:

  • wrap in a function
  • create a generic "run_this()` function that checks exit values for any commands

P.S. Should probably get more testing.

@joshtrichards
Copy link
Member Author

@szaimen FYI - This turned up (on the forum) with an AIO user and both images share similar rsync code in the entrypoint:

https://github.com/nextcloud/all-in-one/blob/c1f1207a0e1cc1e70a0620e4be70d0a2129cc907/Containers/nextcloud/entrypoint.sh#L220

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

Successfully merging this pull request may close these issues.

1 participant