From d58816adf4787692925b97a74bce58d9ee438560 Mon Sep 17 00:00:00 2001 From: ifindlay-cci Date: Fri, 3 Jan 2025 11:20:59 +0000 Subject: [PATCH] chore: Fixes broker 'failed to allocate' test errors We can only have 5 instances of our test broker running. Tests that use the test broker can see 'failed to allocate' errors. This is because in the case of a failed creation of a service in our tests will fail to clean up the test broker, causing other test runs to fail. --- acceptance-tests/helpers/services/delete.go | 8 ++++++-- .../upgrade/update_and_upgrade_mysql_test.go | 18 +++++++++++++++--- .../update_and_upgrade_postgresql_test.go | 16 ++++++++++++---- .../upgrade/update_and_upgrade_storage_test.go | 16 ++++++++++++---- acceptance-tests/withoutcredhub_test.go | 15 +++++++++++---- 5 files changed, 56 insertions(+), 17 deletions(-) diff --git a/acceptance-tests/helpers/services/delete.go b/acceptance-tests/helpers/services/delete.go index 857b523c..5629d209 100644 --- a/acceptance-tests/helpers/services/delete.go +++ b/acceptance-tests/helpers/services/delete.go @@ -10,11 +10,15 @@ import ( ) func (s *ServiceInstance) Delete() { + Delete(s.Name) +} + +func Delete(name string) { switch cf.Version() { case cf.VersionV8: - deleteWithWait(s.Name) + deleteWithWait(name) default: - deleteWithPoll(s.Name) + deleteWithPoll(name) } } diff --git a/acceptance-tests/upgrade/update_and_upgrade_mysql_test.go b/acceptance-tests/upgrade/update_and_upgrade_mysql_test.go index e3894172..94cbdfb7 100644 --- a/acceptance-tests/upgrade/update_and_upgrade_mysql_test.go +++ b/acceptance-tests/upgrade/update_and_upgrade_mysql_test.go @@ -24,8 +24,20 @@ var _ = Describe("UpgradeMYSQLTest", Label("mysql"), func() { defer serviceBroker.Delete() By("creating a service instance") - serviceInstance := services.CreateInstance("csb-google-mysql", "default", services.WithBroker(serviceBroker)) - defer serviceInstance.Delete() + serviceOffering := "csb-google-mysql" + servicePlan := "default" + serviceName := random.Name(random.WithPrefix(serviceOffering, servicePlan)) + // CreateInstance can fail and can leave a service record (albeit a failed one) lying around. + // We can't delete service brokers that have serviceInstances, so we need to ensure the service instance + // is cleaned up regardless as to whether it wa successful. This is important when we use our own service broker + // (which can only have 5 instances at any time) to prevent subsequent test failures. + defer services.Delete(serviceName) + serviceInstance := services.CreateInstance( + serviceOffering, + servicePlan, + services.WithBroker(serviceBroker), + services.WithName(serviceName), + ) By("pushing the unstarted app twice") appOne := apps.Push(apps.WithApp(apps.MySQL)) @@ -54,7 +66,7 @@ var _ = Describe("UpgradeMYSQLTest", Label("mysql"), func() { serviceBroker.UpdateBroker(developmentBuildDir) By("validating that the instance plan is still active") - Expect(plans.ExistsAndAvailable("default", "csb-google-mysql", serviceBroker.Name)) + Expect(plans.ExistsAndAvailable(servicePlan, serviceOffering, serviceBroker.Name)) By("upgrading service instance") serviceInstance.Upgrade() diff --git a/acceptance-tests/upgrade/update_and_upgrade_postgresql_test.go b/acceptance-tests/upgrade/update_and_upgrade_postgresql_test.go index b037b439..6994be43 100644 --- a/acceptance-tests/upgrade/update_and_upgrade_postgresql_test.go +++ b/acceptance-tests/upgrade/update_and_upgrade_postgresql_test.go @@ -23,12 +23,20 @@ var _ = Describe("UpgradePostgreSQLTest", Label("postgresql"), func() { defer serviceBroker.Delete() By("creating a service") + serviceOffering := "csb-google-postgres" + servicePlan := "small" + serviceName := random.Name(random.WithPrefix(serviceOffering, servicePlan)) + // CreateInstance can fail and can leave a service record (albeit a failed one) lying around. + // We can't delete service brokers that have serviceInstances, so we need to ensure the service instance + // is cleaned up regardless as to whether it wa successful. This is important when we use our own service broker + // (which can only have 5 instances at any time) to prevent subsequent test failures. + defer services.Delete(serviceName) serviceInstance := services.CreateInstance( - "csb-google-postgres", - "small", + serviceOffering, + servicePlan, services.WithBroker(serviceBroker), + services.WithName(serviceName), ) - defer serviceInstance.Delete() By("pushing the unstarted app twice") appOne := apps.Push(apps.WithApp(apps.PostgreSQL)) @@ -59,7 +67,7 @@ var _ = Describe("UpgradePostgreSQLTest", Label("postgresql"), func() { serviceBroker.UpdateBroker(developmentBuildDir) By("validating that the instance plan is still active") - Expect(plans.ExistsAndAvailable("small", "csb-google-postgres", serviceBroker.Name)) + Expect(plans.ExistsAndAvailable(servicePlan, serviceOffering, serviceBroker.Name)) By("upgrading service instance") serviceInstance.Upgrade() diff --git a/acceptance-tests/upgrade/update_and_upgrade_storage_test.go b/acceptance-tests/upgrade/update_and_upgrade_storage_test.go index 3e48b5d6..0b7ad6e2 100644 --- a/acceptance-tests/upgrade/update_and_upgrade_storage_test.go +++ b/acceptance-tests/upgrade/update_and_upgrade_storage_test.go @@ -23,12 +23,20 @@ var _ = Describe("UpgradeStorageTest", Label("storage"), func() { defer serviceBroker.Delete() By("creating a service") + serviceOffering := "csb-google-storage-bucket" + servicePlan := "default" + serviceName := random.Name(random.WithPrefix(serviceOffering, servicePlan)) + // CreateInstance can fail and can leave a service record (albeit a failed one) lying around. + // We can't delete service brokers that have serviceInstances, so we need to ensure the service instance + // is cleaned up regardless as to whether it wa successful. This is important when we use our own service broker + // (which can only have 5 instances at any time) to prevent subsequent test failures. + defer services.Delete(serviceName) serviceInstance := services.CreateInstance( - "csb-google-storage-bucket", - "default", + serviceOffering, + servicePlan, services.WithBroker(serviceBroker), + services.WithName(serviceName), ) - defer serviceInstance.Delete() By("pushing the unstarted app twice") appOne := apps.Push(apps.WithApp(apps.Storage)) @@ -53,7 +61,7 @@ var _ = Describe("UpgradeStorageTest", Label("storage"), func() { serviceBroker.UpdateBroker(developmentBuildDir) By("validating that the instance plan is still active") - Expect(plans.ExistsAndAvailable("default", "csb-google-storage-bucket", serviceBroker.Name)) + Expect(plans.ExistsAndAvailable(servicePlan, serviceOffering, serviceBroker.Name)) By("upgrading service instance") serviceInstance.Upgrade() diff --git a/acceptance-tests/withoutcredhub_test.go b/acceptance-tests/withoutcredhub_test.go index 97fd479e..8470d40a 100644 --- a/acceptance-tests/withoutcredhub_test.go +++ b/acceptance-tests/withoutcredhub_test.go @@ -21,14 +21,21 @@ var _ = Describe("Without CredHub", Label("withoutcredhub"), func() { defer broker.Delete() By("creating a service instance") + serviceOffering := "csb-google-storage-bucket" + servicePlan := "default" + serviceName := random.Name(random.WithPrefix(serviceOffering, servicePlan)) + defer services.Delete(serviceName) + // CreateInstance can fail and can leave a service record (albeit a failed one) lying around. + // We can't delete service brokers that have serviceInstances, so we need to ensure the service instance + // is cleaned up regardless as to whether it wa successful. This is important when we use our own service broker + // (which can only have 5 instances at any time) to prevent subsequent test failures. serviceInstance := services.CreateInstance( - "csb-google-storage-bucket", - "default", + serviceOffering, + servicePlan, services.WithBroker(broker), + services.WithName(serviceName), ) - defer serviceInstance.Delete() - By("pushing the unstarted app") app := apps.Push(apps.WithApp(apps.Storage)) defer apps.Delete(app)