Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions api/admin/clean.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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))
}
41 changes: 41 additions & 0 deletions database/executable/clean.go
Original file line number Diff line number Diff line change
@@ -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) {

Check failure on line 30 in database/executable/clean.go

View workflow job for this annotation

GitHub Actions / golangci

[golangci] database/executable/clean.go#L30

unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
Raw output
database/executable/clean.go:30:40: unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
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
}
}
162 changes: 162 additions & 0 deletions database/executable/clean_test.go
Original file line number Diff line number Diff line change
@@ -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),
}
}
2 changes: 2 additions & 0 deletions database/executable/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
28 changes: 28 additions & 0 deletions database/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading