Skip to content

Commit 50e430b

Browse files
saschagrunertsaschagrunert
committed
Fix kubelet cadvisor stats runtime panic
Fixing a kubelet runtime panic when the runtime returns incomplete data: ``` E0729 08:17:47.260393 5218 panic.go:115] "Observed a panic" panic="runtime error: index out of range [0] with length 0" panicGoValue="runtime.boundsError{x:0, y:0, signed:true, code:0x0}" stacktrace=< goroutine 174 [running]: k8s.io/apimachinery/pkg/util/runtime.logPanic({0x33631e8, 0x4ddf5c0}, {0x2c9bfe0, 0xc000a563f0}) k8s.io/apimachinery/pkg/util/runtime/runtime.go:107 +0xbc k8s.io/apimachinery/pkg/util/runtime.handleCrash({0x33631e8, 0x4ddf5c0}, {0x2c9bfe0, 0xc000a563f0}, {0x4ddf5c0, 0x0, 0x10000000043c9e5?}) k8s.io/apimachinery/pkg/util/runtime/runtime.go:82 +0x5e k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0xc000ae08c0?}) k8s.io/apimachinery/pkg/util/runtime/runtime.go:59 +0x108 panic({0x2c9bfe0?, 0xc000a563f0?}) runtime/panic.go:785 +0x132 k8s.io/kubernetes/pkg/kubelet/stats.(*cadvisorStatsProvider).ImageFsStats(0xc000535d10, {0x3363348, 0xc000afa330}) k8s.io/kubernetes/pkg/kubelet/stats/cadvisor_stats_provider.go:277 +0xaba k8s.io/kubernetes/pkg/kubelet/images.(*realImageGCManager).GarbageCollect(0xc000a3c820, {0x33631e8?, 0x4ddf5c0?}, {0x0?, 0x0?, 0x4dbca20?}) k8s.io/kubernetes/pkg/kubelet/images/image_gc_manager.go:354 +0x1d3 k8s.io/kubernetes/pkg/kubelet.(*Kubelet).StartGarbageCollection.func2() k8s.io/kubernetes/pkg/kubelet/kubelet.go:1472 +0x58 k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0x30?) k8s.io/apimachinery/pkg/util/wait/backoff.go:226 +0x33 k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0xc000add110, {0x3330380, 0xc000afa300}, 0x1, 0xc0000ac150) k8s.io/apimachinery/pkg/util/wait/backoff.go:227 +0xaf k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc000add110, 0x45d964b800, 0x0, 0x1, 0xc0000ac150) k8s.io/apimachinery/pkg/util/wait/backoff.go:204 +0x7f k8s.io/apimachinery/pkg/util/wait.Until(...) k8s.io/apimachinery/pkg/util/wait/backoff.go:161 created by k8s.io/kubernetes/pkg/kubelet.(*Kubelet).StartGarbageCollection in goroutine 1 k8s.io/kubernetes/pkg/kubelet/kubelet.go:1470 +0x247 ``` This commit fixes panics if: - `len(imageStats.ImageFilesystems) == 0` - `len(imageStats.ContainerFilesystems) == 0` - `imageStats.ImageFilesystems[0].FsId == nil` - `imageStats.ContainerFilesystems[0].FsId == nil` - `imageStats.ImageFilesystems[0].UsedBytes == nil` - `imageStats.ContainerFilesystems[0].UsedBytes == nil` It also fixes the wrapped `nil` error for the check: `err != nil || imageStats == nil` in case that `imageStats == nil`. Signed-off-by: Sascha Grunert <[email protected]>
1 parent a2106b5 commit 50e430b

File tree

2 files changed

+100
-8
lines changed

2 files changed

+100
-8
lines changed

‎pkg/kubelet/stats/cadvisor_stats_provider.go‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -270,26 +270,34 @@ func (p *cadvisorStatsProvider) ImageFsStats(ctx context.Context) (imageFsRet *s
270270
return imageFs, imageFs, nil
271271
}
272272
imageStats, err := p.imageService.ImageFsInfo(ctx)
273-
if err != nil || imageStats == nil {
274-
return nil, nil, fmt.Errorf("failed to get image stats: %v", err)
273+
if err != nil {
274+
return nil, nil, fmt.Errorf("failed to get image stats: %w", err)
275+
}
276+
if imageStats == nil || len(imageStats.ImageFilesystems) == 0 || len(imageStats.ContainerFilesystems) == 0 {
277+
return nil, nil, fmt.Errorf("missing image stats: %+v", imageStats)
275278
}
276279
splitFileSystem := false
277-
if imageStats.ImageFilesystems[0].FsId.Mountpoint != imageStats.ContainerFilesystems[0].FsId.Mountpoint {
278-
klog.InfoS("Detect Split Filesystem", "ImageFilesystems", imageStats.ImageFilesystems[0], "ContainerFilesystems", imageStats.ContainerFilesystems[0])
280+
imageFs := imageStats.ImageFilesystems[0]
281+
containerFs := imageStats.ContainerFilesystems[0]
282+
if imageFs.FsId != nil && containerFs.FsId != nil && imageFs.FsId.Mountpoint != containerFs.FsId.Mountpoint {
283+
klog.InfoS("Detect Split Filesystem", "ImageFilesystems", imageFs, "ContainerFilesystems", containerFs)
279284
splitFileSystem = true
280285
}
281-
imageFs := imageStats.ImageFilesystems[0]
282286
var imageFsInodesUsed *uint64
283287
if imageFsInfo.Inodes != nil && imageFsInfo.InodesFree != nil {
284288
imageFsIU := *imageFsInfo.Inodes - *imageFsInfo.InodesFree
285289
imageFsInodesUsed = &imageFsIU
286290
}
291+
var usedBytes uint64
292+
if imageFs.GetUsedBytes() != nil {
293+
usedBytes = imageFs.GetUsedBytes().GetValue()
294+
}
287295

288296
fsStats := &statsapi.FsStats{
289297
Time: metav1.NewTime(imageFsInfo.Timestamp),
290298
AvailableBytes: &imageFsInfo.Available,
291299
CapacityBytes: &imageFsInfo.Capacity,
292-
UsedBytes: &imageFs.UsedBytes.Value,
300+
UsedBytes: &usedBytes,
293301
InodesFree: imageFsInfo.InodesFree,
294302
Inodes: imageFsInfo.Inodes,
295303
InodesUsed: imageFsInodesUsed,
@@ -305,18 +313,21 @@ func (p *cadvisorStatsProvider) ImageFsStats(ctx context.Context) (imageFsRet *s
305313
return nil, nil, fmt.Errorf("failed to get container fs info: %w", err)
306314
}
307315

308-
containerFs := imageStats.ContainerFilesystems[0]
309316
var containerFsInodesUsed *uint64
310317
if containerFsInfo.Inodes != nil && containerFsInfo.InodesFree != nil {
311318
containerFsIU := *containerFsInfo.Inodes - *containerFsInfo.InodesFree
312319
containerFsInodesUsed = &containerFsIU
313320
}
321+
var usedContainerBytes uint64
322+
if containerFs.GetUsedBytes() != nil {
323+
usedContainerBytes = containerFs.GetUsedBytes().GetValue()
324+
}
314325

315326
fsContainerStats := &statsapi.FsStats{
316327
Time: metav1.NewTime(containerFsInfo.Timestamp),
317328
AvailableBytes: &containerFsInfo.Available,
318329
CapacityBytes: &containerFsInfo.Capacity,
319-
UsedBytes: &containerFs.UsedBytes.Value,
330+
UsedBytes: &usedContainerBytes,
320331
InodesFree: containerFsInfo.InodesFree,
321332
Inodes: containerFsInfo.Inodes,
322333
InodesUsed: containerFsInodesUsed,

‎pkg/kubelet/stats/cadvisor_stats_provider_test.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323

2424
cadvisorapiv2 "github.com/google/cadvisor/info/v2"
2525
"github.com/stretchr/testify/assert"
26+
"github.com/stretchr/testify/require"
2627

2728
v1 "k8s.io/api/core/v1"
2829
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -546,6 +547,86 @@ func TestCadvisorImagesFsStatsKubeletSeparateDiskOff(t *testing.T) {
546547
assert.Equal(*imageFsInfo.Inodes-*imageFsInfo.InodesFree, *stats.InodesUsed)
547548
}
548549

550+
func TestImageFsStatsCustomResponse(t *testing.T) {
551+
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KubeletSeparateDiskGC, true)
552+
for desc, tc := range map[string]struct {
553+
response *runtimeapi.ImageFsInfoResponse
554+
callContainerFsInfo, shouldErr bool
555+
}{
556+
"image stats are nil": {
557+
shouldErr: true,
558+
},
559+
"no image filesystems in image stats": {
560+
response: &runtimeapi.ImageFsInfoResponse{
561+
ImageFilesystems: []*runtimeapi.FilesystemUsage{},
562+
ContainerFilesystems: []*runtimeapi.FilesystemUsage{{}},
563+
},
564+
shouldErr: true,
565+
},
566+
"no container filesystems in image stats": {
567+
response: &runtimeapi.ImageFsInfoResponse{
568+
ImageFilesystems: []*runtimeapi.FilesystemUsage{{}},
569+
ContainerFilesystems: []*runtimeapi.FilesystemUsage{},
570+
},
571+
shouldErr: true,
572+
},
573+
"image and container filesystem identifiers are nil": {
574+
response: &runtimeapi.ImageFsInfoResponse{
575+
ImageFilesystems: []*runtimeapi.FilesystemUsage{{}},
576+
ContainerFilesystems: []*runtimeapi.FilesystemUsage{{}},
577+
},
578+
shouldErr: false,
579+
},
580+
"using different mountpoints but no used bytes set": {
581+
response: &runtimeapi.ImageFsInfoResponse{
582+
ImageFilesystems: []*runtimeapi.FilesystemUsage{{
583+
FsId: &runtimeapi.FilesystemIdentifier{Mountpoint: "mnt-1"},
584+
}},
585+
ContainerFilesystems: []*runtimeapi.FilesystemUsage{{
586+
FsId: &runtimeapi.FilesystemIdentifier{Mountpoint: "mnt-2"},
587+
}},
588+
},
589+
callContainerFsInfo: true,
590+
shouldErr: false,
591+
},
592+
"using different mountpoints and set used bytes": {
593+
response: &runtimeapi.ImageFsInfoResponse{
594+
ImageFilesystems: []*runtimeapi.FilesystemUsage{{
595+
FsId: &runtimeapi.FilesystemIdentifier{Mountpoint: "mnt-1"},
596+
UsedBytes: &runtimeapi.UInt64Value{Value: 10},
597+
}},
598+
ContainerFilesystems: []*runtimeapi.FilesystemUsage{{
599+
FsId: &runtimeapi.FilesystemIdentifier{Mountpoint: "mnt-2"},
600+
UsedBytes: &runtimeapi.UInt64Value{Value: 20},
601+
}},
602+
},
603+
callContainerFsInfo: true,
604+
shouldErr: false,
605+
},
606+
} {
607+
ctx := context.Background()
608+
mockCadvisor := cadvisortest.NewMockInterface(t)
609+
mockRuntime := containertest.NewMockRuntime(t)
610+
611+
res := getTestFsInfo(1000)
612+
mockCadvisor.EXPECT().ImagesFsInfo().Return(res, nil)
613+
mockRuntime.EXPECT().ImageFsInfo(ctx).Return(tc.response, nil)
614+
if tc.callContainerFsInfo {
615+
mockCadvisor.EXPECT().ContainerFsInfo().Return(res, nil)
616+
}
617+
618+
provider := newCadvisorStatsProvider(mockCadvisor, &fakeResourceAnalyzer{}, mockRuntime, nil, NewFakeHostStatsProvider())
619+
stats, containerfs, err := provider.ImageFsStats(ctx)
620+
if tc.shouldErr {
621+
require.Error(t, err, desc)
622+
assert.Nil(t, stats)
623+
assert.Nil(t, containerfs)
624+
} else {
625+
assert.NoError(t, err, desc)
626+
}
627+
}
628+
}
629+
549630
func TestCadvisorImagesFsStats(t *testing.T) {
550631
ctx := context.Background()
551632
var (

0 commit comments

Comments
 (0)