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

expand node-level log rotation options #14929

Closed
wants to merge 2 commits into from

Conversation

jaypipes
Copy link
Contributor

Removes references to the deprecated /cluster scripts from the logging
documentation, adding in sectional breakdown for log rotation setup
using either logrotate or configuring the Docker container runtime
logging driver.

Related to k/k Issue #78995.

Development docs for logging have moved to sig-instrumentation.
Removes references to the deprecated `/cluster` scripts from the logging
documentation, adding in sectional breakdown for log rotation setup
using either `logrotate` or configuring the Docker container runtime
logging driver.

Related to k/k Issue #78995.
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 16, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign chenopis
You can assign the PR to them by writing /assign @chenopis in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Jun 16, 2019
@k8s-ci-robot k8s-ci-robot requested review from piosz and x13n June 16, 2019 16:49
@netlify
Copy link

netlify bot commented Jun 16, 2019

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 484ea38

https://deploy-preview-14929--kubernetes-io-master-staging.netlify.com

### Use container runtime logging rotation

You can also set up a container runtime to rotate an application's logs
automatically, e.g. by using Docker's `log-opt`.
Copy link
Contributor

Choose a reason for hiding this comment

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

possible edit, remove: , e.g.

`/etc/docker/daemon.json`:

```
{
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add json as code block language


{{< note >}}
Currently, if some external system has performed the rotation,
only the contents of the latest log file will be available through
`kubectl logs`. E.g. if there's a 10MB file, `logrotate` performs
`kubectl logs`. e.g. if there's a 10MB file, `logrotate` performs
the rotation and there are two files, one 10MB in size and one empty,
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you substitute e.g. with For example, if there is a ...
Do you know why an empty file is created in this case? The example is not completely clear to me, unless the empty file is created after a max-size was reached and subsequently logrotate performed?


{{< note >}}
By default, Kubernetes will store a Pod's logs in
`/var/log/pods/{NAMESPACE_NAME_UID}/{POD}.log`. Kubernetes Services, in
Copy link
Contributor

Choose a reason for hiding this comment

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

possible edit: By default, Kubernetes stores Pod log files in /var/log/pods/{NAMESPACE_NAME_UID}/{POD}.log, and Service log files in /var/log/{SERVICE_NAME}.log.

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

I like the fixup aspect of this change request.

However, I am wary about adding a new section Use container runtime logging rotation
It would be OK to describe that in a Task page; perhaps inside /docs/tasks/administer-cluster/cluster-management/?

I'm also concerned that the revised page isn't telling the whole story. There's more to how to manage and rotate Pods' logs. The documentation should at some point mention kubelet flags --container-log-max-size and --container-log-max-files. This could be either in a Task page or in a page that explains logging conceptually (or maybe both).

It's good to let cluster operators know about options. For example if there were a Task page on this it could have tabs for different options. Select one tab and learn about using logrotate via a DaemonSet; another tab can explain logrotate with no DaemonSet, and a third tab can cover CRI container log rotation.

Lots of cluster operators use Docker to run containers and Docker doesn't seem to work with CRIContainerLogRotation. With that in mind the logrotate advice is relevant and I like the idea of making sure that advice is accurate as well.

@jaypipes, would you be willing to narrow the scope of this PR and then follow it up with more improvements? I'm also keen to hear your thoughts.

@jaypipes
Copy link
Contributor Author

@sftim @kbhawkey thanks much for the great feedback! I'll update the patch a little later this evening.

@jaypipes
Copy link
Contributor Author

jaypipes commented Jul 3, 2019

Gonna close this out and submit the link fix separately. I'll do a separate PR that carves out a Task page for logging that has the updated information and removes the references to /cluster scripts.

@jaypipes jaypipes closed this Jul 3, 2019
@jaypipes
Copy link
Contributor Author

jaypipes commented Jul 3, 2019

Separate link fix here: #15270

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants