stefanprodan / k8s-scw-baremetal

Kubernetes installer for Scaleway bare-metal AMD64 and ARMv7
MIT License
177 stars 58 forks source link

Setting up log-rotation #28

Closed stephenmoloney closed 5 years ago

stephenmoloney commented 6 years ago

Issue

I recently started to think about log handling and took a peek into the /var/log/pods folder and here is what I found:

root@node-1:~# du -sh -L /var/log/pods/
18G    /var/log/pods/

This is with just a few days usage of the cluster and not so many pods (about 20).

The issue is that there does not seem to be any default log-rotation.

Proposed solutions

  1. Introduce log rotation at the docker daemon level by modifying the daemon.json configuration file

  2. Introduce log rotation using a cronjob inside the k8s cluster

  3. Introduce log rotation in each node and master using logrotate. Some of the options within the logrotate could come from terraform vars too. Also, it could be entirely optional from a terraform var.

Option 1 has limited options but should be easy and reliable. Option 2 doesn't sound like a great idea. Option 3 sounds best to me.

@stefanprodan I can do some work on this if you think a PR is beneficial, what do you think?

stefanprodan commented 6 years ago

I would go with option 1, a PR would be great. Thanks @stephenmoloney

stephenmoloney commented 6 years ago

The only thing I don't like about option 1 is that it is limited to file size and number or files.

Eg

"log-opts": {
    "max-size": "1m",    
    "max-file": "3"    
    }

whereas logrotate, one can specify the rotation frequency as well.

Also, some of the kubernetes log files are not coming from docker container I believe.

stefanprodan commented 6 years ago

kubelet and dockerd are managed by systemd. I would go with option 1 since long rotate can break dockerd if it renames the file while a log is being written on disk.

stephenmoloney commented 6 years ago

ok, sure. Users can add their own implementations of logrotate if needed.

stephenmoloney commented 6 years ago

I came across this PR but couldn't figure out if it's something beneficial here.

https://github.com/kubernetes/kubernetes/pull/59898

stefanprodan commented 6 years ago

Those settings are being forwarded to docker engine, it's the same as setting the log rotation in docker config.

stephenmoloney commented 6 years ago

That would be a nice way of doing it possibly.

kubeadm init --apiserver-advertise-address=${self.private_ip} \
--apiserver-cert-extra-sans=${self.public_ip} \
--kubernetes-version=${var.k8s_version} \
--ignore-preflight-errors=KubeletVersion
--feature-gates=CRIContainerLogRotation=true

where k8s_version >= 1.11

Not sure where to add the two kubelet flags go?

--container-log-max-size=10m and --container-log-max-files=1

Thoughts?

stefanprodan commented 6 years ago

Those flags should go into the kubeadm CRD, I think we need to create a template that builds up the config yaml and not use kubeadm flags anymore.

stefanprodan commented 6 years ago

Here are the docs: https://kubernetes.io/docs/reference/setup-tools/kubeadm/kubeadm-init/#kubelet-drop-in

stephenmoloney commented 6 years ago

Ok, I'll put something together. Thanks.

stephenmoloney commented 5 years ago

Unless I'm mistaken, this is now a default behaviour since kubernetes 1.12 + and kubeadm 1.12 + AFAIK.

I've checked the configs and this is what it contains for kubelet-1.12.yaml since then:

apiVersion: kubelet.config.k8s.io/v1beta1
kind: KubeletConfiguration
containerLogMaxFiles: 5
containerLogMaxSize: 10Mi
maxPods: 110

From link, jan/2018 image

That said, I think there is a case to be made for adding the kubeadm CRD - even if it's just as a placeholder. I'm looking into it.

stephenmoloney commented 5 years ago

Re using kubeadm config See #34