Merge pull request #3126 from drone/feature/dron-34-cancel-builds

Ability to cancel running builds, if a new commit is pushed

core/repo.go

@@ -53,6 +53,7 @@ type (
 		IgnorePulls   bool   `json:"ignore_pull_requests"`
 		CancelPulls   bool   `json:"auto_cancel_pull_requests"`
 		CancelPush    bool   `json:"auto_cancel_pushes"`
+		CancelRunning bool   `json:"auto_cancel_running"`
 		Timeout       int64  `json:"timeout"`
 		Throttle      int64  `json:"throttle,omitempty"`
 		Counter       int64  `json:"counter"`

handler/api/repos/update.go

@@ -36,6 +36,7 @@ type (
 		IgnorePulls   *bool   `json:"ignore_pull_requests"`
 		CancelPulls   *bool   `json:"auto_cancel_pull_requests"`
 		CancelPush    *bool   `json:"auto_cancel_pushes"`
+		CancelRunning *bool   `json:"auto_cancel_running"`
 		Timeout       *int64  `json:"timeout"`
 		Throttle      *int64  `json:"throttle"`
 		Counter       *int64  `json:"counter"`
@@ -95,6 +96,9 @@ func HandleUpdate(repos core.RepositoryStore) http.HandlerFunc {
 		if in.CancelPush != nil {
 			repo.CancelPush = *in.CancelPush
 		}
+		if in.CancelRunning != nil {
+			repo.CancelRunning = *in.CancelRunning
+		}
 
 		//
 		// system administrator only

handler/api/repos/update_test.go

@@ -12,9 +12,9 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/drone/drone/core"
 	"github.com/drone/drone/handler/api/errors"
 	"github.com/drone/drone/mock"
-	"github.com/drone/drone/core"
 
 	"github.com/go-chi/chi"
 	"github.com/golang/mock/gomock"
@@ -220,3 +220,76 @@ func TestUpdate_UpdateFailed(t *testing.T) {
 		t.Errorf(diff)
 	}
 }
+
+func TestUpdateAutoCancelRunning(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	repo := &core.Repository{
+		ID:         1,
+		UserID:     1,
+		Namespace:  "octocat",
+		Name:       "hello-world",
+		Slug:       "octocat/hello-world",
+		Branch:     "master",
+		Private:    false,
+		Visibility: core.VisibilityPrivate,
+		HTTPURL:    "https://github.com/octocat/hello-world.git",
+		SSHURL:     "git@github.com:octocat/hello-world.git",
+		Link:       "https://github.com/octocat/hello-world",
+		CancelRunning: false,
+	}
+
+	repoInput := &core.Repository{
+		CancelRunning: true,
+		Visibility: core.VisibilityPrivate,
+	}
+
+	shouldBeValue := true
+	checkUpdate := func(_ context.Context, updated *core.Repository) error {
+		if got, want := updated.CancelRunning, shouldBeValue; got != want {
+			t.Errorf("Want repository visibility updated to %v, got %v", want, got)
+		}
+		return nil
+	}
+
+	repos := mock.NewMockRepositoryStore(controller)
+	repos.EXPECT().FindName(gomock.Any(), "octocat", "hello-world").Return(repo, nil)
+	repos.EXPECT().Update(gomock.Any(), repo).Return(nil).Do(checkUpdate)
+
+	c := new(chi.Context)
+	c.URLParams.Add("owner", "octocat")
+	c.URLParams.Add("name", "hello-world")
+
+	in := new(bytes.Buffer)
+	json.NewEncoder(in).Encode(repoInput)
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("POST", "/", in)
+	r = r.WithContext(
+		context.WithValue(r.Context(), chi.RouteCtxKey, c),
+	)
+
+	HandleUpdate(repos)(w, r)
+	if got, want := w.Code, 200; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+
+	got, want := new(core.Repository), &core.Repository{
+		ID:         1,
+		UserID:     1,
+		Namespace:  "octocat",
+		Name:       "hello-world",
+		Slug:       "octocat/hello-world",
+		Branch:     "master",
+		Private:    false,
+		Visibility: core.VisibilityPrivate,
+		HTTPURL:    "https://github.com/octocat/hello-world.git",
+		SSHURL:     "git@github.com:octocat/hello-world.git",
+		Link:       "https://github.com/octocat/hello-world",
+		CancelRunning: true,
+	}
+	json.NewDecoder(w.Body).Decode(got)
+	if diff := cmp.Diff(got, want); len(diff) > 0 {
+		t.Errorf(diff)
+	}
+}

service/canceler/canceler.go

@@ -105,8 +105,7 @@ func (s *service) CancelPending(ctx context.Context, repo *core.Repository, buil
 	var result error
 	for _, item := range incomplete {
 		// ignore incomplete items in the list that do
-		// not match the repository or build, are already
-		// running, or are newer than the current build.
+		// not match the repository or build
 		if !match(build, item) {
 			continue
 		}

service/canceler/canceler_test.go

@@ -31,6 +31,23 @@ func TestCancelPending_IgnoreEvent(t *testing.T) {
 	}
 }
 
+func TestCancelRunning_IgnoreEvent(t *testing.T) {
+	ignore := []string{
+		core.EventCron,
+		core.EventCustom,
+		core.EventPromote,
+		core.EventRollback,
+		core.EventTag,
+	}
+	for _, event := range ignore {
+		s := new(service)
+		err := s.CancelPending(noContext, nil, &core.Build{Event: event})
+		if err != nil {
+			t.Errorf("Expect cancel skipped for event type %s", event)
+		}
+	}
+}
+
 func TestCancel(t *testing.T) {
 	controller := gomock.NewController(t)
 	defer controller.Finish()

service/canceler/match.go

@@ -27,11 +27,17 @@ func match(build *core.Build, with *core.Repository) bool {
 	if with.Build.Number >= build.Number {
 		return false
 	}
-	// filter out builds that are not in a
-	// pending state.
+
+	if with.CancelRunning == true {
+		if with.Build.Status != core.StatusRunning && with.Build.Status != core.StatusPending {
+			return false
+		}
+	} else {
 		if with.Build.Status != core.StatusPending {
 			return false
 		}
+	}
+
 	// filter out builds that do not match
 	// the same event type.
 	if with.Build.Event != build.Event {

service/canceler/match_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/drone/drone/core"
 )
 
-func TestMatch(t *testing.T) {
+func TestMatchPendingBuild(t *testing.T) {
 	tests := []struct {
 		build *core.Build
 		repo  *core.Repository
@@ -72,7 +72,7 @@ func TestMatch(t *testing.T) {
 				Status: core.StatusPending,
 				Event:  core.EventPush,
 				Ref:    "refs/heads/master",
-			}},
+			}, CancelRunning: false},
 			want: true,
 		},
 		{
@@ -82,7 +82,91 @@ func TestMatch(t *testing.T) {
 				Status: core.StatusPending,
 				Event:  core.EventPullRequest,
 				Ref:    "refs/heads/master",
-			}},
+			}, CancelRunning: false},
+			want: true,
+		},
+	}
+
+	for i, test := range tests {
+		if got, want := match(test.build, test.repo), test.want; got != want {
+			t.Errorf("Want match %v at index %d, got %v", want, i, got)
+		}
+	}
+}
+
+func TestMatchRunningBuilds(t *testing.T) {
+	tests := []struct {
+		build *core.Build
+		repo  *core.Repository
+		want  bool
+	}{
+		// does not match repository id
+		{
+			build: &core.Build{RepoID: 2},
+			repo:  &core.Repository{ID: 1},
+			want:  false,
+		},
+		// does not match build number requirement that
+		// must be older than current build
+		{
+			build: &core.Build{RepoID: 1, Number: 2},
+			repo:  &core.Repository{ID: 1, Build: &core.Build{Number: 3}},
+			want:  false,
+		},
+		{
+			build: &core.Build{RepoID: 1, Number: 2},
+			repo:  &core.Repository{ID: 1, Build: &core.Build{Number: 2}},
+			want:  false,
+		},
+		// does not match required status
+		{
+			build: &core.Build{RepoID: 1, Number: 2},
+			repo:  &core.Repository{ID: 1, Build: &core.Build{Number: 1, Status: core.StatusError}},
+			want:  false,
+		},
+		// does not match (one of) required event types
+		{
+			build: &core.Build{RepoID: 1, Number: 2, Event: core.EventPullRequest},
+			repo: &core.Repository{ID: 1, Build: &core.Build{
+				Number: 1,
+				Status: core.StatusRunning,
+				Event:  core.EventPush,
+			}},
+			want: false,
+		},
+		// does not match ref
+		{
+			build: &core.Build{RepoID: 1, Number: 2, Event: core.EventPush, Ref: "refs/heads/master"},
+			repo: &core.Repository{ID: 1, Build: &core.Build{
+				Number: 1,
+				Status: core.StatusRunning,
+				Event:  core.EventPush,
+				Ref:    "refs/heads/develop",
+			}},
+			want: false,
+		},
+
+		//
+		// successful matches
+		//
+		{
+			build: &core.Build{RepoID: 1, Number: 2, Event: core.EventPullRequest, Ref: "refs/heads/master"},
+			repo: &core.Repository{ID: 1, Build: &core.Build{
+				Number: 1,
+				Status: core.StatusRunning,
+				Event:  core.EventPullRequest,
+				Ref:    "refs/heads/master",
+			}, CancelRunning: true},
+			want: true,
+		},
+		{
+			build: &core.Build{RepoID: 1, Number: 2, Event: core.EventPush, Ref: "refs/heads/master"},
+			repo: &core.Repository{ID: 1, Build: &core.Build{
+				Number: 1,
+				Status: core.StatusRunning,
+				Event:  core.EventPush,
+				Ref:    "refs/heads/master",
+			}, CancelRunning: true},
 			want: true,
 		},
 	}

store/batch/batch.go

@@ -186,6 +186,7 @@ const stmtInsertBase = `
 ,repo_no_pulls
 ,repo_cancel_pulls
 ,repo_cancel_push
+,repo_cancel_running
 ,repo_synced
 ,repo_created
 ,repo_updated
@@ -216,6 +217,7 @@ const stmtInsertBase = `
 ,:repo_no_pulls
 ,:repo_cancel_pulls
 ,:repo_cancel_push
+,:repo_cancel_running
 ,:repo_synced
 ,:repo_created
 ,:repo_updated

store/batch2/batch.go

@@ -244,6 +244,7 @@ const stmtInsertBase = `
 ,repo_no_pulls
 ,repo_cancel_pulls
 ,repo_cancel_push
+,repo_cancel_running
 ,repo_synced
 ,repo_created
 ,repo_updated
@@ -274,6 +275,7 @@ const stmtInsertBase = `
 ,:repo_no_pulls
 ,:repo_cancel_pulls
 ,:repo_cancel_push
+,:repo_cancel_running
 ,:repo_synced
 ,:repo_created
 ,:repo_updated

store/repos/repos.go

@@ -290,6 +290,7 @@ SELECT
 ,repo_no_pulls
 ,repo_cancel_pulls
 ,repo_cancel_push
+,repo_cancel_running
 ,repo_synced
 ,repo_created
 ,repo_updated
@@ -386,6 +387,7 @@ INSERT INTO repos (
 ,repo_no_pulls
 ,repo_cancel_pulls
 ,repo_cancel_push
+,repo_cancel_running
 ,repo_synced
 ,repo_created
 ,repo_updated
@@ -416,6 +418,7 @@ INSERT INTO repos (
 ,:repo_no_pulls
 ,:repo_cancel_pulls
 ,:repo_cancel_push
+,:repo_cancel_running
 ,:repo_synced
 ,:repo_created
 ,:repo_updated
@@ -463,6 +466,7 @@ UPDATE repos SET
 ,repo_no_pulls = :repo_no_pulls
 ,repo_cancel_pulls = :repo_cancel_pulls
 ,repo_cancel_push = :repo_cancel_push
+,repo_cancel_running = :repo_cancel_running
 ,repo_timeout = :repo_timeout
 ,repo_throttle = :repo_throttle
 ,repo_counter = :repo_counter

store/repos/scan.go

@@ -46,6 +46,7 @@ func ToParams(v *core.Repository) map[string]interface{} {
 		"repo_no_pulls":       v.IgnorePulls,
 		"repo_cancel_pulls":   v.CancelPulls,
 		"repo_cancel_push":    v.CancelPush,
+		"repo_cancel_running": v.CancelRunning,
 		"repo_timeout":        v.Timeout,
 		"repo_throttle":       v.Throttle,
 		"repo_counter":        v.Counter,
@@ -86,6 +87,7 @@ func scanRow(scanner db.Scanner, dest *core.Repository) error {
 		&dest.IgnorePulls,
 		&dest.CancelPulls,
 		&dest.CancelPush,
+		&dest.CancelRunning,
 		&dest.Synced,
 		&dest.Created,
 		&dest.Updated,
@@ -141,6 +143,7 @@ func scanRowBuild(scanner db.Scanner, dest *core.Repository) error {
 		&dest.IgnorePulls,
 		&dest.CancelPulls,
 		&dest.CancelPush,
+		&dest.CancelRunning,
 		&dest.Synced,
 		&dest.Created,
 		&dest.Updated,

store/shared/migrate/mysql/ddl_gen.go

@@ -36,6 +36,10 @@ var migrations = []struct {
 		name: "alter-table-repos-add-column-throttle",
 		stmt: alterTableReposAddColumnThrottle,
 	},
+	{
+		name: "alter-table-repos-add-column-cancel-running",
+		stmt: alterTableReposAddColumnCancelRunning,
+	},
 	{
 		name: "create-table-perms",
 		stmt: createTablePerms,
@@ -334,6 +338,10 @@ var alterTableReposAddColumnThrottle = `
 ALTER TABLE repos ADD COLUMN repo_throttle INTEGER NOT NULL DEFAULT 0;
 `
 
+var alterTableReposAddColumnCancelRunning = `
+ALTER TABLE repos ADD COLUMN repo_cancel_running BOOLEAN NOT NULL DEFAULT false;
+`
+
 //
 // 003_create_table_perms.sql
 //

store/shared/migrate/mysql/files/002_create_table_repos.sql

@@ -49,3 +49,7 @@ ALTER TABLE repos ADD COLUMN repo_cancel_push BOOLEAN NOT NULL DEFAULT false;
 -- name: alter-table-repos-add-column-throttle
 
 ALTER TABLE repos ADD COLUMN repo_throttle INTEGER NOT NULL DEFAULT 0;
+
+-- name: alter-table-repos-add-column-cancel-running
+
+ALTER TABLE repos ADD COLUMN repo_cancel_running BOOLEAN NOT NULL DEFAULT false;

store/shared/migrate/postgres/ddl_gen.go

@@ -36,6 +36,10 @@ var migrations = []struct {
 		name: "alter-table-repos-add-column-throttle",
 		stmt: alterTableReposAddColumnThrottle,
 	},
+	{
+		name: "alter-table-repos-add-column-cancel-running",
+		stmt: alterTableReposAddColumnCancelRunning,
+	},
 	{
 		name: "create-table-perms",
 		stmt: createTablePerms,
@@ -326,6 +330,10 @@ var alterTableReposAddColumnThrottle = `
 ALTER TABLE repos ADD COLUMN repo_throttle INTEGER NOT NULL DEFAULT 0;
 `
 
+var alterTableReposAddColumnCancelRunning = `
+ALTER TABLE repos ADD COLUMN repo_cancel_running BOOLEAN NOT NULL DEFAULT false;
+`
+
 //
 // 003_create_table_perms.sql
 //

store/shared/migrate/postgres/files/002_create_table_repos.sql

@@ -49,3 +49,7 @@ ALTER TABLE repos ADD COLUMN repo_cancel_push BOOLEAN NOT NULL DEFAULT false;
 -- name: alter-table-repos-add-column-throttle
 
 ALTER TABLE repos ADD COLUMN repo_throttle INTEGER NOT NULL DEFAULT 0;
+
+-- name: alter-table-repos-add-column-cancel-running
+
+ALTER TABLE repos ADD COLUMN repo_cancel_running BOOLEAN NOT NULL DEFAULT false;

store/shared/migrate/sqlite/ddl_gen.go

@@ -36,6 +36,10 @@ var migrations = []struct {
 		name: "alter-table-repos-add-column-throttle",
 		stmt: alterTableReposAddColumnThrottle,
 	},
+	{
+		name: "alter-table-repos-add-column-cancel-running",
+		stmt: alterTableReposAddColumnCancelRunning,
+	},
 	{
 		name: "create-table-perms",
 		stmt: createTablePerms,
@@ -326,6 +330,10 @@ var alterTableReposAddColumnThrottle = `
 ALTER TABLE repos ADD COLUMN repo_throttle INTEGER NOT NULL DEFAULT 0;
 `
 
+var alterTableReposAddColumnCancelRunning = `
+ALTER TABLE repos ADD COLUMN repo_cancel_running BOOLEAN NOT NULL DEFAULT 0;
+`
+
 //
 // 003_create_table_perms.sql
 //

store/shared/migrate/sqlite/files/002_create_table_repos.sql

@@ -49,3 +49,7 @@ ALTER TABLE repos ADD COLUMN repo_cancel_push BOOLEAN NOT NULL DEFAULT 0;
 -- name: alter-table-repos-add-column-throttle
 
 ALTER TABLE repos ADD COLUMN repo_throttle INTEGER NOT NULL DEFAULT 0;
+
+-- name: alter-table-repos-add-column-cancel-running
+
+ALTER TABLE repos ADD COLUMN repo_cancel_running BOOLEAN NOT NULL DEFAULT 0;