From 7be879b689836e17780b7853fa33634fc48eaa31 Mon Sep 17 00:00:00 2001 From: Felisia Martini Date: Tue, 10 Sep 2024 17:06:23 +0100 Subject: [PATCH] Cleaning up resources after tests --- cmd/serve.go | 5 ++- integrationtest/integrationtest_suite_test.go | 5 ++- integrationtest/termination_recovery_test.go | 5 ++- .../storage/recover_in_progress_operations.go | 9 +++-- .../recover_in_progress_operations_test.go | 34 ++++--------------- internal/storage/storage.go | 5 +++ internal/storage/storage_suite_test.go | 6 ++-- internal/storage/terraform_deployment.go | 11 +----- internal/storage/terraform_deployment_test.go | 34 ++++++++++--------- 9 files changed, 51 insertions(+), 63 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index 10db1f22e..41bd83179 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -109,7 +109,10 @@ func serve() { } var serviceBroker domain.ServiceBroker csbStore := storage.New(db, encryptor) - csbStore.RecoverInProgressOperations(logger) + err = csbStore.RecoverInProgressOperations(logger) + if err != nil { + logger.Fatal("Error recovering in-progress operations", err) + } serviceBroker, err = osbapiBroker.New(cfg, csbStore, logger) if err != nil { diff --git a/integrationtest/integrationtest_suite_test.go b/integrationtest/integrationtest_suite_test.go index cae7ec363..8731ffaa9 100644 --- a/integrationtest/integrationtest_suite_test.go +++ b/integrationtest/integrationtest_suite_test.go @@ -46,7 +46,10 @@ var _ = SynchronizedBeforeSuite( var _ = SynchronizedAfterSuite( func() {}, - func() { CleanupBuildArtifacts() }, + func() { + CleanupBuildArtifacts() + os.RemoveAll("/tmp/csb") + }, ) var _ = BeforeEach(func() { diff --git a/integrationtest/termination_recovery_test.go b/integrationtest/termination_recovery_test.go index 912da0d79..06e6d0ff5 100644 --- a/integrationtest/termination_recovery_test.go +++ b/integrationtest/termination_recovery_test.go @@ -27,9 +27,7 @@ var _ = Describe("Recovery From Broker Termination", Ordered, func() { stdout *Buffer stderr *Buffer ) - AfterAll(func() { - os.RemoveAll("/tmp/csb/") - }) + BeforeAll(func() { brokerpak = must(packer.BuildBrokerpak(csb, fixtures("termination-recovery"))) }) @@ -92,6 +90,7 @@ var _ = Describe("Recovery From Broker Termination", Ordered, func() { Expect(broker.Deprovision(si)).To(Succeed()) }) }) + Describe("running csb as a CF app", func() { BeforeEach(func() { broker = must(testdrive.StartBroker(csb, brokerpak, database, testdrive.WithOutputs(stdout, stderr), testdrive.WithEnv("CF_INSTANCE_GUID=dcfa061e-c0e3-4237-a805-734578347393"))) diff --git a/internal/storage/recover_in_progress_operations.go b/internal/storage/recover_in_progress_operations.go index 47e3f5aa7..ae8f48c9c 100644 --- a/internal/storage/recover_in_progress_operations.go +++ b/internal/storage/recover_in_progress_operations.go @@ -8,6 +8,8 @@ import ( "gorm.io/gorm" ) +const FailedMessage = "the broker restarted while the operation was in progress" + func (s *Storage) RecoverInProgressOperations(logger lager.Logger) error { logger = logger.Session("recover-in-progress-operations") @@ -20,7 +22,7 @@ func (s *Storage) RecoverInProgressOperations(logger lager.Logger) error { result := s.db.Where("last_operation_state = ?", "in progress").FindInBatches(&terraformDeploymentBatch, 100, func(tx *gorm.DB, batchNumber int) error { for i := range terraformDeploymentBatch { terraformDeploymentBatch[i].LastOperationState = "failed" - terraformDeploymentBatch[i].LastOperationMessage = "the broker restarted while the operation was in progress" + terraformDeploymentBatch[i].LastOperationMessage = FailedMessage logger.Info("mark-as-failed", lager.Data{"workspace_id": terraformDeploymentBatch[i].ID}) } @@ -29,7 +31,7 @@ func (s *Storage) RecoverInProgressOperations(logger lager.Logger) error { return result.Error } else { - deploymentIds, err := s.LockedDeploymentIds() + deploymentIds, err := s.GetLockedDeploymentIds() if err != nil { return err } @@ -40,10 +42,13 @@ func (s *Storage) RecoverInProgressOperations(logger lager.Logger) error { return err } receiver.LastOperationState = "failed" + receiver.LastOperationMessage = FailedMessage + err := s.db.Save(receiver).Error if err != nil { return err } + logger.Info("mark-as-failed", lager.Data{"workspace_id": id}) } return err } diff --git a/internal/storage/recover_in_progress_operations_test.go b/internal/storage/recover_in_progress_operations_test.go index c3036096f..9ad4fa919 100644 --- a/internal/storage/recover_in_progress_operations_test.go +++ b/internal/storage/recover_in_progress_operations_test.go @@ -1,18 +1,13 @@ package storage_test import ( - "errors" "os" - "strings" "code.cloudfoundry.org/lager/v3/lagertest" "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage" - "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage/storagefakes" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "gorm.io/driver/sqlite" - "gorm.io/gorm" ) const ( @@ -22,12 +17,7 @@ const ( var _ = Describe("RecoverInProgressOperations()", func() { BeforeEach(func() { - // Setup - db, err := gorm.Open(sqlite.Open(":memory:"), nil) - Expect(err).NotTo(HaveOccurred()) - Expect(db.Migrator().CreateTable(&models.TerraformDeployment{})).To(Succeed()) - Expect(db.Create(&models.TerraformDeployment{ ID: recoverID, LastOperationType: "fake-type", @@ -40,21 +30,9 @@ var _ = Describe("RecoverInProgressOperations()", func() { LastOperationState: "succeeded", LastOperationMessage: "fake-type succeeded", }).Error).To(Succeed()) - - encryptor := &storagefakes.FakeEncryptor{ - DecryptStub: func(bytes []byte) ([]byte, error) { - if string(bytes) == `cannot-be-decrypted` { - return nil, errors.New("fake decryption error") - } - return bytes, nil - }, - EncryptStub: func(bytes []byte) ([]byte, error) { - if strings.Contains(string(bytes), `cannot-be-encrypted`) { - return nil, errors.New("fake encryption error") - } - return []byte(`{"encrypted":` + string(bytes) + `}`), nil - }, - } + var rowCount int64 + db.Model(&models.TerraformDeployment{}).Count(&rowCount) + Expect(rowCount).To(BeNumerically("==", 2)) logger = lagertest.NewTestLogger("test") store = storage.New(db, encryptor) @@ -66,7 +44,7 @@ var _ = Describe("RecoverInProgressOperations()", func() { defer os.Unsetenv("CF_INSTANCE_GUID") // Call the function - store.RecoverInProgressOperations(logger) + Expect(store.RecoverInProgressOperations(logger)).To(Succeed()) // Behaviors By("marking the in-progress operation as failed") @@ -92,10 +70,10 @@ var _ = Describe("RecoverInProgressOperations()", func() { When("running on a VM", func() { It("recovers the expected operations", func() { // When running on a VM there will be a lockfile and record in the db - store.WriteLockFile(recoverID) + Expect(store.WriteLockFile(recoverID)).To(Succeed()) // Call the function - store.RecoverInProgressOperations(logger) + Expect(store.RecoverInProgressOperations(logger)).To(Succeed()) // Behaviors By("marking the in-progress operation as failed") diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 5efc7c53c..b647cd2df 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -29,4 +29,9 @@ func New(db *gorm.DB, encryptor Encryptor) *Storage { } } +func (s *Storage) WithLockDir(dir string) *Storage { + s.lockFileDir = dir + return s +} + type JSONObject map[string]any diff --git a/internal/storage/storage_suite_test.go b/internal/storage/storage_suite_test.go index 31728a8c0..fdbe834af 100644 --- a/internal/storage/storage_suite_test.go +++ b/internal/storage/storage_suite_test.go @@ -2,6 +2,7 @@ package storage_test import ( "errors" + "os" "strings" "testing" @@ -52,6 +53,7 @@ var _ = BeforeEach(func() { return []byte(`{"encrypted":` + string(bytes) + `}`), nil }, } - - store = storage.New(db, encryptor) + tempDir, err := os.MkdirTemp("/tmp", "csb-") + Expect(err).NotTo(HaveOccurred()) + store = storage.New(db, encryptor).WithLockDir(tempDir) }) diff --git a/internal/storage/terraform_deployment.go b/internal/storage/terraform_deployment.go index f9b2b1f95..009b0dbfa 100644 --- a/internal/storage/terraform_deployment.go +++ b/internal/storage/terraform_deployment.go @@ -172,16 +172,7 @@ func (s *Storage) RemoveLockFile(deploymentID string) error { return os.Remove(fmt.Sprintf("%s/%s", s.lockFileDir, sanitizeFileName(deploymentID))) } -func (s *Storage) GetLockFileNames() ([]string, error) { - entries, err := os.ReadDir(s.lockFileDir) - var names []string - for _, entry := range entries { - names = append(names, entry.Name()) - } - return names, err -} - -func (s *Storage) LockedDeploymentIds() ([]string, error) { +func (s *Storage) GetLockedDeploymentIds() ([]string, error) { entries, err := os.ReadDir(s.lockFileDir) var names []string for _, entry := range entries { diff --git a/internal/storage/terraform_deployment_test.go b/internal/storage/terraform_deployment_test.go index a6d390e67..a1ff457a0 100644 --- a/internal/storage/terraform_deployment_test.go +++ b/internal/storage/terraform_deployment_test.go @@ -7,13 +7,14 @@ import ( "os" "strings" + "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage/storagefakes" + "github.com/hashicorp/go-version" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage" - "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage/storagefakes" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/providers/tf/workspace" ) @@ -35,8 +36,9 @@ var _ = Describe("TerraformDeployments", func() { }, } - os.RemoveAll("/tmp/csb/") - store = storage.New(db, encryptor) + tempDir, err := os.MkdirTemp("/tmp", "csb-") + Expect(err).NotTo(HaveOccurred()) + store = storage.New(db, encryptor).WithLockDir(tempDir) }) Describe("StoreTerraformDeployments", func() { @@ -225,29 +227,29 @@ var _ = Describe("TerraformDeployments", func() { }) }) - Describe("GetLockFileNames", func() { - FIt("returns correct names", func() { - names, err := store.GetLockFileNames() + Describe("GetLockedDeploymentIds", func() { + It("returns correct names", func() { + names, err := store.GetLockedDeploymentIds() Expect(err).NotTo(HaveOccurred()) Expect(names).To(BeEmpty()) - Expect(store.WriteLockFile("1234")).To(Succeed()) - Expect(store.WriteLockFile("5678")).To(Succeed()) + Expect(store.WriteLockFile("tf:1234:")).To(Succeed()) + Expect(store.WriteLockFile("tf:5678:9123")).To(Succeed()) - names, err = store.GetLockFileNames() + names, err = store.GetLockedDeploymentIds() Expect(err).NotTo(HaveOccurred()) - Expect(names).To(ContainElements("1234", "5678")) + Expect(names).To(ContainElements("tf:1234:", "tf:5678:9123")) - Expect(store.RemoveLockFile("1234")).To(Succeed()) + Expect(store.RemoveLockFile("tf:1234:")).To(Succeed()) - names, err = store.GetLockFileNames() + names, err = store.GetLockedDeploymentIds() Expect(err).NotTo(HaveOccurred()) - Expect(names).To(ContainElements("5678")) - Expect(names).ToNot(ContainElements("1234")) + Expect(names).To(ContainElements("tf:5678:9123")) + Expect(names).ToNot(ContainElements("tf:1234:")) - Expect(store.RemoveLockFile("5678")).To(Succeed()) + Expect(store.RemoveLockFile("tf:5678:9123")).To(Succeed()) - names, err = store.GetLockFileNames() + names, err = store.GetLockedDeploymentIds() Expect(err).NotTo(HaveOccurred()) Expect(names).To(BeEmpty()) })