Skip to content

Commit

Permalink
Promote Graceful termination to stable
Browse files Browse the repository at this point in the history
With the addition of graceful termination in v0.25.x release,
`PipelineRunCancelled` was deprecated. It's been 8 months and no such
issues so far, we can promote these to beta.

In this patch, unguarding the graceful termination from alpha feature
flag and modifying the tests based on this.

Signed-off-by: vinamra28 <jvinamra776@gmail.com>
  • Loading branch information
vinamra28 committed Apr 21, 2022
1 parent 8503243 commit 83807c4
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 73 deletions.
1 change: 0 additions & 1 deletion docs/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,6 @@ Features currently in "alpha" are:
| [`Runs` and `Custom Tasks`](./runs.md) | [TEP-0002](https://github.com/tektoncd/community/blob/main/teps/0002-custom-tasks.md) | [v0.19.0](https://github.com/tektoncd/pipeline/releases/tag/v0.19.0) | `enable-custom-tasks` |
| [Isolated `Step` & `Sidecar` `Workspaces`](./workspaces.md#isolated-workspaces) | [TEP-0029](https://github.com/tektoncd/community/blob/main/teps/0029-step-workspaces.md) | [v0.24.0](https://github.com/tektoncd/pipeline/releases/tag/v0.24.0) | |
| [Hermetic Execution Mode](./hermetic.md) | [TEP-0025](https://github.com/tektoncd/community/blob/main/teps/0025-hermekton.md) | [v0.25.0](https://github.com/tektoncd/pipeline/releases/tag/v0.25.0) | |
| [Graceful Termination](./pipelineruns.md#gracefully-cancelling-a-pipelinerun) | [TEP-0058](https://github.com/tektoncd/community/blob/main/teps/0058-graceful-pipeline-run-termination.md) | [v0.25.0](https://github.com/tektoncd/pipeline/releases/tag/v0.25.0) | |
| [`PipelineRun` Timeouts](./pipelineruns.md#configuring-a-failure-timeout) | [TEP-0046](https://github.com/tektoncd/community/blob/main/teps/0046-finallytask-execution-post-timeout.md) | [v0.25.0](https://github.com/tektoncd/pipeline/releases/tag/v0.25.0) | |
| [Implicit `Parameters`](./taskruns.md#implicit-parameters) | [TEP-0023](https://github.com/tektoncd/community/blob/main/teps/0023-implicit-mapping.md) | [v0.28.0](https://github.com/tektoncd/pipeline/releases/tag/v0.28.0) | |
| [Windows Scripts](./tasks.md#windows-scripts) | [TEP-0057](https://github.com/tektoncd/community/blob/main/teps/0057-windows-support.md) | [v0.28.0](https://github.com/tektoncd/pipeline/releases/tag/v0.28.0) | |
Expand Down
15 changes: 5 additions & 10 deletions docs/pipelineruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -653,12 +653,12 @@ Completion time is set once a `PipelineRun` reaches status `True` or `False`:
:-------|:-------|:---------------------:|--------------:
Unknown|Started|No|The `PipelineRun` has just been picked up by the controller.
Unknown|Running|No|The `PipelineRun` has been validate and started to perform its work.
Unknown|PipelineRunCancelled|No|The user requested the PipelineRun to be cancelled. Cancellation has not be done yet.
Unknown|Cancelled|No|The user requested the PipelineRun to be cancelled. Cancellation has not be done yet.
True|Succeeded|Yes|The `PipelineRun` completed successfully.
True|Completed|Yes|The `PipelineRun` completed successfully, one or more Tasks were skipped.
False|Failed|Yes|The `PipelineRun` failed because one of the `TaskRuns` failed.
False|\[Error message\]|Yes|The `PipelineRun` failed with a permanent error (usually validation).
False|PipelineRunCancelled|Yes|The `PipelineRun` was cancelled successfully.
False|Cancelled|Yes|The `PipelineRun` was cancelled successfully.
False|PipelineRunTimeout|Yes|The `PipelineRun` timed out.

When a `PipelineRun` changes status, [events](events.md#pipelineruns) are triggered accordingly.
Expand Down Expand Up @@ -714,7 +714,7 @@ Some examples:
## Cancelling a `PipelineRun`

To cancel a `PipelineRun` that's currently executing, update its definition
to mark it as "PipelineRunCancelled". When you do so, the spawned `TaskRuns` are also marked
to mark it as "Cancelled". When you do so, the spawned `TaskRuns` are also marked
as cancelled, all associated `Pods` are deleted, and their `Retries` are not executed.
Pending `finally` tasks are not scheduled.

Expand All @@ -727,16 +727,11 @@ metadata:
name: go-example-git
spec:
# […]
status: "PipelineRunCancelled"
status: "Cancelled"
```

Warning: "PipelineRunCancelled" status is deprecated and would be removed in V1, please use "Cancelled" instead.

## Gracefully cancelling a `PipelineRun`

[Graceful pipeline run termination](https://github.com/tektoncd/community/blob/main/teps/0058-graceful-pipeline-run-termination.md)
is currently an **_alpha feature_**.

To gracefully cancel a `PipelineRun` that's currently executing, update its definition
to mark it as "CancelledRunFinally". When you do so, the spawned `TaskRuns` are also marked
as cancelled, all associated `Pods` are deleted, and their `Retries` are not executed.
Expand Down Expand Up @@ -790,7 +785,7 @@ spec:
status: "PipelineRunPending"
```

To start the PipelineRun, clear the `.spec.status` field. Alternatively, update the value to `PipelineRunCancelled` to cancel it.
To start the PipelineRun, clear the `.spec.status` field. Alternatively, update the value to `Cancelled` to cancel it.

---

Expand Down
17 changes: 6 additions & 11 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,20 +129,15 @@ func validateSpecStatus(ctx context.Context, status PipelineRunSpecStatus) *apis
case PipelineRunSpecStatusCancelled,
PipelineRunSpecStatusCancelledRunFinally,
PipelineRunSpecStatusStoppedRunFinally:
return ValidateEnabledAPIFields(ctx, "graceful termination", "alpha")
return nil
}

cfg := config.FromContextOrDefaults(ctx)
if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields {
return apis.ErrInvalidValue(fmt.Sprintf("%s should be %s, %s, %s or %s", status,
PipelineRunSpecStatusCancelled,
PipelineRunSpecStatusCancelledRunFinally,
PipelineRunSpecStatusStoppedRunFinally,
PipelineRunSpecStatusPending), "status")
}
return apis.ErrInvalidValue(fmt.Sprintf("%s should be %s or %s", status,
PipelineRunSpecStatusCancelledDeprecated,
return apis.ErrInvalidValue(fmt.Sprintf("%s should be %s, %s, %s or %s", status,
PipelineRunSpecStatusCancelled,
PipelineRunSpecStatusCancelledRunFinally,
PipelineRunSpecStatusStoppedRunFinally,
PipelineRunSpecStatusPending), "status")

}

func validateTimeoutDuration(field string, d *metav1.Duration) (errs *apis.FieldError) {
Expand Down
32 changes: 0 additions & 32 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,36 +92,7 @@ func TestPipelineRun_Invalid(t *testing.T) {
Status: "PipelineRunCancell",
},
},
want: apis.ErrInvalidValue("PipelineRunCancell should be PipelineRunCancelled or PipelineRunPending", "spec.status"),
}, {
name: "wrong pipelinerun cancel with alpha features",
pr: v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Name: "pipelinelinename",
},
Spec: v1beta1.PipelineRunSpec{
PipelineRef: &v1beta1.PipelineRef{
Name: "prname",
},
Status: "PipelineRunCancell",
},
},
want: apis.ErrInvalidValue("PipelineRunCancell should be Cancelled, CancelledRunFinally, StoppedRunFinally or PipelineRunPending", "spec.status"),
wc: enableAlphaAPIFields,
}, {
name: "alpha pipelinerun graceful cancel",
pr: v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Name: "pipelinelinename",
},
Spec: v1beta1.PipelineRunSpec{
PipelineRef: &v1beta1.PipelineRef{
Name: "prname",
},
Status: v1beta1.PipelineRunSpecStatusCancelled,
},
},
want: apis.ErrGeneric("graceful termination requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""),
}, {
name: "use of bundle without the feature flag set",
pr: v1beta1.PipelineRun{
Expand Down Expand Up @@ -311,7 +282,6 @@ func TestPipelineRun_Validate(t *testing.T) {
},
},
},
wc: enableAlphaAPIFields,
}, {
name: "pipelinerun cancelled deprecated",
pr: v1beta1.PipelineRun{
Expand All @@ -338,7 +308,6 @@ func TestPipelineRun_Validate(t *testing.T) {
},
},
},
wc: enableAlphaAPIFields,
}, {
name: "pipelinerun gracefully stopped",
pr: v1beta1.PipelineRun{
Expand All @@ -352,7 +321,6 @@ func TestPipelineRun_Validate(t *testing.T) {
},
},
},
wc: enableAlphaAPIFields,
}, {
name: "alpha feature: valid resolver",
pr: v1beta1.PipelineRun{
Expand Down
3 changes: 1 addition & 2 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
}

// check if pipeline run is not gracefully cancelled and there are active task runs, which require cancelling
if cfg.FeatureFlags.EnableAPIFields == apisconfig.AlphaAPIFields &&
pr.IsGracefullyCancelled() && pipelineRunFacts.IsRunning() {
if pr.IsGracefullyCancelled() && pipelineRunFacts.IsRunning() {
// If the pipelinerun is cancelled, cancel tasks, but run finally
err := gracefullyCancelPipelineRun(ctx, logger, pr, c.PipelineClientSet)
if err != nil {
Expand Down
12 changes: 3 additions & 9 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1539,7 +1539,7 @@ func runTestReconcileOnCancelledPipelineRun(t *testing.T, embeddedStatus string)
ts := []*v1beta1.Task{simpleHelloWorldTask}
trs := []*v1beta1.TaskRun{createHelloWorldTaskRun(t, "test-pipeline-run-cancelled-hello-world", "foo",
"test-pipeline-run-cancelled", "test-pipeline")}
cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(withEmbeddedStatus(newFeatureFlagsConfigMap(), embeddedStatus))}
cms := []*corev1.ConfigMap{withEmbeddedStatus(newFeatureFlagsConfigMap(), embeddedStatus)}

d := test.Data{
PipelineRuns: prs,
Expand Down Expand Up @@ -1811,12 +1811,10 @@ func runTestReconcileOnCancelledRunFinallyPipelineRun(t *testing.T, embeddedStat
prs := []*v1beta1.PipelineRun{createCancelledPipelineRun(t, "test-pipeline-run-cancelled-run-finally", v1beta1.PipelineRunSpecStatusCancelledRunFinally)}
ps := []*v1beta1.Pipeline{helloWorldPipelineWithRunAfter(t)}
ts := []*v1beta1.Task{simpleHelloWorldTask}
cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(newFeatureFlagsConfigMap())}
d := test.Data{
PipelineRuns: prs,
Pipelines: ps,
Tasks: ts,
ConfigMaps: cms,
}
prt := newPipelineRunTest(d, t)
defer prt.Cancel()
Expand Down Expand Up @@ -1908,7 +1906,7 @@ spec:
simpleHelloWorldTask,
simpleSomeTask,
}
cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(withEmbeddedStatus(newFeatureFlagsConfigMap(), embeddedStatus))}
cms := []*corev1.ConfigMap{withEmbeddedStatus(newFeatureFlagsConfigMap(), embeddedStatus)}

d := test.Data{
PipelineRuns: prs,
Expand Down Expand Up @@ -1991,13 +1989,11 @@ spec:
createHelloWorldTaskRun(t, "test-pipeline-run-cancelled-run-finally-final-task", "foo",
"test-pipeline-run-cancelled-run-finally", "test-pipeline"),
}
cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(newFeatureFlagsConfigMap())}
d := test.Data{
PipelineRuns: prs,
Pipelines: ps,
Tasks: ts,
TaskRuns: trs,
ConfigMaps: cms,
}
prt := newPipelineRunTest(d, t)
defer prt.Cancel()
Expand Down Expand Up @@ -2138,7 +2134,7 @@ func runTestReconcileOnCancelledRunFinallyPipelineRunWithFinalTaskAndRetries(t *
})}

ts := []*v1beta1.Task{simpleHelloWorldTask}
cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(withEmbeddedStatus(newFeatureFlagsConfigMap(), embeddedStatus))}
cms := []*corev1.ConfigMap{withEmbeddedStatus(newFeatureFlagsConfigMap(), embeddedStatus)}

d := test.Data{
PipelineRuns: prs,
Expand Down Expand Up @@ -2350,13 +2346,11 @@ status:
}
ps := []*v1beta1.Pipeline{tc.pipeline}
ts := []*v1beta1.Task{simpleHelloWorldTask}
cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(newFeatureFlagsConfigMap())}
d := test.Data{
PipelineRuns: []*v1beta1.PipelineRun{pr},
Pipelines: ps,
Tasks: ts,
TaskRuns: tc.taskRuns,
ConfigMaps: cms,
}
prt := newPipelineRunTest(d, t)
defer prt.Cancel()
Expand Down
5 changes: 0 additions & 5 deletions test/cancel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,6 @@ func TestTaskRunPipelineRunCancel(t *testing.T) {
ctx, cancel := context.WithCancel(ctx)
defer cancel()
requirements := []func(context.Context, *testing.T, *clients, string){}
if specStatus == v1beta1.PipelineRunSpecStatusCancelled {
requirements = append(requirements, requireAnyGate(map[string]string{
"enable-api-fields": "alpha",
}))
}
c, namespace := setup(ctx, t, requirements...)
t.Parallel()

Expand Down
6 changes: 3 additions & 3 deletions test/v1alpha1/cancel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (

"github.com/tektoncd/pipeline/test/parse"

"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
"gomodules.xyz/jsonpatch/v2"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -106,7 +106,7 @@ spec:
patches := []jsonpatch.JsonPatchOperation{{
Operation: "add",
Path: "/spec/status",
Value: v1alpha1.PipelineRunSpecStatusCancelled,
Value: v1beta1.PipelineRunSpecStatusCancelled,
}}
patchBytes, err := json.Marshal(patches)
if err != nil {
Expand All @@ -117,7 +117,7 @@ spec:
}

t.Logf("Waiting for PipelineRun %s in namespace %s to be cancelled", pipelineRunName, namespace)
if err := WaitForPipelineRunState(ctx, c, pipelineRunName, pipelineRunTimeout, FailedWithReason("PipelineRunCancelled", pipelineRunName), "PipelineRunCancelled"); err != nil {
if err := WaitForPipelineRunState(ctx, c, pipelineRunName, pipelineRunTimeout, FailedWithReason("Cancelled", pipelineRunName), "Cancelled"); err != nil {
t.Errorf("Error waiting for PipelineRun %q to finished: %s", pipelineRunName, err)
}

Expand Down

0 comments on commit 83807c4

Please sign in to comment.