Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TEP-0114: Implements Retries in The Testing Wait Custom Task #5435

Merged
merged 1 commit into from
Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/custom-task-ctrls/wait-task/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ wait custom task removed.
It provides a [Tekton Custom
Task](https://tekton.dev/docs/pipelines/runs/) that, when run, simply waits a
given amount of time before succeeding, specified by an input parameter named
`duration`.
`duration`. It also supports `timeout` and `retries`.
21 changes: 21 additions & 0 deletions test/custom-task-ctrls/wait-task/pkg/reconciler/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,30 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, r *v1alpha1.Run) kreconc
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)

// Retry if the current RetriesStatus hasn't reached the retries limit
XinruZhang marked this conversation as resolved.
Show resolved Hide resolved
if r.Spec.Retries > len(r.Status.RetriesStatus) {
logger.Infof("Run timed out, retrying... %#v", r.Status)
retryRun(r)
return controller.NewRequeueImmediately()
}

return nil
}

// Don't emit events on nop-reconciliations, it causes scale problems.
return nil
}

func retryRun(run *v1alpha1.Run) {
// Add retry history
newStatus := *run.Status.DeepCopy()
newStatus.RetriesStatus = nil
run.Status.RetriesStatus = append(run.Status.RetriesStatus, newStatus)

// Clear status
run.Status.StartTime = nil
run.Status.CompletionTime = nil

run.Status.MarkRunRunning("", "")
}
200 changes: 158 additions & 42 deletions test/custom-task-ctrls/wait-task/pkg/reconciler/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestReconcile(t *testing.T) {
wantRunConditionType apis.ConditionType
wantRunConditionStatus corev1.ConditionStatus
wantRunConditionReason string
isParentPRCancelled bool
isCancelled bool
}{{
name: "duration elapsed",
params: `
Expand All @@ -84,13 +84,13 @@ func TestReconcile(t *testing.T) {
wantRunConditionType: apis.ConditionSucceeded,
wantRunConditionStatus: corev1.ConditionFalse,
wantRunConditionReason: "UnexpectedName",
isParentPRCancelled: false,
isCancelled: false,
}, {
name: "no duration param",
wantRunConditionType: apis.ConditionSucceeded,
wantRunConditionStatus: corev1.ConditionFalse,
wantRunConditionReason: "MissingDuration",
isParentPRCancelled: false,
isCancelled: false,
}, {
name: "extra param",
params: `
Expand All @@ -102,7 +102,7 @@ func TestReconcile(t *testing.T) {
`, wantRunConditionType: apis.ConditionSucceeded,
wantRunConditionStatus: corev1.ConditionFalse,
wantRunConditionReason: "UnexpectedParams",
isParentPRCancelled: false,
isCancelled: false,
}, {
name: "duration param is not a string",
params: `
Expand All @@ -116,7 +116,7 @@ func TestReconcile(t *testing.T) {
wantRunConditionType: apis.ConditionSucceeded,
wantRunConditionStatus: corev1.ConditionFalse,
wantRunConditionReason: "MissingDuration",
isParentPRCancelled: false,
isCancelled: false,
}, {
name: "invalid duration value",
params: `
Expand All @@ -127,7 +127,7 @@ func TestReconcile(t *testing.T) {
wantRunConditionType: apis.ConditionSucceeded,
wantRunConditionStatus: corev1.ConditionFalse,
wantRunConditionReason: "InvalidDuration",
isParentPRCancelled: false,
isCancelled: false,
}, {
name: "timeout",
timeout: "1s",
Expand All @@ -139,7 +139,7 @@ func TestReconcile(t *testing.T) {
wantRunConditionType: apis.ConditionSucceeded,
wantRunConditionStatus: corev1.ConditionFalse,
wantRunConditionReason: "TimedOut",
isParentPRCancelled: false,
isCancelled: false,
}, {
name: "timeout equals duration",
timeout: "1s",
Expand All @@ -151,7 +151,7 @@ func TestReconcile(t *testing.T) {
wantRunConditionType: apis.ConditionSucceeded,
wantRunConditionStatus: corev1.ConditionFalse,
wantRunConditionReason: "InvalidTimeOut",
isParentPRCancelled: false,
isCancelled: false,
}, {
name: "parent pr timeout",
timeout: "1s",
Expand All @@ -163,7 +163,7 @@ func TestReconcile(t *testing.T) {
wantRunConditionType: apis.ConditionSucceeded,
wantRunConditionStatus: corev1.ConditionFalse,
wantRunConditionReason: "Cancelled",
isParentPRCancelled: true,
isCancelled: true,
}} {
t.Run(tc.name, func(t *testing.T) {
ctx := context.Background()
Expand All @@ -186,40 +186,11 @@ spec:
runYAML = runYAML + tc.params
}
r := parse.MustParseRun(t, runYAML)
if tc.isParentPRCancelled {
if tc.isCancelled {
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)
}
}
fakeReconcile(t, ctx, rec, r)

// Compose expected Run
wantRunYAML := fmt.Sprintf(`
Expand All @@ -244,9 +215,8 @@ status:
observedGeneration: 0
`, tc.wantRunConditionReason, tc.wantRunConditionStatus, tc.wantRunConditionType)
wantRun := parse.MustParseRun(t, wantRunYAML)
if tc.isParentPRCancelled {
if tc.isCancelled {
wantRun.Spec.Status = v1alpha1.RunSpecStatusCancelled
wantRun.Spec.StatusMessage = v1alpha1.RunCancelledByPipelineMsg
}

if d := cmp.Diff(wantRun, r,
Expand All @@ -260,3 +230,149 @@ status:
})
}
}

func TestReconcile_Retries(t *testing.T) {
for _, tc := range []struct {
name string
duration string
timeout string
retries int
params string
wantStatus string
isCancelled bool
}{{
name: "retry when timeout",
duration: "2s",
timeout: "1s",
retries: 1,
wantStatus: fmt.Sprintf(`
status:
conditions:
- reason: %s
status: %q
type: %s
observedGeneration: 0
retriesStatus:
- conditions:
- reason: %s
status: %q
type: %s
`, "TimedOut", corev1.ConditionFalse, apis.ConditionSucceeded, "TimedOut", corev1.ConditionFalse, apis.ConditionSucceeded),
isCancelled: false,
}, {
name: "don't retry if retries unspecified",
duration: "2s",
timeout: "1s",
wantStatus: fmt.Sprintf(`
status:
conditions:
- reason: %s
status: %q
type: %s
observedGeneration: 0
`, "TimedOut", corev1.ConditionFalse, apis.ConditionSucceeded),
isCancelled: false,
}, {
name: "don't retry when canceled",
duration: "2s",
wantStatus: fmt.Sprintf(`
status:
conditions:
- reason: %s
status: %q
type: %s
observedGeneration: 0
`, "Cancelled", corev1.ConditionFalse, apis.ConditionSucceeded),
isCancelled: 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:
retries: %d
timeout: %s
ref:
apiVersion: %s
kind: %s
params:
- name: duration
value: %s
`, runName, tc.retries, tc.timeout, apiVersion, kind, tc.duration)
r := parse.MustParseRun(t, runYAML)
if tc.isCancelled {
r.Spec.Status = v1alpha1.RunSpecStatusCancelled
}

fakeReconcile(t, ctx, rec, r)

// Compose expected Run
wantRunYAML := fmt.Sprintf(`
metadata:
name: %s
spec:
retries: %d
timeout: %s
ref:
apiVersion: %s
kind: %s
params:
- name: duration
value: %s
`, runName, tc.retries, tc.timeout, apiVersion, kind, tc.duration)
wantRunYAML = wantRunYAML + tc.wantStatus
wantRun := parse.MustParseRun(t, wantRunYAML)
if tc.isCancelled {
wantRun.Spec.Status = v1alpha1.RunSpecStatusCancelled
}

if d := cmp.Diff(wantRun, r,
filterTypeMeta,
filterObjectMeta,
filterCondition,
filterRunStatus,
); d != "" {
t.Errorf("-got +want: %v", d)
jerop marked this conversation as resolved.
Show resolved Hide resolved
}
})
}
}

// TODO(#5456) Avoid duplicate reconcile logic
func fakeReconcile(t *testing.T, ctx context.Context, rec *Reconciler, r *v1alpha1.Run) {
XinruZhang marked this conversation as resolved.
Show resolved Hide resolved
t.Helper()

// Start reconciling the Run.
// This will not return until the second Reconcile is done.
err := rec.ReconcileKind(ctx, 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)
}
}
}