From 51a92ab71ce96140fef8ea74651b9dcd22450db2 Mon Sep 17 00:00:00 2001 From: Nader Ziada Date: Fri, 10 May 2019 11:55:01 -0400 Subject: [PATCH] improve returned errors in outoutimageexporter + other small fixes --- docs/resources.md | 4 ++-- pkg/apis/pipeline/v1alpha1/image_resource.go | 11 +++++++---- .../v1alpha1/taskrun/resources/image_exporter.go | 16 ++++++---------- .../taskrun/resources/image_exporter_test.go | 14 ++++++-------- pkg/reconciler/v1alpha1/taskrun/taskrun.go | 2 +- 5 files changed, 22 insertions(+), 25 deletions(-) diff --git a/docs/resources.md b/docs/resources.md index a8c20617bd8..a6f6c6bb281 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -164,9 +164,9 @@ spec: #### Surfacing the image digest built in a task -To surface the image digest in the output of the `taskRun` the builder tool should produce this information in a [OCI Image Spec](https://github.com/opencontainers/image-spec/blob/master/image-layout.md) `index.json` file. This file should be placed on a location as specified in the task definition under the resource `outputImageDir`. +To surface the image digest in the output of the `taskRun` the builder tool should produce this information in a [OCI Image Spec](https://github.com/opencontainers/image-spec/blob/master/image-layout.md) `index.json` file. This file should be placed on a location as specified in the task definition under the resource `outputImageDir`. Annotations in `index.json` will be ignored, and if there are multiple versions of the image, the latest will be used. -For example this build-push task defines the `outputImageDir` for the `buildImage` resource in `/worksapce/buildImage` +For example this build-push task defines the `outputImageDir` for the `builtImage` resource in `/workspace/buildImage` ```yaml apiVersion: tekton.dev/v1alpha1 kind: Task diff --git a/pkg/apis/pipeline/v1alpha1/image_resource.go b/pkg/apis/pipeline/v1alpha1/image_resource.go index e753f64eda0..09811c732c6 100644 --- a/pkg/apis/pipeline/v1alpha1/image_resource.go +++ b/pkg/apis/pipeline/v1alpha1/image_resource.go @@ -48,10 +48,10 @@ func NewImageResource(r *PipelineResource) (*ImageResource, error) { // ImageResource defines an endpoint where artifacts can be stored, such as images. type ImageResource struct { - Name string `json:"name"` - Type PipelineResourceType `json:"type"` - URL string `json:"url"` - Digest string `json:"digest"` + Name string `json:"name"` + Type PipelineResourceType `json:"type"` + URL string `json:"url"` + Digest string `json:"digest"` OutputImageDir string } @@ -98,6 +98,9 @@ func (s *ImageResource) GetOutputImageDir() string { } func (s ImageResource) String() string { + // the String() func works as a toString func to return the contents as a string + // and has to follow the interface and therefore cannot return an error + // if the Marshal func gives an error, the returned string will be empty json, _ := json.Marshal(s) return string(json) } diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/image_exporter.go b/pkg/reconciler/v1alpha1/taskrun/resources/image_exporter.go index 7a8c7cd539d..4249e45678d 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/image_exporter.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/image_exporter.go @@ -19,10 +19,10 @@ package resources import ( "encoding/json" "flag" + "fmt" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/names" - "go.uber.org/zap" corev1 "k8s.io/api/core/v1" ) @@ -35,7 +35,6 @@ func AddOutputImageDigestExporter( tr *v1alpha1.TaskRun, taskSpec *v1alpha1.TaskSpec, gr GetResource, - logger *zap.SugaredLogger, ) error { output := []*v1alpha1.ImageResource{} @@ -43,20 +42,17 @@ func AddOutputImageDigestExporter( for _, trb := range tr.Spec.Outputs.Resources { boundResource, err := getBoundResource(trb.Name, tr.Spec.Outputs.Resources) if err != nil { - logger.Errorf("Failed to get bound resource: %s", err) - return err + return fmt.Errorf("Failed to get bound resource: %s while adding output image digest exporter", err) } resource, err := getResource(boundResource, gr) if err != nil { - logger.Errorf("Failed to get output pipeline Resource for taskRun %q resource %v; error: %s", tr.Name, boundResource, err.Error()) - return err + return fmt.Errorf("Failed to get output pipeline Resource for taskRun %q resource %v; error: %s while adding output image digest exporter", tr.Name, boundResource, err.Error()) } if resource.Spec.Type == v1alpha1.PipelineResourceTypeImage { imageResource, err := v1alpha1.NewImageResource(resource) if err != nil { - logger.Errorf("Invalid Image Resource for taskRun %q resource %v; error: %s", tr.Name, boundResource, err.Error()) - return err + return fmt.Errorf("Invalid Image Resource for taskRun %q resource %v; error: %s", tr.Name, boundResource, err.Error()) } for _, o := range taskSpec.Outputs.Resources { if o.Name == boundResource.Name { @@ -74,7 +70,7 @@ func AddOutputImageDigestExporter( augmentedSteps := []corev1.Container{} imagesJSON, err := json.Marshal(output) if err != nil { - return err + return fmt.Errorf("Failed to format image resource data for output image exporter: %s", err) } for _, s := range taskSpec.Steps { @@ -93,7 +89,7 @@ func AddOutputImageDigestExporter( func UpdateTaskRunStatusWithResourceResult(taskRun *v1alpha1.TaskRun, logContent []byte) error { err := json.Unmarshal(logContent, &taskRun.Status.ResourcesResult) if err != nil { - return err + return fmt.Errorf("Failed to unmarshal output image exporter JSON output: %s", err) } return nil } diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/image_exporter_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/image_exporter_test.go index 19e6e3be6c9..eac34f9f80f 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/image_exporter_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/image_exporter_test.go @@ -21,13 +21,12 @@ import ( "github.com/google/go-cmp/cmp" "github.com/knative/pkg/apis" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" - "github.com/tektoncd/pipeline/pkg/logging" "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestExportingOutputImageResource(t *testing.T) { +func TestAddOutputImageDigestExporter(t *testing.T) { currentDir, _ := os.Getwd() for _, c := range []struct { desc string @@ -50,8 +49,8 @@ func TestExportingOutputImageResource(t *testing.T) { }, Outputs: &v1alpha1.Outputs{ Resources: []v1alpha1.TaskResource{{ - Name: "source-image", - Type: "image", + Name: "source-image", + Type: "image", OutputImageDir: currentDir, }}, }, @@ -111,8 +110,8 @@ func TestExportingOutputImageResource(t *testing.T) { }, Outputs: &v1alpha1.Outputs{ Resources: []v1alpha1.TaskResource{{ - Name: "source-image", - Type: "image", + Name: "source-image", + Type: "image", OutputImageDir: currentDir, }}, }, @@ -168,7 +167,6 @@ func TestExportingOutputImageResource(t *testing.T) { }} { t.Run(c.desc, func(t *testing.T) { names.TestingSeed() - logger, _ := logging.NewLogger("", "") gr := func(n string) (*v1alpha1.PipelineResource, error) { return &v1alpha1.PipelineResource{ ObjectMeta: metav1.ObjectMeta{ @@ -190,7 +188,7 @@ func TestExportingOutputImageResource(t *testing.T) { }, }, nil } - err := AddOutputImageDigestExporter(c.taskRun, &c.task.Spec, gr, logger) + err := AddOutputImageDigestExporter(c.taskRun, &c.task.Spec, gr) if err != nil { t.Fatalf("Failed to declare output resources for test %q: error %v", c.desc, err) } diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun.go b/pkg/reconciler/v1alpha1/taskrun/taskrun.go index 95251fa725f..db7319cb402 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun.go @@ -478,7 +478,7 @@ func (c *Reconciler) updateLabels(tr *v1alpha1.TaskRun) (*v1alpha1.TaskRun, erro func (c *Reconciler) createPod(tr *v1alpha1.TaskRun, ts *v1alpha1.TaskSpec, taskName string) (*corev1.Pod, error) { ts = ts.DeepCopy() - err := resources.AddOutputImageDigestExporter(tr, ts, c.resourceLister.PipelineResources(tr.Namespace).Get, c.Logger) + err := resources.AddOutputImageDigestExporter(tr, ts, c.resourceLister.PipelineResources(tr.Namespace).Get) if err != nil { c.Logger.Errorf("Failed to create a build for taskrun: %s due to output image resource error %v", tr.Name, err) return nil, err