agent: addServiceIternalRequest

Move fields that are only relevant for addServiceInternal onto a new
struct and embed AddServiceRequest.

agent/agent.go

@@ -1901,14 +1901,12 @@ func (a *Agent) AddService(req AddServiceRequest) error {
 	defer a.stateLock.Unlock()
 
 	req.snap = a.State.Checks(structs.WildcardEnterpriseMeta())
-	return a.addServiceLocked(&req)
+	return a.addServiceLocked(req)
 }
 
 // addServiceLocked adds a service entry to the service manager if enabled, or directly
 // to the local state if it is not. This function assumes the state lock is already held.
-func (a *Agent) addServiceLocked(req *AddServiceRequest) error {
-	req.fixupForAddServiceLocked()
-
+func (a *Agent) addServiceLocked(req AddServiceRequest) error {
 	req.Service.EnterpriseMeta.Normalize()
 
 	if err := a.validateService(req.Service, req.chkTypes); err != nil {
@@ -1919,12 +1917,8 @@ func (a *Agent) addServiceLocked(req *AddServiceRequest) error {
 		return a.serviceManager.AddService(req)
 	}
 
-	// previousDefaults are ignored here because they are only relevant for central config.
-	req.persistService = nil
-	req.persistDefaults = nil
 	req.persistServiceConfig = false
-
-	return a.addServiceInternal(req)
+	return a.addServiceInternal(addServiceInternalRequest{AddServiceRequest: req})
 }
 
 // AddServiceRequest is the union of arguments for calling both
@@ -1942,8 +1936,6 @@ type AddServiceRequest struct {
 	chkTypes              []*structs.CheckType
 	previousDefaults      *structs.ServiceConfigResponse // just for: addServiceLocked
 	waitForCentralConfig  bool                           // just for: addServiceLocked
-	persistService        *structs.NodeService           // just for: addServiceInternal
-	persistDefaults       *structs.ServiceConfigResponse // just for: addServiceInternal
 	persist               bool
 	persistServiceConfig  bool
 	token                 string
@@ -1952,19 +1944,14 @@ type AddServiceRequest struct {
 	snap                  map[structs.CheckID]*structs.HealthCheck
 }
 
-func (r *AddServiceRequest) fixupForAddServiceLocked() {
-	r.persistService = nil
-	r.persistDefaults = nil
-}
-
-func (r *AddServiceRequest) fixupForAddServiceInternal() {
-	r.previousDefaults = nil
-	r.waitForCentralConfig = false
+type addServiceInternalRequest struct {
+	AddServiceRequest
+	persistService  *structs.NodeService
+	persistDefaults *structs.ServiceConfigResponse
 }
 
 // addServiceInternal adds the given service and checks to the local state.
-func (a *Agent) addServiceInternal(req *AddServiceRequest) error {
-	req.fixupForAddServiceInternal()
+func (a *Agent) addServiceInternal(req addServiceInternalRequest) error {
 	var (
 		service               = req.Service
 		chkTypes              = req.chkTypes
@@ -3100,7 +3087,7 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI
 		ns.Connect.SidecarService = nil
 
 		sid := ns.CompoundServiceID()
-		err = a.addServiceLocked(&AddServiceRequest{
+		err = a.addServiceLocked(AddServiceRequest{
 			Service:               ns,
 			chkTypes:              chkTypes,
 			previousDefaults:      persistedServiceConfigs[sid],
@@ -3119,7 +3106,7 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI
 		// If there is a sidecar service, register that too.
 		if sidecar != nil {
 			sidecarServiceID := sidecar.CompoundServiceID()
-			err = a.addServiceLocked(&AddServiceRequest{
+			err = a.addServiceLocked(AddServiceRequest{
 				Service:               sidecar,
 				chkTypes:              sidecarChecks,
 				previousDefaults:      persistedServiceConfigs[sidecarServiceID],
@@ -3216,7 +3203,7 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI
 				"service", serviceID.String(),
 				"file", file,
 			)
-			err = a.addServiceLocked(&AddServiceRequest{
+			err = a.addServiceLocked(AddServiceRequest{
 				Service:               p.Service,
 				chkTypes:              nil,
 				previousDefaults:      persistedServiceConfigs[serviceID],

agent/service_manager.go

@@ -84,7 +84,7 @@ func (s *ServiceManager) Start() {
 }
 
 // runOnce will process a single registration request
-func (s *ServiceManager) registerOnce(args *AddServiceRequest) error {
+func (s *ServiceManager) registerOnce(args addServiceInternalRequest) error {
 	s.agent.stateLock.Lock()
 	defer s.agent.stateLock.Unlock()
 
@@ -121,19 +121,14 @@ func (s *ServiceManager) registerOnce(args *AddServiceRequest) error {
 // merged with the global defaults before registration.
 //
 // NOTE: the caller must hold the Agent.stateLock!
-func (s *ServiceManager) AddService(req *AddServiceRequest) error {
-	req.fixupForAddServiceLocked()
-
+func (s *ServiceManager) AddService(req AddServiceRequest) error {
 	req.Service.EnterpriseMeta.Normalize()
 
 	// For now only proxies have anything that can be configured
 	// centrally. So bypass the whole manager for regular services.
 	if !req.Service.IsSidecarProxy() && !req.Service.IsGateway() {
-		// previousDefaults are ignored here because they are only relevant for central config.
-		req.persistService = nil
-		req.persistDefaults = nil
 		req.persistServiceConfig = false
-		return s.agent.addServiceInternal(req)
+		return s.agent.addServiceInternal(addServiceInternalRequest{AddServiceRequest: req})
 	}
 
 	var (
@@ -268,17 +263,19 @@ func (w *serviceConfigWatch) RegisterAndStart(
 	// The first time we do this interactively, we need to know if it
 	// failed for validation reasons which we only get back from the
 	// initial underlying add service call.
-	err = w.agent.addServiceInternal(&AddServiceRequest{
-		Service:               merged,
-		chkTypes:              w.registration.chkTypes,
-		persistService:        w.registration.service,
-		persistDefaults:       serviceDefaults,
-		persist:               w.registration.persist,
-		persistServiceConfig:  persistServiceConfig,
-		token:                 w.registration.token,
-		replaceExistingChecks: w.registration.replaceExistingChecks,
-		Source:                w.registration.source,
-		snap:                  w.agent.snapshotCheckState(),
+	err = w.agent.addServiceInternal(addServiceInternalRequest{
+		AddServiceRequest: AddServiceRequest{
+			Service:               merged,
+			chkTypes:              w.registration.chkTypes,
+			persist:               w.registration.persist,
+			persistServiceConfig:  persistServiceConfig,
+			token:                 w.registration.token,
+			replaceExistingChecks: w.registration.replaceExistingChecks,
+			Source:                w.registration.source,
+			snap:                  w.agent.snapshotCheckState(),
+		},
+		persistService:  w.registration.service,
+		persistDefaults: serviceDefaults,
 	})
 	if err != nil {
 		return fmt.Errorf("error updating service registration: %v", err)
@@ -409,16 +406,18 @@ func (w *serviceConfigWatch) handleUpdate(ctx context.Context, event cache.Updat
 	}
 
 	registerReq := &asyncRegisterRequest{
-		Args: &AddServiceRequest{
-			Service:               merged,
-			chkTypes:              w.registration.chkTypes,
-			persistService:        w.registration.service,
-			persistDefaults:       serviceDefaults,
-			persist:               w.registration.persist,
-			persistServiceConfig:  true,
-			token:                 w.registration.token,
-			replaceExistingChecks: w.registration.replaceExistingChecks,
-			Source:                w.registration.source,
+		Args: addServiceInternalRequest{
+			AddServiceRequest: AddServiceRequest{
+				Service:               merged,
+				chkTypes:              w.registration.chkTypes,
+				persist:               w.registration.persist,
+				persistServiceConfig:  true,
+				token:                 w.registration.token,
+				replaceExistingChecks: w.registration.replaceExistingChecks,
+				Source:                w.registration.source,
+			},
+			persistService:  w.registration.service,
+			persistDefaults: serviceDefaults,
 		},
 		Reply: make(chan error, 1),
 	}
@@ -442,7 +441,7 @@ func (w *serviceConfigWatch) handleUpdate(ctx context.Context, event cache.Updat
 }
 
 type asyncRegisterRequest struct {
-	Args  *AddServiceRequest
+	Args  addServiceInternalRequest
 	Reply chan error
 }