[WINC-1979] Implement log rotation for windows-exporter - #4359
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wgahnagl The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@code-rabbit full review |
| windowsExporterServiceCommand := logconfig.GetKubeLogRunnerCmd(windows.KubeLogRunnerPath, | ||
| windows.WindowsExporterPath, windows.WindowsExporterLogPath) | ||
| windowsExporterServiceCommand = fmt.Sprintf("%s --collectors.enabled "+ | ||
| "cpu,logical_disk,net,os,service,system,container,memory,cpu_info --web.config.file %s --log.level %s --log.file stdout", |
There was a problem hiding this comment.
do we need to send the logs to stdout if they are going to a log file?
whats the benefit of doing this and whats the pattern for other services?
There was a problem hiding this comment.
Adding this stdout flag doesn't technically do anything, it just lets you know that the loging is getting sent to stdout for kube-log-runner to capture
There was a problem hiding this comment.
If its not doing anything I would skip it and follow the pattern for rest of the services
| if debug { | ||
| windowsExporterLogLevel = "debug" | ||
| } | ||
| windowsExporterServiceCommand := logconfig.GetKubeLogRunnerCmd(windows.KubeLogRunnerPath, |
There was a problem hiding this comment.
- what is the default log max file size for kubelogrunner at which the logs will start rotating?
- There is a env variable added by Jose to pass user defined log file size, will that be taken into account for windows exporter?
There was a problem hiding this comment.
this is where that is all set
it uses the env variable added by Jose because it uses the GetKubeLogRunnerCmd function which populates all of that by default. This also implements log rotation by using that command.
The default max log file is set in that function as well
| Managed Windows services with log rotation capabilities: | ||
| - kubelet | ||
| - kube-proxy | ||
| - windows_exporter |
There was a problem hiding this comment.
Is this PR implementing log rotation as well?
There was a problem hiding this comment.
yes! KubeLogRunner handles that. It rotates when the max size is reached.
|
@code-rabbit review |
|
@code-rabbit full review |
|
/test aws-e2e-operator |
| windowsExporterServiceCommand := logconfig.GetKubeLogRunnerCmd(windows.KubeLogRunnerPath, | ||
| windows.WindowsExporterPath, windows.WindowsExporterLogPath) | ||
| windowsExporterServiceCommand = fmt.Sprintf("%s --collectors.enabled "+ | ||
| "cpu,logical_disk,net,os,service,system,container,memory,cpu_info --web.config.file %s --log.level %s --log.file stdout", |
There was a problem hiding this comment.
If its not doing anything I would skip it and follow the pattern for rest of the services
|
/test unit |
|
@wgahnagl: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Adds log rotation for windows-exporter
This allows windows-exporter logs to be managed by kube-log-runner, and rotated automatically.