From 122e4dc6ff40880cb1aa22839bc7b30d9fb15e53 Mon Sep 17 00:00:00 2001 From: l-qing Date: Thu, 6 Jun 2024 15:56:07 +0800 Subject: [PATCH] fix: resolve issue where results may not be obtained from sidecar logs fix: #8028 If the sidecar is not completed, the complete log may not be retrieved from it, thus unable to parse the final task results. --- pkg/pod/status.go | 47 ++++++++++++--- pkg/pod/status_test.go | 126 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 9 deletions(-) diff --git a/pkg/pod/status.go b/pkg/pod/status.go index 81b5e0e4979..59a18db7414 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -125,7 +125,7 @@ func MakeTaskRunStatus(ctx context.Context, logger *zap.SugaredLogger, tr v1.Tas sortPodContainerStatuses(pod.Status.ContainerStatuses, pod.Spec.Containers) - complete := areStepsComplete(pod) || pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed + complete := areContainersCompleted(ctx, pod) || pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed if complete { onError, ok := tr.Annotations[v1.PipelineTaskOnErrorAnnotation] @@ -614,16 +614,45 @@ func IsPodArchived(pod *corev1.Pod, trs *v1.TaskRunStatus) bool { return false } -func areStepsComplete(pod *corev1.Pod) bool { - stepsComplete := len(pod.Status.ContainerStatuses) > 0 && pod.Status.Phase == corev1.PodRunning - for _, s := range pod.Status.ContainerStatuses { - if IsContainerStep(s.Name) { - if s.State.Terminated == nil { - stepsComplete = false - } +// containerNameFilter is a function that filters container names. +type containerNameFilter func(name string) bool + +// isMatchingAnyFilter returns true if the container name matches any of the filters. +func isMatchingAnyFilter(name string, filters []containerNameFilter) bool { + for _, filter := range filters { + if filter(name) { + return true + } + } + return false +} + +// areContainersCompleted returns true if all related containers in the pod are completed. +func areContainersCompleted(ctx context.Context, pod *corev1.Pod) bool { + nameFilters := []containerNameFilter{IsContainerStep} + if config.FromContextOrDefaults(ctx).FeatureFlags.ResultExtractionMethod == config.ResultExtractionMethodSidecarLogs { + // If we are using sidecar logs to extract results, we need to wait for the sidecar to complete. + // Avoid failing to obtain the final result from the sidecar because the sidecar is not yet complete. + nameFilters = append(nameFilters, func(name string) bool { + return name == pipeline.ReservedResultsSidecarContainerName + }) + } + return checkContainersCompleted(pod, nameFilters) +} + +// checkContainersCompleted returns true if containers in the pod are completed. +func checkContainersCompleted(pod *corev1.Pod, nameFilters []containerNameFilter) bool { + if len(pod.Status.ContainerStatuses) == 0 || + !(pod.Status.Phase == corev1.PodRunning || pod.Status.Phase == corev1.PodSucceeded) { + return false + } + for _, containerStatus := range pod.Status.ContainerStatuses { + if isMatchingAnyFilter(containerStatus.Name, nameFilters) && containerStatus.State.Terminated == nil { + // if any container is not completed, return false + return false } } - return stepsComplete + return true } func getFailureMessage(logger *zap.SugaredLogger, pod *corev1.Pod) string { diff --git a/pkg/pod/status_test.go b/pkg/pod/status_test.go index 44ac4df717c..b298aba1a54 100644 --- a/pkg/pod/status_test.go +++ b/pkg/pod/status_test.go @@ -1781,6 +1781,132 @@ func TestMakeRunStatus_OnError(t *testing.T) { } } +func TestMakeTaskRunStatus_SidecarNotCompleted(t *testing.T) { + for _, c := range []struct { + desc string + podStatus corev1.PodStatus + pod corev1.Pod + taskSpec v1.TaskSpec + want v1.TaskRunStatus + }{{ + desc: "test sidecar not completed", + podStatus: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "other-prefix-container", + State: corev1.ContainerState{ + Terminated: nil, + }, + }, + { + Name: "step-bar", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultName","value":"", "type":1}, {"key":"digest","value":"sha256:1234","resourceName":"source-image"}]`, + }, + }, + }, + { + Name: "sidecar-tekton-log-results", + State: corev1.ContainerState{ + Terminated: nil, + }, + }, + }, + }, + taskSpec: v1.TaskSpec{ + Results: []v1.TaskResult{ + { + Name: "resultName", + Type: v1.ResultsTypeString, + }, + }, + }, + want: v1.TaskRunStatus{ + Status: statusRunning(), + }, + }, { + desc: "test sidecar already completed", + podStatus: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "step-bar", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultName","value":"", "type":1}, {"key":"digest","value":"sha256:1234","resourceName":"source-image"}]`, + }, + }, + }, + { + Name: "sidecar-tekton-log-results", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + ExitCode: 0, + }, + }, + }, + }, + }, + taskSpec: v1.TaskSpec{ + Results: []v1.TaskResult{ + { + Name: "resultName", + Type: v1.ResultsTypeString, + }, + }, + }, + want: v1.TaskRunStatus{ + Status: statusSuccess(), + }, + }} { + t.Run(c.desc, func(t *testing.T) { + c.pod = corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod", + Namespace: "foo", + CreationTimestamp: metav1.Now(), + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "other-prefix-container", + }, { + Name: "step-bar", + }, { + Name: "sidecar-tekton-log-results", + }}, + }, + Status: c.podStatus, + } + + tr := v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-run", + Namespace: "foo", + }, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + TaskSpec: &c.taskSpec, + }, + }, + } + logger, _ := logging.NewLogger("", "status") + kubeclient := fakek8s.NewSimpleClientset() + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + ResultExtractionMethod: config.ResultExtractionMethodSidecarLogs, + MaxResultSize: 1024, + }, + }) + got, _ := MakeTaskRunStatus(ctx, logger, tr, &c.pod, kubeclient, &c.taskSpec) + if d := cmp.Diff(c.want.Status, got.Status, ignoreVolatileTime); d != "" { + t.Errorf("Unexpected status: %s", diff.PrintWantGot(d)) + } + }) + } +} + func TestMakeTaskRunStatusAlpha(t *testing.T) { for _, c := range []struct { desc string