Skip to content

Commit

Permalink
fix: resolve issue where results may not be obtained from sidecar logs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
l-qing committed Jun 14, 2024
1 parent e506973 commit 122e4dc
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 9 deletions.
47 changes: 38 additions & 9 deletions pkg/pod/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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 {
Expand Down
126 changes: 126 additions & 0 deletions pkg/pod/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 122e4dc

Please sign in to comment.