diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index 2efbcc0b7ea..1dbf25258da 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -169,7 +169,11 @@ func StopSidecars(nopImage string, kubeclient kubernetes.Interface, pod corev1.P updated := false if newPod.Status.Phase == corev1.PodRunning { for _, s := range newPod.Status.ContainerStatuses { - if isContainerSidecar(s.Name) && s.State.Running != nil { + // Stop any running container that isn't a step. + // An injected sidecar container might not have the + // "sidecar-" prefix, so we can't just look for that + // prefix. + if !isContainerStep(s.Name) && s.State.Running != nil { for j, c := range newPod.Spec.Containers { if c.Name == s.Name && c.Image != nopImage { updated = true @@ -187,14 +191,17 @@ func StopSidecars(nopImage string, kubeclient kubernetes.Interface, pod corev1.P return nil } -// isContainerStep returns true if the container name indicates that it represents a step. +// isContainerStep returns true if the container name indicates that it +// represents a step. func isContainerStep(name string) bool { return strings.HasPrefix(name, stepPrefix) } -// isContainerSidecar returns true if the container name indicates that it represents a sidecar. +// isContainerSidecar returns true if the container name indicates that it +// represents a sidecar. func isContainerSidecar(name string) bool { return strings.HasPrefix(name, sidecarPrefix) } // trimStepPrefix returns the container name, stripped of its step prefix. func trimStepPrefix(name string) string { return strings.TrimPrefix(name, stepPrefix) } -// trimSidecarPrefix returns the container name, stripped of its sidecar prefix. +// trimSidecarPrefix returns the container name, stripped of its sidecar +// prefix. func trimSidecarPrefix(name string) string { return strings.TrimPrefix(name, sidecarPrefix) } diff --git a/pkg/pod/entrypoint_test.go b/pkg/pod/entrypoint_test.go index e627aedfe7f..3b22b4aad86 100644 --- a/pkg/pod/entrypoint_test.go +++ b/pkg/pod/entrypoint_test.go @@ -168,18 +168,25 @@ func TestStopSidecars(t *testing.T) { Image: "foo", } sidecarContainer := corev1.Container{ - Name: sidecarPrefix + "my-sidecar", - Image: "original-image", - Command: []string{"my", "command"}, - Args: []string{"my", "args"}, - Env: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + Name: sidecarPrefix + "my-sidecar", + Image: "original-image", } stoppedSidecarContainer := corev1.Container{ - Name: sidecarContainer.Name, - Image: nopImage, - Command: []string{"my", "command"}, - Args: []string{"my", "args"}, - Env: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + Name: sidecarContainer.Name, + Image: nopImage, + } + + // This is a container that doesn't start with the "sidecar-" prefix, + // which indicates it was injected into the Pod by a Mutating Webhook + // Admission Controller. Injected sidecars should also be stopped if + // they're running. + injectedSidecar := corev1.Container{ + Name: "injected", + Image: "original-injected-image", + } + stoppedInjectedSidecar := corev1.Container{ + Name: injectedSidecar.Name, + Image: nopImage, } for _, c := range []struct { @@ -187,13 +194,13 @@ func TestStopSidecars(t *testing.T) { pod corev1.Pod wantContainers []corev1.Container }{{ - desc: "Running sidecar should be stopped", + desc: "Running sidecars (incl injected) should be stopped", pod: corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pod", }, Spec: corev1.PodSpec{ - Containers: []corev1.Container{stepContainer, sidecarContainer}, + Containers: []corev1.Container{stepContainer, sidecarContainer, injectedSidecar}, }, Status: corev1.PodStatus{ Phase: corev1.PodRunning, @@ -203,10 +210,14 @@ func TestStopSidecars(t *testing.T) { Name: sidecarContainer.Name, // Sidecar is running. State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}}, + }, { + Name: injectedSidecar.Name, + // Injected sidecar is running. + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}}, }}, }, }, - wantContainers: []corev1.Container{stepContainer, stoppedSidecarContainer}, + wantContainers: []corev1.Container{stepContainer, stoppedSidecarContainer, stoppedInjectedSidecar}, }, { desc: "Pending Pod should not be updated", pod: corev1.Pod{ @@ -221,13 +232,13 @@ func TestStopSidecars(t *testing.T) { }, wantContainers: []corev1.Container{stepContainer, sidecarContainer}, }, { - desc: "Non-Running sidecar should not be stopped", + desc: "Non-Running sidecars should not be stopped", pod: corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pod", }, Spec: corev1.PodSpec{ - Containers: []corev1.Container{stepContainer, sidecarContainer}, + Containers: []corev1.Container{stepContainer, sidecarContainer, injectedSidecar}, }, Status: corev1.PodStatus{ Phase: corev1.PodRunning, @@ -237,10 +248,14 @@ func TestStopSidecars(t *testing.T) { Name: sidecarContainer.Name, // Sidecar is waiting. State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{}}, + }, { + Name: injectedSidecar.Name, + // Injected sidecar is waiting. + State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{}}, }}, }, }, - wantContainers: []corev1.Container{stepContainer, sidecarContainer}, + wantContainers: []corev1.Container{stepContainer, sidecarContainer, injectedSidecar}, }} { t.Run(c.desc, func(t *testing.T) { kubeclient := fakek8s.NewSimpleClientset(&c.pod) diff --git a/pkg/pod/status.go b/pkg/pod/status.go index b1639502631..c04044549d0 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -71,7 +71,11 @@ func SidecarsReady(podStatus corev1.PodStatus) bool { return false } for _, s := range podStatus.ContainerStatuses { - if !isContainerSidecar(s.Name) { + // If the step indicates that it's a step, skip it. + // An injected sidecar might not have the "sidecar-" prefix, so + // we can't just look for that prefix, we need to look at any + // non-step container. + if isContainerStep(s.Name) { continue } if s.State.Running != nil && s.Ready {