Fix watching for pipelinerun timeout

chmouel · chmouel · commit 3e0d392ae94e · 2022-04-11T14:37:34.000+02:00
First we define a ridiculous large number for taskrun timeout, so the
pac taskrun doesn't timeout before the user defined one.

Second we are making sure we are not using the timeout configuration
instead of the constant (which was 10minutes by default)

Third we are not just warning anymore but actually exit.

Signed-off-by: Chmouel Boudjnah &lt;chmouel@redhat.com&gt;
diff --git a/config/401-github-issue-recheck.yaml b/config/401-github-issue-recheck.yaml
@@ -88,6 +88,7 @@ spec:
           pipelinesascode.tekton.dev/event: $(tt.params.event_type)
       spec:
         serviceAccountName: pipelines-as-code-sa-el
+        timeout: 24h0m0s
         params:
           - name: action
             value: $(tt.params.action)
diff --git a/config/402-github-pull-request.yaml b/config/402-github-pull-request.yaml
@@ -89,6 +89,7 @@ spec:
           app.kubernetes.io/managed-by: pipelines-as-code
           pipelinesascode.tekton.dev/event: $(tt.params.event_type)
       spec:
+        timeout: 24h0m0s
         serviceAccountName: pipelines-as-code-sa-el
         params:
           - name: ghe_host
diff --git a/config/403-github-push.yaml b/config/403-github-push.yaml
@@ -84,6 +84,7 @@ spec:
           app.kubernetes.io/managed-by: pipelines-as-code
           pipelinesascode.tekton.dev/event: $(tt.params.event_type)
       spec:
+        timeout: 24h0m0s
         serviceAccountName: pipelines-as-code-sa-el
         params:
           - name: ghe_host
diff --git a/config/404-github-retest-comment.yaml b/config/404-github-retest-comment.yaml
@@ -74,6 +74,7 @@ spec:
           app.kubernetes.io/managed-by: pipelines-as-code
           pipelinesascode.tekton.dev/event: $(tt.params.event_type)
       spec:
+        timeout: 24h0m0s
         serviceAccountName: pipelines-as-code-sa-el
         params:
           - name: ghe_host
diff --git a/config/422-bitbucket-cloud-pull-request.yaml b/config/422-bitbucket-cloud-pull-request.yaml
@@ -97,6 +97,7 @@ spec:
           # pipelinesascode.tekton.dev/event: $(tt.params.event_type)
       spec:
         serviceAccountName: pipelines-as-code-sa-el
+        timeout: 24h0m0s
         params:
           - name: source_ip
             value: $(tt.params.source_ip)
diff --git a/config/423-bitbucket-cloud-push.yaml b/config/423-bitbucket-cloud-push.yaml
@@ -79,6 +79,7 @@ spec:
           app.kubernetes.io/managed-by: pipelines-as-code
       spec:
         serviceAccountName: pipelines-as-code-sa-el
+        timeout: 24h0m0s
         params:
           - name: source_ip
             value: $(tt.params.source_ip)
diff --git a/config/442-bitbucket-server-pull-request.yaml b/config/442-bitbucket-server-pull-request.yaml
@@ -122,6 +122,7 @@ spec:
           app.kubernetes.io/version: "devel"
           app.kubernetes.io/managed-by: pipelines-as-code
       spec:
+        timeout: 24h0m0s
         serviceAccountName: pipelines-as-code-sa-el
         params:
           - name: trigger_target
diff --git a/config/443-bitbucket-server-push.yaml b/config/443-bitbucket-server-push.yaml
@@ -81,6 +81,7 @@ spec:
           app.kubernetes.io/version: "devel"
           app.kubernetes.io/managed-by: pipelines-as-code
       spec:
+        timeout: 24h0m0s
         serviceAccountName: pipelines-as-code-sa-el
         params:
           - name: "event_type"
diff --git a/docs/install.md b/docs/install.md
@@ -468,6 +468,18 @@ the `pipelines-as-code` namespace.
 
   <https://api.hub.tekton.dev/v1>
 
+* `default-pipelinerun-timeout`: Default timeout to wait for pipelinerun
+  (default value is 2 hours).
+  Note that PaaC doesn't respect the user pipelinerun timeout if it's greater than
+  this value.
+  In other words, if you set paac `default-pipelinerun-timeout` to 2h and
+  your PipelineRun timeout is set to 3h the PipelineRun will timeout after 2h.
+  If your pipelinerun timeout is set to 1h and your
+  `default-pipelinerun-timeout` value is 2h, the PipelineRun will timeout after
+  1h by the tekton controller.
+
+  This value cannot exceed 24 hours.
+
 ## Kubernetes
 
 Pipelines as Code should work directly on kubernetes/minikube/kind. You just need to install the release.yaml
diff --git a/pkg/kubeinteraction/cleanups.go b/pkg/kubeinteraction/cleanups.go
@@ -14,7 +14,8 @@ import (
 )
 
 func (k Interaction) CleanupPipelines(ctx context.Context, repo *v1alpha1.Repository, pr *v1beta1.PipelineRun,
-	maxKeep int) error {
+	maxKeep int,
+) error {
 	repoLabel := filepath.Join(pipelinesascode.GroupName, "repository")
 	originalPRLabel := filepath.Join(pipelinesascode.GroupName, "original-prname")
 	if _, ok := pr.GetLabels()[originalPRLabel]; !ok {
diff --git a/pkg/kubeinteraction/wait.go b/pkg/kubeinteraction/wait.go
@@ -14,8 +14,7 @@ import (
 )
 
 const (
-	interval = 1 * time.Second
-	timeout  = 10 * time.Minute
+	interval = 2 * time.Second
 )
 
 type ConditionAccessorFn func(ca knativeapi.ConditionAccessor) (bool, error)
@@ -83,11 +82,11 @@ func PipelineRunSucceed(name string) ConditionAccessorFn {
 	return Succeed(name)
 }
 
-func PollImmediateWithContext(ctx context.Context, fn func() (bool, error)) error {
-	return wait.PollImmediate(interval, timeout, func() (bool, error) {
+func PollImmediateWithContext(ctx context.Context, pollTimeout time.Duration, fn func() (bool, error)) error {
+	return wait.PollImmediate(interval, pollTimeout, func() (bool, error) {
 		select {
 		case <-ctx.Done():
-			return true, ctx.Err()
+			return true, fmt.Errorf("polling timed out, pipelinerun has exceeded its timeout: %v", pollTimeout)
 		default:
 		}
 		return fn()
@@ -101,7 +100,7 @@ func PollImmediateWithContext(ctx context.Context, fn func() (bool, error)) erro
 func waitForPipelineRunState(ctx context.Context, tektonbeta1 tektonv1beta1client.TektonV1beta1Interface, pr *v1beta1.PipelineRun, polltimeout time.Duration, inState ConditionAccessorFn) error {
 	ctx, cancel := context.WithTimeout(ctx, polltimeout)
 	defer cancel()
-	return PollImmediateWithContext(ctx, func() (bool, error) {
+	return PollImmediateWithContext(ctx, polltimeout, func() (bool, error) {
 		r, err := tektonbeta1.PipelineRuns(pr.Namespace).Get(ctx, pr.Name, metav1.GetOptions{})
 		if err != nil {
 			return true, err
diff --git a/pkg/matcher/annotation_tasks_install.go b/pkg/matcher/annotation_tasks_install.go
@@ -85,7 +85,8 @@ func (rt RemoteTasks) getTask(ctx context.Context, providerintf provider.Interfa
 
 // GetTaskFromAnnotations Get task remotely if they are on Annotations
 func (rt RemoteTasks) GetTaskFromAnnotations(ctx context.Context, providerintf provider.Interface,
-	annotations map[string]string) ([]*tektonv1beta1.Task, error) {
+	annotations map[string]string,
+) ([]*tektonv1beta1.Task, error) {
 	var ret []*tektonv1beta1.Task
 	rtareg := regexp.MustCompile(fmt.Sprintf("%s/%s", pipelinesascode.GroupName, taskAnnotationsRegexp))
 	for annotationK, annotationV := range annotations {
diff --git a/pkg/pipelineascode/pipelineascode.go b/pkg/pipelineascode/pipelineascode.go
@@ -164,8 +164,7 @@ func Run(ctx context.Context, cs *params.Run, providerintf provider.Interface, k
 	cs.Clients.Log.Infof("Waiting for PipelineRun %s/%s to Succeed in a maximum time of %s minutes",
 		pr.Namespace, pr.Name, formatting.HumanDuration(cs.Info.Pac.DefaultPipelineRunTimeout))
 	if err := k8int.WaitForPipelineRunSucceed(ctx, cs.Clients.Tekton.TektonV1beta1(), pr, cs.Info.Pac.DefaultPipelineRunTimeout); err != nil {
-		cs.Clients.Log.Warnf("pipelinerun %s in namespace %s has a failed status",
-			pipelineRun.GetGenerateName(), repo.GetNamespace())
+		return fmt.Errorf("pipelinerun %s in namespace %s has failed: %w", pipelineRun.GetGenerateName(), repo.GetNamespace(), err)
 	}
 
 	// Do cleanups
diff --git a/pkg/pipelineascode/pipelinesascode_github_test.go b/pkg/pipelineascode/pipelinesascode_github_test.go
@@ -70,7 +70,8 @@ func testSetupTektonDir(mux *http.ServeMux, runevent info.Event, directory strin
 }
 
 func testSetupCommonGhReplies(t *testing.T, mux *http.ServeMux, runevent info.Event,
-	finalStatus, finalStatusText string, noReplyOrgPublicMembers bool) {
+	finalStatus, finalStatusText string, noReplyOrgPublicMembers bool,
+) {
 	// Take a directory and generate replies as Github for it
 	replyString(mux,
 		fmt.Sprintf("/repos/%s/%s/contents/internal/task", runevent.Organization, runevent.Repository),
diff --git a/pkg/pipelineascode/status.go b/pkg/pipelineascode/status.go
@@ -56,7 +56,8 @@ func updateRepoRunStatus(ctx context.Context, cs *params.Run, pr *tektonv1beta1.
 }
 
 func postFinalStatus(ctx context.Context, cs *params.Run, providerintf provider.Interface, createdPR *tektonv1beta1.PipelineRun) (
-	*tektonv1beta1.PipelineRun, error) {
+	*tektonv1beta1.PipelineRun, error,
+) {
 	pr, err := cs.Clients.Tekton.TektonV1beta1().PipelineRuns(createdPR.GetNamespace()).Get(
 		ctx, createdPR.GetName(), metav1.GetOptions{},
 	)
diff --git a/pkg/resolve/resolve.go b/pkg/resolve/resolve.go
@@ -80,7 +80,8 @@ func skippingTask(taskName string, skippedTasks []string) bool {
 }
 
 func inlineTasks(tasks []tektonv1beta1.PipelineTask, ropt *Opts, types Types) ([]tektonv1beta1.PipelineTask,
-	error) {
+	error,
+) {
 	pipelineTasks := []tektonv1beta1.PipelineTask{}
 	for _, task := range tasks {
 		if task.TaskRef != nil && task.TaskRef.Bundle == "" &&
@@ -109,7 +110,8 @@ type Opts struct {
 // generateName can be set as True to set the name as a generateName + "-" for
 // unique pipelinerun
 func Resolve(ctx context.Context, cs *params.Run, providerintf provider.Interface, data string, ropt *Opts) (
-	[]*tektonv1beta1.PipelineRun, error) {
+	[]*tektonv1beta1.PipelineRun, error,
+) {
 	s := k8scheme.Scheme
 	if err := tektonv1beta1.AddToScheme(s); err != nil {
 		return []*tektonv1beta1.PipelineRun{}, err
diff --git a/test/pkg/wait/wait.go b/test/pkg/wait/wait.go
@@ -23,7 +23,7 @@ type Opts struct {
 func UntilRepositoryUpdated(ctx context.Context, clients clients.Clients, opts Opts) error {
 	ctx, cancel := context.WithTimeout(ctx, opts.PollTimeout)
 	defer cancel()
-	return kubeinteraction.PollImmediateWithContext(ctx, func() (bool, error) {
+	return kubeinteraction.PollImmediateWithContext(ctx, opts.PollTimeout, func() (bool, error) {
 		clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(opts.Namespace)
 		r, err := clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(opts.Namespace).Get(ctx, opts.RepoName,
 			metav1.GetOptions{})

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,8 @@ import (`
`14`	`14`	`)`
`15`	`15`
`16`	`16`	`func (k Interaction) CleanupPipelines(ctx context.Context, repo v1alpha1.Repository, pr v1beta1.PipelineRun,`
`17`		`- maxKeep int) error {`
	`17`	`+ maxKeep int,`
	`18`	`+) error {`
`18`	`19`	`repoLabel := filepath.Join(pipelinesascode.GroupName, "repository")`
`19`	`20`	`originalPRLabel := filepath.Join(pipelinesascode.GroupName, "original-prname")`
`20`	`21`	`if _, ok := pr.GetLabels()[originalPRLabel]; !ok {`