Skip to content

Commit

Permalink
Refactor some taskrun fields into helper functions
Browse files Browse the repository at this point in the history
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 tektoncd#1093

Also, this replaces some of the TestReconcile_ funcs that were testing
only the functionality in these new helper functions with their own
dedicated unit tests

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
  • Loading branch information
dibyom committed Jul 23, 2019
1 parent 5be3663 commit 3431eb4
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 290 deletions.
131 changes: 72 additions & 59 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 (
Expand Down Expand Up @@ -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 {
Expand All @@ -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{
Expand All @@ -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,
}}

Expand All @@ -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 {
Expand Down
Loading

0 comments on commit 3431eb4

Please sign in to comment.