Uses config to toggle between Persister and DisabledPersister

Based on the config option, if the feature is enabled, we will use the real Persister to create the directory and manifests. Otherwise, use the DisabledPersister which will do nothing. For the cleaner, we always use the Persister - this means we can always clean up old manifests when an instance is deleted, even if the feature is disabled after being previously enabled [#186826718](https://www.pivotaltracker.com/story/show/186826718) Co-authored-by: Ryan Wittrup <[email protected]> Co-authored-by: Andrew Garner <[email protected]>
pivotal-cf · Jun 19, 2024 · 8aaf120 · 8aaf120
1 parent 3177d71
commit 8aaf120
Show file tree

Hide file tree

Showing 8 changed files with 27 additions and 12 deletions.
diff --git a/brokerinitiator/start.go b/brokerinitiator/start.go
@@ -54,12 +54,24 @@ func Initiate(conf config.Config,
 	odbSecrets := manifestsecrets.ODBSecrets{ServiceOfferingID: conf.ServiceCatalog.ID}
 	boshCredhubStore := buildCredhubStore(conf, logger)
 
-	// TODO: Add conditional based on config
-	persister := manifest.Persister{
+	cleaner := manifest.Persister{
 		Prefix: "/var/vcap/data/broker/manifest/",
 		Logger: logger,
 	}
-	deploymentManager := task.NewDeployer(taskBoshClient, manifestGenerator, odbSecrets, boshCredhubStore, &persister)
+
+	var persister interface {
+		PersistManifest(deploymentName, manifestName string, data []byte)
+	}
+	if conf.Broker.EnablePersistManifest {
+		persister = manifest.Persister{
+			Prefix: "/var/vcap/data/broker/manifest/",
+			Logger: logger,
+		}
+	} else {
+		persister = manifest.DisabledPersister{}
+	}
+
+	deploymentManager := task.NewDeployer(taskBoshClient, manifestGenerator, odbSecrets, boshCredhubStore, persister)
 	deploymentManager.DisableBoshConfigs = conf.Broker.DisableBoshConfigs
 
 	manifestSecretManager := manifestsecrets.BuildManager(conf.Broker.EnableSecureManifests, new(manifestsecrets.CredHubPathMatcher), boshCredhubStore)
@@ -72,7 +84,7 @@ func Initiate(conf config.Config,
 	telemetryLogger := telemetry.Build(conf.Broker.EnableTelemetry, conf.ServiceCatalog, logger)
 
 	var onDemandBroker apiserver.CombinedBroker
-	onDemandBroker, err = broker.New(brokerBoshClient, cfClient, conf.ServiceCatalog, conf.Broker, startupChecks, serviceAdapter, deploymentManager, manifestSecretManager, instanceLister, &hasher.MapHasher{}, loggerFactory, telemetryLogger, decider.Decider{}, &persister)
+	onDemandBroker, err = broker.New(brokerBoshClient, cfClient, conf.ServiceCatalog, conf.Broker, startupChecks, serviceAdapter, deploymentManager, manifestSecretManager, instanceLister, &hasher.MapHasher{}, loggerFactory, telemetryLogger, decider.Decider{}, cleaner)
 
 	if err != nil {
 		logger.Fatalf("error starting broker: %s", err)

diff --git a/collaboration_tests/helpers/server_helpers.go b/collaboration_tests/helpers/server_helpers.go
@@ -57,7 +57,7 @@ func StartServer(conf config.Config, stopServerChan chan os.Signal, fakeCommandR
 	taskManifestGenerator := task.NewManifestGenerator(serviceAdapterClient, conf.ServiceCatalog, []serviceadapter.Stemcell{}, serviceadapter.ServiceReleases{})
 	odbSecrets := manifestsecrets.ODBSecrets{ServiceOfferingID: conf.ServiceCatalog.ID}
 
-	deployer := task.NewDeployer(fakeTaskBoshClient, taskManifestGenerator, odbSecrets, fakeTaskBulkSetter, &manifest.DisabledPersister{})
+	deployer := task.NewDeployer(fakeTaskBoshClient, taskManifestGenerator, odbSecrets, fakeTaskBulkSetter, manifest.DisabledPersister{})
 	deployer.DisableBoshConfigs = conf.Broker.DisableBoshConfigs
 
 	loggerFactory := loggerfactory.New(loggerBuffer, "collaboration-tests", loggerfactory.Flags)
@@ -69,7 +69,7 @@ func StartServer(conf config.Config, stopServerChan chan os.Signal, fakeCommandR
 	credhubPathMatcher := new(manifestsecrets.CredHubPathMatcher)
 	secretManager := manifestsecrets.BuildManager(true, credhubPathMatcher, fakeCredhubOperator)
 
-	fakeOnDemandBroker, err := broker.New(fakeBoshClient, fakeCfClient, conf.ServiceCatalog, conf.Broker, nil, serviceAdapterClient, deployer, secretManager, instanceLister, fakeMapHasher, loggerFactory, new(fakes.FakeTelemetryLogger), decider.Decider{}, &manifest.DisabledPersister{})
+	fakeOnDemandBroker, err := broker.New(fakeBoshClient, fakeCfClient, conf.ServiceCatalog, conf.Broker, nil, serviceAdapterClient, deployer, secretManager, instanceLister, fakeMapHasher, loggerFactory, new(fakes.FakeTelemetryLogger), decider.Decider{}, manifest.DisabledPersister{})
 	Expect(err).NotTo(HaveOccurred())
 	var fakeBroker apiserver.CombinedBroker
 	if conf.HasRuntimeCredHub() {

diff --git a/config/config.go b/config/config.go
@@ -53,6 +53,7 @@ type Broker struct {
 	EnableOptimisedUpgrades    bool      `yaml:"enable_optimised_upgrades"`
 	SupportBackupAgentBinding  bool      `yaml:"support_backup_agent_binding"`
 	TLS                        TLSConfig `yaml:"tls"`
+	EnablePersistManifest      bool      `yaml:"enable_persist_manifest"`
 }
 
 type BoshCredhub struct {

diff --git a/config/config_test.go b/config/config_test.go
@@ -66,6 +66,7 @@ var _ = Describe("Broker Config", func() {
 						UsingStdin:                 true,
 						EnableTelemetry:            true,
 						SupportBackupAgentBinding:  true,
+						EnablePersistManifest:      true,
 					},
 					Bosh: config.Bosh{
 						URL:         "some-url",

diff --git a/config/test_assets/good_config.yml b/config/test_assets/good_config.yml
@@ -15,6 +15,7 @@ broker:
   use_stdin: true
   enable_telemetry: true
   support_backup_agent_binding: true
+  enable_persist_manifest: true
 bosh:
   url: some-url
   root_ca_cert: some-cert

diff --git a/manifest/disabled_persister.go b/manifest/disabled_persister.go
@@ -3,6 +3,6 @@ package manifest
 type DisabledPersister struct {
 }
 
-func (p *DisabledPersister) PersistManifest(deploymentName, manifestName string, data []byte) {}
+func (p DisabledPersister) PersistManifest(deploymentName, manifestName string, data []byte) {}
 
-func (p *DisabledPersister) Cleanup(deploymentName string) {}
+func (p DisabledPersister) Cleanup(deploymentName string) {}
diff --git a/manifest/persister.go b/manifest/persister.go
@@ -12,7 +12,7 @@ type Persister struct {
 	Logger *log.Logger
 }
 
-func (p *Persister) PersistManifest(deploymentName, manifestName string, data []byte) {
+func (p Persister) PersistManifest(deploymentName, manifestName string, data []byte) {
 	directory := filepath.Join(p.Prefix, deploymentName)
 	path := filepath.Join(directory, manifestName+".gz")
 	if err := os.Mkdir(directory, 0750); err != nil && !os.IsExist(err) {
@@ -41,7 +41,7 @@ func (p *Persister) PersistManifest(deploymentName, manifestName string, data []
 	}
 }
 
-func (p *Persister) Cleanup(deploymentName string) {
+func (p Persister) Cleanup(deploymentName string) {
 	directory := filepath.Join(p.Prefix, deploymentName)
 
 	if err := os.RemoveAll(directory); err != nil {

diff --git a/system_tests/pending_changes_blocked_tests/fixtures/add_persisted_manifests.yml b/system_tests/pending_changes_blocked_tests/fixtures/add_persisted_manifests.yml
@@ -1,3 +1,3 @@
 - type: replace
-  path: /instance_groups/name=broker/jobs/name=broker/properties/persist_manifests?
-  value: true
+  path: /instance_groups/name=broker/jobs/name=broker/properties/enable_persist_manifest?
+  value: true