Skip to content

Commit 39dcad8

Browse files
saschagrunertsaschagrunert
committed
Populate CRI filesystem info error
Usually we just log the error but since it's used by the GC we now populate it up the call stack. Signed-off-by: Sascha Grunert <[email protected]>
1 parent afc302c commit 39dcad8

File tree

4 files changed

+52
-21
lines changed

4 files changed

+52
-21
lines changed

‎pkg/kubelet/stats/cri_stats_provider.go‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,10 @@ func (p *criStatsProvider) listPodStatsPartiallyFromCRI(ctx context.Context, upd
204204
}
205205

206206
// Fill available stats for full set of required pod stats
207-
cs := p.makeContainerStats(stats, container, rootFsInfo, fsIDtoInfo, podSandbox.GetMetadata(), updateCPUNanoCoreUsage)
207+
cs, err := p.makeContainerStats(stats, container, rootFsInfo, fsIDtoInfo, podSandbox.GetMetadata(), updateCPUNanoCoreUsage)
208+
if err != nil {
209+
return nil, fmt.Errorf("make container stats: %w", err)
210+
}
208211
p.addPodNetworkStats(ps, podSandboxID, caInfos, cs, containerNetworkStats[podSandboxID])
209212
p.addPodCPUMemoryStats(ps, types.UID(podSandbox.Metadata.Uid), allInfos, cs)
210213
p.addSwapStats(ps, types.UID(podSandbox.Metadata.Uid), allInfos, cs)
@@ -249,7 +252,9 @@ func (p *criStatsProvider) listPodStatsStrictlyFromCRI(ctx context.Context, upda
249252
continue
250253
}
251254
ps := buildPodStats(podSandbox)
252-
p.addCRIPodContainerStats(criSandboxStat, ps, fsIDtoInfo, containerMap, podSandbox, rootFsInfo, updateCPUNanoCoreUsage)
255+
if err := p.addCRIPodContainerStats(criSandboxStat, ps, fsIDtoInfo, containerMap, podSandbox, rootFsInfo, updateCPUNanoCoreUsage); err != nil {
256+
return nil, fmt.Errorf("add CRI pod container stats: %w", err)
257+
}
253258
addCRIPodNetworkStats(ps, criSandboxStat)
254259
addCRIPodCPUStats(ps, criSandboxStat)
255260
addCRIPodMemoryStats(ps, criSandboxStat)
@@ -401,7 +406,10 @@ func (p *criStatsProvider) ImageFsStats(ctx context.Context) (*statsapi.FsStats,
401406
if fs.InodesUsed != nil {
402407
s.InodesUsed = &fs.InodesUsed.Value
403408
}
404-
imageFsInfo := p.getFsInfo(fs.GetFsId())
409+
imageFsInfo, err := p.getFsInfo(fs.GetFsId())
410+
if err != nil {
411+
return nil, fmt.Errorf("get filesystem info: %w", err)
412+
}
405413
if imageFsInfo != nil {
406414
// The image filesystem id is unknown to the local node or there's
407415
// an error on retrieving the stats. In these cases, we omit those
@@ -423,7 +431,10 @@ func (p *criStatsProvider) ImageFsDevice(ctx context.Context) (string, error) {
423431
return "", err
424432
}
425433
for _, fs := range resp {
426-
fsInfo := p.getFsInfo(fs.GetFsId())
434+
fsInfo, err := p.getFsInfo(fs.GetFsId())
435+
if err != nil {
436+
return "", fmt.Errorf("get filesystem info: %w", err)
437+
}
427438
if fsInfo != nil {
428439
return fsInfo.Device, nil
429440
}
@@ -434,10 +445,10 @@ func (p *criStatsProvider) ImageFsDevice(ctx context.Context) (string, error) {
434445
// getFsInfo returns the information of the filesystem with the specified
435446
// fsID. If any error occurs, this function logs the error and returns
436447
// nil.
437-
func (p *criStatsProvider) getFsInfo(fsID *runtimeapi.FilesystemIdentifier) *cadvisorapiv2.FsInfo {
448+
func (p *criStatsProvider) getFsInfo(fsID *runtimeapi.FilesystemIdentifier) (*cadvisorapiv2.FsInfo, error) {
438449
if fsID == nil {
439450
klog.V(2).InfoS("Failed to get filesystem info: fsID is nil")
440-
return nil
451+
return nil, nil
441452
}
442453
mountpoint := fsID.GetMountpoint()
443454
fsInfo, err := p.cadvisor.GetDirFsInfo(mountpoint)
@@ -449,10 +460,11 @@ func (p *criStatsProvider) getFsInfo(fsID *runtimeapi.FilesystemIdentifier) *cad
449460
klog.V(2).InfoS(msg, "mountpoint", mountpoint, "err", err)
450461
} else {
451462
klog.ErrorS(err, msg, "mountpoint", mountpoint)
463+
return nil, fmt.Errorf("%s: %w", msg, err)
452464
}
453-
return nil
465+
return nil, nil
454466
}
455-
return &fsInfo
467+
return &fsInfo, nil
456468
}
457469

458470
// buildPodStats returns a PodStats that identifies the Pod managing cinfo
@@ -590,7 +602,7 @@ func (p *criStatsProvider) makeContainerStats(
590602
fsIDtoInfo map[runtimeapi.FilesystemIdentifier]*cadvisorapiv2.FsInfo,
591603
meta *runtimeapi.PodSandboxMetadata,
592604
updateCPUNanoCoreUsage bool,
593-
) *statsapi.ContainerStats {
605+
) (*statsapi.ContainerStats, error) {
594606
result := &statsapi.ContainerStats{
595607
Name: stats.Attributes.Metadata.Name,
596608
// The StartTime in the summary API is the container creation time.
@@ -652,10 +664,14 @@ func (p *criStatsProvider) makeContainerStats(
652664
}
653665
}
654666
fsID := stats.GetWritableLayer().GetFsId()
667+
var err error
655668
if fsID != nil {
656669
imageFsInfo, found := fsIDtoInfo[*fsID]
657670
if !found {
658-
imageFsInfo = p.getFsInfo(fsID)
671+
imageFsInfo, err = p.getFsInfo(fsID)
672+
if err != nil {
673+
return nil, fmt.Errorf("get filesystem info: %w", err)
674+
}
659675
fsIDtoInfo[*fsID] = imageFsInfo
660676
}
661677
if imageFsInfo != nil {
@@ -672,12 +688,11 @@ func (p *criStatsProvider) makeContainerStats(
672688
// NOTE: This doesn't support the old pod log path, `/var/log/pods/UID`. For containers
673689
// using old log path, empty log stats are returned. This is fine, because we don't
674690
// officially support in-place upgrade anyway.
675-
var err error
676691
result.Logs, err = p.hostStatsProvider.getPodContainerLogStats(meta.GetNamespace(), meta.GetName(), types.UID(meta.GetUid()), container.GetMetadata().GetName(), rootFsInfo)
677692
if err != nil {
678693
klog.ErrorS(err, "Unable to fetch container log stats", "containerName", container.GetMetadata().GetName())
679694
}
680-
return result
695+
return result, nil
681696
}
682697

683698
func (p *criStatsProvider) makeContainerCPUAndMemoryStats(

‎pkg/kubelet/stats/cri_stats_provider_linux.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ limitations under the License.
2020
package stats
2121

2222
import (
23+
"fmt"
2324
"time"
2425

2526
cadvisorapiv2 "github.com/google/cadvisor/info/v2"
@@ -32,17 +33,21 @@ func (p *criStatsProvider) addCRIPodContainerStats(criSandboxStat *runtimeapi.Po
3233
ps *statsapi.PodStats, fsIDtoInfo map[runtimeapi.FilesystemIdentifier]*cadvisorapiv2.FsInfo,
3334
containerMap map[string]*runtimeapi.Container,
3435
podSandbox *runtimeapi.PodSandbox,
35-
rootFsInfo *cadvisorapiv2.FsInfo, updateCPUNanoCoreUsage bool) {
36+
rootFsInfo *cadvisorapiv2.FsInfo, updateCPUNanoCoreUsage bool) error {
3637
for _, criContainerStat := range criSandboxStat.Linux.Containers {
3738
container, found := containerMap[criContainerStat.Attributes.Id]
3839
if !found {
3940
continue
4041
}
4142
// Fill available stats for full set of required pod stats
42-
cs := p.makeContainerStats(criContainerStat, container, rootFsInfo, fsIDtoInfo, podSandbox.GetMetadata(),
43+
cs, err := p.makeContainerStats(criContainerStat, container, rootFsInfo, fsIDtoInfo, podSandbox.GetMetadata(),
4344
updateCPUNanoCoreUsage)
45+
if err != nil {
46+
return fmt.Errorf("make container stats: %w", err)
47+
}
4448
ps.Containers = append(ps.Containers, *cs)
4549
}
50+
return nil
4651
}
4752

4853
func addCRIPodNetworkStats(ps *statsapi.PodStats, criPodStat *runtimeapi.PodSandboxStats) {

‎pkg/kubelet/stats/cri_stats_provider_others.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ func (p *criStatsProvider) addCRIPodContainerStats(criSandboxStat *runtimeapi.Po
3535
ps *statsapi.PodStats, fsIDtoInfo map[runtimeapi.FilesystemIdentifier]*cadvisorapiv2.FsInfo,
3636
containerMap map[string]*runtimeapi.Container,
3737
podSandbox *runtimeapi.PodSandbox,
38-
rootFsInfo *cadvisorapiv2.FsInfo, updateCPUNanoCoreUsage bool) {
38+
rootFsInfo *cadvisorapiv2.FsInfo, updateCPUNanoCoreUsage bool) error {
39+
return nil
3940
}
4041

4142
func addCRIPodNetworkStats(ps *statsapi.PodStats, criPodStat *runtimeapi.PodSandboxStats) {

‎pkg/kubelet/stats/cri_stats_provider_windows.go‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ limitations under the License.
2020
package stats
2121

2222
import (
23+
"fmt"
2324
"time"
2425

2526
"github.com/Microsoft/hcsshim"
@@ -86,24 +87,30 @@ func (p *criStatsProvider) addCRIPodContainerStats(criSandboxStat *runtimeapi.Po
8687
containerMap map[string]*runtimeapi.Container,
8788
podSandbox *runtimeapi.PodSandbox,
8889
rootFsInfo *cadvisorapiv2.FsInfo,
89-
updateCPUNanoCoreUsage bool) {
90+
updateCPUNanoCoreUsage bool) error {
9091
for _, criContainerStat := range criSandboxStat.Windows.Containers {
9192
container, found := containerMap[criContainerStat.Attributes.Id]
9293
if !found {
9394
continue
9495
}
9596
// Fill available stats for full set of required pod stats
96-
cs := p.makeWinContainerStats(criContainerStat, container, rootFsInfo, fsIDtoInfo, podSandbox.GetMetadata())
97+
cs, err := p.makeWinContainerStats(criContainerStat, container, rootFsInfo, fsIDtoInfo, podSandbox.GetMetadata())
98+
if err != nil {
99+
return fmt.Errorf("make container stats: %w", err)
100+
101+
}
97102
ps.Containers = append(ps.Containers, *cs)
98103
}
104+
105+
return nil
99106
}
100107

101108
func (p *criStatsProvider) makeWinContainerStats(
102109
stats *runtimeapi.WindowsContainerStats,
103110
container *runtimeapi.Container,
104111
rootFsInfo *cadvisorapiv2.FsInfo,
105112
fsIDtoInfo map[runtimeapi.FilesystemIdentifier]*cadvisorapiv2.FsInfo,
106-
meta *runtimeapi.PodSandboxMetadata) *statsapi.ContainerStats {
113+
meta *runtimeapi.PodSandboxMetadata) (*statsapi.ContainerStats, error) {
107114
result := &statsapi.ContainerStats{
108115
Name: stats.Attributes.Metadata.Name,
109116
// The StartTime in the summary API is the container creation time.
@@ -149,11 +156,15 @@ func (p *criStatsProvider) makeWinContainerStats(
149156
result.Rootfs.UsedBytes = &stats.WritableLayer.UsedBytes.Value
150157
}
151158
}
159+
var err error
152160
fsID := stats.GetWritableLayer().GetFsId()
153161
if fsID != nil {
154162
imageFsInfo, found := fsIDtoInfo[*fsID]
155163
if !found {
156-
imageFsInfo = p.getFsInfo(fsID)
164+
imageFsInfo, err = p.getFsInfo(fsID)
165+
if err != nil {
166+
return nil, fmt.Errorf("get filesystem info: %w", err)
167+
}
157168
fsIDtoInfo[*fsID] = imageFsInfo
158169
}
159170
if imageFsInfo != nil {
@@ -168,12 +179,11 @@ func (p *criStatsProvider) makeWinContainerStats(
168179
// NOTE: This doesn't support the old pod log path, `/var/log/pods/UID`. For containers
169180
// using old log path, empty log stats are returned. This is fine, because we don't
170181
// officially support in-place upgrade anyway.
171-
var err error
172182
result.Logs, err = p.hostStatsProvider.getPodContainerLogStats(meta.GetNamespace(), meta.GetName(), types.UID(meta.GetUid()), container.GetMetadata().GetName(), rootFsInfo)
173183
if err != nil {
174184
klog.ErrorS(err, "Unable to fetch container log stats", "containerName", container.GetMetadata().GetName())
175185
}
176-
return result
186+
return result, nil
177187
}
178188

179189
// hcsStatsToNetworkStats converts hcsshim.Statistics.Network to statsapi.NetworkStats

0 commit comments

Comments
 (0)