diff --git a/cmd/controller/main.go b/cmd/controller/main.go index f0867a5fafc..f01d4f1881a 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -54,7 +54,7 @@ var ( func main() { flag.Parse() images := pipeline.Images{ - EntryPointImage: *entrypointImage, + EntrypointImage: *entrypointImage, NopImage: *nopImage, GitImage: *gitImage, CredsImage: *credsImage, diff --git a/examples/taskruns/steps-run-in-order.yaml b/examples/taskruns/steps-run-in-order.yaml index dd31b3a81b0..d5924a05dff 100644 --- a/examples/taskruns/steps-run-in-order.yaml +++ b/examples/taskruns/steps-run-in-order.yaml @@ -6,8 +6,7 @@ spec: taskSpec: steps: - image: busybox - command: ['/bin/sh'] + # NB: command is not set, so it must be looked up from the registry. args: ['-c', 'sleep 3 && touch foo'] - image: busybox - command: ['/bin/sh'] args: ['-c', 'ls', 'foo'] diff --git a/pkg/apis/pipeline/images.go b/pkg/apis/pipeline/images.go index e4793c152ab..899256e47cd 100644 --- a/pkg/apis/pipeline/images.go +++ b/pkg/apis/pipeline/images.go @@ -19,8 +19,8 @@ package pipeline // Images holds the images reference for a number of container images used // across tektoncd pipelines. type Images struct { - // EntryPointImage is container image containing our entrypoint binary. - EntryPointImage string + // EntrypointImage is container image containing our entrypoint binary. + EntrypointImage string // NopImage is the container image used to kill sidecars. NopImage string // GitImage is the container image with Git that we use to implement the Git source step. diff --git a/pkg/apis/pipeline/v1alpha1/build_gcs_resource_test.go b/pkg/apis/pipeline/v1alpha1/build_gcs_resource_test.go index e3e5ff4d5e7..6316243dc6f 100644 --- a/pkg/apis/pipeline/v1alpha1/build_gcs_resource_test.go +++ b/pkg/apis/pipeline/v1alpha1/build_gcs_resource_test.go @@ -28,7 +28,7 @@ import ( ) var images = pipeline.Images{ - EntryPointImage: "override-with-entrypoint:latest", + EntrypointImage: "override-with-entrypoint:latest", NopImage: "tianon/true", GitImage: "override-with-git:latest", CredsImage: "override-with-creds:latest", diff --git a/pkg/artifacts/artifact_storage_test.go b/pkg/artifacts/artifact_storage_test.go index fd758a09f4d..64809afd917 100644 --- a/pkg/artifacts/artifact_storage_test.go +++ b/pkg/artifacts/artifact_storage_test.go @@ -34,7 +34,7 @@ import ( var ( images = pipeline.Images{ - EntryPointImage: "override-with-entrypoint:latest", + EntrypointImage: "override-with-entrypoint:latest", NopImage: "tianon/true", GitImage: "override-with-git:latest", CredsImage: "override-with-creds:latest", diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go new file mode 100644 index 00000000000..4ac3ed1cbb8 --- /dev/null +++ b/pkg/pod/entrypoint.go @@ -0,0 +1,180 @@ +package pod + +import ( + "errors" + "fmt" + "path/filepath" + "strings" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +const ( + toolsVolumeName = "tools" + mountPoint = "/builder/tools" + entrypointBinary = mountPoint + "/entrypoint" + + downwardVolumeName = "downward" + downwardMountPoint = "/builder/downward" + downwardMountReadyFile = "ready" + ReadyAnnotation = "tekton.dev/ready" + ReadyAnnotationValue = "READY" + + StepPrefix = "step-" + SidecarPrefix = "sidecar-" +) + +var ( + // TODO(#1605): Generate volumeMount names, to avoid collisions. + // TODO(#1605): Unexport these vars when Pod conversion is entirely within + // this package. + ToolsMount = corev1.VolumeMount{ + Name: toolsVolumeName, + MountPath: mountPoint, + } + ToolsVolume = corev1.Volume{ + Name: toolsVolumeName, + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, + } + + // TODO(#1605): Signal sidecar readiness by injecting entrypoint, + // remove dependency on Downward API. + DownwardVolume = corev1.Volume{ + Name: downwardVolumeName, + VolumeSource: corev1.VolumeSource{ + DownwardAPI: &corev1.DownwardAPIVolumeSource{ + Items: []corev1.DownwardAPIVolumeFile{{ + Path: downwardMountReadyFile, + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: fmt.Sprintf("metadata.annotations['%s']", ReadyAnnotation), + }, + }}, + }, + }, + } + DownwardMount = corev1.VolumeMount{ + Name: downwardVolumeName, + MountPath: downwardMountPoint, + } +) + +// OrderContainers returns the specified steps, modified so that they are +// executed in order by overriding the entrypoint binary. It also returns the +// init container that places the entrypoint binary pulled from the +// entrypointImage. +// +// Containers must have Command specified; if the user didn't specify a +// command, we must have fetched the image's ENTRYPOINT before calling this +// method, using entrypoint_lookup.go. +// +// TODO(#1605): Also use entrypoint injection to order sidecar start/stop. +func OrderContainers(entrypointImage string, steps []corev1.Container) (corev1.Container, []corev1.Container, error) { + toolsInit := corev1.Container{ + Name: "place-tools", + Image: entrypointImage, + Command: []string{"cp", "/ko-app/entrypoint", entrypointBinary}, + VolumeMounts: []corev1.VolumeMount{ToolsMount}, + } + + if len(steps) == 0 { + return corev1.Container{}, nil, errors.New("No steps specified") + } + + for i, s := range steps { + var argsForEntrypoint []string + switch i { + case 0: + argsForEntrypoint = []string{ + // First step waits for the Downward volume file. + "-wait_file", filepath.Join(downwardMountPoint, downwardMountReadyFile), + "-wait_file_content", // Wait for file contents, not just an empty file. + // Start next step. + "-post_file", filepath.Join(mountPoint, fmt.Sprintf("%d", i)), + } + default: + // All other steps wait for previous file, write next file. + argsForEntrypoint = []string{ + "-wait_file", filepath.Join(mountPoint, fmt.Sprintf("%d", i-1)), + "-post_file", filepath.Join(mountPoint, fmt.Sprintf("%d", i)), + } + } + + cmd, args := s.Command, s.Args + if len(cmd) == 0 { + return corev1.Container{}, nil, fmt.Errorf("Step %d did not specify command", i) + } + if len(cmd) > 1 { + args = append(cmd[1:], args...) + cmd = []string{cmd[0]} + } + argsForEntrypoint = append(argsForEntrypoint, "-entrypoint", cmd[0], "--") + argsForEntrypoint = append(argsForEntrypoint, args...) + + steps[i].Command = []string{entrypointBinary} + steps[i].Args = argsForEntrypoint + steps[i].VolumeMounts = append(steps[i].VolumeMounts, ToolsMount) + } + // Mount the Downward volume into the first step container. + steps[0].VolumeMounts = append(steps[0].VolumeMounts, DownwardMount) + + return toolsInit, steps, nil +} + +// UpdateReady updates the Pod's annotations to signal the first step to start +// by projecting the ready annotation via the Downward API. +func UpdateReady(kubeclient kubernetes.Interface, pod corev1.Pod) error { + newPod, err := kubeclient.CoreV1().Pods(pod.Namespace).Get(pod.Name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("Error getting Pod %q when updating ready annotation: %w", pod.Name, err) + } + + // Update the Pod's "READY" annotation to signal the first step to + // start. + if newPod.ObjectMeta.Annotations == nil { + newPod.ObjectMeta.Annotations = map[string]string{} + } + if newPod.ObjectMeta.Annotations[ReadyAnnotation] != ReadyAnnotationValue { + newPod.ObjectMeta.Annotations[ReadyAnnotation] = ReadyAnnotationValue + if _, err := kubeclient.CoreV1().Pods(newPod.Namespace).Update(newPod); err != nil { + return fmt.Errorf("Error adding ready annotation to Pod %q: %w", pod.Name, err) + } + } + return nil +} + +// StopSidecars updates sidecar containers in the Pod to a nop image, which +// exits successfully immediately. +func StopSidecars(nopImage string, kubeclient kubernetes.Interface, pod corev1.Pod) error { + newPod, err := kubeclient.CoreV1().Pods(pod.Namespace).Get(pod.Name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("Error getting Pod %q when stopping sidecars: %w", pod.Name, err) + } + + updated := false + if newPod.Status.Phase == corev1.PodRunning { + for _, s := range newPod.Status.ContainerStatuses { + if IsContainerSidecar(s.Name) && s.State.Running != nil { + for j, c := range newPod.Spec.Containers { + if c.Name == s.Name && c.Image != nopImage { + updated = true + newPod.Spec.Containers[j].Image = nopImage + } + } + } + } + } + if updated { + if _, err := kubeclient.CoreV1().Pods(newPod.Namespace).Update(newPod); err != nil { + return fmt.Errorf("Error adding ready annotation to Pod %q: %w", pod.Name, err) + } + } + return nil +} + +func IsContainerStep(name string) bool { return strings.HasPrefix(name, StepPrefix) } +func IsContainerSidecar(name string) bool { return strings.HasPrefix(name, SidecarPrefix) } + +func TrimStepPrefix(name string) string { return strings.TrimPrefix(name, StepPrefix) } +func TrimSidecarPrefix(name string) string { return strings.TrimPrefix(name, SidecarPrefix) } diff --git a/pkg/pod/entrypoint_lookup.go b/pkg/pod/entrypoint_lookup.go new file mode 100644 index 00000000000..0cc1407fc7f --- /dev/null +++ b/pkg/pod/entrypoint_lookup.go @@ -0,0 +1,126 @@ +package pod + +import ( + "fmt" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/authn/k8schain" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/v1/remote" + lru "github.com/hashicorp/golang-lru" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/kubernetes" +) + +// ResolveEntrypoints looks up container image ENTRYPOINTs for all steps that +// don't specify a Command. +// +// Images that are not specified by digest will be specified by digest after +// lookup in the resulting list of containers. +func ResolveEntrypoints(cache EntrypointCache, namespace, serviceAccountName string, steps []corev1.Container) ([]corev1.Container, error) { + // Keep a local cache of image->digest lookups, just for the scope of + // resolving this set of steps. If the image is pushed to, we need to + // resolve its digest and entrypoint again, but we can skip lookups + // while resolving the same TaskRun. + type result struct { + digest name.Digest + ep []string + } + localCache := map[string]result{} + for i, s := range steps { + if len(s.Command) != 0 { + // Nothing to resolve. + continue + } + + var digest name.Digest + var ep []string + var err error + if r, found := localCache[s.Image]; found { + digest = r.digest + ep = r.ep + } else { + // Look it up in the cache, which will also resolve the digest. + ep, digest, err = cache.Get(s.Image, namespace, serviceAccountName) + if err != nil { + return nil, err + } + localCache[s.Image] = result{digest, ep} // Cache it locally in case another step specifies the same image. + } + + steps[i].Image = digest.String() // Specify image by digest, since we know it now. + steps[i].Command = ep // Specify the command explicitly. + } + return steps, nil +} + +const cacheSize = 1024 + +type EntrypointCache interface { + Get(imageName, namespace, serviceAccountName string) (cmd []string, d name.Digest, err error) +} + +type entrypointCache struct { + kubeclient kubernetes.Interface + lru *lru.Cache // cache of digest string -> image entrypoint []string +} + +func NewEntrypointCache(kubeclient kubernetes.Interface) (EntrypointCache, error) { + lru, err := lru.New(cacheSize) + if err != nil { + return nil, err + } + return &entrypointCache{ + kubeclient: kubeclient, + lru: lru, + }, nil +} + +func (e *entrypointCache) Get(imageName, namespace, serviceAccountName string) (cmd []string, d name.Digest, err error) { + ref, err := name.ParseReference(imageName, name.WeakValidation) + if err != nil { + return nil, name.Digest{}, fmt.Errorf("Error parsing reference %q: %v", imageName, err) + } + + // If image is specified by digest, check the local cache. + if digest, ok := ref.(name.Digest); ok { + if ep, ok := e.lru.Get(digest.String()); ok { + return ep.([]string), digest, nil + } + } + + // If the image wasn't specified by digest, or if the entrypoint + // wasn't found, we have to consult the remote registry, using + // imagePullSecrets. + kc, err := k8schain.New(e.kubeclient, k8schain.Options{ + Namespace: namespace, + ServiceAccountName: serviceAccountName, + }) + if err != nil { + return nil, name.Digest{}, fmt.Errorf("Error creating k8schain: %v", err) + } + mkc := authn.NewMultiKeychain(kc) + img, err := remote.Image(ref, remote.WithAuthFromKeychain(mkc)) + if err != nil { + return nil, name.Digest{}, fmt.Errorf("Error getting image manifest: %v", err) + } + digest, err := img.Digest() + if err != nil { + return nil, name.Digest{}, fmt.Errorf("Error getting image digest: %v", err) + } + cfg, err := img.ConfigFile() + if err != nil { + return nil, name.Digest{}, fmt.Errorf("Error getting image config: %v", err) + } + ep := cfg.Config.Entrypoint + if len(ep) == 0 { + ep = cfg.Config.Cmd + } + e.lru.Add(digest.String(), ep) // Populate the cache. + + d, err = name.NewDigest(imageName+"@"+digest.String(), name.WeakValidation) + if err != nil { + return nil, name.Digest{}, fmt.Errorf("Error constructing resulting digest: %v", err) + } + return ep, d, nil +} diff --git a/pkg/pod/entrypoint_lookup_test.go b/pkg/pod/entrypoint_lookup_test.go new file mode 100644 index 00000000000..c60772f7e7d --- /dev/null +++ b/pkg/pod/entrypoint_lookup_test.go @@ -0,0 +1,90 @@ +package pod + +import ( + "errors" + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-containerregistry/pkg/name" + corev1 "k8s.io/api/core/v1" +) + +const validDigest = "sha256:aec27421b7a64a63b5dbf3c62b4a1d44f0bda5632cc74b256651df920d61e09b" + +func TestResolveEntrypoints(t *testing.T) { + cache := fakeCache{ + "image-by-digest@" + validDigest: &data{ + ep: []string{"my", "entrypoint"}, + digest: validDigest, + }, + "my-image": &data{ + ep: []string{"cool", "entrypoint", "bro"}, + digest: validDigest, + }, + } + + got, err := ResolveEntrypoints(cache, "namespace", "serviceAccountName", []corev1.Container{{ + Image: "fully-specified", + Command: []string{"specified", "command"}, // nothing to resolve + }, { + Image: "my-image", + }, { + Image: "image-by-digest@" + validDigest, // specified by digest. + }, { + Image: "my-image", // Check whether we look it up again. + }}) + if err != nil { + t.Fatalf("Error resolving entrypoints: %v", err) + } + + want := []corev1.Container{{ + Image: "fully-specified", + Command: []string{"specified", "command"}, + }, { + Image: "index.docker.io/library/my-image@" + validDigest, // digest was resolved when looking up entrypoint. + Command: []string{"cool", "entrypoint", "bro"}, + }, { + Image: "index.docker.io/library/image-by-digest@" + validDigest, + Command: []string{"my", "entrypoint"}, + }, { + Image: "index.docker.io/library/my-image@" + validDigest, // digest was resolved when looking up entrypoint. + Command: []string{"cool", "entrypoint", "bro"}, + }} + if d := cmp.Diff(want, got); d != "" { + t.Fatalf("Diff (-want, +got): %s", d) + } +} + +type fakeCache map[string]*data +type data struct { + ep []string + digest string + seen bool // Whether the image has been looked up before. +} + +func (f fakeCache) Get(imageName, _, _ string) ([]string, name.Digest, error) { + ref, err := name.ParseReference(imageName, name.WeakValidation) + if err != nil { + return nil, name.Digest{}, err + } + if d, ok := ref.(name.Digest); ok { + if data, found := f[d.String()]; found { + return data.ep, d, nil + } + } + + d, found := f[imageName] + if !found { + return nil, name.Digest{}, errors.New("not found") + } + if d.seen { + return nil, name.Digest{}, fmt.Errorf("Image %q was already looked up!", imageName) + } + d.seen = true + dig, err := name.NewDigest(ref.Context().RepositoryStr()+"@"+d.digest, name.WeakValidation) + if err != nil { + return nil, name.Digest{}, err + } + return d.ep, dig, nil +} diff --git a/pkg/pod/entrypoint_test.go b/pkg/pod/entrypoint_test.go new file mode 100644 index 00000000000..fcfd42aa692 --- /dev/null +++ b/pkg/pod/entrypoint_test.go @@ -0,0 +1,245 @@ +package pod + +import ( + "testing" + "time" + + "github.com/google/go-cmp/cmp" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + fakek8s "k8s.io/client-go/kubernetes/fake" +) + +const entrypointImage = "entrypoint" + +var volumeMount = corev1.VolumeMount{ + Name: "my-mount", + MountPath: "/mount/point", +} + +func TestOrderContainers(t *testing.T) { + steps := []corev1.Container{{ + Image: "step-1", + Command: []string{"cmd"}, + Args: []string{"arg1", "arg2"}, + }, { + Image: "step-2", + Command: []string{"cmd1", "cmd2", "cmd3"}, // multiple cmd elements + Args: []string{"arg1", "arg2"}, + VolumeMounts: []corev1.VolumeMount{volumeMount}, // pre-existing volumeMount + }, { + Image: "step-3", + Command: []string{"cmd"}, + Args: []string{"arg1", "arg2"}, + }} + want := []corev1.Container{{ + Image: "step-1", + Command: []string{entrypointBinary}, + Args: []string{ + "-wait_file", "/builder/downward/ready", + "-wait_file_content", + "-post_file", "/builder/tools/0", + "-entrypoint", "cmd", "--", + "arg1", "arg2", + }, + VolumeMounts: []corev1.VolumeMount{ToolsMount, DownwardMount}, + }, { + Image: "step-2", + Command: []string{entrypointBinary}, + Args: []string{ + "-wait_file", "/builder/tools/0", + "-post_file", "/builder/tools/1", + "-entrypoint", "cmd1", "--", + "cmd2", "cmd3", + "arg1", "arg2", + }, + VolumeMounts: []corev1.VolumeMount{volumeMount, ToolsMount}, + }, { + Image: "step-3", + Command: []string{entrypointBinary}, + Args: []string{ + "-wait_file", "/builder/tools/1", + "-post_file", "/builder/tools/2", + "-entrypoint", "cmd", "--", + "arg1", "arg2", + }, + VolumeMounts: []corev1.VolumeMount{ToolsMount}, + }} + gotInit, got, err := OrderContainers(entrypointImage, steps) + if err != nil { + t.Fatalf("OrderContainers: %v", err) + } + if d := cmp.Diff(want, got); d != "" { + t.Errorf("Diff (-want, +got): %s", d) + } + + wantInit := corev1.Container{ + Name: "place-tools", + Image: entrypointImage, + Command: []string{"cp", "/ko-app/entrypoint", entrypointBinary}, + VolumeMounts: []corev1.VolumeMount{ToolsMount}, + } + if d := cmp.Diff(wantInit, gotInit); d != "" { + t.Errorf("Init Container Diff (-want, +got): %s", d) + } +} + +func TestUpdateReady(t *testing.T) { + for _, c := range []struct { + desc string + pod corev1.Pod + wantAnnotations map[string]string + }{{ + desc: "Pod without any annotations has it added", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod", + Annotations: nil, + }, + }, + wantAnnotations: map[string]string{ + ReadyAnnotation: ReadyAnnotationValue, + }, + }, { + desc: "Pod with existing annotations has it appended", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod", + Annotations: map[string]string{ + "something": "else", + }, + }, + }, + wantAnnotations: map[string]string{ + "something": "else", + ReadyAnnotation: ReadyAnnotationValue, + }, + }, { + desc: "Pod with other annotation value has it updated", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod", + Annotations: map[string]string{ + ReadyAnnotation: "something else", + }, + }, + }, + wantAnnotations: map[string]string{ + ReadyAnnotation: ReadyAnnotationValue, + }, + }} { + t.Run(c.desc, func(t *testing.T) { + kubeclient := fakek8s.NewSimpleClientset(&c.pod) + if err := UpdateReady(kubeclient, c.pod); err != nil { + t.Errorf("UpdateReady: %v", err) + } + + got, err := kubeclient.CoreV1().Pods(c.pod.Namespace).Get(c.pod.Name, metav1.GetOptions{}) + if err != nil { + t.Errorf("Getting pod %q after update: %v", c.pod.Name, err) + } else if d := cmp.Diff(c.wantAnnotations, got.Annotations); d != "" { + t.Errorf("Annotations Diff(-want, +got): %s", d) + } + }) + } +} + +const nopImage = "nop-image" + +// TestStopSidecars tests stopping sidecars by updating their images to a nop +// image. +func TestStopSidecars(t *testing.T) { + stepContainer := corev1.Container{ + Name: StepPrefix + "my-step", + Image: "foo", + } + sidecarContainer := corev1.Container{ + Name: SidecarPrefix + "-my-sidecar", + Image: "original-image", + Command: []string{"my", "command"}, + Args: []string{"my", "args"}, + Env: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + } + stoppedSidecarContainer := corev1.Container{ + Name: sidecarContainer.Name, + Image: nopImage, + Command: []string{"my", "command"}, + Args: []string{"my", "args"}, + Env: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + } + + for _, c := range []struct { + desc string + pod corev1.Pod + wantContainers []corev1.Container + }{{ + desc: "Running sidecar should be stopped", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{stepContainer, sidecarContainer}, + }, + 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())}}, + }}, + }, + }, + wantContainers: []corev1.Container{stepContainer, stoppedSidecarContainer}, + }, { + desc: "Pending Pod should not be updated", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{stepContainer, sidecarContainer}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodPending, + // Container states don't matter. + }, + }, + wantContainers: []corev1.Container{stepContainer, sidecarContainer}, + }, { + desc: "Non-Running sidecar should not be stopped", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{stepContainer, sidecarContainer}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{{ + // Step state doesn't matter. + }, { + Name: sidecarContainer.Name, + // Sidecar is waiting. + State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{}}, + }}, + }, + }, + wantContainers: []corev1.Container{stepContainer, sidecarContainer}, + }} { + t.Run(c.desc, func(t *testing.T) { + kubeclient := fakek8s.NewSimpleClientset(&c.pod) + if err := StopSidecars(nopImage, kubeclient, c.pod); err != nil { + t.Errorf("error stopping sidecar: %v", err) + } + + got, err := kubeclient.CoreV1().Pods(c.pod.Namespace).Get(c.pod.Name, metav1.GetOptions{}) + if err != nil { + t.Errorf("Getting pod %q after update: %v", c.pod.Name, err) + } else if d := cmp.Diff(c.wantContainers, got.Spec.Containers); d != "" { + t.Errorf("Containers Diff(-want, +got): %s", d) + } + }) + } +} diff --git a/pkg/pod/script.go b/pkg/pod/script.go new file mode 100644 index 00000000000..ca57db6f4a4 --- /dev/null +++ b/pkg/pod/script.go @@ -0,0 +1,89 @@ +package pod + +import ( + "fmt" + "path/filepath" + + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/pkg/names" + corev1 "k8s.io/api/core/v1" +) + +const ( + scriptsVolumeName = "scripts" + scriptsDir = "/builder/scripts" +) + +var ( + // Volume definition attached to Pods generated from TaskRuns that have + // steps that specify a Script. + // TODO(#1605): Generate volumeMount names, to avoid collisions. + // TODO(#1605): Unexport these vars when Pod conversion is entirely within + // this package. + ScriptsVolume = corev1.Volume{ + Name: scriptsVolumeName, + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, + } + ScriptsVolumeMount = corev1.VolumeMount{ + Name: scriptsVolumeName, + MountPath: scriptsDir, + } +) + +// ConvertScripts converts any steps that specify a Script field into a normal Container. +// +// It does this by prepending a container that writes specified Script bodies +// to executable files in a shared volumeMount, then produces Containers that +// simply run those executable files. +func ConvertScripts(shellImage string, steps []v1alpha1.Step) (*corev1.Container, []corev1.Container) { + placeScripts := false + placeScriptsInit := corev1.Container{ + Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("place-scripts"), + Image: shellImage, + TTY: true, + Command: []string{"sh"}, + Args: []string{"-c", ""}, + VolumeMounts: []corev1.VolumeMount{ScriptsVolumeMount}, + } + + var containers []corev1.Container + for i, s := range steps { + if s.Script == "" { + // Nothing to convert. + containers = append(containers, s.Container) + continue + } + + // At least one step uses a script, so we should return a + // non-nil init container. + placeScripts = true + + // Append to the place-scripts script to place the + // script file in a known location in the scripts volume. + tmpFile := filepath.Join(scriptsDir, names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("script-%d", i))) + // heredoc is the "here document" placeholder string + // used to cat script contents into the file. Typically + // this is the string "EOF" but if this value were + // "EOF" it would prevent users from including the + // string "EOF" in their own scripts. Instead we + // randomly generate a string to (hopefully) prevent + // collisions. + heredoc := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("script-heredoc-randomly-generated") + placeScriptsInit.Args[1] += fmt.Sprintf(`tmpfile="%s" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << '%s' +%s +%s +`, tmpFile, heredoc, s.Script, heredoc) + + // Set the command to execute the correct script in the mounted volume. + steps[i].Command = []string{tmpFile} + steps[i].VolumeMounts = append(steps[i].VolumeMounts, ScriptsVolumeMount) + containers = append(containers, steps[i].Container) + } + + if placeScripts { + return &placeScriptsInit, containers + } + return nil, containers +} diff --git a/pkg/pod/script_test.go b/pkg/pod/script_test.go new file mode 100644 index 00000000000..625f0b7595d --- /dev/null +++ b/pkg/pod/script_test.go @@ -0,0 +1,90 @@ +package pod + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/test/names" + corev1 "k8s.io/api/core/v1" +) + +func TestConvertScripts_NothingToConvert(t *testing.T) { + gotInit, got := ConvertScripts(shellImage, []v1alpha1.Step{{Container: corev1.Container{ + Image: "step-1", + }}, {Container: corev1.Container{ + Image: "step-2", + }}}) + want := []corev1.Container{{ + Image: "step-1", + }, { + Image: "step-2", + }} + if d := cmp.Diff(want, got); d != "" { + t.Errorf("Diff (-want, +got): %s", d) + } + if gotInit != nil { + t.Errorf("Wanted nil init container, got %v", gotInit) + } +} + +func TestConvertScripts(t *testing.T) { + names.TestingSeed() + + preExistingVolumeMounts := []corev1.VolumeMount{{ + Name: "pre-existing-volume-mount", + MountPath: "/mount/path", + }, { + Name: "another-one", + MountPath: "/another/one", + }} + + gotInit, got := ConvertScripts(shellImage, []v1alpha1.Step{{ + Script: "script-1", + Container: corev1.Container{Image: "step-1"}, + }, { + // No script to convert here. + Container: corev1.Container{Image: "step-2"}, + }, { + Script: "script-3", + Container: corev1.Container{ + Image: "step-3", + VolumeMounts: preExistingVolumeMounts, + }, + }}) + wantInit := &corev1.Container{ + Name: "place-scripts-9l9zj", + Image: shellImage, + TTY: true, + Command: []string{"sh"}, + Args: []string{"-c", `tmpfile="/builder/scripts/script-0-mz4c7" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << 'script-heredoc-randomly-generated-mssqb' +script-1 +script-heredoc-randomly-generated-mssqb +tmpfile="/builder/scripts/script-2-78c5n" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << 'script-heredoc-randomly-generated-6nl7g' +script-3 +script-heredoc-randomly-generated-6nl7g +`}, + VolumeMounts: []corev1.VolumeMount{ScriptsVolumeMount}, + } + want := []corev1.Container{{ + Image: "step-1", + Command: []string{"/builder/scripts/script-0-mz4c7"}, + VolumeMounts: []corev1.VolumeMount{ScriptsVolumeMount}, + }, { + Image: "step-2", + }, { + Image: "step-3", + Command: []string{"/builder/scripts/script-2-78c5n"}, + VolumeMounts: append(preExistingVolumeMounts, ScriptsVolumeMount), + }} + if d := cmp.Diff(wantInit, gotInit); d != "" { + t.Errorf("Init Container Diff (-want, +got): %s", d) + } + if d := cmp.Diff(want, got); d != "" { + t.Errorf("Containers Diff (-want, +got): %s", d) + } +} diff --git a/pkg/pod/workingdir_init.go b/pkg/pod/workingdir_init.go index 2c0d5a73f5a..fd1a4bdd0e4 100644 --- a/pkg/pod/workingdir_init.go +++ b/pkg/pod/workingdir_init.go @@ -38,7 +38,7 @@ const ( // If no such directories need to be created (i.e., no relative workingDirs // are specified), this method returns nil, as no init container is necessary. // -// TODO(jasonhall): This should take []corev1.Container instead of +// TODO(#1605): This should take []corev1.Container instead of // []corev1.Step, but this makes it easier to use in pod.go. When pod.go is // cleaned up, this can take []corev1.Container. func WorkingDirInit(shellImage string, steps []v1alpha1.Step, volumeMounts []corev1.VolumeMount) *corev1.Container { diff --git a/pkg/pod/workingdir_init_test.go b/pkg/pod/workingdir_init_test.go index ff3176bf805..49932da578c 100644 --- a/pkg/pod/workingdir_init_test.go +++ b/pkg/pod/workingdir_init_test.go @@ -74,7 +74,7 @@ func TestWorkingDirInit(t *testing.T) { }, }} { t.Run(c.desc, func(t *testing.T) { - // TODO(jasonhall): WorkingDirInit should take + // TODO(#1605): WorkingDirInit should take // Containers instead of Steps, but while we're // cleaning up pod.go it's easier to have it take // Steps. This test doesn't care, so let's hide this diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 374936192c2..983571e420a 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -45,7 +45,7 @@ import ( var ( ignoreLastTransitionTime = cmpopts.IgnoreTypes(apis.Condition{}.LastTransitionTime.Inner.Time) images = pipeline.Images{ - EntryPointImage: "override-with-entrypoint:latest", + EntrypointImage: "override-with-entrypoint:latest", NopImage: "tianon/true", GitImage: "override-with-git:latest", CredsImage: "override-with-creds:latest", diff --git a/pkg/reconciler/taskrun/controller.go b/pkg/reconciler/taskrun/controller.go index 95af6a3f718..b4594732d94 100644 --- a/pkg/reconciler/taskrun/controller.go +++ b/pkg/reconciler/taskrun/controller.go @@ -27,8 +27,8 @@ import ( resourceinformer "github.com/tektoncd/pipeline/pkg/client/injection/informers/pipeline/v1alpha1/pipelineresource" taskinformer "github.com/tektoncd/pipeline/pkg/client/injection/informers/pipeline/v1alpha1/task" taskruninformer "github.com/tektoncd/pipeline/pkg/client/injection/informers/pipeline/v1alpha1/taskrun" + "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/reconciler" - "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/entrypoint" cloudeventclient "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources/cloudevent" "k8s.io/client-go/tools/cache" kubeclient "knative.dev/pkg/client/injection/kube/client" @@ -67,6 +67,11 @@ func NewController(images pipeline.Images) func(context.Context, configmap.Watch Logger: logger, } + entrypointCache, err := pod.NewEntrypointCache(kubeclientset) + if err != nil { + logger.Fatalf("Error creating entrypoint cache: %v", err) + } + c := &Reconciler{ Base: reconciler.NewBase(opt, taskRunAgentName, images), taskRunLister: taskRunInformer.Lister(), @@ -76,6 +81,7 @@ func NewController(images pipeline.Images) func(context.Context, configmap.Watch timeoutHandler: timeoutHandler, cloudEventClient: cloudeventclient.Get(ctx), metrics: metrics, + entrypointCache: entrypointCache, } impl := controller.NewImpl(c, c.Logger, taskRunControllerName) @@ -95,14 +101,6 @@ func NewController(images pipeline.Images) func(context.Context, configmap.Watch Handler: controller.HandleAll(impl.EnqueueControllerOf), }) - // FIXME(vdemeester) it was never set - //entrypoint cache will be initialized by controller if not provided - c.Logger.Info("Setting up Entrypoint cache") - c.cache = nil - if c.cache == nil { - c.cache, _ = entrypoint.NewCache() - } - return impl } } diff --git a/pkg/reconciler/taskrun/entrypoint/entrypoint.go b/pkg/reconciler/taskrun/entrypoint/entrypoint.go deleted file mode 100644 index afe21890e0d..00000000000 --- a/pkg/reconciler/taskrun/entrypoint/entrypoint.go +++ /dev/null @@ -1,251 +0,0 @@ -/* -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 entrypoint - -import ( - "fmt" - "strconv" - - "github.com/google/go-containerregistry/pkg/authn" - "github.com/google/go-containerregistry/pkg/authn/k8schain" - "github.com/google/go-containerregistry/pkg/name" - v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1/remote" - lru "github.com/hashicorp/golang-lru" - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" - "go.uber.org/zap" - "golang.org/x/xerrors" - corev1 "k8s.io/api/core/v1" - "k8s.io/client-go/kubernetes" -) - -const ( - // MountName is the name of the pvc being mounted (which - // will contain the entrypoint binary and eventually the logs) - MountName = "tools" - mountPoint = "/builder/tools" - DownwardMountName = "downward" - DownwardMountPoint = "/builder/downward" - DownwardMountReadyFile = "ready" - binaryLocation = mountPoint + "/entrypoint" - InitContainerName = "place-tools" - cacheSize = 1024 -) - -var ( - toolsMount = corev1.VolumeMount{ - Name: MountName, - MountPath: mountPoint, - } - downwardMount = corev1.VolumeMount{ - Name: DownwardMountName, - MountPath: DownwardMountPoint, - } -) - -// Cache is a simple caching mechanism allowing for caching the results of -// getting the Entrypoint of a container image from a remote registry. The -// internal lru cache is thread-safe. -type Cache struct { - lru *lru.Cache -} - -// NewCache is a simple helper function that returns a pointer to a Cache that -// has had the internal fixed-sized lru cache initialized. -func NewCache() (*Cache, error) { - lru, err := lru.New(cacheSize) - return &Cache{lru}, err -} - -func (c *Cache) get(sha string) ([]string, bool) { - if ep, ok := c.lru.Get(sha); ok { - return ep.([]string), true - } - return nil, false -} - -func (c *Cache) set(sha string, ep []string) { - c.lru.Add(sha, ep) -} - -// AddToEntrypointCache adds an image digest and its entrypoint -// to the cache -func AddToEntrypointCache(c *Cache, sha string, ep []string) { - c.set(sha, ep) -} - -// AddCopyStep will prepend a Step (Container) that will -// copy the entrypoint binary from the entrypoint image into the -// volume mounted at MountPoint, so that it can be mounted by -// subsequent steps and used to capture logs. -func AddCopyStep(entrypointImage string, spec *v1alpha1.TaskSpec) { - cp := corev1.Container{ - Name: InitContainerName, - Image: entrypointImage, - Command: []string{"cp", "/ko-app/entrypoint", binaryLocation}, - VolumeMounts: []corev1.VolumeMount{toolsMount}, - } - spec.Steps = append([]v1alpha1.Step{{Container: cp}}, spec.Steps...) -} - -// RedirectSteps will modify each of the steps/containers such that -// the binary being run is no longer the one specified by the Command -// and the Args, but is instead the entrypoint binary, which will -// itself invoke the Command and Args, but also capture logs. -func RedirectSteps(cache *Cache, steps []v1alpha1.Step, kubeclient kubernetes.Interface, taskRun *v1alpha1.TaskRun, logger *zap.SugaredLogger) error { - for i := range steps { - step := &steps[i] - if err := RedirectStep(cache, i, step, kubeclient, taskRun, logger); err != nil { - return err - } - } - - return nil -} - -// RedirectStep will modify a step/container such that -// the binary being run is no longer the one specified by the Command -// and the Args, but is instead the entrypoint binary, which will -// itself invoke the Command and Args, but also capture logs. -func RedirectStep(cache *Cache, stepNum int, step *v1alpha1.Step, kubeclient kubernetes.Interface, taskRun *v1alpha1.TaskRun, logger *zap.SugaredLogger) error { - if len(step.Command) == 0 { - logger.Infof("Getting Cmd from remote entrypoint for step: %s", step.Name) - var err error - step.Command, err = GetRemoteEntrypoint(cache, step.Image, kubeclient, taskRun) - if err != nil { - logger.Errorf("Error getting entry point image", err.Error()) - return err - } - } - - step.Args = GetArgs(stepNum, step.Command, step.Args) - step.Command = []string{binaryLocation} - step.VolumeMounts = append(step.VolumeMounts, toolsMount) - // The first step in a Task waits for the existence of a file projected into the Pod - // from the Downward API. That file will be populated only when all sidecars are ready, - // thereby ensuring that steps don't start executing while helper sidecars are still pending. - if stepNum == 0 { - step.VolumeMounts = append(step.VolumeMounts, downwardMount) - } - return nil -} - -// GetArgs returns the arguments that should be specified for the step which has been wrapped -// such that it will execute our custom entrypoint instead of the user provided Command and Args. -func GetArgs(stepNum int, commands, args []string) []string { - waitFile := getWaitFile(stepNum) - // The binary we want to run must be separated from its arguments by -- - // so if commands has more than one value, we'll move the other values - // into the arg list so we can separate them - if len(commands) > 1 { - args = append(commands[1:], args...) - commands = commands[:1] - } - argsForEntrypoint := []string{ - "-wait_file", waitFile, - "-post_file", getWaitFile(stepNum + 1), - } - if stepNum == 0 { - argsForEntrypoint = append(argsForEntrypoint, "-wait_file_content") - } - argsForEntrypoint = append(argsForEntrypoint, "-entrypoint") - argsForEntrypoint = append(argsForEntrypoint, commands...) - // TODO: what if Command has multiple elements, do we need "--" between command and args? - argsForEntrypoint = append(argsForEntrypoint, "--") - return append(argsForEntrypoint, args...) -} - -func getWaitFile(stepNum int) string { - if stepNum == 0 { - return fmt.Sprintf("%s/%s", DownwardMountPoint, DownwardMountReadyFile) - } - - return fmt.Sprintf("%s/%s", mountPoint, strconv.Itoa(stepNum-1)) -} - -// GetRemoteEntrypoint accepts a cache of digest lookups, as well as the digest -// to look for. If the cache does not contain the digest, it will lookup the -// metadata from the images registry, and then commit that to the cache -func GetRemoteEntrypoint(cache *Cache, image string, kubeclient kubernetes.Interface, taskRun *v1alpha1.TaskRun) ([]string, error) { - ref, err := name.ParseReference(image, name.WeakValidation) - if err != nil { - return nil, xerrors.Errorf("Failed to parse image %s: %w", image, err) - } - - var digest string - // If the image is specified as a digest, we can just take the digest from the name and use that in our cache. - // Otherwise we first have to resolve the tag to a digest. - if d, ok := ref.(name.Digest); ok { - digest = d.String() - } else { - img, err := getRemoteImage(image, kubeclient, taskRun) - if err != nil { - return nil, xerrors.Errorf("Failed to fetch remote image %s: %w", digest, err) - } - d, err := img.Digest() - if err != nil { - return nil, xerrors.Errorf("Failed to get digest for image %s: %w", image, err) - } - digest = d.String() - } - - if ep, ok := cache.get(digest); ok { - return ep, nil - } - - img, err := getRemoteImage(image, kubeclient, taskRun) - if err != nil { - return nil, xerrors.Errorf("Failed to fetch remote image %s: %w", digest, err) - } - cfg, err := img.ConfigFile() - if err != nil { - return nil, xerrors.Errorf("Failed to get config for image %s: %w", digest, err) - } - command := cfg.Config.Entrypoint - if len(command) == 0 { - command = cfg.Config.Cmd - } - cache.set(digest, command) - return command, nil -} - -func getRemoteImage(image string, kubeclient kubernetes.Interface, taskRun *v1alpha1.TaskRun) (v1.Image, error) { - // verify the image name, then download the remote config file - ref, err := name.ParseReference(image, name.WeakValidation) - if err != nil { - return nil, xerrors.Errorf("Failed to parse image %s: %w", image, err) - } - - kc, err := k8schain.New(kubeclient, k8schain.Options{ - Namespace: taskRun.Namespace, - ServiceAccountName: taskRun.GetServiceAccountName(), - }) - if err != nil { - return nil, xerrors.Errorf("Failed to create k8schain: %w", err) - } - - // this will first try to authenticate using the k8schain, - // then fall back to the google keychain, - // then fall back to anonymous - mkc := authn.NewMultiKeychain(kc) - img, err := remote.Image(ref, remote.WithAuthFromKeychain(mkc)) - if err != nil { - return nil, xerrors.Errorf("Failed to get container image info from registry %s: %w", image, err) - } - - return img, nil -} diff --git a/pkg/reconciler/taskrun/entrypoint/entrypoint_test.go b/pkg/reconciler/taskrun/entrypoint/entrypoint_test.go deleted file mode 100644 index 9f122d2b48f..00000000000 --- a/pkg/reconciler/taskrun/entrypoint/entrypoint_test.go +++ /dev/null @@ -1,483 +0,0 @@ -/* -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 entrypoint - -import ( - "fmt" - "net/http" - "net/http/httptest" - "path" - "reflect" - "strings" - "testing" - - "github.com/google/go-cmp/cmp" - v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1/partial" - "github.com/google/go-containerregistry/pkg/v1/types" - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" - "go.uber.org/zap" - "go.uber.org/zap/zaptest/observer" - "golang.org/x/xerrors" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - fakekubeclientset "k8s.io/client-go/kubernetes/fake" -) - -const ( - exceedCacheSize = 10 -) - -func TestRewriteSteps(t *testing.T) { - inputs := []v1alpha1.Step{{Container: corev1.Container{ - Image: "image", - Command: []string{"abcd"}, - }}, {Container: corev1.Container{ - Image: "my.registry.svc/image:tag", - Command: []string{"abcd"}, - Args: []string{"efgh"}, - }}} - taskRun := &v1alpha1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "foo", - Name: "taskRun", - }, - Spec: v1alpha1.TaskRunSpec{ - TaskSpec: &v1alpha1.TaskSpec{ - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Image: "ubuntu", - Command: []string{"echo"}, - Args: []string{"hello"}, - }}}, - }, - }, - } - observer, _ := observer.New(zap.InfoLevel) - entrypointCache, _ := NewCache() - c := fakekubeclientset.NewSimpleClientset() - err := RedirectSteps(entrypointCache, inputs, c, taskRun, zap.New(observer).Sugar()) - if err != nil { - t.Errorf("failed to get resources: %v", err) - } - for _, input := range inputs { - if len(input.Command) == 0 || input.Command[0] != binaryLocation { - t.Errorf("command incorrectly set: %q", input.Command) - } - found := false - for _, vm := range input.VolumeMounts { - if vm.Name == MountName { - found = true - break - } - } - if !found { - t.Error("could not find tools volume mount") - } - } -} - -func TestGetArgs(t *testing.T) { - // first step - // multiple commands - // no args - for _, c := range []struct { - desc string - stepNum int - commands []string - args []string - expectedArgs []string - }{{ - desc: "Args for first step", - stepNum: 0, - commands: []string{"echo"}, - args: []string{"hello", "world"}, - expectedArgs: []string{ - "-wait_file", "/builder/downward/ready", - "-post_file", "/builder/tools/0", - "-wait_file_content", - "-entrypoint", "echo", - "--", - "hello", "world", - }, - }, { - desc: "Multiple commands", - stepNum: 4, - commands: []string{"echo", "hello"}, - args: []string{"world"}, - expectedArgs: []string{ - "-wait_file", "/builder/tools/3", - "-post_file", "/builder/tools/4", - "-entrypoint", "echo", - "--", - "hello", "world", - }, - }, { - desc: "No args", - stepNum: 4, - commands: []string{"ls"}, - args: []string{}, - expectedArgs: []string{ - "-wait_file", "/builder/tools/3", - "-post_file", "/builder/tools/4", - "-entrypoint", "ls", - "--", - }, - }} { - t.Run(c.desc, func(t *testing.T) { - a := GetArgs(c.stepNum, c.commands, c.args) - if d := cmp.Diff(a, c.expectedArgs); d != "" { - t.Errorf("Didn't get expected arguments, difference: %s", d) - } - }) - } - -} - -type image struct { - config *v1.ConfigFile -} - -// RawConfigFile implements partial.UncompressedImageCore -func (i *image) RawConfigFile() ([]byte, error) { - return partial.RawConfigFile(i) -} - -// ConfigFile implements v1.Image -func (i *image) ConfigFile() (*v1.ConfigFile, error) { - return i.config, nil -} - -// MediaType implements partial.UncompressedImageCore -func (i *image) MediaType() (types.MediaType, error) { - return types.DockerManifestSchema2, nil -} - -// LayerByDiffID implements partial.UncompressedImageCore -func (i *image) LayerByDiffID(diffID v1.Hash) (partial.UncompressedLayer, error) { - return nil, xerrors.Errorf("unknown diff_id: %v", diffID) -} - -func mustRawManifest(t *testing.T, img v1.Image) []byte { - m, err := img.RawManifest() - if err != nil { - t.Fatalf("RawManifest() = %v", err) - } - return m -} - -func mustRawConfigFile(t *testing.T, img v1.Image) []byte { - c, err := img.RawConfigFile() - if err != nil { - t.Fatalf("RawConfigFile() = %v", err) - } - return c -} - -func getImage(t *testing.T, cfg *v1.ConfigFile) v1.Image { - rnd, err := partial.UncompressedToImage(&image{ - config: cfg, - }) - if err != nil { - t.Fatalf("getImage() = %v", err) - } - return rnd -} - -func mustConfigName(t *testing.T, img v1.Image) v1.Hash { - h, err := img.ConfigName() - if err != nil { - t.Fatalf("ConfigName() = %v", err) - } - return h -} - -func getDigestAsString(image v1.Image) string { - digestHash, _ := image.Digest() - return digestHash.String() -} - -func getServer(t *testing.T, img v1.Image) *httptest.Server { - expectedRepo := "image" - - configPath := fmt.Sprintf("/v2/%s/blobs/%s", expectedRepo, mustConfigName(t, img)) - manifestPath := fmt.Sprintf("/v2/%s/manifests/%s", expectedRepo, getDigestAsString(img)) - latestPath := fmt.Sprintf("/v2/%s/manifests/latest", expectedRepo) - - return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/v2/": - w.WriteHeader(http.StatusOK) - case configPath: - if r.Method != http.MethodGet { - t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet) - } - if _, err := w.Write(mustRawConfigFile(t, img)); err != nil { - t.Fatal(err) - } - case manifestPath, latestPath: - if r.Method != http.MethodGet { - t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet) - } - if _, err := w.Write(mustRawManifest(t, img)); err != nil { - t.Fatal(err) - } - default: - t.Fatalf("Unexpected path: %v", r.URL.Path) - } - })) -} - -func TestGetRemoteEntrypoint(t *testing.T) { - expectedEntrypoint := []string{"/bin/expected", "entrypoint"} - img := getImage(t, &v1.ConfigFile{ - Config: v1.Config{ - Entrypoint: expectedEntrypoint, - }, - }) - expectedRepo := "image" - digetsSha := getDigestAsString(img) - - server := getServer(t, img) - defer server.Close() - image := path.Join(strings.TrimPrefix(server.URL, "http://"), expectedRepo) - finalDigest := image + "@" + digetsSha - - entrypointCache, err := NewCache() - if err != nil { - t.Fatalf("couldn't create new entrypoint cache: %v", err) - } - taskRun := &v1alpha1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "foo", - Name: "taskRun", - }, - Spec: v1alpha1.TaskRunSpec{ - ServiceAccountName: "default", - TaskSpec: &v1alpha1.TaskSpec{ - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Image: "ubuntu", - Command: []string{"echo"}, - Args: []string{"hello"}, - }}}, - }, - }, - } - c := fakekubeclientset.NewSimpleClientset(&corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "default", - Namespace: "foo", - }, - }) - ep, err := GetRemoteEntrypoint(entrypointCache, finalDigest, c, taskRun) - if err != nil { - t.Errorf("couldn't get entrypoint remote: %v", err) - } - if !reflect.DeepEqual(ep, expectedEntrypoint) { - t.Errorf("entrypoints do not match: %s should be %s", ep[0], expectedEntrypoint) - } -} - -func TestGetRemoteEntrypointStale(t *testing.T) { - initialEntrypoint := []string{"/bin/expected", "entrypoint"} - img := getImage(t, &v1.ConfigFile{ - Config: v1.Config{ - Entrypoint: initialEntrypoint, - }, - }) - - server := getServer(t, img) - defer server.Close() - expectedRepo := "image" - image := path.Join(strings.TrimPrefix(server.URL, "http://"), expectedRepo) + ":latest" - - entrypointCache, err := NewCache() - if err != nil { - t.Fatalf("couldn't create new entrypoint cache: %v", err) - } - taskRun := &v1alpha1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "foo", - Name: "taskRun", - }, - Spec: v1alpha1.TaskRunSpec{ - ServiceAccountName: "default", - }, - } - c := fakekubeclientset.NewSimpleClientset(&corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "default", - Namespace: "foo", - }, - }) - ep1, err := GetRemoteEntrypoint(entrypointCache, image, c, taskRun) - if err != nil { - t.Errorf("couldn't get entrypoint remote: %v", err) - } - server.Close() - - // Now change the image - secondEntrypoint := []string{"/bin/expected", "entrypoint2"} - img = getImage(t, &v1.ConfigFile{ - Config: v1.Config{ - Entrypoint: secondEntrypoint, - }, - }) - server2 := getServer(t, img) - image = path.Join(strings.TrimPrefix(server2.URL, "http://"), expectedRepo) + ":latest" - defer server2.Close() - ep2, err := GetRemoteEntrypoint(entrypointCache, image, c, taskRun) - if err != nil { - t.Fatalf("couldn't get entrypoint remote: %v", err) - } - - if !reflect.DeepEqual(ep1, initialEntrypoint) { - t.Errorf("entrypoints do not match: %s should be %s", ep1, initialEntrypoint) - } - - if !reflect.DeepEqual(ep2, secondEntrypoint) { - t.Errorf("entrypoints do not match: %s should be %s", ep2, secondEntrypoint) - } -} - -func TestGetRemoteEntrypointWithNonDefaultSA(t *testing.T) { - expectedEntrypoint := []string{"/bin/expected", "entrypoint"} - img := getImage(t, &v1.ConfigFile{ - Config: v1.Config{ - Entrypoint: expectedEntrypoint, - }, - }) - expectedRepo := "image" - digetsSha := getDigestAsString(img) - configPath := fmt.Sprintf("/v2/%s/blobs/%s", expectedRepo, mustConfigName(t, img)) - manifestPath := fmt.Sprintf("/v2/%s/manifests/%s", expectedRepo, digetsSha) - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/v2/": - w.WriteHeader(http.StatusOK) - case configPath: - if r.Method != http.MethodGet { - t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet) - } - if _, err := w.Write(mustRawConfigFile(t, img)); err != nil { - t.Fatal(err) - } - case manifestPath: - if r.Method != http.MethodGet { - t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet) - } - if _, err := w.Write(mustRawManifest(t, img)); err != nil { - t.Fatal(err) - } - default: - t.Fatalf("Unexpected path: %v", r.URL.Path) - } - })) - defer server.Close() - image := path.Join(strings.TrimPrefix(server.URL, "http://"), expectedRepo) - finalDigest := image + "@" + digetsSha - - c := fakekubeclientset.NewSimpleClientset(&corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-other-sa", - Namespace: "foo", - }, - }) - - for _, tt := range []func(taskRun *v1alpha1.TaskRun) *v1alpha1.TaskRun{ - func(tr *v1alpha1.TaskRun) *v1alpha1.TaskRun { return tr }, - func(tr *v1alpha1.TaskRun) *v1alpha1.TaskRun { - tr.Spec.ServiceAccountName = "" - tr.Spec.DeprecatedServiceAccount = "some-other-sa" - return tr - }, - } { - taskRun := &v1alpha1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "foo", - Name: "taskRun", - }, - Spec: v1alpha1.TaskRunSpec{ - ServiceAccountName: "some-other-sa", - TaskSpec: &v1alpha1.TaskSpec{ - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Image: "ubuntu", - Command: []string{"echo"}, - Args: []string{"hello"}, - }}}, - }, - }, - } - - entrypointCache, err := NewCache() - if err != nil { - t.Fatalf("couldn't create new entrypoint cache: %v", err) - } - - ep, err := GetRemoteEntrypoint(entrypointCache, finalDigest, c, tt(taskRun)) - if err != nil { - t.Errorf("couldn't get entrypoint remote: %v", err) - } - if !reflect.DeepEqual(ep, expectedEntrypoint) { - t.Errorf("entrypoints do not match: %s should be %s", ep[0], expectedEntrypoint) - } - } -} - -func TestEntrypointCacheLRU(t *testing.T) { - entrypoint := []string{"/bin/expected", "entrypoint"} - entrypointCache, err := NewCache() - if err != nil { - t.Fatalf("couldn't create new entrypoint cache: %v", err) - } - - for i := 0; i < cacheSize+exceedCacheSize; i++ { - image := fmt.Sprintf("image%d:latest", i) - entrypointCache.set(image, entrypoint) - } - for i := 0; i < exceedCacheSize; i++ { - image := fmt.Sprintf("image%d:latest", i) - if _, ok := entrypointCache.get(image); ok { - t.Errorf("entrypoint of image %s should be expired", image) - } - } - for i := exceedCacheSize; i < cacheSize+exceedCacheSize; i++ { - image := fmt.Sprintf("image%d:latest", i) - if _, ok := entrypointCache.get(image); !ok { - t.Errorf("entrypoint of image %s shouldn't be expired", image) - } - } -} - -func TestAddCopyStep(t *testing.T) { - ts := &v1alpha1.TaskSpec{ - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "test", - }}, {Container: corev1.Container{ - Name: "test", - }}}, - } - - expectedSteps := len(ts.Steps) + 1 - AddCopyStep("override-with-entrypoint:latest", ts) - if len(ts.Steps) != 3 { - t.Errorf("BuildSpec has the wrong step count: %d should be %d", len(ts.Steps), expectedSteps) - } - if ts.Steps[0].Name != InitContainerName { - t.Errorf("entrypoint is incorrect: %s should be %s", ts.Steps[0].Name, InitContainerName) - } -} diff --git a/pkg/reconciler/taskrun/resources/apply_test.go b/pkg/reconciler/taskrun/resources/apply_test.go index f89db2428af..aceab4e0e6e 100644 --- a/pkg/reconciler/taskrun/resources/apply_test.go +++ b/pkg/reconciler/taskrun/resources/apply_test.go @@ -30,7 +30,7 @@ import ( var ( images = pipeline.Images{ - EntryPointImage: "override-with-entrypoint:latest", + EntrypointImage: "override-with-entrypoint:latest", NopImage: "tianon/true", GitImage: "override-with-git:latest", CredsImage: "override-with-creds:latest", diff --git a/pkg/reconciler/taskrun/resources/input_resource_test.go b/pkg/reconciler/taskrun/resources/input_resource_test.go index 2e83b2e6698..b075179bad0 100644 --- a/pkg/reconciler/taskrun/resources/input_resource_test.go +++ b/pkg/reconciler/taskrun/resources/input_resource_test.go @@ -33,7 +33,7 @@ import ( var ( images = pipeline.Images{ - EntryPointImage: "override-with-entrypoint:latest", + EntrypointImage: "override-with-entrypoint:latest", NopImage: "tianon/true", GitImage: "override-with-git:latest", CredsImage: "override-with-creds:latest", diff --git a/pkg/reconciler/taskrun/resources/pod.go b/pkg/reconciler/taskrun/resources/pod.go index ea91c07b40a..210e4fd4419 100644 --- a/pkg/reconciler/taskrun/resources/pod.go +++ b/pkg/reconciler/taskrun/resources/pod.go @@ -25,13 +25,11 @@ import ( "io" "io/ioutil" "path/filepath" - "strings" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/names" "github.com/tektoncd/pipeline/pkg/pod" - "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/entrypoint" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,8 +44,6 @@ const ( taskRunLabelKey = pipeline.GroupName + pipeline.TaskRunLabelKey ManagedByLabelKey = "app.kubernetes.io/managed-by" ManagedByLabelValue = "tekton-pipelines" - - scriptsDir = "/builder/scripts" ) // These are effectively const, but Go doesn't have such an annotation. @@ -85,34 +81,18 @@ var ( // Random byte reader used for pod name generation. // var for testing. randReader = rand.Reader - - // Volume definition attached to Pods generated from TaskRuns that have - // steps that specify a Script. - scriptsVolume = corev1.Volume{ - Name: "place-scripts", - VolumeSource: emptyVolumeSource, - } - scriptsVolumeMount = corev1.VolumeMount{ - Name: "place-scripts", - MountPath: scriptsDir, - } -) - -const ( - // Prefixes to add to the name of the init containers. - containerPrefix = "step-" - unnamedInitContainerPrefix = "step-unnamed-" - ReadyAnnotation = "tekton.dev/ready" - readyAnnotationValue = "READY" - sidecarPrefix = "sidecar-" ) // MakePod converts TaskRun and TaskSpec objects to a Pod which implements the taskrun specified // by the supplied CRD. -func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient kubernetes.Interface) (*corev1.Pod, error) { +func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient kubernetes.Interface, entrypointCache pod.EntrypointCache) (*corev1.Pod, error) { var initContainers []corev1.Container var volumes []corev1.Volume + // Add our implicit volumes first, so they can be overridden by the user if they prefer. + volumes = append(volumes, implicitVolumes...) + + // Inititalize any credentials found in annotated Secrets. if credsInitContainer, secretsVolumes, err := pod.CredsInit(images.CredsImage, taskRun.GetServiceAccountName(), taskRun.Namespace, kubeclient, implicitVolumeMounts, implicitEnvVars); err != nil { return nil, err } else if credsInitContainer != nil { @@ -120,120 +100,91 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha volumes = append(volumes, secretsVolumes...) } + // Initialize any workingDirs under /workspace. if workingDirInit := pod.WorkingDirInit(images.ShellImage, taskSpec.Steps, implicitVolumeMounts); workingDirInit != nil { initContainers = append(initContainers, *workingDirInit) } - var podSteps []v1alpha1.Step + // Merge step template with steps. + steps, err := v1alpha1.MergeStepsWithStepTemplate(taskSpec.StepTemplate, taskSpec.Steps) + if err != nil { + return nil, err + } - maxIndicesByResource := findMaxResourceRequest(taskSpec.Steps, corev1.ResourceCPU, corev1.ResourceMemory, corev1.ResourceEphemeralStorage) + // Convert any steps with Script to command+args. + // If any are found, append an init container to initialize scripts. + scriptsInit, stepContainers := pod.ConvertScripts(images.ShellImage, steps) + if scriptsInit != nil { + initContainers = append(initContainers, *scriptsInit) + volumes = append(volumes, pod.ScriptsVolume) + } + + // Resolve entrypoint for any steps that don't specify command. + stepContainers, err = pod.ResolveEntrypoints(entrypointCache, taskRun.Namespace, taskRun.Spec.ServiceAccountName, stepContainers) + if err != nil { + return nil, err + } + + // Rewrite steps with entrypoint binary. Append the entrypoint init + // container to place the entrypoint binary. + entrypointInit, stepContainers, err := pod.OrderContainers(images.EntrypointImage, stepContainers) + if err != nil { + return nil, err + } + initContainers = append(initContainers, entrypointInit) + volumes = append(volumes, pod.ToolsVolume, pod.DownwardVolume) - placeScripts := false - placeScriptsInitContainer := corev1.Container{ - Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("place-scripts"), - Image: images.ShellImage, - TTY: true, - Command: []string{"sh"}, - Args: []string{"-c", ""}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount}, + // Zero out non-max resource requests. + // TODO(jasonhall): Split this out so it's more easily testable. + maxIndicesByResource := findMaxResourceRequest(taskSpec.Steps, corev1.ResourceCPU, corev1.ResourceMemory, corev1.ResourceEphemeralStorage) + for i := range stepContainers { + zeroNonMaxResourceRequests(&stepContainers[i], i, maxIndicesByResource) } - for i, s := range taskSpec.Steps { - s.Env = append(implicitEnvVars, s.Env...) - // TODO(mattmoor): Check that volumeMounts match volumes. + // Add implicit env vars. + // They're prepended to the list, so that if the user specified any + // themselves their value takes precedence. + for i, s := range stepContainers { + env := append(implicitEnvVars, s.Env...) + stepContainers[i].Env = env + } - // Add implicit volume mounts, unless the user has requested - // their own volume mount at that path. + // Add implicit volume mounts to each step, unless the step specifies + // its own volume mount at that path. + for i, s := range stepContainers { requestedVolumeMounts := map[string]bool{} for _, vm := range s.VolumeMounts { requestedVolumeMounts[filepath.Clean(vm.MountPath)] = true } + var toAdd []corev1.VolumeMount for _, imp := range implicitVolumeMounts { if !requestedVolumeMounts[filepath.Clean(imp.MountPath)] { - s.VolumeMounts = append(s.VolumeMounts, imp) - } - } - - // If the step specifies a Script, generate and invoke an - // executable script file containing each item in the script. - if s.Script != "" { - placeScripts = true - // Append to the place-scripts script to place the - // script file in a known location in the scripts volume. - tmpFile := filepath.Join(scriptsDir, names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("script-%d", i))) - // heredoc is the "here document" placeholder string - // used to cat script contents into the file. Typically - // this is the string "EOF" but if this value were - // "EOF" it would prevent users from including the - // string "EOF" in their own scripts. Instead we - // randomly generate a string to (hopefully) prevent - // collisions. - heredoc := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("script-heredoc-randomly-generated") - // NOTE: quotes around the heredoc string are - // important. Without them, ${}s in the file are - // interpreted as env vars and likely end up replaced - // with empty strings. See - // https://stackoverflow.com/a/27921346 - placeScriptsInitContainer.Args[1] += fmt.Sprintf(`tmpfile="%s" -touch ${tmpfile} && chmod +x ${tmpfile} -cat > ${tmpfile} << '%s' -%s -%s -`, tmpFile, heredoc, s.Script, heredoc) - // The entrypoint redirecter has already run on this - // step, so we just need to replace the image's - // entrypoint (if any) with the script to run. - // Validation prevents step args from being passed, but - // just to be careful we'll replace any that survived - // entrypoint redirection here. - - // TODO(jasonhall): It's confusing that entrypoint - // redirection isn't done as part of MakePod, and the - // interaction of these two modifications to container - // args might be confusing to debug in the future. - s.Args = append(s.Args, tmpFile) - for i := 0; i < len(s.Args); i++ { - if s.Args[i] == "-entrypoint" { - s.Args = append(s.Args[:i+1], tmpFile) - } + toAdd = append(toAdd, imp) } - s.VolumeMounts = append(s.VolumeMounts, scriptsVolumeMount) } + vms := append(s.VolumeMounts, toAdd...) + stepContainers[i].VolumeMounts = vms + } + // This loop: + // - defaults workingDir to /workspace + // - sets container name to add "step-" prefix or "step-unnamed-#" if not specified. + // TODO(jasonhall): Remove this loop and make each transformation in + // isolation. + for i, s := range stepContainers { if s.WorkingDir == "" { - s.WorkingDir = workspaceDir + stepContainers[i].WorkingDir = workspaceDir } if s.Name == "" { - s.Name = fmt.Sprintf("%v%d", unnamedInitContainerPrefix, i) - } else { - s.Name = names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%v%v", containerPrefix, s.Name)) - } - // use the container name to add the entrypoint binary as an - // init container. - // TODO(jasonhall): Entrypoint init container should be - // explicitly added as an init container, not added to steps - // then pulled out when translating to Pod containers. - if s.Name == names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%v%v", containerPrefix, entrypoint.InitContainerName)) { - initContainers = append(initContainers, s.Container) + stepContainers[i].Name = names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%sunnamed-%d", pod.StepPrefix, i)) } else { - zeroNonMaxResourceRequests(&s, i, maxIndicesByResource) - podSteps = append(podSteps, s) + stepContainers[i].Name = names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%s%s", pod.StepPrefix, s.Name)) } } // Add podTemplate Volumes to the explicitly declared use volumes volumes = append(volumes, taskSpec.Volumes...) volumes = append(volumes, taskRun.Spec.PodTemplate.Volumes...) - // Add our implicit volumes and any volumes needed for secrets to the explicitly - // declared user volumes. - volumes = append(volumes, implicitVolumes...) - - // Add the volume shared to place a script file, if any step specified - // a script. - if placeScripts { - volumes = append(volumes, scriptsVolume) - initContainers = append(initContainers, placeScriptsInitContainer) - } if err := v1alpha1.ValidateVolumes(volumes); err != nil { return nil, err @@ -246,16 +197,10 @@ cat > ${tmpfile} << '%s' } gibberish := hex.EncodeToString(b) - mergedPodSteps, err := v1alpha1.MergeStepsWithStepTemplate(taskSpec.StepTemplate, podSteps) - if err != nil { - return nil, err - } - var mergedPodContainers []corev1.Container - for _, s := range mergedPodSteps { - mergedPodContainers = append(mergedPodContainers, s.Container) - } + // Merge sidecar containers with step containers. + mergedPodContainers := stepContainers for _, sc := range taskSpec.Sidecars { - sc.Name = names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%v%v", sidecarPrefix, sc.Name)) + sc.Name = names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%v%v", pod.SidecarPrefix, sc.Name)) mergedPodContainers = append(mergedPodContainers, sc) } @@ -273,7 +218,7 @@ cat > ${tmpfile} << '%s' OwnerReferences: []metav1.OwnerReference{ *metav1.NewControllerRef(taskRun, groupVersionKind), }, - Annotations: makeAnnotations(taskRun), + Annotations: taskRun.Annotations, Labels: makeLabels(taskRun), }, Spec: corev1.PodSpec{ @@ -291,32 +236,6 @@ cat > ${tmpfile} << '%s' }, nil } -type UpdatePod func(*corev1.Pod) (*corev1.Pod, error) - -// AddReadyAnnotation adds the ready annotation if it is not present. -// Returns any error that comes back from the passed-in update func. -func AddReadyAnnotation(p *corev1.Pod, update UpdatePod) error { - if p.ObjectMeta.Annotations == nil { - p.ObjectMeta.Annotations = make(map[string]string) - } - if p.ObjectMeta.Annotations[ReadyAnnotation] != readyAnnotationValue { - p.ObjectMeta.Annotations[ReadyAnnotation] = readyAnnotationValue - _, err := update(p) - - return err - } - - return nil -} - -func IsContainerStep(name string) bool { - return strings.HasPrefix(name, containerPrefix) -} - -func IsContainerSidecar(name string) bool { - return strings.HasPrefix(name, sidecarPrefix) -} - // makeLabels constructs the labels we will propagate from TaskRuns to Pods. func makeLabels(s *v1alpha1.TaskRun) map[string]string { labels := make(map[string]string, len(s.ObjectMeta.Labels)+1) @@ -335,17 +254,6 @@ func makeLabels(s *v1alpha1.TaskRun) map[string]string { return labels } -// makeAnnotations constructs the annotations we will propagate from TaskRuns to Pods -// and adds any other annotations that will be needed to initialize a Pod. -func makeAnnotations(s *v1alpha1.TaskRun) map[string]string { - annotations := make(map[string]string, len(s.ObjectMeta.Annotations)+1) - for k, v := range s.ObjectMeta.Annotations { - annotations[k] = v - } - annotations[ReadyAnnotation] = "" - return annotations -} - // zeroNonMaxResourceRequests zeroes out the container's cpu, memory, or // ephemeral storage resource requests if the container does not have the // largest request out of all containers in the pod. This is done because Tekton @@ -353,13 +261,13 @@ func makeAnnotations(s *v1alpha1.TaskRun) map[string]string { // one at a time, so we want pods to only request the maximum resources needed // at any single point in time. If no container has an explicit resource // request, all requests are set to 0. -func zeroNonMaxResourceRequests(step *v1alpha1.Step, stepIndex int, maxIndicesByResource map[corev1.ResourceName]int) { - if step.Resources.Requests == nil { - step.Resources.Requests = corev1.ResourceList{} +func zeroNonMaxResourceRequests(c *corev1.Container, stepIndex int, maxIndicesByResource map[corev1.ResourceName]int) { + if c.Resources.Requests == nil { + c.Resources.Requests = corev1.ResourceList{} } for name, maxIdx := range maxIndicesByResource { if maxIdx != stepIndex { - step.Resources.Requests[name] = zeroQty + c.Resources.Requests[name] = zeroQty } } } @@ -385,14 +293,3 @@ func findMaxResourceRequest(steps []v1alpha1.Step, resourceNames ...corev1.Resou } return maxIdxs } - -// TrimContainerNamePrefix trim the container name prefix to get the corresponding step name -func TrimContainerNamePrefix(containerName string) string { - return strings.TrimPrefix(containerName, containerPrefix) -} - -// TrimSidecarNamePrefix trim the sidecar name prefix to get the corresponding -// sidecar name -func TrimSidecarNamePrefix(containerName string) string { - return strings.TrimPrefix(containerName, sidecarPrefix) -} diff --git a/pkg/reconciler/taskrun/resources/pod_test.go b/pkg/reconciler/taskrun/resources/pod_test.go index 73ae0d478f0..c437df5b0cd 100644 --- a/pkg/reconciler/taskrun/resources/pod_test.go +++ b/pkg/reconciler/taskrun/resources/pod_test.go @@ -25,8 +25,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/test/names" - "golang.org/x/xerrors" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -44,14 +44,21 @@ var ( func TestMakePod(t *testing.T) { names.TestingSeed() - secretsVolumeMounts := []corev1.VolumeMount{{ + secretsVolumeMount := corev1.VolumeMount{ Name: "secret-volume-multi-creds-9l9zj", MountPath: "/var/build-secrets/multi-creds", - }} - secretsVolumes := []corev1.Volume{{ + } + secretsVolume := corev1.Volume{ Name: "secret-volume-multi-creds-9l9zj", VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "multi-creds"}}, - }} + } + + placeToolsInit := corev1.Container{ + Name: "place-tools", + Image: images.EntrypointImage, + Command: []string{"cp", "/ko-app/entrypoint", "/builder/tools/entrypoint"}, + VolumeMounts: []corev1.VolumeMount{pod.ToolsMount}, + } runtimeClassName := "gvisor" @@ -59,30 +66,39 @@ func TestMakePod(t *testing.T) { defer func() { randReader = rand.Reader }() for _, c := range []struct { - desc string - trs v1alpha1.TaskRunSpec - ts v1alpha1.TaskSpec - annotations map[string]string - want *corev1.PodSpec - wantErr error + desc string + trs v1alpha1.TaskRunSpec + ts v1alpha1.TaskSpec + want *corev1.PodSpec + wantAnnotations map[string]string }{{ desc: "simple", ts: v1alpha1.TaskSpec{ Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "name", - Image: "image", + Name: "name", + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. }}}, }, - annotations: map[string]string{ - "simple-annotation-key": "simple-annotation-val", - }, want: &corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{placeToolsInit}, Containers: []corev1.Container{{ - Name: "step-name", - Image: "image", + Name: "step-name", + Image: "image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "cmd", + "--", + }, Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, + VolumeMounts: append([]corev1.VolumeMount{pod.ToolsMount, pod.DownwardMount}, implicitVolumeMounts...), WorkingDir: workspaceDir, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -92,14 +108,15 @@ func TestMakePod(t *testing.T) { }, }, }}, - Volumes: implicitVolumes, + Volumes: append(implicitVolumes, pod.ToolsVolume, pod.DownwardVolume), }, }, { - desc: "with-service-account", + desc: "with service account", ts: v1alpha1.TaskSpec{ Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "name", - Image: "image", + Name: "name", + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. }}}, }, trs: v1alpha1.TaskRunSpec{ @@ -118,14 +135,27 @@ func TestMakePod(t *testing.T) { "-basic-git=multi-creds=github.com", "-basic-git=multi-creds=gitlab.com", }, - VolumeMounts: append(implicitVolumeMounts, secretsVolumeMounts...), + VolumeMounts: append(implicitVolumeMounts, secretsVolumeMount), Env: implicitEnvVars, - }}, + }, + placeToolsInit, + }, Containers: []corev1.Container{{ - Name: "step-name", - Image: "image", + Name: "step-name", + Image: "image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "cmd", + "--", + }, Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, + VolumeMounts: append([]corev1.VolumeMount{pod.ToolsMount, pod.DownwardMount}, implicitVolumeMounts...), WorkingDir: workspaceDir, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -135,14 +165,15 @@ func TestMakePod(t *testing.T) { }, }, }}, - Volumes: append(secretsVolumes, implicitVolumes...), + Volumes: append(implicitVolumes, secretsVolume, pod.ToolsVolume, pod.DownwardVolume), }, }, { - desc: "with-deprecated-service-account", + desc: "with_deprecated_service_account", ts: v1alpha1.TaskSpec{ Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "name", - Image: "image", + Name: "name", + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. }}}, }, trs: v1alpha1.TaskRunSpec{ @@ -161,14 +192,27 @@ func TestMakePod(t *testing.T) { "-basic-git=multi-creds=github.com", "-basic-git=multi-creds=gitlab.com", }, - VolumeMounts: append(implicitVolumeMounts, secretsVolumeMounts...), + VolumeMounts: append(implicitVolumeMounts, secretsVolumeMount), Env: implicitEnvVars, - }}, + }, + placeToolsInit, + }, Containers: []corev1.Container{{ - Name: "step-name", - Image: "image", + Name: "step-name", + Image: "image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "cmd", + "--", + }, Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, + VolumeMounts: append([]corev1.VolumeMount{pod.ToolsMount, pod.DownwardMount}, implicitVolumeMounts...), WorkingDir: workspaceDir, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -178,14 +222,15 @@ func TestMakePod(t *testing.T) { }, }, }}, - Volumes: append(secretsVolumes, implicitVolumes...), + Volumes: append(implicitVolumes, secretsVolume, pod.ToolsVolume, pod.DownwardVolume), }, }, { - desc: "with-pod-template", + desc: "with pod template", ts: v1alpha1.TaskSpec{ Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "name", - Image: "image", + Name: "name", + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. }}}, }, trs: v1alpha1.TaskRunSpec{ @@ -199,12 +244,24 @@ func TestMakePod(t *testing.T) { }, }, want: &corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{placeToolsInit}, Containers: []corev1.Container{{ - Name: "step-name", - Image: "image", + Name: "step-name", + Image: "image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "cmd", + "--", + }, Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, + VolumeMounts: append([]corev1.VolumeMount{pod.ToolsMount, pod.DownwardMount}, implicitVolumeMounts...), WorkingDir: workspaceDir, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -214,7 +271,7 @@ func TestMakePod(t *testing.T) { }, }, }}, - Volumes: implicitVolumes, + Volumes: append(implicitVolumes, pod.ToolsVolume, pod.DownwardVolume), SecurityContext: &corev1.PodSecurityContext{ Sysctls: []corev1.Sysctl{ {Name: "net.ipv4.tcp_syncookies", Value: "1"}, @@ -223,23 +280,33 @@ func TestMakePod(t *testing.T) { RuntimeClassName: &runtimeClassName, }, }, { - desc: "very-long-step-name", + desc: "very long step name", ts: v1alpha1.TaskSpec{ Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "a-very-very-long-character-step-name-to-trigger-max-len----and-invalid-characters", - Image: "image", + Name: "a-very-very-long-character-step-name-to-trigger-max-len----and-invalid-characters", + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. }}}, }, - annotations: map[string]string{ - "simple-annotation-key": "simple-annotation-val", - }, want: &corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{placeToolsInit}, Containers: []corev1.Container{{ - Name: "step-a-very-very-long-character-step-name-to-trigger-max-len", - Image: "image", + Name: "step-a-very-very-long-character-step-name-to-trigger-max-len", // step name trimmed. + Image: "image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "cmd", + "--", + }, Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, + VolumeMounts: append([]corev1.VolumeMount{pod.ToolsMount, pod.DownwardMount}, implicitVolumeMounts...), WorkingDir: workspaceDir, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -249,26 +316,36 @@ func TestMakePod(t *testing.T) { }, }, }}, - Volumes: implicitVolumes, + Volumes: append(implicitVolumes, pod.ToolsVolume, pod.DownwardVolume), }, }, { - desc: "step-name-ends-with-non-alphanumeric", + desc: "step name ends with non alphanumeric", ts: v1alpha1.TaskSpec{ Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "ends-with-invalid-%%__$$", - Image: "image", + Name: "ends-with-invalid-%%__$$", + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. }}}, }, - annotations: map[string]string{ - "simple-annotation-key": "simple-annotation-val", - }, want: &corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{placeToolsInit}, Containers: []corev1.Container{{ - Name: "step-ends-with-invalid", - Image: "image", + Name: "step-ends-with-invalid", // invalid suffix removed. + Image: "image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "cmd", + "--", + }, Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, + VolumeMounts: append([]corev1.VolumeMount{pod.ToolsMount, pod.DownwardMount}, implicitVolumeMounts...), WorkingDir: workspaceDir, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -278,14 +355,15 @@ func TestMakePod(t *testing.T) { }, }, }}, - Volumes: implicitVolumes, + Volumes: append(implicitVolumes, pod.ToolsVolume, pod.DownwardVolume), }, }, { - desc: "working-dir-in-workspace-dir", + desc: "workingDir in workspace", ts: v1alpha1.TaskSpec{ Steps: []v1alpha1.Step{{Container: corev1.Container{ Name: "name", Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. WorkingDir: filepath.Join(workspaceDir, "test"), }}}, }, @@ -298,12 +376,25 @@ func TestMakePod(t *testing.T) { Args: []string{"-c", fmt.Sprintf("mkdir -p %s", filepath.Join(workspaceDir, "test"))}, WorkingDir: workspaceDir, VolumeMounts: implicitVolumeMounts, - }}, + }, + placeToolsInit, + }, Containers: []corev1.Container{{ - Name: "step-name", - Image: "image", + Name: "step-name", + Image: "image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "cmd", + "--", + }, Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, + VolumeMounts: append([]corev1.VolumeMount{pod.ToolsMount, pod.DownwardMount}, implicitVolumeMounts...), WorkingDir: filepath.Join(workspaceDir, "test"), Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -313,27 +404,41 @@ func TestMakePod(t *testing.T) { }, }, }}, - Volumes: implicitVolumes, + Volumes: append(implicitVolumes, pod.ToolsVolume, pod.DownwardVolume), }, }, { - desc: "additional-sidecar-container", + desc: "sidecar container", ts: v1alpha1.TaskSpec{ Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "primary-name", - Image: "primary-image", + Name: "primary-name", + Image: "primary-image", + Command: []string{"cmd"}, // avoid entrypoint lookup. }}}, Sidecars: []corev1.Container{{ Name: "sc-name", Image: "sidecar-image", }}, }, + wantAnnotations: map[string]string{}, want: &corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{placeToolsInit}, Containers: []corev1.Container{{ - Name: "step-primary-name", - Image: "primary-image", + Name: "step-primary-name", + Image: "primary-image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "cmd", + "--", + }, Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, + VolumeMounts: append([]corev1.VolumeMount{pod.ToolsMount, pod.DownwardMount}, implicitVolumeMounts...), WorkingDir: workspaceDir, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -349,17 +454,91 @@ func TestMakePod(t *testing.T) { Requests: nil, }, }}, - Volumes: implicitVolumes, + Volumes: append(implicitVolumes, pod.ToolsVolume, pod.DownwardVolume), + }, + }, { + desc: "resource request", + ts: v1alpha1.TaskSpec{ + Steps: []v1alpha1.Step{{Container: corev1.Container{ + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("8"), + corev1.ResourceMemory: resource.MustParse("10Gi"), + }, + }, + }}, {Container: corev1.Container{ + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + corev1.ResourceMemory: resource.MustParse("100Gi"), + }, + }, + }}}, + }, + want: &corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{placeToolsInit}, + Containers: []corev1.Container{{ + Name: "step-unnamed-0", + Image: "image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "cmd", + "--", + }, + Env: implicitEnvVars, + VolumeMounts: append([]corev1.VolumeMount{pod.ToolsMount, pod.DownwardMount}, implicitVolumeMounts...), + WorkingDir: workspaceDir, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("8"), + corev1.ResourceMemory: resource.MustParse("0"), + corev1.ResourceEphemeralStorage: resource.MustParse("0"), + }, + }, + }, { + Name: "step-unnamed-1", + Image: "image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/tools/0", + "-post_file", + "/builder/tools/1", + "-entrypoint", + "cmd", + "--", + }, + Env: implicitEnvVars, + VolumeMounts: append([]corev1.VolumeMount{pod.ToolsMount}, implicitVolumeMounts...), + WorkingDir: workspaceDir, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("0"), + corev1.ResourceMemory: resource.MustParse("100Gi"), + corev1.ResourceEphemeralStorage: resource.MustParse("0"), + }, + }, + }}, + Volumes: append(implicitVolumes, pod.ToolsVolume, pod.DownwardVolume), }, }, { desc: "step with script", ts: v1alpha1.TaskSpec{ Steps: []v1alpha1.Step{{ Container: corev1.Container{ - Name: "one", - Image: "image", - Command: []string{"entrypointer"}, - Args: []string{"wait-file", "out-file", "-entrypoint", "image-entrypoint", "--"}, + Name: "one", + Image: "image", }, Script: "echo hello from step one", }, { @@ -367,9 +546,6 @@ func TestMakePod(t *testing.T) { Name: "two", Image: "image", VolumeMounts: []corev1.VolumeMount{{Name: "i-have-a-volume-mount"}}, - Command: []string{"entrypointer"}, - // args aren't valid, but just in case they end up here we'll replace them. - Args: []string{"wait-file", "out-file", "-entrypoint", "image-entrypoint", "--", "args", "somehow"}, }, Script: `#!/usr/bin/env python print("Hello from Python")`, @@ -394,15 +570,29 @@ cat > ${tmpfile} << 'script-heredoc-randomly-generated-6nl7g' print("Hello from Python") script-heredoc-randomly-generated-6nl7g `}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount}, + VolumeMounts: []corev1.VolumeMount{pod.ScriptsVolumeMount}, + }, { + Name: "place-tools", + Image: images.EntrypointImage, + Command: []string{"cp", "/ko-app/entrypoint", "/builder/tools/entrypoint"}, + VolumeMounts: []corev1.VolumeMount{pod.ToolsMount}, }}, Containers: []corev1.Container{{ - Name: "step-one", - Image: "image", - Command: []string{"entrypointer"}, - Args: []string{"wait-file", "out-file", "-entrypoint", "/builder/scripts/script-0-mz4c7"}, + Name: "step-one", + Image: "image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "/builder/scripts/script-0-mz4c7", + "--", + }, Env: implicitEnvVars, - VolumeMounts: append(implicitVolumeMounts, scriptsVolumeMount), + VolumeMounts: append([]corev1.VolumeMount{pod.ScriptsVolumeMount, pod.ToolsMount, pod.DownwardMount}, implicitVolumeMounts...), WorkingDir: workspaceDir, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -412,12 +602,20 @@ script-heredoc-randomly-generated-6nl7g }, }, }, { - Name: "step-two", - Image: "image", - Command: []string{"entrypointer"}, - Args: []string{"wait-file", "out-file", "-entrypoint", "/builder/scripts/script-1-78c5n"}, + Name: "step-two", + Image: "image", + Command: []string{"/builder/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/builder/tools/0", + "-post_file", + "/builder/tools/1", + "-entrypoint", + "/builder/scripts/script-1-78c5n", + "--", + }, Env: implicitEnvVars, - VolumeMounts: append([]corev1.VolumeMount{{Name: "i-have-a-volume-mount"}}, append(implicitVolumeMounts, scriptsVolumeMount)...), + VolumeMounts: append([]corev1.VolumeMount{{Name: "i-have-a-volume-mount"}, pod.ScriptsVolumeMount, pod.ToolsMount}, implicitVolumeMounts...), WorkingDir: workspaceDir, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -427,14 +625,14 @@ script-heredoc-randomly-generated-6nl7g }, }, }}, - Volumes: append(implicitVolumes, scriptsVolume), + Volumes: append(implicitVolumes, pod.ScriptsVolume, pod.ToolsVolume, pod.DownwardVolume), }, }} { t.Run(c.desc, func(t *testing.T) { names.TestingSeed() - cs := fakek8s.NewSimpleClientset( - &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "default"}}, - &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "service-account"}, + kubeclient := fakek8s.NewSimpleClientset( + &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}, + &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "service-account", Namespace: "default"}, Secrets: []corev1.ObjectReference{{ Name: "multi-creds", }}, @@ -456,13 +654,16 @@ script-heredoc-randomly-generated-6nl7g ) tr := &v1alpha1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ - Name: "taskrun-name", - Annotations: c.annotations, + Name: "taskrun-name", }, Spec: c.trs, } - got, err := MakePod(images, tr, c.ts, cs) - if err != c.wantErr { + entrypointCache, err := pod.NewEntrypointCache(kubeclient) + if err != nil { + t.Fatalf("NewEntrypointCache: %v", err) + } + got, err := MakePod(images, tr, c.ts, kubeclient, entrypointCache) + if err != nil { t.Fatalf("MakePod: %v", err) } @@ -475,16 +676,6 @@ script-heredoc-randomly-generated-6nl7g if d := cmp.Diff(c.want, &got.Spec, resourceQuantityCmp); d != "" { t.Errorf("Diff(-want, +got):\n%s", d) } - - wantAnnotations := map[string]string{ReadyAnnotation: ""} - if c.annotations != nil { - for key, val := range c.annotations { - wantAnnotations[key] = val - } - } - if d := cmp.Diff(got.Annotations, wantAnnotations); d != "" { - t.Errorf("Diff annotations:\n%s", d) - } }) } } @@ -533,65 +724,3 @@ func TestMakeLabels(t *testing.T) { }) } } - -func TestAddReadyAnnotation(t *testing.T) { - pod := &corev1.Pod{} - updateFunc := func(p *corev1.Pod) (*corev1.Pod, error) { return p, nil } - if err := AddReadyAnnotation(pod, updateFunc); err != nil { - t.Errorf("error received: %v", err) - } - if v := pod.ObjectMeta.Annotations[ReadyAnnotation]; v != readyAnnotationValue { - t.Errorf("Annotation %q=%q missing from Pod", ReadyAnnotation, readyAnnotationValue) - } -} - -func TestAddReadyAnnotationUpdateError(t *testing.T) { - testerror := xerrors.New("error updating pod") - pod := &corev1.Pod{} - updateFunc := func(p *corev1.Pod) (*corev1.Pod, error) { return p, testerror } - if err := AddReadyAnnotation(pod, updateFunc); err != testerror { - t.Errorf("expected %v received %v", testerror, err) - } -} - -func TestMakeAnnotations(t *testing.T) { - for _, c := range []struct { - desc string - taskRun *v1alpha1.TaskRun - expectedAnnotationSubset map[string]string - }{{ - desc: "a taskruns annotations are copied to the pod", - taskRun: &v1alpha1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "a-taskrun", - Annotations: map[string]string{ - "foo": "bar", - "baz": "quux", - }, - }, - }, - expectedAnnotationSubset: map[string]string{ - "foo": "bar", - "baz": "quux", - }, - }, { - desc: "initial pod annotations contain the ReadyAnnotation to pause steps until sidecars are ready", - taskRun: &v1alpha1.TaskRun{}, - expectedAnnotationSubset: map[string]string{ - ReadyAnnotation: "", - }, - }} { - t.Run(c.desc, func(t *testing.T) { - annos := makeAnnotations(c.taskRun) - for k, v := range c.expectedAnnotationSubset { - receivedValue, ok := annos[k] - if !ok { - t.Errorf("expected annotation %q was missing", k) - } - if receivedValue != v { - t.Errorf("expected annotation %q=%q, received %q=%q", k, v, k, receivedValue) - } - } - }) - } -} diff --git a/pkg/reconciler/taskrun/sidecars/stop.go b/pkg/reconciler/taskrun/sidecars/stop.go deleted file mode 100644 index d9daa6e8509..00000000000 --- a/pkg/reconciler/taskrun/sidecars/stop.go +++ /dev/null @@ -1,57 +0,0 @@ -/* -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 sidecars - -import ( - "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -type GetPod func(string, metav1.GetOptions) (*corev1.Pod, error) -type UpdatePod func(*corev1.Pod) (*corev1.Pod, error) - -// Stop stops all sidecar containers inside a pod. A container is considered -// to be a sidecar if it is currently running. This func is only expected to -// be called after a TaskRun completes and all Step containers Step containers -// have already stopped. -// -// A sidecar is killed by replacing its current container image with the nop -// image, which in turn quickly exits. If the sidecar defines a command then -// it will exit with a non-zero status. When we check for TaskRun success we -// have to check for the containers we care about - not the final Pod status. -func Stop(pod *corev1.Pod, nopImage string, updatePod UpdatePod) error { - updated := false - if pod.Status.Phase == corev1.PodRunning { - for _, s := range pod.Status.ContainerStatuses { - if resources.IsContainerSidecar(s.Name) && s.State.Running != nil { - for j, c := range pod.Spec.Containers { - if c.Name == s.Name && c.Image != nopImage { - updated = true - pod.Spec.Containers[j].Image = nopImage - } - } - } - } - } - if updated { - if _, err := updatePod(pod); err != nil { - return err - } - } - return nil -} diff --git a/pkg/reconciler/taskrun/sidecars/stop_test.go b/pkg/reconciler/taskrun/sidecars/stop_test.go deleted file mode 100644 index 3982afd4aec..00000000000 --- a/pkg/reconciler/taskrun/sidecars/stop_test.go +++ /dev/null @@ -1,122 +0,0 @@ -/* -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 sidecars - -import ( - "testing" - "time" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -var ( - nopImage = "nopImage" -) - -// TestStop exercises the Stop() method of the sidecars package. -// A sidecar is killed by having its container image changed to that of the nop image. -// This test therefore runs through a series of pod and sidecar configurations, -// checking whether the resulting image in the sidecar's container matches that expected. -func TestStop(t *testing.T) { - testcases := []struct { - description string - podPhase corev1.PodPhase - sidecarContainer corev1.Container - sidecarState corev1.ContainerState - expectedImage string - }{{ - description: "a running sidecar container should be stopped", - podPhase: corev1.PodRunning, - sidecarContainer: corev1.Container{ - Name: "sidecar-echo-hello", - Image: "echo-hello:latest", - Command: []string{"echo"}, - Args: []string{"hello"}, - }, - sidecarState: corev1.ContainerState{ - Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}, - }, - expectedImage: nopImage, - }, { - description: "a pending pod should not have its sidecars stopped", - podPhase: corev1.PodPending, - sidecarContainer: corev1.Container{ - Name: "sidecar-echo-hello", - Image: "echo-hello:latest", - Command: []string{"echo"}, - Args: []string{"hello"}, - }, - sidecarState: corev1.ContainerState{ - Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}, - }, - expectedImage: "echo-hello:latest", - }, { - description: "a sidecar container that is not in a running state should not be stopped", - podPhase: corev1.PodRunning, - sidecarContainer: corev1.Container{ - Name: "sidecar-echo-hello", - Image: "echo-hello:latest", - Command: []string{"echo"}, - Args: []string{"hello"}, - }, - sidecarState: corev1.ContainerState{ - Waiting: &corev1.ContainerStateWaiting{}, - }, - expectedImage: "echo-hello:latest", - }} - for _, tc := range testcases { - t.Run(tc.description, func(t *testing.T) { - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "test_pod", - }, - Spec: corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, - InitContainers: nil, - Containers: []corev1.Container{ - { - Name: "test-task-step", - Image: "test-task-step:latest", - Command: []string{"test"}, - }, - tc.sidecarContainer, - }, - }, - Status: corev1.PodStatus{ - Phase: tc.podPhase, - ContainerStatuses: []corev1.ContainerStatus{ - {}, - { - Name: tc.sidecarContainer.Name, - State: tc.sidecarState, - }, - }, - }, - } - updatePod := func(p *corev1.Pod) (*corev1.Pod, error) { return nil, nil } - if err := Stop(pod, nopImage, updatePod); err != nil { - t.Errorf("error stopping sidecar: %v", err) - } - sidecarIdx := len(pod.Spec.Containers) - 1 - if pod.Spec.Containers[sidecarIdx].Image != tc.expectedImage { - t.Errorf("expected sidecar image %q, actual: %q", tc.expectedImage, pod.Spec.Containers[sidecarIdx].Image) - } - }) - } -} diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index dfcc09cba56..3bb5721e2a8 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -29,11 +29,10 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" listers "github.com/tektoncd/pipeline/pkg/client/listers/pipeline/v1alpha1" + podconvert "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/reconciler" - "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/entrypoint" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources/cloudevent" - "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/sidecars" "github.com/tektoncd/pipeline/pkg/status" "go.uber.org/zap" "golang.org/x/xerrors" @@ -41,7 +40,6 @@ import ( "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" "knative.dev/pkg/apis" "knative.dev/pkg/controller" @@ -69,7 +67,7 @@ type Reconciler struct { resourceLister listers.PipelineResourceLister cloudEventClient cloudevent.CEClient tracker tracker.Interface - cache *entrypoint.Cache + entrypointCache podconvert.EntrypointCache timeoutHandler *reconciler.TimeoutSet metrics *Recorder } @@ -132,7 +130,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { c.timeoutHandler.Release(tr) pod, err := c.KubeClientSet.CoreV1().Pods(tr.Namespace).Get(tr.Status.PodName, metav1.GetOptions{}) if err == nil { - err = sidecars.Stop(pod, c.Images.NopImage, c.KubeClientSet.CoreV1().Pods(tr.Namespace).Update) + err = podconvert.StopSidecars(c.Images.NopImage, c.KubeClientSet, *pod) } else if errors.IsNotFound(err) { return merr.ErrorOrNil() } @@ -317,10 +315,15 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error cloudevent.InitializeCloudEvents(tr, prs) // Get the TaskRun's Pod if it should have one. Otherwise, create the Pod. - pod, err := tryGetPod(tr.Status, c.KubeClientSet.CoreV1().Pods(tr.Namespace).Get) - if err != nil { - c.Logger.Errorf("Error getting pod %q: %v", tr.Status.PodName, err) - return err + var pod *corev1.Pod + if tr.Status.PodName != "" { + pod, err = c.KubeClientSet.CoreV1().Pods(tr.Namespace).Get(tr.Status.PodName, metav1.GetOptions{}) + if errors.IsNotFound(err) { + // Keep going, this will result in the Pod being created below. + } else if err != nil { + c.Logger.Errorf("Error getting pod %q: %v", tr.Status.PodName, err) + return err + } } if pod == nil { pod, err = c.createPod(tr, rtr) @@ -350,7 +353,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error after := tr.Status.GetCondition(apis.ConditionSucceeded) if addReady { - if err := c.updateReady(pod); err != nil { + if err := podconvert.UpdateReady(c.KubeClientSet, *pod); err != nil { return err } } @@ -441,23 +444,6 @@ func (c *Reconciler) updateLabelsAndAnnotations(tr *v1alpha1.TaskRun) (*v1alpha1 return newTr, nil } -// updateReady updates a Pod to include the "ready" annotation, which will be projected by -// the Downward API into a volume mounted by the entrypoint container. This will signal to -// the entrypoint that the TaskRun can proceed. -func (c *Reconciler) updateReady(pod *corev1.Pod) error { - newPod, err := c.KubeClientSet.CoreV1().Pods(pod.Namespace).Get(pod.Name, metav1.GetOptions{}) - if err != nil { - return xerrors.Errorf("Error getting Pod %q when updating ready annotation: %w", pod.Name, err) - } - updatePod := c.KubeClientSet.CoreV1().Pods(newPod.Namespace).Update - if err := resources.AddReadyAnnotation(newPod, updatePod); err != nil { - c.Logger.Errorf("Failed to update ready annotation for pod %q for taskrun %q: %v", pod.Name, pod.Name, err) - return xerrors.Errorf("Error adding ready annotation to Pod %q: %w", pod.Name, err) - } - - return nil -} - // createPod creates a Pod based on the Task's configuration, with pvcName as a volumeMount // TODO(dibyom): Refactor resource setup/substitution logic to its own function in the resources package func (c *Reconciler) createPod(tr *v1alpha1.TaskRun, rtr *resources.ResolvedTaskResources) (*corev1.Pod, error) { @@ -493,11 +479,6 @@ func (c *Reconciler) createPod(tr *v1alpha1.TaskRun, rtr *resources.ResolvedTask return nil, err } - ts, err = createRedirectedTaskSpec(c.KubeClientSet, c.Images.EntryPointImage, ts, tr, c.cache, c.Logger) - if err != nil { - return nil, xerrors.Errorf("couldn't create redirected TaskSpec: %w", err) - } - var defaults []v1alpha1.ParamSpec if ts.Inputs != nil { defaults = append(defaults, ts.Inputs.Params...) @@ -509,7 +490,7 @@ func (c *Reconciler) createPod(tr *v1alpha1.TaskRun, rtr *resources.ResolvedTask ts = resources.ApplyResources(ts, inputResources, "inputs") ts = resources.ApplyResources(ts, outputResources, "outputs") - pod, err := resources.MakePod(c.Images, tr, *ts, c.KubeClientSet) + pod, err := resources.MakePod(c.Images, tr, *ts, c.KubeClientSet, c.entrypointCache) if err != nil { return nil, xerrors.Errorf("translating Build to Pod: %w", err) } @@ -517,51 +498,6 @@ func (c *Reconciler) createPod(tr *v1alpha1.TaskRun, rtr *resources.ResolvedTask return c.KubeClientSet.CoreV1().Pods(tr.Namespace).Create(pod) } -// CreateRedirectedTaskSpec takes a TaskSpec, a persistent volume claim name, a taskrun and -// an entrypoint cache creates a build where all entrypoints are switched to -// be the entrypoint redirector binary. This function assumes that it receives -// its own copy of the TaskSpec and modifies it freely -func createRedirectedTaskSpec(kubeclient kubernetes.Interface, entrypointImage string, ts *v1alpha1.TaskSpec, tr *v1alpha1.TaskRun, cache *entrypoint.Cache, logger *zap.SugaredLogger) (*v1alpha1.TaskSpec, error) { - // RedirectSteps the entrypoint in each container so that we can use our custom - // entrypoint which copies logs to the volume - err := entrypoint.RedirectSteps(cache, ts.Steps, kubeclient, tr, logger) - if err != nil { - return nil, xerrors.Errorf("failed to add entrypoint to steps of TaskRun %s: %w", tr.Name, err) - } - // Add the step which will copy the entrypoint into the volume - // we are going to be using, so that all of the steps will have - // access to it. - entrypoint.AddCopyStep(entrypointImage, ts) - - // Add the volume used for storing the binary and logs - ts.Volumes = append(ts.Volumes, corev1.Volume{ - Name: entrypoint.MountName, - VolumeSource: corev1.VolumeSource{ - // TODO(#107) we need to actually stream these logs somewhere, probably via sidecar. - // Currently these logs will be lost when the pod is unscheduled. - EmptyDir: &corev1.EmptyDirVolumeSource{}, - }, - }) - - ts.Volumes = append(ts.Volumes, corev1.Volume{ - Name: entrypoint.DownwardMountName, - VolumeSource: corev1.VolumeSource{ - DownwardAPI: &corev1.DownwardAPIVolumeSource{ - Items: []corev1.DownwardAPIVolumeFile{ - { - Path: entrypoint.DownwardMountReadyFile, - FieldRef: &corev1.ObjectFieldSelector{ - FieldPath: fmt.Sprintf("metadata.annotations['%s']", resources.ReadyAnnotation), - }, - }, - }, - }, - }, - }) - - return ts, nil -} - type DeletePod func(podName string, options *metav1.DeleteOptions) error func (c *Reconciler) updateTaskRunStatusForTimeout(tr *v1alpha1.TaskRun, dp DeletePod) error { @@ -605,20 +541,3 @@ func resourceImplBinding(resources map[string]*v1alpha1.PipelineResource, images } return p, nil } - -// getPodFunc returns the Pod for the given pod name -type getPodFunc func(string, metav1.GetOptions) (*corev1.Pod, error) - -// tryGetPod fetches the TaskRun's pod, returning nil if it has not been created or it does not exist. -func tryGetPod(taskRunStatus v1alpha1.TaskRunStatus, gp getPodFunc) (*corev1.Pod, error) { - if taskRunStatus.PodName == "" { - return nil, nil - } - - pod, err := gp(taskRunStatus.PodName, metav1.GetOptions{}) - if err == nil || errors.IsNotFound(err) { - return pod, nil - } - - return nil, err -} diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index e83f291eec6..22f12bf4ecd 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -29,7 +29,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" - "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/entrypoint" + "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources/cloudevent" ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" @@ -38,16 +38,12 @@ import ( "github.com/tektoncd/pipeline/test" tb "github.com/tektoncd/pipeline/test/builder" "github.com/tektoncd/pipeline/test/names" - "go.uber.org/zap" - "go.uber.org/zap/zaptest/observer" "golang.org/x/xerrors" corev1 "k8s.io/api/core/v1" k8sapierrors "k8s.io/apimachinery/pkg/api/errors" - kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" k8sruntimeschema "k8s.io/apimachinery/pkg/runtime/schema" fakekubeclientset "k8s.io/client-go/kubernetes/fake" ktesting "k8s.io/client-go/testing" @@ -66,7 +62,7 @@ const ( var ( images = pipeline.Images{ - EntryPointImage: "override-with-entrypoint:latest", + EntrypointImage: "override-with-entrypoint:latest", NopImage: "tianon/true", GitImage: "override-with-git:latest", CredsImage: "override-with-creds:latest", @@ -77,7 +73,6 @@ var ( PRImage: "override-with-pr:latest", ImageDigestExporterImage: "override-with-imagedigest-exporter-image:latest", } - entrypointCache *entrypoint.Cache ignoreLastTransitionTime = cmpopts.IgnoreTypes(apis.Condition{}.LastTransitionTime.Inner.Time) // Pods are created with a random 3-byte (6 hex character) suffix that we want to ignore in our diffs. ignoreRandomPodNameSuffix = cmp.FilterPath(func(path cmp.Path) bool { @@ -116,15 +111,6 @@ var ( saTask = tb.Task("test-with-sa", "foo", tb.TaskSpec(tb.Step("sa-step", "foo", tb.StepCommand("/mycmd")))) - taskEnvTask = tb.Task("test-task-env", "foo", tb.TaskSpec( - tb.TaskStepTemplate( - tb.EnvVar("FRUIT", "APPLE"), - ), - tb.Step("env-step", "foo", - tb.StepEnvVar("ANOTHER", "VARIABLE"), - tb.StepEnvVar("FRUIT", "LEMON"), - tb.StepCommand("/mycmd")))) - templatedTask = tb.Task("test-task-with-substitution", "foo", tb.TaskSpec( tb.TaskInputs( tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit), @@ -192,7 +178,7 @@ var ( EmptyDir: &corev1.EmptyDirVolumeSource{}, }, } - downward = corev1.Volume{ + downwardVolume = corev1.Volume{ Name: "downward", VolumeSource: corev1.VolumeSource{ DownwardAPI: &corev1.DownwardAPIVolumeSource{ @@ -209,7 +195,16 @@ var ( getMkdirResourceContainer = func(name, dir, suffix string, ops ...tb.ContainerOp) tb.PodSpecOp { actualOps := []tb.ContainerOp{ tb.Command("/builder/tools/entrypoint"), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "mkdir", "--", "-p", dir), + tb.Args("-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "mkdir", + "--", + "-p", + dir), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), tb.VolumeMount("tools", "/builder/tools"), @@ -231,17 +226,13 @@ var ( getPlaceToolsInitContainer = func(ops ...tb.ContainerOp) tb.PodSpecOp { actualOps := []tb.ContainerOp{ tb.Command("cp", "/ko-app/entrypoint", entrypointLocation), - tb.Args(), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), + tb.Args(), } actualOps = append(actualOps, ops...) - return tb.PodInitContainer("step-place-tools", "override-with-entrypoint:latest", actualOps...) + return tb.PodInitContainer("place-tools", "override-with-entrypoint:latest", actualOps...) } ) @@ -258,7 +249,6 @@ func getTaskRunController(t *testing.T, d test.Data) (test.TestAssets, func()) { SendSuccessfully: true, } ctx = cloudevent.WithClient(ctx, &cloudEventClientBehaviour) - entrypointCache, _ = entrypoint.NewCache() c, _ := test.SeedTestData(t, ctx, d) configMapWatcher := configmap.NewInformedWatcher(c.Kube, system.GetNamespace()) return test.TestAssets{ @@ -297,7 +287,6 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { name: "success", taskRun: taskRunSuccess, wantPod: tb.Pod("test-taskrun-run-success-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), tb.PodLabel(taskNameLabelKey, "test-task"), tb.PodLabel(taskRunNameLabelKey, "test-taskrun-run-success"), tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), @@ -305,12 +294,20 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { tb.OwnerReferenceAPIVersion(currentApiVersion)), tb.PodSpec( tb.PodServiceAccountName(defaultSAName), - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), + tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume), tb.PodRestartPolicy(corev1.RestartPolicyNever), getPlaceToolsInitContainer(), tb.PodContainer("step-simple-step", "foo", tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), + tb.Args("-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "/mycmd", + "--", + ), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), tb.VolumeMount("tools", "/builder/tools"), @@ -329,7 +326,6 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { name: "serviceaccount", taskRun: taskRunWithSaSuccess, wantPod: tb.Pod("test-taskrun-with-sa-run-success-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), tb.PodLabel(taskNameLabelKey, "test-with-sa"), tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-sa-run-success"), tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), @@ -337,12 +333,20 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { tb.OwnerReferenceAPIVersion(currentApiVersion)), tb.PodSpec( tb.PodServiceAccountName("test-sa"), - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), + tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume), tb.PodRestartPolicy(corev1.RestartPolicyNever), getPlaceToolsInitContainer(), tb.PodContainer("step-sa-step", "foo", tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), + tb.Args("-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "/mycmd", + "--", + ), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), tb.VolumeMount("tools", "/builder/tools"), @@ -432,12 +436,6 @@ func TestReconcile(t *testing.T) { taskRunWithSaSuccess := tb.TaskRun("test-taskrun-with-sa-run-success", "foo", tb.TaskRunSpec( tb.TaskRunTaskRef(saTask.Name, tb.TaskRefAPIVersion("a1")), tb.TaskRunServiceAccountName("test-sa"), )) - taskRunWithDeprecatedSaSuccess := tb.TaskRun("test-taskrun-with-deprecated-sa-run-success", "foo", tb.TaskRunSpec( - tb.TaskRunTaskRef(saTask.Name, tb.TaskRefAPIVersion("a1")), tb.TaskRunDeprecatedServiceAccount("", "test-deprecated-sa"), - )) - taskRunTaskEnv := tb.TaskRun("test-taskrun-task-env", "foo", tb.TaskRunSpec( - tb.TaskRunTaskRef(taskEnvTask.Name, tb.TaskRefAPIVersion("a1")), - )) taskRunSubstitution := tb.TaskRun("test-taskrun-substitution", "foo", tb.TaskRunSpec( tb.TaskRunTaskRef(templatedTask.Name, tb.TaskRefAPIVersion("a1")), tb.TaskRunInputs( @@ -525,55 +523,21 @@ func TestReconcile(t *testing.T) { ), ) - taskRunWithResourceRequests := tb.TaskRun("test-taskrun-with-resource-requests", "foo", - tb.TaskRunSpec( - tb.TaskRunTaskSpec( - tb.Step("step1", "foo", - tb.StepCommand("/mycmd"), - tb.StepResources( - tb.Limits( - tb.CPU("8"), - tb.Memory("10Gi"), - ), - tb.Requests( - tb.CPU("4"), - tb.Memory("3Gi"), - ), - ), - ), - tb.Step("step2", "foo", - tb.StepCommand("/mycmd"), - tb.StepResources( - tb.Limits(tb.Memory("5Gi")), - tb.Requests( - tb.CPU("2"), - tb.Memory("5Gi"), - tb.EphemeralStorage("25Gi"), - ), - ), - ), - tb.Step("step3", "foo", - tb.StepCommand("/mycmd"), - ), - ), - ), - ) - taskRunWithPod := tb.TaskRun("test-taskrun-with-pod", "foo", tb.TaskRunSpec(tb.TaskRunTaskRef(simpleTask.Name)), tb.TaskRunStatus(tb.PodName("some-pod-that-no-longer-exists")), ) taskruns := []*v1alpha1.TaskRun{ - taskRunSuccess, taskRunWithSaSuccess, taskRunWithDeprecatedSaSuccess, + taskRunSuccess, taskRunWithSaSuccess, taskRunSubstitution, taskRunInputOutput, taskRunWithTaskSpec, taskRunWithClusterTask, taskRunWithResourceSpecAndTaskSpec, - taskRunWithLabels, taskRunWithAnnotations, taskRunWithResourceRequests, taskRunTaskEnv, taskRunWithPod, + taskRunWithLabels, taskRunWithAnnotations, taskRunWithPod, } d := test.Data{ TaskRuns: taskruns, - Tasks: []*v1alpha1.Task{simpleTask, saTask, templatedTask, outputTask, taskEnvTask}, + Tasks: []*v1alpha1.Task{simpleTask, saTask, templatedTask, outputTask}, ClusterTasks: []*v1alpha1.ClusterTask{clustertask}, PipelineResources: []*v1alpha1.PipelineResource{gitResource, anotherGitResource, imageResource}, } @@ -585,19 +549,26 @@ func TestReconcile(t *testing.T) { name: "success", taskRun: taskRunSuccess, wantPod: tb.Pod("test-taskrun-run-success-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), tb.PodLabel(taskNameLabelKey, "test-task"), tb.PodLabel(taskRunNameLabelKey, "test-taskrun-run-success"), tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), tb.PodOwnerReference("TaskRun", "test-taskrun-run-success", tb.OwnerReferenceAPIVersion(currentApiVersion)), tb.PodSpec( - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), + tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume), tb.PodRestartPolicy(corev1.RestartPolicyNever), getPlaceToolsInitContainer(), tb.PodContainer("step-simple-step", "foo", tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), + tb.Args("-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "/mycmd", + "--", + ), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), tb.VolumeMount("tools", "/builder/tools"), @@ -616,7 +587,6 @@ func TestReconcile(t *testing.T) { name: "serviceaccount", taskRun: taskRunWithSaSuccess, wantPod: tb.Pod("test-taskrun-with-sa-run-success-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), tb.PodLabel(taskNameLabelKey, "test-with-sa"), tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-sa-run-success"), tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), @@ -624,44 +594,20 @@ func TestReconcile(t *testing.T) { tb.OwnerReferenceAPIVersion(currentApiVersion)), tb.PodSpec( tb.PodServiceAccountName("test-sa"), - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), + tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume), tb.PodRestartPolicy(corev1.RestartPolicyNever), getPlaceToolsInitContainer(), tb.PodContainer("step-sa-step", "foo", tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("downward", "/builder/downward"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - ), - ), - ), - }, { - name: "deprecated-serviceaccount", - taskRun: taskRunWithDeprecatedSaSuccess, - wantPod: tb.Pod("test-taskrun-with-deprecated-sa-run-success-pod-d406f0", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), - tb.PodLabel(taskNameLabelKey, "test-with-sa"), - tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-deprecated-sa-run-success"), - tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), - tb.PodOwnerReference("TaskRun", "test-taskrun-with-deprecated-sa-run-success", - tb.OwnerReferenceAPIVersion(currentApiVersion)), - tb.PodSpec( - tb.PodServiceAccountName("test-deprecated-sa"), - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), - tb.PodRestartPolicy(corev1.RestartPolicyNever), - getPlaceToolsInitContainer(), - tb.PodContainer("step-sa-step", "foo", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), + tb.Args("-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "/mycmd", + "--", + ), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), tb.VolumeMount("tools", "/builder/tools"), @@ -680,23 +626,25 @@ func TestReconcile(t *testing.T) { name: "params", taskRun: taskRunSubstitution, wantPod: tb.Pod("test-taskrun-substitution-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), tb.PodLabel(taskNameLabelKey, "test-task-with-substitution"), tb.PodLabel(taskRunNameLabelKey, "test-taskrun-substitution"), tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), tb.PodOwnerReference("TaskRun", "test-taskrun-substitution", tb.OwnerReferenceAPIVersion(currentApiVersion)), tb.PodSpec( - tb.PodVolumes(corev1.Volume{ - Name: "volume-configmap", - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "configbar", + tb.PodVolumes( + workspaceVolume, homeVolume, toolsVolume, downwardVolume, + corev1.Volume{ + Name: "volume-configmap", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "configbar", + }, }, }, }, - }, toolsVolume, downward, workspaceVolume, homeVolume), + ), tb.PodRestartPolicy(corev1.RestartPolicyNever), getPlaceToolsInitContainer(), getMkdirResourceContainer("myimage", "/workspace/output/myimage", "mssqb"), @@ -765,150 +713,35 @@ func TestReconcile(t *testing.T) { ), ), ), - }, { - name: "wrap-steps", - taskRun: taskRunInputOutput, - wantPod: tb.Pod("test-taskrun-input-output-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), - tb.PodLabel(taskNameLabelKey, "test-output-task"), - tb.PodLabel(taskRunNameLabelKey, "test-taskrun-input-output"), - tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), - tb.PodOwnerReference("TaskRun", "test-taskrun-input-output", - tb.OwnerReferenceAPIVersion(currentApiVersion)), - tb.PodSpec( - tb.PodVolumes(corev1.Volume{ - Name: "test-pvc", - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: "test-pvc", - ReadOnly: false, - }, - }, - }, toolsVolume, downward, workspaceVolume, homeVolume), - tb.PodRestartPolicy(corev1.RestartPolicyNever), - getPlaceToolsInitContainer(), - getMkdirResourceContainer("git-resource", "/workspace/output/git-resource", "6nl7g"), - tb.PodContainer("step-create-dir-git-resource-78c5n", "busybox", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/tools/0", "-post_file", "/builder/tools/1", "-entrypoint", "mkdir", "--", "-p", "/workspace/git-resource"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - ), - tb.PodContainer("step-source-copy-git-resource-mssqb", "busybox", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/tools/1", "-post_file", "/builder/tools/2", "-entrypoint", "cp", "--", "-r", "source-folder/.", "/workspace/git-resource"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("test-pvc", "/pvc"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - ), - tb.PodContainer("step-create-dir-another-git-resource-mz4c7", "busybox", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/tools/2", "-post_file", "/builder/tools/3", "-entrypoint", "mkdir", "--", "-p", "/workspace/another-git-resource"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - ), - tb.PodContainer("step-source-copy-another-git-resource-9l9zj", "busybox", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/tools/3", "-post_file", "/builder/tools/4", "-entrypoint", "cp", "--", "-r", "source-folder/.", "/workspace/another-git-resource"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("test-pvc", "/pvc"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - ), - tb.PodContainer("step-simple-step", "foo", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/tools/4", "-post_file", "/builder/tools/5", "-entrypoint", "/mycmd", "--"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - ), - tb.PodContainer("step-source-mkdir-git-resource-j2tds", "busybox", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/tools/5", "-post_file", "/builder/tools/6", "-entrypoint", "mkdir", "--", "-p", "output-folder"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("test-pvc", "/pvc"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - ), - tb.PodContainer("step-source-copy-git-resource-vr6ds", "busybox", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/tools/6", "-post_file", "/builder/tools/7", "-entrypoint", "cp", "--", "-r", "/workspace/output/git-resource/.", "output-folder"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("test-pvc", "/pvc"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - ), - ), - ), }, { name: "taskrun-with-taskspec", taskRun: taskRunWithTaskSpec, wantPod: tb.Pod("test-taskrun-with-taskspec-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-taskspec"), tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), tb.PodOwnerReference("TaskRun", "test-taskrun-with-taskspec", tb.OwnerReferenceAPIVersion(currentApiVersion)), tb.PodSpec( - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), + tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume), tb.PodRestartPolicy(corev1.RestartPolicyNever), getPlaceToolsInitContainer(), tb.PodContainer("step-git-source-git-resource-9l9zj", "override-with-git:latest", tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/ko-app/git-init", "--", - "-url", "https://foo.git", "-revision", "master", "-path", "/workspace/workspace"), + tb.Args("-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "/ko-app/git-init", + "--", + "-url", + "https://foo.git", + "-revision", + "master", + "-path", + "/workspace/workspace", + ), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), tb.EnvVar("TEKTON_RESOURCE_NAME", "git-resource"), @@ -943,19 +776,26 @@ func TestReconcile(t *testing.T) { name: "success-with-cluster-task", taskRun: taskRunWithClusterTask, wantPod: tb.Pod("test-taskrun-with-cluster-task-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), tb.PodLabel(taskNameLabelKey, "test-cluster-task"), tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-cluster-task"), tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), tb.PodOwnerReference("TaskRun", "test-taskrun-with-cluster-task", tb.OwnerReferenceAPIVersion(currentApiVersion)), tb.PodSpec( - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), + tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume), tb.PodRestartPolicy(corev1.RestartPolicyNever), getPlaceToolsInitContainer(), tb.PodContainer("step-simple-step", "foo", tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), + tb.Args("-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "/mycmd", + "--", + ), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), tb.VolumeMount("tools", "/builder/tools"), @@ -974,19 +814,29 @@ func TestReconcile(t *testing.T) { name: "taskrun-with-resource-spec-task-spec", taskRun: taskRunWithResourceSpecAndTaskSpec, wantPod: tb.Pod("test-taskrun-with-resource-spec-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-resource-spec"), tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), tb.PodOwnerReference("TaskRun", "test-taskrun-with-resource-spec", tb.OwnerReferenceAPIVersion(currentApiVersion)), tb.PodSpec( - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), + tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume), tb.PodRestartPolicy(corev1.RestartPolicyNever), getPlaceToolsInitContainer(), tb.PodContainer("step-git-source-workspace-9l9zj", "override-with-git:latest", tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/ko-app/git-init", "--", - "-url", "github.com/foo/bar.git", "-revision", "rel-can", "-path", + tb.Args("-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "/ko-app/git-init", + "--", + "-url", + "github.com/foo/bar.git", + "-revision", + "rel-can", + "-path", "/workspace/workspace"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), @@ -1017,187 +867,29 @@ func TestReconcile(t *testing.T) { ), ), ), - }, { - name: "taskrun-with-labels", - taskRun: taskRunWithLabels, - wantPod: tb.Pod("test-taskrun-with-labels-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), - tb.PodLabel("TaskRunLabel", "TaskRunValue"), - tb.PodLabel(taskNameLabelKey, "test-task"), - tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-labels"), - tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), - tb.PodOwnerReference("TaskRun", "test-taskrun-with-labels", - tb.OwnerReferenceAPIVersion(currentApiVersion)), - tb.PodSpec( - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), - tb.PodRestartPolicy(corev1.RestartPolicyNever), - getPlaceToolsInitContainer(), - tb.PodContainer("step-simple-step", "foo", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("downward", "/builder/downward"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - ), - ), - ), - }, { - name: "taskrun-with-annotations", - taskRun: taskRunWithAnnotations, - wantPod: tb.Pod("test-taskrun-with-annotations-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), - tb.PodAnnotation("TaskRunAnnotation", "TaskRunValue"), - tb.PodLabel(taskNameLabelKey, "test-task"), - tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-annotations"), - tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), - tb.PodOwnerReference("TaskRun", "test-taskrun-with-annotations", - tb.OwnerReferenceAPIVersion(currentApiVersion)), - tb.PodSpec( - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), - tb.PodRestartPolicy(corev1.RestartPolicyNever), - getPlaceToolsInitContainer(), - tb.PodContainer("step-simple-step", "foo", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("downward", "/builder/downward"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - ), - ), - ), - }, { - name: "task-env", - taskRun: taskRunTaskEnv, - wantPod: tb.Pod("test-taskrun-task-env-pod-311bc9", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), - tb.PodLabel(taskNameLabelKey, "test-task-env"), - tb.PodLabel(taskRunNameLabelKey, "test-taskrun-task-env"), - tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), - tb.PodOwnerReference("TaskRun", "test-taskrun-task-env", - tb.OwnerReferenceAPIVersion("tekton.dev/v1alpha1")), - tb.PodSpec( - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), - tb.PodRestartPolicy(corev1.RestartPolicyNever), - getPlaceToolsInitContainer(), - tb.PodContainer("step-env-step", "foo", tb.Command(entrypointLocation), - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("downward", "/builder/downward"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.EnvVar("ANOTHER", "VARIABLE"), - tb.EnvVar("FRUIT", "LEMON"), - ), - ), - ), - }, { - name: "taskrun-with-resource-requests", - taskRun: taskRunWithResourceRequests, - wantPod: tb.Pod("test-taskrun-with-resource-requests-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), - tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-resource-requests"), - tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), - tb.PodOwnerReference("TaskRun", "test-taskrun-with-resource-requests", - tb.OwnerReferenceAPIVersion(currentApiVersion)), - tb.PodSpec( - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), - tb.PodRestartPolicy(corev1.RestartPolicyNever), - getPlaceToolsInitContainer(), - tb.PodContainer("step-step1", "foo", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("downward", "/builder/downward"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources( - tb.Limits( - tb.CPU("8"), - tb.Memory("10Gi"), - ), - tb.Requests( - tb.CPU("4"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - ), - ), - ), - tb.PodContainer("step-step2", "foo", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/tools/0", "-post_file", "/builder/tools/1", "-entrypoint", "/mycmd", "--"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources( - tb.Limits(tb.Memory("5Gi")), - tb.Requests( - tb.CPU("0"), - tb.Memory("5Gi"), - tb.EphemeralStorage("25Gi"), - ), - ), - ), - tb.PodContainer("step-step3", "foo", - tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/tools/1", "-post_file", "/builder/tools/2", "-entrypoint", "/mycmd", "--"), - tb.WorkingDir(workspaceDir), - tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("tools", "/builder/tools"), - tb.VolumeMount("workspace", workspaceDir), - tb.VolumeMount("home", "/builder/home"), - tb.Resources(tb.Requests( - tb.CPU("0"), - tb.Memory("0"), - tb.EphemeralStorage("0"), - )), - ), - ), - ), }, { name: "taskrun-with-pod", taskRun: taskRunWithPod, wantPod: tb.Pod("test-taskrun-with-pod-pod-123456", "foo", - tb.PodAnnotation("tekton.dev/ready", ""), tb.PodLabel(taskNameLabelKey, "test-task"), tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-pod"), tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), tb.PodOwnerReference("TaskRun", "test-taskrun-with-pod", tb.OwnerReferenceAPIVersion(currentApiVersion)), tb.PodSpec( - tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), + tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume), tb.PodRestartPolicy(corev1.RestartPolicyNever), getPlaceToolsInitContainer(), tb.PodContainer("step-simple-step", "foo", tb.Command(entrypointLocation), - tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), + tb.Args("-wait_file", + "/builder/downward/ready", + "-wait_file_content", + "-post_file", + "/builder/tools/0", + "-entrypoint", + "/mycmd", + "--"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), tb.VolumeMount("tools", "/builder/tools"), @@ -1232,9 +924,6 @@ func TestReconcile(t *testing.T) { t.Fatal(err) } - entrypoint.AddToEntrypointCache(entrypointCache, "override-with-git:latest", []string{"/ko-app/git-init"}) - entrypoint.AddToEntrypointCache(entrypointCache, "busybox", []string{"sh"}) - if err := c.Reconciler.Reconcile(context.Background(), getRunName(tc.taskRun)); err != nil { t.Errorf("expected no error. Got error %v", err) } @@ -1503,12 +1192,19 @@ func makePod(taskRun *v1alpha1.TaskRun, task *v1alpha1.Task) (*corev1.Pod, error // specify the Pod we want to exist directly, and not call MakePod from // the build. This will break the cycle and allow us to simply use // clients normally. - return resources.MakePod(images, taskRun, task.Spec, fakekubeclientset.NewSimpleClientset(&corev1.ServiceAccount{ + kubeclient := fakekubeclientset.NewSimpleClientset(&corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: "default", Namespace: taskRun.Namespace, }, - })) + }) + + entrypointCache, err := pod.NewEntrypointCache(kubeclient) + if err != nil { + return nil, err + } + + return resources.MakePod(images, taskRun, task.Spec, kubeclient, entrypointCache) } func TestReconcilePodUpdateStatus(t *testing.T) { @@ -1573,34 +1269,6 @@ func TestReconcilePodUpdateStatus(t *testing.T) { } } -func TestCreateRedirectedTaskSpec(t *testing.T) { - tr := tb.TaskRun("tr", "tr", tb.TaskRunSpec( - tb.TaskRunServiceAccountName("sa"), - )) - task := tb.Task("tr-ts", "tr", tb.TaskSpec( - tb.Step("foo1", "bar1", tb.StepCommand("abcd"), tb.StepArgs("efgh")), - tb.Step("foo2", "bar2", tb.StepCommand("abcd"), tb.StepArgs("efgh")), - tb.TaskVolume("v"), - )) - - expectedSteps := len(task.Spec.Steps) + 1 - expectedVolumes := len(task.Spec.Volumes) + 2 - - observer, _ := observer.New(zap.InfoLevel) - entrypointCache, _ := entrypoint.NewCache() - c := fakekubeclientset.NewSimpleClientset() - ts, err := createRedirectedTaskSpec(c, "override-with-entrypoint:latest", &task.Spec, tr, entrypointCache, zap.New(observer).Sugar()) - if err != nil { - t.Errorf("expected createRedirectedTaskSpec to pass: %v", err) - } - if len(ts.Steps) != expectedSteps { - t.Errorf("step counts do not match: %d should be %d", len(ts.Steps), expectedSteps) - } - if len(ts.Volumes) != expectedVolumes { - t.Errorf("volumes do not match: %d should be %d", len(ts.Volumes), expectedVolumes) - } -} - func TestReconcileOnCompletedTaskRun(t *testing.T) { taskSt := &apis.Condition{ Type: apis.ConditionSucceeded, @@ -2117,65 +1785,3 @@ func TestUpdateTaskRunStatus_withInvalidJson(t *testing.T) { }) } } - -func TestTryGetPod(t *testing.T) { - err := xerrors.New("something went wrong") - for _, c := range []struct { - desc string - trs v1alpha1.TaskRunStatus - gp getPodFunc - wantNil bool - wantErr error - }{{ - desc: "no-pod", - trs: v1alpha1.TaskRunStatus{}, - gp: func(string, metav1.GetOptions) (*corev1.Pod, error) { - t.Errorf("Did not expect pod to be fetched") - return nil, nil - }, - wantNil: true, - wantErr: nil, - }, { - desc: "non-existent-pod", - trs: v1alpha1.TaskRunStatus{ - PodName: "no-longer-exist", - }, - gp: func(name string, opts metav1.GetOptions) (*corev1.Pod, error) { - return nil, kerrors.NewNotFound(schema.GroupResource{}, name) - }, - wantNil: true, - wantErr: nil, - }, { - desc: "existing-pod", - trs: v1alpha1.TaskRunStatus{ - PodName: "exists", - }, - gp: func(name string, opts metav1.GetOptions) (*corev1.Pod, error) { - return &corev1.Pod{}, nil - }, - wantNil: false, - wantErr: nil, - }, { - desc: "pod-fetch-error", - trs: v1alpha1.TaskRunStatus{ - PodName: "something-went-wrong", - }, - gp: func(name string, opts metav1.GetOptions) (*corev1.Pod, error) { - return nil, err - }, - wantNil: true, - wantErr: err, - }} { - t.Run(c.desc, func(t *testing.T) { - pod, err := tryGetPod(c.trs, c.gp) - if err != c.wantErr { - t.Fatalf("tryGetPod: %v", err) - } - - wasNil := pod == nil - if wasNil != c.wantNil { - t.Errorf("Pod got %v, want %v", wasNil, c.wantNil) - } - }) - } -} diff --git a/pkg/status/taskrunpod.go b/pkg/status/taskrunpod.go index f62792f6865..7e075cc8da3 100644 --- a/pkg/status/taskrunpod.go +++ b/pkg/status/taskrunpod.go @@ -23,7 +23,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" listers "github.com/tektoncd/pipeline/pkg/client/listers/pipeline/v1alpha1" - "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" + podconvert "github.com/tektoncd/pipeline/pkg/pod" "go.uber.org/zap" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -49,16 +49,16 @@ func UpdateStatusFromPod(taskRun *v1alpha1.TaskRun, pod *corev1.Pod, resourceLis taskRun.Status.Steps = []v1alpha1.StepState{} taskRun.Status.Sidecars = []v1alpha1.SidecarState{} for _, s := range pod.Status.ContainerStatuses { - if resources.IsContainerStep(s.Name) { + if podconvert.IsContainerStep(s.Name) { taskRun.Status.Steps = append(taskRun.Status.Steps, v1alpha1.StepState{ ContainerState: *s.State.DeepCopy(), - Name: resources.TrimContainerNamePrefix(s.Name), + Name: podconvert.TrimStepPrefix(s.Name), ContainerName: s.Name, ImageID: s.ImageID, }) - } else if resources.IsContainerSidecar(s.Name) { + } else if podconvert.IsContainerSidecar(s.Name) { taskRun.Status.Sidecars = append(taskRun.Status.Sidecars, v1alpha1.SidecarState{ - Name: resources.TrimSidecarNamePrefix(s.Name), + Name: podconvert.TrimSidecarPrefix(s.Name), ImageID: s.ImageID, }) } @@ -128,7 +128,7 @@ func updateIncompleteTaskRun(taskRun *v1alpha1.TaskRun, pod *corev1.Pod) { func didTaskRunFail(pod *corev1.Pod) bool { f := pod.Status.Phase == corev1.PodFailed for _, s := range pod.Status.ContainerStatuses { - if resources.IsContainerStep(s.Name) { + if podconvert.IsContainerStep(s.Name) { if s.State.Terminated != nil { f = f || s.State.Terminated.ExitCode != 0 } @@ -140,7 +140,7 @@ func didTaskRunFail(pod *corev1.Pod) bool { func areStepsComplete(pod *corev1.Pod) bool { stepsComplete := len(pod.Status.ContainerStatuses) > 0 && pod.Status.Phase == corev1.PodRunning for _, s := range pod.Status.ContainerStatuses { - if resources.IsContainerStep(s.Name) { + if podconvert.IsContainerStep(s.Name) { if s.State.Terminated == nil { stepsComplete = false } @@ -151,7 +151,7 @@ func areStepsComplete(pod *corev1.Pod) bool { func countSidecars(pod *corev1.Pod) (total int, readyOrTerminated int) { for _, s := range pod.Status.ContainerStatuses { - if resources.IsContainerSidecar(s.Name) { + if podconvert.IsContainerSidecar(s.Name) { if s.State.Running != nil && s.Ready { readyOrTerminated++ } else if s.State.Terminated != nil { diff --git a/test/builder/pod.go b/test/builder/pod.go index 7123ad80d7e..431318ed301 100644 --- a/test/builder/pod.go +++ b/test/builder/pod.go @@ -37,8 +37,9 @@ type PodStatusOp func(status *corev1.PodStatus) func Pod(name, namespace string, ops ...PodOp) *corev1.Pod { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: name, + Namespace: namespace, + Name: name, + Annotations: map[string]string{}, }, } for _, op := range ops {