enable reaper routine by default

cmd/drone-server/config/config.go

@@ -51,6 +51,7 @@ type (
 		Agent        Agent
 		AzureBlob    AzureBlob
 		Convert      Convert
+		Cleanup      Cleanup
 		Cron         Cron
 		Cloning      Cloning
 		Database     Database
@@ -96,6 +97,13 @@ type (
 		Pull       string `envconfig:"DRONE_GIT_IMAGE_PULL" default:"IfNotExists"`
 	}
 
+	Cleanup struct {
+		Disabled  bool         `envconfig:"DRONE_CLEANUP_DISABLED"`
+		Interval time.Duration `envconfig:"DRONE_CLEANUP_INTERVAL"         default:"24h"`
+		Running  time.Duration `envconfig:"DRONE_CLEANUP_DEADLINE_RUNNING" default:"24h"`
+		Pending  time.Duration `envconfig:"DRONE_CLEANUP_DEADLINE_PENDING" default:"24h"`
+	}
+
 	// Cron provides the cron configuration.
 	Cron struct {
 		Disabled bool          `envconfig:"DRONE_CRON_DISABLED"`

cmd/drone-server/inject_service.go

@@ -23,6 +23,7 @@ import (
 	"github.com/drone/drone/metric/sink"
 	"github.com/drone/drone/pubsub"
 	"github.com/drone/drone/service/canceler"
+	"github.com/drone/drone/service/canceler/reaper"
 	"github.com/drone/drone/service/commit"
 	contents "github.com/drone/drone/service/content"
 	"github.com/drone/drone/service/content/cache"
@@ -64,6 +65,7 @@ var serviceSet = wire.NewSet(
 	provideHookService,
 	provideNetrcService,
 	provideOrgService,
+	provideReaper,
 	provideSession,
 	provideStatusService,
 	provideSyncer,
@@ -170,6 +172,25 @@ func provideSystem(config config.Config) *core.System {
 	}
 }
 
+// provideReaper is a Wire provider function that returns the
+// zombie build reaper.
+func provideReaper(
+	repos core.RepositoryStore,
+	builds core.BuildStore,
+	stages core.StageStore,
+	canceler core.Canceler,
+	config config.Config,
+) *reaper.Reaper {
+	return reaper.New(
+		repos,
+		builds,
+		stages,
+		canceler,
+		config.Cleanup.Running,
+		config.Cleanup.Pending,
+	)
+}
+
 // provideDatadog is a Wire provider function that returns the
 // datadog sink.
 func provideDatadog(

cmd/drone-server/main.go

@@ -24,6 +24,7 @@ import (
 	"github.com/drone/drone/core"
 	"github.com/drone/drone/metric/sink"
 	"github.com/drone/drone/operator/runner"
+	"github.com/drone/drone/service/canceler/reaper"
 	"github.com/drone/drone/server"
 	"github.com/drone/drone/trigger/cron"
 	"github.com/drone/signal"
@@ -114,6 +115,18 @@ func main() {
 		return app.cron.Start(ctx, config.Cron.Interval)
 	})
 
+	// launches the reaper in a goroutine. If the reaper
+	// is disabled, the goroutine exits immediately
+	// without error.
+	g.Go(func() (err error) {
+		if config.Cleanup.Disabled {
+			return nil
+		}
+		logrus.WithField("interval", config.Cleanup.Interval.String()).
+			Infoln("starting the zombie build reaper")
+		return app.reaper.Start(ctx, config.Cleanup.Interval)
+	})
+
 	// launches the build runner in a goroutine. If the local
 	// runner is disabled (because nomad or kubernetes is enabled)
 	// then the goroutine exits immediately without error.
@@ -154,6 +167,7 @@ func initLogging(c config.Config) {
 // application is the main struct for the Drone server.
 type application struct {
 	cron   *cron.Scheduler
+	reaper *reaper.Reaper
 	sink   *sink.Datadog
 	runner *runner.Runner
 	server *server.Server
@@ -163,6 +177,7 @@ type application struct {
 // newApplication creates a new application struct.
 func newApplication(
 	cron *cron.Scheduler,
+	reaper *reaper.Reaper,
 	sink *sink.Datadog,
 	runner *runner.Runner,
 	server *server.Server,
@@ -173,5 +188,6 @@ func newApplication(
 		sink:   sink,
 		server: server,
 		runner: runner,
+		reaper: reaper,
 	}
 }

cmd/drone-server/wire_gen.go

@@ -63,6 +63,7 @@ func InitializeApplication(config2 config.Config) (application, error) {
 	validateService := provideValidatePlugin(config2)
 	triggerer := trigger.New(coreCanceler, configService, convertService, commitService, statusService, buildStore, scheduler, repositoryStore, userStore, validateService, webhookSender)
 	cronScheduler := cron2.New(commitService, cronStore, repositoryStore, userStore, triggerer)
+	reaper := provideReaper(repositoryStore, buildStore, stageStore, coreCanceler, config2)
 	coreLicense := provideLicense(client, config2)
 	datadog := provideDatadog(userStore, repositoryStore, buildStore, system, coreLicense, config2)
 	logStore := provideLogStore(db, config2)
@@ -104,6 +105,6 @@ func InitializeApplication(config2 config.Config) (application, error) {
 	mainPprofHandler := providePprof(config2)
 	mux := provideRouter(server, webServer, mainRpcHandlerV1, mainRpcHandlerV2, mainHealthzHandler, metricServer, mainPprofHandler)
 	serverServer := provideServer(mux, config2)
-	mainApplication := newApplication(cronScheduler, datadog, runner, serverServer, userStore)
+	mainApplication := newApplication(cronScheduler, reaper, datadog, runner, serverServer, userStore)
 	return mainApplication, nil
 }

service/canceler/reaper/reaper.go

@@ -28,14 +28,54 @@ type Reaper struct {
 	Builds   core.BuildStore
 	Stages   core.StageStore
 	Canceler core.Canceler
+	Pending  time.Duration // Pending is the pending pipeline deadline
+	Running  time.Duration // Running is the running pipeline deadline
+}
+
+// New returns a new Reaper.
+func New(
+	repos core.RepositoryStore,
+	builds core.BuildStore,
+	stages core.StageStore,
+	canceler core.Canceler,
+	running time.Duration,
+	pending time.Duration,
+) *Reaper {
+	if running == 0 {
+		running = time.Hour * 24
+	}
+	if pending == 0 {
+		pending = time.Hour * 24
+	}
+	return &Reaper{
+		Repos:    repos,
+		Builds:   builds,
+		Stages:   stages,
+		Canceler: canceler,
+		Pending:  pending,
+		Running:  running,
+	}
 }
 
 // TODO use multierror to aggregate errors encountered
 // TODO use trace logging
 
-func (r *Reaper) reap(ctx context.Context) error {
+// Start starts the reaper.
-	ttl := time.Hour*24
+func (r *Reaper) Start(ctx context.Context, dur time.Duration) error {
+	ticker := time.NewTicker(dur)
+	defer ticker.Stop()
 
+	for {
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		case <-ticker.C:
+			r.reap(ctx)
+		}
+	}
+}
+
+func (r *Reaper) reap(ctx context.Context) error {
 	pending, err := r.Builds.Pending(ctx)
 	if err != nil {
 		return err
@@ -43,7 +83,7 @@ func (r *Reaper) reap(ctx context.Context) error {
 	for _, build := range pending {
 		// if a build is pending for longer than the maximum
 		// pending time limit, the build is maybe cancelled.
-		if isExceeded(build.Created, ttl, buffer) {
+		if isExceeded(build.Created, r.Pending, buffer) {
 			err = r.reapMaybe(ctx, build)
 			if err != nil {
 				return err
@@ -58,7 +98,7 @@ func (r *Reaper) reap(ctx context.Context) error {
 	for _, build := range running {
 		// if a build is running for longer than the maximum
 		// running time limit, the build is maybe cancelled.
-		if isExceeded(build.Started, ttl, buffer) {
+		if isExceeded(build.Started, r.Running, buffer) {
 			err = r.reapMaybe(ctx, build)
 			if err != nil {
 				return err
@@ -91,7 +131,7 @@ func (r *Reaper) reapMaybe(ctx context.Context, build *core.Build) error {
 		if stage.IsDone() {
 			continue
 		}
-		if stage.Started > started  {
+		if stage.Started > started {
 			started = stage.Started
 		}
 	}

service/canceler/reaper/reaper_test.go

@@ -17,6 +17,137 @@ import (
 
 var nocontext = context.Background()
 
+//
+// reap tests
+//
+
+// this test confirms that pending builds that
+// exceed the deadline are canceled, and pending
+// builds that do not exceed the deadline are
+// ignored.
+func TestReapPending(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	defer func() {
+		now = time.Now
+	}()
+	now = func() time.Time {
+		return mustParse("2006-01-02T15:00:00")
+	}
+
+	mockRepo := &core.Repository{
+		ID: 2,
+	}
+	mockBuild := &core.Build{
+		ID:      1,
+		RepoID:  mockRepo.ID,
+		Status:  core.StatusPending,
+		Created: mustParse("2006-01-01T00:00:00").Unix(), // expire > 24 hours, must cancel
+	}
+	mockPending := []*core.Build{
+		mockBuild,
+		{
+			ID:      2,
+			RepoID:  mockRepo.ID,
+			Status:  core.StatusPending,
+			Created: mustParse("2006-01-02T14:30:00").Unix(), // expire < 1 hours, must ignore
+		},
+	}
+
+	repos := mock.NewMockRepositoryStore(controller)
+	repos.EXPECT().Find(gomock.Any(), mockBuild.RepoID).Return(mockRepo, nil).Times(1)
+
+	builds := mock.NewMockBuildStore(controller)
+	builds.EXPECT().Pending(gomock.Any()).Return(mockPending, nil)
+	builds.EXPECT().Running(gomock.Any()).Return(nil, nil)
+
+	canceler := mock.NewMockCanceler(controller)
+	canceler.EXPECT().Cancel(gomock.Any(), mockRepo, mockBuild)
+
+	r := New(
+		repos,
+		builds,
+		nil,
+		canceler,
+		time.Hour*24,
+		time.Hour*24,
+	)
+
+	r.reap(nocontext)
+}
+
+// this test confirms that running builds that
+// exceed the deadline are canceled, and running
+// builds that do not exceed the deadline are
+// ignored.
+func TestReapRunning(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	defer func() {
+		now = time.Now
+	}()
+	now = func() time.Time {
+		return mustParse("2006-01-02T15:00:00")
+	}
+
+	mockRepo := &core.Repository{
+		ID:      2,
+		Timeout: 60,
+	}
+	mockBuild := &core.Build{
+		ID:      1,
+		RepoID:  mockRepo.ID,
+		Status:  core.StatusRunning,
+		Started: mustParse("2006-01-01T00:00:00").Unix(), // expire > 24 hours, must cancel
+	}
+	mockRunning := []*core.Build{
+		mockBuild,
+		{
+			ID:      2,
+			RepoID:  mockRepo.ID,
+			Status:  core.StatusRunning,
+			Started: mustParse("2006-01-02T14:30:00").Unix(), // expire < 1 hours, must ignore
+		},
+	}
+	mockStages := []*core.Stage{
+		{
+			BuildID: mockBuild.ID,
+			Status:  core.StatusPending,
+			Started: 0,
+		},
+	}
+
+	repos := mock.NewMockRepositoryStore(controller)
+	repos.EXPECT().Find(gomock.Any(), mockBuild.RepoID).Return(mockRepo, nil).Times(1)
+
+	builds := mock.NewMockBuildStore(controller)
+	builds.EXPECT().Pending(gomock.Any()).Return(nil, nil)
+	builds.EXPECT().Running(gomock.Any()).Return(mockRunning, nil)
+
+	stages := mock.NewMockStageStore(controller)
+	stages.EXPECT().List(gomock.Any(), mockBuild.ID).Return(mockStages, nil)
+
+	canceler := mock.NewMockCanceler(controller)
+	canceler.EXPECT().Cancel(gomock.Any(), mockRepo, mockBuild)
+
+	r := New(
+		repos,
+		builds,
+		stages,
+		canceler,
+		time.Hour*24,
+		time.Hour*24,
+	)
+
+	r.reap(nocontext)
+}
+
+//
+// reap maybe tests
+//
+
 // this test confirms that the build is cancelled
 // if the build status is pending.
 func TestReapPendingMaybe(t *testing.T) {
@@ -24,24 +155,23 @@ func TestReapPendingMaybe(t *testing.T) {
 	defer controller.Finish()
 
 	mockBuild := &core.Build{
-		ID: 1,
+		ID:     1,
 		RepoID: 2,
 		Status: core.StatusPending,
 	}
-	mockRepo := &core.Repository {
+	mockRepo := &core.Repository{
 		ID: 2,
 	}
 
 	repos := mock.NewMockRepositoryStore(controller)
 	repos.EXPECT().Find(gomock.Any(), mockBuild.RepoID).Return(mockRepo, nil)
 
-
 	canceler := mock.NewMockCanceler(controller)
 	canceler.EXPECT().Cancel(gomock.Any(), mockRepo, mockBuild)
 
 	r := &Reaper{
-		Repos: repos,
+		Repos:    repos,
-		Stages: nil,
+		Stages:   nil,
 		Canceler: canceler,
 	}
 
@@ -63,17 +193,17 @@ func TestReapRunningMaybe(t *testing.T) {
 	}
 
 	mockBuild := &core.Build{
-		ID: 1,
+		ID:     1,
 		RepoID: 2,
 		Status: core.StatusRunning,
 	}
-	mockRepo := &core.Repository {
+	mockRepo := &core.Repository{
-		ID: 2,
+		ID:      2,
 		Timeout: 60,
 	}
 	mockStages := []*core.Stage{
 		{
-			Status: core.StatusRunning,
+			Status:  core.StatusRunning,
 			Started: mustParse("2006-01-02T13:00:00").Unix(), // running 2 hours, 1 hour longer than timeout
 		},
 	}
@@ -88,8 +218,8 @@ func TestReapRunningMaybe(t *testing.T) {
 	canceler.EXPECT().Cancel(gomock.Any(), mockRepo, mockBuild)
 
 	r := &Reaper{
-		Repos: repos,
+		Repos:    repos,
-		Stages: stages,
+		Stages:   stages,
 		Canceler: canceler,
 	}
 
@@ -112,21 +242,21 @@ func TestReapRunningMaybe_AllStagesPending(t *testing.T) {
 	}
 
 	mockBuild := &core.Build{
-		ID: 1,
+		ID:     1,
 		RepoID: 2,
 		Status: core.StatusRunning,
 	}
-	mockRepo := &core.Repository {
+	mockRepo := &core.Repository{
-		ID: 2,
+		ID:      2,
 		Timeout: 60,
 	}
 	mockStages := []*core.Stage{
 		{
-			Status: core.StatusPending,
+			Status:  core.StatusPending,
 			Started: 0,
 		},
 		{
-			Status: core.StatusPending,
+			Status:  core.StatusPending,
 			Started: 0,
 		},
 	}
@@ -141,8 +271,8 @@ func TestReapRunningMaybe_AllStagesPending(t *testing.T) {
 	canceler.EXPECT().Cancel(gomock.Any(), mockRepo, mockBuild)
 
 	r := &Reaper{
-		Repos: repos,
+		Repos:    repos,
-		Stages: stages,
+		Stages:   stages,
 		Canceler: canceler,
 	}
 
@@ -165,21 +295,21 @@ func TestReapRunningMaybe_AllStagesFinished(t *testing.T) {
 	}
 
 	mockBuild := &core.Build{
-		ID: 1,
+		ID:     1,
 		RepoID: 2,
 		Status: core.StatusRunning,
 	}
-	mockRepo := &core.Repository {
+	mockRepo := &core.Repository{
-		ID: 2,
+		ID:      2,
 		Timeout: 60,
 	}
 	mockStages := []*core.Stage{
 		{
-			Status: core.StatusPassing,
+			Status:  core.StatusPassing,
 			Started: mustParse("2006-01-02T14:40:00").Unix(),
 		},
 		{
-			Status: core.StatusPassing,
+			Status:  core.StatusPassing,
 			Started: mustParse("2006-01-02T14:50:00").Unix(),
 		},
 	}
@@ -194,8 +324,8 @@ func TestReapRunningMaybe_AllStagesFinished(t *testing.T) {
 	canceler.EXPECT().Cancel(gomock.Any(), mockRepo, mockBuild)
 
 	r := &Reaper{
-		Repos: repos,
+		Repos:    repos,
-		Stages: stages,
+		Stages:   stages,
 		Canceler: canceler,
 	}
 
@@ -217,21 +347,21 @@ func TestReapRunningMaybe_NotExpired(t *testing.T) {
 	}
 
 	mockBuild := &core.Build{
-		ID: 1,
+		ID:     1,
 		RepoID: 2,
 		Status: core.StatusRunning,
 	}
-	mockRepo := &core.Repository {
+	mockRepo := &core.Repository{
-		ID: 2,
+		ID:      2,
 		Timeout: 60,
 	}
 	mockStages := []*core.Stage{
 		{
-			Status: core.StatusPassing,
+			Status:  core.StatusPassing,
 			Started: mustParse("2006-01-02T14:50:00").Unix(),
 		},
 		{
-			Status: core.StatusRunning,
+			Status:  core.StatusRunning,
 			Started: mustParse("2006-01-02T14:55:00").Unix(),
 		},
 	}
@@ -243,8 +373,8 @@ func TestReapRunningMaybe_NotExpired(t *testing.T) {
 	stages.EXPECT().List(gomock.Any(), mockBuild.ID).Return(mockStages, nil)
 
 	r := &Reaper{
-		Repos: repos,
+		Repos:    repos,
-		Stages: stages,
+		Stages:   stages,
 		Canceler: nil,
 	}
 

service/canceler/reaper/util_test.go

@@ -5,8 +5,8 @@
 package reaper
 
 import (
-	"time"
 	"testing"
+	"time"
 )
 
 func TestIsExceeded(t *testing.T) {
@@ -16,45 +16,45 @@ func TestIsExceeded(t *testing.T) {
 	now = func() time.Time {
 		return mustParse("2006-01-02T15:00:00")
 	}
-	var tests = []struct{
+	var tests = []struct {
-		unix int64
+		unix     int64
-		timeout time.Duration
+		timeout  time.Duration
-		buffer  time.Duration
+		buffer   time.Duration
 		exceeded bool
 	}{
 		// timestamp equal to current time, not expired
 		{
-			unix: mustParse("2006-01-02T15:00:00").Unix(),
+			unix:     mustParse("2006-01-02T15:00:00").Unix(),
-			timeout: time.Minute*60,
+			timeout:  time.Minute * 60,
-			buffer: time.Minute*5,
+			buffer:   time.Minute * 5,
 			exceeded: false,
 		},
 		// timestamp is not gt current time - timeout, not expired
 		{
-			unix: mustParse("2006-01-02T14:00:00").Unix(),
+			unix:     mustParse("2006-01-02T14:00:00").Unix(),
-			timeout: time.Minute*60,
+			timeout:  time.Minute * 60,
-			buffer: 0,
+			buffer:   0,
 			exceeded: false,
 		},
 		// timestamp is gt current time - timeout, expired
 		{
-			unix: mustParse("2006-01-02T13:59:00").Unix(),
+			unix:     mustParse("2006-01-02T13:59:00").Unix(),
-			timeout: time.Minute*60,
+			timeout:  time.Minute * 60,
-			buffer: 0,
+			buffer:   0,
 			exceeded: true,
 		},
 		// timestamp is not gt current time - timeout - buffer, not expired
 		{
-			unix: mustParse("2006-01-02T13:59:00").Unix(),
+			unix:     mustParse("2006-01-02T13:59:00").Unix(),
-			timeout: time.Minute*60,
+			timeout:  time.Minute * 60,
-			buffer: time.Minute*5,
+			buffer:   time.Minute * 5,
 			exceeded: false,
 		},
 		// timestamp is gt current time - timeout - buffer, expired
 		{
-			unix: mustParse("2006-01-02T13:04:05").Unix(),
+			unix:     mustParse("2006-01-02T13:04:05").Unix(),
-			timeout: time.Minute*60,
+			timeout:  time.Minute * 60,
-			buffer: time.Minute*5,
+			buffer:   time.Minute * 5,
 			exceeded: true,
 		},
 	}