diff --git a/core/repo.go b/core/repo.go index d474c4a0b..74f554fbf 100644 --- a/core/repo.go +++ b/core/repo.go @@ -32,38 +32,39 @@ const ( type ( // Repository represents a source code repository. Repository struct { - ID int64 `json:"id"` - UID string `json:"uid"` - UserID int64 `json:"user_id"` - Namespace string `json:"namespace"` - Name string `json:"name"` - Slug string `json:"slug"` - SCM string `json:"scm"` - HTTPURL string `json:"git_http_url"` - SSHURL string `json:"git_ssh_url"` - Link string `json:"link"` - Branch string `json:"default_branch"` - Private bool `json:"private"` - Visibility string `json:"visibility"` - Active bool `json:"active"` - Config string `json:"config_path"` - Trusted bool `json:"trusted"` - Protected bool `json:"protected"` - IgnoreForks bool `json:"ignore_forks"` - IgnorePulls bool `json:"ignore_pull_requests"` - CancelPulls bool `json:"auto_cancel_pull_requests"` - CancelPush bool `json:"auto_cancel_pushes"` - Timeout int64 `json:"timeout"` - Throttle int64 `json:"throttle,omitempty"` - Counter int64 `json:"counter"` - Synced int64 `json:"synced"` - Created int64 `json:"created"` - Updated int64 `json:"updated"` - Version int64 `json:"version"` - Signer string `json:"-"` - Secret string `json:"-"` - Build *Build `json:"build,omitempty"` - Perms *Perm `json:"permissions,omitempty"` + ID int64 `json:"id"` + UID string `json:"uid"` + UserID int64 `json:"user_id"` + Namespace string `json:"namespace"` + Name string `json:"name"` + Slug string `json:"slug"` + SCM string `json:"scm"` + HTTPURL string `json:"git_http_url"` + SSHURL string `json:"git_ssh_url"` + Link string `json:"link"` + Branch string `json:"default_branch"` + Private bool `json:"private"` + Visibility string `json:"visibility"` + Active bool `json:"active"` + Config string `json:"config_path"` + Trusted bool `json:"trusted"` + Protected bool `json:"protected"` + IgnoreForks bool `json:"ignore_forks"` + 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"` + Synced int64 `json:"synced"` + Created int64 `json:"created"` + Updated int64 `json:"updated"` + Version int64 `json:"version"` + Signer string `json:"-"` + Secret string `json:"-"` + Build *Build `json:"build,omitempty"` + Perms *Perm `json:"permissions,omitempty"` } // RepositoryStore defines operations for working with repositories. diff --git a/handler/api/repos/update.go b/handler/api/repos/update.go index 489f4fcb7..8736c7c1b 100644 --- a/handler/api/repos/update.go +++ b/handler/api/repos/update.go @@ -28,17 +28,18 @@ import ( type ( repositoryInput struct { - Visibility *string `json:"visibility"` - Config *string `json:"config_path"` - Trusted *bool `json:"trusted"` - Protected *bool `json:"protected"` - IgnoreForks *bool `json:"ignore_forks"` - IgnorePulls *bool `json:"ignore_pull_requests"` - CancelPulls *bool `json:"auto_cancel_pull_requests"` - CancelPush *bool `json:"auto_cancel_pushes"` - Timeout *int64 `json:"timeout"` - Throttle *int64 `json:"throttle"` - Counter *int64 `json:"counter"` + Visibility *string `json:"visibility"` + Config *string `json:"config_path"` + Trusted *bool `json:"trusted"` + Protected *bool `json:"protected"` + IgnoreForks *bool `json:"ignore_forks"` + 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 diff --git a/handler/api/repos/update_test.go b/handler/api/repos/update_test.go index 5e12718fd..0d221e193 100644 --- a/handler/api/repos/update_test.go +++ b/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) + } +} diff --git a/service/canceler/canceler.go b/service/canceler/canceler.go index a75204703..bfef4d77e 100644 --- a/service/canceler/canceler.go +++ b/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 } diff --git a/service/canceler/canceler_test.go b/service/canceler/canceler_test.go index 9a4da5a20..b04c78ad4 100644 --- a/service/canceler/canceler_test.go +++ b/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() diff --git a/service/canceler/match.go b/service/canceler/match.go index 07c2f8056..d868fa620 100644 --- a/service/canceler/match.go +++ b/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.Build.Status != core.StatusPending { - return false + + 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 { diff --git a/service/canceler/match_test.go b/service/canceler/match_test.go index 7ba3e0c41..a35628dfc 100644 --- a/service/canceler/match_test.go +++ b/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, }, } diff --git a/store/batch/batch.go b/store/batch/batch.go index 849b92a12..3bfd04e94 100644 --- a/store/batch/batch.go +++ b/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 diff --git a/store/batch2/batch.go b/store/batch2/batch.go index 741de7705..e51a61d1b 100644 --- a/store/batch2/batch.go +++ b/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 diff --git a/store/repos/repos.go b/store/repos/repos.go index 1251f2712..a48a524a9 100644 --- a/store/repos/repos.go +++ b/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 diff --git a/store/repos/scan.go b/store/repos/scan.go index a6efd37e6..4dd2f1a3e 100644 --- a/store/repos/scan.go +++ b/store/repos/scan.go @@ -25,36 +25,37 @@ import ( // of named query parameters. func ToParams(v *core.Repository) map[string]interface{} { return map[string]interface{}{ - "repo_id": v.ID, - "repo_uid": v.UID, - "repo_user_id": v.UserID, - "repo_namespace": v.Namespace, - "repo_name": v.Name, - "repo_slug": v.Slug, - "repo_scm": v.SCM, - "repo_clone_url": v.HTTPURL, - "repo_ssh_url": v.SSHURL, - "repo_html_url": v.Link, - "repo_branch": v.Branch, - "repo_private": v.Private, - "repo_visibility": v.Visibility, - "repo_active": v.Active, - "repo_config": v.Config, - "repo_trusted": v.Trusted, - "repo_protected": v.Protected, - "repo_no_forks": v.IgnoreForks, - "repo_no_pulls": v.IgnorePulls, - "repo_cancel_pulls": v.CancelPulls, - "repo_cancel_push": v.CancelPush, - "repo_timeout": v.Timeout, - "repo_throttle": v.Throttle, - "repo_counter": v.Counter, - "repo_synced": v.Synced, - "repo_created": v.Created, - "repo_updated": v.Updated, - "repo_version": v.Version, - "repo_signer": v.Signer, - "repo_secret": v.Secret, + "repo_id": v.ID, + "repo_uid": v.UID, + "repo_user_id": v.UserID, + "repo_namespace": v.Namespace, + "repo_name": v.Name, + "repo_slug": v.Slug, + "repo_scm": v.SCM, + "repo_clone_url": v.HTTPURL, + "repo_ssh_url": v.SSHURL, + "repo_html_url": v.Link, + "repo_branch": v.Branch, + "repo_private": v.Private, + "repo_visibility": v.Visibility, + "repo_active": v.Active, + "repo_config": v.Config, + "repo_trusted": v.Trusted, + "repo_protected": v.Protected, + "repo_no_forks": v.IgnoreForks, + "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, + "repo_synced": v.Synced, + "repo_created": v.Created, + "repo_updated": v.Updated, + "repo_version": v.Version, + "repo_signer": v.Signer, + "repo_secret": v.Secret, } } @@ -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, diff --git a/store/shared/migrate/mysql/ddl_gen.go b/store/shared/migrate/mysql/ddl_gen.go index 37af63600..05772ddb3 100644 --- a/store/shared/migrate/mysql/ddl_gen.go +++ b/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 // diff --git a/store/shared/migrate/mysql/files/002_create_table_repos.sql b/store/shared/migrate/mysql/files/002_create_table_repos.sql index 73132b57e..feabda2ff 100644 --- a/store/shared/migrate/mysql/files/002_create_table_repos.sql +++ b/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; diff --git a/store/shared/migrate/postgres/ddl_gen.go b/store/shared/migrate/postgres/ddl_gen.go index 149dac1a3..cfcd17313 100644 --- a/store/shared/migrate/postgres/ddl_gen.go +++ b/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 // diff --git a/store/shared/migrate/postgres/files/002_create_table_repos.sql b/store/shared/migrate/postgres/files/002_create_table_repos.sql index f0a95e0bb..06e0020ba 100644 --- a/store/shared/migrate/postgres/files/002_create_table_repos.sql +++ b/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; \ No newline at end of file diff --git a/store/shared/migrate/sqlite/ddl_gen.go b/store/shared/migrate/sqlite/ddl_gen.go index e49312b60..55454a2fb 100644 --- a/store/shared/migrate/sqlite/ddl_gen.go +++ b/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 // diff --git a/store/shared/migrate/sqlite/files/002_create_table_repos.sql b/store/shared/migrate/sqlite/files/002_create_table_repos.sql index 70e0180a1..e17d2e400 100644 --- a/store/shared/migrate/sqlite/files/002_create_table_repos.sql +++ b/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; \ No newline at end of file