Remove deprecated PipelineRunSpecStatusCancelledDeprecated

Fixes #4611. We deprecated the original cancelled status (with the value `PipelineRunCancelled`) in #3915, and we're now at the time for it to be removed in favor of the value `Cancelled`. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
tektoncd · Jun 22, 2022 · add0dae · add0dae
1 parent e13e6bf
commit add0dae
Show file tree

Hide file tree

Showing 11 changed files with 122 additions and 168 deletions.
diff --git a/docs/deprecations.md b/docs/deprecations.md
@@ -19,7 +19,6 @@ being deprecated.
 
 | Feature Being Deprecated                                                                                                                                            | Deprecation Announcement                                             | [API Compatibility Policy](https://github.com/tektoncd/pipeline/tree/main/api_compatibility_policy.md) | Earliest Date or Release of Removal |
 |---------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------|-------------------------------------|
-| [`PipelineRunCancelled` is deprecated and will be removed](https://github.com/tektoncd/pipeline/issues/4611)                                                        | [v0.25.0](https://github.com/tektoncd/pipeline/releases/tag/v0.25.0) | Beta                                                                                                   | July 12 2022                       |
 | [`PipelineResources` are deprecated.](https://github.com/tektoncd/community/blob/main/teps/0074-deprecate-pipelineresources.md)                                     | [v0.30.0](https://github.com/tektoncd/pipeline/releases/tag/v0.30.0) | Alpha                                                                                                  | Dec 20 2021                         |
 | [The `PipelineRun.Status.TaskRuns` and `PipelineRun.Status.Runs` fields are deprecated and will be removed.](https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md) | v0.35.0                                             | Beta                                                                                                   | Jan 25, 2023                        |
 | [PipelineRun.Timeout is deprecated and will be removed](https://github.com/tektoncd/community/blob/main/teps/0046-finallytask-execution-post-timeout.md) | v0.36.0 (to be released)                                             | Beta                                                                                                   | Feb 25, 2023                        |

diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go
@@ -83,7 +83,7 @@ func (pr *PipelineRun) HasStarted() bool {
 
 // IsCancelled returns true if the PipelineRun's spec status is set to Cancelled state
 func (pr *PipelineRun) IsCancelled() bool {
-	return pr.Spec.Status == PipelineRunSpecStatusCancelled || pr.Spec.Status == PipelineRunSpecStatusCancelledDeprecated
+	return pr.Spec.Status == PipelineRunSpecStatusCancelled
 }
 
 // IsGracefullyCancelled returns true if the PipelineRun's spec status is set to CancelledRunFinally state
@@ -242,10 +242,6 @@ type TimeoutFields struct {
 type PipelineRunSpecStatus string
 
 const (
-	// PipelineRunSpecStatusCancelledDeprecated Deprecated: indicates that the user wants to cancel the task,
-	// if not already cancelled or terminated (replaced by "Cancelled")
-	PipelineRunSpecStatusCancelledDeprecated = "PipelineRunCancelled"
-
 	// PipelineRunSpecStatusCancelled indicates that the user wants to cancel the task,
 	// if not already cancelled or terminated
 	PipelineRunSpecStatusCancelled = "Cancelled"

diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go
@@ -121,8 +121,7 @@ func validateSpecStatus(ctx context.Context, status PipelineRunSpecStatus) *apis
 	switch status {
 	case "":
 		return nil
-	case PipelineRunSpecStatusPending,
-		PipelineRunSpecStatusCancelledDeprecated:
+	case PipelineRunSpecStatusPending:
 		return nil
 	case PipelineRunSpecStatusCancelled,
 		PipelineRunSpecStatusCancelledRunFinally,

diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
@@ -280,19 +280,6 @@ func TestPipelineRun_Validate(t *testing.T) {
 				},
 			},
 		},
-	}, {
-		name: "pipelinerun cancelled deprecated",
-		pr: v1beta1.PipelineRun{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "pipelinerunname",
-			},
-			Spec: v1beta1.PipelineRunSpec{
-				Status: v1beta1.PipelineRunSpecStatusCancelledDeprecated,
-				PipelineRef: &v1beta1.PipelineRef{
-					Name: "prname",
-				},
-			},
-		},
 	}, {
 		name: "pipelinerun gracefully cancelled",
 		pr: v1beta1.PipelineRun{

diff --git a/pkg/pipelinerunmetrics/metrics.go b/pkg/pipelinerunmetrics/metrics.go
@@ -64,8 +64,6 @@ var (
 const (
 	// ReasonCancelled indicates that a PipelineRun was cancelled.
 	ReasonCancelled = "Cancelled"
-	// ReasonCancelledDeprecated Deprecated: "PipelineRunCancelled" indicates that a PipelineRun was cancelled.
-	ReasonCancelledDeprecated = "PipelineRunCancelled"
 )
 
 // Recorder holds keys for Tekton metrics
@@ -236,7 +234,7 @@ func (r *Recorder) DurationAndCount(pr *v1beta1.PipelineRun, beforeCondition *ap
 	status := "success"
 	if cond := pr.Status.GetCondition(apis.ConditionSucceeded); cond.Status == corev1.ConditionFalse {
 		status = "failed"
-		if cond.Reason == ReasonCancelled || cond.Reason == ReasonCancelledDeprecated {
+		if cond.Reason == ReasonCancelled {
 			status = "cancelled"
 		}
 	}

diff --git a/pkg/reconciler/pipelinerun/cancel.go b/pkg/reconciler/pipelinerun/cancel.go
@@ -71,9 +71,6 @@ func cancelPipelineRun(ctx context.Context, logger *zap.SugaredLogger, pr *v1bet
 	// If we successfully cancelled all the TaskRuns and Runs, we can consider the PipelineRun cancelled.
 	if len(errs) == 0 {
 		reason := ReasonCancelled
-		if pr.Spec.Status == v1beta1.PipelineRunSpecStatusCancelledDeprecated {
-			reason = ReasonCancelledDeprecated
-		}
 
 		pr.Status.SetCondition(&apis.Condition{
 			Type:    apis.ConditionSucceeded,

diff --git a/pkg/reconciler/pipelinerun/cancel_test.go b/pkg/reconciler/pipelinerun/cancel_test.go
@@ -108,15 +108,6 @@ func TestCancelPipelineRun(t *testing.T) {
 			{ObjectMeta: metav1.ObjectMeta{Name: "t1"}},
 			{ObjectMeta: metav1.ObjectMeta{Name: "t2"}},
 		},
-	}, {
-		name:           "deprecated-state",
-		embeddedStatus: config.DefaultEmbeddedStatus,
-		pipelineRun: &v1beta1.PipelineRun{
-			ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"},
-			Spec: v1beta1.PipelineRunSpec{
-				Status: v1beta1.PipelineRunSpecStatusCancelledDeprecated,
-			},
-		},
 	}, {
 		name:           "child-references-with-both",
 		embeddedStatus: config.BothEmbeddedStatus,

diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go
@@ -104,8 +104,6 @@ const (
 	ReasonInvalidGraph = "PipelineInvalidGraph"
 	// ReasonCancelled indicates that a PipelineRun was cancelled.
 	ReasonCancelled = pipelinerunmetrics.ReasonCancelled
-	// ReasonCancelledDeprecated Deprecated: "PipelineRunCancelled" indicates that a PipelineRun was cancelled.
-	ReasonCancelledDeprecated = pipelinerunmetrics.ReasonCancelledDeprecated
 	// ReasonPending indicates that a PipelineRun is pending.
 	ReasonPending = "PipelineRunPending"
 	// ReasonCouldntCancel indicates that a PipelineRun was cancelled but attempting to update

diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go
@@ -1512,11 +1512,6 @@ func runTestReconcileOnCancelledPipelineRun(t *testing.T, embeddedStatus string)
 			specStatus: v1beta1.PipelineRunSpecStatusCancelled,
 			reason:     ReasonCancelled,
 		},
-		{
-			name:       "cancelled deprecated",
-			specStatus: v1beta1.PipelineRunSpecStatusCancelledDeprecated,
-			reason:     ReasonCancelledDeprecated,
-		},
 	}
 
 	for _, tc := range testCases {

diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go
@@ -361,8 +361,7 @@ func (facts *PipelineRunFacts) IsRunning() bool {
 
 // IsCancelled returns true if the PipelineRun was cancelled
 func (facts *PipelineRunFacts) IsCancelled() bool {
-	return facts.SpecStatus == v1beta1.PipelineRunSpecStatusCancelledDeprecated ||
-		facts.SpecStatus == v1beta1.PipelineRunSpecStatusCancelled
+	return facts.SpecStatus == v1beta1.PipelineRunSpecStatusCancelled
 }
 
 // IsGracefullyCancelled returns true if the PipelineRun was gracefully cancelled

diff --git a/test/cancel_test.go b/test/cancel_test.go
@@ -44,20 +44,19 @@ func TestTaskRunPipelineRunCancel(t *testing.T) {
 	// the retrying TaskRun to retry.
 	for _, numRetries := range []int{0, 1} {
 		numRetries := numRetries // capture range variable
-		for _, specStatus := range []string{v1beta1.PipelineRunSpecStatusCancelledDeprecated, v1beta1.PipelineRunSpecStatusCancelled} {
-			specStatus := specStatus // capture status variable
-			t.Run(fmt.Sprintf("retries=%d,status=%s", numRetries, specStatus), func(t *testing.T) {
-				ctx := context.Background()
-				ctx, cancel := context.WithCancel(ctx)
-				defer cancel()
-				requirements := []func(context.Context, *testing.T, *clients, string){}
-				c, namespace := setup(ctx, t, requirements...)
-				t.Parallel()
-
-				knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf)
-				defer tearDown(ctx, t, c, namespace)
-
-				pipelineRun := parse.MustParsePipelineRun(t, fmt.Sprintf(`
+		specStatus := v1beta1.PipelineRunSpecStatusCancelled
+		t.Run(fmt.Sprintf("retries=%d,status=%s", numRetries, specStatus), func(t *testing.T) {
+			ctx := context.Background()
+			ctx, cancel := context.WithCancel(ctx)
+			defer cancel()
+			requirements := []func(context.Context, *testing.T, *clients, string){}
+			c, namespace := setup(ctx, t, requirements...)
+			t.Parallel()
+
+			knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf)
+			defer tearDown(ctx, t, c, namespace)
+
+			pipelineRun := parse.MustParsePipelineRun(t, fmt.Sprintf(`
 metadata:
   name: %s
   namespace: %s
@@ -72,116 +71,112 @@ spec:
           script: 'sleep 5000'
 `, helpers.ObjectNameForTest(t), namespace, numRetries))
 
-				t.Logf("Creating PipelineRun in namespace %s", namespace)
-				if _, err := c.PipelineRunClient.Create(ctx, pipelineRun, metav1.CreateOptions{}); err != nil {
-					t.Fatalf("Failed to create PipelineRun `%s`: %s", pipelineRun.Name, err)
-				}
-
-				t.Logf("Waiting for Pipelinerun %s in namespace %s to be started", pipelineRun.Name, namespace)
-				if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, Running(pipelineRun.Name), "PipelineRunRunning"); err != nil {
-					t.Fatalf("Error waiting for PipelineRun %s to be running: %s", pipelineRun.Name, err)
-				}
-
-				taskrunList, err := c.TaskRunClient.List(ctx, metav1.ListOptions{LabelSelector: "tekton.dev/pipelineRun=" + pipelineRun.Name})
-				if err != nil {
-					t.Fatalf("Error listing TaskRuns for PipelineRun %s: %s", pipelineRun.Name, err)
-				}
-
-				var wg sync.WaitGroup
-				t.Logf("Waiting for TaskRuns from PipelineRun %s in namespace %s to be running", pipelineRun.Name, namespace)
-				for _, taskrunItem := range taskrunList.Items {
-					wg.Add(1)
-					go func(name string) {
-						defer wg.Done()
-						err := WaitForTaskRunState(ctx, c, name, Running(name), "TaskRunRunning")
-						if err != nil {
-							t.Errorf("Error waiting for TaskRun %s to be running: %v", name, err)
-						}
-					}(taskrunItem.Name)
-				}
-				wg.Wait()
-
-				pr, err := c.PipelineRunClient.Get(ctx, pipelineRun.Name, metav1.GetOptions{})
-				if err != nil {
-					t.Fatalf("Failed to get PipelineRun `%s`: %s", pipelineRun.Name, err)
-				}
-
-				patches := []jsonpatch.JsonPatchOperation{{
-					Operation: "add",
-					Path:      "/spec/status",
-					Value:     specStatus,
-				}}
-				patchBytes, err := json.Marshal(patches)
-				if err != nil {
-					t.Fatalf("failed to marshal patch bytes in order to cancel")
-				}
-				if _, err := c.PipelineRunClient.Patch(ctx, pr.Name, types.JSONPatchType, patchBytes, metav1.PatchOptions{}, ""); err != nil {
-					t.Fatalf("Failed to patch PipelineRun `%s` with cancellation: %s", pipelineRun.Name, err)
-				}
-
-				expectedReason := v1beta1.PipelineRunReasonCancelled.String()
-				if specStatus == v1beta1.PipelineRunSpecStatusCancelledDeprecated {
-					expectedReason = "PipelineRunCancelled"
-				}
-				expectedCondition := FailedWithReason(expectedReason, pipelineRun.Name)
-				t.Logf("Waiting for PipelineRun %s in namespace %s to be cancelled", pipelineRun.Name, namespace)
-				if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, expectedCondition, expectedReason); err != nil {
-					t.Errorf("Error waiting for PipelineRun %q to finished: %s", pipelineRun.Name, err)
-				}
-
-				t.Logf("Waiting for TaskRuns in PipelineRun %s in namespace %s to be cancelled", pipelineRun.Name, namespace)
-				for _, taskrunItem := range taskrunList.Items {
-					wg.Add(1)
-					go func(name string) {
-						defer wg.Done()
-						err := WaitForTaskRunState(ctx, c, name, FailedWithReason("TaskRunCancelled", name), "TaskRunCancelled")
-						if err != nil {
-							t.Errorf("Error waiting for TaskRun %s to be finished: %v", name, err)
-						}
-					}(taskrunItem.Name)
-				}
-				wg.Wait()
-
-				var trName []string
-				taskrunList, err = c.TaskRunClient.List(ctx, metav1.ListOptions{LabelSelector: "tekton.dev/pipelineRun=" + pipelineRun.Name})
-				if err != nil {
-					t.Fatalf("Error listing TaskRuns for PipelineRun %s: %s", pipelineRun.Name, err)
-				}
-				for _, taskrunItem := range taskrunList.Items {
-					trName = append(trName, taskrunItem.Name)
-				}
-
-				matchKinds := map[string][]string{"PipelineRun": {pipelineRun.Name}}
-				// Expected failure events: 1 for the pipelinerun cancel
-				expectedNumberOfEvents := 1
-				t.Logf("Making sure %d events were created from pipelinerun with kinds %v", expectedNumberOfEvents, matchKinds)
-				events, err := collectMatchingEvents(ctx, c.KubeClient, namespace, matchKinds, "Failed")
-				if err != nil {
-					t.Fatalf("Failed to collect matching events: %q", err)
-				}
-				if len(events) < expectedNumberOfEvents {
-					collectedEvents := make([]string, 0, len(events))
-					for _, event := range events {
-						collectedEvents = append(collectedEvents, fmt.Sprintf("%#v", event))
+			t.Logf("Creating PipelineRun in namespace %s", namespace)
+			if _, err := c.PipelineRunClient.Create(ctx, pipelineRun, metav1.CreateOptions{}); err != nil {
+				t.Fatalf("Failed to create PipelineRun `%s`: %s", pipelineRun.Name, err)
+			}
+
+			t.Logf("Waiting for Pipelinerun %s in namespace %s to be started", pipelineRun.Name, namespace)
+			if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, Running(pipelineRun.Name), "PipelineRunRunning"); err != nil {
+				t.Fatalf("Error waiting for PipelineRun %s to be running: %s", pipelineRun.Name, err)
+			}
+
+			taskrunList, err := c.TaskRunClient.List(ctx, metav1.ListOptions{LabelSelector: "tekton.dev/pipelineRun=" + pipelineRun.Name})
+			if err != nil {
+				t.Fatalf("Error listing TaskRuns for PipelineRun %s: %s", pipelineRun.Name, err)
+			}
+
+			var wg sync.WaitGroup
+			t.Logf("Waiting for TaskRuns from PipelineRun %s in namespace %s to be running", pipelineRun.Name, namespace)
+			for _, taskrunItem := range taskrunList.Items {
+				wg.Add(1)
+				go func(name string) {
+					defer wg.Done()
+					err := WaitForTaskRunState(ctx, c, name, Running(name), "TaskRunRunning")
+					if err != nil {
+						t.Errorf("Error waiting for TaskRun %s to be running: %v", name, err)
 					}
-					t.Fatalf("Expected %d number of failed events from pipelinerun but got %d; list of received events : %s", expectedNumberOfEvents, len(events), strings.Join(collectedEvents, ", "))
-				}
-				matchKinds = map[string][]string{"TaskRun": trName}
-				// Expected failure events: 1 for each TaskRun
-				expectedNumberOfEvents = len(trName)
-				t.Logf("Making sure %d events were created from taskruns with kinds %v", expectedNumberOfEvents, matchKinds)
-				events, err = collectMatchingEvents(ctx, c.KubeClient, namespace, matchKinds, "Failed")
-				if err != nil {
-					t.Fatalf("Failed to collect matching events: %q", err)
-				}
-				if len(events) < expectedNumberOfEvents {
-					collectedEvents := make([]string, 0, len(events))
-					for _, event := range events {
-						collectedEvents = append(collectedEvents, fmt.Sprintf("%#v", event))
+				}(taskrunItem.Name)
+			}
+			wg.Wait()
+
+			pr, err := c.PipelineRunClient.Get(ctx, pipelineRun.Name, metav1.GetOptions{})
+			if err != nil {
+				t.Fatalf("Failed to get PipelineRun `%s`: %s", pipelineRun.Name, err)
+			}
+
+			patches := []jsonpatch.JsonPatchOperation{{
+				Operation: "add",
+				Path:      "/spec/status",
+				Value:     specStatus,
+			}}
+			patchBytes, err := json.Marshal(patches)
+			if err != nil {
+				t.Fatalf("failed to marshal patch bytes in order to cancel")
+			}
+			if _, err := c.PipelineRunClient.Patch(ctx, pr.Name, types.JSONPatchType, patchBytes, metav1.PatchOptions{}, ""); err != nil {
+				t.Fatalf("Failed to patch PipelineRun `%s` with cancellation: %s", pipelineRun.Name, err)
+			}
+
+			expectedReason := v1beta1.PipelineRunReasonCancelled.String()
+			expectedCondition := FailedWithReason(expectedReason, pipelineRun.Name)
+			t.Logf("Waiting for PipelineRun %s in namespace %s to be cancelled", pipelineRun.Name, namespace)
+			if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, expectedCondition, expectedReason); err != nil {
+				t.Errorf("Error waiting for PipelineRun %q to finished: %s", pipelineRun.Name, err)
+			}
+
+			t.Logf("Waiting for TaskRuns in PipelineRun %s in namespace %s to be cancelled", pipelineRun.Name, namespace)
+			for _, taskrunItem := range taskrunList.Items {
+				wg.Add(1)
+				go func(name string) {
+					defer wg.Done()
+					err := WaitForTaskRunState(ctx, c, name, FailedWithReason("TaskRunCancelled", name), "TaskRunCancelled")
+					if err != nil {
+						t.Errorf("Error waiting for TaskRun %s to be finished: %v", name, err)
 					}
-					t.Fatalf("Expected %d number of failed events from taskrun but got %d; list of received events : %s", expectedNumberOfEvents, len(events), strings.Join(collectedEvents, ", "))
-				}
-			})
-		}
+				}(taskrunItem.Name)
+			}
+			wg.Wait()
+
+			var trName []string
+			taskrunList, err = c.TaskRunClient.List(ctx, metav1.ListOptions{LabelSelector: "tekton.dev/pipelineRun=" + pipelineRun.Name})
+			if err != nil {
+				t.Fatalf("Error listing TaskRuns for PipelineRun %s: %s", pipelineRun.Name, err)
+			}
+			for _, taskrunItem := range taskrunList.Items {
+				trName = append(trName, taskrunItem.Name)
+			}
+
+			matchKinds := map[string][]string{"PipelineRun": {pipelineRun.Name}}
+			// Expected failure events: 1 for the pipelinerun cancel
+			expectedNumberOfEvents := 1
+			t.Logf("Making sure %d events were created from pipelinerun with kinds %v", expectedNumberOfEvents, matchKinds)
+			events, err := collectMatchingEvents(ctx, c.KubeClient, namespace, matchKinds, "Failed")
+			if err != nil {
+				t.Fatalf("Failed to collect matching events: %q", err)
+			}
+			if len(events) < expectedNumberOfEvents {
+				collectedEvents := make([]string, 0, len(events))
+				for _, event := range events {
+					collectedEvents = append(collectedEvents, fmt.Sprintf("%#v", event))
+				}
+				t.Fatalf("Expected %d number of failed events from pipelinerun but got %d; list of received events : %s", expectedNumberOfEvents, len(events), strings.Join(collectedEvents, ", "))
+			}
+			matchKinds = map[string][]string{"TaskRun": trName}
+			// Expected failure events: 1 for each TaskRun
+			expectedNumberOfEvents = len(trName)
+			t.Logf("Making sure %d events were created from taskruns with kinds %v", expectedNumberOfEvents, matchKinds)
+			events, err = collectMatchingEvents(ctx, c.KubeClient, namespace, matchKinds, "Failed")
+			if err != nil {
+				t.Fatalf("Failed to collect matching events: %q", err)
+			}
+			if len(events) < expectedNumberOfEvents {
+				collectedEvents := make([]string, 0, len(events))
+				for _, event := range events {
+					collectedEvents = append(collectedEvents, fmt.Sprintf("%#v", event))
+				}
+				t.Fatalf("Expected %d number of failed events from taskrun but got %d; list of received events : %s", expectedNumberOfEvents, len(events), strings.Join(collectedEvents, ", "))
+			}
+		})
 	}
 }