Skip to content

Add log rotation scheduler job for the nginx logs #619

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

Merged
mishaschwartz merged 6 commits into from
Dec 16, 2025
Merged

Add log rotation scheduler job for the nginx logs #619

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
acda3ed
rotate proxy logs
mishaschwartz Dec 11, 2025
3f51a9f
add config options and warnings
mishaschwartz Dec 12, 2025
3061518
review suggestions
mishaschwartz Dec 15, 2025
0961f43
update tests
mishaschwartz Dec 16, 2025
eeb9268
review suggestions
mishaschwartz Dec 16, 2025
3ad5beb
Bump version: 2.20.0 → 2.20.1
mishaschwartz Dec 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .bumpversion.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tool.bumpversion]
current_version = "2.20.0"
current_version = "2.20.1"
commit = true
tag = false
tag_name = "{new_version}"
Expand Down
24 changes: 24 additions & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,30 @@

[//]: # (list changes here, using '-' for each new entry, remove this when items are added)

[2.20.1](https://github.com/bird-house/birdhouse-deploy/tree/2.20.1) (2025-12-16)
------------------------------------------------------------------------------------------------------------------

## Fixes

- Add log rotation scheduler job for the nginx logs

Currently the nginx logs are not rotated so they can build up to quite a large size.
Copy link
Collaborator

@tlvu tlvu Dec 15, 2025

Choose a reason for hiding this comment

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

It's not just a large size it's also the hogging of CPU and RAM to parse that gigantic large log file on restart.

Since it's a regression, please also mention when the regression started "Broken since 2.3.0 (#452)."

Copy link
Collaborator

@tlvu tlvu Dec 16, 2025

Choose a reason for hiding this comment

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

Sorry I wasn't clear, please also add the part about "hogging of CPU and RAM to parse that gigantic large log file on restart."


Previously, they were rotated by the `canarie-api` component but that is no longer a required
component and CanarieAPI hasn't handled log rotation since CanarieAPI version 1.0.0 (see
https://github.com/bird-house/birdhouse-deploy/pull/452 for details).

Fixes https://github.com/bird-house/birdhouse-deploy/issues/593. Here is a summary of the issue:

- CanarieApi parses the log file every minute starting from the beginning of the file
- If the log file is really big it uses a lot of CPU and memory to read through the whole file
- This would use a lot less CPU and memory if the log file was smaller

This quick and least disruptive fix to get the production server out of the water should be a
temporary solution until a better solution using container STDOUT parsing is implemented for
the CanarieAPI and prometheus-log-parser (https://github.com/bird-house/birdhouse-deploy/issues/618).
Then we can deprecate this scheduler job.

[2.20.0](https://github.com/bird-house/birdhouse-deploy/tree/2.20.0) (2025-12-10)
------------------------------------------------------------------------------------------------------------------

Expand Down
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ override BIRDHOUSE_MAKE_DIR := $(shell realpath -P $$(dirname $(BIRDHOUSE_MAKE_C
# Generic variables
override SHELL := bash
override APP_NAME := birdhouse-deploy
override APP_VERSION := 2.20.0
override APP_VERSION := 2.20.1

# utility to remove comments after value of an option variable
override clean_opt = $(shell echo "$(1)" | $(_SED) -r -e "s/[ '$'\t'']+$$//g")
Expand Down
8 changes: 4 additions & 4 deletions README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ for a full-fledged production platform.
* - citation
- | |citation|

.. |commits-since| image:: https://img.shields.io/github/commits-since/bird-house/birdhouse-deploy/2.20.0.svg
.. |commits-since| image:: https://img.shields.io/github/commits-since/bird-house/birdhouse-deploy/2.20.1.svg
:alt: Commits since latest release
:target: https://github.com/bird-house/birdhouse-deploy/compare/2.20.0...master
:target: https://github.com/bird-house/birdhouse-deploy/compare/2.20.1...master

.. |latest-version| image:: https://img.shields.io/badge/tag-2.20.0-blue.svg?style=flat
.. |latest-version| image:: https://img.shields.io/badge/tag-2.20.1-blue.svg?style=flat
:alt: Latest Tag
:target: https://github.com/bird-house/birdhouse-deploy/tree/2.20.0
:target: https://github.com/bird-house/birdhouse-deploy/tree/2.20.1

.. |readthedocs| image:: https://readthedocs.org/projects/birdhouse-deploy/badge/?version=latest
:alt: ReadTheDocs Build Status (latest version)
Expand Down
2 changes: 1 addition & 1 deletion RELEASE.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.20.0 2025-12-10T22:06:17Z
2.20.1 2025-12-16T18:09:59Z
8 changes: 4 additions & 4 deletions birdhouse/components/canarie-api/docker_configuration.py.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ SERVICES = {
# NOTE:
# Below version and release time auto-managed by 'make VERSION=x.y.z bump'.
# Do NOT modify it manually. See 'Tagging policy' in 'birdhouse/README.rst'.
'version': '2.20.0',
'releaseTime': '2025-12-10T22:06:17Z',
'version': '2.20.1',
'releaseTime': '2025-12-16T18:09:59Z',
'institution': '${BIRDHOUSE_INSTITUTION}',
'researchSubject': '${BIRDHOUSE_SUBJECT}',
'supportEmail': '${BIRDHOUSE_SUPPORT_EMAIL}',
Expand Down Expand Up @@ -141,8 +141,8 @@ PLATFORMS = {
# NOTE:
# Below version and release time auto-managed by 'make VERSION=x.y.z bump'.
# Do NOT modify it manually. See 'Tagging policy' in 'birdhouse/README.rst'.
'version': '2.20.0',
'releaseTime': '2025-12-10T22:06:17Z',
'version': '2.20.1',
'releaseTime': '2025-12-16T18:09:59Z',
'institution': '${BIRDHOUSE_INSTITUTION}',
'researchSubject': '${BIRDHOUSE_SUBJECT}',
'supportEmail': '${BIRDHOUSE_SUPPORT_EMAIL}',
Expand Down
6 changes: 6 additions & 0 deletions birdhouse/components/proxy/default.env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,9 @@ export OPTIONAL_VARS="
\$PROXY_LISTEN_443_PARAMS
\$PROXY_LISTEN_80_PARAMS
"

# TODO: remove this dependency once https://github.com/bird-house/birdhouse-deploy/issues/618 is resolved
COMPONENT_DEPENDENCIES="
./components/scheduler
./optional-components/scheduler-job-logrotate-nginx
"
2 changes: 2 additions & 0 deletions birdhouse/optional-components/scheduler-job-logrotate-nginx/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
config.yml
nginx.logrotate
12 changes: 12 additions & 0 deletions birdhouse/optional-components/scheduler-job-logrotate-nginx/config.yml.template
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
- name: logrotate-nginx
comment: Rotate log files for nginx
schedule: '${SCHEDULER_JOB_LOGROTATE_NGINX_JOB_SCHEDULE}'
command: bash -c 'cp /etc/logrotate.conf.orig /etc/logrotate.conf && chown root:root /etc/logrotate.conf && chmod 644 /etc/logrotate.conf && /usr/sbin/logrotate -v /etc/logrotate.conf'
dockerargs: >-
--rm --name logrotate-nginx
--volume ${COMPOSE_PROJECT_NAME}_proxy-logs:/var/log/nginx/:rw
--volume ${COMPOSE_DIR}/optional-components/scheduler-job-logrotate-nginx/nginx.logrotate:/etc/logrotate.conf.orig:ro
trigger:
- command: sh -c '[ -f /var/run/nginx.pid ] && kill -USR1 $(cat /var/run/nginx.pid)'
container: proxy
image: 'stakater/logrotate:3.13.0'
7 changes: 7 additions & 0 deletions ...e/optional-components/scheduler-job-logrotate-nginx/config/proxy/docker-compose-extra.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
services:
proxy:
volumes:
- proxy-logs:${PROXY_LOG_DIR}

volumes:
proxy-logs:
4 changes: 4 additions & 0 deletions ...tional-components/scheduler-job-logrotate-nginx/config/scheduler/docker-compose-extra.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
services:
scheduler:
volumes:
- ./optional-components/scheduler-job-logrotate-nginx/config.yml:/scheduler-job-configs/logrotate-nginx.yml:ro
13 changes: 13 additions & 0 deletions birdhouse/optional-components/scheduler-job-logrotate-nginx/default.env
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export SCHEDULER_JOB_LOGROTATE_NGINX_JOB_SCHEDULE="* * * * *" # UTC
export SCHEDULER_JOB_LOGROTATE_NGINX_ROTATE=150
export SCHEDULER_JOB_LOGROTATE_NGINX_SIZE=1M

VARS="
$VARS
\$SCHEDULER_JOB_LOGROTATE_NGINX_JOB_SCHEDULE
\$SCHEDULER_JOB_LOGROTATE_NGINX_ROTATE
\$SCHEDULER_JOB_LOGROTATE_NGINX_SIZE
"

# TODO: deprecate this component once https://github.com/bird-house/birdhouse-deploy/issues/618 is resolved
Copy link
Collaborator

@tlvu tlvu Dec 16, 2025

Choose a reason for hiding this comment

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

Add a note to keep the .gitignore on deprecation, else will break autodeploy on existing deployment because those ignore files will become uncommitted files if the .gitignore is removed.

# remember that deprecating involves adding an entry to optional-components/.gitignore
11 changes: 11 additions & 0 deletions birdhouse/optional-components/scheduler-job-logrotate-nginx/nginx.logrotate.template
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/var/log/nginx/${PROXY_LOG_FILE} {
Copy link
Collaborator

@tlvu tlvu Dec 15, 2025

Choose a reason for hiding this comment

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

Why not just simply *.log as the default built-in logrotate config from the Nginx image? Any particular reason to restrict logrotation to only one single file? I understand right now we only need for this file but why not already provide logrotation for all future files? Especially when it is so simple?

Not a blocking concern, I am just being curious.

Copy link
Collaborator Author

@mishaschwartz mishaschwartz Dec 15, 2025

Choose a reason for hiding this comment

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

access.log and error.log both exist in that directory and both are symlinks to /dev/stdout and /dev/stderr so logrotate is unable to rotate them since they are not regular files.

I'm not sure what other future files we should be considering here? Especially since we're planning on moving to logging to stdout and stderr only in the future.

Copy link
Collaborator

@tlvu tlvu Dec 16, 2025

Choose a reason for hiding this comment

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

Yes right now no other candidate to logrotate but I usually prefer to avoid locking myself and future proof myself (in general, I understand this case is special since it's a temporary fix), especially when it's very easy and there are no bad impact.

So the reverse question is what mistake are we trying to prevent by locking this logrotate to one single file?

Copy link
Collaborator Author

@mishaschwartz mishaschwartz Dec 16, 2025

Choose a reason for hiding this comment

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

logrotate shows warnings when you tell it to rotate a stream. warnings warn us when we are doing something that the software doesn't like. I would prefer to not intentionally do something that raises warnings.

PROXY_LOG_FILE is configurable. You can change the filename to anything you want and it doesn't have to have a .log extension so changing the file name could break log rotation if we're just rotating *.log files.

Copy link
Collaborator

@tlvu tlvu Dec 16, 2025

Choose a reason for hiding this comment

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

No I do not meant to change the value of PROXY_LOG_FILE, I meant to already be ready for other *.log files. Anyways, forget this point. This is meant to be a temporary solution so hopefully you can implement the stream parsing in not too long so we don't need this PR anymore.

missingok
# https://github.com/Ouranosinc/CanarieAPI/blob/cc0ae59231ee4b58a34571bd12097c660aefb2e3/canarieapi/logparser.py#L15
rotate ${SCHEDULER_JOB_LOGROTATE_NGINX_ROTATE}
size ${SCHEDULER_JOB_LOGROTATE_NGINX_SIZE}
compress
delaycompress
notifempty
# log files in the proxy container are owned by root currently
create 640 root root
}
4 changes: 2 additions & 2 deletions docs/source/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@
# built documents.
#
# The short X.Y version.
version = '2.20.0'
version = '2.20.1'
# The full version, including alpha/beta/rc tags.
release = '2.20.0'
release = '2.20.1'

# The language for content autogenerated by Sphinx. Refer to documentation
# for a list of supported languages.
Expand Down
4 changes: 4 additions & 0 deletions tests/unit/test_read_configs_include.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ class TestReadConfigs(TestReadBasicConfigs):
]

default_all_conf_order_with_dependencies: list[str] = [
# TODO: remove the scheduler dependencies once
# https://github.com/bird-house/birdhouse-deploy/issues/618 is resolved
"./components/scheduler",
"./optional-components/scheduler-job-logrotate-nginx",
"./components/proxy",
"./components/magpie",
"./components/twitcher",
Expand Down
Loading