From f46d1b5c9424c413fb1eb2942fa935620bd92b53 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 15 Apr 2020 12:03:29 -0400 Subject: [PATCH] agent/structs: Remove ServiceID.Init and CheckID.Init The Init method provided the same functionality as the New constructor. The constructor is both more widely used, and more idiomatic, so remove the Init method. This change is in preparation for fixing printing of these IDs. --- agent/agent.go | 20 +++++++------------- agent/agent_endpoint.go | 6 ++---- agent/local/state.go | 9 +++------ agent/structs/structs.go | 12 ++---------- agent/ui_endpoint.go | 4 +--- 5 files changed, 15 insertions(+), 36 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 581199f229..d03ff87ca3 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -2456,8 +2456,7 @@ func (a *Agent) addServiceInternal(req *addServiceRequest, snap map[structs.Chec } } - var cid structs.CheckID - cid.Init(types.CheckID(checkID), &service.EnterpriseMeta) + cid := structs.NewCheckID(types.CheckID(checkID), &service.EnterpriseMeta) existingChecks[cid] = true name := chkType.Name @@ -2531,8 +2530,7 @@ func (a *Agent) addServiceInternal(req *addServiceRequest, snap map[structs.Chec // If a proxy service wishes to expose checks, check targets need to be rerouted to the proxy listener // This needs to be called after chkTypes are added to the agent, to avoid being overwritten - var psid structs.ServiceID - psid.Init(service.Proxy.DestinationServiceID, &service.EnterpriseMeta) + psid := structs.NewServiceID(service.Proxy.DestinationServiceID, &service.EnterpriseMeta) if service.Proxy.Expose.Checks { err := a.rerouteExposedChecks(psid, service.Address) @@ -2740,8 +2738,7 @@ func (a *Agent) removeServiceLocked(serviceID structs.ServiceID, persist bool) e svc := a.State.Service(serviceID) if svc != nil { - var psid structs.ServiceID - psid.Init(svc.Proxy.DestinationServiceID, &svc.EnterpriseMeta) + psid := structs.NewServiceID(svc.Proxy.DestinationServiceID, &svc.EnterpriseMeta) a.resetExposedChecks(psid) } @@ -2784,8 +2781,7 @@ func (a *Agent) removeServiceLocked(serviceID structs.ServiceID, persist bool) e } func (a *Agent) removeServiceSidecars(serviceID structs.ServiceID, persist bool) error { - var sidecarSID structs.ServiceID - sidecarSID.Init(a.sidecarServiceID(serviceID.ID), &serviceID.EnterpriseMeta) + sidecarSID := structs.NewServiceID(a.sidecarServiceID(serviceID.ID), &serviceID.EnterpriseMeta) if sidecar := a.State.Service(sidecarSID); sidecar != nil { // Double check that it's not just an ID collision and we actually added // this from a sidecar. @@ -3138,8 +3134,7 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType, rpcReq.Token = token } - var aliasServiceID structs.ServiceID - aliasServiceID.Init(chkType.AliasService, &check.EnterpriseMeta) + aliasServiceID := structs.NewServiceID(chkType.AliasService, &check.EnterpriseMeta) chkImpl := &checks.CheckAlias{ Notify: a.State, RPC: a.delegate, @@ -3927,9 +3922,8 @@ func (a *Agent) unloadMetadata() { // serviceMaintCheckID returns the ID of a given service's maintenance check func serviceMaintCheckID(serviceID structs.ServiceID) structs.CheckID { - var cid structs.CheckID - cid.Init(types.CheckID(structs.ServiceMaintPrefix+serviceID.ID), &serviceID.EnterpriseMeta) - return cid + cid := types.CheckID(structs.ServiceMaintPrefix + serviceID.ID) + return structs.NewCheckID(cid, &serviceID.EnterpriseMeta) } // EnableServiceMaintenance will register a false health check against the given diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index b182e5291f..769d755ed2 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -767,8 +767,7 @@ func (s *HTTPServer) AgentHealthServiceByID(resp http.ResponseWriter, req *http. return nil, err } - var sid structs.ServiceID - sid.Init(serviceID, &entMeta) + sid := structs.NewServiceID(serviceID, &entMeta) if service := s.agent.State.Service(sid); service != nil { if authz != nil && authz.ServiceRead(service.Service, &authzContext) != acl.Allow { @@ -830,8 +829,7 @@ func (s *HTTPServer) AgentHealthServiceByName(resp http.ResponseWriter, req *htt result := make([]api.AgentServiceChecksInfo, 0, 16) for _, service := range services { if service.Service == serviceName { - var sid structs.ServiceID - sid.Init(service.ID, &entMeta) + sid := structs.NewServiceID(service.ID, &entMeta) scode, sstatus, healthChecks := agentHealthService(sid, s) serviceInfo := buildAgentService(service) diff --git a/agent/local/state.go b/agent/local/state.go index 026e2550a0..9470716c6b 100644 --- a/agent/local/state.go +++ b/agent/local/state.go @@ -1224,9 +1224,8 @@ func (l *State) syncService(key structs.ServiceID) error { // Given how the register API works, this info is also updated // every time we sync a service. l.nodeInfoInSync = true - var checkKey structs.CheckID for _, check := range checks { - checkKey.Init(check.CheckID, &check.EnterpriseMeta) + checkKey := structs.NewCheckID(check.CheckID, &check.EnterpriseMeta) l.checks[checkKey].InSync = true } l.logger.Info("Synced service", "service", key.String()) @@ -1236,9 +1235,8 @@ func (l *State) syncService(key structs.ServiceID) error { // todo(fs): mark the service and the checks to be in sync to prevent excessive retrying before next full sync // todo(fs): some backoff strategy might be a better solution l.services[key].InSync = true - var checkKey structs.CheckID for _, check := range checks { - checkKey.Init(check.CheckID, &check.EnterpriseMeta) + checkKey := structs.NewCheckID(check.CheckID, &check.EnterpriseMeta) l.checks[checkKey].InSync = true } accessorID := l.aclAccessorID(st) @@ -1272,8 +1270,7 @@ func (l *State) syncCheck(key structs.CheckID) error { SkipNodeUpdate: l.nodeInfoInSync, } - var serviceKey structs.ServiceID - serviceKey.Init(c.Check.ServiceID, &key.EnterpriseMeta) + serviceKey := structs.NewServiceID(c.Check.ServiceID, &key.EnterpriseMeta) // Pull in the associated service if any s := l.services[serviceKey] diff --git a/agent/structs/structs.go b/agent/structs/structs.go index ea2bec108b..d558662a27 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -1619,11 +1619,6 @@ type CheckID struct { func NewCheckID(id types.CheckID, entMeta *EnterpriseMeta) CheckID { var cid CheckID - cid.Init(id, entMeta) - return cid -} - -func (cid *CheckID) Init(id types.CheckID, entMeta *EnterpriseMeta) { cid.ID = id if entMeta == nil { entMeta = DefaultEnterpriseMeta() @@ -1631,6 +1626,7 @@ func (cid *CheckID) Init(id types.CheckID, entMeta *EnterpriseMeta) { cid.EnterpriseMeta = *entMeta cid.EnterpriseMeta.Normalize() + return cid } // StringHash is used mainly to populate part of the filename of a check @@ -1649,11 +1645,6 @@ type ServiceID struct { func NewServiceID(id string, entMeta *EnterpriseMeta) ServiceID { var sid ServiceID - sid.Init(id, entMeta) - return sid -} - -func (sid *ServiceID) Init(id string, entMeta *EnterpriseMeta) { sid.ID = id if entMeta == nil { entMeta = DefaultEnterpriseMeta() @@ -1661,6 +1652,7 @@ func (sid *ServiceID) Init(id string, entMeta *EnterpriseMeta) { sid.EnterpriseMeta = *entMeta sid.EnterpriseMeta.Normalize() + return sid } func (sid *ServiceID) Matches(other *ServiceID) bool { diff --git a/agent/ui_endpoint.go b/agent/ui_endpoint.go index c2fdae6cf0..8e38458e58 100644 --- a/agent/ui_endpoint.go +++ b/agent/ui_endpoint.go @@ -178,11 +178,9 @@ func summarizeServices(dump structs.CheckServiceNodes) []*ServiceSummary { return serv } - var sid structs.ServiceID for _, csn := range dump { svc := csn.Service - sid.Init(svc.Service, &svc.EnterpriseMeta) - sum := getService(sid) + sum := getService(structs.NewServiceID(svc.Service, &svc.EnterpriseMeta)) sum.Nodes = append(sum.Nodes, csn.Node.Node) sum.Kind = svc.Kind sum.InstanceCount += 1