From 5ef2ec03f3c759dad748fe35f3e64c48f70cf234 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Wed, 20 Nov 2019 07:10:37 -0800 Subject: [PATCH] Move GetPod from pod.go to taskrun.go GetPod is only called from taskrun.go, so let's just move it to that package and unexport it. --- pkg/reconciler/taskrun/resources/pod.go | 18 ------ pkg/reconciler/taskrun/resources/pod_test.go | 64 -------------------- pkg/reconciler/taskrun/taskrun.go | 20 +++++- pkg/reconciler/taskrun/taskrun_test.go | 64 ++++++++++++++++++++ 4 files changed, 82 insertions(+), 84 deletions(-) diff --git a/pkg/reconciler/taskrun/resources/pod.go b/pkg/reconciler/taskrun/resources/pod.go index 0a830799d52..464ffb773ec 100644 --- a/pkg/reconciler/taskrun/resources/pod.go +++ b/pkg/reconciler/taskrun/resources/pod.go @@ -36,7 +36,6 @@ import ( "github.com/tektoncd/pipeline/pkg/names" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/entrypoint" corev1 "k8s.io/api/core/v1" - "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/schema" @@ -218,23 +217,6 @@ func makeWorkingDirInitializer(shellImage string, steps []v1alpha1.Step) *v1alph return nil } -// GetPod returns the Pod for the given pod name -type GetPod 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 GetPod) (*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 -} - // 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) { diff --git a/pkg/reconciler/taskrun/resources/pod_test.go b/pkg/reconciler/taskrun/resources/pod_test.go index 0b7f51afe7c..89d0e28c694 100644 --- a/pkg/reconciler/taskrun/resources/pod_test.go +++ b/pkg/reconciler/taskrun/resources/pod_test.go @@ -28,10 +28,8 @@ import ( "github.com/tektoncd/pipeline/test/names" "golang.org/x/xerrors" corev1 "k8s.io/api/core/v1" - "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/schema" fakek8s "k8s.io/client-go/kubernetes/fake" ) @@ -43,68 +41,6 @@ var ( shellImage = "busybox" ) -func TestTryGetPod(t *testing.T) { - err := xerrors.New("something went wrong") - for _, c := range []struct { - desc string - trs v1alpha1.TaskRunStatus - gp GetPod - 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, errors.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) - } - }) - } -} - func TestMakePod(t *testing.T) { names.TestingSeed() diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 9ac0e74bab0..dfcc09cba56 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -35,7 +35,6 @@ import ( "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" corev1 "k8s.io/api/core/v1" @@ -318,7 +317,7 @@ 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 := resources.TryGetPod(tr.Status, c.KubeClientSet.CoreV1().Pods(tr.Namespace).Get) + 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 @@ -606,3 +605,20 @@ 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 28a3b5e92c9..23b52e65920 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -43,9 +43,11 @@ import ( "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" @@ -2144,3 +2146,65 @@ 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) + } + }) + } +}