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

CRI: Implement container log rotation #58823

Closed
yujuhong opened this Issue Jan 25, 2018 · 6 comments

Comments

Projects
None yet
5 participants
@yujuhong
Copy link
Member

yujuhong commented Jan 25, 2018

TL;DR: We want to allow kubelet to rotate the container stdout/stderr logs, and add a CRI call to ask the runtime to reopen the log file.

kubelet decides the log directory structure and passes to the runtime the path where container log should be stored. Given the level of knowledge and control kubelet has, it is currently the best candidate to rotate the logs (vs. runtime or a separate daemon). The detailed evaluation is in the proposal doc.

IMO, the initial version should target a simple policy of size limit per file. We can iterate and add more advanced features based on disk management in the future. The stretch goal for this release will be supporting reading logs (kubectl logs) from rotated log files.

As for the docker integration (i.e., --container-runtime=docker) that does not fully implement CRI logging, kubelet should skip log rotation.

/cc @kubernetes/sig-node-feature-requests @dchen1107 @Random-Liu @mrunalp @resouer @feiskyer

@yujuhong yujuhong added this to the v1.10 milestone Jan 25, 2018

@lichuqiang

This comment has been minimized.

Copy link
Member

lichuqiang commented Jan 26, 2018

/cc

k8s-github-robot pushed a commit that referenced this issue Jan 30, 2018

Kubernetes Submit Queue
Merge pull request #58899 from yujuhong/reopen-logs
Automatic merge from submit-queue (batch tested with PRs 58899, 58980). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

CRI: Add a call to reopen log file for a container

This allows a daemon external to the container runtime to rotate the log
file, and then ask the runtime to reopen the files.

**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 #58823

**Release note**:
```release-note
CRI: Add a call to reopen log file for a container. 
```
@yujuhong

This comment has been minimized.

Copy link
Member Author

yujuhong commented Jan 31, 2018

The CRI change has been merged. What's left to do is implementing in kubelet, and adding a test for it once we have a CRI runtime supporting the feature.

@yujuhong yujuhong reopened this Feb 1, 2018

@jberkus

This comment has been minimized.

Copy link

jberkus commented Feb 2, 2018

Hey, if the remaining work for this is going into 1.10, can you please add a priority label? I also don't see the remaining work on the Features board? If this isn't rolled into another feature, then you've missed feature freeze for this one ...

@idvoretskyi

This comment has been minimized.

Copy link
Member

idvoretskyi commented Feb 3, 2018

@yujuhong can you link this issue to the one in a features repo?

@yujuhong

This comment has been minimized.

Copy link
Member Author

yujuhong commented Feb 5, 2018

@yujuhong can you link this issue to the one in a features repo?

@jberkus @idvoretskyi a lot of these are ongoing work in the past few releases to bridge the gap between docker and non-docker runtimes using CRI. They are not used with docker (the only runtime with in-tree support), and are in general not user facing. If all milestone issues must link to a feature, hope this would help: kubernetes/enhancements#552

@yujuhong

This comment has been minimized.

Copy link
Member Author

yujuhong commented Feb 5, 2018

@Random-Liu is going to own the issue unless we find someone else with more bandwidth this release.

k8s-github-robot pushed a commit that referenced this issue Feb 23, 2018

Kubernetes Submit Queue
Merge pull request #59898 from Random-Liu/add-log-rotation
Automatic merge from submit-queue (batch tested with PRs 60214, 58762, 59898, 59897, 60204). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add CRI container log rotation support

Fixes #58823.

This PR:
1) Added `pkg/kubelet/logs/container_log_manager.go` which manages and rotates container logs.
2) Added a feature gate `CRIContainerLogRotation` to enable the alpha feature. And 2 kubelet flags `--container-log-max-size` and `--container-log-max-files` to configure the rotation behavior.
3) Added unit test and node e2e test for container log rotation.

Note that:
1) Container log manager only starts when the container runtime is `remote` (not docker), because we can't implement `ReopenContainerLog` for docker.
2) Rotated logs are compressed with `gzip`.
2) The latest rotated log is not compressed. Because fluentd may still be reading the file right after rotation.
3) `kubectl logs` still doesn't support log rotation. This is not a regression anyway, it doesn't support log rotation for docker log today. We'll probably fix this in the future. (Issue: #59902)

An example of container log directory with `--container-log-max-files=3`:
```console
$ ls -al /var/log/pods/57146449-11ec-11e8-90e1-42010af00002
total 592
drwxr-xr-x 2 root root   4096 Feb 15 01:07 .
drwxr-xr-x 3 root root  12288 Feb 15 01:06 ..
-rw-r----- 1 root root 176870 Feb 15 01:07 log-container_0.log
-rw-r--r-- 1 root root  40239 Feb 15 01:07 log-container_0.log.20180215-010737.gz
-rw-r----- 1 root root 365996 Feb 15 01:07 log-container_0.log.20180215-010747
```

/assign @mtaufen for the config change.
/assign @dashpole @crassirostris for the log change.
/assign @feiskyer for CRI related change.
/cc @yujuhong @feiskyer @abhi @mikebrow @mrunalp @runcom 
/cc @kubernetes/sig-node-pr-reviews @kubernetes/sig-instrumentation-pr-reviews 

**Release note**:

```release-note
[Alpha] Kubelet now supports container log rotation for container runtime which implements CRI(container runtime interface).
The feature can be enabled with feature gate `CRIContainerLogRotation`.
The flags `--container-log-max-size` and `--container-log-max-files` can be used to configure the rotation behavior.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.