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

Fix log rotation description in the logging doc #3918

Merged
merged 4 commits into from
May 31, 2017
Merged

Fix log rotation description in the logging doc #3918

merged 4 commits into from
May 31, 2017

Conversation

crassirostris
Copy link

@crassirostris crassirostris commented May 26, 2017

To address kubernetes/kubernetes#40634 (comment)


This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 26, 2017
@crassirostris
Copy link
Author

/cc @piosz @fgrzadkowski

Copy link
Contributor

@chenopis chenopis left a comment

Choose a reason for hiding this comment

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

Some minor suggested edits.

directory are rotated daily and based on the log size. However,
system component logs have a higher size retention: by default,
they can store up to 100MB.
directory should be rotated. In Kubernetes clusters brough up by
Copy link
Contributor

Choose a reason for hiding this comment

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

"brough" -> "brought"

Copy link
Author

Choose a reason for hiding this comment

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

ACK

they can store up to 100MB.
directory should be rotated. In Kubernetes clusters brough up by
`kube-up.sh` script, those logs are configured to be rotated by
`logrotate` tool daily or once the size exceeds 100MB.
Copy link
Contributor

Choose a reason for hiding this comment

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

add "the": "...the logrotate tool..."

Copy link
Author

Choose a reason for hiding this comment

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

ACK

system component logs have a higher size retention: by default,
they can store up to 100MB.
directory should be rotated. In Kubernetes clusters brough up by
`kube-up.sh` script, those logs are configured to be rotated by
Copy link
Contributor

Choose a reason for hiding this comment

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

add "the": "...the kube-up.sh script..."

Copy link
Author

Choose a reason for hiding this comment

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

ACK

When you run [`kubectl logs`](/docs/user-guide/kubectl/v1.6/#logs), as in the basic logging example, the kubelet on the node handles the request and reads directly from the log file, returning the contents in the response. Note that `kubectl logs` **only returns the last rotation**; you must manually extract prior rotations, if desired and cluster-level logging is not enabled.
An important consideration in node-level logging is implementing log rotation,
so that logs don't consume all available storage on the node. Kubernetes
up to 1.7 is not responsible for rotating logs, deployment tool should set it
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested edit: "Kubernetes versions less than 1.7 are not responsible for rotating logs, so a deployment tool should be set up to address that."

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to refer to a specific kubelet version? We'll have to update it every release.

Copy link
Author

Choose a reason for hiding this comment

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

It's still not responsible and it's unknown when/whether it'll be responsible

I'll try to make it clearer

Copy link
Author

Choose a reason for hiding this comment

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

Do we want to refer to a specific kubelet version? We'll have to update it every release.

When I was addressing this part, I forgot to refresh the page and this comment wasn't visible

I agree, it's better to remove the mention of the version altogether

An important consideration in node-level logging is implementing log rotation,
so that logs don't consume all available storage on the node. Kubernetes
up to 1.7 is not responsible for rotating logs, deployment tool should set it
up. For example, in Kubernetes clusters, deployed by `kube-up.sh` script,
Copy link
Contributor

Choose a reason for hiding this comment

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

add "the": "...the kube-up.sh script..."

Copy link
Author

Choose a reason for hiding this comment

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

ACK

so that logs don't consume all available storage on the node. Kubernetes
up to 1.7 is not responsible for rotating logs, deployment tool should set it
up. For example, in Kubernetes clusters, deployed by `kube-up.sh` script,
there is [`logrotate`](http://www.linuxcommand.org/man_pages/logrotate8.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

add "a": "...there is a logrotate tool..."

Copy link
Author

Choose a reason for hiding this comment

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

ACK

up to 1.7 is not responsible for rotating logs, deployment tool should set it
up. For example, in Kubernetes clusters, deployed by `kube-up.sh` script,
there is [`logrotate`](http://www.linuxcommand.org/man_pages/logrotate8.html)
tool configured to perform log rotations daily or once application's log
Copy link
Contributor

Choose a reason for hiding this comment

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

add "an": "...or once an application's log..."

Copy link
Author

Choose a reason for hiding this comment

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

ACK

up. For example, in Kubernetes clusters, deployed by `kube-up.sh` script,
there is [`logrotate`](http://www.linuxcommand.org/man_pages/logrotate8.html)
tool configured to perform log rotations daily or once application's log
file grows beyond 10MB. You can also set up container runtime to rotate logs
Copy link
Contributor

Choose a reason for hiding this comment

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

add "a": "You can also set up a container runtime..."

Copy link
Author

Choose a reason for hiding this comment

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

ACK

the basic logging example, the kubelet on the node handles the request and
reads directly from the log file, returning the contents in the response.
**Note:** if some external system has manipulated the log file, e.g.
logrotate performed rotation, only the content of the original log file
Copy link
Contributor

Choose a reason for hiding this comment

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

add "the": "...logrotate performed the rotation..."

Copy link
Author

Choose a reason for hiding this comment

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

ACK

@crassirostris
Copy link
Author

@chenopis I swear when I'm creating the PR the checkbox "Allow edits from maintainers" is checked, but after PR is created, it's unchecked. That's what I was missing, sorry

tool configured to perform log rotations daily or once application's log
file grows beyond 10MB. You can also set up container runtime to rotate logs
automatically, e.g. by using Docker's `log-opt` or by sending logs to the
system that does this for you, e.g. journald. As an example, you can find
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIU journald doesn't work with kubectl logs or /logs http handler. I wouldn't recommend it in our docs.

Copy link
Author

Choose a reason for hiding this comment

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

ACK

the basic logging example, the kubelet on the node handles the request and
reads directly from the log file, returning the contents in the response.
**Note:** if some external system has manipulated the log file, e.g.
logrotate performed rotation, only the content of the original log file
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean by "original"? I think it's unclear.

Copy link
Author

Choose a reason for hiding this comment

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

ACK

@fgrzadkowski fgrzadkowski self-assigned this May 29, 2017
@crassirostris
Copy link
Author

@chenopis @fgrzadkowski Addressed comments, PTAL

there is a [`logrotate`](http://www.linuxcommand.org/man_pages/logrotate8.html)
tool configured to perform log rotations daily or once an application's log
file grows beyond 10MB. You can also set up a container runtime to rotate logs
automatically, e.g. by using Docker's `log-opt`. As an example, you can find
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't we using Docker to rotate logs? I though we now use 'logrotate' only for logs from system components.

Copy link
Author

Choose a reason for hiding this comment

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

In COS on GCP, yes. Agree, considering also the link below, this part may be confusing. I'll address it, thanks

Copy link
Author

Choose a reason for hiding this comment

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

Done

@chenopis
Copy link
Contributor

@fgrzadkowski or @piosz does this have tech lgtm?

@fgrzadkowski
Copy link
Contributor

@chenopis Now it does :)

@piosz piosz removed their request for review May 31, 2017 10:25
@chenopis chenopis merged commit f4fd9e7 into kubernetes:master May 31, 2017
chenopis added a commit that referenced this pull request May 31, 2017
…hub.io into release-1.7

* 'master' of https://github.com/kubernetes/kubernetes.github.io:
  Fix log rotation description in the logging doc (#3918)
  Add colon to #3897
  add prompt about KUBE_REPO_PREFIX (#3597)
  Warn against managing ReplicaSets owned by Deployments
  Update small typo in components page (#3930)
  change the service name to pod ip (#3926)
chenopis added a commit that referenced this pull request May 31, 2017
…hub.io into release-1.6

* 'master' of https://github.com/kubernetes/kubernetes.github.io:
  networking.md: Add reference to the Cilium network plugin
  Fix typo in cluster api docs
  [Kubeadm] bugfix in the index listing
  Fix log rotation description in the logging doc (#3918)
  Add colon to #3897
  add prompt about KUBE_REPO_PREFIX (#3597)
  Warn against managing ReplicaSets owned by Deployments
  Update small typo in components page (#3930)
  change the service name to pod ip (#3926)
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants