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

feat(entrypoint): add debug mode for entrypoint #2311

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

joshtrichards
Copy link
Member

@joshtrichards joshtrichards commented Oct 10, 2024

This PR does two things:

  • Adds a new environment variable (IMAGE_DEBUG) that toggles on a verbose/debug mode for key image functions (entrypoint, install, rsync)
  • Adjusts the retry output slightly to be clearer about when retry will start / has started

Details:

  • Adds a new environment variable that toggles on verbose/debug output for key image functions
    • the entrypoint.sh itself
    • occ maintennace:install
    • rsync
    • Note: occ upgrade didn't need it because it's already fairly verbose by default
  • Adjusts the retry output to be clearer about when retry will start / has started
    • Adjusts initial message to tell operator it'll be retried in 10s
    • Adds a message when 10s timer has expired to make clear when retry attempt has started for real (since I noticed that in normal - i.e. non-IMAGE_DEBUG mode - the install retry just runs again silently so its unclear it's actually retried until the install completes)
    • Adds retry count to output

New retry output:

test-nc30-fpm-subdirectory-app-1    | Nextcloud installation failed; will retry in 10s...
[...]
test-nc30-fpm-subdirectory-db-1     | 2024-10-16 14:14:41 0 [Note] mariadbd: ready for connections.
test-nc30-fpm-subdirectory-db-1     | Version: '10.6.19-MariaDB-ubu2004-log'  socket: '/usr/local/run/mysql.sock'  port: 0  mariadb.org binary distribution
test-nc30-fpm-subdirectory-app-1    | Retrying nextcloud install now... (1 of 10 attempts)
test-nc30-fpm-subdirectory-app-1    | Nextcloud was successfully installed
[...]

TODO:

  • sh debugging / verbose mode
  • occ maintenance:install debugging / verbose mode
  • hide all behind a toggle config variable
  • anything else?
    • rsync
    • occ upgrade (only adds timestamps so not necessary)
    • other occ calls (other than the simple ones)?
  • docs: README coverage of config variable that toggles this feature on
  • push latest changes
  • request review

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards

This comment was marked as resolved.

Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards requested a review from J0WI October 16, 2024 14:31
@joshtrichards
Copy link
Member Author

Cc: @tzerber You might find this of interest too

@tzerber
Copy link
Contributor

tzerber commented Oct 16, 2024

Cc: @tzerber You might find this of interest too

I do like it, neat. Would also help debugging fresh installs like the one we had few weeks(months? can't remember) ago where upon fresh install on slower hardware it was reporting weird errors and the only solution was to down/up the container. Good job!

@J0WI
Copy link
Contributor

J0WI commented Oct 24, 2024

This would be an option would work exclusively on the Nextcloud image.
It has almost the same effect as docker run nextcloud sh -eux /entrypoint.sh apache2-foreground but this works on any other image from the official library.

@joshtrichards
Copy link
Member Author

joshtrichards commented Oct 27, 2024

It has almost the same effect as

@J0WI Well, it also turns on verbose mode for rsync and the installer (added in v28: see nextcloud/server#41214) too.

Perhaps verbose mode for the installer should on by default in the entrypoint (with or without this PR). After all, updating is already fairly verbose. The installer just isn't by default. It's a one-off event so maybe that's the approach there.

Verbose mode for rsync could be helpful in troubleshooting situations that arise with folks with whacky NFS storage/etc. (seems to arise with k8s/helm users most often), but having it on by default doesn't make sense.

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.

3 participants