Skip to content

Commit

Permalink
Use a feature flag for cancellation using entrypoint
Browse files Browse the repository at this point in the history
Cancellation using entrypoint binary is now behind a new feature flag
named 'enable-cancel-using-entrypoint'.

Signed-off-by: Arash Deshmeh <adeshmeh@ca.ibm.com>
  • Loading branch information
adshmh committed Mar 27, 2022
1 parent f6b9f3f commit b5bfc5a
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 1 deletion.
7 changes: 7 additions & 0 deletions pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ const (
DefaultSendCloudEventsForRuns = false
// DefaultEmbeddedStatus is the default value for "embedded-status".
DefaultEmbeddedStatus = FullEmbeddedStatus
// DefaultEnableCancelUsingEntrypoint is the default value for "enable-cancel-using-entrypoint"
DefaultEnableCancelUsingEntrypoint = false

disableAffinityAssistantKey = "disable-affinity-assistant"
disableCredsInitKey = "disable-creds-init"
Expand All @@ -70,6 +72,7 @@ const (
scopeWhenExpressionsToTask = "scope-when-expressions-to-task"
sendCloudEventsForRuns = "send-cloudevents-for-runs"
embeddedStatus = "embedded-status"
enableCancelUsingEntrypoint = "enable-cancel-using-entrypoint"
)

// FeatureFlags holds the features configurations
Expand All @@ -85,6 +88,7 @@ type FeatureFlags struct {
EnableAPIFields string
SendCloudEventsForRuns bool
EmbeddedStatus string
EnableCancelUsingEntrypoint bool
}

// GetFeatureFlagsConfigName returns the name of the configmap containing all
Expand Down Expand Up @@ -136,6 +140,9 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
if err := setEmbeddedStatus(cfgMap, DefaultEmbeddedStatus, &tc.EmbeddedStatus); err != nil {
return nil, err
}
if err := setFeature(enableCancelUsingEntrypoint, DefaultEnableCancelUsingEntrypoint, &tc.EnableCancelUsingEntrypoint); err != nil {
return nil, err
}

// Given that they are alpha features, Tekton Bundles and Custom Tasks should be switched on if
// enable-api-fields is "alpha". If enable-api-fields is not "alpha" then fall back to the value of
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/config/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
EnableAPIFields: "alpha",
SendCloudEventsForRuns: true,
EmbeddedStatus: "both",
EnableCancelUsingEntrypoint: true,
},
fileName: "feature-flags-all-flags-set",
},
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/config/testdata/feature-flags-all-flags-set.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ data:
enable-api-fields: "alpha"
send-cloudevents-for-runs: "true"
embedded-status: "both"
enable-cancel-using-entrypoint: "true"
15 changes: 14 additions & 1 deletion pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,12 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg
// If the TaskRun is cancelled, kill resources and update status
if tr.IsCancelled() {
message := fmt.Sprintf("TaskRun %q was cancelled", tr.Name)
err := c.cancelTaskRun(ctx, tr, v1beta1.TaskRunReasonCancelled, message)
var err error
if isCancelUsingEntrypointEnabled(ctx) {
err = c.cancelTaskRun(ctx, tr, v1beta1.TaskRunReasonCancelled, message)
} else {
err = c.failTaskRun(ctx, tr, v1beta1.TaskRunReasonCancelled, message)
}
return c.finishReconcileUpdateEmitEvents(ctx, tr, before, err)
}

Expand Down Expand Up @@ -866,3 +871,11 @@ func willOverwritePodSetAffinity(taskRun *v1beta1.TaskRun) bool {
}
return taskRun.Annotations[workspace.AnnotationAffinityAssistantName] != "" && podTemplate.Affinity != nil
}

// isCancelUsingEntrypointEnabled returns a bool indicating whether the cancellation of tasks
// should be performed using the entrypoint binary. The default behaviour is to delete the pods
// corresponding to a task.
func isCancelUsingEntrypointEnabled(ctx context.Context) bool {
cfg := config.FromContextOrDefaults(ctx)
return cfg.FeatureFlags.EnableCancelUsingEntrypoint
}
105 changes: 105 additions & 0 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1922,6 +1922,111 @@ func TestReconcileOnCancelledTaskRun(t *testing.T) {
}
}

func TestReconcileOnCancelledTaskRunWithFeatureFlag(t *testing.T) {
taskRun := &v1beta1.TaskRun{
ObjectMeta: objectMeta("test-taskrun-run-cancelled", "foo"),
Spec: v1beta1.TaskRunSpec{
TaskRef: &v1beta1.TaskRef{
Name: simpleTask.Name,
},
Status: v1beta1.TaskRunSpecStatusCancelled,
},
Status: v1beta1.TaskRunStatus{
Status: duckv1beta1.Status{
Conditions: duckv1beta1.Conditions{
apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionUnknown,
},
},
},
},
}
pod, err := makePod(taskRun, simpleTask)
if err != nil {
t.Fatalf("MakePod: %v", err)
}
taskRun.Status.PodName = pod.Name

expectedStatus := &apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse,
Reason: "TaskRunCancelled",
Message: `TaskRun "test-taskrun-run-cancelled" was cancelled`,
}

testCases := []struct {
name string
cancelEnabled bool
}{
{
name: "Taskrun cancellation with entrypoint cancel enabled",
cancelEnabled: true,
},
{
name: "Taskrun cancellation without entrypoint cancel",
cancelEnabled: false,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
d := test.Data{
TaskRuns: []*v1beta1.TaskRun{taskRun},
Tasks: []*v1beta1.Task{simpleTask},
Pods: []*corev1.Pod{pod},
}

if tc.cancelEnabled {
d.ConfigMaps = []*corev1.ConfigMap{
{
ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()},
Data: map[string]string{
"enable-cancel-using-entrypoint": "true",
},
},
}
}

testAssets, cancel := getTaskRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients

if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err != nil {
t.Fatalf("Unexpected error when reconciling completed TaskRun : %v", err)
}
newTr, err := clients.Pipeline.TektonV1beta1().TaskRuns(taskRun.Namespace).Get(testAssets.Ctx, taskRun.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("Expected completed TaskRun %s to exist but instead got error when getting it: %v", taskRun.Name, err)
}

if d := cmp.Diff(expectedStatus, newTr.Status.GetCondition(apis.ConditionSucceeded), ignoreLastTransitionTime); d != "" {
t.Fatalf("Did not get expected condition %s", diff.PrintWantGot(d))
}

wantEvents := []string{
"Normal Started",
"Warning Failed TaskRun \"test-taskrun-run-cancelled\" was cancelled",
}
err = eventstest.CheckEventsOrdered(t, testAssets.Recorder.Events, "test-reconcile-on-cancelled-taskrun", wantEvents)
if !(err == nil) {
t.Errorf(err.Error())
}

_, err = clients.Kube.CoreV1().Pods(pod.ObjectMeta.Namespace).Get(testAssets.Ctx, pod.ObjectMeta.Name, metav1.GetOptions{})
if tc.cancelEnabled {
if err != nil {
t.Errorf("unexpected error: %v", err)
}
} else {
if !k8sapierrors.IsNotFound(err) {
t.Errorf("expected a NotFound response, got: %v", err)
}
}
})
}
}

func TestReconcileTimeouts(t *testing.T) {
type testCase struct {
name string
Expand Down

0 comments on commit b5bfc5a

Please sign in to comment.