luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

agent: move deprecated AddServiceFromSource to a test file 

The method is only used in tests, and only exists for legacy calls.

There was one other package which used this method in tests. Export
the AddServiceRequest and a couple of its fields so the new function can
be used in those tests.
This commit is contained in:
Daniel Nephin 2020-11-30 13:26:58 -05:00
parent de1a80b368
commit a0b11b4c20
10 changed files with 153 additions and 151 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
agent/agent.go
View File

@ -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 {

12
agent/agent_endpoint.go
View File

@ -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 {

36
agent/agent_endpoint_test.go
View File

@ -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)
}

100
agent/agent_test.go
View File

@ -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)
}

4
agent/service_checks_test.go
View File

@ -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)
}

33
agent/service_manager.go
View File

@ -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
}

16
agent/service_manager_test.go
View File

@ -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))

2
agent/sidecar_service.go
View File

@ -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

2
agent/sidecar_service_test.go
View File

@ -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)
}

33
command/maint/maint_test.go
View File

@ -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)
}