Handle injected sidecars more correctly

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) }

pkg/pod/entrypoint_test.go

@@ -168,32 +168,39 @@ 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 {
 		desc           string
 		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)

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 {