enhance: auto cancel builds pending approval when they become obsolete (

#1066)

api/build/auto_cancel.go

@@ -27,10 +27,14 @@ func AutoCancel(c *gin.Context, b *library.Build, rB *library.Build, r *library.
 		return false, nil
 	}
 
+	status := rB.GetStatus()
+
 	// ensure criteria is met
 	if isCancelable(rB, b) {
 		switch {
-		case strings.EqualFold(rB.GetStatus(), constants.StatusPending) && cancelOpts.Pending:
+		case strings.EqualFold(status, constants.StatusPendingApproval) ||
+			(strings.EqualFold(status, constants.StatusPending) &&
+				cancelOpts.Pending):
 			// pending build will be handled gracefully by worker once pulled off queue
 			rB.SetStatus(constants.StatusCanceled)
 
@@ -44,7 +48,7 @@ func AutoCancel(c *gin.Context, b *library.Build, rB *library.Build, r *library.
 			if err != nil {
 				return true, err
 			}
-		case strings.EqualFold(rB.GetStatus(), constants.StatusRunning) && cancelOpts.Running:
+		case strings.EqualFold(status, constants.StatusRunning) && cancelOpts.Running:
 			// call cancelRunning routine for builds already running on worker
 			err := cancelRunning(c, rB, r)
 			if err != nil {
@@ -55,7 +59,7 @@ func AutoCancel(c *gin.Context, b *library.Build, rB *library.Build, r *library.
 		}
 
 		// set error message that references current build
-		rB.SetError(fmt.Sprintf("build was auto canceled in favor of build %d", b.GetNumber()))
+		rB.SetError(fmt.Sprintf("%s build was auto canceled in favor of build %d", status, b.GetNumber()))
 
 		_, err := database.FromContext(c).UpdateBuild(c, rB)
 		if err != nil {
@@ -201,6 +205,11 @@ func isCancelable(target *library.Build, current *library.Build) bool {
 // ShouldAutoCancel is a helper function that determines whether or not a build should be eligible to
 // auto cancel currently running / pending builds.
 func ShouldAutoCancel(opts *pipeline.CancelOptions, b *library.Build, defaultBranch string) bool {
+	// if the build is pending approval, it should always be eligible to auto cancel
+	if strings.EqualFold(b.GetStatus(), constants.StatusPendingApproval) {
+		return true
+	}
+
 	// if anything is provided in the auto_cancel metadata, then we start with true
 	runAutoCancel := opts.Running || opts.Pending || opts.DefaultBranch
 

api/build/auto_cancel_test.go

@@ -146,6 +146,8 @@ func Test_ShouldAutoCancel(t *testing.T) {
 	branchDev := "dev"
 	branchPatch := "patch-1"
 
+	statusPendingApproval := constants.StatusPendingApproval
+
 	actionOpened := constants.ActionOpened
 	actionSync := constants.ActionSynchronize
 
@@ -256,6 +258,22 @@ func Test_ShouldAutoCancel(t *testing.T) {
 			branch: branchDev,
 			want:   false,
 		},
+		{
+			name: "Pending Approval Build",
+			opts: &pipeline.CancelOptions{
+				Running:       false,
+				Pending:       false,
+				DefaultBranch: false,
+			},
+			build: &library.Build{
+				Event:       &pullEvent,
+				Branch:      &branchDev,
+				EventAction: &actionOpened,
+				Status:      &statusPendingApproval,
+			},
+			branch: branchDev,
+			want:   true,
+		},
 	}
 
 	for _, tt := range tests {

api/webhook/post.go

@@ -736,6 +736,30 @@ func PostWebhook(c *gin.Context) {
 		return
 	}
 
+	// regardless of whether the build is published to queue, we want to attempt to auto-cancel if no errors
+	defer func() {
+		if err == nil && build.ShouldAutoCancel(p.Metadata.AutoCancel, b, repo.GetBranch()) {
+			// fetch pending and running builds
+			rBs, err := database.FromContext(c).ListPendingAndRunningBuildsForRepo(c, repo)
+			if err != nil {
+				logrus.Errorf("unable to fetch pending and running builds for %s: %v", repo.GetFullName(), err)
+			}
+
+			for _, rB := range rBs {
+				// call auto cancel routine
+				canceled, err := build.AutoCancel(c, b, rB, repo, p.Metadata.AutoCancel)
+				if err != nil {
+					// continue cancel loop if error, but log based on type of error
+					if canceled {
+						logrus.Errorf("unable to update canceled build error message: %v", err)
+					} else {
+						logrus.Errorf("unable to cancel running build: %v", err)
+					}
+				}
+			}
+		}
+	}()
+
 	// if the webhook was from a Pull event from a forked repository, verify it is allowed to run
 	if webhook.PullRequest.IsFromFork {
 		switch repo.GetApproveBuild() {
@@ -802,27 +826,6 @@ func PostWebhook(c *gin.Context) {
 		u,
 		route,
 	)
-
-	if build.ShouldAutoCancel(p.Metadata.AutoCancel, b, repo.GetBranch()) {
-		// fetch pending and running builds
-		rBs, err := database.FromContext(c).ListPendingAndRunningBuildsForRepo(c, repo)
-		if err != nil {
-			logrus.Errorf("unable to fetch pending and running builds for %s: %v", repo.GetFullName(), err)
-		}
-
-		for _, rB := range rBs {
-			// call auto cancel routine
-			canceled, err := build.AutoCancel(c, b, rB, repo, p.Metadata.AutoCancel)
-			if err != nil {
-				// continue cancel loop if error, but log based on type of error
-				if canceled {
-					logrus.Errorf("unable to update canceled build error message: %v", err)
-				} else {
-					logrus.Errorf("unable to cancel running build: %v", err)
-				}
-			}
-		}
-	}
 }
 
 // handleRepositoryEvent is a helper function that processes repository events from the SCM and updates

database/build/list_pending_running_repo.go

@@ -23,7 +23,7 @@ func (e *engine) ListPendingAndRunningBuildsForRepo(ctx context.Context, repo *l
 		Table(constants.TableBuild).
 		Select("*").
 		Where("repo_id = ?", repo.GetID()).
-		Where("status = 'running' OR status = 'pending'").
+		Where("status = 'running' OR status = 'pending' OR status = 'pending approval'").
 		Find(&b).
 		Error
 	if err != nil {

database/build/list_pending_running_repo_test.go

@@ -67,7 +67,7 @@ func TestBuild_Engine_ListPendingAndRunningBuildsForRepo(t *testing.T) {
 		AddRow(1, 1, nil, 1, 0, "", "", "running", "", 0, 1, 0, 0, "", nil, "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", 0, "", 0)
 
 	// ensure the mock expects the name query
-	_mock.ExpectQuery(`SELECT * FROM "builds" WHERE repo_id = $1 AND (status = 'running' OR status = 'pending')`).WithArgs(1).WillReturnRows(_rows)
+	_mock.ExpectQuery(`SELECT * FROM "builds" WHERE repo_id = $1 AND (status = 'running' OR status = 'pending' OR status = 'pending approval')`).WithArgs(1).WillReturnRows(_rows)
 
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()