WIP: investigate kubelet stats slowdown

pkg/kubelet/cm/cgroup_manager_linux.go

@@ -101,7 +101,7 @@ func ParseSystemdToCgroupName(name string) CgroupName {
 	driverName := path.Base(name)
 	driverName = strings.TrimSuffix(driverName, systemdSuffix)
 	parts := strings.Split(driverName, "-")
-	result := []string{}
+	result := make([]string, 0, len(parts))
 	for _, part := range parts {
 		result = append(result, unescapeSystemdCgroupName(part))
 	}

pkg/kubelet/server/stats/handler.go

@@ -78,8 +78,6 @@ type Provider interface {
 	// GetPodByName returns the spec of the pod with the name in the specified
 	// namespace.
 	GetPodByName(namespace, name string) (*v1.Pod, bool)
-	// GetNode returns the spec of the local node.
-	GetNode() (*v1.Node, error)
 	// GetNodeConfig returns the configuration of the local node.
 	GetNodeConfig() cm.NodeConfig
 	// ListVolumesForPod returns the stats of the volume used by the pod with

pkg/kubelet/server/stats/summary.go

@@ -69,12 +69,6 @@ func NewSummaryProvider(statsProvider Provider) SummaryProvider {
 }
 
 func (sp *summaryProviderImpl) Get(ctx context.Context, updateStats bool) (*statsapi.Summary, error) {
-	// TODO(timstclair): Consider returning a best-effort response if any of
-	// the following errors occur.
-	node, err := sp.provider.GetNode()
-	if err != nil {
-		return nil, fmt.Errorf("failed to get node info: %v", err)
-	}
 	nodeConfig := sp.provider.GetNodeConfig()
 	rootStats, networkStats, err := sp.provider.GetCgroupStats("/", updateStats)
 	if err != nil {
@@ -104,7 +98,7 @@ func (sp *summaryProviderImpl) Get(ctx context.Context, updateStats bool) (*stat
 	}
 
 	nodeStats := statsapi.NodeStats{
-		NodeName:         node.Name,
+		NodeName:         string(nodeConfig.NodeName),
 		CPU:              rootStats.CPU,
 		Memory:           rootStats.Memory,
 		Swap:             rootStats.Swap,
@@ -126,12 +120,6 @@ func (sp *summaryProviderImpl) Get(ctx context.Context, updateStats bool) (*stat
 }
 
 func (sp *summaryProviderImpl) GetCPUAndMemoryStats(ctx context.Context) (*statsapi.Summary, error) {
-	// TODO(timstclair): Consider returning a best-effort response if any of
-	// the following errors occur.
-	node, err := sp.provider.GetNode()
-	if err != nil {
-		return nil, fmt.Errorf("failed to get node info: %v", err)
-	}
 	nodeConfig := sp.provider.GetNodeConfig()
 	rootStats, err := sp.provider.GetCgroupCPUAndMemoryStats("/", false)
 	if err != nil {
@@ -144,7 +132,7 @@ func (sp *summaryProviderImpl) GetCPUAndMemoryStats(ctx context.Context) (*stats
 	}
 
 	nodeStats := statsapi.NodeStats{
-		NodeName:         node.Name,
+		NodeName:         string(nodeConfig.NodeName),
 		CPU:              rootStats.CPU,
 		Memory:           rootStats.Memory,
 		Swap:             rootStats.Swap,

pkg/kubelet/server/stats/summary_test.go

@@ -29,6 +29,7 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	statsapi "k8s.io/kubelet/pkg/apis/stats/v1alpha1"
@@ -42,6 +43,7 @@ var (
 	rootFsStats  = getFsStats()
 	node         = &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "test-node"}}
 	nodeConfig   = cm.NodeConfig{
+		NodeName:           types.NodeName("test-node"),
 		RuntimeCgroupsName: "/runtime",
 		SystemCgroupsName:  "/misc",
 		KubeletCgroupsName: "/kubelet",
@@ -77,7 +79,6 @@ func TestSummaryProviderGetStatsNoSplitFileSystem(t *testing.T) {
 
 	mockStatsProvider := statstest.NewMockProvider(t)
 
-	mockStatsProvider.EXPECT().GetNode().Return(node, nil)
 	mockStatsProvider.EXPECT().GetNodeConfig().Return(nodeConfig)
 	mockStatsProvider.EXPECT().GetPodCgroupRoot().Return(cgroupRoot)
 	mockStatsProvider.EXPECT().ListPodStats(ctx).Return(podStats, nil).Maybe()
@@ -178,7 +179,6 @@ func TestSummaryProviderGetStatsSplitImageFs(t *testing.T) {
 
 	mockStatsProvider := statstest.NewMockProvider(t)
 
-	mockStatsProvider.EXPECT().GetNode().Return(node, nil)
 	mockStatsProvider.EXPECT().GetNodeConfig().Return(nodeConfig)
 	mockStatsProvider.EXPECT().GetPodCgroupRoot().Return(cgroupRoot)
 	mockStatsProvider.EXPECT().ListPodStats(ctx).Return(podStats, nil).Maybe()
@@ -278,7 +278,6 @@ func TestSummaryProviderGetCPUAndMemoryStats(t *testing.T) {
 
 	mockStatsProvider := statstest.NewMockProvider(t)
 
-	mockStatsProvider.EXPECT().GetNode().Return(node, nil)
 	mockStatsProvider.EXPECT().GetNodeConfig().Return(nodeConfig)
 	mockStatsProvider.EXPECT().GetPodCgroupRoot().Return(cgroupRoot)
 	mockStatsProvider.EXPECT().ListPodCPUAndMemoryStats(ctx).Return(podStats, nil)

pkg/kubelet/server/stats/summary_windows_test.go

@@ -27,7 +27,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"sigs.k8s.io/randfill"
 
-	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	statsapi "k8s.io/kubelet/pkg/apis/stats/v1alpha1"
 	"k8s.io/kubernetes/pkg/kubelet/cm"
@@ -40,7 +39,6 @@ func TestSummaryProvider(t *testing.T) {
 		podStats       = []statsapi.PodStats{*getPodStats()}
 		imageFsStats   = getFsStats()
 		rootFsStats    = getFsStats()
-		node           = &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "test-node"}}
 		nodeConfig     = cm.NodeConfig{}
 		cgroupRoot     = "/kubepods"
 		cgroupStatsMap = map[string]struct {
@@ -55,7 +53,6 @@ func TestSummaryProvider(t *testing.T) {
 	assert := assert.New(t)
 
 	mockStatsProvider := statstest.NewMockProvider(t)
-	mockStatsProvider.EXPECT().GetNode().Return(node, nil).Maybe()
 	mockStatsProvider.EXPECT().GetNodeConfig().Return(nodeConfig).Maybe()
 	mockStatsProvider.EXPECT().GetPodCgroupRoot().Return(cgroupRoot).Maybe()
 	mockStatsProvider.EXPECT().ListPodStats(ctx).Return(podStats, nil).Maybe()

pkg/kubelet/stats/cadvisor_stats_provider.go

@@ -97,18 +97,7 @@ func (p *cadvisorStatsProvider) ListPodStats(ctx context.Context) ([]statsapi.Po
 	filteredInfos, allInfos := filterTerminatedContainerInfoAndAssembleByPodCgroupKey(infos)
 	// Map each container to a pod and update the PodStats with container data.
 	podToStats := map[statsapi.PodReference]*statsapi.PodStats{}
-	for key, cinfo := range filteredInfos {
-		// On systemd using devicemapper each mount into the container has an
-		// associated cgroup. We ignore them to ensure we do not get duplicate
-		// entries in our summary. For details on .mount units:
-		// http://man7.org/linux/man-pages/man5/systemd.mount.5.html
-		if strings.HasSuffix(key, ".mount") {
-			continue
-		}
-		// Build the Pod key if this container is managed by a Pod
-		if !isPodManagedContainer(&cinfo) {
-			continue
-		}
+	for _, cinfo := range filteredInfos {
 		ref := buildPodRef(cinfo.Spec.Labels)
 
 		// Lookup the PodStats for the pod using the PodRef. If none exists,
@@ -190,18 +179,7 @@ func (p *cadvisorStatsProvider) ListPodCPUAndMemoryStats(_ context.Context) ([]s
 	filteredInfos, allInfos := filterTerminatedContainerInfoAndAssembleByPodCgroupKey(infos)
 	// Map each container to a pod and update the PodStats with container data.
 	podToStats := map[statsapi.PodReference]*statsapi.PodStats{}
-	for key, cinfo := range filteredInfos {
-		// On systemd using devicemapper each mount into the container has an
-		// associated cgroup. We ignore them to ensure we do not get duplicate
-		// entries in our summary. For details on .mount units:
-		// http://man7.org/linux/man-pages/man5/systemd.mount.5.html
-		if strings.HasSuffix(key, ".mount") {
-			continue
-		}
-		// Build the Pod key if this container is managed by a Pod
-		if !isPodManagedContainer(&cinfo) {
-			continue
-		}
+	for _, cinfo := range filteredInfos {
 		ref := buildPodRef(cinfo.Spec.Labels)
 
 		// Lookup the PodStats for the pod using the PodRef. If none exists,
@@ -394,19 +372,27 @@ func filterTerminatedContainerInfoAndAssembleByPodCgroupKey(containerInfo map[st
 	cinfoMap := make(map[containerID][]containerInfoWithCgroup)
 	cinfosByPodCgroupKey := make(map[string]cadvisorapiv2.ContainerInfo)
 	for key, cinfo := range containerInfo {
+		if !isPodManagedContainer(&cinfo) {
+			continue
+		}
+
 		var podCgroupKey string
 		if cm.IsSystemdStyleName(key) {
 			// Convert to internal cgroup name and take the last component only.
 			internalCgroupName := cm.ParseSystemdToCgroupName(key)
 			podCgroupKey = internalCgroupName[len(internalCgroupName)-1]
+		} else if strings.HasSuffix(key, ".mount") {
+			// On systemd using devicemapper each mount into the container has an
+			// associated cgroup. We ignore them to ensure we do not get duplicate
+			// entries in our summary. For details on .mount units:
+			// http://man7.org/linux/man-pages/man5/systemd.mount.5.html
+			continue
 		} else {
 			// Take last component only.
 			podCgroupKey = filepath.Base(key)
 		}
+
 		cinfosByPodCgroupKey[podCgroupKey] = cinfo
-		if !isPodManagedContainer(&cinfo) {
-			continue
-		}
 		cinfoID := containerID{
 			podRef:        buildPodRef(cinfo.Spec.Labels),
 			containerName: kubetypes.GetContainerName(cinfo.Spec.Labels),
@@ -419,9 +405,9 @@ func filterTerminatedContainerInfoAndAssembleByPodCgroupKey(containerInfo map[st
 	result := make(map[string]cadvisorapiv2.ContainerInfo)
 	for _, refs := range cinfoMap {
 		if len(refs) == 1 {
-			// ContainerInfo with no CPU/memory/network usage for uncleaned cgroups of
+			// ContainerInfo with no CPU/memory usage for uncleaned cgroups of
 			// already terminated containers, which should not be shown in the results.
-			if !isContainerTerminated(&refs[0].cinfo) {
+			if hasMemoryAndCPUInstUsage(&refs[0].cinfo) {
 				result[refs[0].cgroup] = refs[0].cinfo
 			}
 			continue
@@ -477,41 +463,12 @@ func hasMemoryAndCPUInstUsage(info *cadvisorapiv2.ContainerInfo) bool {
 	if !found {
 		return false
 	}
-	if cstat.CpuInst == nil {
+	if cstat.CpuInst == nil || cstat.Memory == nil {
 		return false
 	}
 	return cstat.CpuInst.Usage.Total != 0 && cstat.Memory.RSS != 0
 }
 
-// isContainerTerminated returns true if the specified container meet one of the following conditions
-// 1. info.spec both cpu memory and network are false conditions
-// 2. info.Stats both network and cpu or memory are nil
-// 3. both zero CPU instantaneous usage zero memory RSS usage and zero network usage,
-// and false otherwise.
-func isContainerTerminated(info *cadvisorapiv2.ContainerInfo) bool {
-	if !info.Spec.HasCpu && !info.Spec.HasMemory && !info.Spec.HasNetwork {
-		return true
-	}
-	cstat, found := latestContainerStats(info)
-	if !found {
-		return true
-	}
-	if cstat.Network != nil {
-		iStats := cadvisorInfoToNetworkStats(info)
-		if iStats != nil {
-			for _, iStat := range iStats.Interfaces {
-				if *iStat.RxErrors != 0 || *iStat.TxErrors != 0 || *iStat.RxBytes != 0 || *iStat.TxBytes != 0 {
-					return false
-				}
-			}
-		}
-	}
-	if cstat.CpuInst == nil || cstat.Memory == nil {
-		return true
-	}
-	return cstat.CpuInst.Usage.Total == 0 && cstat.Memory.RSS == 0
-}
-
 func getCadvisorContainerInfo(ca cadvisor.Interface) (map[string]cadvisorapiv2.ContainerInfo, error) {
 	infos, err := ca.ContainerInfoV2("/", cadvisorapiv2.RequestOptions{
 		IdType:    cadvisorapiv2.TypeName,