From 8218f4fc2e06329c5738dd7f59ae6415b5660b7c Mon Sep 17 00:00:00 2001 From: Dibyo Mukherjee Date: Mon, 22 Jul 2019 16:08:45 -0400 Subject: [PATCH] Refactor some taskrun fields into helper functions This commit refactors the labels, annotations, default timeouts, and service account fields from taskruns into helper functions. This will allow us to easily reuse some common defaults for regular taskruns as well as those created for conditionals in #1093 Signed-off-by: Dibyo Mukherjee --- .../v1alpha1/pipelinerun/pipelinerun.go | 131 ++++++++++-------- .../v1alpha1/pipelinerun/pipelinerun_test.go | 7 +- 2 files changed, 76 insertions(+), 62 deletions(-) diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go index 6b64f77e1fe..b585d268fcd 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go @@ -26,6 +26,14 @@ import ( "github.com/knative/pkg/configmap" "github.com/knative/pkg/controller" "github.com/knative/pkg/tracker" + "go.uber.org/zap" + "golang.org/x/xerrors" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/cache" + "github.com/tektoncd/pipeline/pkg/apis/config" apisconfig "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" @@ -36,13 +44,6 @@ import ( "github.com/tektoncd/pipeline/pkg/reconciler/v1alpha1/pipeline/dag" "github.com/tektoncd/pipeline/pkg/reconciler/v1alpha1/pipelinerun/resources" "github.com/tektoncd/pipeline/pkg/reconciler/v1alpha1/taskrun" - "go.uber.org/zap" - "golang.org/x/xerrors" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/tools/cache" ) const ( @@ -396,54 +397,6 @@ func (c *Reconciler) updateTaskRunsStatusDirectly(pr *v1alpha1.PipelineRun) erro } func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.ResolvedPipelineRunTask, pr *v1alpha1.PipelineRun, storageBasePath string) (*v1alpha1.TaskRun, error) { - var taskRunTimeout = &metav1.Duration{Duration: apisconfig.NoTimeoutDuration} - - var timeout time.Duration - if pr.Spec.Timeout == nil { - timeout = config.DefaultTimeoutMinutes - } else { - timeout = pr.Spec.Timeout.Duration - } - // If the value of the timeout is 0 for any resource, there is no timeout. - // It is impossible for pr.Spec.Timeout to be nil, since SetDefault always assigns it with a value. - if timeout != apisconfig.NoTimeoutDuration { - pTimeoutTime := pr.Status.StartTime.Add(timeout) - if time.Now().After(pTimeoutTime) { - // Just in case something goes awry and we're creating the TaskRun after it should have already timed out, - // set the timeout to 1 second. - taskRunTimeout = &metav1.Duration{Duration: time.Until(pTimeoutTime)} - if taskRunTimeout.Duration < 0 { - taskRunTimeout = &metav1.Duration{Duration: 1 * time.Second} - } - } else { - taskRunTimeout = &metav1.Duration{Duration: timeout} - } - } - - // If service account is configured for a given PipelineTask, override PipelineRun's seviceAccount - serviceAccount := pr.Spec.ServiceAccount - for _, sa := range pr.Spec.ServiceAccounts { - if sa.TaskName == rprt.PipelineTask.Name { - serviceAccount = sa.ServiceAccount - } - } - - // Propagate labels from PipelineRun to TaskRun. - labels := make(map[string]string, len(pr.ObjectMeta.Labels)+1) - for key, val := range pr.ObjectMeta.Labels { - labels[key] = val - } - labels[pipeline.GroupName+pipeline.PipelineRunLabelKey] = pr.Name - if rprt.PipelineTask.Name != "" { - labels[pipeline.GroupName+pipeline.PipelineTaskLabelKey] = rprt.PipelineTask.Name - } - - // Propagate annotations from PipelineRun to TaskRun. - annotations := make(map[string]string, len(pr.ObjectMeta.Annotations)+1) - for key, val := range pr.ObjectMeta.Annotations { - annotations[key] = val - } - tr, _ := c.taskRunLister.TaskRuns(pr.Namespace).Get(rprt.TaskRunName) if tr != nil { @@ -463,8 +416,8 @@ func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.Re Name: rprt.TaskRunName, Namespace: pr.Namespace, OwnerReferences: pr.GetOwnerReference(), - Labels: labels, - Annotations: annotations, + Labels: getTaskrunLabels(pr, rprt.PipelineTask.Name), + Annotations: getTaskrunAnnotations(pr), }, Spec: v1alpha1.TaskRunSpec{ TaskRef: &v1alpha1.TaskRef{ @@ -474,8 +427,8 @@ func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.Re Inputs: v1alpha1.TaskRunInputs{ Params: rprt.PipelineTask.Params, }, - ServiceAccount: serviceAccount, - Timeout: taskRunTimeout, + ServiceAccount: getServiceAccount(pr, rprt.PipelineTask.Name), + Timeout: getTaskRunTimeout(pr), PodTemplate: podTemplate, }} @@ -497,6 +450,66 @@ func clearStatus(tr *v1alpha1.TaskRun) { tr.Status.PodName = "" } +func getTaskrunAnnotations(pr *v1alpha1.PipelineRun) map[string]string { + // Propagate annotations from PipelineRun to TaskRun. + annotations := make(map[string]string, len(pr.ObjectMeta.Annotations)+1) + for key, val := range pr.ObjectMeta.Annotations { + annotations[key] = val + } + return annotations +} + +func getTaskrunLabels(pr *v1alpha1.PipelineRun, pipelineTaskName string) map[string]string { + // Propagate labels from PipelineRun to TaskRun. + labels := make(map[string]string, len(pr.ObjectMeta.Labels)+1) + for key, val := range pr.ObjectMeta.Labels { + labels[key] = val + } + labels[pipeline.GroupName+pipeline.PipelineRunLabelKey] = pr.Name + if pipelineTaskName != "" { + labels[pipeline.GroupName+pipeline.PipelineTaskLabelKey] = pipelineTaskName + } + return labels +} + +func getTaskRunTimeout(pr *v1alpha1.PipelineRun) *metav1.Duration { + var taskRunTimeout = &metav1.Duration{Duration: apisconfig.NoTimeoutDuration} + + var timeout time.Duration + if pr.Spec.Timeout == nil { + timeout = config.DefaultTimeoutMinutes + } else { + timeout = pr.Spec.Timeout.Duration + } + // If the value of the timeout is 0 for any resource, there is no timeout. + // It is impossible for pr.Spec.Timeout to be nil, since SetDefault always assigns it with a value. + if timeout != apisconfig.NoTimeoutDuration { + pTimeoutTime := pr.Status.StartTime.Add(timeout) + if time.Now().After(pTimeoutTime) { + // Just in case something goes awry and we're creating the TaskRun after it should have already timed out, + // set the timeout to 1 second. + taskRunTimeout = &metav1.Duration{Duration: time.Until(pTimeoutTime)} + if taskRunTimeout.Duration < 0 { + taskRunTimeout = &metav1.Duration{Duration: 1 * time.Second} + } + } else { + taskRunTimeout = &metav1.Duration{Duration: timeout} + } + } + return taskRunTimeout +} + +func getServiceAccount(pr *v1alpha1.PipelineRun, pipelineTaskName string) string { + // If service account is configured for a given PipelineTask, override PipelineRun's seviceAccount + serviceAccount := pr.Spec.ServiceAccount + for _, sa := range pr.Spec.ServiceAccounts { + if sa.TaskName == pipelineTaskName { + serviceAccount = sa.ServiceAccount + } + } + return serviceAccount +} + func (c *Reconciler) updateStatus(pr *v1alpha1.PipelineRun) (*v1alpha1.PipelineRun, error) { newPr, err := c.pipelineRunLister.PipelineRuns(pr.Namespace).Get(pr.Name) if err != nil { diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go index 22abd63a2df..529c2586349 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go @@ -26,6 +26,10 @@ import ( duckv1beta1 "github.com/knative/pkg/apis/duck/v1beta1" "github.com/knative/pkg/configmap" rtesting "github.com/knative/pkg/reconciler/testing" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ktesting "k8s.io/client-go/testing" + "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/reconciler/v1alpha1/pipelinerun/resources" @@ -34,9 +38,6 @@ import ( "github.com/tektoncd/pipeline/test" tb "github.com/tektoncd/pipeline/test/builder" "github.com/tektoncd/pipeline/test/names" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - ktesting "k8s.io/client-go/testing" ) func getRunName(pr *v1alpha1.PipelineRun) string {