diff --git a/pkg/kubelet/kuberuntime/helpers.go b/pkg/kubelet/kuberuntime/helpers.go index 8901b0f8ea0..bc1dbd90be8 100644 --- a/pkg/kubelet/kuberuntime/helpers.go +++ b/pkg/kubelet/kuberuntime/helpers.go @@ -42,20 +42,28 @@ func (b podsByID) Len() int { return len(b) } func (b podsByID) Swap(i, j int) { b[i], b[j] = b[j], b[i] } func (b podsByID) Less(i, j int) bool { return b[i].ID < b[j].ID } -type containersByID []*kubecontainer.Container +type containerByCreatedThenID []*runtimeapi.Container -func (b containersByID) Len() int { return len(b) } -func (b containersByID) Swap(i, j int) { b[i], b[j] = b[j], b[i] } -func (b containersByID) Less(i, j int) bool { return b[i].ID.ID < b[j].ID.ID } +func (b containerByCreatedThenID) Len() int { return len(b) } +func (b containerByCreatedThenID) Swap(i, j int) { b[i], b[j] = b[j], b[i] } +func (b containerByCreatedThenID) Less(i, j int) bool { + if b[i].CreatedAt != b[j].CreatedAt { + return b[i].CreatedAt > (b[j].CreatedAt) + } + return b[i].Id < b[j].Id +} // Newest first. -type podSandboxByCreated []*runtimeapi.PodSandbox +type podSandboxByCreatedThenID []*runtimeapi.PodSandbox -func (p podSandboxByCreated) Len() int { return len(p) } -func (p podSandboxByCreated) Swap(i, j int) { p[i], p[j] = p[j], p[i] } -func (p podSandboxByCreated) Less(i, j int) bool { - if p[i].Metadata == nil || p[j].Metadata == nil { - return p[i].CreatedAt > p[j].CreatedAt +func (p podSandboxByCreatedThenID) Len() int { return len(p) } +func (p podSandboxByCreatedThenID) Swap(i, j int) { p[i], p[j] = p[j], p[i] } +func (p podSandboxByCreatedThenID) Less(i, j int) bool { + if p[i].Metadata == nil || p[j].Metadata == nil || (p[i].Metadata.Attempt == p[j].Metadata.Attempt) { + if p[i].CreatedAt != p[j].CreatedAt { + return p[i].CreatedAt > p[j].CreatedAt + } + return p[i].Id < p[j].Id } return p[i].Metadata.Attempt > p[j].Metadata.Attempt } diff --git a/pkg/kubelet/kuberuntime/helpers_test.go b/pkg/kubelet/kuberuntime/helpers_test.go index 606d94cf16b..0f9e7f82680 100644 --- a/pkg/kubelet/kuberuntime/helpers_test.go +++ b/pkg/kubelet/kuberuntime/helpers_test.go @@ -19,7 +19,9 @@ package kuberuntime import ( "context" "fmt" + "sort" "testing" + "time" "github.com/stretchr/testify/assert" @@ -600,3 +602,218 @@ func TestConvertResourceConfigToLinuxContainerResources(t *testing.T) { assert.Equal(t, *resCfg.Memory, lcr.MemoryLimitInBytes) assert.Equal(t, resCfg.Unified, lcr.Unified) } + +func TestContainerByCreatedThenID(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + input []*runtimeapi.Container + expected []*runtimeapi.Container + }{ + { + name: "different CreatedAt sort by time desc (newest first)", + input: []*runtimeapi.Container{ + {Id: "c1", CreatedAt: now.Add(-30 * time.Minute).Unix()}, + {Id: "c2", CreatedAt: now.Unix()}, + {Id: "c3", CreatedAt: now.Add(-10 * time.Minute).Unix()}, + }, + expected: []*runtimeapi.Container{ + {Id: "c2"}, // newest + {Id: "c3"}, + {Id: "c1"}, // oldest + }, + }, + { + name: "same CreatedAt sort by ID asc", + input: []*runtimeapi.Container{ + {Id: "container-zzz", CreatedAt: now.Unix()}, + {Id: "container-aaa", CreatedAt: now.Unix()}, + {Id: "container-mmm", CreatedAt: now.Unix()}, + }, + expected: []*runtimeapi.Container{ + {Id: "container-aaa"}, + {Id: "container-mmm"}, + {Id: "container-zzz"}, + }, + }, + { + name: "mixed: some same time, some different", + input: []*runtimeapi.Container{ + {Id: "c-old-2", CreatedAt: now.Add(-1 * time.Hour).Unix()}, + {Id: "c-new-1", CreatedAt: now.Unix()}, + {Id: "c-group-b", CreatedAt: now.Unix()}, + {Id: "c-group-a", CreatedAt: now.Unix()}, + {Id: "c-old-1", CreatedAt: now.Add(-1 * time.Hour).Unix()}, + }, + expected: []*runtimeapi.Container{ + {Id: "c-group-a"}, + {Id: "c-group-b"}, + {Id: "c-new-1"}, + {Id: "c-old-1"}, + {Id: "c-old-2"}, + }, + }, + { + name: "empty slice", + input: []*runtimeapi.Container{}, + expected: []*runtimeapi.Container{}, + }, + { + name: "single element", + input: []*runtimeapi.Container{ + {Id: "only-one", CreatedAt: now.Add(-5 * time.Minute).Unix()}, + }, + expected: []*runtimeapi.Container{ + {Id: "only-one"}, + }, + }, + { + name: "all CreatedAt same, IDs already sorted should remain stable", + input: []*runtimeapi.Container{ + {Id: "id-001", CreatedAt: now.Unix()}, + {Id: "id-002", CreatedAt: now.Unix()}, + {Id: "id-003", CreatedAt: now.Unix()}, + }, + expected: []*runtimeapi.Container{ + {Id: "id-001"}, + {Id: "id-002"}, + {Id: "id-003"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + containers := make([]*runtimeapi.Container, len(tt.input)) + copy(containers, tt.input) + + sort.Sort(containerByCreatedThenID(containers)) + + assert.Len(t, containers, len(tt.expected), "length mismatch") + + for i := range tt.expected { + assert.Equal(t, tt.expected[i].Id, containers[i].Id, "ID mismatch at index %d", i) + } + }) + } +} + +func TestPodSandboxByCreatedThenID(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + input []*runtimeapi.PodSandbox + expected []string + }{ + { + name: "different Attempt higher Attempt first", + input: []*runtimeapi.PodSandbox{ + { + Id: "sandbox-c", + CreatedAt: now.UnixNano(), + Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 0}, + }, + { + Id: "sandbox-a", + CreatedAt: now.UnixNano(), + Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 2}, + }, + { + Id: "sandbox-b", + CreatedAt: now.UnixNano(), + Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 1}, + }, + }, + expected: []string{"sandbox-a", "sandbox-b", "sandbox-c"}, + }, + { + name: "same Attempt newer CreatedAt first", + input: []*runtimeapi.PodSandbox{ + { + Id: "old", + CreatedAt: now.Add(-10 * time.Minute).UnixNano(), + Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 5}, + }, + { + Id: "newest", + CreatedAt: now.UnixNano(), + Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 5}, + }, + { + Id: "medium", + CreatedAt: now.Add(-5 * time.Minute).UnixNano(), + Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 5}, + }, + }, + expected: []string{"newest", "medium", "old"}, + }, + { + name: "same Attempt and CreatedAt sort by Id ascending", + input: []*runtimeapi.PodSandbox{ + {Id: "zzz-3", CreatedAt: now.UnixNano(), Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 1}}, + {Id: "aaa-1", CreatedAt: now.UnixNano(), Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 1}}, + {Id: "mmm-2", CreatedAt: now.UnixNano(), Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 1}}, + }, + expected: []string{"aaa-1", "mmm-2", "zzz-3"}, + }, + { + name: "mixed: Attempt CreatedAt + Id", + input: []*runtimeapi.PodSandbox{ + {Id: "c1", CreatedAt: now.UnixNano(), Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 0}}, + {Id: "a2", CreatedAt: now.Add(-20 * time.Minute).UnixNano(), Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 2}}, + {Id: "b3", CreatedAt: now.UnixNano(), Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 1}}, + {Id: "group-b", CreatedAt: now.UnixNano(), Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 1}}, + {Id: "group-a", CreatedAt: now.UnixNano(), Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 1}}, + }, + expected: []string{ + "a2", + "b3", + "group-a", + "group-b", + "c1", + }, + }, + { + name: "empty slice", + input: []*runtimeapi.PodSandbox{}, + expected: []string{}, + }, + { + name: "single sandbox", + input: []*runtimeapi.PodSandbox{ + {Id: "single", CreatedAt: now.UnixNano(), Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 3}}, + }, + expected: []string{"single"}, + }, + { + name: "Metadata nil fallback only CreatedAt then Id", + input: []*runtimeapi.PodSandbox{ + {Id: "nil-b", CreatedAt: now.UnixNano(), Metadata: nil}, + {Id: "nil-a", CreatedAt: now.UnixNano(), Metadata: nil}, + {Id: "with-meta", CreatedAt: now.UnixNano(), Metadata: &runtimeapi.PodSandboxMetadata{Attempt: 0}}, + }, + expected: []string{"nil-a", "nil-b", "with-meta"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sandboxes := make([]*runtimeapi.PodSandbox, len(tt.input)) + copy(sandboxes, tt.input) + + sort.Sort(podSandboxByCreatedThenID(sandboxes)) + + assert.Len(t, sandboxes, len(tt.expected), "length mismatch") + + actualIDs := make([]string, len(sandboxes)) + for i, s := range sandboxes { + actualIDs[i] = s.Id + } + + assert.Equal(t, tt.expected, actualIDs, "sorting order mismatch") + }) + } +} diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go index b3dba2c520d..e9988172f90 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go @@ -482,7 +482,7 @@ func (m *kubeGenericRuntimeManager) getPods(ctx context.Context, opts listOption return nil, err } // Sort sandboxes by creation time, newest first. - sort.Sort(podSandboxByCreated(sandboxes)) + sort.Sort(podSandboxByCreatedThenID(sandboxes)) for i := range sandboxes { s := sandboxes[i] if s.Metadata == nil { @@ -511,6 +511,13 @@ func (m *kubeGenericRuntimeManager) getPods(ctx context.Context, opts listOption if err != nil { return nil, err } + // Sort containers: newest CreatedAt first, then by container ID for stability. + // There are scenarios where multiple pods are running in parallel having + // the same name, because one of them have not been fully terminated yet. + // To avoid unexpected behavior on container name based search (for example + // by calling *Kubelet.findContainer() without specifying a pod ID), we + // return the list of pods ordered by their creation time. + sort.Sort(containerByCreatedThenID(containers)) for i := range containers { c := containers[i] if c.Metadata == nil { diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go index 75b3dd50cb8..a5810eda401 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go @@ -251,14 +251,6 @@ func verifyPods(a, b []*kubecontainer.Pod) bool { return false } - // Sort the containers within a pod. - for i := range a { - sort.Sort(containersByID(a[i].Containers)) - } - for i := range b { - sort.Sort(containersByID(b[i].Containers)) - } - // Sort the pods by UID. sort.Sort(podsByID(a)) sort.Sort(podsByID(b)) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go index ef4f3ad40ff..843838cae2f 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go @@ -327,7 +327,7 @@ func (m *kubeGenericRuntimeManager) getSandboxIDByPodUID(ctx context.Context, po // Sort with newest first. sandboxIDs := make([]string, len(sandboxes)) - sort.Sort(podSandboxByCreated(sandboxes)) + sort.Sort(podSandboxByCreatedThenID(sandboxes)) for i, s := range sandboxes { sandboxIDs[i] = s.Id }