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 #263: Implement log rotation for server/error logs in docker-zulip #473

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

Conversation

savaliyabhargav
Copy link

This PR implements proper log rotation for server and error logs in docker-zulip.

Changes made:

  • Added logrotate configuration
  • Updated supervisor configuration
  • Modified entrypoint.sh to handle log rotation setup
  • Updated docker-compose.yml with proper volume mounts

Testing:

  • Logs are now properly rotated daily
  • Maximum file size is enforced
  • Old logs are compressed and maintained for 7 days
  • Supervisor properly reopens log files after rotation

Fixes #263

supervisorctl reopen-logs
endscript
}
EOF
Copy link
Member

Choose a reason for hiding this comment

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

Zulip ships with a logrotate configuration file; I don't think we should need to provide this in the container image as well, as zulip itself should have installed it already. Would be good to avoid duplicating, as the copy here will get stale.

RUN apt-get update && apt-get install -y \
logrotate \
cron \
&& rm -rf /var/lib/apt/lists/*
Copy link
Member

Choose a reason for hiding this comment

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

We install dependencies via puppet, you shouldn't need to add anything here. I expect these files are already present, and the actual bug is needing to actually run logrotate via supervisord (like how we do cron), since it likely is installed but not running.

@savaliyabhargav
Copy link
Author

@timabbott Sir, I would like to make another attempt at resolving this issue. Kindly assign it to me once again. Additionally, I seek your guidance in solving the problem and improving my solution. Please provide suggestions on how I can refine my approach. I am confident that I can resolve this issue successfully.

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.

Figure out why log rotation for server/error logs do not work in docker-zulip
2 participants