diff --git a/agent/agent.go b/agent/agent.go index 93958e6d6..ebe6d92a0 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -1893,7 +1893,7 @@ func (a *Agent) readPersistedServiceConfigs() (map[structs.ServiceID]*structs.Se // AddService is used to add a service entry and its check. Any check for this service missing from chkTypes will be deleted. // This entry is persistent and the agent will make a best effort to // 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.persistServiceConfig = true @@ -1904,36 +1904,14 @@ func (a *Agent) AddService(req addServiceRequest) error { return a.addServiceLocked(&req) } -// AddServiceFromSource is used to add a service entry. -// This entry is persistent and the agent will make a best effort to -// ensure it is registered. -// TODO: move to _test.go -// Deprecated: use AddService -func (a *Agent) AddServiceFromSource(service *structs.NodeService, chkTypes []*structs.CheckType, persist bool, token string, source configSource) error { - a.stateLock.Lock() - defer a.stateLock.Unlock() - return a.addServiceLocked(&addServiceRequest{ - service: service, - chkTypes: chkTypes, - previousDefaults: nil, - waitForCentralConfig: true, - persist: persist, - persistServiceConfig: true, - token: token, - replaceExistingChecks: false, - source: source, - snap: a.snapshotCheckState(), - }) -} - // 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 { +func (a *Agent) addServiceLocked(req *AddServiceRequest) error { req.fixupForAddServiceLocked() - req.service.EnterpriseMeta.Normalize() + req.Service.EnterpriseMeta.Normalize() - if err := a.validateService(req.service, req.chkTypes); err != nil { + if err := a.validateService(req.Service, req.chkTypes); err != nil { return err } @@ -1949,7 +1927,7 @@ func (a *Agent) addServiceLocked(req *addServiceRequest) error { return a.addServiceInternal(req) } -// addServiceRequest is the union of arguments for calling both +// AddServiceRequest is the union of arguments for calling both // addServiceLocked and addServiceInternal. The overlap was significant enough // to warrant merging them and indicating which fields are meant to be set only // in one of the two contexts. @@ -1959,8 +1937,8 @@ func (a *Agent) addServiceLocked(req *addServiceRequest) error { // // The ServiceManager.AddService signature is largely just a passthrough for // addServiceLocked and should be treated as such. -type addServiceRequest struct { - service *structs.NodeService +type AddServiceRequest struct { + Service *structs.NodeService chkTypes []*structs.CheckType previousDefaults *structs.ServiceConfigResponse // just for: addServiceLocked waitForCentralConfig bool // just for: addServiceLocked @@ -1970,25 +1948,25 @@ type addServiceRequest struct { persistServiceConfig bool token string replaceExistingChecks bool - source configSource + Source configSource snap map[structs.CheckID]*structs.HealthCheck } -func (r *addServiceRequest) fixupForAddServiceLocked() { +func (r *AddServiceRequest) fixupForAddServiceLocked() { r.persistService = nil r.persistDefaults = nil } -func (r *addServiceRequest) fixupForAddServiceInternal() { +func (r *AddServiceRequest) fixupForAddServiceInternal() { r.previousDefaults = nil r.waitForCentralConfig = false } // addServiceInternal adds the given service and checks to the local state. -func (a *Agent) addServiceInternal(req *addServiceRequest) error { +func (a *Agent) addServiceInternal(req *AddServiceRequest) error { req.fixupForAddServiceInternal() var ( - service = req.service + service = req.Service chkTypes = req.chkTypes persistService = req.persistService persistDefaults = req.persistDefaults @@ -1996,7 +1974,7 @@ func (a *Agent) addServiceInternal(req *addServiceRequest) error { persistServiceConfig = req.persistServiceConfig token = req.token replaceExistingChecks = req.replaceExistingChecks - source = req.source + source = req.Source snap = req.snap ) @@ -3122,8 +3100,8 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI ns.Connect.SidecarService = nil sid := ns.CompoundServiceID() - err = a.addServiceLocked(&addServiceRequest{ - service: ns, + err = a.addServiceLocked(&AddServiceRequest{ + Service: ns, chkTypes: chkTypes, previousDefaults: persistedServiceConfigs[sid], waitForCentralConfig: false, // exclusively use cached values @@ -3131,7 +3109,7 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI persistServiceConfig: false, // don't rewrite the file with the same data we just read token: service.Token, replaceExistingChecks: false, // do default behavior - source: ConfigSourceLocal, + Source: ConfigSourceLocal, snap: snap, }) if err != nil { @@ -3141,8 +3119,8 @@ 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{ - service: sidecar, + err = a.addServiceLocked(&AddServiceRequest{ + Service: sidecar, chkTypes: sidecarChecks, previousDefaults: persistedServiceConfigs[sidecarServiceID], waitForCentralConfig: false, // exclusively use cached values @@ -3150,7 +3128,7 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI persistServiceConfig: false, // don't rewrite the file with the same data we just read token: sidecarToken, replaceExistingChecks: false, // do default behavior - source: ConfigSourceLocal, + Source: ConfigSourceLocal, snap: snap, }) if err != nil { @@ -3238,8 +3216,8 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI "service", serviceID.String(), "file", file, ) - err = a.addServiceLocked(&addServiceRequest{ - service: p.Service, + err = a.addServiceLocked(&AddServiceRequest{ + Service: p.Service, chkTypes: nil, previousDefaults: persistedServiceConfigs[serviceID], waitForCentralConfig: false, // exclusively use cached values @@ -3247,7 +3225,7 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI persistServiceConfig: false, // don't rewrite the file with the same data we just read token: p.Token, replaceExistingChecks: false, // do default behavior - source: source, + Source: source, snap: snap, }) if err != nil { diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index 780933d84..10fe70c6d 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -992,12 +992,12 @@ func (s *HTTPHandlers) AgentRegisterService(resp http.ResponseWriter, req *http. replaceExistingChecks = true } - addReq := addServiceRequest{ - service: ns, + addReq := AddServiceRequest{ + Service: ns, chkTypes: chkTypes, persist: true, token: token, - source: ConfigSourceRemote, + Source: ConfigSourceRemote, replaceExistingChecks: replaceExistingChecks, } if err := s.agent.AddService(addReq); err != nil { @@ -1005,12 +1005,12 @@ func (s *HTTPHandlers) AgentRegisterService(resp http.ResponseWriter, req *http. } if sidecar != nil { - addReq := addServiceRequest{ - service: sidecar, + addReq := AddServiceRequest{ + Service: sidecar, chkTypes: sidecarChecks, persist: true, token: sidecarToken, - source: ConfigSourceRemote, + Source: ConfigSourceRemote, replaceExistingChecks: replaceExistingChecks, } if err := s.agent.AddService(addReq); err != nil { diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index a19b84940..1949ed82a 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -736,21 +736,21 @@ func TestAgent_HealthServiceByID(t *testing.T) { ID: "mysql", Service: "mysql", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } service = &structs.NodeService{ ID: "mysql2", Service: "mysql2", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } service = &structs.NodeService{ ID: "mysql3", Service: "mysql3", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -934,42 +934,42 @@ func TestAgent_HealthServiceByName(t *testing.T) { ID: "mysql1", Service: "mysql-pool-r", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } service = &structs.NodeService{ ID: "mysql2", Service: "mysql-pool-r", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } service = &structs.NodeService{ ID: "mysql3", Service: "mysql-pool-rw", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } service = &structs.NodeService{ ID: "mysql4", Service: "mysql-pool-rw", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } service = &structs.NodeService{ ID: "httpd1", Service: "httpd", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } service = &structs.NodeService{ ID: "httpd2", Service: "httpd", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -1181,13 +1181,13 @@ func TestAgent_HealthServicesACLEnforcement(t *testing.T) { ID: "mysql1", Service: "mysql", } - require.NoError(t, a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal)) + require.NoError(t, a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal)) service = &structs.NodeService{ ID: "foo1", Service: "foo", } - require.NoError(t, a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal)) + require.NoError(t, a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal)) // no token t.Run("no-token-health-by-id", func(t *testing.T) { @@ -4015,10 +4015,10 @@ func testAgent_RegisterServiceDeregisterService_Sidecar(t *testing.T, extraHCL s testrpc.WaitForLeader(t, a.RPC, "dc1") if tt.preRegister != nil { - require.NoError(a.AddServiceFromSource(tt.preRegister, nil, false, "", ConfigSourceLocal)) + require.NoError(a.addServiceFromSource(tt.preRegister, nil, false, "", ConfigSourceLocal)) } if tt.preRegister2 != nil { - require.NoError(a.AddServiceFromSource(tt.preRegister2, nil, false, "", ConfigSourceLocal)) + require.NoError(a.addServiceFromSource(tt.preRegister2, nil, false, "", ConfigSourceLocal)) } // Create an ACL token with require policy @@ -4320,7 +4320,7 @@ func TestAgent_DeregisterService(t *testing.T) { ID: "test", Service: "test", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -4352,7 +4352,7 @@ func TestAgent_DeregisterService_ACLDeny(t *testing.T) { ID: "test", Service: "test", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -4430,7 +4430,7 @@ func TestAgent_ServiceMaintenance_Enable(t *testing.T) { ID: "test", Service: "test", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -4477,7 +4477,7 @@ func TestAgent_ServiceMaintenance_Disable(t *testing.T) { ID: "test", Service: "test", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -4518,7 +4518,7 @@ func TestAgent_ServiceMaintenance_ACLDeny(t *testing.T) { ID: "test", Service: "test", } - if err := a.AddServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(service, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } diff --git a/agent/agent_test.go b/agent/agent_test.go index 71e943694..4fb395277 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -529,7 +529,7 @@ func testAgent_AddService(t *testing.T, extraHCL string) { t.Run(tt.desc, func(t *testing.T) { // check the service registration t.Run(tt.srv.ID, func(t *testing.T) { - err := a.AddServiceFromSource(tt.srv, tt.chkTypes, false, "", ConfigSourceLocal) + err := a.addServiceFromSource(tt.srv, tt.chkTypes, false, "", ConfigSourceLocal) if err != nil { t.Fatalf("err: %v", err) } @@ -572,6 +572,20 @@ func testAgent_AddService(t *testing.T, extraHCL string) { } } +// addServiceFromSource is a test helper that exists to maintain an old function +// signature that was used in many tests. +// Deprecated: use AddService +func (a *Agent) addServiceFromSource(service *structs.NodeService, chkTypes []*structs.CheckType, persist bool, token string, source configSource) error { + return a.AddService(AddServiceRequest{ + Service: service, + chkTypes: chkTypes, + persist: persist, + token: token, + replaceExistingChecks: false, + Source: source, + }) +} + func TestAgent_AddServices_AliasUpdateCheckNotReverted(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") @@ -638,7 +652,7 @@ func testAgent_AddServices_AliasUpdateCheckNotReverted(t *testing.T, extraHCL st chkTypes, err := service.CheckTypes() require.NoError(t, err) - require.NoError(t, a.AddServiceFromSource(ns, chkTypes, false, service.Token, ConfigSourceLocal)) + require.NoError(t, a.addServiceFromSource(ns, chkTypes, false, service.Token, ConfigSourceLocal)) } retry.Run(t, func(r *retry.R) { @@ -665,7 +679,7 @@ func test_createAlias(t *testing.T, agent *TestAgent, chk *structs.CheckType, ex if chk.CheckID == "" { chk.CheckID = types.CheckID(fmt.Sprintf("check-%d", serviceNum)) } - err := agent.AddServiceFromSource(srv, []*structs.CheckType{chk}, false, "", ConfigSourceLocal) + err := agent.addServiceFromSource(srv, []*structs.CheckType{chk}, false, "", ConfigSourceLocal) assert.NoError(t, err) return func(r *retry.R) { t.Helper() @@ -712,7 +726,7 @@ func TestAgent_CheckAliasRPC(t *testing.T) { // We ensure to not block and update Agent's index srv.Tags = []string{fmt.Sprintf("tag-%s", time.Now())} assert.NoError(t, a.waitForUp()) - err := a.AddServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceLocal) + err := a.addServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceLocal) assert.NoError(t, err) } shutdownAgent := func() { @@ -727,7 +741,7 @@ func TestAgent_CheckAliasRPC(t *testing.T) { testrpc.WaitForTestAgent(t, a.RPC, "dc1") assert.NoError(t, a.waitForUp()) - err := a.AddServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceLocal) + err := a.addServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceLocal) assert.NoError(t, err) retry.Run(t, func(r *retry.R) { @@ -832,12 +846,12 @@ func testAgent_AddServiceNoExec(t *testing.T, extraHCL string) { Interval: 15 * time.Second, } - err := a.AddServiceFromSource(srv, []*structs.CheckType{chk}, false, "", ConfigSourceLocal) + err := a.addServiceFromSource(srv, []*structs.CheckType{chk}, false, "", ConfigSourceLocal) if err == nil || !strings.Contains(err.Error(), "Scripts are disabled on this agent") { t.Fatalf("err: %v", err) } - err = a.AddServiceFromSource(srv, []*structs.CheckType{chk}, false, "", ConfigSourceRemote) + err = a.addServiceFromSource(srv, []*structs.CheckType{chk}, false, "", ConfigSourceRemote) if err == nil || !strings.Contains(err.Error(), "Scripts are disabled on this agent") { t.Fatalf("err: %v", err) } @@ -879,7 +893,7 @@ func testAgent_AddServiceNoRemoteExec(t *testing.T, extraHCL string) { Interval: 15 * time.Second, } - err := a.AddServiceFromSource(srv, []*structs.CheckType{chk}, false, "", ConfigSourceRemote) + err := a.addServiceFromSource(srv, []*structs.CheckType{chk}, false, "", ConfigSourceRemote) if err == nil || !strings.Contains(err.Error(), "Scripts are disabled on this agent") { t.Fatalf("err: %v", err) } @@ -932,7 +946,7 @@ func TestCacheRateLimit(t *testing.T) { Address: fmt.Sprintf("10.0.1.%d", i%255), } - err := a.AddServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceRemote) + err := a.addServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceRemote) require.Nil(t, err) } @@ -1007,7 +1021,7 @@ func TestAddServiceIPv4TaggedDefault(t *testing.T) { Address: "10.0.1.2", } - err := a.AddServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceRemote) + err := a.addServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceRemote) require.Nil(t, err) ns := a.State.Service(structs.NewServiceID("my_service_id", nil)) @@ -1040,7 +1054,7 @@ func TestAddServiceIPv6TaggedDefault(t *testing.T) { Address: "::5", } - err := a.AddServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceRemote) + err := a.addServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceRemote) require.Nil(t, err) ns := a.State.Service(structs.NewServiceID("my_service_id", nil)) @@ -1079,7 +1093,7 @@ func TestAddServiceIPv4TaggedSet(t *testing.T) { }, } - err := a.AddServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceRemote) + err := a.addServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceRemote) require.Nil(t, err) ns := a.State.Service(structs.NewServiceID("my_service_id", nil)) @@ -1118,7 +1132,7 @@ func TestAddServiceIPv6TaggedSet(t *testing.T) { }, } - err := a.AddServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceRemote) + err := a.addServiceFromSource(srv, []*structs.CheckType{}, false, "", ConfigSourceRemote) require.Nil(t, err) ns := a.State.Service(structs.NewServiceID("my_service_id", nil)) @@ -1173,7 +1187,7 @@ func testAgent_RemoveService(t *testing.T, extraHCL string) { } chkTypes := []*structs.CheckType{{TTL: time.Minute}} - if err := a.AddServiceFromSource(srv, chkTypes, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(srv, chkTypes, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -1208,7 +1222,7 @@ func testAgent_RemoveService(t *testing.T, extraHCL string) { {TTL: time.Minute}, {TTL: 30 * time.Second}, } - if err := a.AddServiceFromSource(srv, chkTypes, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(srv, chkTypes, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -1222,7 +1236,7 @@ func testAgent_RemoveService(t *testing.T, extraHCL string) { {TTL: time.Minute}, {TTL: 30 * time.Second}, } - if err := a.AddServiceFromSource(srv, chkTypes, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(srv, chkTypes, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -1294,7 +1308,7 @@ func testAgent_RemoveServiceRemovesAllChecks(t *testing.T, extraHCL string) { } // register service with chk1 - if err := a.AddServiceFromSource(svc, []*structs.CheckType{chk1}, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, []*structs.CheckType{chk1}, false, "", ConfigSourceLocal); err != nil { t.Fatal("Failed to register service", err) } @@ -1302,7 +1316,7 @@ func testAgent_RemoveServiceRemovesAllChecks(t *testing.T, extraHCL string) { requireCheckExists(t, a, "chk1") // update the service with chk2 - if err := a.AddServiceFromSource(svc, []*structs.CheckType{chk2}, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, []*structs.CheckType{chk2}, false, "", ConfigSourceLocal); err != nil { t.Fatal("Failed to update service", err) } @@ -1359,7 +1373,7 @@ func verifyIndexChurn(t *testing.T, tags []string) { Tags: tags, Weights: weights, } - if err := a.AddServiceFromSource(svc, nil, true, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, true, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -1767,7 +1781,7 @@ func TestAgent_RestoreServiceWithAliasCheck(t *testing.T) { registerServicesAndChecks := func(t *testing.T, a *TestAgent) { // add one persistent service with a simple check - require.NoError(t, a.AddServiceFromSource( + require.NoError(t, a.addServiceFromSource( &structs.NodeService{ ID: "ping", Service: "ping", @@ -1786,7 +1800,7 @@ func TestAgent_RestoreServiceWithAliasCheck(t *testing.T) { // add one persistent sidecar service with an alias check in the manner // of how sidecar_service would add it - require.NoError(t, a.AddServiceFromSource( + require.NoError(t, a.addServiceFromSource( &structs.NodeService{ ID: "ping-sidecar-proxy", Service: "ping-sidecar-proxy", @@ -2276,7 +2290,7 @@ func testAgent_PersistService(t *testing.T, extraHCL string) { file := filepath.Join(a.Config.DataDir, servicesDir, stringHash(svc.ID)) // Check is not persisted unless requested - if err := a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } if _, err := os.Stat(file); err == nil { @@ -2284,7 +2298,7 @@ func testAgent_PersistService(t *testing.T, extraHCL string) { } // Persists to file if requested - if err := a.AddServiceFromSource(svc, nil, true, "mytoken", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, true, "mytoken", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } if _, err := os.Stat(file); err != nil { @@ -2308,7 +2322,7 @@ func testAgent_PersistService(t *testing.T, extraHCL string) { // Updates service definition on disk svc.Port = 8001 - if err := a.AddServiceFromSource(svc, nil, true, "mytoken", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, true, "mytoken", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } expected, err = json.Marshal(persistedService{ @@ -2431,7 +2445,7 @@ func testAgent_PurgeService(t *testing.T, extraHCL string) { } file := filepath.Join(a.Config.DataDir, servicesDir, stringHash(svc.ID)) - if err := a.AddServiceFromSource(svc, nil, true, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, true, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } // Exists @@ -2448,7 +2462,7 @@ func testAgent_PurgeService(t *testing.T, extraHCL string) { } // Re-add the service - if err := a.AddServiceFromSource(svc, nil, true, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, true, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -2494,7 +2508,7 @@ func testAgent_PurgeServiceOnDuplicate(t *testing.T, extraHCL string) { } // First persist the service - require.NoError(t, a.AddServiceFromSource(svc1, nil, true, "", ConfigSourceLocal)) + require.NoError(t, a.addServiceFromSource(svc1, nil, true, "", ConfigSourceLocal)) a.Shutdown() // Try bringing the agent back up with the service already @@ -2742,9 +2756,9 @@ func TestAgent_DeregisterPersistedSidecarAfterRestart(t *testing.T) { require.NoError(t, err) // First persist the check - err = a.AddServiceFromSource(srv, nil, true, "", ConfigSourceLocal) + err = a.addServiceFromSource(srv, nil, true, "", ConfigSourceLocal) require.NoError(t, err) - err = a.AddServiceFromSource(connectSrv, nil, true, "", ConfigSourceLocal) + err = a.addServiceFromSource(connectSrv, nil, true, "", ConfigSourceLocal) require.NoError(t, err) // check both services were registered @@ -2814,7 +2828,7 @@ func TestAgent_unloadChecks(t *testing.T) { Tags: []string{"foo"}, Port: 8000, } - if err := a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -3093,7 +3107,7 @@ func testAgent_unloadServices(t *testing.T, extraHCL string) { } // Register the service - if err := a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -3125,7 +3139,7 @@ func TestAgent_Service_MaintenanceMode(t *testing.T) { } // Register the service - if err := a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -3206,7 +3220,7 @@ func TestAgent_Service_Reap(t *testing.T) { } // Register the service. - if err := a.AddServiceFromSource(svc, chkTypes, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, chkTypes, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -3263,7 +3277,7 @@ func TestAgent_Service_NoReap(t *testing.T) { } // Register the service. - if err := a.AddServiceFromSource(svc, chkTypes, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, chkTypes, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -3310,7 +3324,7 @@ func testAgent_AddService_restoresSnapshot(t *testing.T, extraHCL string) { Tags: []string{"foo"}, Port: 8000, } - require.NoError(t, a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) + require.NoError(t, a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) // Register a check check1 := &structs.HealthCheck{ @@ -3325,7 +3339,7 @@ func testAgent_AddService_restoresSnapshot(t *testing.T, extraHCL string) { // Re-registering the service preserves the state of the check chkTypes := []*structs.CheckType{{TTL: 30 * time.Second}} - require.NoError(t, a.AddServiceFromSource(svc, chkTypes, false, "", ConfigSourceLocal)) + require.NoError(t, a.addServiceFromSource(svc, chkTypes, false, "", ConfigSourceLocal)) check := requireCheckExists(t, a, "service:redis") require.Equal(t, api.HealthPassing, check.Status) } @@ -3346,7 +3360,7 @@ func TestAgent_AddCheck_restoresSnapshot(t *testing.T) { Tags: []string{"foo"}, Port: 8000, } - if err := a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -3431,7 +3445,7 @@ func TestAgent_checkStateSnapshot(t *testing.T) { Tags: []string{"foo"}, Port: 8000, } - if err := a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("err: %v", err) } @@ -4250,7 +4264,7 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) { TLSSkipVerify: true, }, } - if err := a.AddServiceFromSource(svc, chks, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, chks, false, "", ConfigSourceLocal); err != nil { t.Fatalf("failed to add svc: %v", err) } @@ -4273,7 +4287,7 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) { }, }, } - if err := a.AddServiceFromSource(proxy, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(proxy, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("failed to add svc: %v", err) } @@ -4326,7 +4340,7 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) { }, }, } - if err := a.AddServiceFromSource(proxy, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(proxy, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("failed to add svc: %v", err) } @@ -4369,7 +4383,7 @@ func TestAgent_RerouteNewHTTPChecks(t *testing.T) { Address: "localhost", Port: 8080, } - if err := a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("failed to add svc: %v", err) } @@ -4391,7 +4405,7 @@ func TestAgent_RerouteNewHTTPChecks(t *testing.T) { }, }, } - if err := a.AddServiceFromSource(proxy, nil, false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(proxy, nil, false, "", ConfigSourceLocal); err != nil { t.Fatalf("failed to add svc: %v", err) } diff --git a/agent/service_checks_test.go b/agent/service_checks_test.go index 5b82aca28..19c6c60be 100644 --- a/agent/service_checks_test.go +++ b/agent/service_checks_test.go @@ -63,7 +63,7 @@ func TestAgent_ServiceHTTPChecksNotification(t *testing.T) { }, } // Adding TTL type should lead to a timeout, since only HTTP-based checks are watched - if err := a.AddServiceFromSource(&service, chkTypes[2:], false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(&service, chkTypes[2:], false, "", ConfigSourceLocal); err != nil { t.Fatalf("failed to add service: %v", err) } @@ -75,7 +75,7 @@ func TestAgent_ServiceHTTPChecksNotification(t *testing.T) { } // Adding service with HTTP checks should lead notification for them - if err := a.AddServiceFromSource(&service, chkTypes[0:2], false, "", ConfigSourceLocal); err != nil { + if err := a.addServiceFromSource(&service, chkTypes[0:2], false, "", ConfigSourceLocal); err != nil { t.Fatalf("failed to add service: %v", err) } diff --git a/agent/service_manager.go b/agent/service_manager.go index d6874adee..725b93df1 100644 --- a/agent/service_manager.go +++ b/agent/service_manager.go @@ -4,12 +4,13 @@ import ( "fmt" "sync" - "github.com/hashicorp/consul/agent/cache" - cachetype "github.com/hashicorp/consul/agent/cache-types" - "github.com/hashicorp/consul/agent/structs" "github.com/imdario/mergo" "github.com/mitchellh/copystructure" "golang.org/x/net/context" + + "github.com/hashicorp/consul/agent/cache" + cachetype "github.com/hashicorp/consul/agent/cache-types" + "github.com/hashicorp/consul/agent/structs" ) // The ServiceManager is a layer for service registration in between the agent @@ -83,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 *AddServiceRequest) error { s.agent.stateLock.Lock() defer s.agent.stateLock.Unlock() @@ -120,14 +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 { +func (s *ServiceManager) AddService(req *AddServiceRequest) error { req.fixupForAddServiceLocked() - req.service.EnterpriseMeta.Normalize() + 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() { + 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 @@ -136,7 +137,7 @@ func (s *ServiceManager) AddService(req *addServiceRequest) error { } var ( - service = req.service + service = req.Service chkTypes = req.chkTypes previousDefaults = req.previousDefaults waitForCentralConfig = req.waitForCentralConfig @@ -144,7 +145,7 @@ func (s *ServiceManager) AddService(req *addServiceRequest) error { persistServiceConfig = req.persistServiceConfig token = req.token replaceExistingChecks = req.replaceExistingChecks - source = req.source + source = req.Source ) reg := &serviceRegistration{ @@ -267,8 +268,8 @@ 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, + err = w.agent.addServiceInternal(&AddServiceRequest{ + Service: merged, chkTypes: w.registration.chkTypes, persistService: w.registration.service, persistDefaults: serviceDefaults, @@ -276,7 +277,7 @@ func (w *serviceConfigWatch) RegisterAndStart( persistServiceConfig: persistServiceConfig, token: w.registration.token, replaceExistingChecks: w.registration.replaceExistingChecks, - source: w.registration.source, + Source: w.registration.source, snap: w.agent.snapshotCheckState(), }) if err != nil { @@ -408,8 +409,8 @@ func (w *serviceConfigWatch) handleUpdate(ctx context.Context, event cache.Updat } registerReq := &asyncRegisterRequest{ - Args: &addServiceRequest{ - service: merged, + Args: &AddServiceRequest{ + Service: merged, chkTypes: w.registration.chkTypes, persistService: w.registration.service, persistDefaults: serviceDefaults, @@ -417,7 +418,7 @@ func (w *serviceConfigWatch) handleUpdate(ctx context.Context, event cache.Updat persistServiceConfig: true, token: w.registration.token, replaceExistingChecks: w.registration.replaceExistingChecks, - source: w.registration.source, + Source: w.registration.source, }, Reply: make(chan error, 1), } @@ -441,7 +442,7 @@ func (w *serviceConfigWatch) handleUpdate(ctx context.Context, event cache.Updat } type asyncRegisterRequest struct { - Args *addServiceRequest + Args *AddServiceRequest Reply chan error } diff --git a/agent/service_manager_test.go b/agent/service_manager_test.go index 073ac577d..ea1d6c4f1 100644 --- a/agent/service_manager_test.go +++ b/agent/service_manager_test.go @@ -48,7 +48,7 @@ func TestServiceManager_RegisterService(t *testing.T) { Port: 8000, EnterpriseMeta: *structs.DefaultEnterpriseMeta(), } - require.NoError(a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) + require.NoError(a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) // Verify both the service and sidecar. redisService := a.State.Service(structs.NewServiceID("redis", nil)) @@ -119,7 +119,7 @@ func TestServiceManager_RegisterSidecar(t *testing.T) { }, EnterpriseMeta: *structs.DefaultEnterpriseMeta(), } - require.NoError(a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) + require.NoError(a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) // Verify sidecar got global config loaded sidecarService := a.State.Service(structs.NewServiceID("web-sidecar-proxy", nil)) @@ -192,7 +192,7 @@ func TestServiceManager_RegisterMeshGateway(t *testing.T) { EnterpriseMeta: *structs.DefaultEnterpriseMeta(), } - require.NoError(a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) + require.NoError(a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) // Verify gateway got global config loaded gateway := a.State.Service(structs.NewServiceID("mesh-gateway", nil)) @@ -252,7 +252,7 @@ func TestServiceManager_RegisterTerminatingGateway(t *testing.T) { EnterpriseMeta: *structs.DefaultEnterpriseMeta(), } - require.NoError(a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) + require.NoError(a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) // Verify gateway got global config loaded gateway := a.State.Service(structs.NewServiceID("terminating-gateway", nil)) @@ -387,12 +387,12 @@ 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)) + require.NoError(a.addServiceFromSource(svc, nil, false, "", ConfigSourceRemote)) requireFileIsAbsent(t, svcFile) requireFileIsPresent(t, configFile) // Persists to file if requested - require.NoError(a.AddServiceFromSource(svc, nil, true, "mytoken", ConfigSourceRemote)) + require.NoError(a.addServiceFromSource(svc, nil, true, "mytoken", ConfigSourceRemote)) requireFileIsPresent(t, svcFile) requireFileIsPresent(t, configFile) @@ -433,7 +433,7 @@ func TestServiceManager_PersistService_API(t *testing.T) { // Updates service definition on disk svc.Proxy.LocalServicePort = 8001 - require.NoError(a.AddServiceFromSource(svc, nil, true, "mytoken", ConfigSourceRemote)) + require.NoError(a.addServiceFromSource(svc, nil, true, "mytoken", ConfigSourceRemote)) requireFileIsPresent(t, svcFile) requireFileIsPresent(t, configFile) @@ -721,7 +721,7 @@ func TestServiceManager_Disabled(t *testing.T) { }, EnterpriseMeta: *structs.DefaultEnterpriseMeta(), } - require.NoError(a.AddServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) + require.NoError(a.addServiceFromSource(svc, nil, false, "", ConfigSourceLocal)) // Verify sidecar got global config loaded sidecarService := a.State.Service(structs.NewServiceID("web-sidecar-proxy", nil)) diff --git a/agent/sidecar_service.go b/agent/sidecar_service.go index bd78adb15..ac3e02c66 100644 --- a/agent/sidecar_service.go +++ b/agent/sidecar_service.go @@ -29,7 +29,7 @@ func sidecarServiceID(serviceID string) string { // The third return argument is the effective Token to use for the sidecar // registration. This will be the same as the token parameter passed unless the // SidecarService definition contains a distinct one. -// TODO: return addServiceRequest +// TODO: return AddServiceRequest func (a *Agent) sidecarServiceFromNodeService(ns *structs.NodeService, token string) (*structs.NodeService, []*structs.CheckType, string, error) { if ns.Connect.SidecarService == nil { return nil, nil, "", nil diff --git a/agent/sidecar_service_test.go b/agent/sidecar_service_test.go index d287c169a..0840d8044 100644 --- a/agent/sidecar_service_test.go +++ b/agent/sidecar_service_test.go @@ -334,7 +334,7 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) { defer a.Shutdown() if tt.preRegister != nil { - err := a.AddServiceFromSource(tt.preRegister.NodeService(), nil, false, "", ConfigSourceLocal) + err := a.addServiceFromSource(tt.preRegister.NodeService(), nil, false, "", ConfigSourceLocal) require.NoError(err) } diff --git a/command/maint/maint_test.go b/command/maint/maint_test.go index 78e481339..18ee9eb40 100644 --- a/command/maint/maint_test.go +++ b/command/maint/maint_test.go @@ -50,11 +50,14 @@ func TestMaintCommand_NoArgs(t *testing.T) { defer a.Shutdown() // Register the service and put it into maintenance mode - service := &structs.NodeService{ - ID: "test", - Service: "test", + addReq := agent.AddServiceRequest{ + Service: &structs.NodeService{ + ID: "test", + Service: "test", + }, + Source: agent.ConfigSourceLocal, } - if err := a.AddServiceFromSource(service, nil, false, "", agent.ConfigSourceLocal); err != nil { + if err := a.AddService(addReq); err != nil { t.Fatalf("err: %v", err) } if err := a.EnableServiceMaintenance(structs.NewServiceID("test", nil), "broken 1", ""); err != nil { @@ -158,11 +161,14 @@ func TestMaintCommand_EnableServiceMaintenance(t *testing.T) { defer a.Shutdown() // Register the service - service := &structs.NodeService{ - ID: "test", - Service: "test", + addReq := agent.AddServiceRequest{ + Service: &structs.NodeService{ + ID: "test", + Service: "test", + }, + Source: agent.ConfigSourceLocal, } - if err := a.AddServiceFromSource(service, nil, false, "", agent.ConfigSourceLocal); err != nil { + if err := a.AddService(addReq); err != nil { t.Fatalf("err: %v", err) } @@ -196,11 +202,14 @@ func TestMaintCommand_DisableServiceMaintenance(t *testing.T) { defer a.Shutdown() // Register the service - service := &structs.NodeService{ - ID: "test", - Service: "test", + addReq := agent.AddServiceRequest{ + Service: &structs.NodeService{ + ID: "test", + Service: "test", + }, + Source: agent.ConfigSourceLocal, } - if err := a.AddServiceFromSource(service, nil, false, "", agent.ConfigSourceLocal); err != nil { + if err := a.AddService(addReq); err != nil { t.Fatalf("err: %v", err) }