KEP-4786: direct cgroup status collection on Node

[linxiulei]

• One-line PR description: Direct cgroup stats collection on Node

• Issue link: Direct cgroup stats collection #4786

• Other comments:

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: linxiulei
Once this PR has been reviewed and has the lgtm label, please assign mrunalp for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• keps/sig-node/OWNERS

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

[kannon92]

/retitle KEP-4786: direct cgroup status collection on Node

[kannon92]

Image filesystem also.

[linxiulei]

Pls correct me if I'm wrong. I think imagefs is taken care of by CRI, no?

[kannon92]

It is not.

[linxiulei]

did you refer to https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/stats/cri_stats_provider.go#L389 called by https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/server/stats/summary.go#L85?

[kannon92]

Looking at that code, we are using cadvisor for Availability and Capacity.

[linxiulei]

Yes, but that cadvisor will be replaced to not use cadvisor

[iholder101]

So, IIUC, KEP-2371 is about implementing the missing bits from the CRI/CRI-API perspective and this KEP would actually remove the use of cadvisor wherever it's necessary?

[linxiulei]

Yes, but not entirely removing the use of cadvisor. More specifically, this KEP will remove the background task that runs cadvisor routines but still call cadvisor code on demand.

[pacoxu]

Does this KEP will cover both cgroup v1 and v2?

[haircommander]

it should only cover v2, as v1 is feature frozen

[linxiulei]

It does support both cgroup versions but it's not intentional. The implementation would take advantage of libraries that support collecting stats for both versions so it's agnostic to cgroup versions.

[haircommander]

I think something we have to be wary about this approach is how metrics are undocumented/unintended GA APIs in kubernetes. While it seems natural to exclude cgroup stats collection for all cgroups other than the ones kubelet is aware of, there are likely users who are relying on cadvisor collecting those stats and will be upset by us dropping them.

I wonder if the same performance gains can be made by adding a tunable in the kubelet configuration that allows a user to say which cgroups we collect metrics about. Then we could get data from users on whether they want the non-kube cgroups to have stats collected for them. WDYT @linxiulei

[linxiulei]

wonder if the same performance gains can be made by adding a tunable in the kubelet configuration that allows a user to say which cgroups we collect metrics about.

The cgroups we will drop in this KEP are individual Pods' cgroups. I am not sure how configurable they are since they are all under */kubepod/ path. Also adding a tunable for this KEP will significantly increase the complexity so I'd refrain doing so.

Then we could get data from users on whether they want the non-kube cgroups to have stats collected for them.

Alternatively, we can make this KEP an opt-in feature, so users who still want non-kube cgroup stats, they can opt out this feature or until they find an alternative to collect non-kube cgroup stats, which I genuinely think should not be part of kubelet.

[haircommander]

 The cgroups we will drop in this KEP are individual Pods' cgroups. I am not sure how configurable they are since they are all under */kubepod/ path. Also adding a tunable for this KEP will significantly increase the complexity so I'd refrain doing so.

pod cgroups like the pod slice, or the container scopes as well? theoretically, CRI stats KEP should cover the container scope piece (I don't think it does today but it should). if you're talking about the pod cgroup, then I still maintain there may be users relying on these metrics (along with any others we may be dropping, unfortunately)

[linxiulei]

Sorry for the confusion. Let me clarify, currently there are following cgroup stats collected by kubelet and cAdvisor

• Root cgroup
• All non-pod cgroups
  • Including non-pod cgroups as specified in --runtime-cgroups,
    --system-cgroups and --kubelet-cgroups
• Root cgroup for pods (e.g. /sys/fs/cgroup/kubepods.slice/)
• Pod cgroups

This KEP won't drop any of them. However, Pod cgroups are collected by cAdvisor and CRI stats KEP (if enabled) at the same time. Therefore, this KEP removes the cAdvisor's collection by collecting what CRI stats KEP is not yet collecting.

So after this KEP, kubelet collects

• Root cgroup
• All non-pod cgroups
  • Including non-pod cgroups as specified in --runtime-cgroups,
    --system-cgroups and --kubelet-cgroups
• Root cgroup for pods (e.g. /sys/fs/cgroup/kubepods.slice/)

(here what I meant dropping pod cgroups incorrectly)

And CRI stats collects

• Pod cgroups