From 459a4c0d19a8c495ed9b36baa8259577e5c2bc03 Mon Sep 17 00:00:00 2001 From: Abin Simon Date: Thu, 17 Mar 2022 14:26:02 +0530 Subject: [PATCH] Update more resources to use transactions --- internal/cluster/dao/cluster.go | 2 +- internal/dao/bootstrap.go | 3 --- pkg/service/cluster.go | 31 ++++++++++++++++++++------ pkg/service/cluster_test.go | 3 +-- pkg/service/kubeconfig_revocation.go | 9 ++++---- pkg/service/kubeconfig_settings.go | 9 ++++---- pkg/service/kubectl_cluster_setting.go | 9 ++++---- 7 files changed, 38 insertions(+), 28 deletions(-) diff --git a/internal/cluster/dao/cluster.go b/internal/cluster/dao/cluster.go index 44a14ff..f23c2ac 100644 --- a/internal/cluster/dao/cluster.go +++ b/internal/cluster/dao/cluster.go @@ -13,7 +13,7 @@ import ( "github.com/uptrace/bun" ) -func CreateCluster(ctx context.Context, tx bun.Tx, cluster *models.Cluster) error { +func CreateCluster(ctx context.Context, tx bun.IDB, cluster *models.Cluster) error { clstrToken := &models.ClusterToken{ OrganizationId: cluster.OrganizationId, diff --git a/internal/dao/bootstrap.go b/internal/dao/bootstrap.go index 9e3f630..3bcf48d 100644 --- a/internal/dao/bootstrap.go +++ b/internal/dao/bootstrap.go @@ -128,10 +128,7 @@ func CreateBootstrapAgent(ctx context.Context, db bun.IDB, ba *models.BootstrapA return err } -// We are explicitly taking in Tx here as it was previously doing RunInTx -// TODO: should we take Tx here or just assume we will be passed in a tx? Or should we create one? func RegisterBootstrapAgent(ctx context.Context, db bun.Tx, token string) error { - ba, err := getBootstrapAgentForToken(ctx, db, token) if err != nil { return err diff --git a/pkg/service/cluster.go b/pkg/service/cluster.go index e9a5384..e15fcac 100644 --- a/pkg/service/cluster.go +++ b/pkg/service/cluster.go @@ -235,13 +235,17 @@ func (es *clusterService) Create(ctx context.Context, cluster *infrav3.Cluster) cluster.Spec.ClusterData.Health = infrav3.Health_EDGE_IGNORE - err = es.db.RunInTx(ctx, &sql.TxOptions{}, func(ctx context.Context, tx bun.Tx) error { - return dao.CreateCluster(ctx, tx, edb) - }) + tx, err := es.db.BeginTx(ctx, &sql.TxOptions{}) if err != nil { return &infrav3.Cluster{}, err } + err = dao.CreateCluster(ctx, tx, edb) + if err != nil { + tx.Rollback() + return &infrav3.Cluster{}, err + } + // if project is set create project cluster var pc *models.ProjectCluster pcList := make([]models.ProjectCluster, 0) @@ -250,8 +254,9 @@ func (es *clusterService) Create(ctx context.Context, cluster *infrav3.Cluster) ProjectID: edb.ProjectId, ClusterID: edb.ID, } - err = dao.CreateProjectCluster(ctx, es.db, pc) + err = dao.CreateProjectCluster(ctx, tx, pc) if err != nil { + tx.Rollback() return &infrav3.Cluster{}, err } pcList = append(pcList, *pc) @@ -273,9 +278,21 @@ func (es *clusterService) Create(ctx context.Context, cluster *infrav3.Cluster) YamlContent: operatorSpecEncoded, } - es.db.RunInTx(ctx, &sql.TxOptions{}, func(ctx context.Context, tx bun.Tx) error { - return dao.CreateOperatorBootstrap(ctx, tx, &bootstrapData) - }) + err = dao.CreateOperatorBootstrap(ctx, tx, &bootstrapData) + if err != nil { + tx.Rollback() + cluster.Status = &commonv3.Status{ + ConditionType: "Create", + ConditionStatus: commonv3.ConditionStatus_StatusFailed, + Reason: err.Error(), + } + return cluster, err + } + } + + err = tx.Commit() + if err != nil { + fmt.Println("unable to commit changes", err) } ev := event.Resource{ diff --git a/pkg/service/cluster_test.go b/pkg/service/cluster_test.go index 07ae858..b11ed08 100644 --- a/pkg/service/cluster_test.go +++ b/pkg/service/cluster_test.go @@ -47,10 +47,9 @@ func TestCreateCluster(t *testing.T) { WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(cuuid)) mock.ExpectExec(`UPDATE "cluster_clusters"`). WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectCommit() - mock.ExpectExec(`INSERT INTO "cluster_project_cluster"`). WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectCommit() cluster := &infrav3.Cluster{ Metadata: &v3.Metadata{Id: cuuid, Name: "cluster-" + cuuid, Organization: "orgname", Project: "project-" + puuid}, diff --git a/pkg/service/kubeconfig_revocation.go b/pkg/service/kubeconfig_revocation.go index 7f2c95d..7817bf7 100644 --- a/pkg/service/kubeconfig_revocation.go +++ b/pkg/service/kubeconfig_revocation.go @@ -52,16 +52,15 @@ func prepareKubeCfgRevocationResponse(kr *models.KubeconfigRevocation) *sentry.K } func (krs *kubeconfigRevocationService) Patch(ctx context.Context, kr *sentry.KubeconfigRevocation) error { - err := krs.db.RunInTx(ctx, &sql.TxOptions{}, func(ctx context.Context, tx bun.Tx) error { - _, err := dao.GetKubeconfigRevocation(ctx, krs.db, uuid.MustParse(kr.OrganizationID), uuid.MustParse(kr.AccountID), kr.IsSSOUser) + return krs.db.RunInTx(ctx, &sql.TxOptions{}, func(ctx context.Context, tx bun.Tx) error { + _, err := dao.GetKubeconfigRevocation(ctx, tx, uuid.MustParse(kr.OrganizationID), uuid.MustParse(kr.AccountID), kr.IsSSOUser) if err != nil && err == sql.ErrNoRows { kcr := convertToModel(kr) kcr.CreatedAt = time.Now() - return dao.CreateKubeconfigRevocation(ctx, krs.db, kcr) + return dao.CreateKubeconfigRevocation(ctx, tx, kcr) } - return dao.UpdateKubeconfigRevocation(ctx, krs.db, convertToModel(kr)) + return dao.UpdateKubeconfigRevocation(ctx, tx, convertToModel(kr)) }) - return err } func convertToModel(kr *sentry.KubeconfigRevocation) *models.KubeconfigRevocation { diff --git a/pkg/service/kubeconfig_settings.go b/pkg/service/kubeconfig_settings.go index 95a28ac..7319729 100644 --- a/pkg/service/kubeconfig_settings.go +++ b/pkg/service/kubeconfig_settings.go @@ -54,17 +54,16 @@ func (kss *kubeconfigSettingService) Patch(ctx context.Context, ks *sentry.Kubec if err != nil { accId = uuid.Nil } - err = kss.db.RunInTx(ctx, &sql.TxOptions{}, func(ctx context.Context, tx bun.Tx) error { - _, err := dao.GetKubeconfigSetting(ctx, kss.db, uuid.MustParse(ks.OrganizationID), accId, ks.IsSSOUser) + return kss.db.RunInTx(ctx, &sql.TxOptions{}, func(ctx context.Context, tx bun.Tx) error { + _, err := dao.GetKubeconfigSetting(ctx, tx, uuid.MustParse(ks.OrganizationID), accId, ks.IsSSOUser) db := convertToKubeCfgSettingModel(ks) if err != nil && err == sql.ErrNoRows { db.CreatedAt = time.Now() - return dao.CreateKubeconfigSetting(ctx, kss.db, convertToKubeCfgSettingModel(ks)) + return dao.CreateKubeconfigSetting(ctx, tx, convertToKubeCfgSettingModel(ks)) } db.ModifiedAt = time.Now() - return dao.UpdateKubeconfigSetting(ctx, kss.db, convertToKubeCfgSettingModel(ks)) + return dao.UpdateKubeconfigSetting(ctx, tx, convertToKubeCfgSettingModel(ks)) }) - return err } func prepareKubeCfgSettingResponse(ks *models.KubeconfigSetting) *sentry.KubeconfigSetting { diff --git a/pkg/service/kubectl_cluster_setting.go b/pkg/service/kubectl_cluster_setting.go index 86aaf75..50e4375 100644 --- a/pkg/service/kubectl_cluster_setting.go +++ b/pkg/service/kubectl_cluster_setting.go @@ -41,21 +41,20 @@ func (kcs *kubectlClusterSettingsService) Get(ctx context.Context, orgID string, } func (kcs *kubectlClusterSettingsService) Patch(ctx context.Context, kc *sentry.KubectlClusterSettings) error { - err := kcs.db.RunInTx(ctx, &sql.TxOptions{}, func(ctx context.Context, tx bun.Tx) error { - _, err := dao.GetkubectlClusterSettings(ctx, kcs.db, uuid.MustParse(kc.OrganizationID), kc.Name) + return kcs.db.RunInTx(ctx, &sql.TxOptions{}, func(ctx context.Context, tx bun.Tx) error { + _, err := dao.GetkubectlClusterSettings(ctx, tx, uuid.MustParse(kc.OrganizationID), kc.Name) if err != nil { if err == sql.ErrNoRows { kcsdb := convertToKubeCtlSettingModel(kc) kcsdb.CreatedAt = time.Now() - dao.CreatekubectlClusterSettings(ctx, kcs.db, kcsdb) + dao.CreatekubectlClusterSettings(ctx, tx, kcsdb) } return err } kcsdb := convertToKubeCtlSettingModel(kc) kcsdb.ModifiedAt = time.Now() - return dao.UpdatekubectlClusterSettings(ctx, kcs.db, kcsdb) + return dao.UpdatekubectlClusterSettings(ctx, tx, kcsdb) }) - return err } func convertToKubeCtlSettingModel(kcs *sentry.KubectlClusterSettings) *models.KubectlClusterSetting {