enabling log rotate for api logs in nginx #56979
Conversation
k8s-ci-robot
added
release-note
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
|
@hyperbolic2346 Please squash your commits. Thanks. |
… the nginx directory where the log rotation is setup. Adding a log rotation file for these logs.
|
@dixudx Done, thanks for the help. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/approve no-issue 👍 |
k8s-ci-robot
added
the
approved
| @@ -89,6 +109,14 @@ def close_old_port(): | |||
| hookenv.log('Port %d already closed, skipping.' % old_port) | |||
|
|
|||
|
|
|||
| def maybe_write_apilb_logrotate_config(): | |||
There was a problem hiding this comment.
Choice message signature :) 👍
| @@ -35,6 +36,25 @@ | |||
| from subprocess import CalledProcessError | |||
|
|
|||
|
|
|||
| apilb_nginx = """/var/log/nginx.*.log { | |||
There was a problem hiding this comment.
question:
Why declare this inline instead of using the template system that exists and is used elsewhere? I'm not generally +/- for this approach, just curious why you chose the approach you did.
There was a problem hiding this comment.
I'm not +/- either way and mainly just selected this method randomly. I'd be happy to convert it over, but I felt that it would be more obvious to someone reading the code what was happening if they could easily find that data. It probably isn't worth it and should be a template though.
There was a problem hiding this comment.
Good enough to satisfy my curiosity. Thanks for the explanation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chuckbutler, Cynerva, hyperbolic2346, marcoceppi Associated issue requirement bypassed by: Cynerva The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/retest Review the full test history for this PR. |
|
Automatic merge from submit-queue (batch tested with PRs 56858, 57040, 56979, 57051, 57052). If you want to cherry-pick this change to another branch, please follow the instructions here. |
What this PR does / why we need it:
Prevent the load balancer from filling the disk with logs from the api server
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
https://github.com/juju-solutions/bundle-canonical-kubernetes/issues/449
Special notes for your reviewer:
Release note: