From 45b315060ae4aa58deeb693e20d3fb2aaeb62936 Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@hashicorp.com>
Date: Mon, 30 Nov 2020 17:50:06 -0500
Subject: [PATCH] agent: Minor cosmetic changes in ServiceManager

Also use the non-deprecated func in a test
---
 agent/service_manager.go      | 20 +++++++-------------
 agent/service_manager_test.go | 11 +++++++++--
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/agent/service_manager.go b/agent/service_manager.go
index eddc12cfe7..ba226bb44a 100644
--- a/agent/service_manager.go
+++ b/agent/service_manager.go
@@ -134,7 +134,10 @@ func (s *ServiceManager) AddService(req addServiceLockedRequest) error {
 		agent:        s.agent,
 		registerCh:   s.registerCh,
 	}
-	if err := watch.RegisterAndStart(s.ctx, &s.running); err != nil {
+	if err := watch.register(s.ctx); err != nil {
+		return err
+	}
+	if err := watch.start(s.ctx, &s.running); err != nil {
 		return err
 	}
 
@@ -178,7 +181,7 @@ type serviceConfigWatch struct {
 }
 
 // NOTE: this is called while holding the Agent.stateLock
-func (w *serviceConfigWatch) RegisterAndStart(ctx context.Context, wg *sync.WaitGroup) error {
+func (w *serviceConfigWatch) register(ctx context.Context) error {
 	serviceDefaults, err := w.registration.serviceDefaults(ctx)
 	if err != nil {
 		return fmt.Errorf("could not retrieve initial service_defaults config for service %q: %v",
@@ -204,10 +207,7 @@ func (w *serviceConfigWatch) RegisterAndStart(ctx context.Context, wg *sync.Wait
 	if err != nil {
 		return fmt.Errorf("error updating service registration: %v", err)
 	}
-
-	// Start the config watch, which starts a blocking query for the
-	// resolved service config in the background.
-	return w.start(ctx, wg)
+	return nil
 }
 
 func serviceDefaultsFromStruct(v *structs.ServiceConfigResponse) func(context.Context) (*structs.ServiceConfigResponse, error) {
@@ -256,13 +256,7 @@ func (w *serviceConfigWatch) start(ctx context.Context, wg *sync.WaitGroup) erro
 	// context before we cancel and so might still deliver the old event. Using
 	// the cacheKey allows us to ignore updates from the old cache watch and makes
 	// even this rare edge case safe.
-	err := w.agent.cache.Notify(
-		ctx,
-		cachetype.ResolvedServiceConfigName,
-		req,
-		w.cacheKey,
-		updateCh,
-	)
+	err := w.agent.cache.Notify(ctx, cachetype.ResolvedServiceConfigName, req, w.cacheKey, updateCh)
 	if err != nil {
 		w.cancelFunc()
 		return err
diff --git a/agent/service_manager_test.go b/agent/service_manager_test.go
index ea1d6c4f11..8ed22e9f5a 100644
--- a/agent/service_manager_test.go
+++ b/agent/service_manager_test.go
@@ -387,12 +387,19 @@ func TestServiceManager_PersistService_API(t *testing.T) {
 	configFile := filepath.Join(a.Config.DataDir, serviceConfigDir, svcID.StringHash())
 
 	// Service is not persisted unless requested, but we always persist service configs.
-	require.NoError(a.addServiceFromSource(svc, nil, false, "", ConfigSourceRemote))
+	err = a.AddService(AddServiceRequest{Service: svc, Source: ConfigSourceRemote})
+	require.NoError(err)
 	requireFileIsAbsent(t, svcFile)
 	requireFileIsPresent(t, configFile)
 
 	// Persists to file if requested
-	require.NoError(a.addServiceFromSource(svc, nil, true, "mytoken", ConfigSourceRemote))
+	err = a.AddService(AddServiceRequest{
+		Service: svc,
+		persist: true,
+		token:   "mytoken",
+		Source:  ConfigSourceRemote,
+	})
+	require.NoError(err)
 	requireFileIsPresent(t, svcFile)
 	requireFileIsPresent(t, configFile)