From ba42f517d4d943abe212fdd3e7e685b5f0f6b992 Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Tue, 15 Oct 2024 17:24:25 +0200 Subject: [PATCH 01/18] Added/Fixed some traces --- pkg/db/transactions.go | 12 +++++++----- .../manifest-repo-export-service/pkg/cmd/server.go | 8 ++++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/pkg/db/transactions.go b/pkg/db/transactions.go index 222fe9257..345e84d4e 100644 --- a/pkg/db/transactions.go +++ b/pkg/db/transactions.go @@ -104,7 +104,7 @@ type transactionOptions struct { // withTransactionAllOptions offers all options and returns multiple values. func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts transactionOptions, f DBFunctionMultipleEntriesT[T]) ([]T, error) { - span, ctx := tracer.StartSpanFromContext(ctx, "DBTransaction") + span, attempt_ctx := tracer.StartSpanFromContext(ctx, "DBTransaction") defer span.Finish() span.SetTag("readonly", opts.readonly) span.SetTag("maxRetries", opts.maxRetries) @@ -120,21 +120,23 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr } if IsRetryablePostgresError(e) { duration := 50 * time.Millisecond - logger.FromContext(ctx).Sugar().Warnf("%s transaction failed, will retry in %v: %v", msg, duration, e) + logger.FromContext(attempt_ctx).Sugar().Warnf("%s transaction failed, will retry in %v: %v", msg, duration, e) _ = transaction.Rollback() span.Finish() time.Sleep(duration) + // We must use original ctx here so that a retry attempt registers + // A DBTransaction span independent from previous DBTransaction return withTransactionAllOptions(h, ctx, transactionOptions{ maxRetries: opts.maxRetries - 1, readonly: opts.readonly, }, f) } else { - logger.FromContext(ctx).Sugar().Warnf("%s transaction failed, will NOT retry error: %v", msg, e) + logger.FromContext(attempt_ctx).Sugar().Warnf("%s transaction failed, will NOT retry error: %v", msg, e) } return nil, e } - tx, err := h.BeginTransaction(ctx, opts.readonly) + tx, err := h.BeginTransaction(attempt_ctx, opts.readonly) if err != nil { return retryMaybe("beginning", err, tx) } @@ -144,7 +146,7 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr // because it is always set when Commit() was successful }(tx) - result, err := f(ctx, tx) + result, err := f(attempt_ctx, tx) if err != nil { return retryMaybe("within", err, tx) } diff --git a/services/manifest-repo-export-service/pkg/cmd/server.go b/services/manifest-repo-export-service/pkg/cmd/server.go index 1bf952d6f..0f36db104 100755 --- a/services/manifest-repo-export-service/pkg/cmd/server.go +++ b/services/manifest-repo-export-service/pkg/cmd/server.go @@ -19,11 +19,12 @@ package cmd import ( "context" "database/sql" - "github.com/cenkalti/backoff/v4" - "github.com/freiheit-com/kuberpult/pkg/valid" "strconv" "time" + "github.com/cenkalti/backoff/v4" + "github.com/freiheit-com/kuberpult/pkg/valid" + "encoding/json" "fmt" "os" @@ -342,11 +343,14 @@ func processEsls(ctx context.Context, repo repository.Repository, dbHandler *db. measurePushes(ddMetrics, log, false) } + span, ctx := tracer.StartSpanFromContext(ctx, "DBWriteCommitTransactionTimestamp") + defer span.Finish() //Get latest commit. Write esl timestamp and commit hash. commit, err := repo.GetHeadCommit() if err != nil { return err } + span.SetTag("commitHash", commit.Id().String()) return dbHandler.DBWriteCommitTransactionTimestamp(ctx, transaction, commit.Id().String(), esl.Created) }) if err != nil { From bc2bc179288319c980df5a42f914c8d566b6a1a0 Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Wed, 16 Oct 2024 14:03:29 +0200 Subject: [PATCH 02/18] Add some logs for dev-env --- .../pkg/cmd/server.go | 2 +- .../pkg/repository/repository.go | 37 ++++++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/services/manifest-repo-export-service/pkg/cmd/server.go b/services/manifest-repo-export-service/pkg/cmd/server.go index 0f36db104..8c1b5fd3d 100755 --- a/services/manifest-repo-export-service/pkg/cmd/server.go +++ b/services/manifest-repo-export-service/pkg/cmd/server.go @@ -346,7 +346,7 @@ func processEsls(ctx context.Context, repo repository.Repository, dbHandler *db. span, ctx := tracer.StartSpanFromContext(ctx, "DBWriteCommitTransactionTimestamp") defer span.Finish() //Get latest commit. Write esl timestamp and commit hash. - commit, err := repo.GetHeadCommit() + commit, err := repo.GetHeadCommit(ctx) if err != nil { return err } diff --git a/services/manifest-repo-export-service/pkg/repository/repository.go b/services/manifest-repo-export-service/pkg/repository/repository.go index 50faac7e4..fb4735667 100644 --- a/services/manifest-repo-export-service/pkg/repository/repository.go +++ b/services/manifest-repo-export-service/pkg/repository/repository.go @@ -62,7 +62,7 @@ type Repository interface { StateAt(oid *git.Oid) (*State, error) FetchAndReset(ctx context.Context) error PushRepo(ctx context.Context) error - GetHeadCommit() (*git.Commit, error) + GetHeadCommit(ctx context.Context) (*git.Commit, error) } type TransformerBatchApplyError struct { @@ -418,8 +418,39 @@ func (r *repository) PushRepo(ctx context.Context) error { return nil } -func (r *repository) GetHeadCommit() (*git.Commit, error) { +func (r *repository) GetHeadCommit(ctx context.Context) (*git.Commit, error) { + r.repository.Workdir() + + msg := "" + ite, err := r.repository.NewBranchIterator(git.BranchAll) + if err != nil { + return nil, fmt.Errorf("Error looping through branches: %v", err) + } else { + msg += "Branches Info\n" + for { + branch, branchType, error := ite.Next() + if error != nil { + break + } + name, errorName := branch.Name() + if errorName == nil { + msg = msg + fmt.Sprintf("Branch: %v\n", name) + } else { + msg = msg + fmt.Sprintf("Failed to get a branch name.\n") + } + + msg += fmt.Sprintf("\tBranchType: %v\n\tBranch Points to: %v\n", branchType, branch.Reference.Target()) + } + } + ref, err := r.repository.Head() + name, err := ref.Branch().Name() + if err != nil { + msg += fmt.Sprintf("Failed to get branch name. %s", err.Error()) + } else { + msg += fmt.Sprintf("Current Branch: %s\n", name) + } + msg += fmt.Sprintf("Head target: %v\n", ref.Target()) if err != nil { return nil, fmt.Errorf("Error fetching HEAD: %v", err) } @@ -427,6 +458,8 @@ func (r *repository) GetHeadCommit() (*git.Commit, error) { if err != nil { return nil, fmt.Errorf("Error transalting into commit: %v", err) } + msg += fmt.Sprintf("Commit id: %v\n", commit.Id().String()) + logger.FromContext(ctx).Warn(msg) return commit, nil } From 5b934b806565ed836979d50da93e7cefaf41ed2c Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Wed, 16 Oct 2024 14:07:49 +0200 Subject: [PATCH 03/18] removed bad spans --- pkg/db/transactions.go | 10 +++++----- .../manifest-repo-export-service/pkg/cmd/server.go | 3 --- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pkg/db/transactions.go b/pkg/db/transactions.go index 345e84d4e..9f07c65fe 100644 --- a/pkg/db/transactions.go +++ b/pkg/db/transactions.go @@ -104,7 +104,7 @@ type transactionOptions struct { // withTransactionAllOptions offers all options and returns multiple values. func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts transactionOptions, f DBFunctionMultipleEntriesT[T]) ([]T, error) { - span, attempt_ctx := tracer.StartSpanFromContext(ctx, "DBTransaction") + span, ctx := tracer.StartSpanFromContext(ctx, "DBTransaction") defer span.Finish() span.SetTag("readonly", opts.readonly) span.SetTag("maxRetries", opts.maxRetries) @@ -120,7 +120,7 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr } if IsRetryablePostgresError(e) { duration := 50 * time.Millisecond - logger.FromContext(attempt_ctx).Sugar().Warnf("%s transaction failed, will retry in %v: %v", msg, duration, e) + logger.FromContext(ctx).Sugar().Warnf("%s transaction failed, will retry in %v: %v", msg, duration, e) _ = transaction.Rollback() span.Finish() time.Sleep(duration) @@ -131,12 +131,12 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr readonly: opts.readonly, }, f) } else { - logger.FromContext(attempt_ctx).Sugar().Warnf("%s transaction failed, will NOT retry error: %v", msg, e) + logger.FromContext(ctx).Sugar().Warnf("%s transaction failed, will NOT retry error: %v", msg, e) } return nil, e } - tx, err := h.BeginTransaction(attempt_ctx, opts.readonly) + tx, err := h.BeginTransaction(ctx, opts.readonly) if err != nil { return retryMaybe("beginning", err, tx) } @@ -146,7 +146,7 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr // because it is always set when Commit() was successful }(tx) - result, err := f(attempt_ctx, tx) + result, err := f(ctx, tx) if err != nil { return retryMaybe("within", err, tx) } diff --git a/services/manifest-repo-export-service/pkg/cmd/server.go b/services/manifest-repo-export-service/pkg/cmd/server.go index 8c1b5fd3d..36832bf45 100755 --- a/services/manifest-repo-export-service/pkg/cmd/server.go +++ b/services/manifest-repo-export-service/pkg/cmd/server.go @@ -343,14 +343,11 @@ func processEsls(ctx context.Context, repo repository.Repository, dbHandler *db. measurePushes(ddMetrics, log, false) } - span, ctx := tracer.StartSpanFromContext(ctx, "DBWriteCommitTransactionTimestamp") - defer span.Finish() //Get latest commit. Write esl timestamp and commit hash. commit, err := repo.GetHeadCommit(ctx) if err != nil { return err } - span.SetTag("commitHash", commit.Id().String()) return dbHandler.DBWriteCommitTransactionTimestamp(ctx, transaction, commit.Id().String(), esl.Created) }) if err != nil { From ce0abab52f71ca655bbaa2b8bb4b9a9cf576bc32 Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Wed, 16 Oct 2024 14:08:42 +0200 Subject: [PATCH 04/18] removed useless comment --- pkg/db/transactions.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/db/transactions.go b/pkg/db/transactions.go index 9f07c65fe..222fe9257 100644 --- a/pkg/db/transactions.go +++ b/pkg/db/transactions.go @@ -124,8 +124,6 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr _ = transaction.Rollback() span.Finish() time.Sleep(duration) - // We must use original ctx here so that a retry attempt registers - // A DBTransaction span independent from previous DBTransaction return withTransactionAllOptions(h, ctx, transactionOptions{ maxRetries: opts.maxRetries - 1, readonly: opts.readonly, From 92a1c4944f9ca3210a7794c64c15a2f4e2d941ea Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Wed, 16 Oct 2024 14:17:39 +0200 Subject: [PATCH 05/18] Removed useless call to WorkDir --- .../manifest-repo-export-service/pkg/repository/repository.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/manifest-repo-export-service/pkg/repository/repository.go b/services/manifest-repo-export-service/pkg/repository/repository.go index fb4735667..0cda11a06 100644 --- a/services/manifest-repo-export-service/pkg/repository/repository.go +++ b/services/manifest-repo-export-service/pkg/repository/repository.go @@ -419,8 +419,6 @@ func (r *repository) PushRepo(ctx context.Context) error { } func (r *repository) GetHeadCommit(ctx context.Context) (*git.Commit, error) { - r.repository.Workdir() - msg := "" ite, err := r.repository.NewBranchIterator(git.BranchAll) if err != nil { From 965d7c14f7e6abd9a6ab6e73a8752bc848528085 Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Wed, 16 Oct 2024 14:32:05 +0200 Subject: [PATCH 06/18] Solved linting issue --- .../manifest-repo-export-service/pkg/repository/repository.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/manifest-repo-export-service/pkg/repository/repository.go b/services/manifest-repo-export-service/pkg/repository/repository.go index 0cda11a06..ab761af24 100644 --- a/services/manifest-repo-export-service/pkg/repository/repository.go +++ b/services/manifest-repo-export-service/pkg/repository/repository.go @@ -434,7 +434,7 @@ func (r *repository) GetHeadCommit(ctx context.Context) (*git.Commit, error) { if errorName == nil { msg = msg + fmt.Sprintf("Branch: %v\n", name) } else { - msg = msg + fmt.Sprintf("Failed to get a branch name.\n") + msg = msg + "Failed to get a branch name.\n" } msg += fmt.Sprintf("\tBranchType: %v\n\tBranch Points to: %v\n", branchType, branch.Reference.Target()) From b6da59fdc089b9ddf0efd9aa1e0de0d50d23a3e6 Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Wed, 16 Oct 2024 16:54:07 +0200 Subject: [PATCH 07/18] New spans --- pkg/db/db.go | 2 ++ pkg/db/transactions.go | 27 ++++++++++---- .../pkg/cmd/server.go | 4 ++- .../pkg/repository/repository.go | 35 ++----------------- 4 files changed, 28 insertions(+), 40 deletions(-) diff --git a/pkg/db/db.go b/pkg/db/db.go index 4c89387c5..c831aeed3 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -5918,6 +5918,8 @@ func (h *DBHandler) DBWriteCommitTransactionTimestamp(ctx context.Context, tx *s ) span.SetTag("query", insertQuery) + span.SetTag("commitHash", commitHash) + span.SetTag("timestamp", timestamp) _, err := tx.Exec( insertQuery, commitHash, diff --git a/pkg/db/transactions.go b/pkg/db/transactions.go index 222fe9257..71f24cb65 100644 --- a/pkg/db/transactions.go +++ b/pkg/db/transactions.go @@ -70,6 +70,9 @@ func (h *DBHandler) WithTransactionR(ctx context.Context, maxRetries uint8, read // WithTransactionT is the same as WithTransaction, but you can also return data, not just the error. func WithTransactionT[T any](h *DBHandler, ctx context.Context, maxRetries uint8, readonly bool, f DBFunctionT[T]) (*T, error) { + span, ctx := tracer.StartSpanFromContext(ctx, "DBTransactionWithRetries") + defer span.Finish() + span.SetTag("retries", maxRetries) res, err := withTransactionAllOptions(h, ctx, transactionOptions{ maxRetries: maxRetries, readonly: readonly, @@ -84,6 +87,7 @@ func WithTransactionT[T any](h *DBHandler, ctx context.Context, maxRetries uint8 return []T{*fRes}, nil }) if err != nil || len(res) == 0 { + span.Finish(tracer.WithError(err)) return nil, err } return &res[0], err @@ -104,7 +108,7 @@ type transactionOptions struct { // withTransactionAllOptions offers all options and returns multiple values. func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts transactionOptions, f DBFunctionMultipleEntriesT[T]) ([]T, error) { - span, ctx := tracer.StartSpanFromContext(ctx, "DBTransaction") + span, attempt_ctx := tracer.StartSpanFromContext(ctx, "DBTransaction") defer span.Finish() span.SetTag("readonly", opts.readonly) span.SetTag("maxRetries", opts.maxRetries) @@ -113,6 +117,7 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr span.Finish(tracer.WithError(e)) return nil, e } + time.Sleep(100 * time.Millisecond) retryMaybe := func(msg string, e error, transaction *sql.Tx) ([]T, error) { if opts.maxRetries == 0 { @@ -120,21 +125,23 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr } if IsRetryablePostgresError(e) { duration := 50 * time.Millisecond - logger.FromContext(ctx).Sugar().Warnf("%s transaction failed, will retry in %v: %v", msg, duration, e) + logger.FromContext(attempt_ctx).Sugar().Warnf("%s transaction failed, will retry in %v: %v", msg, duration, e) _ = transaction.Rollback() span.Finish() time.Sleep(duration) + // We must use original ctx here so that a retry attempt registers + // A DBTransaction span independent from previous DBTransaction return withTransactionAllOptions(h, ctx, transactionOptions{ maxRetries: opts.maxRetries - 1, readonly: opts.readonly, }, f) } else { - logger.FromContext(ctx).Sugar().Warnf("%s transaction failed, will NOT retry error: %v", msg, e) + logger.FromContext(attempt_ctx).Sugar().Warnf("%s transaction failed, will NOT retry error: %v", msg, e) } return nil, e } - tx, err := h.BeginTransaction(ctx, opts.readonly) + tx, err := h.BeginTransaction(attempt_ctx, opts.readonly) if err != nil { return retryMaybe("beginning", err, tx) } @@ -144,8 +151,16 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr // because it is always set when Commit() was successful }(tx) - result, err := f(ctx, tx) + result, err := f(attempt_ctx, tx) + fmt.Println("Ouch ", opts.maxRetries) + if opts.maxRetries >= 0 { + err = &pq.Error{ + Code: "40", + Message: "Ouch", + } + } if err != nil { + span.Finish(tracer.WithError(err)) return retryMaybe("within", err, tx) } err = tx.Commit() @@ -166,7 +181,7 @@ func IsRetryablePostgresError(err error) bool { var pgErr = UnwrapUntilPostgresError(err) if pgErr == nil { // it's not even a postgres error, so we can't check if it's retryable - return false + return true } codeStr := string(pgErr.Code) // for a list of all postgres error codes, see https://www.postgresql.org/docs/9.3/errcodes-appendix.html diff --git a/services/manifest-repo-export-service/pkg/cmd/server.go b/services/manifest-repo-export-service/pkg/cmd/server.go index 36832bf45..0a0f3bff2 100755 --- a/services/manifest-repo-export-service/pkg/cmd/server.go +++ b/services/manifest-repo-export-service/pkg/cmd/server.go @@ -326,6 +326,7 @@ func processEsls(ctx context.Context, repo repository.Repository, dbHandler *db. time.Sleep(d) continue } + log.Infof("event processed successfully, now writing to cutoff and pushing...") err = dbHandler.WithTransactionR(ctx, 2, false, func(ctx context.Context, transaction *sql.Tx) error { err2 := db.DBWriteCutoff(dbHandler, ctx, transaction, esl.EslVersion) @@ -344,12 +345,13 @@ func processEsls(ctx context.Context, repo repository.Repository, dbHandler *db. } //Get latest commit. Write esl timestamp and commit hash. - commit, err := repo.GetHeadCommit(ctx) + commit, err := repo.GetHeadCommit() if err != nil { return err } return dbHandler.DBWriteCommitTransactionTimestamp(ctx, transaction, commit.Id().String(), esl.Created) }) + if err != nil { err3 := repo.FetchAndReset(ctx) if err3 != nil { diff --git a/services/manifest-repo-export-service/pkg/repository/repository.go b/services/manifest-repo-export-service/pkg/repository/repository.go index ab761af24..50faac7e4 100644 --- a/services/manifest-repo-export-service/pkg/repository/repository.go +++ b/services/manifest-repo-export-service/pkg/repository/repository.go @@ -62,7 +62,7 @@ type Repository interface { StateAt(oid *git.Oid) (*State, error) FetchAndReset(ctx context.Context) error PushRepo(ctx context.Context) error - GetHeadCommit(ctx context.Context) (*git.Commit, error) + GetHeadCommit() (*git.Commit, error) } type TransformerBatchApplyError struct { @@ -418,37 +418,8 @@ func (r *repository) PushRepo(ctx context.Context) error { return nil } -func (r *repository) GetHeadCommit(ctx context.Context) (*git.Commit, error) { - msg := "" - ite, err := r.repository.NewBranchIterator(git.BranchAll) - if err != nil { - return nil, fmt.Errorf("Error looping through branches: %v", err) - } else { - msg += "Branches Info\n" - for { - branch, branchType, error := ite.Next() - if error != nil { - break - } - name, errorName := branch.Name() - if errorName == nil { - msg = msg + fmt.Sprintf("Branch: %v\n", name) - } else { - msg = msg + "Failed to get a branch name.\n" - } - - msg += fmt.Sprintf("\tBranchType: %v\n\tBranch Points to: %v\n", branchType, branch.Reference.Target()) - } - } - +func (r *repository) GetHeadCommit() (*git.Commit, error) { ref, err := r.repository.Head() - name, err := ref.Branch().Name() - if err != nil { - msg += fmt.Sprintf("Failed to get branch name. %s", err.Error()) - } else { - msg += fmt.Sprintf("Current Branch: %s\n", name) - } - msg += fmt.Sprintf("Head target: %v\n", ref.Target()) if err != nil { return nil, fmt.Errorf("Error fetching HEAD: %v", err) } @@ -456,8 +427,6 @@ func (r *repository) GetHeadCommit(ctx context.Context) (*git.Commit, error) { if err != nil { return nil, fmt.Errorf("Error transalting into commit: %v", err) } - msg += fmt.Sprintf("Commit id: %v\n", commit.Id().String()) - logger.FromContext(ctx).Warn(msg) return commit, nil } From cc1656733be065fd5e5b3d4d4a411054d30ab2e1 Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Wed, 16 Oct 2024 17:02:41 +0200 Subject: [PATCH 08/18] removed trah --- pkg/db/transactions.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/pkg/db/transactions.go b/pkg/db/transactions.go index 71f24cb65..8ec2682c5 100644 --- a/pkg/db/transactions.go +++ b/pkg/db/transactions.go @@ -117,7 +117,6 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr span.Finish(tracer.WithError(e)) return nil, e } - time.Sleep(100 * time.Millisecond) retryMaybe := func(msg string, e error, transaction *sql.Tx) ([]T, error) { if opts.maxRetries == 0 { @@ -127,7 +126,7 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr duration := 50 * time.Millisecond logger.FromContext(attempt_ctx).Sugar().Warnf("%s transaction failed, will retry in %v: %v", msg, duration, e) _ = transaction.Rollback() - span.Finish() + span.Finish(tracer.WithError(e)) time.Sleep(duration) // We must use original ctx here so that a retry attempt registers // A DBTransaction span independent from previous DBTransaction @@ -152,15 +151,7 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr }(tx) result, err := f(attempt_ctx, tx) - fmt.Println("Ouch ", opts.maxRetries) - if opts.maxRetries >= 0 { - err = &pq.Error{ - Code: "40", - Message: "Ouch", - } - } if err != nil { - span.Finish(tracer.WithError(err)) return retryMaybe("within", err, tx) } err = tx.Commit() From e591b2346c15028e6a167a969fc376db0e3fe78a Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Wed, 16 Oct 2024 17:03:55 +0200 Subject: [PATCH 09/18] Useless empty lines removed --- services/manifest-repo-export-service/pkg/cmd/server.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/manifest-repo-export-service/pkg/cmd/server.go b/services/manifest-repo-export-service/pkg/cmd/server.go index 0a0f3bff2..9f2e29bad 100755 --- a/services/manifest-repo-export-service/pkg/cmd/server.go +++ b/services/manifest-repo-export-service/pkg/cmd/server.go @@ -326,7 +326,6 @@ func processEsls(ctx context.Context, repo repository.Repository, dbHandler *db. time.Sleep(d) continue } - log.Infof("event processed successfully, now writing to cutoff and pushing...") err = dbHandler.WithTransactionR(ctx, 2, false, func(ctx context.Context, transaction *sql.Tx) error { err2 := db.DBWriteCutoff(dbHandler, ctx, transaction, esl.EslVersion) @@ -351,7 +350,6 @@ func processEsls(ctx context.Context, repo repository.Repository, dbHandler *db. } return dbHandler.DBWriteCommitTransactionTimestamp(ctx, transaction, commit.Id().String(), esl.Created) }) - if err != nil { err3 := repo.FetchAndReset(ctx) if err3 != nil { From aff7d6ff1ff1c40f0099ee47225d0912bb77abfb Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Wed, 16 Oct 2024 17:18:11 +0200 Subject: [PATCH 10/18] Reseted mock flag --- pkg/db/transactions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/db/transactions.go b/pkg/db/transactions.go index 8ec2682c5..fda2c7f5b 100644 --- a/pkg/db/transactions.go +++ b/pkg/db/transactions.go @@ -172,7 +172,7 @@ func IsRetryablePostgresError(err error) bool { var pgErr = UnwrapUntilPostgresError(err) if pgErr == nil { // it's not even a postgres error, so we can't check if it's retryable - return true + return false } codeStr := string(pgErr.Code) // for a list of all postgres error codes, see https://www.postgresql.org/docs/9.3/errcodes-appendix.html From d3a4a718ef51d938e3c8b1d4566a1fc6fc6c73de Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Thu, 17 Oct 2024 16:01:22 +0200 Subject: [PATCH 11/18] fix(db): select query was ignoring recent eslversion if they were deleted --- pkg/db/db.go | 43 +++++++++++++++---- .../cd-service/pkg/repository/transformer.go | 2 +- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/pkg/db/db.go b/pkg/db/db.go index c831aeed3..841d8702c 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -789,26 +789,53 @@ 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) { +// 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, "DBSelectReleasesByApp") 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;`, + ) 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) { +func (h *DBHandler) DBSelectReleasesByAppOrderedByEslVersion(ctx context.Context, tx *sql.Tx, app string, deleted bool, ignorePrepublishes bool) ([]*DBReleaseWithMetaData, error) { span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByApp") defer span.Finish() selectQuery := h.AdaptQuery(fmt.Sprintf( diff --git a/services/cd-service/pkg/repository/transformer.go b/services/cd-service/pkg/repository/transformer.go index 7ca899204..7337b5e03 100644 --- a/services/cd-service/pkg/repository/transformer.go +++ b/services/cd-service/pkg/repository/transformer.go @@ -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 } From dd36ff3f5d3041634b90ac1f922e1608a747dc5e Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Thu, 17 Oct 2024 16:03:16 +0200 Subject: [PATCH 12/18] Fixed some stuf --- services/cd-service/pkg/repository/transformer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/cd-service/pkg/repository/transformer.go b/services/cd-service/pkg/repository/transformer.go index 7337b5e03..d2e913792 100644 --- a/services/cd-service/pkg/repository/transformer.go +++ b/services/cd-service/pkg/repository/transformer.go @@ -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 } From da8c7dd5963bd1fcad779668fc62278148514dc7 Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Thu, 17 Oct 2024 16:18:06 +0200 Subject: [PATCH 13/18] Added new test for deleted release case --- pkg/db/db_test.go | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/pkg/db/db_test.go b/pkg/db/db_test.go index 7221dffef..03fc771f8 100644 --- a/pkg/db/db_test.go +++ b/pkg/db/db_test.go @@ -2311,6 +2311,34 @@ 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, + App: "app1", + Manifests: DBReleaseManifests{Manifests: map[string]string{"dev": "manifest1"}}, + Environments: []string{"dev"}, + }, + }, + }, { Name: "Retrieve multiple releases", Releases: []DBReleaseWithMetaData{ @@ -2525,7 +2553,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) } From 10313c21114d9f08268b76572e64d61a750fba90 Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Thu, 17 Oct 2024 16:43:07 +0200 Subject: [PATCH 14/18] Fixed test cases and added new one --- pkg/db/db_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/db/db_test.go b/pkg/db/db_test.go index 03fc771f8..ffa776602 100644 --- a/pkg/db/db_test.go +++ b/pkg/db/db_test.go @@ -25,6 +25,7 @@ import ( "os" "os/exec" "path" + "slices" "strconv" "testing" "time" @@ -2333,6 +2334,7 @@ func TestReadReleasesByApp(t *testing.T) { { EslVersion: 2, ReleaseNumber: 10, + Deleted: true, App: "app1", Manifests: DBReleaseManifests{Manifests: map[string]string{"dev": "manifest1"}}, Environments: []string{"dev"}, @@ -2554,6 +2556,9 @@ func TestReadReleasesByApp(t *testing.T) { } } releases, err := dbHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, tc.AppName, !tc.RetrievePrepublishes) + slices.SortFunc(releases, func(r1 *DBReleaseWithMetaData, r2 *DBReleaseWithMetaData) int { + return int(r2.ReleaseNumber) - int(r1.ReleaseNumber) + }) if err != nil { return fmt.Errorf("error while selecting release, error: %w", err) } From 4a79c5ee381b001fc986dfbdd468ed501463d65e Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Thu, 17 Oct 2024 16:44:45 +0200 Subject: [PATCH 15/18] Fixed span names --- pkg/db/db.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/db/db.go b/pkg/db/db.go index bec1f78cc..b53c82252 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -792,7 +792,7 @@ func (h *DBHandler) processReleaseManifestRows(ctx context.Context, err error, r // 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, "DBSelectReleasesByApp") + span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByAppLatestEslVersion") defer span.Finish() selectQuery := h.AdaptQuery( `SELECT @@ -836,7 +836,7 @@ func (h *DBHandler) DBSelectReleasesByAppLatestEslVersion(ctx context.Context, t } func (h *DBHandler) DBSelectReleasesByAppOrderedByEslVersion(ctx context.Context, tx *sql.Tx, app string, deleted bool, ignorePrepublishes bool) ([]*DBReleaseWithMetaData, error) { - span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByApp") + span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByAppOrderedByEslVersion") defer span.Finish() selectQuery := h.AdaptQuery(fmt.Sprintf( "SELECT eslVersion, created, appName, metadata, releaseVersion, deleted, environments " + From dc97e65b5bdc3417dc59797edc93d0a1eb3ff606 Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Thu, 17 Oct 2024 16:55:28 +0200 Subject: [PATCH 16/18] Added ordering to the queries --- pkg/db/db.go | 3 ++- pkg/db/db_test.go | 4 ---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/pkg/db/db.go b/pkg/db/db.go index b53c82252..e2246b9a2 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -823,7 +823,8 @@ func (h *DBHandler) DBSelectReleasesByAppLatestEslVersion(ctx context.Context, t AND currentEslReleases.latesteslversion = releases.eslversion AND - currentEslReleases.releaseversion = releases.releaseversion;`, + currentEslReleases.releaseversion = releases.releaseversion + ORDER BY currentEslReleases.releaseversion DESC;`, ) span.SetTag("query", selectQuery) rows, err := tx.QueryContext( diff --git a/pkg/db/db_test.go b/pkg/db/db_test.go index ffa776602..35563e9e6 100644 --- a/pkg/db/db_test.go +++ b/pkg/db/db_test.go @@ -25,7 +25,6 @@ import ( "os" "os/exec" "path" - "slices" "strconv" "testing" "time" @@ -2556,9 +2555,6 @@ func TestReadReleasesByApp(t *testing.T) { } } releases, err := dbHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, tc.AppName, !tc.RetrievePrepublishes) - slices.SortFunc(releases, func(r1 *DBReleaseWithMetaData, r2 *DBReleaseWithMetaData) int { - return int(r2.ReleaseNumber) - int(r1.ReleaseNumber) - }) if err != nil { return fmt.Errorf("error while selecting release, error: %w", err) } From 6acc32533e7f619596840461cc42af32a897208f Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Thu, 17 Oct 2024 18:11:27 +0200 Subject: [PATCH 17/18] Fixed transformer db test --- services/cd-service/pkg/repository/transformer_db_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/services/cd-service/pkg/repository/transformer_db_test.go b/services/cd-service/pkg/repository/transformer_db_test.go index 16b87918d..973c987e8 100644 --- a/services/cd-service/pkg/repository/transformer_db_test.go +++ b/services/cd-service/pkg/repository/transformer_db_test.go @@ -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" @@ -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) } @@ -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 } From 6a5c5799e08b98ba2715bdb8bd34ff33200e3625 Mon Sep 17 00:00:00 2001 From: diogo-nogueira-freiheit Date: Mon, 21 Oct 2024 09:51:21 +0100 Subject: [PATCH 18/18] Fixed integration test --- tests/integration-tests/release_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration-tests/release_test.go b/tests/integration-tests/release_test.go index 81809fe43..513dcb3a6 100644 --- a/tests/integration-tests/release_test.go +++ b/tests/integration-tests/release_test.go @@ -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)