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.

Sign up for GitHub

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

Jump to bottom

Consider setting set -eu globally in nextcloud container entrypoint to catch more errors #6014

Open
joshtrichards opened this issue Feb 5, 2025 · 1 comment · May be fixed by #6028
Open

Consider setting set -eu globally in nextcloud container entrypoint to catch more errors #6014

joshtrichards opened this issue Feb 5, 2025 · 1 comment · May be fixed by #6028
Labels
1. to develop Accepted and waiting to be taken care of help wanted Extra attention is needed needs info Not enough information provided
Milestone
next

Comments

@joshtrichards
Copy link
Member

The app container's entrypoint currently only uses set in a couple spots. We should test impact of enabling set -eu globally at the top, just like in the community micro-services image here. It should be safe to use alongside the existing ones already in the AIO entrypoint.

This should prevent problems that lead to broken installations.

This originally came up because there was a forum post a few weeks I saw where rsync failed, yet the failure was not detected and the installation proceeded.

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...
@joshtrichards joshtrichards added the 0. Needs triage Pending approval or rejection. This issue is pending approval. label Feb 5, 2025
@joshtrichards joshtrichards changed the title Consider setting set -eu globally in nextcloud container entrypoint Consider setting set -eu globally in nextcloud container entrypoint to catch more errors Feb 5, 2025
@szaimen szaimen linked a pull request Feb 11, 2025 that will close this issue
Draft
@szaimen
Copy link
Collaborator

szaimen commented Feb 11, 2025 •
edited
Loading

Hey, I am honestly against adding this globally as things are not possible to easily debug anymore if we just exit anywhere (it will be put into an uncontrollable state). So I would only add this at places where it is necessary to run without any issue like for the rsync job. I created #6028. However it is not ready yet as we would probably need to think what to do if rsync actually fails: do we touch a file so that when the container restarts, we can print a specific warning or how do we handle this case? What would be your proposal?

@szaimen szaimen added 1. to develop Accepted and waiting to be taken care of help wanted Extra attention is needed needs info Not enough information provided and removed 0. Needs triage Pending approval or rejection. This issue is pending approval. labels Feb 11, 2025
@szaimen szaimen modified the milestones: next, v10.6.0 Feb 11, 2025
@szaimen szaimen modified the milestones: v10.6.1, next Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
1. to develop Accepted and waiting to be taken care of help wanted Extra attention is needed needs info Not enough information provided
Projects
None yet
Milestone
next
Development

Successfully merging a pull request may close this issue.

exit if rsync fails nextcloud/all-in-one
2 participants
@joshtrichards @szaimen