Factor out a method to parse termination messages

go.mod

@@ -4,6 +4,7 @@ go 1.13
 
 require (
 	cloud.google.com/go v0.47.0 // indirect
+	cloud.google.com/go/storage v1.0.0
 	contrib.go.opencensus.io/exporter/prometheus v0.1.0 // indirect
 	contrib.go.opencensus.io/exporter/stackdriver v0.12.8 // indirect
 	github.com/Azure/azure-sdk-for-go v36.1.0+incompatible // indirect
@@ -51,7 +52,7 @@ require (
 	golang.org/x/sys v0.0.0-20191119060738-e882bf8e40c2 // indirect
 	golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
 	golang.org/x/tools v0.0.0-20191118222007-07fc4c7f2b98 // indirect
-	google.golang.org/api v0.10.0 // indirect
+	google.golang.org/api v0.10.0
 	google.golang.org/appengine v1.6.5 // indirect
 	google.golang.org/grpc v1.24.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect

pkg/pod/status.go

@@ -25,6 +25,7 @@ import (
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	"github.com/tektoncd/pipeline/pkg/logging"
+	"github.com/tektoncd/pipeline/pkg/termination"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/apis"
@@ -119,8 +120,8 @@ func MakeTaskRunStatus(tr v1alpha1.TaskRun, pod *corev1.Pod, taskSpec v1alpha1.T
 		if isContainerStep(s.Name) {
 			if s.State.Terminated != nil && len(s.State.Terminated.Message) != 0 {
 				msg := s.State.Terminated.Message
-				var r []v1alpha1.PipelineResourceResult
-				if err := json.Unmarshal([]byte(msg), &r); err != nil {
+				r, err := termination.ParseMessage(msg)
+				if err != nil {
 					logger.Errorf("Could not parse json message %q because of %w", msg, err)
 					break
 				}

pkg/reconciler/taskrun/taskrun.go

@@ -18,7 +18,6 @@ package taskrun
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"reflect"
@@ -35,6 +34,7 @@ import (
 	"github.com/tektoncd/pipeline/pkg/reconciler"
 	"github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
 	"github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources/cloudevent"
+	"github.com/tektoncd/pipeline/pkg/termination"
 	"github.com/tektoncd/pipeline/pkg/workspace"
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
@@ -357,7 +357,9 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error
 	// Convert the Pod's status to the equivalent TaskRun Status.
 	tr.Status = podconvert.MakeTaskRunStatus(*tr, pod, *taskSpec)
 
-	updateTaskRunResourceResult(tr, pod, c.Logger)
+	if err := updateTaskRunResourceResult(tr, pod.Status); err != nil {
+		return err
+	}
 
 	after := tr.Status.GetCondition(apis.ConditionSucceeded)
 
@@ -397,28 +399,19 @@ func (c *Reconciler) handlePodCreationError(tr *v1alpha1.TaskRun, err error) {
 	c.Logger.Errorf("Failed to create build pod for task %q: %v", tr.Name, err)
 }
 
-func updateTaskRunResourceResult(taskRun *v1alpha1.TaskRun, pod *corev1.Pod, logger *zap.SugaredLogger) {
+func updateTaskRunResourceResult(taskRun *v1alpha1.TaskRun, podStatus corev1.PodStatus) error {
 	if taskRun.IsSuccessful() {
-		for _, cs := range pod.Status.ContainerStatuses {
+		for idx, cs := range podStatus.ContainerStatuses {
 			if cs.State.Terminated != nil {
 				msg := cs.State.Terminated.Message
-				if msg != "" {
-					if err := updateTaskRunStatusWithResourceResult(taskRun, []byte(msg)); err != nil {
-						logger.Infof("No resource result from %s for %s/%s: %s", cs.Name, taskRun.Name, taskRun.Namespace, err)
-					}
+				r, err := termination.ParseMessage(msg)
+				if err != nil {
+					return fmt.Errorf("parsing message for container status %d: %v", idx, err)
 				}
+				taskRun.Status.ResourcesResult = append(taskRun.Status.ResourcesResult, r...)
 			}
 		}
 	}
-}
-
-// updateTaskRunStatusWithResourceResult if there is an update to the outout image resource, add to taskrun status result
-func updateTaskRunStatusWithResourceResult(taskRun *v1alpha1.TaskRun, logContent []byte) error {
-	results := []v1alpha1.PipelineResourceResult{}
-	if err := json.Unmarshal(logContent, &results); err != nil {
-		return fmt.Errorf("failed to unmarshal output image exporter JSON output: %w", err)
-	}
-	taskRun.Status.ResourcesResult = append(taskRun.Status.ResourcesResult, results...)
 	return nil
 }
 

pkg/reconciler/taskrun/taskrun_test.go

@@ -1515,42 +1515,22 @@ func TestReconcileCloudEvents(t *testing.T) {
 	}
 }
 
-func TestUpdateTaskRunStatus_withValidJson(t *testing.T) {
+func TestUpdateTaskRunResourceResult(t *testing.T) {
 	for _, c := range []struct {
-		desc    string
-		podLog  []byte
-		taskRun *v1alpha1.TaskRun
-		want    []v1alpha1.PipelineResourceResult
+		desc          string
+		podStatus     corev1.PodStatus
+		taskRunStatus *v1alpha1.TaskRunStatus
+		want          []v1alpha1.PipelineResourceResult
 	}{{
-		desc:   "image resource updated",
-		podLog: []byte("[{\"name\":\"source-image\",\"digest\":\"sha256:1234\"}]"),
-		taskRun: &v1alpha1.TaskRun{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test-taskrun-run-output-steps",
-				Namespace: "marshmallow",
-			},
-			Spec: v1alpha1.TaskRunSpec{
-				Inputs: v1alpha1.TaskRunInputs{
-					Resources: []v1alpha1.TaskResourceBinding{{
-						PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-							Name: "source-image",
-							ResourceRef: &v1alpha1.PipelineResourceRef{
-								Name: "source-image-1",
-							},
-						},
-					}},
-				},
-				Outputs: v1alpha1.TaskRunOutputs{
-					Resources: []v1alpha1.TaskResourceBinding{{
-						PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-							Name: "source-image",
-							ResourceRef: &v1alpha1.PipelineResourceRef{
-								Name: "source-image-1",
-							},
-						},
-					}},
+		desc: "image resource updated",
+		podStatus: corev1.PodStatus{
+			ContainerStatuses: []corev1.ContainerStatus{{
+				State: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{
+						Message: `[{"name":"source-image","digest":"sha256:1234"}]`,
+					},
 				},
-			},
+			}},
 		},
 		want: []v1alpha1.PipelineResourceResult{{
 			Name:   "source-image",
@@ -1559,80 +1539,53 @@ func TestUpdateTaskRunStatus_withValidJson(t *testing.T) {
 	}} {
 		t.Run(c.desc, func(t *testing.T) {
 			names.TestingSeed()
-			c.taskRun.Status.SetCondition(&apis.Condition{
+			tr := &v1alpha1.TaskRun{}
+			tr.Status.SetCondition(&apis.Condition{
 				Type:   apis.ConditionSucceeded,
 				Status: corev1.ConditionTrue,
 			})
-			if err := updateTaskRunStatusWithResourceResult(c.taskRun, c.podLog); err != nil {
-				t.Errorf("UpdateTaskRunStatusWithResourceResult failed with error: %s", err)
+			if err := updateTaskRunResourceResult(tr, c.podStatus); err != nil {
+				t.Errorf("updateTaskRunResourceResult: %s", err)
 			}
-			if d := cmp.Diff(c.want, c.taskRun.Status.ResourcesResult); d != "" {
-				t.Errorf("post build steps mismatch (-want, +got): %s", d)
+			if d := cmp.Diff(c.want, tr.Status.ResourcesResult); d != "" {
+				t.Errorf("updateTaskRunResourceResult (-want, +got): %s", d)
 			}
 		})
 	}
 }
 
-func TestUpdateTaskRunStatus_withInvalidJson(t *testing.T) {
+func TestUpdateTaskRunResourceResult_Errors(t *testing.T) {
 	for _, c := range []struct {
-		desc    string
-		podLog  []byte
-		taskRun *v1alpha1.TaskRun
-		want    []v1alpha1.PipelineResourceResult
+		desc          string
+		podStatus     corev1.PodStatus
+		taskRunStatus *v1alpha1.TaskRunStatus
+		want          []v1alpha1.PipelineResourceResult
 	}{{
-		desc:   "image resource exporter with malformed json output",
-		podLog: []byte("extralogscamehere[{\"name\":\"source-image\",\"digest\":\"sha256:1234\"}]"),
-		taskRun: &v1alpha1.TaskRun{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test-taskrun-run-output-steps",
-				Namespace: "marshmallow",
-			},
-			Spec: v1alpha1.TaskRunSpec{
-				Inputs: v1alpha1.TaskRunInputs{
-					Resources: []v1alpha1.TaskResourceBinding{{
-						PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-							Name: "source-image",
-							ResourceRef: &v1alpha1.PipelineResourceRef{
-								Name: "source-image-1",
-							},
-						},
-					}},
-				},
-				Outputs: v1alpha1.TaskRunOutputs{
-					Resources: []v1alpha1.TaskResourceBinding{{
-						PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-							Name: "source-image",
-							ResourceRef: &v1alpha1.PipelineResourceRef{
-								Name: "source-image-1",
-							},
-						},
-					}},
+		desc: "image resource exporter with malformed json output",
+		podStatus: corev1.PodStatus{
+			ContainerStatuses: []corev1.ContainerStatus{{
+				State: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{
+						Message: `MALFORMED JSON{"digest":"sha256:1234"}`,
+					},
 				},
-			},
+			}},
 		},
-		want: nil,
-	}, {
-		desc:   "task with no image resource ",
-		podLog: []byte(""),
-		taskRun: &v1alpha1.TaskRun{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test-taskrun-run-output-steps",
-				Namespace: "marshmallow",
-			},
+		taskRunStatus: &v1alpha1.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()
-			c.taskRun.Status.SetCondition(&apis.Condition{
-				Type:   apis.ConditionSucceeded,
-				Status: corev1.ConditionTrue,
-			})
-			if err := updateTaskRunStatusWithResourceResult(c.taskRun, c.podLog); err == nil {
-				t.Error("UpdateTaskRunStatusWithResourceResult expected to fail with error")
+			if err := updateTaskRunResourceResult(&v1alpha1.TaskRun{Status: *c.taskRunStatus}, c.podStatus); err == nil {
+				t.Error("Expected error, got nil")
 			}
-			if d := cmp.Diff(c.want, c.taskRun.Status.ResourcesResult); d != "" {
-				t.Errorf("post build steps mismatch (-want, +got): %s", d)
+			if d := cmp.Diff(c.want, c.taskRunStatus.ResourcesResult); d != "" {
+				t.Errorf("updateTaskRunResourceResult (-want, +got): %s", d)
 			}
 		})
 	}

pkg/termination/parse.go

@@ -0,0 +1,35 @@
+/*
+Copyright 2019 The Tekton Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package termination
+
+import (
+	"encoding/json"
+	"fmt"
+
+	v1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
+)
+
+// ParseMessage parses a termination message as results.
+func ParseMessage(msg string) ([]v1alpha1.PipelineResourceResult, error) {
+	if msg == "" {
+		return nil, nil
+	}
+	var r []v1alpha1.PipelineResourceResult
+	if err := json.Unmarshal([]byte(msg), &r); err != nil {
+		return nil, fmt.Errorf("parsing message json: %v", err)
+	}
+	return r, nil
+}

pkg/termination/parse_test.go

@@ -0,0 +1,59 @@
+/*
+Copyright 2019 The Tekton Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package termination
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	v1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
+)
+
+func TestParseMessage(t *testing.T) {
+	for _, c := range []struct {
+		desc, msg string
+		want      []v1alpha1.PipelineResourceResult
+	}{{
+		desc: "valid message",
+		msg:  `[{"digest":"foo"},{"key":"foo","value":"bar"}]`,
+		want: []v1alpha1.PipelineResourceResult{{
+			Digest: "foo",
+		}, {
+			Key:   "foo",
+			Value: "bar",
+		}},
+	}, {
+		desc: "empty message",
+		msg:  "",
+		want: nil,
+	}} {
+		t.Run(c.desc, func(t *testing.T) {
+			got, err := ParseMessage(c.msg)
+			if err != nil {
+				t.Fatalf("ParseMessage: %v", err)
+			}
+			if d := cmp.Diff(c.want, got); d != "" {
+				t.Fatalf("ParseMessage(-want,+got): %s", d)
+			}
+		})
+	}
+}
+
+func TestParseMessage_Invalid(t *testing.T) {
+	if _, err := ParseMessage("INVALID NOT JSON"); err == nil {
+		t.Error("Expected error parsing invalid JSON, got nil")
+	}
+}

pkg/termination/termination.go → pkg/termination/write.go

@@ -23,6 +23,7 @@ import (
 	v1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 )
 
+// WriteMessage writes the results to the termination message path.
 func WriteMessage(path string, pro []v1alpha1.PipelineResourceResult) error {
 	// if the file at path exists, concatenate the new values otherwise create it
 	// file at path already exists