From 59617fa7ae1ca2f8e01d2d0dc047639d381c49d4 Mon Sep 17 00:00:00 2001 From: Dan Lorenc Date: Fri, 11 Oct 2019 12:54:58 -0500 Subject: [PATCH] Refactor Resource result output, and add support for Git resources. This change builds upon #1406, and logs the exact Git commit used by a Git input to the ResourceResults field. Some other cleanups are included: - Removing custom TerminationMessagePath from the Image resource. The default is fine. - Test cleanup. - A new helper to write termination messages from resource containers. And finally, this adds a new environment variable to the git resource steps, indicating the name of the resource instance they are running as. We should make this more generic and apply it to all resource steps as part of the extensiblity refactor. --- cmd/git-init/main.go | 22 +- cmd/imagedigestexporter/main.go | 34 +- docs/resources.md | 14 +- pkg/apis/pipeline/v1alpha1/git_resource.go | 5 + .../pipeline/v1alpha1/git_resource_test.go | 1 + pkg/git/git.go | 33 +- .../taskrun/resources/image_exporter.go | 30 +- .../taskrun/resources/image_exporter_test.go | 312 +----------------- .../taskrun/resources/input_resource_test.go | 6 + pkg/reconciler/taskrun/taskrun.go | 28 +- pkg/reconciler/taskrun/taskrun_test.go | 130 +++++++- pkg/termination/termination.go | 45 +++ test/builder/container.go | 7 - test/kaniko_task_test.go | 16 +- 14 files changed, 293 insertions(+), 390 deletions(-) create mode 100644 pkg/termination/termination.go diff --git a/cmd/git-init/main.go b/cmd/git-init/main.go index f11ec09e7e1..2bab57c6742 100644 --- a/cmd/git-init/main.go +++ b/cmd/git-init/main.go @@ -18,14 +18,17 @@ package main import ( "flag" + v1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/git" + "github.com/tektoncd/pipeline/pkg/termination" "knative.dev/pkg/logging" ) var ( - url = flag.String("url", "", "The url of the Git repository to initialize.") - revision = flag.String("revision", "", "The Git revision to make the repository HEAD") - path = flag.String("path", "", "Path of directory under which git repository will be copied") + url = flag.String("url", "", "The url of the Git repository to initialize.") + revision = flag.String("revision", "", "The Git revision to make the repository HEAD") + path = flag.String("path", "", "Path of directory under which git repository will be copied") + terminationMessagePath = flag.String("terminationMessagePath", "/dev/termination-log", "Location of file containing termination message") ) func main() { @@ -36,4 +39,17 @@ func main() { if err := git.Fetch(logger, *revision, *path, *url); err != nil { logger.Fatalf("Error fetching git repository: %s", err) } + + commit, err := git.Commit(logger, *revision, *path) + if err != nil { + logger.Fatalf("Error parsing commit of git repository: %s", err) + } + output := []v1alpha1.PipelineResourceResult{ + { + Key: "commit", + Value: commit, + }, + } + + termination.WriteMessage(logger, *terminationMessagePath, output) } diff --git a/cmd/imagedigestexporter/main.go b/cmd/imagedigestexporter/main.go index e39fc867bc3..972777176b3 100644 --- a/cmd/imagedigestexporter/main.go +++ b/cmd/imagedigestexporter/main.go @@ -19,8 +19,9 @@ package main import ( "encoding/json" "flag" - "log" - "os" + + "github.com/tektoncd/pipeline/pkg/termination" + "knative.dev/pkg/logging" "github.com/google/go-containerregistry/pkg/v1/layout" v1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" @@ -28,7 +29,7 @@ import ( var ( images = flag.String("images", "", "List of images resources built by task in json format") - terminationMessagePath = flag.String("terminationMessagePath", "", "Location of file containing termination message") + terminationMessagePath = flag.String("terminationMessagePath", "/dev/termination-log", "Location of file containing termination message") ) /* The input of this go program will be a JSON string with all the output PipelineResources of type @@ -40,10 +41,12 @@ The output is an array of PipelineResourceResult, ex: [{"name":"image","digest": */ func main() { flag.Parse() + logger, _ := logging.NewLogger("", "image-digest-exporter") + defer logger.Sync() imageResources := []*v1alpha1.ImageResource{} if err := json.Unmarshal([]byte(*images), &imageResources); err != nil { - log.Fatalf("Error reading images array: %v", err) + logger.Fatalf("Error reading images array: %v", err) } output := []v1alpha1.PipelineResourceResult{} @@ -52,12 +55,12 @@ func main() { if err != nil { // if this image doesn't have a builder that supports index.json file, // then it will be skipped - log.Printf("ImageResource %s doesn't have an index.json file: %s", imageResource.Name, err) + logger.Infof("ImageResource %s doesn't have an index.json file: %s", imageResource.Name, err) continue } digest, err := GetDigest(ii) if err != nil { - log.Fatalf("Unexpected error getting image digest for %s: %v", imageResource.Name, err) + logger.Fatalf("Unexpected error getting image digest for %s: %v", imageResource.Name, err) } // We need to write both the old Name/Digest style and the new Key/Value styles. output = append(output, v1alpha1.PipelineResourceResult{ @@ -75,22 +78,5 @@ func main() { } - imagesJSON, err := json.Marshal(output) - if err != nil { - log.Fatalf("Unexpected error converting images to json %v: %v", output, err) - } - log.Printf("Image digest exporter output: %s ", string(imagesJSON)) - f, err := os.OpenFile(*terminationMessagePath, os.O_WRONLY|os.O_CREATE, 0666) - if err != nil { - log.Fatalf("Unexpected error converting images to json %v: %v", output, err) - } - defer f.Close() - - _, err = f.Write(imagesJSON) - if err != nil { - log.Fatalf("Unexpected error converting images to json %v: %v", output, err) - } - if err := f.Sync(); err != nil { - log.Fatalf("Unexpected error converting images to json %v: %v", output, err) - } + termination.WriteMessage(logger, *terminationMessagePath, output) } diff --git a/docs/resources.md b/docs/resources.md index 53792e60a50..ec77aab0d8a 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -198,8 +198,7 @@ spec: When resources are bound inside a TaskRun, they can include extra information in the TaskRun Status.ResourcesResult field. This information can be useful for auditing the exact resources used by a TaskRun later. -Currently the only resource that uses this mechanism is the Image resource, which includes the exact digest -of an image built by a TaskRun and declared as an output. +Currently the Image and Git resources use this mechanism. For an example of what this output looks like: @@ -258,6 +257,17 @@ Params that can be added are the following: commit [or branch](#using-a-branch) is used. _If no revision is specified, the resource will default to `latest` from `master`._ +When used as an input, the Git resource includes the exact commit fetched in the `resourceResults` +section of the `taskRun`'s status object: + +```yaml +resourceResults: +- key: commit + value: 6ed7aad5e8a36052ee5f6079fc91368e362121f7 + resourceRef: + name: skaffold-git +``` + #### Using a fork The `Url` parameter can be used to point at any git repository, for example to diff --git a/pkg/apis/pipeline/v1alpha1/git_resource.go b/pkg/apis/pipeline/v1alpha1/git_resource.go index 8f19e8e8418..051a728e350 100644 --- a/pkg/apis/pipeline/v1alpha1/git_resource.go +++ b/pkg/apis/pipeline/v1alpha1/git_resource.go @@ -109,6 +109,11 @@ func (s *GitResource) GetInputTaskModifier(_ *TaskSpec, path string) (TaskModifi Command: []string{"/ko-app/git-init"}, Args: args, WorkingDir: WorkspaceDir, + // This is used to populate the ResourceResult status. + Env: []corev1.EnvVar{{ + Name: "TEKTON_RESOURCE_NAME", + Value: s.Name, + }}, }, } return &InternalTaskModifier{ diff --git a/pkg/apis/pipeline/v1alpha1/git_resource_test.go b/pkg/apis/pipeline/v1alpha1/git_resource_test.go index af741651d0f..5f3a72a16cd 100644 --- a/pkg/apis/pipeline/v1alpha1/git_resource_test.go +++ b/pkg/apis/pipeline/v1alpha1/git_resource_test.go @@ -132,6 +132,7 @@ func Test_GitResource_GetDownloadTaskModifier(t *testing.T) { "/test/test", }, WorkingDir: "/workspace", + Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}}, }}} if diff := cmp.Diff(want, modifier.GetStepsToPrepend()); diff != "" { diff --git a/pkg/git/git.go b/pkg/git/git.go index ab3d3299e6e..4d3a33cc26d 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -26,16 +26,21 @@ import ( "golang.org/x/xerrors" ) -func run(logger *zap.SugaredLogger, args ...string) error { +func run(logger *zap.SugaredLogger, dir string, args ...string) (string, error) { c := exec.Command("git", args...) var output bytes.Buffer c.Stderr = &output c.Stdout = &output + // This is the optional working directory. If not set, it defaults to the current + // working directory of the process. + if dir != "" { + c.Dir = dir + } if err := c.Run(); err != nil { logger.Errorf("Error running git %v: %v\n%v", args, err, output.String()) - return err + return "", err } - return nil + return output.String(), nil } // Fetch fetches the specified git repository at the revision into path. @@ -70,31 +75,39 @@ func Fetch(logger *zap.SugaredLogger, revision, path, url string) error { revision = "master" } if path != "" { - if err := run(logger, "init", path); err != nil { + if _, err := run(logger, "", "init", path); err != nil { return err } if err := os.Chdir(path); err != nil { return xerrors.Errorf("Failed to change directory with path %s; err: %w", path, err) } - } else if err := run(logger, "init"); err != nil { + } else if _, err := run(logger, "", "init"); err != nil { return err } trimmedURL := strings.TrimSpace(url) - if err := run(logger, "remote", "add", "origin", trimmedURL); err != nil { + if _, err := run(logger, "", "remote", "add", "origin", trimmedURL); err != nil { return err } - if err := run(logger, "fetch", "--depth=1", "--recurse-submodules=yes", "origin", revision); err != nil { + if _, err := run(logger, "", "fetch", "--depth=1", "--recurse-submodules=yes", "origin", revision); err != nil { // Fetch can fail if an old commitid was used so try git pull, performing regardless of error // as no guarantee that the same error is returned by all git servers gitlab, github etc... - if err := run(logger, "pull", "--recurse-submodules=yes", "origin"); err != nil { + if _, err := run(logger, "", "pull", "--recurse-submodules=yes", "origin"); err != nil { logger.Warnf("Failed to pull origin : %s", err) } - if err := run(logger, "checkout", revision); err != nil { + if _, err := run(logger, "", "checkout", revision); err != nil { return err } - } else if err := run(logger, "reset", "--hard", "FETCH_HEAD"); err != nil { + } else if _, err := run(logger, "", "reset", "--hard", "FETCH_HEAD"); err != nil { return err } logger.Infof("Successfully cloned %s @ %s in path %s", trimmedURL, revision, path) return nil } + +func Commit(logger *zap.SugaredLogger, revision, path string) (string, error) { + output, err := run(logger, path, "rev-parse", "HEAD") + if err != nil { + return "", err + } + return strings.TrimSuffix(output, "\n"), nil +} diff --git a/pkg/reconciler/taskrun/resources/image_exporter.go b/pkg/reconciler/taskrun/resources/image_exporter.go index c74dbd0e701..ba06d17b8a2 100644 --- a/pkg/reconciler/taskrun/resources/image_exporter.go +++ b/pkg/reconciler/taskrun/resources/image_exporter.go @@ -25,7 +25,7 @@ import ( corev1 "k8s.io/api/core/v1" ) -const TerminationMessagePath = "/builder/home/image-outputs/termination-log" +const imageDigestExporterContainerName = "image-digest-exporter" // AddOutputImageDigestExporter add a step to check the index.json for all output images func AddOutputImageDigestExporter( @@ -82,40 +82,14 @@ func AddOutputImageDigestExporter( return nil } -// UpdateTaskRunStatusWithResourceResult if there is an update to the outout image resource, add to taskrun status result -func UpdateTaskRunStatusWithResourceResult(taskRun *v1alpha1.TaskRun, logContent []byte) error { - if err := json.Unmarshal(logContent, &taskRun.Status.ResourcesResult); err != nil { - return xerrors.Errorf("Failed to unmarshal output image exporter JSON output: %w", err) - } - return nil -} - func imageDigestExporterStep(imageDigestExporterImage string, imagesJSON []byte) v1alpha1.Step { return v1alpha1.Step{Container: corev1.Container{ - Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("image-digest-exporter"), + Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(imageDigestExporterContainerName), Image: imageDigestExporterImage, Command: []string{"/ko-app/imagedigestexporter"}, Args: []string{ "-images", string(imagesJSON), - "-terminationMessagePath", TerminationMessagePath, }, - TerminationMessagePath: TerminationMessagePath, TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError, }} } - -// TaskRunHasOutputImageResource return true if the task has any output resources of type image -func TaskRunHasOutputImageResource(gr GetResource, taskRun *v1alpha1.TaskRun) bool { - if len(taskRun.Spec.Outputs.Resources) > 0 { - for _, r := range taskRun.Spec.Outputs.Resources { - resource, err := gr(r.ResourceRef.Name) - if err != nil { - return false - } - if resource.Spec.Type == v1alpha1.PipelineResourceTypeImage { - return true - } - } - } - return false -} diff --git a/pkg/reconciler/taskrun/resources/image_exporter_test.go b/pkg/reconciler/taskrun/resources/image_exporter_test.go index 3e70bbcdcdd..315a369a8d7 100644 --- a/pkg/reconciler/taskrun/resources/image_exporter_test.go +++ b/pkg/reconciler/taskrun/resources/image_exporter_test.go @@ -26,7 +26,6 @@ import ( "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "knative.dev/pkg/apis" ) func TestAddOutputImageDigestExporter(t *testing.T) { @@ -96,12 +95,10 @@ func TestAddOutputImageDigestExporter(t *testing.T) { wantSteps: []v1alpha1.Step{{Container: corev1.Container{ Name: "step1", }}, {Container: corev1.Container{ - Name: "image-digest-exporter-9l9zj", - Image: "override-with-imagedigest-exporter-image:latest", - Command: []string{"/ko-app/imagedigestexporter"}, - Args: []string{"-images", fmt.Sprintf("[{\"name\":\"source-image-1\",\"type\":\"image\",\"url\":\"gcr.io/some-image-1\",\"digest\":\"\",\"OutputImageDir\":\"%s\"}]", currentDir), - "-terminationMessagePath", "/builder/home/image-outputs/termination-log"}, - TerminationMessagePath: TerminationMessagePath, + Name: "image-digest-exporter-9l9zj", + Image: "override-with-imagedigest-exporter-image:latest", + Command: []string{"/ko-app/imagedigestexporter"}, + Args: []string{"-images", fmt.Sprintf("[{\"name\":\"source-image-1\",\"type\":\"image\",\"url\":\"gcr.io/some-image-1\",\"digest\":\"\",\"OutputImageDir\":\"%s\"}]", currentDir)}, TerminationMessagePolicy: "FallbackToLogsOnError", }}}, }, { @@ -168,12 +165,10 @@ func TestAddOutputImageDigestExporter(t *testing.T) { }}, {Container: corev1.Container{ Name: "step2", }}, {Container: corev1.Container{ - Name: "image-digest-exporter-9l9zj", - Image: "override-with-imagedigest-exporter-image:latest", - Command: []string{"/ko-app/imagedigestexporter"}, - Args: []string{"-images", fmt.Sprintf("[{\"name\":\"source-image-1\",\"type\":\"image\",\"url\":\"gcr.io/some-image-1\",\"digest\":\"\",\"OutputImageDir\":\"%s\"}]", currentDir), - "-terminationMessagePath", "/builder/home/image-outputs/termination-log"}, - TerminationMessagePath: TerminationMessagePath, + Name: "image-digest-exporter-9l9zj", + Image: "override-with-imagedigest-exporter-image:latest", + Command: []string{"/ko-app/imagedigestexporter"}, + Args: []string{"-images", fmt.Sprintf("[{\"name\":\"source-image-1\",\"type\":\"image\",\"url\":\"gcr.io/some-image-1\",\"digest\":\"\",\"OutputImageDir\":\"%s\"}]", currentDir)}, TerminationMessagePolicy: "FallbackToLogsOnError", }}}, }} { @@ -211,294 +206,3 @@ func TestAddOutputImageDigestExporter(t *testing.T) { }) } } - -func TestUpdateTaskRunStatus_withValidJson(t *testing.T) { - for _, c := range []struct { - desc string - podLog []byte - taskRun *v1alpha1.TaskRun - 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", - }, - }, - }}, - }, - }, - }, - want: []v1alpha1.PipelineResourceResult{{ - Name: "source-image", - Digest: "sha256:1234", - }}, - }} { - t.Run(c.desc, func(t *testing.T) { - names.TestingSeed() - c.taskRun.Status.SetCondition(&apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }) - err := UpdateTaskRunStatusWithResourceResult(c.taskRun, c.podLog) - if err != nil { - t.Errorf("UpdateTaskRunStatusWithResourceResult failed with error: %s", err) - } - if d := cmp.Diff(c.taskRun.Status.ResourcesResult, c.want); d != "" { - t.Errorf("post build steps mismatch: %s", d) - } - }) - } -} - -func TestUpdateTaskRunStatus_withInvalidJson(t *testing.T) { - for _, c := range []struct { - desc string - podLog []byte - taskRun *v1alpha1.TaskRun - 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", - }, - }, - }}, - }, - }, - }, - want: nil, - }, { - desc: "task with no image resource ", - podLog: []byte(""), - taskRun: &v1alpha1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-taskrun-run-output-steps", - Namespace: "marshmallow", - }, - }, - want: nil, - }} { - t.Run(c.desc, func(t *testing.T) { - names.TestingSeed() - c.taskRun.Status.SetCondition(&apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }) - err := UpdateTaskRunStatusWithResourceResult(c.taskRun, c.podLog) - if err == nil { - t.Error("UpdateTaskRunStatusWithResourceResult expected to fail with error") - } - if d := cmp.Diff(c.taskRun.Status.ResourcesResult, c.want); d != "" { - t.Errorf("post build steps mismatch: %s", d) - } - }) - } -} - -func TestTaskRunHasOutputImageResource(t *testing.T) { - for _, c := range []struct { - desc string - task *v1alpha1.Task - taskRun *v1alpha1.TaskRun - wantResult bool - }{{ - desc: "image resource as output", - task: &v1alpha1.Task{ - ObjectMeta: metav1.ObjectMeta{ - Name: "task1", - Namespace: "marshmallow", - }, - Spec: v1alpha1.TaskSpec{ - Inputs: &v1alpha1.Inputs{ - Resources: []v1alpha1.TaskResource{{ - ResourceDeclaration: v1alpha1.ResourceDeclaration{ - Name: "source-image", - Type: "image", - }}}, - }, - Outputs: &v1alpha1.Outputs{ - Resources: []v1alpha1.TaskResource{{ - ResourceDeclaration: v1alpha1.ResourceDeclaration{ - Name: "source-image", - Type: "image", - }}}, - }, - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "step1", - }}}, - }, - }, - 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", - }, - }, - }}, - }, - }, - }, - wantResult: true, - }, { - desc: "task with no image resource", - task: &v1alpha1.Task{ - ObjectMeta: metav1.ObjectMeta{ - Name: "task1", - Namespace: "marshmallow", - }, - Spec: v1alpha1.TaskSpec{ - Inputs: &v1alpha1.Inputs{ - Resources: []v1alpha1.TaskResource{{ - ResourceDeclaration: v1alpha1.ResourceDeclaration{ - Name: "source", - Type: "git", - }}}, - }, - Outputs: &v1alpha1.Outputs{ - Resources: []v1alpha1.TaskResource{{ - ResourceDeclaration: v1alpha1.ResourceDeclaration{ - Name: "source", - Type: "git", - }}}, - }, - }, - }, - 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", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git-1", - }, - }, - }}, - }, - Outputs: v1alpha1.TaskRunOutputs{ - Resources: []v1alpha1.TaskResourceBinding{{ - PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ - Name: "source", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git-1", - }, - }, - }}, - }, - }, - }, - wantResult: false, - }} { - t.Run(c.desc, func(t *testing.T) { - gr := func(name string) (*v1alpha1.PipelineResource, error) { - var r *v1alpha1.PipelineResource - if name == "source-image-1" { - r = &v1alpha1.PipelineResource{ - ObjectMeta: metav1.ObjectMeta{ - Name: "source-image-1", - Namespace: "marshmallow", - }, - Spec: v1alpha1.PipelineResourceSpec{ - Type: "image", - Params: []v1alpha1.ResourceParam{{ - Name: "url", - Value: "gcr.io/some-image-1", - }, { - Name: "digest", - Value: "", - }, { - Name: "OutputImageDir", - Value: "/workspace/source-image-1/index.json", - }}, - }, - } - } else if name == "source-git-1" { - r = &v1alpha1.PipelineResource{ - ObjectMeta: metav1.ObjectMeta{ - Name: "source-git-1", - Namespace: "marshmallow", - }, - Spec: v1alpha1.PipelineResourceSpec{ - Type: "git", - Params: []v1alpha1.ResourceParam{{ - Name: "url", - Value: "github.com/repo", - }, - }, - }, - } - } - return r, nil - } - result := TaskRunHasOutputImageResource(gr, c.taskRun) - - if d := cmp.Diff(result, c.wantResult); d != "" { - t.Fatalf("post build steps mismatch: %s", d) - } - }) - } -} diff --git a/pkg/reconciler/taskrun/resources/input_resource_test.go b/pkg/reconciler/taskrun/resources/input_resource_test.go index aec89f2dd92..66ae49ebb7c 100644 --- a/pkg/reconciler/taskrun/resources/input_resource_test.go +++ b/pkg/reconciler/taskrun/resources/input_resource_test.go @@ -322,6 +322,7 @@ func TestAddResourceToTask(t *testing.T) { Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "master", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", + Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git"}}, }}}, }, }, { @@ -357,6 +358,7 @@ func TestAddResourceToTask(t *testing.T) { Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", + Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}}, }}}, }, }, { @@ -399,12 +401,14 @@ func TestAddResourceToTask(t *testing.T) { Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", + Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}}, }}, {Container: corev1.Container{ Name: "git-source-the-git-with-branch-9l9zj", Image: "override-with-git:latest", Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/git-duplicate-space"}, WorkingDir: "/workspace", + Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}}, }}}, }, }, { @@ -440,6 +444,7 @@ func TestAddResourceToTask(t *testing.T) { Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "master", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", + Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git"}}, }}}, }, }, { @@ -475,6 +480,7 @@ func TestAddResourceToTask(t *testing.T) { Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", + Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}}, }}}, }, }, { diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 2d83d041364..5203b90af93 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -18,6 +18,7 @@ package taskrun import ( "context" + "encoding/json" "fmt" "reflect" "strings" @@ -55,7 +56,6 @@ const ( // imageDigestExporterContainerName defines the name of the container that will collect the // built images digest - imageDigestExporterContainerName = "step-image-digest-exporter" ) // Reconciler implements controller.Reconciler for Configuration resources. @@ -312,7 +312,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error status.SortTaskRunStepOrder(tr.Status.Steps, taskSpec.Steps) - updateTaskRunResourceResult(tr, pod, c.resourceLister, c.Logger) + updateTaskRunResourceResult(tr, pod, c.Logger) after := tr.Status.GetCondition(apis.ConditionSucceeded) @@ -358,19 +358,31 @@ 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, resourceLister listers.PipelineResourceLister, logger *zap.SugaredLogger) { - if resources.TaskRunHasOutputImageResource(resourceLister.PipelineResources(taskRun.Namespace).Get, taskRun) && taskRun.IsSuccessful() { +func updateTaskRunResourceResult(taskRun *v1alpha1.TaskRun, pod *corev1.Pod, logger *zap.SugaredLogger) { + if taskRun.IsSuccessful() { for _, cs := range pod.Status.ContainerStatuses { - if strings.HasPrefix(cs.Name, imageDigestExporterContainerName) { - err := resources.UpdateTaskRunStatusWithResourceResult(taskRun, []byte(cs.State.Terminated.Message)) - if err != nil { - logger.Errorf("Error getting output from image-digest-exporter for %s/%s: %s", taskRun.Name, taskRun.Namespace, err) + 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) + } } } } } } +// 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 xerrors.Errorf("Failed to unmarshal output image exporter JSON output: %w", err) + } + taskRun.Status.ResourcesResult = append(taskRun.Status.ResourcesResult, results...) + return nil +} + func (c *Reconciler) updateStatus(taskrun *v1alpha1.TaskRun) (*v1alpha1.TaskRun, error) { newtaskrun, err := c.taskRunLister.TaskRuns(taskrun.Namespace).Get(taskrun.Name) if err != nil { diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index f8480266cf6..582bb69631b 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -727,6 +727,7 @@ func TestReconcile(t *testing.T) { "-url", "https://foo.git", "-revision", "master", "-path", "/workspace/workspace"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), + tb.EnvVar("TEKTON_RESOURCE_NAME", "git-resource"), tb.VolumeMount("tools", "/builder/tools"), tb.VolumeMount("workspace", workspaceDir), tb.VolumeMount("home", "/builder/home"), @@ -770,8 +771,7 @@ func TestReconcile(t *testing.T) { tb.PodContainer("step-image-digest-exporter-9l9zj", "override-with-imagedigest-exporter-image:latest", tb.Command(entrypointLocation), tb.Args("-wait_file", "/builder/tools/3", "-post_file", "/builder/tools/4", "-entrypoint", "/ko-app/imagedigestexporter", "--", - "-images", "[{\"name\":\"image-resource\",\"type\":\"image\",\"url\":\"gcr.io/kristoff/sven\",\"digest\":\"\",\"OutputImageDir\":\"\"}]", - "-terminationMessagePath", "/builder/home/image-outputs/termination-log"), + "-images", "[{\"name\":\"image-resource\",\"type\":\"image\",\"url\":\"gcr.io/kristoff/sven\",\"digest\":\"\",\"OutputImageDir\":\"\"}]"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), tb.VolumeMount("tools", "/builder/tools"), @@ -782,7 +782,6 @@ func TestReconcile(t *testing.T) { tb.Memory("0"), tb.EphemeralStorage("0"), )), - tb.TerminationMessagePath("/builder/home/image-outputs/termination-log"), tb.TerminationMessagePolicy(corev1.TerminationMessageFallbackToLogsOnError), ), ), @@ -941,6 +940,7 @@ func TestReconcile(t *testing.T) { "-url", "https://foo.git", "-revision", "master", "-path", "/workspace/workspace"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), + tb.EnvVar("TEKTON_RESOURCE_NAME", "git-resource"), tb.VolumeMount("tools", "/builder/tools"), tb.VolumeMount("downward", "/builder/downward"), tb.VolumeMount("workspace", workspaceDir), @@ -1021,6 +1021,7 @@ func TestReconcile(t *testing.T) { "/workspace/workspace"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), + tb.EnvVar("TEKTON_RESOURCE_NAME", "workspace"), tb.VolumeMount("tools", "/builder/tools"), tb.VolumeMount("downward", "/builder/downward"), tb.VolumeMount("workspace", workspaceDir), @@ -2033,3 +2034,126 @@ func TestReconcileCloudEvents(t *testing.T) { }) } } + +func TestUpdateTaskRunStatus_withValidJson(t *testing.T) { + for _, c := range []struct { + desc string + podLog []byte + taskRun *v1alpha1.TaskRun + 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", + }, + }, + }}, + }, + }, + }, + want: []v1alpha1.PipelineResourceResult{{ + Name: "source-image", + Digest: "sha256:1234", + }}, + }} { + 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.Errorf("UpdateTaskRunStatusWithResourceResult failed with error: %s", err) + } + if d := cmp.Diff(c.taskRun.Status.ResourcesResult, c.want); d != "" { + t.Errorf("post build steps mismatch: %s", d) + } + }) + } +} + +func TestUpdateTaskRunStatus_withInvalidJson(t *testing.T) { + for _, c := range []struct { + desc string + podLog []byte + taskRun *v1alpha1.TaskRun + 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", + }, + }, + }}, + }, + }, + }, + want: nil, + }, { + desc: "task with no image resource ", + podLog: []byte(""), + taskRun: &v1alpha1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-taskrun-run-output-steps", + Namespace: "marshmallow", + }, + }, + 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 d := cmp.Diff(c.taskRun.Status.ResourcesResult, c.want); d != "" { + t.Errorf("post build steps mismatch: %s", d) + } + }) + } +} diff --git a/pkg/termination/termination.go b/pkg/termination/termination.go new file mode 100644 index 00000000000..ab1e3601a64 --- /dev/null +++ b/pkg/termination/termination.go @@ -0,0 +1,45 @@ +/* +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" + "log" + "os" + + v1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "go.uber.org/zap" +) + +func WriteMessage(logger *zap.SugaredLogger, path string, pro []v1alpha1.PipelineResourceResult) { + jsonOutput, err := json.Marshal(pro) + if err != nil { + logger.Fatalf("Error marshaling json: %s", err) + } + f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE, 0666) + if err != nil { + log.Fatalf("Unexpected error converting output to json %v: %v", pro, err) + } + defer f.Close() + + _, err = f.Write(jsonOutput) + if err != nil { + logger.Fatalf("Unexpected error converting output to json %v: %v", pro, err) + } + if err := f.Sync(); err != nil { + logger.Fatalf("Unexpected error converting output to json %v: %v", pro, err) + } +} diff --git a/test/builder/container.go b/test/builder/container.go index 41ffcef29c9..8c8f71a241d 100644 --- a/test/builder/container.go +++ b/test/builder/container.go @@ -132,13 +132,6 @@ func EphemeralStorage(val string) ResourceListOp { } } -// TerminationMessagePath sets the source of the termination message. -func TerminationMessagePath(terminationMessagePath string) ContainerOp { - return func(c *corev1.Container) { - c.TerminationMessagePath = terminationMessagePath - } -} - // TerminationMessagePolicy sets the policy of the termination message. func TerminationMessagePolicy(terminationMessagePolicy corev1.TerminationMessagePolicy) ContainerOp { return func(c *corev1.Container) { diff --git a/test/kaniko_task_test.go b/test/kaniko_task_test.go index 2ee8685979f..7e10a8f801c 100644 --- a/test/kaniko_task_test.go +++ b/test/kaniko_task_test.go @@ -39,12 +39,15 @@ const ( kanikoTaskRunName = "kanikotask-run" kanikoGitResourceName = "go-example-git" kanikoImageResourceName = "go-example-image" + // This is a random revision chosen on 10/11/2019 + revision = "1c9d566ecd13535f93789595740f20932f655905" ) func getGitResource(namespace string) *v1alpha1.PipelineResource { return tb.PipelineResource(kanikoGitResourceName, namespace, tb.PipelineResourceSpec( v1alpha1.PipelineResourceTypeGit, tb.PipelineResourceSpecParam("Url", "https://github.com/GoogleContainerTools/kaniko"), + tb.PipelineResourceSpecParam("Revision", revision), )) } @@ -141,14 +144,25 @@ func TestKanikoTaskRun(t *testing.T) { t.Errorf("Error retrieving taskrun: %s", err) } digest := "" + commit := "" for _, rr := range tr.Status.ResourcesResult { - if rr.Key == "digest" { + switch rr.Key { + case "digest": digest = rr.Value + case "commit": + commit = rr.Value } } if digest == "" { t.Errorf("Digest not found in TaskRun.Status: %v", tr.Status) } + if commit == "" { + t.Errorf("Commit not found in TaskRun.Status: %v", tr.Status) + } + + if revision != commit { + t.Fatalf("Expected remote commit to match local revision: %s, %s", commit, revision) + } // match the local digest, which is first capture group against the remote image remoteDigest, err := getRemoteDigest(repo)