This patch fixes:

- Cancelling a pipelinerun with reason cancelled now throws an error Fixes: tektoncd#1139 Signed-off-by: Puneet Punamiya <ppunamiy@redhat.com>
devopstoday11 · Sep 28, 2020 · 7168e57 · 7168e57
1 parent 53e4a0c
commit 7168e57
Showing 4 changed files with 97 additions and 2 deletions.
diff --git a/pkg/cmd/pipelinerun/cancel.go b/pkg/cmd/pipelinerun/cancel.go
@@ -29,6 +29,7 @@ var (
 	succeeded   = formatted.ColorStatus("Succeeded")
 	failed      = formatted.ColorStatus("Failed")
 	prCancelled = formatted.ColorStatus("Cancelled") + "(PipelineRunCancelled)"
+	cancelled   = formatted.ColorStatus("Cancelled") + "(Cancelled)"
 	prTimeout   = formatted.ColorStatus("Failed") + "(" + v1beta1.PipelineRunReasonTimedOut.String() + ")"
 )
 
@@ -75,7 +76,7 @@ func cancelPipelineRun(p cli.Params, s *cli.Stream, prName string) error {
 	}
 
 	prCond := formatted.Condition(pr.Status.Conditions)
-	if prCond == succeeded || prCond == failed || prCond == prCancelled || prCond == prTimeout {
+	if prCond == succeeded || prCond == failed || prCond == prCancelled || prCond == prTimeout || prCond == cancelled {
 		return fmt.Errorf("failed to cancel PipelineRun %s: PipelineRun has already finished execution", prName)
 	}
 

diff --git a/pkg/cmd/pipelinerun/cancel_test.go b/pkg/cmd/pipelinerun/cancel_test.go
@@ -812,3 +812,88 @@ func Test_finished_pipelinerun_timeout_v1beta1(t *testing.T) {
 	expected := "Error: failed to cancel PipelineRun " + prName + ": PipelineRun has already finished execution\n"
 	tu.AssertOutput(t, expected, got)
 }
+
+func Test_finished_pipelinerun_cancelled(t *testing.T) {
+	ns := []*corev1.Namespace{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "ns",
+			},
+		},
+	}
+
+	prName := "test-pipeline-run-123"
+
+	prs := []*v1beta1.PipelineRun{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      prName,
+				Namespace: "ns",
+				Labels:    map[string]string{"tekton.dev/pipeline": "pipelineName"},
+			},
+			Spec: v1beta1.PipelineRunSpec{
+				PipelineRef: &v1beta1.PipelineRef{
+					Name: "pipelineName",
+				},
+				ServiceAccountName: "test-sa",
+				Resources: []v1beta1.PipelineResourceBinding{
+					{
+						Name: "git-repo",
+						ResourceRef: &v1beta1.PipelineResourceRef{
+							Name: "some-repo",
+						},
+					},
+					{
+						Name: "build-image",
+						ResourceRef: &v1beta1.PipelineResourceRef{
+							Name: "some-image",
+						},
+					},
+				},
+				Params: []v1beta1.Param{
+					{
+						Name: "pipeline-param-1",
+						Value: v1beta1.ArrayOrString{
+							Type:      v1beta1.ParamTypeString,
+							StringVal: "somethingmorefun",
+						},
+					},
+					{
+						Name: "rev-param",
+						Value: v1beta1.ArrayOrString{
+							Type:      v1beta1.ParamTypeString,
+							StringVal: "revision1",
+						},
+					},
+				},
+			},
+			Status: v1beta1.PipelineRunStatus{
+				Status: duckv1beta1.Status{
+					Conditions: duckv1beta1.Conditions{
+						{
+							Status: corev1.ConditionFalse,
+							Reason: "Cancelled",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	cs, _ := test.SeedV1beta1TestData(t, pipelinev1beta1test.Data{PipelineRuns: prs, Namespaces: ns})
+	cs.Pipeline.Resources = cb.APIResourceList(versionB1, []string{"pipeline", "pipelinerun"})
+	tdc := testDynamic.Options{}
+	dc, err := tdc.Client(
+		cb.UnstructuredV1beta1PR(prs[0], versionB1),
+	)
+	if err != nil {
+		t.Errorf("unable to create dynamic client: %v", err)
+	}
+	p := &tu.Params{Tekton: cs.Pipeline, Kube: cs.Kube, Dynamic: dc}
+
+	pRun := Command(p)
+	got, _ := tu.ExecuteCommand(pRun, "cancel", prName, "-n", "ns")
+
+	expected := "Error: failed to cancel PipelineRun " + prName + ": PipelineRun has already finished execution\n"
+	tu.AssertOutput(t, expected, got)
+}
diff --git a/pkg/formatted/k8s.go b/pkg/formatted/k8s.go
@@ -69,7 +69,7 @@ func Condition(c v1beta1.Conditions) string {
 
 	if c[0].Reason != "" && c[0].Reason != status {
 		switch c[0].Reason {
-		case "PipelineRunCancelled", "TaskRunCancelled":
+		case "PipelineRunCancelled", "TaskRunCancelled", "Cancelled":
 			return ColorStatus("Cancelled") + "(" + c[0].Reason + ")"
 		case "PipelineRunStopping", "TaskRunStopping":
 			return ColorStatus("Failed") + "(" + c[0].Reason + ")"

diff --git a/pkg/formatted/k8s_test.go b/pkg/formatted/k8s_test.go
@@ -77,6 +77,15 @@ func TestCondition(t *testing.T) {
 			}},
 			want: "Cancelled(TaskRunCancelled)",
 		},
+		{
+			name: "Cancelled status reason",
+			condition: []apis.Condition{{
+				Type:   apis.ConditionSucceeded,
+				Status: corev1.ConditionFalse,
+				Reason: "Cancelled",
+			}},
+			want: "Cancelled(Cancelled)",
+		},
 		{
 			name: "PipelineRunTimeout status reason",
 			condition: []apis.Condition{{