From 4da0718c57c53926d39510be774a9256b6860897 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 30 Nov 2020 14:08:26 -0500 Subject: [PATCH] agent: use fields directly, not temp variables The temprorary variables make it much harder to trace where and how struct fields are used. If a field is only used a small number of times than refer to the field directly. --- agent/agent.go | 42 +++++++++++++++------------------------- agent/service_manager.go | 37 +++++++++++++---------------------- 2 files changed, 29 insertions(+), 50 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 38a02792fa..6d688b804f 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -1894,7 +1894,6 @@ func (a *Agent) readPersistedServiceConfigs() (map[structs.ServiceID]*structs.Se // This entry is persistent and the agent will make a best effort to // ensure it is registered func (a *Agent) AddService(req AddServiceRequest) error { - // service *structs.NodeService, chkTypes []*structs.CheckType, persist bool, token string, source configSource req.waitForCentralConfig = true req.persistServiceConfig = true a.stateLock.Lock() @@ -1952,18 +1951,7 @@ type addServiceInternalRequest struct { // addServiceInternal adds the given service and checks to the local state. func (a *Agent) addServiceInternal(req addServiceInternalRequest) error { - var ( - service = req.Service - chkTypes = req.chkTypes - persistService = req.persistService - persistDefaults = req.persistDefaults - persist = req.persist - persistServiceConfig = req.persistServiceConfig - token = req.token - replaceExistingChecks = req.replaceExistingChecks - source = req.Source - snap = req.snap - ) + service := req.Service // Pause the service syncs during modification a.PauseSync() @@ -1999,11 +1987,11 @@ func (a *Agent) addServiceInternal(req addServiceInternalRequest) error { } // Create an associated health check - for i, chkType := range chkTypes { + for i, chkType := range req.chkTypes { checkID := string(chkType.CheckID) if checkID == "" { checkID = fmt.Sprintf("service:%s", service.ID) - if len(chkTypes) > 1 { + if len(req.chkTypes) > 1 { checkID += fmt.Sprintf(":%d", i+1) } } @@ -2032,7 +2020,7 @@ func (a *Agent) addServiceInternal(req addServiceInternalRequest) error { } // Restore the fields from the snapshot. - prev, ok := snap[cid] + prev, ok := req.snap[cid] if ok { check.Output = prev.Output check.Status = prev.Status @@ -2059,20 +2047,22 @@ func (a *Agent) addServiceInternal(req addServiceInternalRequest) error { } } - err := a.State.AddServiceWithChecks(service, checks, token) + err := a.State.AddServiceWithChecks(service, checks, req.token) if err != nil { a.cleanupRegistration(cleanupServices, cleanupChecks) return err } + source := req.Source + persist := req.persist for i := range checks { - if err := a.addCheck(checks[i], chkTypes[i], service, token, source); err != nil { + if err := a.addCheck(checks[i], req.chkTypes[i], service, req.token, source); err != nil { a.cleanupRegistration(cleanupServices, cleanupChecks) return err } if persist && a.config.DataDir != "" { - if err := a.persistCheck(checks[i], chkTypes[i], source); err != nil { + if err := a.persistCheck(checks[i], req.chkTypes[i], source); err != nil { a.cleanupRegistration(cleanupServices, cleanupChecks) return err @@ -2095,10 +2085,10 @@ func (a *Agent) addServiceInternal(req addServiceInternalRequest) error { a.resetExposedChecks(psid) } - if persistServiceConfig && a.config.DataDir != "" { + if req.persistServiceConfig && a.config.DataDir != "" { var err error - if persistDefaults != nil { - err = a.persistServiceConfig(service.CompoundServiceID(), persistDefaults) + if req.persistDefaults != nil { + err = a.persistServiceConfig(service.CompoundServiceID(), req.persistDefaults) } else { err = a.purgeServiceConfig(service.CompoundServiceID()) } @@ -2111,17 +2101,17 @@ func (a *Agent) addServiceInternal(req addServiceInternalRequest) error { // Persist the service to a file if persist && a.config.DataDir != "" { - if persistService == nil { - persistService = service + if req.persistService == nil { + req.persistService = service } - if err := a.persistService(persistService, source); err != nil { + if err := a.persistService(req.persistService, source); err != nil { a.cleanupRegistration(cleanupServices, cleanupChecks) return err } } - if replaceExistingChecks { + if req.replaceExistingChecks { for checkID, keep := range existingChecks { if !keep { a.removeCheckLocked(checkID, persist) diff --git a/agent/service_manager.go b/agent/service_manager.go index 5fcaa33340..5badb3a263 100644 --- a/agent/service_manager.go +++ b/agent/service_manager.go @@ -131,31 +131,20 @@ func (s *ServiceManager) AddService(req AddServiceRequest) error { return s.agent.addServiceInternal(addServiceInternalRequest{AddServiceRequest: req}) } - var ( - service = req.Service - chkTypes = req.chkTypes - previousDefaults = req.previousDefaults - waitForCentralConfig = req.waitForCentralConfig - persist = req.persist - persistServiceConfig = req.persistServiceConfig - token = req.token - replaceExistingChecks = req.replaceExistingChecks - source = req.Source - ) - + // TODO: replace serviceRegistration with AddServiceRequest reg := &serviceRegistration{ - service: service, - chkTypes: chkTypes, - persist: persist, - token: token, - replaceExistingChecks: replaceExistingChecks, - source: source, + service: req.Service, + chkTypes: req.chkTypes, + persist: req.persist, + token: req.token, + replaceExistingChecks: req.replaceExistingChecks, + source: req.Source, } s.servicesLock.Lock() defer s.servicesLock.Unlock() - sid := service.CompoundServiceID() + sid := req.Service.CompoundServiceID() // If a service watch already exists, shut it down and replace it. oldWatch, updating := s.services[sid] @@ -174,9 +163,9 @@ func (s *ServiceManager) AddService(req AddServiceRequest) error { err := watch.RegisterAndStart( s.ctx, - previousDefaults, - waitForCentralConfig, - persistServiceConfig, + req.previousDefaults, + req.waitForCentralConfig, + req.persistServiceConfig, &s.running, ) if err != nil { @@ -186,9 +175,9 @@ func (s *ServiceManager) AddService(req AddServiceRequest) error { s.services[sid] = watch if updating { - s.agent.logger.Debug("updated local registration for service", "service", service.ID) + s.agent.logger.Debug("updated local registration for service", "service", req.Service.ID) } else { - s.agent.logger.Debug("added local registration for service", "service", service.ID) + s.agent.logger.Debug("added local registration for service", "service", req.Service.ID) } return nil