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

Enable log file support #672

Merged
merged 6 commits into from
Jul 31, 2024

Conversation

JenySadadia
Copy link
Collaborator

@JenySadadia JenySadadia commented Jul 3, 2024

Closes kernelci/kernelci-project#334

Enable multiple log streams for Logger.

Create a file handler to dump logs to files. Use stream and file handlers for all the pipeline services to redirect logs to both the streams including stdout and log file.
Update the logger config file.

@JenySadadia
Copy link
Collaborator Author

Staging seems working and creating log files for pipeline services:

kernelci@kernelci-staging:~/kernelci-deploy/checkout/kernelci-pipeline$ ls -lrt logs
total 68
-rw-r--r-- 1 kernelci kernelci  159 Jul  3 13:04 monitor_2024_07_03-13:03:55.log
-rw-r--r-- 1 kernelci kernelci  104 Jul  3 13:08 result_summary_2024_07_03-13:08:05.log
-rw-r--r-- 1 kernelci kernelci  164 Jul  3 13:10 test_report_2024_07_03-13:08:10.log
-rw-r--r-- 1 kernelci kernelci  159 Jul  3 13:10 monitor_2024_07_03-13:08:08.log
-rw-r--r-- 1 kernelci kernelci  642 Jul  3 13:10 timeout-closing_2024_07_03-13:08:10.log
-rw-r--r-- 1 kernelci kernelci  159 Jul  3 13:10 regression_tracker_2024_07_03-13:08:09.log
-rw-r--r-- 1 kernelci kernelci  175 Jul  3 13:10 timeout-holdoff_2024_07_03-13:08:10.log
-rw-r--r-- 1 kernelci kernelci  174 Jul  3 13:10 scheduler_2024_07_03-13:08:05.log
-rw-r--r-- 1 root     root      318 Jul  3 13:10 scheduler_2024_07_03-13:08:08.log
-rw-r--r-- 1 kernelci kernelci 8926 Jul  3 13:10 trigger_2024_07_03-13:08:10.log
-rw-r--r-- 1 kernelci kernelci 9832 Jul  3 13:10 timeout_2024_07_03-13:08:06.log
-rw-r--r-- 1 kernelci kernelci 1962 Jul  3 13:10 scheduler_2024_07_03-13:08:10.log
-rw-r--r-- 1 kernelci kernelci 1410 Jul  3 13:10 tarball_2024_07_03-13:08:10.log

@nuclearcat Let me know if the file naming convention needs to be tweaked.

@JenySadadia
Copy link
Collaborator Author

@nuclearcat We also need to add some kind of purge service that will discard the log files older than some decided number of days. Otherwise, It will create storage issues on VM.

@JenySadadia JenySadadia force-pushed the enable-log-file-support branch 2 times, most recently from ce9426c to 28092c3 Compare July 4, 2024 06:42
@JenySadadia JenySadadia requested a review from pawiecz July 5, 2024 09:01
Copy link
Contributor

@pawiecz pawiecz left a comment

Choose a reason for hiding this comment

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

I must be missing something obvious - please help me understand:

Currently config/logger.conf handles 11 services even though there are 16 overall [1]. Are the potentially missing ones (lava-callback, patchset, scheduler-{docker,k8s,lava}) already covered implicitly somehow?

I also don't get the convention for passing service name as an argument - it's used like that for schedulers, other services have it set by their constructor parameter or even hardcoded directly in the constructor (e.g. timeout-s: 1, 2, 3).

Any additional pointers will be much appreciated.

[1] Compose files in use for staging deployment converted to a single configuration file for inspection:

$ docker-compose -f docker-compose.yaml -f docker-compose-kcidb.yaml -f docker-compose-lava.yaml -f docker-compose-production.yaml config > staging.yaml

Jeny Sadadia added 6 commits July 29, 2024 13:05
Create a file handler to dump logs to files.
Use stream and file handlers for all the
pipeline services to redirect logs to both
the streams including `stdout` and log file.

Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>
As `notifier` service has been renamed to `monitor`,
update its logger configurations accordingly.
Also, add section for `result_summary` service.

Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>
Create a directory named `logs` to dump
log files to. Mount the directory for all
the services in `docker-compose.yaml`.

Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>
Fix the below error for `patchset` service:
```
FileNotFoundError: [Errno 2] No such file or directory: '/home/kernelci/logs/tarball_2024_07_04-06:24:16.log'
```

Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>
Add `name` docker argument to provide service name to
scheduler services. Without it, all 4 container i.e
`scheduler`, `scheduler-docker`, `scheduler-lava`, and
`scheduler-k8s` were using the same log file.
That was leading in-accessbility of log files and raised
the below error:
`PermissionError: [Errno 13] Permission denied:
'/home/kernelci/logs/scheduler_2024_07_04-06:42:21.log'`

Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>
Add logger config sections for remaining pipeline
services.

Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>
@JenySadadia
Copy link
Collaborator Author

Hey,

Currently config/logger.conf handles 11 services even though there are 16 overall [1]. Are the potentially missing ones (lava-callback, patchset, scheduler-{docker,k8s,lava}) already covered implicitly somehow?

Thanks for pointing this out. Whenever a logger section is not found with the particular name, it simply uses root configs.
Bdw I have added missing logger sections to the config file. For lava-callback, we need to integrate Logger module first.

I also don't get the convention for passing service name as an argument - it's used like that for schedulers, other services have it set by their constructor parameter or even hardcoded directly in the constructor (e.g. timeout-s: 1, 2, 3).

For timeout services, we have different class for all 3 of them. And that's why we can use static service name for them.
In the case of scheduler, it just has a single class implementation. So it will not be possible to identify which service (docker, lava, or k8s) invoked the class. That's why I introduced --name argument in docker-compose to use it to pass the service name.

Copy link
Contributor

@pawiecz pawiecz left a comment

Choose a reason for hiding this comment

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

LGTM

@JenySadadia JenySadadia added this pull request to the merge queue Jul 31, 2024
Merged via the queue into kernelci:main with commit 9522825 Jul 31, 2024
3 checks passed
@JenySadadia JenySadadia deleted the enable-log-file-support branch July 31, 2024 11:33
@nuclearcat
Copy link
Member

I was going to ask, is it going to work in kubernetes? as likely logs directory wont be created yet (it needs some changes in pipeline yaml specifications), as i planned production update today

@JenySadadia
Copy link
Collaborator Author

I was going to ask, is it going to work in kubernetes? as likely logs directory wont be created yet (it needs some changes in pipeline yaml specifications), as i planned production update today

Oh, sorry. I already merged it.
I will check k8s config files and get back to you.

@nuclearcat
Copy link
Member

Just make sure fallback scenario - if directory doesn't exist - it should not crash service, and in such case it just wont log to file, only to console, for example, so i can plan log volume properly during this week.

nuclearcat added a commit to nuclearcat/kernelci-pipeline that referenced this pull request Aug 7, 2024
PR kernelci#672 introduced
new mandatory option. Except docker-compose such options should
be added to k8s manifests too.

Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 7, 2024
PR #672 introduced
new mandatory option. Except docker-compose such options should
be added to k8s manifests too.

Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
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.

Dump logs of API and Pipeline services
3 participants