Merge pull request #10724 from rosenvered/cri_stats_container_not_found

make ListContainerStats handle container that is removed before its sandbox

internal/cri/server/container_stats_list.go

@@ -140,15 +140,9 @@ func (c *criService) toContainerStats(
 		if !ok {
 			handler, err = c.getMetricsHandler(ctx, cntr.SandboxID)
 			if err != nil {
-				// If the sandbox is not found, it may have been removed. we need to check container whether it is still exist
-				if errdefs.IsNotFound(err) {
-					_, err = c.containerStore.Get(cntr.ID)
-					if err != nil && errdefs.IsNotFound(err) {
-						log.G(ctx).Warnf("container %q is not found, skip it", cntr.ID)
-						continue
-					}
-				}
-				return nil, fmt.Errorf("failed to get metrics handler for container %q: %w", cntr.ID, err)
+				// If an error occurred while determining metrics handler for the sandbox, skip this container
+				log.G(ctx).Warnf("skipping container %q, failed to get metrics handler: %v", cntr.ID, err.Error())
+				continue
 			}
 			sandboxToMetricsHandler[cntr.SandboxID] = handler
 		} else {
@@ -164,7 +158,9 @@ func (c *criService) toContainerStats(
 			// this is a calculated value and should be computed for all OSes
 			nanoUsage, err := c.getUsageNanoCores(cntr.Metadata.ID, false, cs.stats.Cpu.UsageCoreNanoSeconds.Value, time.Unix(0, cs.stats.Cpu.Timestamp))
 			if err != nil {
-				return nil, fmt.Errorf("failed to get usage nano cores, containerID: %s: %w", cntr.Metadata.ID, err)
+				// If an error occurred when getting nano cores usage, skip the container
+				log.G(ctx).Warnf("skipping container %q, failed to get metrics handler: %v", cntr.ID, err.Error())
+				continue
 			}
 			cs.stats.Cpu.UsageNanoCores = &runtime.UInt64Value{Value: nanoUsage}
 		}

internal/cri/server/container_stats_list_test.go

@@ -20,15 +20,21 @@ import (
 	"context"
 	"math"
 	"reflect"
+	goruntime "runtime"
 	"testing"
 	"time"
 
+	wstats "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/stats"
 	v1 "github.com/containerd/cgroups/v3/cgroup1/stats"
 	v2 "github.com/containerd/cgroups/v3/cgroup2/stats"
 	"github.com/containerd/containerd/api/types"
 	containerstore "github.com/containerd/containerd/v2/internal/cri/store/container"
 	sandboxstore "github.com/containerd/containerd/v2/internal/cri/store/sandbox"
+	"github.com/containerd/containerd/v2/pkg/protobuf"
+	"github.com/containerd/platforms"
+	"github.com/containerd/typeurl/v2"
 	"github.com/stretchr/testify/assert"
+	"google.golang.org/protobuf/types/known/anypb"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 )
 
@@ -346,7 +352,12 @@ func TestContainerMetricsMemory(t *testing.T) {
 }
 
 func TestListContainerStats(t *testing.T) {
+	if goruntime.GOOS == "darwin" {
+		t.Skip("not implemented on Darwin")
+	}
+
 	c := newTestCRIService()
+
 	type args struct {
 		ctx        context.Context
 		stats      []*types.Metric
@@ -422,6 +433,35 @@ func TestListContainerStats(t *testing.T) {
 			wantErr: true,
 			want:    nil,
 		},
+		{
+			name: "args containers has c1 of sandbox s1, s1 exists in sandboxStore, but c1 not exists in containerStore, so filter c1",
+			args: args{
+				ctx: context.Background(),
+				stats: []*types.Metric{
+					{
+						ID:   "c1",
+						Data: platformBasedMetricsData(t),
+					},
+				},
+				containers: []containerstore.Container{
+					{
+						Metadata: containerstore.Metadata{
+							ID:        "c1",
+							SandboxID: "s1",
+						},
+					},
+				},
+			},
+			before: func() {
+				c.sandboxStore.Add(sandboxstore.Sandbox{
+					Metadata: sandboxstore.Metadata{
+						ID: "s1",
+					},
+				})
+			},
+			wantErr: false,
+			want:    &runtime.ListContainerStatsResponse{},
+		},
 	}
 
 	for _, tt := range tests {
@@ -448,3 +488,26 @@ func TestListContainerStats(t *testing.T) {
 	}
 
 }
+
+func platformBasedMetricsData(t *testing.T) *anypb.Any {
+	var data *anypb.Any
+	var err error
+
+	p := platforms.DefaultSpec()
+	switch p.OS {
+	case "windows":
+		data, err = typeurl.MarshalAnyToProto(&wstats.Statistics{Container: &wstats.Statistics_Windows{
+			Windows: &wstats.WindowsContainerStatistics{
+				Timestamp: protobuf.ToTimestamp(time.Now()),
+				Processor: &wstats.WindowsContainerProcessorStatistics{
+					TotalRuntimeNS: 100,
+				},
+			}}})
+	case "linux":
+		data, err = typeurl.MarshalAnyToProto(&v2.Metrics{CPU: &v2.CPUStat{UsageUsec: 100}})
+	default:
+		t.Fail()
+	}
+	assert.NoError(t, err)
+	return data
+}