Dummy backend support cancel (#6390)

2026-04-15 01:41:56 +00:00 · 2026-04-06 20:21:23 +02:00
parent 02eceb9bb8
commit f0e56485dc
3 changed files with 170 additions and 26 deletions
@@ -49,8 +49,18 @@ const (
 	stepStateStarted   = "started"
 	stepStateDone      = "done"
 	testServiceTimeout = 1 * time.Second
+
+	// ExitCodeCanceled is the exit code returned when a step's context is
+	// canceled while it is sleeping. 130 matches the SIGINT shell convention
+	// (128 + signal 2) used by real container runtimes.
+	ExitCodeCanceled = 130
 )

+// stepKey returns the kv-store key for a step's state.
+func stepKey(taskUUID, stepUUID string) string {
+	return "task_" + taskUUID + "_step_" + stepUUID
+}
+
 // New returns a dummy backend.
 func New() backend_types.Backend {
 	return &dummy{
@@ -94,7 +104,8 @@ func (e *dummy) StartStep(_ context.Context, step *backend_types.Step, taskUUID
 	if !exist {
 		return fmt.Errorf("expect env of workflow %s to exist but found none to destroy", taskUUID)
 	}
-	stepState, stepExist := e.kv.Load(fmt.Sprintf("task_%s_step_%s", taskUUID, step.UUID))
+	key := stepKey(taskUUID, step.UUID)
+	stepState, stepExist := e.kv.Load(key)
 	if stepExist {
 		// Detect issues like https://github.com/woodpecker-ci/woodpecker/issues/3494
 		return fmt.Errorf("StartStep detected already started step '%s' (%s) in state: %s", step.Name, step.UUID, stepState)
@@ -109,10 +120,31 @@ func (e *dummy) StartStep(_ context.Context, step *backend_types.Step, taskUUID
 		return fmt.Errorf("expected step type '%s' but got '%s'", expectStepType, step.Type)
 	}

-	e.kv.Store(fmt.Sprintf("task_%s_step_%s", taskUUID, step.UUID), stepStateStarted)
+	e.kv.Store(key, stepStateStarted)
 	return nil
 }

+// canceledState returns the state for a step whose context was canceled.
+func canceledState() *backend_types.State {
+	return &backend_types.State{ExitCode: ExitCodeCanceled, Exited: true}
+}
+
+// sleepWithContext blocks for the given duration or until ctx is canceled.
+// Returns true if canceled, false if the sleep completed normally.
+func sleepWithContext(ctx context.Context, d time.Duration) (canceled bool) {
+	if ctx.Err() != nil {
+		return true
+	}
+	t := time.NewTimer(d)
+	defer t.Stop()
+	select {
+	case <-t.C:
+		return false
+	case <-ctx.Done():
+		return true
+	}
+}
+
 func (e *dummy) WaitStep(ctx context.Context, step *backend_types.Step, taskUUID string) (*backend_types.State, error) {
 	log.Trace().Str("taskUUID", taskUUID).Msgf("wait for step %s", step.Name)

@@ -122,8 +154,10 @@ func (e *dummy) WaitStep(ctx context.Context, step *backend_types.Step, taskUUID
 		return &backend_types.State{Error: err}, err
 	}

+	key := stepKey(taskUUID, step.UUID)
+
 	// check state
-	stepState, stepExist := e.kv.Load(fmt.Sprintf("task_%s_step_%s", taskUUID, step.UUID))
+	stepState, stepExist := e.kv.Load(key)
 	if !stepExist {
 		err := fmt.Errorf("WaitStep expect step '%s' (%s) to be created but found none", step.Name, step.UUID)
 		return &backend_types.State{Error: err}, err
@@ -133,29 +167,28 @@ func (e *dummy) WaitStep(ctx context.Context, step *backend_types.Step, taskUUID
 		return &backend_types.State{Error: err}, err
 	}

-	// extend wait time logic
 	if sleep, sleepExist := step.Environment[EnvKeyStepSleep]; sleepExist {
 		toSleep, err := time.ParseDuration(sleep)
 		if err != nil {
 			err = fmt.Errorf("WaitStep fail to parse sleep duration: %w", err)
 			return &backend_types.State{Error: err}, err
 		}
-		time.Sleep(toSleep)
-	} else {
-		if step.Type == backend_types.StepTypeService {
-			select {
-			case <-time.NewTimer(testServiceTimeout).C:
-				err := fmt.Errorf("WaitStep fail due to timeout of service after 1 second")
-				return &backend_types.State{Error: err}, err
-			case <-ctx.Done():
-				// context for service closed ... we can move forward
-			}
-		} else {
-			time.Sleep(time.Nanosecond)
+		if sleepWithContext(ctx, toSleep) {
+			e.kv.Store(key, stepStateDone)
+			return canceledState(), nil
 		}
+	} else if step.Type == backend_types.StepTypeService {
+		if sleepWithContext(ctx, testServiceTimeout) {
+			// context for service closed — we can move forward
+		} else {
+			err := fmt.Errorf("WaitStep fail due to timeout of service after 1 second")
+			return &backend_types.State{Error: err}, err
+		}
+	} else {
+		time.Sleep(time.Nanosecond)
 	}

-	e.kv.Store(fmt.Sprintf("task_%s_step_%s", taskUUID, step.UUID), stepStateDone)
+	e.kv.Store(key, stepStateDone)

 	oomKilled, _ := strconv.ParseBool(step.Environment[EnvKeyStepOOMKilled])
 	exitCode := 0
@@ -179,13 +212,15 @@ func (e *dummy) TailStep(_ context.Context, step *backend_types.Step, taskUUID s
 		return nil, fmt.Errorf("expect env of workflow %s to exist but found none to destroy", taskUUID)
 	}

+	key := stepKey(taskUUID, step.UUID)
+
 	// check state
-	stepState, stepExist := e.kv.Load(fmt.Sprintf("task_%s_step_%s", taskUUID, step.UUID))
+	stepState, stepExist := e.kv.Load(key)
 	if !stepExist {
-		return nil, fmt.Errorf("WaitStep expect step '%s' (%s) to be created but found none", step.Name, step.UUID)
+		return nil, fmt.Errorf("TailStep expect step '%s' (%s) to be created but found none", step.Name, step.UUID)
 	}
 	if stepState != stepStateStarted {
-		return nil, fmt.Errorf("WaitStep expect step '%s' (%s) to be '%s' but it is: %s", step.Name, step.UUID, stepStateStarted, stepState)
+		return nil, fmt.Errorf("TailStep expect step '%s' (%s) to be '%s' but it is: %s", step.Name, step.UUID, stepStateStarted, stepState)
 	}

 	if tailShouldFail, _ := strconv.ParseBool(step.Environment[EnvKeyStepTailFail]); tailShouldFail {
@@ -203,16 +238,20 @@ func (e *dummy) DestroyStep(_ context.Context, step *backend_types.Step, taskUUI
 		return nil
 	}

+	key := stepKey(taskUUID, step.UUID)
+
 	// check state
-	stepState, stepExist := e.kv.Load(fmt.Sprintf("task_%s_step_%s", taskUUID, step.UUID))
+	stepState, stepExist := e.kv.Load(key)
 	if !stepExist {
-		return fmt.Errorf("WaitStep expect step '%s' (%s) to be created but found none", step.Name, step.UUID)
+		return fmt.Errorf("DestroyStep expect step '%s' (%s) to be created but found none", step.Name, step.UUID)
 	}
-	if stepState != stepStateDone {
-		return fmt.Errorf("WaitStep expect step '%s' (%s) to be '%s' but it is: %s", step.Name, step.UUID, stepStateDone, stepState)
+	// Allow destroying a step in 'started' state: this happens when the
+	// workflow context is canceled before WaitStep completes.
+	if stepState != stepStateDone && stepState != stepStateStarted {
+		return fmt.Errorf("DestroyStep expect step '%s' (%s) to be '%s' or '%s' but it is: %s", step.Name, step.UUID, stepStateDone, stepStateStarted, stepState)
 	}

-	e.kv.Delete(fmt.Sprintf("task_%s_step_%s", taskUUID, step.UUID))
+	e.kv.Delete(key)
 	return nil
 }

@@ -15,10 +15,12 @@
 package dummy_test

 import (
+	"context"
 	"io"
 	"testing"

 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"

 	"go.woodpecker-ci.org/woodpecker/v3/pipeline/backend/dummy"
 	"go.woodpecker-ci.org/woodpecker/v3/pipeline/backend/types"
@@ -31,7 +33,7 @@ func TestSmalPipelineDummyRun(t *testing.T) {
 	assert.True(t, dummyEngine.IsAvailable(ctx))
 	assert.EqualValues(t, "dummy", dummyEngine.Name())
 	_, err := dummyEngine.Load(ctx)
-	assert.NoError(t, err)
+	require.NoError(t, err)

 	assert.Error(t, dummyEngine.SetupWorkflow(ctx, nil, dummy.WorkflowSetupFailUUID))

@@ -162,3 +164,37 @@ echo nein
 		assert.NoError(t, dummyEngine.DestroyWorkflow(ctx, nil, workflowUUID))
 	})
 }
+
+func TestWaitStepCanceledBySleep(t *testing.T) {
+	ctx, cancel := context.WithCancelCause(t.Context())
+
+	dummyEngine := dummy.New()
+	_, err := dummyEngine.Load(ctx)
+	require.NoError(t, err)
+
+	const taskUUID = "cancel-task"
+	assert.NoError(t, dummyEngine.SetupWorkflow(ctx, nil, taskUUID))
+
+	step := &types.Step{
+		Name: "slow-step",
+		UUID: "slow-uuid",
+		Type: types.StepTypeCommands,
+		Environment: map[string]string{
+			dummy.EnvKeyStepSleep: "30s",
+		},
+	}
+	assert.NoError(t, dummyEngine.StartStep(ctx, step, taskUUID))
+
+	// Cancel before WaitStep — the pre-select ctx.Err() check handles this deterministically.
+	cancel(nil)
+
+	state, err := dummyEngine.WaitStep(ctx, step, taskUUID)
+	assert.NoError(t, err, "WaitStep should not return an error on cancellation")
+	assert.True(t, state.Exited, "step should be marked as exited")
+	assert.Equal(t, dummy.ExitCodeCanceled, state.ExitCode,
+		"canceled step must exit with code %d", dummy.ExitCodeCanceled)
+
+	// DestroyStep must succeed even though the step was canceled mid-sleep.
+	assert.NoError(t, dummyEngine.DestroyStep(ctx, step, taskUUID))
+	assert.NoError(t, dummyEngine.DestroyWorkflow(ctx, nil, taskUUID))
+}
@@ -21,6 +21,7 @@ import (
 	"errors"
 	"fmt"
 	"testing"
+	"time"

 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -1154,3 +1155,71 @@ func TestWorkflowContextCancelWithServiceStep(t *testing.T) {

 	assert.ErrorIs(t, err, pipeline_errors.ErrCancel)
 }
+
+// TestWorkflowCancelDuringStepSleep verifies that canceling the workflow context
+// while a step is sleeping (via SLEEP env) causes the runtime to return ErrCancel
+// promptly — without waiting the full sleep duration — and that subsequent stages
+// are never executed.
+//
+// The tracer callback cancels the context the moment the first stage ("prepare")
+// completes. The "slow" step uses a short sleep so that even if WaitStep enters
+// the sleep select, the context cancellation unblocks it quickly.
+//
+// Note: we do not assert on the slow step's exit code here because Run() may
+// return (via ctx.Done()) before the stage goroutine's WaitStep completes,
+// causing DestroyWorkflow to clean up state that WaitStep still needs. The
+// exit-code-130 behavior of a canceled sleep is verified at the backend unit
+// level in TestWaitStepCanceledBySleep.
+func TestWorkflowCancelDuringStepSleep(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithCancelCause(t.Context())
+
+	var prepareExited int
+	tracer := tracer_mocks.NewMockTracer(t)
+	tracer.On("Trace", mock.Anything).Run(func(args mock.Arguments) {
+		s, _ := args.Get(0).(*state.State)
+		if s == nil || s.CurrStep == nil {
+			return
+		}
+		// Cancel as soon as the first stage ("prepare") finishes.
+		if s.CurrStep.Name == "prepare" && s.CurrStepState.Exited {
+			prepareExited++
+			if prepareExited >= 1 {
+				cancel(nil)
+			}
+		}
+	}).Return(nil).Maybe()
+
+	r := New(
+		&backend_types.Config{
+			Stages: []*backend_types.Stage{
+				{Steps: []*backend_types.Step{
+					cmdStep("prepare"),
+				}},
+				{Steps: []*backend_types.Step{
+					// Short sleep so the test doesn't hang if WaitStep enters the timer.
+					cmdStep("slow", func(s *backend_types.Step) {
+						s.Environment[dummy.EnvKeyStepSleep] = "100ms"
+					}),
+				}},
+				{Steps: []*backend_types.Step{
+					cmdStep("never-reached"),
+				}},
+			},
+		},
+		dummy.New(),
+		WithTracer(tracer),
+		WithContext(ctx),
+		WithLogger(newTestLogger(t)),
+	)
+
+	err := r.Run(t.Context())
+	assert.ErrorIs(t, err, pipeline_errors.ErrCancel, "canceled workflow must return ErrCancel")
+
+	// Give the orphaned stage goroutine a moment to finish tracing (best effort).
+	time.Sleep(200 * time.Millisecond)
+
+	assert.Nil(t, findFirstTraceByName(getTracerStates(tracer), "never-reached"),
+		"never-reached must not have been traced")
+}