Skip to content

Commit

Permalink
fix(db): fixed select query that ignored eslversions with deleted flag (
Browse files Browse the repository at this point in the history
#2053)

In the DeleteEnvFromApp a query was made which selected the most recent
eslversions for each release of the app.
This query ignored rows that has the deleted flag set to true. When the
transformer later updated each release if the lated eslversion was
deleted then an error would occur.

This query was fixed and, in the process, altered to be more efficient.

Ref: SRX-9MM6SC
  • Loading branch information
diogo-nogueira-freiheit authored Oct 21, 2024
1 parent c9db634 commit 51d74ae
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 17 deletions.
48 changes: 38 additions & 10 deletions pkg/db/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -789,27 +789,55 @@ func (h *DBHandler) processReleaseManifestRows(ctx context.Context, err error, r
return result, nil
}

func (h *DBHandler) DBSelectReleasesByApp(ctx context.Context, tx *sql.Tx, app string, deleted bool, ignorePrepublishes bool) ([]*DBReleaseWithMetaData, error) {
span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByApp")
// DBSelectReleasesByAppLatestEslVersion returns the latest eslversion
// for each release of an app. It includes deleted releases and loads manifests.
func (h *DBHandler) DBSelectReleasesByAppLatestEslVersion(ctx context.Context, tx *sql.Tx, app string, ignorePrepublishes bool) ([]*DBReleaseWithMetaData, error) {
span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByAppLatestEslVersion")
defer span.Finish()
selectQuery := h.AdaptQuery(fmt.Sprintf(
"SELECT eslVersion, created, appName, metadata, manifests, releaseVersion, deleted, environments " +
" FROM releases " +
" WHERE appName=? AND deleted=?" +
" ORDER BY releaseVersion DESC, eslVersion DESC, created DESC;"))
selectQuery := h.AdaptQuery(
`SELECT
releases.eslVersion,
releases.created,
releases.appName,
releases.metadata,
releases.manifests,
releases.releaseVersion,
releases.deleted,
releases.environments
FROM (
SELECT
MAX(eslVersion) AS latestEslVersion,
appname,
releaseversion
FROM
releases
WHERE
appname=?
GROUP BY
appname, releaseversion
) as currentEslReleases
JOIN
releases
ON
currentEslReleases.appname = releases.appname
AND
currentEslReleases.latesteslversion = releases.eslversion
AND
currentEslReleases.releaseversion = releases.releaseversion
ORDER BY currentEslReleases.releaseversion DESC;`,
)
span.SetTag("query", selectQuery)
rows, err := tx.QueryContext(
ctx,
selectQuery,
app,
deleted,
)

return h.processReleaseRows(ctx, err, rows, ignorePrepublishes, true)
}

func (h *DBHandler) DBSelectReleasesByAppLatestEslVersion(ctx context.Context, tx *sql.Tx, app string, deleted bool, ignorePrepublishes bool) ([]*DBReleaseWithMetaData, error) {
span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByApp")
func (h *DBHandler) DBSelectReleasesByAppOrderedByEslVersion(ctx context.Context, tx *sql.Tx, app string, deleted bool, ignorePrepublishes bool) ([]*DBReleaseWithMetaData, error) {
span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByAppOrderedByEslVersion")
defer span.Finish()
selectQuery := h.AdaptQuery(fmt.Sprintf(
"SELECT eslVersion, created, appName, metadata, releaseVersion, deleted, environments " +
Expand Down
31 changes: 30 additions & 1 deletion pkg/db/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2311,6 +2311,35 @@ func TestReadReleasesByApp(t *testing.T) {
},
},
},
{
Name: "Retrieve deleted release",
Releases: []DBReleaseWithMetaData{
{
EslVersion: 1,
ReleaseNumber: 10,
App: "app1",
Manifests: DBReleaseManifests{Manifests: map[string]string{"dev": "manifest1"}},
},
{
EslVersion: 2,
ReleaseNumber: 10,
App: "app1",
Deleted: true,
Manifests: DBReleaseManifests{Manifests: map[string]string{"dev": "manifest1"}},
},
},
AppName: "app1",
Expected: []*DBReleaseWithMetaData{
{
EslVersion: 2,
ReleaseNumber: 10,
Deleted: true,
App: "app1",
Manifests: DBReleaseManifests{Manifests: map[string]string{"dev": "manifest1"}},
Environments: []string{"dev"},
},
},
},
{
Name: "Retrieve multiple releases",
Releases: []DBReleaseWithMetaData{
Expand Down Expand Up @@ -2525,7 +2554,7 @@ func TestReadReleasesByApp(t *testing.T) {
return fmt.Errorf("error while writing release, error: %w", err)
}
}
releases, err := dbHandler.DBSelectReleasesByApp(ctx, transaction, tc.AppName, false, !tc.RetrievePrepublishes)
releases, err := dbHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, tc.AppName, !tc.RetrievePrepublishes)
if err != nil {
return fmt.Errorf("error while selecting release, error: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions services/cd-service/pkg/repository/transformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ func (c *CreateApplicationVersion) Transform(
sortedKeys := sorting.SortKeys(c.Manifests)

if state.DBHandler.ShouldUseOtherTables() {
prevRelease, err := state.DBHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, c.Application, false, false)
prevRelease, err := state.DBHandler.DBSelectReleasesByAppOrderedByEslVersion(ctx, transaction, c.Application, false, false)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -1711,7 +1711,7 @@ func (u *DeleteEnvFromApp) Transform(
return "", err
}
if state.DBHandler.ShouldUseOtherTables() {
releases, err := state.DBHandler.DBSelectReleasesByApp(ctx, transaction, u.Application, false, true)
releases, err := state.DBHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, u.Application, true)
if err != nil {
return "", err
}
Expand Down
7 changes: 4 additions & 3 deletions services/cd-service/pkg/repository/transformer_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/lib/pq"
"regexp"
"testing"
gotime "time"

"github.com/lib/pq"

"github.com/freiheit-com/kuberpult/pkg/api/v1"
"github.com/freiheit-com/kuberpult/pkg/event"

Expand Down Expand Up @@ -2264,7 +2265,7 @@ func TestDeleteEnvFromAppWithDB(t *testing.T) {
if err != nil {
return fmt.Errorf("error: %v", err)
}
releases, err2 := state.DBHandler.DBSelectReleasesByApp(ctx, transaction, appName, false, true)
releases, err2 := state.DBHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, appName, true)
if err2 != nil {
return fmt.Errorf("error retrieving release: %v", err2)
}
Expand Down Expand Up @@ -3649,7 +3650,7 @@ func TestTimestampConsistency(t *testing.T) {
t.Fatalf("error mismatch on envAcceptance(-want, +got):\n%s", diff)
}
//Release
releases, err := state.DBHandler.DBSelectReleasesByApp(ctx, transaction, testAppName, false, true)
releases, err := state.DBHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, testAppName, true)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion tests/integration-tests/release_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ func callDBForLock(t *testing.T, dbHandler *db.DBHandler, ctx context.Context, e

func callDBForReleases(t *testing.T, dbHandler *db.DBHandler, ctx context.Context, appName string) []*db.DBReleaseWithMetaData {
release, err := db.WithTransactionMultipleEntriesT(dbHandler, ctx, true, func(ctx context.Context, transaction *sql.Tx) ([]*db.DBReleaseWithMetaData, error) {
return dbHandler.DBSelectReleasesByApp(ctx, transaction, appName, false, true)
return dbHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, appName, true)
})
if err != nil {
t.Fatalf("DBSelectReleasesByApp failed %s", err)
Expand Down

0 comments on commit 51d74ae

Please sign in to comment.