TEP-0114: Implements Timeout of the Testing Wait Custom Task

Prior to this commit, the testing custom task wait-task, under test/wait-task,
can only wait for a duration of time. This commit adds a new functionality
timeout to its controller, so that we can test the behavior of Custom
Task Run time out.

test/wait-task/cmd/controller/main.go → test/custom-task-ctrls/wait-task/cmd/controller/main.go

@@ -23,6 +23,7 @@ import (
 	runreconciler "github.com/tektoncd/pipeline/pkg/client/injection/reconciler/pipeline/v1alpha1/run"
 	tkncontroller "github.com/tektoncd/pipeline/pkg/controller"
 	"github.com/tektoncd/pipeline/test/wait-task/pkg/reconciler"
+	"k8s.io/apimachinery/pkg/util/clock"
 	"k8s.io/client-go/tools/cache"
 	"knative.dev/pkg/configmap"
 	"knative.dev/pkg/controller"
@@ -36,7 +37,9 @@ func main() {
 }
 
 func newController(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
-	c := &reconciler.Reconciler{}
+	c := &reconciler.Reconciler{
+		Clock: clock.RealClock{},
+	}
 	impl := runreconciler.NewImpl(ctx, c, func(impl *controller.Impl) controller.Options {
 		return controller.Options{
 			AgentName: controllerName,

test/custom-task-ctrls/wait-task/go.mod

@@ -0,0 +1,12 @@
+module github.com/tektoncd/pipeline/test/wait-task
+
+go 1.15
+
+require (
+	github.com/google/go-cmp v0.5.8
+	github.com/tektoncd/pipeline v0.39.0
+	k8s.io/api v0.23.9
+	k8s.io/apimachinery v0.23.9
+	k8s.io/client-go v0.23.9
+	knative.dev/pkg v0.0.0-20220805012121-7b8b06028e4f
+)

test/wait-task/pkg/reconciler/reconciler.go → test/custom-task-ctrls/wait-task/pkg/reconciler/reconciler.go

@@ -18,16 +18,23 @@ package reconciler
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/clock"
 	"knative.dev/pkg/controller"
 	"knative.dev/pkg/logging"
 	kreconciler "knative.dev/pkg/reconciler"
 )
 
-type Reconciler struct{}
+const WaitTaskCancelledByRunTimeoutMsg string = "Wait Task cancelled as it times out."
+
+// Reconciler implements controller.Reconciler for Configuration resources.
+type Reconciler struct {
+	Clock clock.PassiveClock
+}
 
 // ReconcileKind implements Interface.ReconcileKind.
 func (c *Reconciler) ReconcileKind(ctx context.Context, r *v1alpha1.Run) kreconciler.Event {
@@ -67,27 +74,60 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, r *v1alpha1.Run) kreconc
 		return nil
 	}
 
-	dur, err := time.ParseDuration(expr.Value.StringVal)
-	if err != nil {
-		r.Status.MarkRunFailed("InvalidDuration", "The duration param was invalid: %v", err)
+	// Skip if the Run is cancelled.
+	if r.IsCancelled() {
+		logger.Infof("The Custom Task Run %v has been cancelled", r.GetName())
+		r.Status.CompletionTime = &metav1.Time{Time: c.Clock.Now()}
+		var msg string = fmt.Sprint(r.Spec.StatusMessage)
+		if msg == "" {
+			msg = "The Wait Task is cancelled"
+		}
+		r.Status.MarkRunFailed("Cancelled", msg)
 		return nil
 	}
 
-	if r.Status.StartTime == nil {
-		now := metav1.Now()
-		r.Status.StartTime = &now
+	if !r.HasStarted() {
+		logger.Info("Run hasn't started, start it")
+		r.Status.InitializeConditions()
+		r.Status.StartTime = &metav1.Time{Time: c.Clock.Now()}
 		r.Status.MarkRunRunning("Running", "Waiting for duration to elapse")
 	}
 
-	done := r.Status.StartTime.Time.Add(dur)
+	duration, err := time.ParseDuration(expr.Value.StringVal)
+	if err != nil {
+		r.Status.MarkRunFailed("InvalidDuration", "The duration param was invalid: %v", err)
+		return nil
+	}
+	timeout := r.GetTimeout()
+	if duration == timeout {
+		r.Status.MarkRunFailed("InvalidTimeOut", "Spec.Timeout shouldn't equal duration")
+		return nil
+	}
+	elapsed := c.Clock.Since(r.Status.StartTime.Time)
+
+	// Custom Task is running and not timed out
+	if r.Status.StartTime != nil && elapsed <= duration && elapsed <= timeout {
+		logger.Infof("The Custom Task Run %s is running", r.GetName())
+		waitTime := duration.Nanoseconds()
+		if timeout.Nanoseconds() < waitTime {
+			waitTime = timeout.Nanoseconds()
+		}
+		return controller.NewRequeueAfter(time.Duration(waitTime))
+	}
 
-	if time.Now().After(done) {
-		now := metav1.Now()
-		r.Status.CompletionTime = &now
+	if r.Status.StartTime != nil && elapsed > duration && elapsed <= timeout {
+		logger.Infof("The Custom Task Run %v finished", r.GetName())
+		r.Status.CompletionTime = &metav1.Time{Time: c.Clock.Now()}
 		r.Status.MarkRunSucceeded("DurationElapsed", "The wait duration has elapsed")
-	} else {
-		// Enqueue another check when the timeout should be elapsed.
-		return controller.NewRequeueAfter(time.Until(r.Status.StartTime.Time.Add(dur)))
+		return nil
+	}
+
+	// Custom Task timed out
+	if r.Status.StartTime != nil && elapsed > timeout {
+		logger.Infof("The Custom Task Run %v timed out", r.GetName())
+		r.Status.CompletionTime = &metav1.Time{Time: c.Clock.Now()}
+		r.Status.MarkRunFailed("TimedOut", WaitTaskCancelledByRunTimeoutMsg)
+		return nil
 	}
 
 	// Don't emit events on nop-reconciliations, it causes scale problems.

test/custom-task-ctrls/wait-task/pkg/reconciler/reconciler_test.go

@@ -0,0 +1,262 @@
+/*
+Copyright 2021 The Tekton Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package reconciler
+
+import (
+	"context"
+	"fmt"
+	"math"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
+	"github.com/tektoncd/pipeline/test/parse"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/clock"
+	"knative.dev/pkg/apis"
+	"knative.dev/pkg/test/helpers"
+)
+
+const (
+	apiVersion string = "wait.testing.tekton.dev/v1alpha1"
+	kind       string = "Wait"
+)
+
+var (
+	filterTypeMeta          = cmpopts.IgnoreFields(metav1.TypeMeta{}, "Kind", "APIVersion")
+	filterObjectMeta        = cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ResourceVersion", "UID", "CreationTimestamp", "Generation", "ManagedFields")
+	filterCondition         = cmpopts.IgnoreFields(apis.Condition{}, "LastTransitionTime.Inner.Time", "Message")
+	filterRunStatus         = cmpopts.IgnoreFields(v1alpha1.RunStatusFields{}, "StartTime", "CompletionTime")
+	filterPipelineRunStatus = cmpopts.IgnoreFields(v1beta1.PipelineRunStatusFields{}, "StartTime", "CompletionTime")
+
+	now       = time.Date(2022, time.January, 1, 0, 0, 0, 0, time.UTC)
+	testClock = clock.NewFakePassiveClock(now)
+)
+
+func TestReconcile(t *testing.T) {
+	t.Parallel()
+	for _, tc := range []struct {
+		name                   string
+		refName                string
+		timeout                string
+		params                 string
+		wantRunConditionType   apis.ConditionType
+		wantRunConditionStatus corev1.ConditionStatus
+		wantRunConditionReason string
+		isParentPRCancelled    bool
+	}{{
+		name: "duration elapsed",
+		params: `
+  params:
+  - name: duration
+    value: 1s
+`,
+		wantRunConditionType:   apis.ConditionSucceeded,
+		wantRunConditionStatus: corev1.ConditionTrue,
+		wantRunConditionReason: "DurationElapsed",
+	}, {
+		name:    "unexpected ref name",
+		refName: "meow",
+		params: `
+  params:
+  - name: duration
+    value: 1s
+`,
+		wantRunConditionType:   apis.ConditionSucceeded,
+		wantRunConditionStatus: corev1.ConditionFalse,
+		wantRunConditionReason: "UnexpectedName",
+		isParentPRCancelled:    false,
+	}, {
+		name:                   "no duration param",
+		wantRunConditionType:   apis.ConditionSucceeded,
+		wantRunConditionStatus: corev1.ConditionFalse,
+		wantRunConditionReason: "MissingDuration",
+		isParentPRCancelled:    false,
+	}, {
+		name: "extra param",
+		params: `
+  params:
+  - name: duration
+    value: 1s
+  - name: not-duration
+    value: blah
+`, wantRunConditionType: apis.ConditionSucceeded,
+		wantRunConditionStatus: corev1.ConditionFalse,
+		wantRunConditionReason: "UnexpectedParams",
+		isParentPRCancelled:    false,
+	}, {
+		name: "duration param is not a string",
+		params: `
+  params:
+  - name: duration
+    value:
+    - blah
+    - blah
+    - blah
+`,
+		wantRunConditionType:   apis.ConditionSucceeded,
+		wantRunConditionStatus: corev1.ConditionFalse,
+		wantRunConditionReason: "MissingDuration",
+		isParentPRCancelled:    false,
+	}, {
+		name: "invalid duration value",
+		params: `
+  params:
+  - name: duration
+    value: blah
+`,
+		wantRunConditionType:   apis.ConditionSucceeded,
+		wantRunConditionStatus: corev1.ConditionFalse,
+		wantRunConditionReason: "InvalidDuration",
+		isParentPRCancelled:    false,
+	}, {
+		name:    "timeout",
+		timeout: "1s",
+		params: `
+  params:
+  - name: duration
+    value: 2s
+`,
+		wantRunConditionType:   apis.ConditionSucceeded,
+		wantRunConditionStatus: corev1.ConditionFalse,
+		wantRunConditionReason: "TimedOut",
+		isParentPRCancelled:    false,
+	}, {
+		name:    "timeout equals duration",
+		timeout: "1s",
+		params: `
+  params:
+  - name: duration
+    value: 1s
+`,
+		wantRunConditionType:   apis.ConditionSucceeded,
+		wantRunConditionStatus: corev1.ConditionFalse,
+		wantRunConditionReason: "InvalidTimeOut",
+		isParentPRCancelled:    false,
+	}, {
+		name:    "parent pr timeout",
+		timeout: "1s",
+		params: `
+  params:
+  - name: duration
+    value: 2s
+`,
+		wantRunConditionType:   apis.ConditionSucceeded,
+		wantRunConditionStatus: corev1.ConditionFalse,
+		wantRunConditionReason: "Cancelled",
+		isParentPRCancelled:    true,
+	}} {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			rec := &Reconciler{
+				Clock: testClock,
+			}
+
+			runName := helpers.ObjectNameForTest(t)
+			runYAML := fmt.Sprintf(`
+metadata:
+  name: %s
+spec:
+  timeout: %s
+  ref:
+    apiVersion: %s
+    kind: %s
+    name: %s
+`, runName, tc.timeout, apiVersion, kind, tc.refName)
+			if tc.params != "" {
+				runYAML = runYAML + tc.params
+			}
+			r := parse.MustParseRun(t, runYAML)
+			if tc.isParentPRCancelled {
+				r.Spec.Status = v1alpha1.RunSpecStatusCancelled
+				r.Spec.StatusMessage = v1alpha1.RunCancelledByPipelineMsg
+			}
+
+			// Start reconciling the Run.
+			// This will not return until the second Reconcile is done.
+			err := rec.ReconcileKind(ctx, r)
+
+			t.Logf("%#v", r)
+
+			for err != nil {
+				if strings.Contains(err.Error(), "requeue") {
+					// simulate EnqueueAfter
+					var dur time.Duration
+					var dr error
+					for _, p := range r.Spec.Params {
+						if p.Name == "duration" {
+							dur, dr = time.ParseDuration(p.Value.StringVal)
+							if dr != nil {
+								t.Error("failed to parse duration")
+							}
+							break
+						}
+					}
+					to := r.GetTimeout()
+					sleep := int(math.Min(to.Seconds(), dur.Seconds()))
+					testClock.SetTime(testClock.Now().Add(time.Duration(sleep) * time.Second))
+					rec.Clock = testClock
+					err = rec.ReconcileKind(ctx, r)
+				} else {
+					t.Fatalf("ReconcileKind() = %v", err)
+				}
+			}
+
+			// Compose expected Run
+			wantRunYAML := fmt.Sprintf(`
+metadata:
+  name: %s
+spec:
+  timeout: %s
+  ref:
+    apiVersion: %s
+    kind: %s
+    name: %s
+`, runName, tc.timeout, apiVersion, kind, tc.refName)
+			if tc.params != "" {
+				wantRunYAML = wantRunYAML + tc.params
+			}
+			wantRunYAML = wantRunYAML + fmt.Sprintf(`
+status:
+  conditions:
+  - reason: %s
+    status: %q
+    type: %s
+  observedGeneration: 0
+`, tc.wantRunConditionReason, tc.wantRunConditionStatus, tc.wantRunConditionType)
+			wantRun := parse.MustParseRun(t, wantRunYAML)
+			if tc.isParentPRCancelled {
+				wantRun.Spec.Status = v1alpha1.RunSpecStatusCancelled
+				wantRun.Spec.StatusMessage = v1alpha1.RunCancelledByPipelineMsg
+			}
+
+			if d := cmp.Diff(wantRun, r,
+				filterTypeMeta,
+				filterObjectMeta,
+				filterCondition,
+				filterRunStatus,
+			); d != "" {
+				t.Errorf("-got +want: %v", d)
+			}
+		})
+	}
+}