Skip to content

feat: add cleanup strategy to loadtest #4991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 10, 2022
Merged

feat: add cleanup strategy to loadtest #4991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cli/loadtest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,9 @@ func loadtest() *cobra.Command {
client.PropagateTracing = tracePropagate

// Prepare the test.
strategy := config.Strategy.ExecutionStrategy()
th := harness.NewTestHarness(strategy)
runStrategy := config.Strategy.ExecutionStrategy()
cleanupStrategy := config.CleanupStrategy.ExecutionStrategy()
th := harness.NewTestHarness(runStrategy, cleanupStrategy)

for i, t := range config.Tests {
name := fmt.Sprintf("%s-%d", t.Type, i)
Expand Down
10 changes: 10 additions & 0 deletions cli/loadtest_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ func TestLoadTest(t *testing.T) {
Strategy: cli.LoadTestStrategy{
Type: cli.LoadTestStrategyTypeLinear,
},
CleanupStrategy: cli.LoadTestStrategy{
Type: cli.LoadTestStrategyTypeLinear,
},
Tests: []cli.LoadTest{
{
Type: cli.LoadTestTypePlacebo,
Expand Down Expand Up @@ -89,6 +92,10 @@ func TestLoadTest(t *testing.T) {
Type: cli.LoadTestStrategyTypeConcurrent,
ConcurrencyLimit: 2,
},
CleanupStrategy: cli.LoadTestStrategy{
Type: cli.LoadTestStrategyTypeConcurrent,
ConcurrencyLimit: 2,
},
Tests: []cli.LoadTest{
{
Type: cli.LoadTestTypeWorkspaceBuild,
Expand Down Expand Up @@ -210,6 +217,9 @@ func TestLoadTest(t *testing.T) {
Strategy: cli.LoadTestStrategy{
Type: cli.LoadTestStrategyTypeLinear,
},
CleanupStrategy: cli.LoadTestStrategy{
Type: cli.LoadTestStrategyTypeLinear,
},
Tests: []cli.LoadTest{
{
Type: cli.LoadTestTypePlacebo,
Expand Down
9 changes: 7 additions & 2 deletions cli/loadtestconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ import (

// LoadTestConfig is the overall configuration for a call to `coder loadtest`.
type LoadTestConfig struct {
Strategy LoadTestStrategy `json:"strategy"`
Tests []LoadTest `json:"tests"`
Strategy LoadTestStrategy `json:"strategy"`
CleanupStrategy LoadTestStrategy `json:"cleanup_strategy"`
Tests []LoadTest `json:"tests"`
// Timeout sets a timeout for the entire test run, to control the timeout
// for each individual run use strategy.timeout.
Timeout httpapi.Duration `json:"timeout"`
Expand Down Expand Up @@ -134,6 +135,10 @@ func (c *LoadTestConfig) Validate() error {
if err != nil {
return xerrors.Errorf("validate strategy: %w", err)
}
err = c.CleanupStrategy.Validate()
if err != nil {
return xerrors.Errorf("validate cleanup_strategy: %w", err)
}

for i, test := range c.Tests {
err := test.Validate()
Expand Down
66 changes: 40 additions & 26 deletions loadtest/harness/harness.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,11 @@ import (
"github.com/coder/coder/coderd/tracing"
)

// ExecutionStrategy defines how a TestHarness should execute a set of runs. It
// essentially defines the concurrency model for a given testing session.
type ExecutionStrategy interface {
// Execute runs the given runs in whatever way the strategy wants. An error
// may only be returned if the strategy has a failure itself, not if any of
// the runs fail.
Execute(ctx context.Context, runs []*TestRun) error
}

// TestHarness runs a bunch of registered test runs using the given
// ExecutionStrategy.
// TestHarness runs a bunch of registered test runs using the given execution
// strategies.
type TestHarness struct {
strategy ExecutionStrategy
runStrategy ExecutionStrategy
cleanupStrategy ExecutionStrategy

mut *sync.Mutex
runIDs map[string]struct{}
Expand All @@ -33,14 +25,15 @@ type TestHarness struct {
elapsed time.Duration
}

// NewTestHarness creates a new TestHarness with the given ExecutionStrategy.
func NewTestHarness(strategy ExecutionStrategy) *TestHarness {
// NewTestHarness creates a new TestHarness with the given execution strategies.
func NewTestHarness(runStrategy, cleanupStrategy ExecutionStrategy) *TestHarness {
return &TestHarness{
strategy: strategy,
mut: new(sync.Mutex),
runIDs: map[string]struct{}{},
runs: []*TestRun{},
done: make(chan struct{}),
runStrategy: runStrategy,
cleanupStrategy: cleanupStrategy,
mut: new(sync.Mutex),
runIDs: map[string]struct{}{},
runs: []*TestRun{},
done: make(chan struct{}),
}
}

Expand All @@ -62,11 +55,16 @@ func (h *TestHarness) Run(ctx context.Context) (err error) {
h.started = true
h.mut.Unlock()

runFns := make([]TestFn, len(h.runs))
for i, run := range h.runs {
runFns[i] = run.Run
}

defer close(h.done)
defer func() {
e := recover()
if e != nil {
err = xerrors.Errorf("execution strategy panicked: %w", e)
err = xerrors.Errorf("panic in harness.Run: %+v", e)
}
}()

Expand All @@ -77,7 +75,9 @@ func (h *TestHarness) Run(ctx context.Context) (err error) {
h.elapsed = time.Since(start)
}()

err = h.strategy.Execute(ctx, h.runs)
// We don't care about test failures here since they already get recorded
// by the *TestRun.
_, err = h.runStrategy.Run(ctx, runFns)
//nolint:revive // we use named returns because we mutate it in a defer
return
}
Expand All @@ -96,20 +96,34 @@ func (h *TestHarness) Cleanup(ctx context.Context) (err error) {
panic("harness has not finished")
}

cleanupFns := make([]TestFn, len(h.runs))
for i, run := range h.runs {
cleanupFns[i] = run.Cleanup
}

defer func() {
e := recover()
if e != nil {
err = multierror.Append(err, xerrors.Errorf("panic in cleanup: %w", e))
err = xerrors.Errorf("panic in harness.Cleanup: %+v", e)
}
}()

for _, run := range h.runs {
e := run.Cleanup(ctx)
if e != nil {
err = multierror.Append(err, xerrors.Errorf("cleanup for %s failed: %w", run.FullID(), e))
var cleanupErrs []error
cleanupErrs, err = h.cleanupStrategy.Run(ctx, cleanupFns)
if err != nil {
err = xerrors.Errorf("cleanup strategy error: %w", err)
//nolint:revive // we use named returns because we mutate it in a defer
return
}

var merr error
for _, cleanupErr := range cleanupErrs {
if cleanupErr != nil {
merr = multierror.Append(merr, cleanupErr)
}
}

err = merr
//nolint:revive // we use named returns because we mutate it in a defer
return
}
70 changes: 54 additions & 16 deletions loadtest/harness/harness_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type panickingExecutionStrategy struct{}

var _ harness.ExecutionStrategy = panickingExecutionStrategy{}

func (panickingExecutionStrategy) Execute(_ context.Context, _ []*harness.TestRun) error {
func (panickingExecutionStrategy) Run(_ context.Context, _ []harness.TestFn) ([]error, error) {
panic(testPanicMessage)
}

Expand All @@ -28,8 +28,8 @@ type erroringExecutionStrategy struct {

var _ harness.ExecutionStrategy = erroringExecutionStrategy{}

func (e erroringExecutionStrategy) Execute(_ context.Context, _ []*harness.TestRun) error {
return e.err
func (e erroringExecutionStrategy) Run(_ context.Context, _ []harness.TestFn) ([]error, error) {
return []error{}, e.err
}

func Test_TestHarness(t *testing.T) {
Expand All @@ -40,7 +40,7 @@ func Test_TestHarness(t *testing.T) {

expectedErr := xerrors.New("expected error")

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
r1 := h.AddRun("test", "1", fakeTestFns(nil, nil))
r2 := h.AddRun("test", "2", fakeTestFns(expectedErr, nil))

Expand All @@ -65,7 +65,7 @@ func Test_TestHarness(t *testing.T) {

expectedErr := xerrors.New("expected error")

h := harness.NewTestHarness(erroringExecutionStrategy{err: expectedErr})
h := harness.NewTestHarness(erroringExecutionStrategy{err: expectedErr}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", fakeTestFns(nil, nil))

err := h.Run(context.Background())
Expand All @@ -76,7 +76,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("CatchesExecutionPanic", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(panickingExecutionStrategy{})
h := harness.NewTestHarness(panickingExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", fakeTestFns(nil, nil))

err := h.Run(context.Background())
Expand All @@ -93,7 +93,7 @@ func Test_TestHarness(t *testing.T) {

expectedErr := xerrors.New("expected error")

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", fakeTestFns(nil, expectedErr))

err := h.Run(context.Background())
Expand All @@ -107,7 +107,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("Panic", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", testFns{
RunFn: func(_ context.Context, _ string, _ io.Writer) error {
return nil
Expand All @@ -125,6 +125,44 @@ func Test_TestHarness(t *testing.T) {
require.ErrorContains(t, err, "panic")
require.ErrorContains(t, err, testPanicMessage)
})

t.Run("CatchesExecutionError", func(t *testing.T) {
t.Parallel()

expectedErr := xerrors.New("expected error")

h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, erroringExecutionStrategy{err: expectedErr})
_ = h.AddRun("test", "1", fakeTestFns(nil, nil))

err := h.Run(context.Background())
require.NoError(t, err)

err = h.Cleanup(context.Background())
require.Error(t, err)
require.ErrorIs(t, err, expectedErr)
})

t.Run("CatchesExecutionPanic", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, panickingExecutionStrategy{})
_ = h.AddRun("test", "1", testFns{
RunFn: func(_ context.Context, _ string, _ io.Writer) error {
return nil
},
CleanupFn: func(_ context.Context, _ string) error {
return nil
},
})

err := h.Run(context.Background())
require.NoError(t, err)

err = h.Cleanup(context.Background())
require.Error(t, err)
require.ErrorContains(t, err, "panic")
require.ErrorContains(t, err, testPanicMessage)
})
})

t.Run("Panics", func(t *testing.T) {
Expand All @@ -133,7 +171,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("RegisterAfterStart", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.Run(context.Background())

require.Panics(t, func() {
Expand All @@ -144,7 +182,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("DuplicateTestID", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})

name, id := "test", "1"
_ = h.AddRun(name, id, fakeTestFns(nil, nil))
Expand All @@ -157,7 +195,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("StartedTwice", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
h.Run(context.Background())

require.Panics(t, func() {
Expand All @@ -168,7 +206,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("ResultsBeforeStart", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})

require.Panics(t, func() {
h.Results()
Expand All @@ -183,7 +221,7 @@ func Test_TestHarness(t *testing.T) {
endRun = make(chan struct{})
testsEnded = make(chan struct{})
)
h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", testFns{
RunFn: func(_ context.Context, _ string, _ io.Writer) error {
close(started)
Expand All @@ -210,22 +248,22 @@ func Test_TestHarness(t *testing.T) {
t.Run("CleanupBeforeStart", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})

require.Panics(t, func() {
h.Cleanup(context.Background())
})
})

t.Run("ClenaupBeforeFinish", func(t *testing.T) {
t.Run("CleanupBeforeFinish", func(t *testing.T) {
t.Parallel()

var (
started = make(chan struct{})
endRun = make(chan struct{})
testsEnded = make(chan struct{})
)
h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", testFns{
RunFn: func(_ context.Context, _ string, _ io.Writer) error {
close(started)
Expand Down
Loading