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.

Sign up for GitHub

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

Jump to bottom

Work-around for missing memory metrics on CRI-O exited containers #88734

Merged
merged 1 commit into from
Mar 10, 2020
Merged

Work-around for missing memory metrics on CRI-O exited containers #88734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 47 additions & 1 deletion pkg/kubelet/stats/cadvisor_stats_provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ func TestCadvisorListPodStats(t *testing.T) {
seedPod1Container = 4000
seedPod2Infra = 5000
seedPod2Container = 6000
seedPod3Infra = 7000
seedPod3Container0 = 8000
seedPod3Container1 = 8001
seedEphemeralVolume1 = 10000
seedEphemeralVolume2 = 10001
seedPersistentVolume1 = 20000
Expand All @@ -98,12 +101,15 @@ func TestCadvisorListPodStats(t *testing.T) {
pName0 = "pod0"
pName1 = "pod1"
pName2 = "pod0" // ensure pName2 conflicts with pName0, but is in a different namespace
pName3 = "pod3"
)
const (
cName00 = "c0"
cName01 = "c1"
cName10 = "c0" // ensure cName10 conflicts with cName02, but is in a different pod
cName20 = "c1" // ensure cName20 conflicts with cName01, but is in a different pod + namespace
cName30 = "c0-init"
cName31 = "c1"
)
const (
rootfsCapacity = uint64(10000000)
Expand All @@ -119,6 +125,7 @@ func TestCadvisorListPodStats(t *testing.T) {
prf0 := statsapi.PodReference{Name: pName0, Namespace: namespace0, UID: "UID" + pName0}
prf1 := statsapi.PodReference{Name: pName1, Namespace: namespace0, UID: "UID" + pName1}
prf2 := statsapi.PodReference{Name: pName2, Namespace: namespace2, UID: "UID" + pName2}
prf3 := statsapi.PodReference{Name: pName3, Namespace: namespace0, UID: "UID" + pName3}
infos := map[string]cadvisorapiv2.ContainerInfo{
"/": getTestContainerInfo(seedRoot, "", "", ""),
"/docker-daemon": getTestContainerInfo(seedRuntime, "", "", ""),
Expand All @@ -136,6 +143,10 @@ func TestCadvisorListPodStats(t *testing.T) {
"/pod2-c0": getTestContainerInfo(seedPod2Container, pName2, namespace2, cName20),
"/kubepods/burstable/podUIDpod0": getTestContainerInfo(seedPod0Infra, pName0, namespace0, leaky.PodInfraContainerName),
"/kubepods/podUIDpod1": getTestContainerInfo(seedPod1Infra, pName1, namespace0, leaky.PodInfraContainerName),
// Pod3 - Namespace0
"/pod3-i": getTestContainerInfo(seedPod3Infra, pName3, namespace0, leaky.PodInfraContainerName),
"/pod3-c0-init": getTestContainerInfo(seedPod3Container0, pName3, namespace0, cName30),
"/pod3-c1": getTestContainerInfo(seedPod3Container1, pName3, namespace0, cName31),
}

freeRootfsInodes := rootfsInodesFree
Expand Down Expand Up @@ -169,6 +180,21 @@ func TestCadvisorListPodStats(t *testing.T) {
info.Spec.Memory.Limit = memoryLimitOverride
infos[name] = info
}
// any container for which cadvisor should return no stats (as might be the case for an exited init container)
nostatsOverrides := []string{
"/pod3-c0-init",
}
for _, name := range nostatsOverrides {
info, found := infos[name]
if !found {
t.Errorf("No container defined with name %v", name)
}
info.Spec.Memory = cadvisorapiv2.MemorySpec{}
info.Spec.Cpu = cadvisorapiv2.CpuSpec{}
info.Spec.HasMemory = false
info.Spec.HasCpu = false
infos[name] = info
}

options := cadvisorapiv2.RequestOptions{
IdType: cadvisorapiv2.TypeName,
Expand Down Expand Up @@ -197,18 +223,20 @@ func TestCadvisorListPodStats(t *testing.T) {
p0Time := metav1.Now()
p1Time := metav1.Now()
p2Time := metav1.Now()
p3Time := metav1.Now()
mockStatus := new(statustest.MockStatusProvider)
mockStatus.On("GetPodStatus", types.UID("UID"+pName0)).Return(v1.PodStatus{StartTime: &p0Time}, true)
mockStatus.On("GetPodStatus", types.UID("UID"+pName1)).Return(v1.PodStatus{StartTime: &p1Time}, true)
mockStatus.On("GetPodStatus", types.UID("UID"+pName2)).Return(v1.PodStatus{StartTime: &p2Time}, true)
mockStatus.On("GetPodStatus", types.UID("UID"+pName3)).Return(v1.PodStatus{StartTime: &p3Time}, true)

resourceAnalyzer := &fakeResourceAnalyzer{podVolumeStats: volumeStats}

p := NewCadvisorStatsProvider(mockCadvisor, resourceAnalyzer, nil, nil, mockRuntime, mockStatus)
pods, err := p.ListPodStats()
assert.NoError(t, err)

assert.Equal(t, 3, len(pods))
assert.Equal(t, 4, len(pods))
indexPods := make(map[statsapi.PodReference]statsapi.PodStats, len(pods))
for _, pod := range pods {
indexPods[pod.PodRef] = pod
Expand Down Expand Up @@ -261,6 +289,24 @@ func TestCadvisorListPodStats(t *testing.T) {
checkCPUStats(t, "Pod2Container0", seedPod2Container, con.CPU)
checkMemoryStats(t, "Pod2Container0", seedPod2Container, infos["/pod2-c0"], con.Memory)
checkNetworkStats(t, "Pod2", seedPod2Infra, ps.Network)

// Validate Pod3 Results

ps, found = indexPods[prf3]
assert.True(t, found)
assert.Len(t, ps.Containers, 2)
indexCon = make(map[string]statsapi.ContainerStats, len(ps.Containers))
for _, con := range ps.Containers {
indexCon[con.Name] = con
}
con = indexCon[cName31]
assert.Equal(t, cName31, con.Name)
checkCPUStats(t, "Pod3Container1", seedPod3Container1, con.CPU)
checkMemoryStats(t, "Pod3Container1", seedPod3Container1, infos["/pod3-c1"], con.Memory)
con = indexCon[cName30]
assert.Equal(t, cName30, con.Name)
checkEmptyCPUStats(t, "Pod3Container0", seedPod3Container0, con.CPU)
checkEmptyMemoryStats(t, "Pod3Container0", seedPod3Container0, infos["/pod3-c0-init"], con.Memory)
}

func TestCadvisorListPodCPUAndMemoryStats(t *testing.T) {
Expand Down
13 changes: 10 additions & 3 deletions pkg/kubelet/stats/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,12 @@ func cadvisorInfoToCPUandMemoryStats(info *cadvisorapiv2.ContainerInfo) (*statsa
}
var cpuStats *statsapi.CPUStats
var memoryStats *statsapi.MemoryStats
cpuStats = &statsapi.CPUStats{
Time: metav1.NewTime(cstat.Timestamp),
UsageNanoCores: uint64Ptr(0),
UsageCoreNanoSeconds: uint64Ptr(0),
}
if info.Spec.HasCpu {
cpuStats = &statsapi.CPUStats{
Time: metav1.NewTime(cstat.Timestamp),
}
if cstat.CpuInst != nil {
cpuStats.UsageNanoCores = &cstat.CpuInst.Usage.Total
}
Expand All @@ -67,6 +69,11 @@ func cadvisorInfoToCPUandMemoryStats(info *cadvisorapiv2.ContainerInfo) (*statsa
availableBytes := info.Spec.Memory.Limit - cstat.Memory.WorkingSet
memoryStats.AvailableBytes = &availableBytes
}
} else {
memoryStats = &statsapi.MemoryStats{
Time: metav1.NewTime(cstat.Timestamp),
WorkingSetBytes: uint64Ptr(0),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we default other fields to 0 to have consistent behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps. I was just trying to match the behavior in the CRI stats provider, here: https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/stats/cri_stats_provider.go#L537

}
}
return cpuStats, memoryStats
}
Expand Down
25 changes: 25 additions & 0 deletions pkg/kubelet/stats/stats_provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,32 @@ func checkNetworkStats(t *testing.T, label string, seed int, stats *statsapi.Net

}

// container which had no stats should have zero-valued CPU usage
func checkEmptyCPUStats(t *testing.T, label string, seed int, stats *statsapi.CPUStats) {
require.NotNil(t, stats.Time, label+".CPU.Time")
require.NotNil(t, stats.UsageNanoCores, label+".CPU.UsageNanoCores")
require.NotNil(t, stats.UsageNanoCores, label+".CPU.UsageCoreSeconds")
assert.EqualValues(t, testTime(timestamp, seed).Unix(), stats.Time.Time.Unix(), label+".CPU.Time")
assert.EqualValues(t, 0, *stats.UsageNanoCores, label+".CPU.UsageCores")
assert.EqualValues(t, 0, *stats.UsageCoreNanoSeconds, label+".CPU.UsageCoreSeconds")
}

// container which had no stats should have zero-valued Memory usage
func checkEmptyMemoryStats(t *testing.T, label string, seed int, info cadvisorapiv2.ContainerInfo, stats *statsapi.MemoryStats) {
assert.EqualValues(t, testTime(timestamp, seed).Unix(), stats.Time.Time.Unix(), label+".Mem.Time")
require.NotNil(t, stats.WorkingSetBytes, label+".Mem.WorkingSetBytes")
assert.EqualValues(t, 0, *stats.WorkingSetBytes, label+".Mem.WorkingSetBytes")
assert.Nil(t, stats.UsageBytes, label+".Mem.UsageBytes")
assert.Nil(t, stats.RSSBytes, label+".Mem.RSSBytes")
assert.Nil(t, stats.PageFaults, label+".Mem.PageFaults")
assert.Nil(t, stats.MajorPageFaults, label+".Mem.MajorPageFaults")
assert.Nil(t, stats.AvailableBytes, label+".Mem.AvailableBytes")
}

func checkCPUStats(t *testing.T, label string, seed int, stats *statsapi.CPUStats) {
require.NotNil(t, stats.Time, label+".CPU.Time")
require.NotNil(t, stats.UsageNanoCores, label+".CPU.UsageNanoCores")
require.NotNil(t, stats.UsageNanoCores, label+".CPU.UsageCoreSeconds")
assert.EqualValues(t, testTime(timestamp, seed).Unix(), stats.Time.Time.Unix(), label+".CPU.Time")
assert.EqualValues(t, seed+offsetCPUUsageCores, *stats.UsageNanoCores, label+".CPU.UsageCores")
assert.EqualValues(t, seed+offsetCPUUsageCoreSeconds, *stats.UsageCoreNanoSeconds, label+".CPU.UsageCoreSeconds")
Expand Down