Skip to content

Instantly share code, notes, and snippets.

@mcastelino
Forked from egernst/top-failure.md
Created April 11, 2019 00:52
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save mcastelino/81b95a903724129099a7dbb94b4165ae to your computer and use it in GitHub Desktop.
Save mcastelino/81b95a903724129099a7dbb94b4165ae to your computer and use it in GitHub Desktop.

In the not-dockershim and not-CRIO normal socket path case, we are handled by the cri stats provider: https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/stats/cri_stats_provider.go

The 'magic' happens in the listPodStats function

Looping over each managed container, kubelet calculates the container statics at https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/stats/cri_stats_provider.go#L198, then calculate a running total of the pod usage at https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/stats/cri_stats_provider.go#L200, and then eventually return the results.

Potential issue

Initial potential issue I noticed was that we will run into is at the top of addPodCPUMemoryStats function. The default action is to try to grab out the podCgroupInfo from cadvisor: https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/stats/cri_stats_provider.go#L449-L456. If this is available, we simply return. Of not, we go ahead and create running total for each of the containers' usages This could be okay, assuming everything is sitting appropriately in the pod cgroup. I guess this is a chance where Kubelet can 'fast path' and not need to do the pod usage += container. OK optimization, assuming podcgroup is accurate, though AFAICT it'll still go through and grab these stats from CAdvisor "N" times, where N is the number of containers in the pod... not sure which is more expensive, a few CAdvisor calls or a couple of additions.

Bigger issue

Second bigger issue occurs at the tail end of loop over all of the managed containers: "If cadvisor stats is available for the container, use it to populate container stats". Umm; we just did a calculation in makeContainerStats and now, 8 lines later, we overwrite the CPU and Memory values in the addCadvisorContainerStats call:

		// If cadvisor stats is available for the container, use it to populate
		// container stats
		caStats, caFound := caInfos[containerID]
		if !caFound {
			klog.V(5).Infof("Unable to find cadvisor stats for %q", containerID)
		} else {
			p.addCadvisorContainerStats(cs, &caStats)
		}
		ps.Containers = append(ps.Containers, *cs)

https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/stats/cri_stats_provider.go#L202-L210

This is definitely an issue, though it could be introduced by changes in Kata. We need to understand how a container is marked to say "cadvisor stats is available".

In our current scenario, the accurate value is overwritten with zero, resulting in kubectl top pod --containers=true always reporting zero.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment