diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 905fa98ea15..87282584633 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -1695,11 +1695,15 @@ func (kl *Kubelet) convertToAPIContainerStatuses(pod *v1.Pod, podStatus *kubecon statuses[container.Name] = status } + // Copy the slice before sorting it + containerStatusesCopy := make([]*kubecontainer.Status, len(podStatus.ContainerStatuses)) + copy(containerStatusesCopy, podStatus.ContainerStatuses) + // Make the latest container status comes first. - sort.Sort(sort.Reverse(kubecontainer.SortContainerStatusesByCreationTime(podStatus.ContainerStatuses))) + sort.Sort(sort.Reverse(kubecontainer.SortContainerStatusesByCreationTime(containerStatusesCopy))) // Set container statuses according to the statuses seen in pod status containerSeen := map[string]int{} - for _, cStatus := range podStatus.ContainerStatuses { + for _, cStatus := range containerStatusesCopy { cName := cStatus.Name if _, ok := statuses[cName]; !ok { // This would also ignore the infra container. diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index 0bb28d26a02..d3b1d185421 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -26,6 +26,7 @@ import ( "reflect" "sort" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -3220,3 +3221,33 @@ func TestSortPodIPs(t *testing.T) { }) } } + +func TestConvertToAPIContainerStatusesDataRace(t *testing.T) { + pod := podWithUIDNameNs("12345", "test-pod", "test-namespace") + + testTimestamp := time.Unix(123456789, 987654321) + + criStatus := &kubecontainer.PodStatus{ + ID: pod.UID, + Name: pod.Name, + Namespace: pod.Namespace, + ContainerStatuses: []*kubecontainer.Status{ + {Name: "containerA", CreatedAt: testTimestamp}, + {Name: "containerB", CreatedAt: testTimestamp.Add(1)}, + }, + } + + testKubelet := newTestKubelet(t, false) + defer testKubelet.Cleanup() + kl := testKubelet.kubelet + + // convertToAPIContainerStatuses is purely transformative and shouldn't alter the state of the kubelet + // as there are no synchronisation events in that function (no locks, no channels, ...) each test routine + // should have its own vector clock increased independently. Golang race detector uses pure happens-before + // detection, so would catch a race condition consistently, despite only spawning 2 goroutines + for i := 0; i < 2; i++ { + go func() { + kl.convertToAPIContainerStatuses(pod, criStatus, []v1.ContainerStatus{}, []v1.Container{}, false, false) + }() + } +}