From 989ce87db63785e5ed8a716680450478b5081604 Mon Sep 17 00:00:00 2001 From: Andrea Frittoli Date: Tue, 9 Aug 2022 14:09:23 +0200 Subject: [PATCH] Make pipeline cancel robust to missing resources When a pipelinerun is cancelled, it may be that some of the resources managed by the pipelinerun are missing (NotFound). In this case we should not fail, and let the pipelinerun be cancelled. Discussion about this was initially triggered by https://github.com/tektoncd/pipeline/issues/5282, but this feature makes sense regardless of the bug that exposed its need. Signed-off-by: Andrea Frittoli --- pkg/reconciler/pipelinerun/cancel.go | 18 +++- pkg/reconciler/pipelinerun/cancel_test.go | 114 ++++++++++++++++++++++ 2 files changed, 131 insertions(+), 1 deletion(-) diff --git a/pkg/reconciler/pipelinerun/cancel.go b/pkg/reconciler/pipelinerun/cancel.go index 4e0faf59d47..55d34d802e5 100644 --- a/pkg/reconciler/pipelinerun/cancel.go +++ b/pkg/reconciler/pipelinerun/cancel.go @@ -32,6 +32,7 @@ import ( "go.uber.org/zap" jsonpatch "gomodules.xyz/jsonpatch/v2" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "knative.dev/pkg/apis" @@ -73,6 +74,21 @@ func init() { func cancelRun(ctx context.Context, runName string, namespace string, clientSet clientset.Interface) error { _, err := clientSet.TektonV1alpha1().Runs(namespace).Patch(ctx, runName, types.JSONPatchType, cancelRunPatchBytes, metav1.PatchOptions{}, "") + if errors.IsNotFound(err) { + // The resource may have been deleted in the meanwhile, but we should + // still be able to cancel the PipelineRun + return nil + } + return err +} + +func cancelTaskRun(ctx context.Context, taskRunName string, namespace string, clientSet clientset.Interface) error { + _, err := clientSet.TektonV1beta1().TaskRuns(namespace).Patch(ctx, taskRunName, types.JSONPatchType, cancelTaskRunPatchBytes, metav1.PatchOptions{}, "") + if errors.IsNotFound(err) { + // The resource may have been deleted in the meanwhile, but we should + // still be able to cancel the PipelineRun + return nil + } return err } @@ -118,7 +134,7 @@ func cancelPipelineTaskRuns(ctx context.Context, logger *zap.SugaredLogger, pr * for _, taskRunName := range trNames { logger.Infof("cancelling TaskRun %s", taskRunName) - if _, err := clientSet.TektonV1beta1().TaskRuns(pr.Namespace).Patch(ctx, taskRunName, types.JSONPatchType, cancelTaskRunPatchBytes, metav1.PatchOptions{}, ""); err != nil { + if err := cancelTaskRun(ctx, taskRunName, pr.Namespace, clientSet); err != nil { errs = append(errs, fmt.Errorf("Failed to patch TaskRun `%s` with cancellation: %s", taskRunName, err).Error()) continue } diff --git a/pkg/reconciler/pipelinerun/cancel_test.go b/pkg/reconciler/pipelinerun/cancel_test.go index 4bd9b3ac438..d03dbda3ae0 100644 --- a/pkg/reconciler/pipelinerun/cancel_test.go +++ b/pkg/reconciler/pipelinerun/cancel_test.go @@ -70,6 +70,24 @@ func TestCancelPipelineRun(t *testing.T) { taskRuns: []*v1beta1.TaskRun{ {ObjectMeta: metav1.ObjectMeta{Name: "t1"}}, }, + }, { + name: "multiple-taskruns-one-missing", + embeddedStatus: config.DefaultEmbeddedStatus, + pipelineRun: &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"}, + Spec: v1beta1.PipelineRunSpec{ + Status: v1beta1.PipelineRunSpecStatusCancelled, + }, + Status: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{ + TaskRuns: map[string]*v1beta1.PipelineRunTaskRunStatus{ + "t1": {PipelineTaskName: "task-1"}, + "t2": {PipelineTaskName: "task-2"}, + }, + }}, + }, + taskRuns: []*v1beta1.TaskRun{ + {ObjectMeta: metav1.ObjectMeta{Name: "t2"}}, + }, }, { name: "multiple-taskruns", embeddedStatus: config.DefaultEmbeddedStatus, @@ -108,6 +126,24 @@ func TestCancelPipelineRun(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "t1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "t2"}}, }, + }, { + name: "multiple-runs-one-missing", + embeddedStatus: config.DefaultEmbeddedStatus, + pipelineRun: &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"}, + Spec: v1beta1.PipelineRunSpec{ + Status: v1beta1.PipelineRunSpecStatusCancelled, + }, + Status: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{ + Runs: map[string]*v1beta1.PipelineRunRunStatus{ + "t1": {PipelineTaskName: "task-1"}, + "t2": {PipelineTaskName: "task-2"}, + }, + }}, + }, + runs: []*v1alpha1.Run{ + {ObjectMeta: metav1.ObjectMeta{Name: "t1"}}, + }, }, { name: "child-references-with-both", embeddedStatus: config.BothEmbeddedStatus, @@ -149,6 +185,45 @@ func TestCancelPipelineRun(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "r1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "r2"}}, }, + }, { + name: "child-references-with-both-some-missing", + embeddedStatus: config.BothEmbeddedStatus, + pipelineRun: &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"}, + Spec: v1beta1.PipelineRunSpec{ + Status: v1beta1.PipelineRunSpecStatusCancelled, + }, + Status: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{ + ChildReferences: []v1beta1.ChildStatusReference{ + { + TypeMeta: runtime.TypeMeta{Kind: "TaskRun"}, + Name: "t1", + PipelineTaskName: "task-1", + }, + { + TypeMeta: runtime.TypeMeta{Kind: "TaskRun"}, + Name: "t2", + PipelineTaskName: "task-2", + }, + { + TypeMeta: runtime.TypeMeta{Kind: "Run"}, + Name: "r1", + PipelineTaskName: "run-1", + }, + { + TypeMeta: runtime.TypeMeta{Kind: "Run"}, + Name: "r2", + PipelineTaskName: "run-2", + }, + }, + }}, + }, + taskRuns: []*v1beta1.TaskRun{ + {ObjectMeta: metav1.ObjectMeta{Name: "t1"}}, + }, + runs: []*v1alpha1.Run{ + {ObjectMeta: metav1.ObjectMeta{Name: "r2"}}, + }, }, { name: "child-references-with-minimal", embeddedStatus: config.MinimalEmbeddedStatus, @@ -190,6 +265,45 @@ func TestCancelPipelineRun(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "r1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "r2"}}, }, + }, { + name: "child-references-with-minimal-some-missing", + embeddedStatus: config.MinimalEmbeddedStatus, + pipelineRun: &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"}, + Spec: v1beta1.PipelineRunSpec{ + Status: v1beta1.PipelineRunSpecStatusCancelled, + }, + Status: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{ + ChildReferences: []v1beta1.ChildStatusReference{ + { + TypeMeta: runtime.TypeMeta{Kind: "TaskRun"}, + Name: "t1", + PipelineTaskName: "task-1", + }, + { + TypeMeta: runtime.TypeMeta{Kind: "TaskRun"}, + Name: "t2", + PipelineTaskName: "task-2", + }, + { + TypeMeta: runtime.TypeMeta{Kind: "Run"}, + Name: "r1", + PipelineTaskName: "run-1", + }, + { + TypeMeta: runtime.TypeMeta{Kind: "Run"}, + Name: "r2", + PipelineTaskName: "run-2", + }, + }, + }}, + }, + taskRuns: []*v1beta1.TaskRun{ + {ObjectMeta: metav1.ObjectMeta{Name: "t2"}}, + }, + runs: []*v1alpha1.Run{ + {ObjectMeta: metav1.ObjectMeta{Name: "r1"}}, + }, }, { name: "unknown-kind-on-child-references", embeddedStatus: config.MinimalEmbeddedStatus,