From 8efcd6140e363baacbf9ba4cf1f8dde12bf937a6 Mon Sep 17 00:00:00 2001 From: ericzzzzzzz <102683393+ericzzzzzzz@users.noreply.github.com> Date: Sat, 24 Feb 2024 20:43:44 -0500 Subject: [PATCH] kind/feat: surface artifacts through sidecar container --- cmd/controller/main.go | 2 +- cmd/sidecarlogartifacts/main.go | 44 +++ config/controller.yaml | 1 + .../alpha/produce-consume-artifacts.yaml | 4 +- .../sidecarlogartifacts.go | 141 ++++++++++ .../sidecarlogartifacts_test.go | 255 ++++++++++++++++++ pkg/apis/pipeline/images.go | 4 + pkg/apis/pipeline/images_test.go | 24 +- pkg/apis/pipeline/sidecarlogs.go | 8 + pkg/pod/entrypoint.go | 5 +- pkg/pod/entrypoint_test.go | 63 +++++ pkg/pod/pod.go | 75 +++++- pkg/pod/pod_test.go | 46 ++++ pkg/pod/status.go | 29 +- pkg/pod/status_test.go | 4 +- test/artifacts_test.go | 130 +++++---- 16 files changed, 752 insertions(+), 83 deletions(-) create mode 100644 cmd/sidecarlogartifacts/main.go create mode 100644 internal/sidecarlogartifacts/sidecarlogartifacts.go create mode 100644 internal/sidecarlogartifacts/sidecarlogartifacts_test.go diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 664eafe21cc..22d681ab5fa 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -56,7 +56,7 @@ func main() { flag.StringVar(&opts.Images.ShellImage, "shell-image", "", "The container image containing a shell") flag.StringVar(&opts.Images.ShellImageWin, "shell-image-win", "", "The container image containing a windows shell") flag.StringVar(&opts.Images.WorkingDirInitImage, "workingdirinit-image", "", "The container image containing our working dir init binary.") - + flag.StringVar(&opts.Images.SidecarLogArtifactsImage, "sidecarlogartifacts-image", "", "The container image containing the binary for accessing artifacts.") // This parses flags. cfg := injection.ParseAndGetRESTConfigOrDie() diff --git a/cmd/sidecarlogartifacts/main.go b/cmd/sidecarlogartifacts/main.go new file mode 100644 index 00000000000..51158454aaa --- /dev/null +++ b/cmd/sidecarlogartifacts/main.go @@ -0,0 +1,44 @@ +/* +Copyright 2024 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 main + +import ( + "encoding/json" + "flag" + "log" + "os" + "strings" + + "github.com/tektoncd/pipeline/internal/sidecarlogartifacts" + "github.com/tektoncd/pipeline/pkg/pod" +) + +func main() { + var stepNames string + flag.StringVar(&stepNames, "step-names", "", "comma separated step names to expect from the steps running in the pod. eg. foo,bar,baz") + flag.Parse() + if stepNames == "" { + log.Fatal("step-names were not provided") + } + names := strings.Split(stepNames, ",") + artifacts, err := sidecarlogartifacts.LookForArtifacts(names, pod.RunDir) + if err != nil { + log.Fatal(err) + } + err = json.NewEncoder(os.Stdout).Encode(artifacts) + + if err != nil { + log.Fatal(err) + } +} diff --git a/config/controller.yaml b/config/controller.yaml index dad4866396b..4d3acb55449 100644 --- a/config/controller.yaml +++ b/config/controller.yaml @@ -68,6 +68,7 @@ spec: "-entrypoint-image", "ko://github.com/tektoncd/pipeline/cmd/entrypoint", "-nop-image", "ko://github.com/tektoncd/pipeline/cmd/nop", "-sidecarlogresults-image", "ko://github.com/tektoncd/pipeline/cmd/sidecarlogresults", + "-sidecarlogartifacts-image", "ko://github.com/tektoncd/pipeline/cmd/sidecarlogartifacts", "-workingdirinit-image", "ko://github.com/tektoncd/pipeline/cmd/workingdirinit", # The shell image must allow root in order to create directories and copy files to PVCs. diff --git a/examples/v1/taskruns/alpha/produce-consume-artifacts.yaml b/examples/v1/taskruns/alpha/produce-consume-artifacts.yaml index e270b1b85ff..0079fe930cf 100644 --- a/examples/v1/taskruns/alpha/produce-consume-artifacts.yaml +++ b/examples/v1/taskruns/alpha/produce-consume-artifacts.yaml @@ -17,7 +17,7 @@ spec: "name":"input-artifacts", "values":[ { - "uri":"git:jjjsss", + "uri":"pkg:example.github.com/inputs", "digest":{ "sha256":"b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0" } @@ -30,7 +30,7 @@ spec: "name":"image", "values":[ { - "uri":"pkg:balba", + "uri":"pkg:github/package-url/purl-spec@244fd47e07d1004f0aed9c", "digest":{ "sha256":"df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48", "sha1":"95588b8f34c31eb7d62c92aaa4e6506639b06ef2" diff --git a/internal/sidecarlogartifacts/sidecarlogartifacts.go b/internal/sidecarlogartifacts/sidecarlogartifacts.go new file mode 100644 index 00000000000..308baf4bcd7 --- /dev/null +++ b/internal/sidecarlogartifacts/sidecarlogartifacts.go @@ -0,0 +1,141 @@ +/* +Copyright 2024 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 sidecarlogartifacts + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/tektoncd/pipeline/pkg/apis/pipeline" + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/kubernetes" +) + +// for testing +var stepDir = pipeline.StepsDir + +func fileExists(filename string) (bool, error) { + info, err := os.Stat(filename) + if os.IsNotExist(err) { + return false, nil + } else if err != nil { + return false, fmt.Errorf("error checking for file existence %w", err) + } + return !info.IsDir(), nil +} + +func waitForStepsToFinish(runDir string) error { + steps := make(map[string]bool) + files, err := os.ReadDir(runDir) + if err != nil { + return fmt.Errorf("error parsing the run dir %w", err) + } + for _, file := range files { + steps[filepath.Join(runDir, file.Name(), "out")] = true + } + for len(steps) > 0 { + for stepFile := range steps { + // check if there is a post file without error + time.Sleep(200 * time.Millisecond) + exists, err := fileExists(stepFile) + if err != nil { + return fmt.Errorf("error checking for out file's existence %w", err) + } + if exists { + delete(steps, stepFile) + continue + } + + // This is the same as results sidecar, it checks if there is a post file with error + // if err is nil then either the out.err file does not exist or it does and there was no issue + // in either case, existence of out.err marks that the step errored and the following steps will + // not run. We want the function to break out with nil error in that case so that + // the existing results can be logged. + if exists, err = fileExists(stepFile + ".err"); exists || err != nil { + return err + } + } + } + return nil +} + +func parseArtifacts(fileContent []byte) (v1.Artifacts, error) { + var as v1.Artifacts + if err := json.Unmarshal(fileContent, &as); err != nil { + return as, fmt.Errorf("invalid artifacts : %w", err) + } + return as, nil +} + +func extractArtifactsFromFile(filename string) (v1.Artifacts, error) { + b, err := os.ReadFile(filename) + if err != nil { + return v1.Artifacts{}, err + } + return parseArtifacts(b) +} + +type SidecarArtifacts map[string]v1.Artifacts + +// GetArtifactsFromSidecarLogs retrieves artifacts from sidecar logs in a Kubernetes pod. +// It returns a SidecarArtifacts map (step name -> v1.Artifacts) and an error. If the pod is in the +// Pending phase, an empty map is returned without error. +func GetArtifactsFromSidecarLogs(ctx context.Context, clientset kubernetes.Interface, namespace string, name string, container string, podPhase corev1.PodPhase) (SidecarArtifacts, error) { + sidecarArtifacts := SidecarArtifacts{} + if podPhase == corev1.PodPending { + return sidecarArtifacts, nil + } + podLogOpts := corev1.PodLogOptions{Container: container} + req := clientset.CoreV1().Pods(namespace).GetLogs(name, &podLogOpts) + stream, err := req.Stream(ctx) + if err != nil { + return sidecarArtifacts, err + } + err = json.NewDecoder(stream).Decode(&sidecarArtifacts) + if err != nil { + return sidecarArtifacts, err + } + + return sidecarArtifacts, nil +} + +// LookForArtifacts searches for provenance.json files in the specified run directory +// and extracts artifacts from them. +func LookForArtifacts(names []string, runDir string) (SidecarArtifacts, error) { + err := waitForStepsToFinish(runDir) + if err != nil { + return nil, err + } + artifacts := SidecarArtifacts{} + for _, name := range names { + p := filepath.Join(stepDir, name, "artifacts", "provenance.json") + if exist, err := fileExists(p); err != nil { + return artifacts, err + } else if !exist { + continue + } + subRes, err := extractArtifactsFromFile(p) + if err != nil { + return SidecarArtifacts{}, err + } + artifacts[name] = v1.Artifacts{Inputs: subRes.Inputs, Outputs: subRes.Outputs} + } + return artifacts, nil +} diff --git a/internal/sidecarlogartifacts/sidecarlogartifacts_test.go b/internal/sidecarlogartifacts/sidecarlogartifacts_test.go new file mode 100644 index 00000000000..345c5b5f6a2 --- /dev/null +++ b/internal/sidecarlogartifacts/sidecarlogartifacts_test.go @@ -0,0 +1,255 @@ +/* +Copyright 2024 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 sidecarlogartifacts + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/google/go-cmp/cmp" + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "github.com/tektoncd/pipeline/test/diff" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + fakekubeclientset "k8s.io/client-go/kubernetes/fake" +) + +func TestLookForArtifacts_WaitForFiles(t *testing.T) { + tests := []struct { + desc string + runDirMode os.FileMode + stepDir string + outFile string + }{ + { + desc: "out.err file exist, no err", + runDirMode: 0o755, + stepDir: "first", + outFile: "out.err", + }, + { + desc: "out file exist, no err", + runDirMode: 0o755, + stepDir: "first", + outFile: "out", + }, + } + + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + dir := t.TempDir() + _ = os.Chmod(dir, tc.runDirMode) + if tc.stepDir != "" { + _ = os.MkdirAll(filepath.Join(dir, tc.stepDir), os.ModePerm) + + outFile := filepath.Join(dir, tc.stepDir, tc.outFile) + _, err := os.Create(outFile) + if err != nil { + t.Fatalf("failed to create file %v", err) + } + } + _, err := LookForArtifacts([]string{}, dir) + if err != nil { + t.Fatalf("failed to look for artifacts %v", err) + } + }) + } +} + +func TestLookForArtifacts(t *testing.T) { + base := basicArtifacts() + var modified = base.DeepCopy() + modified.Outputs[0].Name = "tests" + type Arg struct { + stepName string + artifacts *v1.Artifacts + customContent []byte + } + tests := []struct { + desc string + wantErr bool + args []Arg + expected SidecarArtifacts + }{ + { + desc: "one step produces artifacts, read success", + args: []Arg{{stepName: "first", artifacts: &base}}, + expected: map[string]v1.Artifacts{"first": base}, + }, { + desc: "two step produce artifacts, read success", + args: []Arg{{stepName: "first", artifacts: &base}, {stepName: "second", artifacts: modified}}, + expected: map[string]v1.Artifacts{"first": base, "second": *modified}, + }, + { + desc: "one step produces artifacts, one step does not, read success", + args: []Arg{{stepName: "first", artifacts: &base}, {stepName: "second"}}, + expected: map[string]v1.Artifacts{"first": base}, + }, + { + desc: "two step produces, one read success, one not, error out and result is empty.", + args: []Arg{{stepName: "first", artifacts: &base}, {stepName: "second", artifacts: modified, customContent: []byte("this is to break json")}}, + expected: map[string]v1.Artifacts{}, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + dir := t.TempDir() + curStepDir := stepDir + stepDir = dir + t.Cleanup(func() { + stepDir = curStepDir + }) + + var names []string + for _, arg := range tc.args { + names = append(names, arg.stepName) + if err := os.MkdirAll(filepath.Join(dir, arg.stepName, "artifacts"), os.ModePerm); err != nil { + t.Errorf("failed to create artifacts folder, err: %v", err) + } + if _, err := os.Create(filepath.Join(dir, arg.stepName, "out")); err != nil { + t.Errorf("failed to file, err: %v", err) + } + if arg.artifacts != nil { + if err := writeArtifacts(filepath.Join(dir, arg.stepName, "artifacts", "provenance.json"), arg.artifacts); err != nil { + t.Errorf("failed to write artifacts to provenance.json, err: %v", err) + } + } + if arg.customContent != nil { + if err := os.WriteFile(filepath.Join(dir, arg.stepName, "artifacts", "provenance.json"), arg.customContent, os.ModePerm); err != nil { + t.Errorf("failed to write customContent to provenance.json, err: %v", err) + } + } + } + got, err := LookForArtifacts(names, dir) + if (err != nil) != tc.wantErr { + t.Errorf("error checking failed, wantErr: %v, got: %v", tc.wantErr, err) + } + if d := cmp.Diff(tc.expected, got); d != "" { + t.Errorf(diff.PrintWantGot(d)) + } + }) + } +} + +func TestGetArtifactsFromSidecarLogs(t *testing.T) { + for _, c := range []struct { + desc string + podPhase corev1.PodPhase + wantError bool + }{{ + desc: "pod pending to start", + podPhase: corev1.PodPending, + wantError: false, + }, { + desc: "pod running extract logs", + podPhase: corev1.PodRunning, + wantError: true, + }} { + t.Run(c.desc, func(t *testing.T) { + ctx := context.Background() + clientset := fakekubeclientset.NewSimpleClientset() + pod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + Kind: "Pod", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "pod", + Namespace: "foo", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container", + Image: "image", + }, + }, + }, + Status: corev1.PodStatus{ + Phase: c.podPhase, + }, + } + pod, err := clientset.CoreV1().Pods(pod.Namespace).Create(context.TODO(), pod, metav1.CreateOptions{}) + if err != nil { + t.Errorf("Error occurred while creating pod %s: %s", pod.Name, err.Error()) + } + + // Fake logs are not formatted properly so there will be an error + _, err = GetArtifactsFromSidecarLogs(ctx, clientset, "foo", "pod", "container", pod.Status.Phase) + if err != nil && !c.wantError { + t.Fatalf("did not expect an error but got: %v", err) + } + if c.wantError && err == nil { + t.Fatal("expected to get an error but did not") + } + }) + } +} + +func writeArtifacts(path string, artifacts *v1.Artifacts) error { + f, err := os.Create(path) + if err != nil { + return err + } + defer f.Close() + res := json.NewEncoder(f).Encode(artifacts) + return res +} + +func basicArtifacts() v1.Artifacts { + data := `{ + "inputs":[ + { + "name":"inputs", + "values":[ + { + "uri":"pkg:example.github.com/inputs", + "digest":{ + "sha256":"b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0" + } + } + ] + } + ], + "outputs":[ + { + "name":"image", + "values":[ + { + "uri":"pkg:github/package-url/purl-spec@244fd47e07d1004f0aed9c", + "digest":{ + "sha256":"df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48", + "sha1":"95588b8f34c31eb7d62c92aaa4e6506639b06ef2" + } + } + ] + } + ] + } +` + var ars v1.Artifacts + err := json.Unmarshal([]byte(data), &ars) + if err != nil { + panic(err) + } + return ars +} diff --git a/pkg/apis/pipeline/images.go b/pkg/apis/pipeline/images.go index ae3127ca734..e3df0be9cfc 100644 --- a/pkg/apis/pipeline/images.go +++ b/pkg/apis/pipeline/images.go @@ -37,6 +37,9 @@ type Images struct { // WorkingDirInitImage is the container image containing our working dir init binary. WorkingDirInitImage string + // SidecarLogArtifactsImage is container image containing the binary that fetches artifacts from the steps and logs it to stdout. + SidecarLogArtifactsImage string + // NOTE: Make sure to add any new images to Validate below! } @@ -52,6 +55,7 @@ func (i Images) Validate() error { {i.ShellImage, "shell-image"}, {i.ShellImageWin, "shell-image-win"}, {i.WorkingDirInitImage, "workingdirinit-image"}, + {i.SidecarLogArtifactsImage, "sidecarlogartifacts-image"}, } { if f.v == "" { unset = append(unset, f.name) diff --git a/pkg/apis/pipeline/images_test.go b/pkg/apis/pipeline/images_test.go index ca25053f8d3..d43db2fb304 100644 --- a/pkg/apis/pipeline/images_test.go +++ b/pkg/apis/pipeline/images_test.go @@ -24,23 +24,25 @@ import ( func TestValidate(t *testing.T) { valid := pipeline.Images{ - EntrypointImage: "set", - SidecarLogResultsImage: "set", - NopImage: "set", - ShellImage: "set", - ShellImageWin: "set", - WorkingDirInitImage: "set", + EntrypointImage: "set", + SidecarLogResultsImage: "set", + SidecarLogArtifactsImage: "set", + NopImage: "set", + ShellImage: "set", + ShellImageWin: "set", + WorkingDirInitImage: "set", } if err := valid.Validate(); err != nil { t.Errorf("valid Images returned error: %v", err) } invalid := pipeline.Images{ - EntrypointImage: "set", - SidecarLogResultsImage: "set", - NopImage: "set", - ShellImage: "", // unset! - ShellImageWin: "set", + EntrypointImage: "set", + SidecarLogResultsImage: "set", + SidecarLogArtifactsImage: "set", + NopImage: "set", + ShellImage: "", // unset! + ShellImageWin: "set", } wantErr := "found unset image flags: [shell-image workingdirinit-image]" if err := invalid.Validate(); err == nil { diff --git a/pkg/apis/pipeline/sidecarlogs.go b/pkg/apis/pipeline/sidecarlogs.go index a0c570675ef..2f56e438073 100644 --- a/pkg/apis/pipeline/sidecarlogs.go +++ b/pkg/apis/pipeline/sidecarlogs.go @@ -21,7 +21,15 @@ const ( // when the results-from feature-flag is set to "sidecar-logs". ReservedResultsSidecarName = "tekton-log-results" + // ReservedArtifactsSidecarName is the name of the artifacts sidecar that outputs the artifacts to stdout + // when the results-from feature-flag is set to "sidecar-logs". + ReservedArtifactsSidecarName = "tekton-log-artifacts" + // ReservedResultsSidecarContainerName is the name of the results sidecar container that is injected // by the reconciler. ReservedResultsSidecarContainerName = "sidecar-tekton-log-results" + + // ReservedArtifactsSidecarContainerName is the name of the artifacts sidecar container that is injected + // by the reconciler. + ReservedArtifactsSidecarContainerName = "sidecar-tekton-log-artifacts" ) diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index 5997131e5bd..fdbb28be973 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -126,7 +126,7 @@ var ( // Additionally, Step timeouts are added as entrypoint flag. func orderContainers(ctx context.Context, commonExtraEntrypointArgs []string, steps []corev1.Container, taskSpec *v1.TaskSpec, breakpointConfig *v1.TaskRunDebug, waitForReadyAnnotation, enableKeepPodOnCancel bool) ([]corev1.Container, error) { if len(steps) == 0 { - return nil, errors.New("No steps specified") + return nil, errors.New("no steps specified") } for i, s := range steps { @@ -300,6 +300,9 @@ func StopSidecars(ctx context.Context, nopImage string, kubeclient kubernetes.In if config.FromContextOrDefaults(ctx).FeatureFlags.ResultExtractionMethod == config.ResultExtractionMethodSidecarLogs && s.Name == pipeline.ReservedResultsSidecarContainerName { continue } + if config.FromContextOrDefaults(ctx).FeatureFlags.ResultExtractionMethod == config.ResultExtractionMethodSidecarLogs && s.Name == pipeline.ReservedArtifactsSidecarContainerName { + continue + } // Stop any running container that isn't a step. // An injected sidecar container might not have the // "sidecar-" prefix, so we can't just look for that diff --git a/pkg/pod/entrypoint_test.go b/pkg/pod/entrypoint_test.go index 296bedc3cd8..ba19e20659e 100644 --- a/pkg/pod/entrypoint_test.go +++ b/pkg/pod/entrypoint_test.go @@ -842,6 +842,18 @@ func TestStopSidecars(t *testing.T) { Image: nopImage, } + // This is a container that is added by the controller for accessing sidecar logs. + // This should not be stopped as long as results-from is set to sidecar-logs. + artifactsSidecar := corev1.Container{ + Name: pipeline.ReservedArtifactsSidecarContainerName, + Image: "original-injected-image", + } + // This container can be stopped if the results-from is not set to sidecar-logs. + stoppedArtifactsSidecar := corev1.Container{ + Name: pipeline.ReservedArtifactsSidecarContainerName, + Image: nopImage, + } + for _, c := range []struct { desc string pod corev1.Pod @@ -923,6 +935,57 @@ func TestStopSidecars(t *testing.T) { }, }, wantContainers: []corev1.Container{stepContainer, stoppedSidecarContainer, stoppedResultsSidecar}, + }, { + desc: "Artifacts Sidecar should not be stopped", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{stepContainer, sidecarContainer, artifactsSidecar}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{{ + // Step state doesn't matter. + }, { + Name: sidecarContainer.Name, + // Sidecar is running. + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}}, + }, { + Name: artifactsSidecar.Name, + // Artifacts sidecar is running. + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}}, + }}, + }, + }, + resultExtractionMethod: "sidecar-logs", + wantContainers: []corev1.Container{stepContainer, stoppedSidecarContainer, artifactsSidecar}, + }, { + desc: "Artifacts Sidecar should be stopped result method is not sidecar logs", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{stepContainer, sidecarContainer, artifactsSidecar}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{{ + // Step state doesn't matter. + }, { + Name: sidecarContainer.Name, + // Sidecar is running. + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}}, + }, { + Name: artifactsSidecar.Name, + // Artifacts sidecar is running. + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}}, + }}, + }, + }, + wantContainers: []corev1.Container{stepContainer, stoppedSidecarContainer, stoppedArtifactsSidecar}, }, { desc: "Pending Pod should not be updated", pod: corev1.Pod{ diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 5beefb8fa35..df6ae436693 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -201,15 +201,23 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta tasklevel.ApplyTaskLevelComputeResources(steps, taskRun.Spec.ComputeResources) } windows := usesWindows(taskRun) - if sidecarLogsResultsEnabled && taskSpec.Results != nil { - // create a results sidecar - resultsSidecar, err := createResultsSidecar(taskSpec, b.Images.SidecarLogResultsImage, setSecurityContext, windows) + if sidecarLogsResultsEnabled { + artifactsSidecar, err := createArtifactsSidecar(taskSpec, b.Images.SidecarLogArtifactsImage, setSecurityContext, windows) if err != nil { return nil, err } - taskSpec.Sidecars = append(taskSpec.Sidecars, resultsSidecar) + taskSpec.Sidecars = append(taskSpec.Sidecars, artifactsSidecar) + if taskSpec.Results != nil { + // create a results sidecar + resultsSidecar, err := createResultsSidecar(taskSpec, b.Images.SidecarLogResultsImage, setSecurityContext, windows) + if err != nil { + return nil, err + } + taskSpec.Sidecars = append(taskSpec.Sidecars, resultsSidecar) + } commonExtraEntrypointArgs = append(commonExtraEntrypointArgs, "-result_from", config.ResultExtractionMethodSidecarLogs) } + sidecars, err := v1.MergeSidecarsWithSpecs(taskSpec.Sidecars, taskRun.Spec.SidecarSpecs) if err != nil { return nil, err @@ -339,10 +347,37 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta stepContainers[i].VolumeMounts = vms } - if sidecarLogsResultsEnabled && taskSpec.Results != nil { + if sidecarLogsResultsEnabled { // Mount implicit volumes onto sidecarContainers // so that they can access /tekton/results and /tekton/run. + if taskSpec.Results != nil { + for i, s := range sidecarContainers { + if s.Name != pipeline.ReservedResultsSidecarName { + continue + } + for j := 0; j < len(stepContainers); j++ { + s.VolumeMounts = append(s.VolumeMounts, runMount(j, true)) + } + requestedVolumeMounts := map[string]bool{} + for _, vm := range s.VolumeMounts { + requestedVolumeMounts[filepath.Clean(vm.MountPath)] = true + } + var toAdd []corev1.VolumeMount + for _, imp := range volumeMounts { + if !requestedVolumeMounts[filepath.Clean(imp.MountPath)] { + toAdd = append(toAdd, imp) + } + } + vms := append(s.VolumeMounts, toAdd...) //nolint:gocritic + sidecarContainers[i].VolumeMounts = vms + } + } + for i, s := range sidecarContainers { + if s.Name != pipeline.ReservedArtifactsSidecarName { + continue + } + for j := 0; j < len(stepContainers); j++ { s.VolumeMounts = append(s.VolumeMounts, runMount(j, true)) } @@ -734,3 +769,33 @@ func usesWindows(tr *v1.TaskRun) bool { osSelector := tr.Spec.PodTemplate.NodeSelector[osSelectorLabel] return osSelector == "windows" } + +// createArtifactsSidecar creates a sidecar that will run the sidecarlogartifacts binary, +// based on the spec of the Task, the image that should run in the artifacts sidecar, +// whether it will run on a windows node, and whether the sidecar should include a security context +// that will allow it to run in namespaces with "restricted" pod security admission. +// It will also provide arguments to the binary that allow it to surface the step artifacts. +func createArtifactsSidecar(taskSpec v1.TaskSpec, image string, setSecurityContext, windows bool) (v1.Sidecar, error) { + var stepNames []string + + for i, s := range taskSpec.Steps { + stepName := StepName(s.Name, i) + stepNames = append(stepNames, stepName) + } + + command := []string{"/ko-app/sidecarlogartifacts", "-step-names", strings.Join(stepNames, ",")} + + sidecar := v1.Sidecar{ + Name: pipeline.ReservedArtifactsSidecarName, + Image: image, + Command: command, + } + securityContext := linuxSecurityContext + if windows { + securityContext = windowsSecurityContext + } + if setSecurityContext { + sidecar.SecurityContext = securityContext + } + return sidecar, nil +} diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 0ff4e0f0154..b09e86c6bf9 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -1982,6 +1982,21 @@ _EOF_ MountPath: "/tekton/creds", }}, implicitVolumeMounts...), TerminationMessagePath: "/tekton/termination", + }, { + Name: pipeline.ReservedArtifactsSidecarContainerName, + Image: "", + Command: []string{ + "/ko-app/sidecarlogartifacts", + "-step-names", + "step-name", + }, + Resources: corev1.ResourceRequirements{ + Requests: nil, + }, + VolumeMounts: append([]corev1.VolumeMount{ + {Name: "tekton-internal-bin", ReadOnly: true, MountPath: "/tekton/bin"}, + {Name: "tekton-internal-run-0", ReadOnly: true, MountPath: "/tekton/run/0"}, + }, implicitVolumeMounts...), }, { Name: pipeline.ReservedResultsSidecarContainerName, Image: "", @@ -2061,6 +2076,21 @@ _EOF_ MountPath: "/tekton/creds", }}, implicitVolumeMounts...), TerminationMessagePath: "/tekton/termination", + }, { + Name: pipeline.ReservedArtifactsSidecarContainerName, + Image: "", + Command: []string{ + "/ko-app/sidecarlogartifacts", + "-step-names", + "step-name", + }, + Resources: corev1.ResourceRequirements{ + Requests: nil, + }, + VolumeMounts: append([]corev1.VolumeMount{ + {Name: "tekton-internal-bin", ReadOnly: true, MountPath: "/tekton/bin"}, + {Name: "tekton-internal-run-0", ReadOnly: true, MountPath: "/tekton/run/0"}, + }, implicitVolumeMounts...), }, { Name: pipeline.ReservedResultsSidecarContainerName, Image: "", @@ -2134,6 +2164,22 @@ _EOF_ MountPath: "/tekton/creds", }}, implicitVolumeMounts...), TerminationMessagePath: "/tekton/termination", + }, { + Name: pipeline.ReservedArtifactsSidecarContainerName, + Image: "", + Command: []string{ + "/ko-app/sidecarlogartifacts", + "-step-names", + "step-name", + }, + Resources: corev1.ResourceRequirements{ + Requests: nil, + }, + VolumeMounts: append([]corev1.VolumeMount{ + {Name: "tekton-internal-bin", ReadOnly: true, MountPath: "/tekton/bin"}, + {Name: "tekton-internal-run-0", ReadOnly: true, MountPath: "/tekton/run/0"}, + }, implicitVolumeMounts...), + SecurityContext: linuxSecurityContext, }, { Name: pipeline.ReservedResultsSidecarContainerName, Image: "", diff --git a/pkg/pod/status.go b/pkg/pod/status.go index 0b034e11fda..d99216f0d1d 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -25,6 +25,7 @@ import ( "time" "github.com/hashicorp/go-multierror" + "github.com/tektoncd/pipeline/internal/sidecarlogartifacts" "github.com/tektoncd/pipeline/internal/sidecarlogresults" "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" @@ -223,13 +224,24 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL // Extract results from sidecar logs sidecarLogsResultsEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.ResultExtractionMethod == config.ResultExtractionMethodSidecarLogs sidecarLogResults := []result.RunResult{} - if sidecarLogsResultsEnabled && tr.Status.TaskSpec.Results != nil { + + sidecarArtifacts := map[string]v1.Artifacts{} + + if sidecarLogsResultsEnabled { // extraction of results from sidecar logs - slr, err := sidecarlogresults.GetResultsFromSidecarLogs(ctx, kubeclient, tr.Namespace, tr.Status.PodName, pipeline.ReservedResultsSidecarContainerName, podPhase) + if tr.Status.TaskSpec.Results != nil { + slr, err := sidecarlogresults.GetResultsFromSidecarLogs(ctx, kubeclient, tr.Namespace, tr.Status.PodName, pipeline.ReservedResultsSidecarContainerName, podPhase) + if err != nil { + merr = multierror.Append(merr, err) + } + sidecarLogResults = append(sidecarLogResults, slr...) + } + + sa, err := sidecarlogartifacts.GetArtifactsFromSidecarLogs(ctx, kubeclient, tr.Namespace, tr.Status.PodName, pipeline.ReservedArtifactsSidecarContainerName, podPhase) + sidecarArtifacts = sa if err != nil { merr = multierror.Append(merr, err) } - sidecarLogResults = append(sidecarLogResults, slr...) } // Populate Task results from sidecar logs taskResultsFromSidecarLogs := getTaskResultsFromSidecarLogs(sidecarLogResults) @@ -270,10 +282,19 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL // Set TaskResults from StepResults trs.Results = append(trs.Results, createTaskResultsFromStepResults(stepRunRes, neededStepResults)...) } + var as v1.Artifacts + + if v, ok := sidecarArtifacts[s.Name]; ok { + as.Inputs = v.Inputs + as.Outputs = v.Outputs + } + + if err != nil { + merr = multierror.Append(merr, err) + } // Parse termination messages terminationReason := "" - var as v1.Artifacts if state.Terminated != nil && len(state.Terminated.Message) != 0 { msg := state.Terminated.Message diff --git a/pkg/pod/status_test.go b/pkg/pod/status_test.go index 93796e0759c..c18ec10e9b2 100644 --- a/pkg/pod/status_test.go +++ b/pkg/pod/status_test.go @@ -122,11 +122,11 @@ func TestSetTaskRunStatusBasedOnStepStatus_sidecar_logs(t *testing.T) { }{{ desc: "test result with sidecar logs too large", maxResultSize: 1, - wantErr: sidecarlogresults.ErrSizeExceeded, + wantErr: multierror.Append(sidecarlogresults.ErrSizeExceeded, fmt.Errorf("%s", "invalid character 'k' in literal false (expecting 'l')")), }, { desc: "test result with sidecar logs bad format", maxResultSize: 4096, - wantErr: fmt.Errorf("%s", "invalid result \"\": invalid character 'k' in literal false (expecting 'l')"), + wantErr: multierror.Append(fmt.Errorf("%s", "invalid result \"\": invalid character 'k' in literal false (expecting 'l')"), fmt.Errorf("%s", "invalid character 'k' in literal false (expecting 'l')")), }} { t.Run(c.desc, func(t *testing.T) { tr := v1.TaskRun{ diff --git a/test/artifacts_test.go b/test/artifacts_test.go index 39ca286b3aa..03c2137b3b7 100644 --- a/test/artifacts_test.go +++ b/test/artifacts_test.go @@ -40,43 +40,57 @@ var ( } ) -func TestSurfaceArtifactsThroughTerminationMessage(t *testing.T) { - featureFlags := getFeatureFlagsBaseOnAPIFlag(t) - checkFlagsEnabled := requireAllGates(requireEnableStepArtifactsGate) +func TestSurfaceArtifacts(t *testing.T) { + tests := []struct { + desc string + resultExtractionMethod string + }{ + { + desc: "surface artifacts through termination message", + resultExtractionMethod: config.ResultExtractionMethodTerminationMessage}, + { + desc: "surface artifacts through sidecar logs", + resultExtractionMethod: config.ResultExtractionMethodSidecarLogs}, + } - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() - c, namespace := setup(ctx, t) - checkFlagsEnabled(ctx, t, c, "") - previous := featureFlags.ResultExtractionMethod - updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ - "results-from": config.ResultExtractionMethodTerminationMessage, - }) + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + featureFlags := getFeatureFlagsBaseOnAPIFlag(t) + checkFlagsEnabled := requireAllGates(requireEnableStepArtifactsGate) - knativetest.CleanupOnInterrupt(func() { - updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ - "results-from": previous, - }) - tearDown(ctx, t, c, namespace) - }, t.Logf) - defer func() { - updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ - "results-from": previous, - }) - tearDown(ctx, t, c, namespace) - }() + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + c, namespace := setup(ctx, t) + checkFlagsEnabled(ctx, t, c, "") + previous := featureFlags.ResultExtractionMethod + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": tc.resultExtractionMethod, + }) - taskRunName := helpers.ObjectNameForTest(t) + knativetest.CleanupOnInterrupt(func() { + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": previous, + }) + tearDown(ctx, t, c, namespace) + }, t.Logf) + defer func() { + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": previous, + }) + tearDown(ctx, t, c, namespace) + }() - fqImageName := getTestImage(busyboxImage) + taskRunName := helpers.ObjectNameForTest(t) - t.Logf("Creating Task and TaskRun in namespace %s", namespace) - task := simpleArtifactProducerTask(t, namespace, fqImageName) - if _, err := c.V1TaskClient.Create(ctx, task, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create Task: %s", err) - } - taskRun := parse.MustParseV1TaskRun(t, fmt.Sprintf(` + fqImageName := getTestImage(busyboxImage) + + t.Logf("Creating Task and TaskRun in namespace %s", namespace) + task := simpleArtifactProducerTask(t, namespace, fqImageName) + if _, err := c.V1TaskClient.Create(ctx, task, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create Task: %s", err) + } + taskRun := parse.MustParseV1TaskRun(t, fmt.Sprintf(` metadata: name: %s namespace: %s @@ -84,31 +98,33 @@ spec: taskRef: name: %s `, taskRunName, namespace, task.Name)) - if _, err := c.V1TaskRunClient.Create(ctx, taskRun, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create TaskRun: %s", err) - } + if _, err := c.V1TaskRunClient.Create(ctx, taskRun, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create TaskRun: %s", err) + } - if err := WaitForTaskRunState(ctx, c, taskRunName, TaskRunSucceed(taskRunName), "TaskRunSucceed", v1Version); err != nil { - t.Errorf("Error waiting for TaskRun to finish: %s", err) - } + if err := WaitForTaskRunState(ctx, c, taskRunName, TaskRunSucceed(taskRunName), "TaskRunSucceed", v1Version); err != nil { + t.Errorf("Error waiting for TaskRun to finish: %s", err) + } - taskrun, err := c.V1TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{}) - if err != nil { - t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err) - } - if d := cmp.Diff([]v1.TaskRunStepArtifact{{Name: "input-artifacts", - Values: []v1.ArtifactValue{{Digest: map[v1.Algorithm]string{"sha256": "b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0"}, - Uri: "git:jjjsss", - }}, - }}, taskrun.Status.Steps[0].Inputs); d != "" { - t.Fatalf(`The expected stepState Inputs does not match created taskrun stepState Inputs. Here is the diff: %v`, d) - } - if d := cmp.Diff([]v1.TaskRunStepArtifact{{Name: "build-result", - Values: []v1.ArtifactValue{{Digest: map[v1.Algorithm]string{"sha1": "95588b8f34c31eb7d62c92aaa4e6506639b06ef2", "sha256": "df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48"}, - Uri: "pkg:balba", - }}, - }}, taskrun.Status.Steps[0].Outputs); d != "" { - t.Fatalf(`The expected stepState Outputs does not match created taskrun stepState Outputs. Here is the diff: %v`, d) + taskrun, err := c.V1TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err) + } + if d := cmp.Diff([]v1.TaskRunStepArtifact{{Name: "source", + Values: []v1.ArtifactValue{{Digest: map[v1.Algorithm]string{"sha256": "b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0"}, + Uri: "git:jjjsss", + }}, + }}, taskrun.Status.Steps[0].Inputs); d != "" { + t.Fatalf(`The expected stepState Inputs does not match created taskrun stepState Inputs. Here is the diff: %v`, d) + } + if d := cmp.Diff([]v1.TaskRunStepArtifact{{Name: "image", + Values: []v1.ArtifactValue{{Digest: map[v1.Algorithm]string{"sha1": "95588b8f34c31eb7d62c92aaa4e6506639b06ef2", "sha256": "df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48"}, + Uri: "pkg:balba", + }}, + }}, taskrun.Status.Steps[0].Outputs); d != "" { + t.Fatalf(`The expected stepState Outputs does not match created taskrun stepState Outputs. Here is the diff: %v`, d) + } + }) } } @@ -281,7 +297,7 @@ spec: { "inputs":[ { - "name":"input-artifacts", + "name":"source", "values":[ { "uri":"git:jjjsss", @@ -294,7 +310,7 @@ spec: ], "outputs":[ { - "name":"build-result", + "name":"image", "values":[ { "uri":"pkg:balba",