fix(admin/clean): clean executable when build is cleaned (#988)

* fix(admin/clean): clean executable when build is cleaned

* report number of rows affected

* integration test with rows affected

---------

Co-authored-by: David May <[email protected]>

api/admin/clean.go

@@ -108,10 +108,20 @@ func CleanResources(c *gin.Context) {
 
 	logrus.Infof("platform admin %s: cleaned %d builds in database", u.GetName(), builds)
 
+	// clean executables
+	executables, err := database.FromContext(c).CleanBuildExecutables(ctx)
+	if err != nil {
+		retErr := fmt.Errorf("%d builds cleaned. unable to clean build executables: %w", builds, err)
+
+		util.HandleError(c, http.StatusInternalServerError, retErr)
+
+		return
+	}
+
 	// clean services
 	services, err := database.FromContext(c).CleanServices(ctx, msg, before)
 	if err != nil {
-		retErr := fmt.Errorf("%d builds cleaned. unable to update services: %w", builds, err)
+		retErr := fmt.Errorf("%d builds cleaned. %d executables cleaned. unable to update services: %w", builds, executables, err)
 
 		util.HandleError(c, http.StatusInternalServerError, retErr)
 
@@ -123,7 +133,7 @@ func CleanResources(c *gin.Context) {
 	// clean steps
 	steps, err := database.FromContext(c).CleanSteps(msg, before)
 	if err != nil {
-		retErr := fmt.Errorf("%d builds cleaned. %d services cleaned. unable to update steps: %w", builds, services, err)
+		retErr := fmt.Errorf("%d builds cleaned. %d executables cleaned. %d services cleaned. unable to update steps: %w", builds, executables, services, err)
 
 		util.HandleError(c, http.StatusInternalServerError, retErr)
 
@@ -132,5 +142,5 @@ func CleanResources(c *gin.Context) {
 
 	logrus.Infof("platform admin %s: cleaned %d steps in database", u.GetName(), steps)
 
-	c.JSON(http.StatusOK, fmt.Sprintf("%d builds cleaned. %d services cleaned. %d steps cleaned.", builds, services, steps))
+	c.JSON(http.StatusOK, fmt.Sprintf("%d builds cleaned. %d executables cleaned. %d services cleaned. %d steps cleaned.", builds, executables, services, steps))
 }

database/executable/clean.go

@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package executable
+
+import (
+	"context"
+
+	"github.com/go-vela/types/constants"
+	"github.com/sirupsen/logrus"
+)
+
+const CleanExecutablesPostgres = `
+	DELETE FROM build_executables 
+	USING builds 
+	WHERE builds.id = build_executables.build_id 
+	AND builds.status = 'error';
+`
+
+const CleanExecutablesSqlite = `
+	DELETE FROM build_executables
+	WHERE build_id IN (
+  		SELECT build_id FROM build_executables e
+  		INNER JOIN builds b
+    		ON e.build_id=b.id
+  		WHERE b.status = 'error'
+	);
+`
+
+// CleanBuildExecutables pops executables which have a corresponding build that was cleaned.
+func (e *engine) CleanBuildExecutables(ctx context.Context) (int64, error) {
+	logrus.Trace("clearing build executables in the database")
+
+	switch e.config.Driver {
+	case constants.DriverPostgres:
+		res := e.client.Exec(CleanExecutablesPostgres)
+		return res.RowsAffected, res.Error
+	default:
+		res := e.client.Exec(CleanExecutablesSqlite)
+		return res.RowsAffected, res.Error
+	}
+}

database/executable/clean_test.go

@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package executable
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/DATA-DOG/go-sqlmock"
+	"github.com/go-vela/types/constants"
+	"github.com/go-vela/types/database"
+	"github.com/go-vela/types/library"
+)
+
+func TestExecutable_Engine_CleanExecutables(t *testing.T) {
+	// setup types
+	_buildOne := testBuild()
+	_buildOne.SetID(1)
+	_buildOne.SetRepoID(1)
+	_buildOne.SetNumber(1)
+	_buildOne.SetStatus("pending")
+	_buildOne.SetCreated(1)
+	_buildOne.SetDeployPayload(nil)
+
+	_buildTwo := testBuild()
+	_buildTwo.SetID(2)
+	_buildTwo.SetRepoID(1)
+	_buildTwo.SetNumber(2)
+	_buildTwo.SetStatus("error")
+	_buildTwo.SetCreated(1)
+	_buildTwo.SetDeployPayload(nil)
+
+	_bExecutableOne := testBuildExecutable()
+	_bExecutableOne.SetID(1)
+	_bExecutableOne.SetBuildID(1)
+	_bExecutableOne.SetData([]byte("foo"))
+
+	_bExecutableTwo := testBuildExecutable()
+	_bExecutableTwo.SetID(2)
+	_bExecutableTwo.SetBuildID(2)
+	_bExecutableTwo.SetData([]byte("bar"))
+
+	_postgres, _mock := testPostgres(t)
+	defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }()
+
+	_mock.ExpectExec("DELETE FROM build_executables USING builds WHERE builds.id = build_executables.build_id AND builds.status = 'error';").
+		WillReturnResult(sqlmock.NewResult(1, 1))
+
+	_mock.ExpectQuery(`DELETE FROM "build_executables" WHERE build_id = $1 RETURNING *`).WithArgs(2).WillReturnError(fmt.Errorf("not found"))
+
+	_sqlite := testSqlite(t)
+	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
+
+	err := _sqlite.CreateBuildExecutable(context.TODO(), _bExecutableOne)
+	if err != nil {
+		t.Errorf("unable to create test build for sqlite: %v", err)
+	}
+
+	err = _sqlite.CreateBuildExecutable(context.TODO(), _bExecutableTwo)
+	if err != nil {
+		t.Errorf("unable to create test build for sqlite: %v", err)
+	}
+
+	err = _sqlite.client.AutoMigrate(&database.Build{})
+	if err != nil {
+		t.Errorf("unable to create repo table for sqlite: %v", err)
+	}
+
+	err = _sqlite.client.Table(constants.TableBuild).Create(database.BuildFromLibrary(_buildOne)).Error
+	if err != nil {
+		t.Errorf("unable to create test repo for sqlite: %v", err)
+	}
+
+	err = _sqlite.client.Table(constants.TableBuild).Create(database.BuildFromLibrary(_buildTwo)).Error
+	if err != nil {
+		t.Errorf("unable to create test repo for sqlite: %v", err)
+	}
+
+	// setup tests
+	tests := []struct {
+		failure  bool
+		name     string
+		database *engine
+	}{
+		{
+			failure:  false,
+			name:     "postgres",
+			database: _postgres,
+		},
+		{
+			failure:  false,
+			name:     "sqlite3",
+			database: _sqlite,
+		},
+	}
+
+	// run tests
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got, err := test.database.CleanBuildExecutables(context.TODO())
+
+			if test.failure {
+				if err == nil {
+					t.Errorf("CleanExecutables for %s should have returned err", test.name)
+				}
+
+				return
+			}
+
+			if err != nil {
+				t.Errorf("CleanExecutables for %s returned err: %v", test.name, err)
+			}
+
+			if got != 1 {
+				t.Errorf("CleanExecutables for %s should have affected 1 row, affected %d", test.name, got)
+			}
+
+			_, err = test.database.PopBuildExecutable(context.TODO(), 2)
+			if err == nil {
+				t.Errorf("CleanExecutables for %s should have returned an error", test.name)
+			}
+		})
+	}
+}
+
+// testBuild is a test helper function to create a library
+// Build type with all fields set to their zero values.
+func testBuild() *library.Build {
+	return &library.Build{
+		ID:           new(int64),
+		RepoID:       new(int64),
+		PipelineID:   new(int64),
+		Number:       new(int),
+		Parent:       new(int),
+		Event:        new(string),
+		EventAction:  new(string),
+		Status:       new(string),
+		Error:        new(string),
+		Enqueued:     new(int64),
+		Created:      new(int64),
+		Started:      new(int64),
+		Finished:     new(int64),
+		Deploy:       new(string),
+		Clone:        new(string),
+		Source:       new(string),
+		Title:        new(string),
+		Message:      new(string),
+		Commit:       new(string),
+		Sender:       new(string),
+		Author:       new(string),
+		Email:        new(string),
+		Link:         new(string),
+		Branch:       new(string),
+		Ref:          new(string),
+		BaseRef:      new(string),
+		HeadRef:      new(string),
+		Host:         new(string),
+		Runtime:      new(string),
+		Distribution: new(string),
+	}
+}

database/executable/interface.go

@@ -20,6 +20,8 @@ type BuildExecutableInterface interface {
 	//
 	// https://en.wikipedia.org/wiki/Data_manipulation_language
 
+	// CleanBuildExecutables defines a function that deletes errored builds' corresponding executables.
+	CleanBuildExecutables(context.Context) (int64, error)
 	// CreateBuildExecutable defines a function that creates a build executable.
 	CreateBuildExecutable(context.Context, *library.BuildExecutable) error
 	// PopBuildExecutable defines a function that gets and deletes a build executable.

database/integration_test.go

@@ -423,6 +423,34 @@ func testExecutables(t *testing.T, db Interface, resources *Resources) {
 	}
 	methods["PopBuildExecutable"] = true
 
+	resources.Builds[0].SetStatus(constants.StatusError)
+
+	_, err := db.UpdateBuild(context.TODO(), resources.Builds[0])
+	if err != nil {
+		t.Errorf("unable to update build for clean executables test")
+	}
+
+	err = db.CreateBuildExecutable(context.TODO(), resources.Executables[0])
+	if err != nil {
+		t.Errorf("unable to create executable %d: %v", resources.Executables[0].GetID(), err)
+	}
+
+	count, err := db.CleanBuildExecutables(context.TODO())
+	if err != nil {
+		t.Errorf("unable to clean executable %d: %v", resources.Executables[0].GetID(), err)
+	}
+
+	if count != 1 {
+		t.Errorf("CleanBuildExecutables should have affected 1 row, affected %d", count)
+	}
+
+	_, err = db.PopBuildExecutable(context.TODO(), resources.Builds[0].GetID())
+	if err == nil {
+		t.Errorf("build executable not cleaned")
+	}
+
+	methods["CleanBuildExecutables"] = true
+
 	// ensure we called all the methods we expected to
 	for method, called := range methods {
 		if !called {