diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 168077bacdf..27828cda433 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -253,11 +253,6 @@ func (c *Reconciler) stopSidecars(ctx context.Context, tr *v1beta1.TaskRun) erro return nil } - // do not continue if the TaskSpec had no sidecars - if tr.Status.TaskSpec != nil && len(tr.Status.TaskSpec.Sidecars) == 0 && len(tr.Status.Sidecars) == 0 { - return nil - } - // do not continue if the TaskRun was canceled or timed out as this caused the pod to be deleted in failTaskRun condition := tr.Status.GetCondition(apis.ConditionSucceeded) if condition != nil { @@ -267,11 +262,6 @@ func (c *Reconciler) stopSidecars(ctx context.Context, tr *v1beta1.TaskRun) erro } } - // do not continue if there are no Running sidecars. - if !podconvert.IsSidecarStatusRunning(tr) { - return nil - } - pod, err := podconvert.StopSidecars(ctx, c.Images.NopImage, c.KubeClientSet, tr.Namespace, tr.Status.PodName) if err == nil { // Check if any SidecarStatuses are still shown as Running after stopping diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index de6b97d203e..bc71a4c2fb2 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -4078,65 +4078,85 @@ status: } } -func TestStopSidecars_NoClientGetPodForTaskSpecWithoutRunningSidecars(t *testing.T) { - for _, tc := range []struct { - desc string - tr *v1beta1.TaskRun - }{{ - desc: "no sidecars", - tr: parse.MustParseV1beta1TaskRun(t, ` -metadata: - name: test-taskrun - namespace: foo -status: - conditions: - - status: "True" - type: Succeeded - podName: test-taskrun-pod - startTime: "2000-01-01T01:01:01Z" -`), - }, { - desc: "sidecars are terminated", - tr: parse.MustParseV1beta1TaskRun(t, ` +func TestStopSidecars_WithInjectedSidecarsNoTaskSpecSidecars(t *testing.T) { + taskRun := parse.MustParseV1beta1TaskRun(t, ` metadata: - name: test-taskrun + name: test-taskrun-injected-sidecars namespace: foo +spec: + taskRef: + name: test-task status: + podName: test-taskrun-injected-sidecars-pod conditions: - - status: "True" + - message: Build succeeded + reason: Build succeeded + status: "True" type: Succeeded - podName: test-taskrun-pod - sidecars: - - terminated: - exitCode: 0 - finishedAt: "2000-01-01T01:01:01Z" - startedAt: "2000-01-01T01:01:01Z" - startTime: "2000-01-01T01:01:01Z" -`), - }} { - t.Run(tc.desc, func(t *testing.T) { - d := test.Data{ - TaskRuns: []*v1beta1.TaskRun{tc.tr}, - } +`) - testAssets, cancel := getTaskRunController(t, d) - defer cancel() - c := testAssets.Controller - clients := testAssets.Clients - reconcileErr := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tc.tr)) + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-taskrun-injected-sidecars-pod", + Namespace: "foo", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "step-do-something", + Image: "my-step-image", + }, + { + Name: "injected-sidecar", + Image: "some-image", + }, + }, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "step-do-something", + State: v1.ContainerState{Terminated: &corev1.ContainerStateTerminated{}}, + }, + { + Name: "injected-sidecar", + State: v1.ContainerState{Running: &corev1.ContainerStateRunning{}}, + }, + }, + }, + } - // We do not expect an error - if reconcileErr != nil { - t.Errorf("Expected no error to be returned by reconciler: %v", reconcileErr) - } + d := test.Data{ + Pods: []*corev1.Pod{pod}, + TaskRuns: []*v1beta1.TaskRun{taskRun}, + Tasks: []*v1beta1.Task{simpleTask}, + } - // Verify that the pod was not retrieved - for _, action := range clients.Kube.Actions() { - if action.Matches("get", "pods") { - t.Errorf("expected the pod not to be retrieved because the TaskRun has no sidecars") - } - } - }) + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + c := testAssets.Controller + clients := testAssets.Clients + + reconcileErr := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)) + + // We do not expect an error + if reconcileErr != nil { + t.Errorf("Expected no error to be returned by reconciler: %v", reconcileErr) + } + + retrievedPod, err := clients.Kube.CoreV1().Pods(pod.Namespace).Get(testAssets.Ctx, pod.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("error retrieving pod: %s", err) + } + + if len(retrievedPod.Spec.Containers) != 2 { + t.Fatalf("expected pod with two containers") + } + + // check that injected sidecar is replaced with nop image + if d := cmp.Diff(retrievedPod.Spec.Containers[1].Image, images.NopImage); d != "" { + t.Errorf("expected injected sidecar image to be replaced with nop image %s", diff.PrintWantGot(d)) } }