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
#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 @tekton-robot
afrittoli authored and tekton-robot committed Aug 18, 2022
1 parent 9976df8 commit 989ce87
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 989ce87

Please sign in to comment.