Skip to content

Commit

Permalink
Make pipeline cancel robust to missing resources
Browse files Browse the repository at this point in the history 
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
tektoncd#5282, but this feature
makes sense regardless of the bug that exposed its need.

Signed-off-by: Andrea Frittoli <[email protected]>
  • Loading branch information
@afrittoli
afrittoli committed Aug 9, 2022
1 parent 369665c commit 6687c0f
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 1 deletion.
18 changes: 17 additions & 1 deletion pkg/reconciler/pipelinerun/cancel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down
114 changes: 114 additions & 0 deletions pkg/reconciler/pipelinerun/cancel_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 6687c0f

Please sign in to comment.