diff --git a/pkg/cri/sbserver/sandbox_stats_windows.go b/pkg/cri/sbserver/sandbox_stats_windows.go index 40b8eeb07e68..30b163de92a9 100644 --- a/pkg/cri/sbserver/sandbox_stats_windows.go +++ b/pkg/cri/sbserver/sandbox_stats_windows.go @@ -129,6 +129,11 @@ func (c *criService) toPodSandboxStats(sandbox sandboxstore.Sandbox, statsMap ma for _, cntr := range containers { containerMetric := statsMap[cntr.ID] + if cntr.Status.Get().State() != runtime.ContainerState_CONTAINER_RUNNING { + // containers that are just created, in a failed state or exited (init containers) will not have stats + continue + } + if containerMetric == nil { return nil, nil, fmt.Errorf("failed to find metrics for container with id %s: %w", cntr.ID, err) } diff --git a/pkg/cri/sbserver/sandbox_stats_windows_test.go b/pkg/cri/sbserver/sandbox_stats_windows_test.go index 47bd20faab27..21b18ed14575 100644 --- a/pkg/cri/sbserver/sandbox_stats_windows_test.go +++ b/pkg/cri/sbserver/sandbox_stats_windows_test.go @@ -112,7 +112,99 @@ func Test_criService_podSandboxStats(t *testing.T) { }, sandbox: sandboxstore.Sandbox{Metadata: sandboxstore.Metadata{ID: "s1"}}, containers: []containerstore.Container{ - {Metadata: containerstore.Metadata{ID: "c1"}}, + newContainer("c1", running, nil), + }, + expectedPodStats: expectedStats{ + UsageCoreNanoSeconds: 400, + UsageNanoCores: 0, + WorkingSetBytes: 40, + }, + expectedContainerStats: []expectedStats{ + { + UsageCoreNanoSeconds: 200, + UsageNanoCores: 0, + WorkingSetBytes: 20, + }, + }, + expectError: false, + }, + "pod stats will include the init container stats": { + metrics: map[string]*wstats.Statistics{ + "c1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + "s1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + "i1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + }, + sandbox: sandboxstore.Sandbox{Metadata: sandboxstore.Metadata{ID: "s1"}}, + containers: []containerstore.Container{ + newContainer("c1", running, nil), + newContainer("i1", running, nil), + }, + expectedPodStats: expectedStats{ + UsageCoreNanoSeconds: 600, + UsageNanoCores: 0, + WorkingSetBytes: 60, + }, + expectedContainerStats: []expectedStats{ + { + UsageCoreNanoSeconds: 200, + UsageNanoCores: 0, + WorkingSetBytes: 20, + }, + { + UsageCoreNanoSeconds: 200, + UsageNanoCores: 0, + WorkingSetBytes: 20, + }, + }, + expectError: false, + }, + "pod stats will not include the init container stats if it is stopped": { + metrics: map[string]*wstats.Statistics{ + "c1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + "s1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + }, + sandbox: sandboxstore.Sandbox{Metadata: sandboxstore.Metadata{ID: "s1"}}, + containers: []containerstore.Container{ + newContainer("c1", running, nil), + newContainer("i1", exitedValid, nil), + }, + expectedPodStats: expectedStats{ + UsageCoreNanoSeconds: 400, + UsageNanoCores: 0, + WorkingSetBytes: 40, + }, + expectedContainerStats: []expectedStats{ + { + UsageCoreNanoSeconds: 200, + UsageNanoCores: 0, + WorkingSetBytes: 20, + }, + }, + expectError: false, + }, + "pod stats will not include the init container stats if it is stopped in failed state": { + metrics: map[string]*wstats.Statistics{ + "c1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + "s1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + }, + sandbox: sandboxstore.Sandbox{Metadata: sandboxstore.Metadata{ID: "s1"}}, + containers: []containerstore.Container{ + newContainer("c1", running, nil), + newContainer("i1", exitedInvalid, nil), }, expectedPodStats: expectedStats{ UsageCoreNanoSeconds: 400, @@ -139,13 +231,10 @@ func Test_criService_podSandboxStats(t *testing.T) { }, sandbox: sandboxPod("s1", initialStatsTimestamp, 400), containers: []containerstore.Container{ - { - Metadata: containerstore.Metadata{ID: "c1"}, - Stats: &stats.ContainerStats{ - Timestamp: initialStatsTimestamp, - UsageCoreNanoSeconds: 200, - }, - }, + newContainer("c1", running, &stats.ContainerStats{ + Timestamp: initialStatsTimestamp, + UsageCoreNanoSeconds: 200, + }), }, expectedPodStats: expectedStats{ UsageCoreNanoSeconds: 800, @@ -170,13 +259,10 @@ func Test_criService_podSandboxStats(t *testing.T) { }, sandbox: sandboxPod("s1", initialStatsTimestamp, 200), containers: []containerstore.Container{ - { - Metadata: containerstore.Metadata{ID: "c1"}, - Stats: &stats.ContainerStats{ - Timestamp: initialStatsTimestamp, - UsageCoreNanoSeconds: 200, - }, - }, + newContainer("c1", running, &stats.ContainerStats{ + Timestamp: initialStatsTimestamp, + UsageCoreNanoSeconds: 200, + }), }, expectedPodStats: expectedStats{ UsageCoreNanoSeconds: 400, @@ -201,13 +287,10 @@ func Test_criService_podSandboxStats(t *testing.T) { }, sandbox: sandboxPod("s1", initialStatsTimestamp, 200), containers: []containerstore.Container{ - { - Metadata: containerstore.Metadata{ID: "c1"}, - Stats: &stats.ContainerStats{ - Timestamp: initialStatsTimestamp, - UsageCoreNanoSeconds: 200, - }, - }, + newContainer("c1", running, &stats.ContainerStats{ + Timestamp: initialStatsTimestamp, + UsageCoreNanoSeconds: 200, + }), }, expectedPodStats: expectedStats{ UsageCoreNanoSeconds: 400, @@ -230,7 +313,7 @@ func Test_criService_podSandboxStats(t *testing.T) { assert.NotNil(t, err) return } - + assert.Nil(t, err) assert.Equal(t, test.expectedPodStats.UsageCoreNanoSeconds, actualPodStats.Cpu.UsageCoreNanoSeconds.Value) assert.Equal(t, test.expectedPodStats.UsageNanoCores, actualPodStats.Cpu.UsageNanoCores.Value) @@ -266,6 +349,33 @@ func windowsStat(timestamp time.Time, cpu uint64, memory uint64) *wstats.Statist } } +func newContainer(id string, status containerstore.Status, stats *stats.ContainerStats) containerstore.Container { + cntr, err := containerstore.NewContainer(containerstore.Metadata{ID: id}, containerstore.WithFakeStatus(status)) + if err != nil { + panic(err) + } + if stats != nil { + cntr.Stats = stats + } + return cntr +} + +var exitedValid = containerstore.Status{ + StartedAt: time.Now().UnixNano(), + FinishedAt: time.Now().UnixNano(), + ExitCode: 0, +} + +var exitedInvalid = containerstore.Status{ + StartedAt: time.Now().UnixNano(), + FinishedAt: time.Now().UnixNano(), + ExitCode: 1, +} + +var running = containerstore.Status{ + StartedAt: time.Now().UnixNano(), +} + func Test_criService_saveSandBoxMetrics(t *testing.T) { timestamp := time.Now() diff --git a/pkg/cri/server/sandbox_stats_windows.go b/pkg/cri/server/sandbox_stats_windows.go index 5de4a42951c7..37704dba3c12 100644 --- a/pkg/cri/server/sandbox_stats_windows.go +++ b/pkg/cri/server/sandbox_stats_windows.go @@ -71,7 +71,6 @@ func (c *criService) podSandboxStats( podSandboxStats.Windows.Cpu = podCPU.Cpu podSandboxStats.Windows.Memory = podCPU.Memory podSandboxStats.Windows.Containers = containerStats - podSandboxStats.Windows.Network = windowsNetworkUsage(ctx, sandbox, timestamp) pidCount, err := c.getSandboxPidCount(ctx, sandbox) @@ -128,6 +127,11 @@ func (c *criService) toPodSandboxStats(sandbox sandboxstore.Sandbox, statsMap ma for _, cntr := range containers { containerMetric := statsMap[cntr.ID] + if cntr.Status.Get().State() != runtime.ContainerState_CONTAINER_RUNNING { + // containers that are just created, in a failed state or exited (init containers) will not have stats + continue + } + if containerMetric == nil { return nil, nil, fmt.Errorf("failed to find metrics for container with id %s: %w", cntr.ID, err) } diff --git a/pkg/cri/server/sandbox_stats_windows_test.go b/pkg/cri/server/sandbox_stats_windows_test.go index 6bb9efea5671..5a4a6fb1c59b 100644 --- a/pkg/cri/server/sandbox_stats_windows_test.go +++ b/pkg/cri/server/sandbox_stats_windows_test.go @@ -112,7 +112,99 @@ func Test_criService_podSandboxStats(t *testing.T) { }, sandbox: sandboxstore.Sandbox{Metadata: sandboxstore.Metadata{ID: "s1"}}, containers: []containerstore.Container{ - {Metadata: containerstore.Metadata{ID: "c1"}}, + newContainer("c1", running, nil), + }, + expectedPodStats: expectedStats{ + UsageCoreNanoSeconds: 400, + UsageNanoCores: 0, + WorkingSetBytes: 40, + }, + expectedContainerStats: []expectedStats{ + { + UsageCoreNanoSeconds: 200, + UsageNanoCores: 0, + WorkingSetBytes: 20, + }, + }, + expectError: false, + }, + "pod stats will include the init container stats": { + metrics: map[string]*wstats.Statistics{ + "c1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + "s1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + "i1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + }, + sandbox: sandboxstore.Sandbox{Metadata: sandboxstore.Metadata{ID: "s1"}}, + containers: []containerstore.Container{ + newContainer("c1", running, nil), + newContainer("i1", running, nil), + }, + expectedPodStats: expectedStats{ + UsageCoreNanoSeconds: 600, + UsageNanoCores: 0, + WorkingSetBytes: 60, + }, + expectedContainerStats: []expectedStats{ + { + UsageCoreNanoSeconds: 200, + UsageNanoCores: 0, + WorkingSetBytes: 20, + }, + { + UsageCoreNanoSeconds: 200, + UsageNanoCores: 0, + WorkingSetBytes: 20, + }, + }, + expectError: false, + }, + "pod stats will not include the init container stats if it is stopped": { + metrics: map[string]*wstats.Statistics{ + "c1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + "s1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + }, + sandbox: sandboxstore.Sandbox{Metadata: sandboxstore.Metadata{ID: "s1"}}, + containers: []containerstore.Container{ + newContainer("c1", running, nil), + newContainer("i1", exitedValid, nil), + }, + expectedPodStats: expectedStats{ + UsageCoreNanoSeconds: 400, + UsageNanoCores: 0, + WorkingSetBytes: 40, + }, + expectedContainerStats: []expectedStats{ + { + UsageCoreNanoSeconds: 200, + UsageNanoCores: 0, + WorkingSetBytes: 20, + }, + }, + expectError: false, + }, + "pod stats will not include the init container stats if it is stopped in failed state": { + metrics: map[string]*wstats.Statistics{ + "c1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + "s1": { + Container: windowsStat(currentStatsTimestamp, 200, 20), + }, + }, + sandbox: sandboxstore.Sandbox{Metadata: sandboxstore.Metadata{ID: "s1"}}, + containers: []containerstore.Container{ + newContainer("c1", running, nil), + newContainer("i1", exitedInvalid, nil), }, expectedPodStats: expectedStats{ UsageCoreNanoSeconds: 400, @@ -139,13 +231,10 @@ func Test_criService_podSandboxStats(t *testing.T) { }, sandbox: sandboxPod("s1", initialStatsTimestamp, 400), containers: []containerstore.Container{ - { - Metadata: containerstore.Metadata{ID: "c1"}, - Stats: &stats.ContainerStats{ - Timestamp: initialStatsTimestamp, - UsageCoreNanoSeconds: 200, - }, - }, + newContainer("c1", running, &stats.ContainerStats{ + Timestamp: initialStatsTimestamp, + UsageCoreNanoSeconds: 200, + }), }, expectedPodStats: expectedStats{ UsageCoreNanoSeconds: 800, @@ -170,13 +259,10 @@ func Test_criService_podSandboxStats(t *testing.T) { }, sandbox: sandboxPod("s1", initialStatsTimestamp, 200), containers: []containerstore.Container{ - { - Metadata: containerstore.Metadata{ID: "c1"}, - Stats: &stats.ContainerStats{ - Timestamp: initialStatsTimestamp, - UsageCoreNanoSeconds: 200, - }, - }, + newContainer("c1", running, &stats.ContainerStats{ + Timestamp: initialStatsTimestamp, + UsageCoreNanoSeconds: 200, + }), }, expectedPodStats: expectedStats{ UsageCoreNanoSeconds: 400, @@ -201,13 +287,10 @@ func Test_criService_podSandboxStats(t *testing.T) { }, sandbox: sandboxPod("s1", initialStatsTimestamp, 200), containers: []containerstore.Container{ - { - Metadata: containerstore.Metadata{ID: "c1"}, - Stats: &stats.ContainerStats{ - Timestamp: initialStatsTimestamp, - UsageCoreNanoSeconds: 200, - }, - }, + newContainer("c1", running, &stats.ContainerStats{ + Timestamp: initialStatsTimestamp, + UsageCoreNanoSeconds: 200, + }), }, expectedPodStats: expectedStats{ UsageCoreNanoSeconds: 400, @@ -230,7 +313,7 @@ func Test_criService_podSandboxStats(t *testing.T) { assert.NotNil(t, err) return } - + assert.Nil(t, err) assert.Equal(t, test.expectedPodStats.UsageCoreNanoSeconds, actualPodStats.Cpu.UsageCoreNanoSeconds.Value) assert.Equal(t, test.expectedPodStats.UsageNanoCores, actualPodStats.Cpu.UsageNanoCores.Value) @@ -266,6 +349,33 @@ func windowsStat(timestamp time.Time, cpu uint64, memory uint64) *wstats.Statist } } +func newContainer(id string, status containerstore.Status, stats *stats.ContainerStats) containerstore.Container { + cntr, err := containerstore.NewContainer(containerstore.Metadata{ID: id}, containerstore.WithFakeStatus(status)) + if err != nil { + panic(err) + } + if stats != nil { + cntr.Stats = stats + } + return cntr +} + +var exitedValid = containerstore.Status{ + StartedAt: time.Now().UnixNano(), + FinishedAt: time.Now().UnixNano(), + ExitCode: 0, +} + +var exitedInvalid = containerstore.Status{ + StartedAt: time.Now().UnixNano(), + FinishedAt: time.Now().UnixNano(), + ExitCode: 1, +} + +var running = containerstore.Status{ + StartedAt: time.Now().UnixNano(), +} + func Test_criService_saveSandBoxMetrics(t *testing.T) { timestamp := time.Now()