diff --git a/pkg/apis/pipeline/v1beta1/task_types.go b/pkg/apis/pipeline/v1beta1/task_types.go index 34fe2c3bb87..405527f8b32 100644 --- a/pkg/apis/pipeline/v1beta1/task_types.go +++ b/pkg/apis/pipeline/v1beta1/task_types.go @@ -26,6 +26,8 @@ const ( TaskRunResultType ResultType = "TaskRunResult" // PipelineResourceResultType default pipeline result value PipelineResourceResultType ResultType = "PipelineResourceResult" + // InternalTektonResultType default internal tekton result value + InternalTektonResultType ResultType = "InternalTektonResult" // UnknownResultType default unknown result type value UnknownResultType ResultType = "" ) diff --git a/pkg/entrypoint/entrypointer.go b/pkg/entrypoint/entrypointer.go index 188344a8fe7..6c9ed4edbc4 100644 --- a/pkg/entrypoint/entrypointer.go +++ b/pkg/entrypoint/entrypointer.go @@ -102,8 +102,9 @@ func (e Entrypointer) Go() error { // *but* we write postfile to make next steps bail too. e.WritePostFile(e.PostFile, err) output = append(output, v1beta1.PipelineResourceResult{ - Key: "StartedAt", - Value: time.Now().Format(timeFormat), + Key: "StartedAt", + Value: time.Now().Format(timeFormat), + ResultType: v1beta1.InternalTektonResultType, }) return err @@ -114,8 +115,9 @@ func (e Entrypointer) Go() error { e.Args = append([]string{e.Entrypoint}, e.Args...) } output = append(output, v1beta1.PipelineResourceResult{ - Key: "StartedAt", - Value: time.Now().Format(timeFormat), + Key: "StartedAt", + Value: time.Now().Format(timeFormat), + ResultType: v1beta1.InternalTektonResultType, }) err := e.Runner.Run(e.Args...) diff --git a/pkg/pod/status.go b/pkg/pod/status.go index f7d9ad66625..67256c1349a 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -23,6 +23,7 @@ import ( "strings" "time" + "github.com/hashicorp/go-multierror" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/names" "github.com/tektoncd/pipeline/pkg/termination" @@ -97,89 +98,178 @@ func SidecarsReady(podStatus corev1.PodStatus) bool { } // MakeTaskRunStatus returns a TaskRunStatus based on the Pod's status. -func MakeTaskRunStatus(logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev1.Pod, taskSpec v1beta1.TaskSpec) v1beta1.TaskRunStatus { +func MakeTaskRunStatus(logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev1.Pod, taskSpec v1beta1.TaskSpec) (v1beta1.TaskRunStatus, error) { trs := &tr.Status if trs.GetCondition(apis.ConditionSucceeded) == nil || trs.GetCondition(apis.ConditionSucceeded).Status == corev1.ConditionUnknown { // If the taskRunStatus doesn't exist yet, it's because we just started running MarkStatusRunning(trs, v1beta1.TaskRunReasonRunning.String(), "Not all Steps in the Task have finished executing") } + // Complete if we did not find a step that is not complete, or the pod is in a definitely complete phase + complete := areStepsComplete(pod) || pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed + + if complete { + updateCompletedTaskRun(trs, pod) + } else { + updateIncompleteTaskRun(trs, pod) + } + trs.PodName = pod.Name trs.Steps = []v1beta1.StepState{} trs.Sidecars = []v1beta1.SidecarState{} + var stepStatuses []corev1.ContainerStatus + var sidecarStatuses []corev1.ContainerStatus for _, s := range pod.Status.ContainerStatuses { if IsContainerStep(s.Name) { - if s.State.Terminated != nil && len(s.State.Terminated.Message) != 0 { - message, time, err := removeStartInfoFromTerminationMessage(s) + stepStatuses = append(stepStatuses, s) + } else if isContainerSidecar(s.Name) { + sidecarStatuses = append(sidecarStatuses, s) + } + } + + var merr *multierror.Error + if err := setTaskRunStatusBasedOnStepStatus(logger, stepStatuses, &tr); err != nil { + merr = multierror.Append(merr, err) + } + + setTaskRunStatusBasedOnSidecarStatus(sidecarStatuses, trs) + + trs.TaskRunResults = removeDuplicateResults(trs.TaskRunResults) + + // Sort step states according to the order specified in the TaskRun spec's steps. + trs.Steps = sortTaskRunStepOrder(trs.Steps, taskSpec.Steps) + + return *trs, merr.ErrorOrNil() +} + +func setTaskRunStatusBasedOnStepStatus(logger *zap.SugaredLogger, stepStatuses []corev1.ContainerStatus, tr *v1beta1.TaskRun) *multierror.Error { + + trs := &tr.Status + var merr *multierror.Error + + for _, s := range stepStatuses { + if s.State.Terminated != nil && len(s.State.Terminated.Message) != 0 { + msg := s.State.Terminated.Message + + results, err := termination.ParseMessage(msg) + if err != nil { + logger.Errorf("termination message could not be parsed as JSON: %v", err) + merr = multierror.Append(merr, err) + } else { + time, err := extractStartedAtTimeFromResults(results) if err != nil { - logger.Errorf("error setting the start time of step %q in taskrun %q: %w", s.Name, tr.Name, err) + logger.Errorf("error setting the start time of step %q in taskrun %q: %v", s.Name, tr.Name, err) + merr = multierror.Append(merr, err) + } + taskResults, pipelineResourceResults, filteredResults := filterResultsAndResources(results) + if tr.IsSuccessful() { + trs.TaskRunResults = append(trs.TaskRunResults, taskResults...) + trs.ResourcesResult = append(trs.ResourcesResult, pipelineResourceResults...) } if time != nil { s.State.Terminated.StartedAt = *time - s.State.Terminated.Message = message + msg, err = createMessageFromResults(filteredResults) + if err != nil { + logger.Errorf("%v", err) + err = multierror.Append(merr, err) + } + s.State.Terminated.Message = msg } } - trs.Steps = append(trs.Steps, v1beta1.StepState{ - ContainerState: *s.State.DeepCopy(), - Name: trimStepPrefix(s.Name), - ContainerName: s.Name, - ImageID: s.ImageID, - }) - } else if isContainerSidecar(s.Name) { - trs.Sidecars = append(trs.Sidecars, v1beta1.SidecarState{ - ContainerState: *s.State.DeepCopy(), - Name: TrimSidecarPrefix(s.Name), - ContainerName: s.Name, - ImageID: s.ImageID, - }) } + trs.Steps = append(trs.Steps, v1beta1.StepState{ + ContainerState: *s.State.DeepCopy(), + Name: trimStepPrefix(s.Name), + ContainerName: s.Name, + ImageID: s.ImageID, + }) } - // Complete if we did not find a step that is not complete, or the pod is in a definitely complete phase - complete := areStepsComplete(pod) || pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed + return merr - if complete { - updateCompletedTaskRun(trs, pod) - } else { - updateIncompleteTaskRun(trs, pod) +} + +func setTaskRunStatusBasedOnSidecarStatus(sidecarStatuses []corev1.ContainerStatus, trs *v1beta1.TaskRunStatus) { + for _, s := range sidecarStatuses { + trs.Sidecars = append(trs.Sidecars, v1beta1.SidecarState{ + ContainerState: *s.State.DeepCopy(), + Name: TrimSidecarPrefix(s.Name), + ContainerName: s.Name, + ImageID: s.ImageID, + }) } +} - // Sort step states according to the order specified in the TaskRun spec's steps. - trs.Steps = sortTaskRunStepOrder(trs.Steps, taskSpec.Steps) +func createMessageFromResults(results []v1beta1.PipelineResourceResult) (string, error) { + if len(results) == 0 { + return "", nil + } + bytes, err := json.Marshal(results) + if err != nil { + return "", fmt.Errorf("error marshalling remaining results back into termination message: %w", err) + } + return string(bytes), nil +} - return *trs +func filterResultsAndResources(results []v1beta1.PipelineResourceResult) ([]v1beta1.TaskRunResult, []v1beta1.PipelineResourceResult, []v1beta1.PipelineResourceResult) { + var taskResults []v1beta1.TaskRunResult + var pipelineResourceResults []v1beta1.PipelineResourceResult + var filteredResults []v1beta1.PipelineResourceResult + for _, r := range results { + switch r.ResultType { + case v1beta1.TaskRunResultType: + taskRunResult := v1beta1.TaskRunResult{ + Name: r.Key, + Value: r.Value, + } + taskResults = append(taskResults, taskRunResult) + filteredResults = append(filteredResults, r) + case v1beta1.InternalTektonResultType: + // Internal messages are ignored because they're not used as external result + continue + case v1beta1.PipelineResourceResultType: + fallthrough + default: + pipelineResourceResults = append(pipelineResourceResults, r) + filteredResults = append(filteredResults, r) + } + } + + return taskResults, pipelineResourceResults, filteredResults } -// removeStartInfoFromTerminationMessage searches for a result called "StartedAt" in the JSON-formatted -// termination message of a step and returns the values to use for sets State.Terminated if it's -// found. The "StartedAt" result is also removed from the list of results in the container status. -func removeStartInfoFromTerminationMessage(s corev1.ContainerStatus) (string, *metav1.Time, error) { - r, err := termination.ParseMessage(s.State.Terminated.Message) - if err != nil { - return "", nil, fmt.Errorf("termination message could not be parsed as JSON: %w", err) +func removeDuplicateResults(taskRunResult []v1beta1.TaskRunResult) []v1beta1.TaskRunResult { + if len(taskRunResult) == 0 { + return nil } - for index, result := range r { + + uniq := make([]v1beta1.TaskRunResult, 0) + latest := make(map[string]v1beta1.TaskRunResult, 0) + for _, res := range taskRunResult { + if _, seen := latest[res.Name]; !seen { + uniq = append(uniq, res) + } + latest[res.Name] = res + } + for i, res := range uniq { + uniq[i] = latest[res.Name] + } + return uniq +} + +func extractStartedAtTimeFromResults(results []v1beta1.PipelineResourceResult) (*metav1.Time, error) { + for _, result := range results { if result.Key == "StartedAt" { t, err := time.Parse(timeFormat, result.Value) if err != nil { - return "", nil, fmt.Errorf("could not parse time value %q in StartedAt field: %w", result.Value, err) + return nil, fmt.Errorf("could not parse time value %q in StartedAt field: %w", result.Value, err) } - message := "" startedAt := metav1.NewTime(t) - // remove the entry for the starting time - r = append(r[:index], r[index+1:]...) - if len(r) == 0 { - message = "" - } else if bytes, err := json.Marshal(r); err != nil { - return "", nil, fmt.Errorf("error marshalling remaining results back into termination message: %w", err) - } else { - message = string(bytes) - } - return message, &startedAt, nil + return &startedAt, nil } } - return "", nil, nil + return nil, nil } func updateCompletedTaskRun(trs *v1beta1.TaskRunStatus, pod *corev1.Pod) { diff --git a/pkg/pod/status_test.go b/pkg/pod/status_test.go index 11a6913315d..d73e28f04eb 100644 --- a/pkg/pod/status_test.go +++ b/pkg/pod/status_test.go @@ -33,13 +33,20 @@ import ( var ignoreVolatileTime = cmp.Comparer(func(_, _ apis.VolatileTime) bool { return true }) +var conditionRunning apis.Condition = apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: v1beta1.TaskRunReasonRunning.String(), + Message: "Not all Steps in the Task have finished executing", +} +var conditionSucceeded apis.Condition = apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + Reason: v1beta1.TaskRunReasonSuccessful.String(), + Message: "All Steps have completed executing", +} + func TestMakeTaskRunStatus(t *testing.T) { - conditionRunning := apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionUnknown, - Reason: v1beta1.TaskRunReasonRunning.String(), - Message: "Not all Steps in the Task have finished executing", - } for _, c := range []struct { desc string podStatus corev1.PodStatus @@ -604,6 +611,336 @@ func TestMakeTaskRunStatus(t *testing.T) { }}, }, }, + }, { + desc: "image resource updated", + podStatus: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-foo", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"digest","value":"sha256:12345","resourceRef":{"name":"source-image"}}]`, + }, + }, + }}, + }, + want: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: []apis.Condition{conditionSucceeded}, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"digest","value":"sha256:12345","resourceRef":{"name":"source-image"}}]`, + }}, + Name: "foo", + ContainerName: "step-foo", + }}, + Sidecars: []v1beta1.SidecarState{}, + ResourcesResult: []v1beta1.PipelineResourceResult{{ + Key: "digest", + Value: "sha256:12345", + ResourceRef: v1beta1.PipelineResourceRef{Name: "source-image"}, + }}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }, { + desc: "test result with pipeline result", + podStatus: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-bar", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`, + }, + }, + }}, + }, + want: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: []apis.Condition{conditionSucceeded}, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`, + }}, + Name: "bar", + ContainerName: "step-bar", + }}, + Sidecars: []v1beta1.SidecarState{}, + ResourcesResult: []v1beta1.PipelineResourceResult{{ + Key: "digest", + Value: "sha256:1234", + ResourceRef: v1beta1.PipelineResourceRef{Name: "source-image"}, + }}, + TaskRunResults: []v1beta1.TaskRunResult{{ + Name: "resultName", + Value: "resultValue", + }}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }, { + desc: "test result with pipeline result - no result type", + podStatus: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-banana", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`, + }, + }, + }}, + }, + want: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: []apis.Condition{conditionSucceeded}, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`, + }}, + Name: "banana", + ContainerName: "step-banana", + }}, + Sidecars: []v1beta1.SidecarState{}, + ResourcesResult: []v1beta1.PipelineResourceResult{{ + Key: "digest", + Value: "sha256:1234", + ResourceRef: v1beta1.PipelineResourceRef{Name: "source-image"}, + }}, + TaskRunResults: []v1beta1.TaskRunResult{{ + Name: "resultName", + Value: "resultValue", + }}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }, { + desc: "two test results", + podStatus: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-one", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultNameOne","value":"resultValueOne", "type": "TaskRunResult"}, {"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`, + }, + }, + }, { + Name: "step-two", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultNameOne","value":"resultValueThree", "type": "TaskRunResult"}, {"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`, + }, + }, + }}, + }, + want: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: []apis.Condition{conditionSucceeded}, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultNameOne","value":"resultValueOne", "type": "TaskRunResult"}, {"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`, + }}, + Name: "one", + ContainerName: "step-one", + }, { + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultNameOne","value":"resultValueThree", "type": "TaskRunResult"}, {"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`, + }}, + Name: "two", + ContainerName: "step-two", + }}, + Sidecars: []v1beta1.SidecarState{}, + TaskRunResults: []v1beta1.TaskRunResult{{ + Name: "resultNameOne", + Value: "resultValueThree", + }, { + Name: "resultNameTwo", + Value: "resultValueTwo", + }}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }, { + desc: "update task results when task fails", + podStatus: corev1.PodStatus{ + Phase: corev1.PodFailed, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-mango", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"name":"source-image","digest":"sha256:1234"}]`, + }, + }, + }}, + }, + want: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{Conditions: []apis.Condition{{ + Reason: "Failed", + Message: "build failed for unspecified reasons.", + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + }}}, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"name":"source-image","digest":"sha256:1234"}]`, + }}, + Name: "mango", + ContainerName: "step-mango", + }}, + Sidecars: []v1beta1.SidecarState{}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }, { + desc: "filter internaltektonresult", + podStatus: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-pear", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultNameOne","value":"", "type": "PipelineResourceResult"}, {"key":"resultNameTwo","value":"", "type": "InternalTektonResult"}, {"key":"resultNameThree","value":"", "type": "TaskRunResult"}]`, + }, + }, + }}, + }, + want: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: []apis.Condition{conditionSucceeded}, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultNameOne","value":"", "type": "PipelineResourceResult"}, {"key":"resultNameTwo","value":"", "type": "InternalTektonResult"}, {"key":"resultNameThree","value":"", "type": "TaskRunResult"}]`, + }}, + Name: "pear", + ContainerName: "step-pear", + }}, + Sidecars: []v1beta1.SidecarState{}, + ResourcesResult: []v1beta1.PipelineResourceResult{{ + Key: "resultNameOne", + Value: "", + ResultType: "PipelineResourceResult", + }}, + TaskRunResults: []v1beta1.TaskRunResult{{ + Name: "resultNameThree", + Value: "", + }}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }} { + t.Run(c.desc, func(t *testing.T) { + now := metav1.Now() + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod", + Namespace: "foo", + CreationTimestamp: now, + }, + Status: c.podStatus, + } + startTime := time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC) + tr := v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-run", + Namespace: "foo", + }, + Status: v1beta1.TaskRunStatus{ + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + StartTime: &metav1.Time{Time: startTime}, + }, + }, + } + + logger, _ := logging.NewLogger("", "status") + got, err := MakeTaskRunStatus(logger, tr, pod, c.taskSpec) + + // Common traits, set for test case brevity. + c.want.PodName = "pod" + c.want.StartTime = &metav1.Time{Time: startTime} + + ensureTimeNotNil := cmp.Comparer(func(x, y *metav1.Time) bool { + if x == nil { + return y == nil + } + return y != nil + }) + if err != nil { + t.Errorf("MakeTaskRunResult: %s", err) + } + if d := cmp.Diff(c.want, got, ignoreVolatileTime, ensureTimeNotNil); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } + if tr.Status.StartTime.Time != c.want.StartTime.Time { + t.Errorf("Expected TaskRun startTime to be unchanged but was %s", tr.Status.StartTime) + } + }) + } +} + +func TestMakeRunStatusErrors(t *testing.T) { + for _, c := range []struct { + desc string + podStatus corev1.PodStatus + taskSpec v1beta1.TaskSpec + want v1beta1.TaskRunStatus + }{{ + desc: "malformed StartedAt time is shown in term message", + podStatus: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-bar", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"StartedAt", "value":"invalid", "type":"InternalTektonResult"}]`, + }, + }, + }}, + Phase: corev1.PodSucceeded, + }, + want: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: []apis.Condition{conditionSucceeded}, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"StartedAt", "value":"invalid", "type":"InternalTektonResult"}]`, + }, + }, + Name: "bar", + ContainerName: "step-bar", + }}, + Sidecars: []v1beta1.SidecarState{}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, }, { desc: "non-json-termination-message-with-steps-afterwards-shouldnt-panic", taskSpec: v1beta1.TaskSpec{ @@ -664,7 +1001,6 @@ func TestMakeTaskRunStatus(t *testing.T) { ExitCode: 1, Message: "this is a non-json termination message. dont panic!", }}, - Name: "non-json", ContainerName: "step-non-json", ImageID: "image", @@ -694,34 +1030,24 @@ func TestMakeTaskRunStatus(t *testing.T) { }, }} { t.Run(c.desc, func(t *testing.T) { - now := metav1.Now() pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: "pod", - Namespace: "foo", - CreationTimestamp: now, + Name: "pod", + Namespace: "foo", }, Status: c.podStatus, } - startTime := time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC) tr := v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ Name: "task-run", Namespace: "foo", }, - Status: v1beta1.TaskRunStatus{ - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - StartTime: &metav1.Time{Time: startTime}, - }, - }, } logger, _ := logging.NewLogger("", "status") - got := MakeTaskRunStatus(logger, tr, pod, c.taskSpec) + got, err := MakeTaskRunStatus(logger, tr, pod, c.taskSpec) - // Common traits, set for test case brevity. c.want.PodName = "pod" - c.want.StartTime = &metav1.Time{Time: startTime} ensureTimeNotNil := cmp.Comparer(func(x, y *metav1.Time) bool { if x == nil { @@ -729,12 +1055,12 @@ func TestMakeTaskRunStatus(t *testing.T) { } return y != nil }) + if err == nil { + t.Error("Expected error, got nil") + } if d := cmp.Diff(c.want, got, ignoreVolatileTime, ensureTimeNotNil); d != "" { t.Errorf("Diff %s", diff.PrintWantGot(d)) } - if tr.Status.StartTime.Time != c.want.StartTime.Time { - t.Errorf("Expected TaskRun startTime to be unchanged but was %s", tr.Status.StartTime) - } }) } } diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 24ff8fefbbb..fcc2736f01e 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -41,7 +41,6 @@ import ( "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim" - "github.com/tektoncd/pipeline/pkg/termination" "github.com/tektoncd/pipeline/pkg/timeout" "github.com/tektoncd/pipeline/pkg/workspace" corev1 "k8s.io/api/core/v1" @@ -185,7 +184,9 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg // Reconcile this copy of the task run and then write back any status // updates regardless of whether the reconciliation errored out. if err = c.reconcile(ctx, tr, taskSpec, rtr); err != nil { - logger.Errorf("Reconcile error: %v", err.Error()) + if merr, ok := err.(*multierror.Error); ok { + logger.Errorf("Reconcile: %v", merr.Error()) + } } // Emit events (only when ConditionSucceeded was changed) return c.finishReconcileUpdateEmitEvents(ctx, tr, before, err) @@ -397,9 +398,8 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun, } // Convert the Pod's status to the equivalent TaskRun Status. - tr.Status = podconvert.MakeTaskRunStatus(logger, *tr, pod, *taskSpec) - - if err := updateTaskRunResourceResult(tr, *pod); err != nil { + tr.Status, err = podconvert.MakeTaskRunStatus(logger, *tr, pod, *taskSpec) + if err != nil { return err } @@ -627,62 +627,6 @@ func (c *Reconciler) createPod(ctx context.Context, tr *v1beta1.TaskRun, rtr *re type DeletePod func(podName string, options *metav1.DeleteOptions) error -func updateTaskRunResourceResult(taskRun *v1beta1.TaskRun, pod corev1.Pod) error { - podconvert.SortContainerStatuses(&pod) - - if taskRun.IsSuccessful() { - for idx, cs := range pod.Status.ContainerStatuses { - if cs.State.Terminated != nil { - msg := cs.State.Terminated.Message - r, err := termination.ParseMessage(msg) - if err != nil { - return fmt.Errorf("parsing message for container status %d: %v", idx, err) - } - taskResults, pipelineResourceResults := getResults(r) - taskRun.Status.TaskRunResults = append(taskRun.Status.TaskRunResults, taskResults...) - taskRun.Status.ResourcesResult = append(taskRun.Status.ResourcesResult, pipelineResourceResults...) - } - } - taskRun.Status.TaskRunResults = removeDuplicateResults(taskRun.Status.TaskRunResults) - } - return nil -} - -func getResults(results []v1beta1.PipelineResourceResult) ([]v1beta1.TaskRunResult, []v1beta1.PipelineResourceResult) { - var taskResults []v1beta1.TaskRunResult - var pipelineResourceResults []v1beta1.PipelineResourceResult - for _, r := range results { - switch r.ResultType { - case v1beta1.TaskRunResultType: - taskRunResult := v1beta1.TaskRunResult{ - Name: r.Key, - Value: r.Value, - } - taskResults = append(taskResults, taskRunResult) - case v1beta1.PipelineResourceResultType: - fallthrough - default: - pipelineResourceResults = append(pipelineResourceResults, r) - } - } - return taskResults, pipelineResourceResults -} - -func removeDuplicateResults(taskRunResult []v1beta1.TaskRunResult) []v1beta1.TaskRunResult { - uniq := make([]v1beta1.TaskRunResult, 0) - latest := make(map[string]v1beta1.TaskRunResult, 0) - for _, res := range taskRunResult { - if _, seen := latest[res.Name]; !seen { - uniq = append(uniq, res) - } - latest[res.Name] = res - } - for i, res := range uniq { - uniq[i] = latest[res.Name] - } - return uniq -} - func isExceededResourceQuotaError(err error) bool { return err != nil && k8serrors.IsForbidden(err) && strings.Contains(err.Error(), "exceeded quota") } diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 9f6897fd2c2..0ca7dc3e1ca 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -53,7 +53,6 @@ import ( ktesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/record" "knative.dev/pkg/apis" - duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" "knative.dev/pkg/configmap" "knative.dev/pkg/controller" "knative.dev/pkg/logging" @@ -2318,277 +2317,6 @@ func TestReconcileCloudEvents(t *testing.T) { } } -func TestUpdateTaskRunResourceResult(t *testing.T) { - for _, c := range []struct { - desc string - pod corev1.Pod - taskRunStatus *v1beta1.TaskRunStatus - want []resourcev1alpha1.PipelineResourceResult - }{{ - desc: "image resource updated", - pod: corev1.Pod{ - Status: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{{ - State: corev1.ContainerState{ - Terminated: &corev1.ContainerStateTerminated{ - Message: `[{"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`, - }, - }, - }}, - }, - }, - want: []resourcev1alpha1.PipelineResourceResult{{ - Key: "digest", - Value: "sha256:1234", - ResourceRef: resourcev1alpha1.PipelineResourceRef{Name: "source-image"}, - }}, - }} { - t.Run(c.desc, func(t *testing.T) { - names.TestingSeed() - tr := &v1beta1.TaskRun{} - tr.Status.SetCondition(&apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }) - if err := updateTaskRunResourceResult(tr, c.pod); err != nil { - t.Errorf("updateTaskRunResourceResult: %s", err) - } - if d := cmp.Diff(c.want, tr.Status.ResourcesResult); d != "" { - t.Errorf("updateTaskRunResourceResult %s", diff.PrintWantGot(d)) - } - }) - } -} - -func TestUpdateTaskRunResult(t *testing.T) { - for _, c := range []struct { - desc string - pod corev1.Pod - taskRunStatus *v1beta1.TaskRunStatus - wantResults []v1beta1.TaskRunResult - want []resourcev1alpha1.PipelineResourceResult - }{{ - desc: "test result with pipeline result", - pod: corev1.Pod{ - Status: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{{ - State: corev1.ContainerState{ - Terminated: &corev1.ContainerStateTerminated{ - Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}, "type": "PipelineResourceResult"}]`, - }, - }, - }}, - }, - }, - wantResults: []v1beta1.TaskRunResult{{ - Name: "resultName", - Value: "resultValue", - }}, - want: []resourcev1alpha1.PipelineResourceResult{{ - Key: "digest", - Value: "sha256:1234", - ResourceRef: resourcev1alpha1.PipelineResourceRef{Name: "source-image"}, - ResultType: "PipelineResourceResult", - }}, - }} { - t.Run(c.desc, func(t *testing.T) { - names.TestingSeed() - tr := &v1beta1.TaskRun{} - tr.Status.SetCondition(&apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }) - if err := updateTaskRunResourceResult(tr, c.pod); err != nil { - t.Errorf("updateTaskRunResourceResult: %s", err) - } - if d := cmp.Diff(c.wantResults, tr.Status.TaskRunResults); d != "" { - t.Errorf("updateTaskRunResourceResult TaskRunResults %s", diff.PrintWantGot(d)) - } - if d := cmp.Diff(c.want, tr.Status.ResourcesResult); d != "" { - t.Errorf("updateTaskRunResourceResult ResourcesResult %s", diff.PrintWantGot(d)) - } - }) - } -} - -func TestUpdateTaskRunResult2(t *testing.T) { - for _, c := range []struct { - desc string - pod corev1.Pod - taskRunStatus *v1beta1.TaskRunStatus - wantResults []v1beta1.TaskRunResult - want []resourcev1alpha1.PipelineResourceResult - }{{ - desc: "test result with pipeline result - no result type", - pod: corev1.Pod{ - Status: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{{ - State: corev1.ContainerState{ - Terminated: &corev1.ContainerStateTerminated{ - Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`, - }, - }, - }}, - }, - }, - wantResults: []v1beta1.TaskRunResult{{ - Name: "resultName", - Value: "resultValue", - }}, - want: []resourcev1alpha1.PipelineResourceResult{{ - Key: "digest", - Value: "sha256:1234", - ResourceRef: resourcev1alpha1.PipelineResourceRef{Name: "source-image"}, - }}, - }} { - t.Run(c.desc, func(t *testing.T) { - names.TestingSeed() - tr := &v1beta1.TaskRun{} - tr.Status.SetCondition(&apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }) - if err := updateTaskRunResourceResult(tr, c.pod); err != nil { - t.Errorf("updateTaskRunResourceResult: %s", err) - } - if d := cmp.Diff(c.wantResults, tr.Status.TaskRunResults); d != "" { - t.Errorf("updateTaskRunResourceResult %s", diff.PrintWantGot(d)) - } - if d := cmp.Diff(c.want, tr.Status.ResourcesResult); d != "" { - t.Errorf("updateTaskRunResourceResult %s", diff.PrintWantGot(d)) - } - }) - } -} - -func TestUpdateTaskRunResultTwoResults(t *testing.T) { - for _, c := range []struct { - desc string - pod corev1.Pod - taskRunStatus *v1beta1.TaskRunStatus - want []v1beta1.TaskRunResult - }{{ - desc: "two test results", - pod: corev1.Pod{ - Status: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{{ - State: corev1.ContainerState{ - Terminated: &corev1.ContainerStateTerminated{ - Message: `[{"key":"resultNameOne","value":"resultValueOne", "type": "TaskRunResult"},{"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`, - }, - }, - }, { - State: corev1.ContainerState{ - Terminated: &corev1.ContainerStateTerminated{ - Message: `[{"key":"resultNameOne","value":"resultValueThree", "type": "TaskRunResult"},{"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`, - }, - }, - }}, - }, - }, - want: []v1beta1.TaskRunResult{{ - Name: "resultNameOne", - Value: "resultValueThree", - }, { - Name: "resultNameTwo", - Value: "resultValueTwo", - }}, - }} { - t.Run(c.desc, func(t *testing.T) { - names.TestingSeed() - tr := &v1beta1.TaskRun{} - tr.Status.SetCondition(&apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }) - if err := updateTaskRunResourceResult(tr, c.pod); err != nil { - t.Errorf("updateTaskRunResourceResult: %s", err) - } - if d := cmp.Diff(c.want, tr.Status.TaskRunResults); d != "" { - t.Errorf("updateTaskRunResourceResult %s", diff.PrintWantGot(d)) - } - }) - } -} - -func TestUpdateTaskRunResultWhenTaskFailed(t *testing.T) { - for _, c := range []struct { - desc string - podStatus corev1.PodStatus - taskRunStatus *v1beta1.TaskRunStatus - wantResults []v1beta1.TaskRunResult - want []resourcev1alpha1.PipelineResourceResult - }{{ - desc: "update task results when task fails", - podStatus: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{{ - State: corev1.ContainerState{ - Terminated: &corev1.ContainerStateTerminated{ - Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"name":"source-image","digest":"sha256:1234"}]`, - }, - }, - }}, - }, - taskRunStatus: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionFalse, - }}}, - }, - wantResults: nil, - want: nil, - }} { - t.Run(c.desc, func(t *testing.T) { - names.TestingSeed() - if d := cmp.Diff(c.want, c.taskRunStatus.ResourcesResult); d != "" { - t.Errorf("updateTaskRunResourceResult resources %s", diff.PrintWantGot(d)) - } - if d := cmp.Diff(c.wantResults, c.taskRunStatus.TaskRunResults); d != "" { - t.Errorf("updateTaskRunResourceResult results %s", diff.PrintWantGot(d)) - } - }) - } -} - -func TestUpdateTaskRunResourceResult_Errors(t *testing.T) { - for _, c := range []struct { - desc string - pod corev1.Pod - taskRunStatus *v1beta1.TaskRunStatus - want []resourcev1alpha1.PipelineResourceResult - }{{ - desc: "image resource exporter with malformed json output", - pod: corev1.Pod{ - Status: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{{ - State: corev1.ContainerState{ - Terminated: &corev1.ContainerStateTerminated{ - Message: `MALFORMED JSON{"digest":"sha256:1234"}`, - }, - }, - }}, - }, - }, - taskRunStatus: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}}, - }, - want: nil, - }} { - t.Run(c.desc, func(t *testing.T) { - names.TestingSeed() - if err := updateTaskRunResourceResult(&v1beta1.TaskRun{Status: *c.taskRunStatus}, c.pod); err == nil { - t.Error("Expected error, got nil") - } - if d := cmp.Diff(c.want, c.taskRunStatus.ResourcesResult); d != "" { - t.Errorf("updateTaskRunResourceResult %s", diff.PrintWantGot(d)) - } - }) - } -} - func TestReconcile_Single_SidecarState(t *testing.T) { runningState := corev1.ContainerStateRunning{StartedAt: metav1.Time{Time: time.Now()}} taskRun := tb.TaskRun("test-taskrun-sidecars",