From b94514661ac029087e7de6f3b4533930c94652f2 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Wed, 23 Jan 2019 10:41:26 -0500 Subject: [PATCH] Add Pipeline-level timeouts Rebased and squashed the initial work, more to come. Fixes #222 --- pkg/apis/pipeline/v1alpha1/pipeline_types.go | 9 ++- .../pipeline/v1alpha1/pipeline_validation.go | 13 ++++ .../v1alpha1/pipeline_validation_test.go | 40 +++++++++++ .../pipeline/v1alpha1/pipelinerun_types.go | 10 +++ .../v1alpha1/pipelinerun_types_test.go | 4 ++ .../pipeline/v1alpha1/taskrun_validation.go | 1 + .../v1alpha1/zz_generated.deepcopy.go | 27 ++++++++ .../v1alpha1/pipelinerun/pipelinerun.go | 27 ++++++-- .../v1alpha1/pipelinerun/pipelinerun_test.go | 68 +++++++++++++++++-- .../resources/pipelinerunresolution.go | 33 +++++++-- .../resources/pipelinerunresolution_test.go | 4 +- pkg/reconciler/v1alpha1/taskrun/taskrun.go | 4 ++ .../v1alpha1/taskrun/taskrun_test.go | 1 + test/builder/pipeline.go | 15 ++++ test/builder/pipeline_test.go | 14 +++- test/builder/task.go | 7 ++ 16 files changed, 254 insertions(+), 23 deletions(-) diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_types.go b/pkg/apis/pipeline/v1alpha1/pipeline_types.go index cc5c0e89f31..f5b4d494225 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_types.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_types.go @@ -23,8 +23,13 @@ import ( // PipelineSpec defines the desired state of PipeLine. type PipelineSpec struct { - Tasks []PipelineTask `json:"tasks"` - Generation int64 `json:"generation,omitempty"` + Tasks []PipelineTask `json:"tasks"` + // Time after which the build times out. Defaults to never. + // Specified build timeout should be less than 24h. + // Refer Go's ParseDuration documentation for expected format: https://golang.org/pkg/time/#ParseDuration + // +optional + Timeout *metav1.Duration `json:"timeout,omitempty"` + Generation int64 `json:"generation,omitempty"` } // PipelineStatus does not contain anything because Pipelines on their own diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation.go index b13d9dc650a..647e11e5f45 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_validation.go @@ -18,6 +18,7 @@ package v1alpha1 import ( "fmt" + "time" "github.com/knative/pkg/apis" "k8s.io/apimachinery/pkg/api/equality" @@ -58,5 +59,17 @@ func (ps *PipelineSpec) Validate() *apis.FieldError { } } } + + if ps.Timeout != nil { + // timeout should be a valid duration of between 0 and 24 hours. + maxTimeout := time.Duration(24 * time.Hour) + + if ps.Timeout.Duration > maxTimeout { + return apis.ErrInvalidValue(fmt.Sprintf("%s should be < 24h", ps.Timeout.Duration.String()), "spec.timeout") + } else if ps.Timeout.Duration <= 0 { + return apis.ErrInvalidValue(fmt.Sprintf("%s should be > 0", ps.Timeout.Duration.String()), "spec.timeout") + } + } + return nil } diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go index d7249dab921..a618b7ab138 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go @@ -18,11 +18,15 @@ package v1alpha1 import ( "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestPipelineSpec_Validate_Error(t *testing.T) { type fields struct { Tasks []PipelineTask + Timeout *metav1.Duration Generation int64 } tests := []struct { @@ -50,11 +54,34 @@ func TestPipelineSpec_Validate_Error(t *testing.T) { }}, }, }, + { + name: "negative pipeline timeout", + fields: fields{ + Tasks: []PipelineTask{{ + Name: "foo", + }, { + Name: "baz", + }}, + Timeout: &metav1.Duration{Duration: -48 * time.Hour}, + }, + }, + { + name: "maximum timeout", + fields: fields{ + Tasks: []PipelineTask{{ + Name: "foo", + }, { + Name: "baz", + }}, + Timeout: &metav1.Duration{Duration: 48 * time.Hour}, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ps := &PipelineSpec{ Tasks: tt.fields.Tasks, + Timeout: tt.fields.Timeout, Generation: tt.fields.Generation, } if err := ps.Validate(); err == nil { @@ -67,6 +94,7 @@ func TestPipelineSpec_Validate_Error(t *testing.T) { func TestPipelineSpec_Validate_Valid(t *testing.T) { type fields struct { Tasks []PipelineTask + Timeout *metav1.Duration Generation int64 } tests := []struct { @@ -96,11 +124,23 @@ func TestPipelineSpec_Validate_Valid(t *testing.T) { }}, }, }, + { + name: "valid timeout", + fields: fields{ + Tasks: []PipelineTask{{ + Name: "foo", + }, { + Name: "baz", + }}, + Timeout: &metav1.Duration{Duration: 24 * time.Hour}, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ps := &PipelineSpec{ Tasks: tt.fields.Tasks, + Timeout: tt.fields.Timeout, Generation: tt.fields.Generation, } if err := ps.Validate(); err != nil { diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go index e2f64283dc3..151d976d677 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go @@ -18,6 +18,7 @@ package v1alpha1 import ( "fmt" + "time" duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1" "github.com/knative/pkg/webhook" @@ -118,6 +119,12 @@ type PipelineRunStatus struct { // In #107 should be updated to hold the location logs have been uploaded to // +optional Results *Results `json:"results,omitempty"` + // StartTime is the time the build is actually started. + // +optional + StartTime *metav1.Time `json:"startTime,omitempty"` + // CompletionTime is the time the build completed. + // +optional + CompletionTime *metav1.Time `json:"completionTime,omitempty"` // map of TaskRun Status with the taskRun name as the key //+optional TaskRuns map[string]TaskRunStatus `json:"taskRuns,omitempty"` @@ -135,6 +142,9 @@ func (pr *PipelineRunStatus) InitializeConditions() { if pr.TaskRuns == nil { pr.TaskRuns = make(map[string]TaskRunStatus) } + if pr.StartTime.IsZero() { + pr.StartTime = &metav1.Time{time.Now()} + } pipelineRunCondSet.Manage(pr).InitializeConditions() } diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go index fa8192d2d21..7e4298bdb37 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go @@ -76,6 +76,10 @@ func TestInitializeConditions(t *testing.T) { t.Fatalf("PipelineRun status not initialized correctly") } + if p.Status.StartTime.IsZero() { + t.Fatalf("PipelineRun StartTime not initialized correctly") + } + p.Status.TaskRuns["fooTask"] = TaskRunStatus{} p.Status.InitializeConditions() diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_validation.go b/pkg/apis/pipeline/v1alpha1/taskrun_validation.go index 68fe8aba088..444c219b2b1 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_validation.go @@ -69,6 +69,7 @@ func (ts *TaskRunSpec) Validate() *apis.FieldError { return err } } + return nil } diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index 5c6e4aeb1cc..5900f3482c3 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -544,6 +544,24 @@ func (in *PipelineRunStatus) DeepCopyInto(out *PipelineRunStatus) { **out = **in } } + if in.StartTime != nil { + in, out := &in.StartTime, &out.StartTime + if *in == nil { + *out = nil + } else { + *out = new(v1.Time) + (*in).DeepCopyInto(*out) + } + } + if in.CompletionTime != nil { + in, out := &in.CompletionTime, &out.CompletionTime + if *in == nil { + *out = nil + } else { + *out = new(v1.Time) + (*in).DeepCopyInto(*out) + } + } if in.TaskRuns != nil { in, out := &in.TaskRuns, &out.TaskRuns *out = make(map[string]TaskRunStatus, len(*in)) @@ -576,6 +594,15 @@ func (in *PipelineSpec) DeepCopyInto(out *PipelineSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + if *in == nil { + *out = nil + } else { + *out = new(v1.Duration) + **out = **in + } + } return } diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go index 735fbbd2069..235694eb8a6 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go @@ -256,7 +256,8 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er if rprt != nil { c.Logger.Infof("Creating a new TaskRun object %s", rprt.TaskRunName) - rprt.TaskRun, err = c.createTaskRun(c.Logger, rprt.ResolvedTaskResources.TaskName, rprt.TaskRunName, pr, rprt.PipelineTask, serviceAccount) + rprt.TaskRun, err = c.createTaskRun(c.Logger, rprt.ResolvedTaskResources.TaskName, rprt.TaskRunName, rprt.ResolvedTaskResources.TaskSpec, + pr, rprt.PipelineTask, serviceAccount, p.Spec.Timeout) if err != nil { c.Recorder.Eventf(pr, corev1.EventTypeWarning, "TaskRunCreationFailed", "Failed to create TaskRun %q: %v", rprt.TaskRunName, err) return fmt.Errorf("error creating TaskRun called %s for PipelineTask %s from PipelineRun %s: %s", rprt.TaskRunName, rprt.PipelineTask.Name, pr.Name, err) @@ -264,7 +265,8 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er } before := pr.Status.GetCondition(duckv1alpha1.ConditionSucceeded) - after := resources.GetPipelineConditionStatus(pr.Name, pipelineState, c.Logger) + after := resources.GetPipelineConditionStatus(pr.Name, pipelineState, c.Logger, pr.Status.StartTime, p.Spec.Timeout) + pr.Status.SetCondition(after) reconciler.EmitEvent(c.Recorder, before, after, pr) @@ -283,7 +285,8 @@ func updateTaskRunsStatus(pr *v1alpha1.PipelineRun, pipelineState []*resources.R } } -func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, taskName, taskRunName string, pr *v1alpha1.PipelineRun, pt *v1alpha1.PipelineTask, sa string) (*v1alpha1.TaskRun, error) { +func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, taskName, taskRunName string, ts *v1alpha1.TaskSpec, + pr *v1alpha1.PipelineRun, pt *v1alpha1.PipelineTask, sa string, pipelineTimeout *metav1.Duration) (*v1alpha1.TaskRun, error) { tr := &v1alpha1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ Name: taskRunName, @@ -295,15 +298,25 @@ func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, taskName, taskRunN }, }, Spec: v1alpha1.TaskRunSpec{ - TaskRef: &v1alpha1.TaskRef{ - Name: taskName, - }, Inputs: v1alpha1.TaskRunInputs{ Params: pt.Params, }, - ServiceAccount: sa, + ServiceAccount: sa, }, } + + if pipelineTimeout != nil { + pTimeoutTime := pr.Status.StartTime.Add(pipelineTimeout.Duration) + if ts.Timeout != nil && time.Now().Add(ts.Timeout.Duration).After(pTimeoutTime){ + ts.Timeout.Duration = pTimeoutTime.Sub(time.Now()) + } + tr.Spec.TaskSpec = ts + } else { + tr.Spec.TaskRef = &v1alpha1.TaskRef{ + Name: taskName, + } + } + resources.WrapSteps(&tr.Spec, pr.Spec.PipelineTaskResources, pt) return c.PipelineClientSet.PipelineV1alpha1().TaskRuns(pr.Namespace).Create(tr) diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go index a5e9f3204c3..4b9f97f68f6 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go @@ -396,8 +396,7 @@ func TestReconcileOnCompletedPipelineRun(t *testing.T) { })), )} ps := []*v1alpha1.Pipeline{tb.Pipeline("test-pipeline", "foo", tb.PipelineSpec( - tb.PipelineTask("hello-world-1", "hellow-world"), - ))} + tb.PipelineTask("hello-world-1", "hellow-world")))} ts := []*v1alpha1.Task{tb.Task("hello-world", "foo")} d := test.Data{ PipelineRuns: prs, @@ -467,6 +466,7 @@ func TestReconcileOnCancelledPipelineRun(t *testing.T) { ), ), } + d := test.Data{ PipelineRuns: prs, Pipelines: ps, @@ -488,12 +488,70 @@ func TestReconcileOnCancelledPipelineRun(t *testing.T) { // Check that the PipelineRun was reconciled correctly reconciledRun, err := clients.Pipeline.Pipeline().PipelineRuns("foo").Get("test-pipeline-run-cancelled", metav1.GetOptions{}) - if err != nil { - t.Fatalf("Somehow had error getting completed reconciled run out of fake client: %s", err) - } // This PipelineRun should still be complete and false, and the status should reflect that if !reconciledRun.Status.GetCondition(duckv1alpha1.ConditionSucceeded).IsFalse() { t.Errorf("Expected PipelineRun status to be complete and false, but was %v", reconciledRun.Status.GetCondition(duckv1alpha1.ConditionSucceeded)) } } + +func TestReconcileWithTimeout(t *testing.T) { + ps := []*v1alpha1.Pipeline{tb.Pipeline("test-pipeline", "foo", tb.PipelineSpec( + tb.PipelineTask("hello-world-1", "hello-world"), + tb.PipelineTimeout(&metav1.Duration{Duration: 12 * time.Hour}), + ))} + prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-run-with-timeout", "foo", + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunServiceAccount("test-sa"), + ), + tb.PipelineRunStatus( + tb.PipelineRunStartTime(time.Now().AddDate(0, 0, -1))), + )} + ts := []*v1alpha1.Task{tb.Task("hello-world", "foo")} + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + } + + // create fake recorder for testing + fr := record.NewFakeRecorder(1) + + testAssets := getPipelineRunController(d, fr) + c := testAssets.Controller + clients := testAssets.Clients + + c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-run-with-timeout") + + // make sure there is no failed events + validateEvents(t, fr) + + // Check that the PipelineRun was reconciled correctly + reconciledRun, err := clients.Pipeline.Pipeline().PipelineRuns("foo").Get("test-pipeline-run-with-timeout", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Somehow had error getting completed reconciled run out of fake client: %s", err) + } + + // The PipelineRun should be timed out. + if reconciledRun.Status.GetCondition(duckv1alpha1.ConditionSucceeded).Reason != resources.ReasonTimedOut { + t.Errorf("Expected PipelineRun to be timed out, but condition reason is %s", reconciledRun.Status.GetCondition(duckv1alpha1.ConditionSucceeded)) + } + + // 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.") + } + + // There should be a time out for the TaskRun + if actual.Spec.TaskSpec.Timeout == nil { + t.Errorf("TaskSpec.Timeout shouldn't be nil") + } + + // The TaskRun's timeout date should be the same as the PipelineRun's start time plus the PipelineRun timeout duration + if actual.CreationTimestamp.Time.Add(actual.Spec.TaskSpec.Timeout.Duration) != reconciledRun.Status.StartTime.Add(ps[0].Spec.Timeout.Duration) { + t.Errorf("run start: %s, run timeout: %s, task timeout: %s", reconciledRun.Status.StartTime.String(), ps[0].Spec.Timeout.String(), + actual.Spec.TaskSpec.Timeout.Duration.String()) + } +} diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go index 8ded24b6855..9e8749f0e88 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go @@ -18,14 +18,15 @@ package resources import ( "fmt" + "time" + "github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/knative/build-pipeline/pkg/reconciler/v1alpha1/taskrun/resources" duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1" "go.uber.org/zap" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - - "github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1" - "github.com/knative/build-pipeline/pkg/reconciler/v1alpha1/taskrun/resources" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -39,6 +40,10 @@ const ( // ReasonSucceeded indicates that the reason for the finished status is that all of the TaskRuns // completed successfully ReasonSucceeded = "Succeeded" + + // ReasonTimedOut indicates that the PipelineRun has taken longer than its configured + // timeout + ReasonTimedOut = "PipelineRunTimeout" ) // GetNextTask returns the next Task for which a TaskRun should be created, @@ -188,7 +193,8 @@ func getTaskRunName(prName string, pt *v1alpha1.PipelineTask) string { // GetPipelineConditionStatus will return the Condition that the PipelineRun prName should be // updated with, based on the status of the TaskRuns in state. -func GetPipelineConditionStatus(prName string, state []*ResolvedPipelineRunTask, logger *zap.SugaredLogger) *duckv1alpha1.Condition { +func GetPipelineConditionStatus(prName string, state []*ResolvedPipelineRunTask, logger *zap.SugaredLogger, startTime *metav1.Time, + pipelineTimeout *metav1.Duration) *duckv1alpha1.Condition { allFinished := true for _, rprt := range state { if rprt.TaskRun == nil { @@ -198,7 +204,7 @@ func GetPipelineConditionStatus(prName string, state []*ResolvedPipelineRunTask, } c := rprt.TaskRun.Status.GetCondition(duckv1alpha1.ConditionSucceeded) if c == nil { - logger.Infof("TaskRun %s doens't have a condition, so PipelineRun %s isn't finished", rprt.TaskRunName, prName) + logger.Infof("TaskRun %s doesn't have a condition, so PipelineRun %s isn't finished", rprt.TaskRunName, prName) allFinished = false break } @@ -218,6 +224,22 @@ func GetPipelineConditionStatus(prName string, state []*ResolvedPipelineRunTask, } } if !allFinished { + if !startTime.IsZero() && pipelineTimeout != nil { + timeout := pipelineTimeout.Duration + runtime := time.Since(startTime.Time) + if runtime > timeout { + logger.Infof("PipelineRun %q has timed out(runtime %s over %s)", prName, runtime, timeout) + + timeoutMsg := fmt.Sprintf("PipelineRun %q failed to finish within %q", prName, timeout.String()) + return &duckv1alpha1.Condition{ + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: ReasonTimedOut, + Message: timeoutMsg, + } + } + } + logger.Infof("PipelineRun %s still has running TaskRuns so it isn't yet done", prName) return &duckv1alpha1.Condition{ Type: duckv1alpha1.ConditionSucceeded, @@ -226,6 +248,7 @@ func GetPipelineConditionStatus(prName string, state []*ResolvedPipelineRunTask, Message: "Not all Tasks in the Pipeline have finished executing", } } + logger.Infof("All TaskRuns have finished for PipelineRun %s so it has finished", prName) return &duckv1alpha1.Condition{ Type: duckv1alpha1.ConditionSucceeded, diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go index b21ad59fa9e..09d51bc111d 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go @@ -20,6 +20,7 @@ import ( "fmt" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -556,7 +557,8 @@ func TestGetPipelineConditionStatus(t *testing.T) { } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - c := GetPipelineConditionStatus("somepipelinerun", tc.state, zap.NewNop().Sugar()) + c := GetPipelineConditionStatus("somepipelinerun", tc.state, zap.NewNop().Sugar(), &metav1.Time{time.Now()}, + nil) if c.Status != tc.expectedStatus { t.Fatalf("Expected to get status %s but got %s for state %v", tc.expectedStatus, c.Status, tc.state) } diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun.go b/pkg/reconciler/v1alpha1/taskrun/taskrun.go index aef0bc77061..80f23d2a0e5 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun.go @@ -63,6 +63,9 @@ const ( // is just starting to be reconciled reasonRunning = "Running" + // reasonTimedOut indicates that the TaskRun has taken longer than its configured timeout + reasonTimedOut = "TaskRunTimeout" + // taskRunAgentName defines logging agent name for TaskRun Controller taskRunAgentName = "taskrun-controller" // taskRunControllerName defines name for TaskRun Controller @@ -308,6 +311,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error updateStatusFromBuildStatus(tr, buildStatus) after := tr.Status.GetCondition(duckv1alpha1.ConditionSucceeded) + reconciler.EmitEvent(c.Recorder, before, after, tr) c.Logger.Infof("Successfully reconciled taskrun %s/%s with status: %#v", tr.Name, tr.Namespace, after) diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go index 82fe52514ca..3d2977cffcb 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go @@ -825,6 +825,7 @@ func TestReconcileOnCancelledTaskRun(t *testing.T) { if err != nil { t.Fatalf("Expected completed TaskRun %s to exist but instead got error when getting it: %v", taskRun.Name, err) } + expectedStatus := &duckv1alpha1.Condition{ Type: duckv1alpha1.ConditionSucceeded, Status: corev1.ConditionFalse, diff --git a/test/builder/pipeline.go b/test/builder/pipeline.go index 411811ac695..ed3eedf7a56 100644 --- a/test/builder/pipeline.go +++ b/test/builder/pipeline.go @@ -17,6 +17,7 @@ import ( "github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1" duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "time" ) // PipelineOp is an operation which modify a Pipeline struct. @@ -102,6 +103,13 @@ func PipelineTask(name, taskName string, ops ...PipelineTaskOp) PipelineSpecOp { } } +// PipelineTimeout sets the timeout to the PipelineSpec. +func PipelineTimeout(duration *metav1.Duration) PipelineSpecOp { + return func(ps *v1alpha1.PipelineSpec) { + ps.Timeout = duration + } +} + // PipelineTaskRefKind sets the TaskKind to the PipelineTaskRef. func PipelineTaskRefKind(kind v1alpha1.TaskKind) PipelineTaskOp { return func(pt *v1alpha1.PipelineTask) { @@ -242,6 +250,13 @@ func PipelineRunStatusCondition(condition duckv1alpha1.Condition) PipelineRunSta } } +// PipelineRunStartTime sets the start time to the PipelineRunStatus. +func PipelineRunStartTime(startTime time.Time) PipelineRunStatusOp { + return func(s *v1alpha1.PipelineRunStatus) { + s.StartTime = &metav1.Time{Time: startTime} + } +} + // PipelineResource creates a PipelineResource with default values. // Any number of PipelineResource modifier can be passed to transform it. func PipelineResource(name, namespace string, ops ...PipelineResourceOp) *v1alpha1.PipelineResource { diff --git a/test/builder/pipeline_test.go b/test/builder/pipeline_test.go index 5885356a2f2..decf89fa1af 100644 --- a/test/builder/pipeline_test.go +++ b/test/builder/pipeline_test.go @@ -15,6 +15,7 @@ package builder_test import ( "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1" @@ -32,6 +33,7 @@ func TestPipeline(t *testing.T) { tb.PipelineTaskRefKind(v1alpha1.ClusterTaskKind), tb.PipelineTaskResourceDependency("i-am", tb.ProvidedBy("foo")), ), + tb.PipelineTimeout(&metav1.Duration{Duration: 1 * time.Hour}), )) expectedPipeline := &v1alpha1.Pipeline{ ObjectMeta: metav1.ObjectMeta{Name: "tomatoes", Namespace: "foo"}, @@ -45,6 +47,7 @@ func TestPipeline(t *testing.T) { TaskRef: v1alpha1.TaskRef{Name: "chocolate", Kind: v1alpha1.ClusterTaskKind}, ResourceDependencies: []v1alpha1.ResourceDependency{{Name: "i-am", ProvidedBy: []string{"foo"}}}, }}, + Timeout: &metav1.Duration{Duration: 1 * time.Hour}, }, } if d := cmp.Diff(expectedPipeline, pipeline); d != "" { @@ -53,15 +56,19 @@ func TestPipeline(t *testing.T) { } func TestPipelineRun(t *testing.T) { + startTime := time.Now() pipelineRun := tb.PipelineRun("pear", "foo", tb.PipelineRunSpec( "tomatoes", tb.PipelineRunServiceAccount("sa"), tb.PipelineRunTaskResource("res1", tb.PipelineTaskResourceInputs("inputs"), tb.PipelineTaskResourceOutputs("outputs"), ), - ), tb.PipelineRunStatus(tb.PipelineRunStatusCondition(duckv1alpha1.Condition{ - Type: duckv1alpha1.ConditionSucceeded, - }))) + ), tb.PipelineRunStatus( + tb.PipelineRunStatusCondition(duckv1alpha1.Condition{ + Type: duckv1alpha1.ConditionSucceeded, + }), + tb.PipelineRunStartTime(startTime), + )) expectedPipelineRun := &v1alpha1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{Name: "pear", Namespace: "foo"}, Spec: v1alpha1.PipelineRunSpec{ @@ -76,6 +83,7 @@ func TestPipelineRun(t *testing.T) { }, Status: v1alpha1.PipelineRunStatus{ Conditions: []duckv1alpha1.Condition{{Type: duckv1alpha1.ConditionSucceeded}}, + StartTime: &metav1.Time{Time: startTime}, }, } if d := cmp.Diff(expectedPipelineRun, pipelineRun); d != "" { diff --git a/test/builder/task.go b/test/builder/task.go index 57a9cc6365c..60d45dfc3cc 100644 --- a/test/builder/task.go +++ b/test/builder/task.go @@ -312,6 +312,13 @@ func StepState(ops ...StepStateOp) TaskRunStatusOp { } } +// TaskRunStartTime sets the start time to the TaskRunStatus. +func TaskRunStartTime(startTime time.Time) TaskRunStatusOp { + return func(s *v1alpha1.TaskRunStatus) { + s.StartTime = &metav1.Time{Time: startTime} + } +} + // StateTerminated set Terminated to the StepState. func StateTerminated(exitcode int) StepStateOp { return func(s *v1alpha1.StepState) {