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.
This commit is contained in:
Daniel Nephin 2020-11-30 14:08:26 -05:00
parent 5b6f806f4f
commit 4da0718c57
2 changed files with 29 additions and 50 deletions

View File

@ -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 // This entry is persistent and the agent will make a best effort to
// ensure it is registered // ensure it is registered
func (a *Agent) AddService(req AddServiceRequest) error { func (a *Agent) AddService(req AddServiceRequest) error {
// service *structs.NodeService, chkTypes []*structs.CheckType, persist bool, token string, source configSource
req.waitForCentralConfig = true req.waitForCentralConfig = true
req.persistServiceConfig = true req.persistServiceConfig = true
a.stateLock.Lock() a.stateLock.Lock()
@ -1952,18 +1951,7 @@ type addServiceInternalRequest struct {
// addServiceInternal adds the given service and checks to the local state. // addServiceInternal adds the given service and checks to the local state.
func (a *Agent) addServiceInternal(req addServiceInternalRequest) error { func (a *Agent) addServiceInternal(req addServiceInternalRequest) error {
var ( service := req.Service
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
)
// Pause the service syncs during modification // Pause the service syncs during modification
a.PauseSync() a.PauseSync()
@ -1999,11 +1987,11 @@ func (a *Agent) addServiceInternal(req addServiceInternalRequest) error {
} }
// Create an associated health check // Create an associated health check
for i, chkType := range chkTypes { for i, chkType := range req.chkTypes {
checkID := string(chkType.CheckID) checkID := string(chkType.CheckID)
if checkID == "" { if checkID == "" {
checkID = fmt.Sprintf("service:%s", service.ID) checkID = fmt.Sprintf("service:%s", service.ID)
if len(chkTypes) > 1 { if len(req.chkTypes) > 1 {
checkID += fmt.Sprintf(":%d", i+1) checkID += fmt.Sprintf(":%d", i+1)
} }
} }
@ -2032,7 +2020,7 @@ func (a *Agent) addServiceInternal(req addServiceInternalRequest) error {
} }
// Restore the fields from the snapshot. // Restore the fields from the snapshot.
prev, ok := snap[cid] prev, ok := req.snap[cid]
if ok { if ok {
check.Output = prev.Output check.Output = prev.Output
check.Status = prev.Status 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 { if err != nil {
a.cleanupRegistration(cleanupServices, cleanupChecks) a.cleanupRegistration(cleanupServices, cleanupChecks)
return err return err
} }
source := req.Source
persist := req.persist
for i := range checks { 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) a.cleanupRegistration(cleanupServices, cleanupChecks)
return err return err
} }
if persist && a.config.DataDir != "" { 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) a.cleanupRegistration(cleanupServices, cleanupChecks)
return err return err
@ -2095,10 +2085,10 @@ func (a *Agent) addServiceInternal(req addServiceInternalRequest) error {
a.resetExposedChecks(psid) a.resetExposedChecks(psid)
} }
if persistServiceConfig && a.config.DataDir != "" { if req.persistServiceConfig && a.config.DataDir != "" {
var err error var err error
if persistDefaults != nil { if req.persistDefaults != nil {
err = a.persistServiceConfig(service.CompoundServiceID(), persistDefaults) err = a.persistServiceConfig(service.CompoundServiceID(), req.persistDefaults)
} else { } else {
err = a.purgeServiceConfig(service.CompoundServiceID()) err = a.purgeServiceConfig(service.CompoundServiceID())
} }
@ -2111,17 +2101,17 @@ func (a *Agent) addServiceInternal(req addServiceInternalRequest) error {
// Persist the service to a file // Persist the service to a file
if persist && a.config.DataDir != "" { if persist && a.config.DataDir != "" {
if persistService == nil { if req.persistService == nil {
persistService = service req.persistService = service
} }
if err := a.persistService(persistService, source); err != nil { if err := a.persistService(req.persistService, source); err != nil {
a.cleanupRegistration(cleanupServices, cleanupChecks) a.cleanupRegistration(cleanupServices, cleanupChecks)
return err return err
} }
} }
if replaceExistingChecks { if req.replaceExistingChecks {
for checkID, keep := range existingChecks { for checkID, keep := range existingChecks {
if !keep { if !keep {
a.removeCheckLocked(checkID, persist) a.removeCheckLocked(checkID, persist)

View File

@ -131,31 +131,20 @@ func (s *ServiceManager) AddService(req AddServiceRequest) error {
return s.agent.addServiceInternal(addServiceInternalRequest{AddServiceRequest: req}) return s.agent.addServiceInternal(addServiceInternalRequest{AddServiceRequest: req})
} }
var ( // TODO: replace serviceRegistration with AddServiceRequest
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
)
reg := &serviceRegistration{ reg := &serviceRegistration{
service: service, service: req.Service,
chkTypes: chkTypes, chkTypes: req.chkTypes,
persist: persist, persist: req.persist,
token: token, token: req.token,
replaceExistingChecks: replaceExistingChecks, replaceExistingChecks: req.replaceExistingChecks,
source: source, source: req.Source,
} }
s.servicesLock.Lock() s.servicesLock.Lock()
defer s.servicesLock.Unlock() defer s.servicesLock.Unlock()
sid := service.CompoundServiceID() sid := req.Service.CompoundServiceID()
// If a service watch already exists, shut it down and replace it. // If a service watch already exists, shut it down and replace it.
oldWatch, updating := s.services[sid] oldWatch, updating := s.services[sid]
@ -174,9 +163,9 @@ func (s *ServiceManager) AddService(req AddServiceRequest) error {
err := watch.RegisterAndStart( err := watch.RegisterAndStart(
s.ctx, s.ctx,
previousDefaults, req.previousDefaults,
waitForCentralConfig, req.waitForCentralConfig,
persistServiceConfig, req.persistServiceConfig,
&s.running, &s.running,
) )
if err != nil { if err != nil {
@ -186,9 +175,9 @@ func (s *ServiceManager) AddService(req AddServiceRequest) error {
s.services[sid] = watch s.services[sid] = watch
if updating { 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 { } 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 return nil