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

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
  • Loading branch information
@dibyom
dibyom committed Jul 24, 2019
1 parent 096dcd4 commit b50c2dc
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 114 deletions.
131 changes: 72 additions & 59 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
View file
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
121 changes: 66 additions & 55 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -945,63 +946,73 @@ func TestReconcileWithTimeoutAndRetry(t *testing.T) {
}
}

func TestReconcilePropagateAnnotations(t *testing.T) {
names.TestingSeed()

ps := []*v1alpha1.Pipeline{tb.Pipeline("test-pipeline", "foo", tb.PipelineSpec(
tb.PipelineTask("hello-world-1", "hello-world"),
))}
prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-run-with-annotations", "foo",
tb.PipelineRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"),
tb.PipelineRunSpec("test-pipeline",
tb.PipelineRunServiceAccount("test-sa"),
),
)}
ts := []*v1alpha1.Task{tb.Task("hello-world", "foo")}

d := test.Data{
PipelineRuns: prs,
Pipelines: ps,
Tasks: ts,
func TestGetTaskRunAnnotations(t *testing.T) {
pr := tb.PipelineRun("pipeline-run-annotations", "foo", tb.PipelineRunAnnotation("abc", "def"))
expected := map[string]string{
"abc": "def",
}

testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients

err := c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-run-with-annotations")
if err != nil {
t.Errorf("Did not expect to see error when reconciling completed PipelineRun but saw %s", err)
if d := cmp.Diff(getTaskrunAnnotations(pr), expected); d != "" {
t.Errorf("Expected pipelinerun annotations to propagate but it didn't: %s", d)
}
}

// Check that the PipelineRun was reconciled correctly
_, err = clients.Pipeline.Tekton().PipelineRuns("foo").Get("test-pipeline-run-with-annotations", metav1.GetOptions{})
if err != nil {
t.Fatalf("Somehow had error getting completed reconciled run out of fake client: %s", err)
}
func TestGetTaskRunLabels(t *testing.T) {
pr := tb.PipelineRun("pipeline-run-labels", "foo", tb.PipelineRunLabel("abc", "def"))
tcs := []struct {
name string
pt string
expected map[string]string
}{{
name: "pipelineTaskName empty",
pt: "",
expected: map[string]string{
"tekton.dev/pipelineRun": "pipeline-run-labels",
"abc": "def",
},
}, {
name: "pipelineTaskName empty",
pt: "foo",
expected: map[string]string{
"tekton.dev/pipelineRun": "pipeline-run-labels",
"tekton.dev/pipelineTask": "foo",
"abc": "def",
},
}}

// Check that the expected TaskRun was created
actual := clients.Pipeline.Actions()[0].(ktesting.CreateAction).GetObject().(*v1alpha1.TaskRun)
if actual == nil {
t.Errorf("Expected a TaskRun to be created, but it wasn't.")
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
if d := cmp.Diff(getTaskrunLabels(pr, tc.pt), tc.expected); d != "" {
t.Errorf("Unexpected taskrun labels: %s", d)
}
})
}
expectedTaskRun := tb.TaskRun("test-pipeline-run-with-annotations-hello-world-1-9l9zj", "foo",
tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-with-annotations",
tb.OwnerReferenceAPIVersion("tekton.dev/v1alpha1"),
tb.Controller, tb.BlockOwnerDeletion,
),
tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"),
tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineTaskLabelKey, "hello-world-1"),
tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-with-annotations"),
tb.TaskRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"),
tb.TaskRunSpec(
tb.TaskRunTaskRef("hello-world"),
tb.TaskRunServiceAccount("test-sa"),
),
)
}

if d := cmp.Diff(actual, expectedTaskRun); d != "" {
t.Errorf("expected to see TaskRun %v created. Diff %s", expectedTaskRun, d)
func TestGetTaskRunServiceAccounts(t *testing.T) {
pr := tb.PipelineRun("pipeline-run-service-accounts", "foo", tb.PipelineRunSpec(
"pipeline-run-name",
tb.PipelineRunServiceAccount("default-sa"),
tb.PipelineRunServiceAccountTask("task-1", "task-1-sa"),
))
tcs := []struct {
name string
pt string
expected string
}{{
name: "pr default service account",
pt: "task-2",
expected: "default-sa",
}, {
name: "task specific service account",
pt: "task-1",
expected: "task-1-sa",
}}

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
if d := cmp.Diff(getServiceAccount(pr, tc.pt), tc.expected); d != "" {
t.Errorf("Unexpected taskrun service account: %s", d)
}
})
}
}

0 comments on commit b50c2dc

Please sign in to comment.