tethysplatform / tethys

The Tethys Platform main Django website project repository.
http://tethysplatform.org/
BSD 2-Clause "Simplified" License
92 stars 50 forks source link

BUG - NGINX and Tethys Logs not output to container logs like we expect #1048

Closed swainn closed 2 months ago

swainn commented 5 months ago

The run.sh ends with a tail follow of supervisor, nginx, and tethys logs, but only the supervisor logs seem to be coming through in kubernetes logs for the container.

https://github.com/tethysplatform/tethys/blob/9ec09bd229a788a65a2b61d89b8786236f64ba61/docker/run.sh#L110

swainn commented 5 months ago

It's probably b/c this supervisor call is blocking: https://github.com/tethysplatform/tethys/blob/9ec09bd229a788a65a2b61d89b8786236f64ba61/docker/run.sh#L104

I never see the Watching Logs output in the text.

gronka commented 3 months ago

@swainn You're correct. Running supervisord in the foreground is the recommended way to execute it inside a container, since as a daemon the container will exit as there is no long-lived process. The tail command can be the long-lived process, however.

I was able to allow the tail command to run by extending the container with a new container with the line:

RUN sed -i '/usr\/bin\/supervisord/c\  /usr/bin/supervisord' ${TETHYS_HOME}/run.sh

This removes the no_daemon = true part.

However, there is a hiccup - running tail on /var/log/tethys/* will fail because passing an asterisk to tail does not work for a nonexistent directory. It seems like the /var/log/tethys directory is created almost immediately, so sleep can work.

Hotfix in Dockerfile:
RUN sed -i '/usr\/bin\/supervisord/c\  /usr/bin/supervisord\n  sleep 10s' run.sh

I don't know how much shorter the sleep can safely be.

Solution A in run.sh:
  /usr/bin/supervisord
  sleep 10s

It would be better to create the directories with the correct permissions, and remove sleep 10s. It seems like the correct permission is root:root, but maybe someone with more experience can validate that.

Solution B in run.sh:
  mkdir /var/log/tethys
  /usr/bin/supervisord
  {original tail line}

Finally, I don't like the current method of using an asterisk, because log rotation will generate a frenzy of output. And if we specify specific files, we don't need to create the directory beforehand:

Solution C in run.sh (these filenames might be incorrect - my vpn is down and we are using apache):
  /usr/bin/supervisord
  tail -qF \
    /var/log/supervisor/supervisord.log \
    /var/log/nginx/access.log \
    /var/log/nginx/error.log \
    /var/log/tethys/tethys.log

I like C the most. Let me know if you want me to validate the filenames and create a PR or if more discussion needs to occur.

swainn commented 3 months ago

Hi @gronka thank you for looking into this. I agree that Solution A is suboptimal and of Solutions B and C, I think I prefer C. Please prepare a PR with Solution C. I believe you have all of the important logs included in your example, but I'll take a close look when I review the PR.

swainn commented 2 months ago

Fixed by #1066