chore: removed unused method AddService (#13905)

- This AddService is not used anywhere.
  AddServiceWithChecks is place of AddService
- Test code is updated

agent/acl_test.go

@@ -274,10 +274,10 @@ func TestACL_vetServiceRegister(t *testing.T) {
 
 	// Try to register over a service without write privs to the existing
 	// service.
-	a.State.AddService(&structs.NodeService{
+	a.State.AddServiceWithChecks(&structs.NodeService{
 		ID:      "my-service",
 		Service: "other",
-	}, "")
+	}, nil, "")
 	err = a.vetServiceRegister(serviceRWSecret, &structs.NodeService{
 		ID:      "my-service",
 		Service: "service",
@@ -304,10 +304,10 @@ func TestACL_vetServiceUpdateWithAuthorizer(t *testing.T) {
 	require.Contains(t, err.Error(), "Unknown service")
 
 	// Update with write privs.
-	a.State.AddService(&structs.NodeService{
+	a.State.AddServiceWithChecks(&structs.NodeService{
 		ID:      "my-service",
 		Service: "service",
-	}, "")
+	}, nil, "")
 	err = vetServiceUpdate(serviceRWSecret, structs.NewServiceID("my-service", nil))
 	require.NoError(t, err)
 
@@ -361,10 +361,10 @@ func TestACL_vetCheckRegisterWithAuthorizer(t *testing.T) {
 
 	// Try to register over a service check without write privs to the
 	// existing service.
-	a.State.AddService(&structs.NodeService{
+	a.State.AddServiceWithChecks(&structs.NodeService{
 		ID:      "my-service",
 		Service: "service",
-	}, "")
+	}, nil, "")
 	a.State.AddCheck(&structs.HealthCheck{
 		CheckID:     types.CheckID("my-check"),
 		ServiceID:   "my-service",
@@ -410,10 +410,10 @@ func TestACL_vetCheckUpdateWithAuthorizer(t *testing.T) {
 	require.Contains(t, err.Error(), "Unknown check")
 
 	// Update service check with write privs.
-	a.State.AddService(&structs.NodeService{
+	a.State.AddServiceWithChecks(&structs.NodeService{
 		ID:      "my-service",
 		Service: "service",
-	}, "")
+	}, nil, "")
 	a.State.AddCheck(&structs.HealthCheck{
 		CheckID:     types.CheckID("my-service-check"),
 		ServiceID:   "my-service",

agent/agent_endpoint_test.go

@@ -93,7 +93,7 @@ func TestAgent_Services(t *testing.T) {
 		},
 		Port: 5000,
 	}
-	require.NoError(t, a.State.AddService(srv1, ""))
+	require.NoError(t, a.State.AddServiceWithChecks(srv1, nil, ""))
 
 	req, _ := http.NewRequest("GET", "/v1/agent/services", nil)
 	resp := httptest.NewRecorder()
@@ -128,7 +128,7 @@ func TestAgent_ServicesFiltered(t *testing.T) {
 		},
 		Port: 5000,
 	}
-	require.NoError(t, a.State.AddService(srv1, ""))
+	require.NoError(t, a.State.AddServiceWithChecks(srv1, nil, ""))
 
 	// Add another service
 	srv2 := &structs.NodeService{
@@ -140,7 +140,7 @@ func TestAgent_ServicesFiltered(t *testing.T) {
 		},
 		Port: 1234,
 	}
-	require.NoError(t, a.State.AddService(srv2, ""))
+	require.NoError(t, a.State.AddServiceWithChecks(srv2, nil, ""))
 
 	req, _ := http.NewRequest("GET", "/v1/agent/services?filter="+url.QueryEscape("foo in Meta"), nil)
 	resp := httptest.NewRecorder()
@@ -188,7 +188,7 @@ func TestAgent_Services_ExternalConnectProxy(t *testing.T) {
 			Upstreams:              structs.TestUpstreams(t),
 		},
 	}
-	a.State.AddService(srv1, "")
+	a.State.AddServiceWithChecks(srv1, nil, "")
 
 	req, _ := http.NewRequest("GET", "/v1/agent/services", nil)
 	resp := httptest.NewRecorder()
@@ -232,7 +232,7 @@ func TestAgent_Services_Sidecar(t *testing.T) {
 			},
 		},
 	}
-	a.State.AddService(srv1, "")
+	a.State.AddServiceWithChecks(srv1, nil, "")
 
 	req, _ := http.NewRequest("GET", "/v1/agent/services", nil)
 	resp := httptest.NewRecorder()
@@ -281,7 +281,7 @@ func TestAgent_Services_MeshGateway(t *testing.T) {
 			},
 		},
 	}
-	a.State.AddService(srv1, "")
+	a.State.AddServiceWithChecks(srv1, nil, "")
 
 	req, _ := http.NewRequest("GET", "/v1/agent/services", nil)
 	resp := httptest.NewRecorder()
@@ -325,7 +325,7 @@ func TestAgent_Services_TerminatingGateway(t *testing.T) {
 			},
 		},
 	}
-	require.NoError(t, a.State.AddService(srv1, ""))
+	require.NoError(t, a.State.AddServiceWithChecks(srv1, nil, ""))
 
 	req, _ := http.NewRequest("GET", "/v1/agent/services", nil)
 	resp := httptest.NewRecorder()
@@ -370,7 +370,7 @@ func TestAgent_Services_ACLFilter(t *testing.T) {
 		},
 	}
 	for _, s := range services {
-		a.State.AddService(s, "")
+		a.State.AddServiceWithChecks(s, nil, "")
 	}
 
 	t.Run("no token", func(t *testing.T) {
@@ -7994,7 +7994,7 @@ func TestAgent_Services_ExposeConfig(t *testing.T) {
 			},
 		},
 	}
-	a.State.AddService(srv1, "")
+	a.State.AddServiceWithChecks(srv1, nil, "")
 
 	req, _ := http.NewRequest("GET", "/v1/agent/services", nil)
 	resp := httptest.NewRecorder()

agent/local/state.go

@@ -256,15 +256,6 @@ func (l *State) aclTokenForServiceSync(id structs.ServiceID, fallback func() str
 	return fallback()
 }
 
-// AddService is used to add a service entry to the local state.
-// This entry is persistent and the agent will make a best effort to
-// ensure it is registered
-func (l *State) AddService(service *structs.NodeService, token string) error {
-	l.Lock()
-	defer l.Unlock()
-	return l.addServiceLocked(service, token)
-}
-
 func (l *State) addServiceLocked(service *structs.NodeService, token string) error {
 	if service == nil {
 		return fmt.Errorf("no service")
@@ -293,7 +284,9 @@ func (l *State) addServiceLocked(service *structs.NodeService, token string) err
 	return nil
 }
 
-// AddServiceWithChecks adds a service and its check tp the local state atomically
+// AddServiceWithChecks adds a service entry and its checks to the local state atomically
+// This entry is persistent and the agent will make a best effort to
+// ensure it is registered
 func (l *State) AddServiceWithChecks(service *structs.NodeService, checks []*structs.HealthCheck, token string) error {
 	l.Lock()
 	defer l.Unlock()

agent/local/state_test.go

@@ -64,7 +64,7 @@ func TestAgentAntiEntropy_Services(t *testing.T) {
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
 	assert.False(t, a.State.ServiceExists(structs.ServiceID{ID: srv1.ID}))
-	a.State.AddService(srv1, "")
+	a.State.AddServiceWithChecks(srv1, nil, "")
 	assert.True(t, a.State.ServiceExists(structs.ServiceID{ID: srv1.ID}))
 	args.Service = srv1
 	if err := a.RPC("Catalog.Register", args, &out); err != nil {
@@ -83,7 +83,7 @@ func TestAgentAntiEntropy_Services(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv2, "")
+	a.State.AddServiceWithChecks(srv2, nil, "")
 
 	srv2_mod := new(structs.NodeService)
 	*srv2_mod = *srv2
@@ -105,7 +105,7 @@ func TestAgentAntiEntropy_Services(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv3, "")
+	a.State.AddServiceWithChecks(srv3, nil, "")
 
 	// Exists remote (delete)
 	srv4 := &structs.NodeService{
@@ -137,7 +137,7 @@ func TestAgentAntiEntropy_Services(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv5, "")
+	a.State.AddServiceWithChecks(srv5, nil, "")
 
 	srv5_mod := new(structs.NodeService)
 	*srv5_mod = *srv5
@@ -290,7 +290,7 @@ func TestAgentAntiEntropy_Services_ConnectProxy(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv1, "")
+	a.State.AddServiceWithChecks(srv1, nil, "")
 	require.NoError(t, a.RPC("Catalog.Register", &structs.RegisterRequest{
 		Datacenter: "dc1",
 		Node:       a.Config.NodeName,
@@ -311,7 +311,7 @@ func TestAgentAntiEntropy_Services_ConnectProxy(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv2, "")
+	a.State.AddServiceWithChecks(srv2, nil, "")
 
 	srv2_mod := clone(srv2)
 	srv2_mod.Port = 9000
@@ -335,7 +335,7 @@ func TestAgentAntiEntropy_Services_ConnectProxy(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv3, "")
+	a.State.AddServiceWithChecks(srv3, nil, "")
 
 	// Exists remote (delete)
 	srv4 := &structs.NodeService{
@@ -496,7 +496,7 @@ func TestAgent_ServiceWatchCh(t *testing.T) {
 		Tags:    []string{"tag1"},
 		Port:    6100,
 	}
-	require.NoError(t, a.State.AddService(srv1, ""))
+	require.NoError(t, a.State.AddServiceWithChecks(srv1, nil, ""))
 
 	verifyState := func(ss *local.ServiceState) {
 		require.NotNil(t, ss)
@@ -518,7 +518,7 @@ func TestAgent_ServiceWatchCh(t *testing.T) {
 	go func() {
 		srv2 := srv1
 		srv2.Port = 6200
-		require.NoError(t, a.State.AddService(srv2, ""))
+		require.NoError(t, a.State.AddServiceWithChecks(srv2, nil, ""))
 	}()
 
 	// We should observe WatchCh close
@@ -595,7 +595,7 @@ func TestAgentAntiEntropy_EnableTagOverride(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv1, "")
+	a.State.AddServiceWithChecks(srv1, nil, "")
 
 	// register a local service with tag override disabled
 	srv2 := &structs.NodeService{
@@ -610,7 +610,7 @@ func TestAgentAntiEntropy_EnableTagOverride(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv2, "")
+	a.State.AddServiceWithChecks(srv2, nil, "")
 
 	// make sure they are both in the catalog
 	if err := a.State.SyncChanges(); err != nil {
@@ -722,7 +722,7 @@ func TestAgentAntiEntropy_Services_WithChecks(t *testing.T) {
 			Tags:    []string{"primary"},
 			Port:    5000,
 		}
-		a.State.AddService(srv, "")
+		a.State.AddServiceWithChecks(srv, nil, "")
 
 		chk := &structs.HealthCheck{
 			Node:      a.Config.NodeName,
@@ -772,7 +772,7 @@ func TestAgentAntiEntropy_Services_WithChecks(t *testing.T) {
 			Tags:    []string{"primary"},
 			Port:    5000,
 		}
-		a.State.AddService(srv, "")
+		a.State.AddServiceWithChecks(srv, nil, "")
 
 		chk1 := &structs.HealthCheck{
 			Node:      a.Config.NodeName,
@@ -873,7 +873,7 @@ func TestAgentAntiEntropy_Services_ACLDeny(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv1, token)
+	a.State.AddServiceWithChecks(srv1, nil, token)
 
 	// Create service (allowed)
 	srv2 := &structs.NodeService{
@@ -887,7 +887,7 @@ func TestAgentAntiEntropy_Services_ACLDeny(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv2, token)
+	a.State.AddServiceWithChecks(srv2, nil, token)
 
 	if err := a.State.SyncFull(); err != nil {
 		t.Fatalf("err: %v", err)
@@ -1332,7 +1332,7 @@ func TestAgentAntiEntropy_Checks_ACLDeny(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv1, "root")
+	a.State.AddServiceWithChecks(srv1, nil, "root")
 	srv2 := &structs.NodeService{
 		ID:      "api",
 		Service: "api",
@@ -1344,7 +1344,7 @@ func TestAgentAntiEntropy_Checks_ACLDeny(t *testing.T) {
 		},
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
 	}
-	a.State.AddService(srv2, "root")
+	a.State.AddServiceWithChecks(srv2, nil, "root")
 
 	if err := a.State.SyncFull(); err != nil {
 		t.Fatalf("err: %v", err)
@@ -1861,14 +1861,14 @@ func TestState_ServiceTokens(t *testing.T) {
 	})
 
 	t.Run("empty string when there is no token", func(t *testing.T) {
-		err := l.AddService(&structs.NodeService{ID: "redis"}, "")
+		err := l.AddServiceWithChecks(&structs.NodeService{ID: "redis"}, nil, "")
 		require.NoError(t, err)
 
 		require.Equal(t, "", l.ServiceToken(id))
 	})
 
 	t.Run("returns configured token", func(t *testing.T) {
-		err := l.AddService(&structs.NodeService{ID: "redis"}, "abc123")
+		err := l.AddServiceWithChecks(&structs.NodeService{ID: "redis"}, nil, "abc123")
 		require.NoError(t, err)
 
 		require.Equal(t, "abc123", l.ServiceToken(id))
@@ -1931,7 +1931,7 @@ func TestAgent_CheckCriticalTime(t *testing.T) {
 	l.TriggerSyncChanges = func() {}
 
 	svc := &structs.NodeService{ID: "redis", Service: "redis", Port: 8000}
-	l.AddService(svc, "")
+	l.AddServiceWithChecks(svc, nil, "")
 
 	// Add a passing check and make sure it's not critical.
 	checkID := types.CheckID("redis:1")
@@ -2017,8 +2017,8 @@ func TestAgent_AliasCheck(t *testing.T) {
 	l.TriggerSyncChanges = func() {}
 
 	// Add checks
-	require.NoError(t, l.AddService(&structs.NodeService{Service: "s1"}, ""))
-	require.NoError(t, l.AddService(&structs.NodeService{Service: "s2"}, ""))
+	require.NoError(t, l.AddServiceWithChecks(&structs.NodeService{Service: "s1"}, nil, ""))
+	require.NoError(t, l.AddServiceWithChecks(&structs.NodeService{Service: "s2"}, nil, ""))
 	require.NoError(t, l.AddCheck(&structs.HealthCheck{CheckID: types.CheckID("c1"), ServiceID: "s1"}, ""))
 	require.NoError(t, l.AddCheck(&structs.HealthCheck{CheckID: types.CheckID("c2"), ServiceID: "s2"}, ""))
 
@@ -2071,7 +2071,7 @@ func TestAgent_AliasCheck_ServiceNotification(t *testing.T) {
 	require.NoError(t, l.AddAliasCheck(structs.NewCheckID(types.CheckID("a1"), nil), structs.NewServiceID("s1", nil), notifyCh))
 
 	// Add aliased service, s1, and verify we get notified
-	require.NoError(t, l.AddService(&structs.NodeService{Service: "s1"}, ""))
+	require.NoError(t, l.AddServiceWithChecks(&structs.NodeService{Service: "s1"}, nil, ""))
 	select {
 	case <-notifyCh:
 	default:
@@ -2079,7 +2079,7 @@ func TestAgent_AliasCheck_ServiceNotification(t *testing.T) {
 	}
 
 	// Re-adding same service should not lead to a notification
-	require.NoError(t, l.AddService(&structs.NodeService{Service: "s1"}, ""))
+	require.NoError(t, l.AddServiceWithChecks(&structs.NodeService{Service: "s1"}, nil, ""))
 	select {
 	case <-notifyCh:
 		t.Fatal("notify received")
@@ -2087,7 +2087,7 @@ func TestAgent_AliasCheck_ServiceNotification(t *testing.T) {
 	}
 
 	// Add different service and verify we do not get notified
-	require.NoError(t, l.AddService(&structs.NodeService{Service: "s2"}, ""))
+	require.NoError(t, l.AddServiceWithChecks(&structs.NodeService{Service: "s2"}, nil, ""))
 	select {
 	case <-notifyCh:
 		t.Fatal("notify received")
@@ -2189,10 +2189,10 @@ func TestState_RemoveServiceErrorMessages(t *testing.T) {
 	state.TriggerSyncChanges = func() {}
 
 	// Add 1 service
-	err := state.AddService(&structs.NodeService{
+	err := state.AddServiceWithChecks(&structs.NodeService{
 		ID:      "web-id",
 		Service: "web-name",
-	}, "")
+	}, nil, "")
 	require.NoError(t, err)
 
 	// Attempt to remove service that doesn't exist
@@ -2230,9 +2230,9 @@ func TestState_Notify(t *testing.T) {
 	drainCh(notifyCh)
 
 	// Add a service
-	err := state.AddService(&structs.NodeService{
+	err := state.AddServiceWithChecks(&structs.NodeService{
 		Service: "web",
-	}, "fake-token-web")
+	}, nil, "fake-token-web")
 	require.NoError(t, err)
 
 	// Should have a notification
@@ -2240,10 +2240,10 @@ func TestState_Notify(t *testing.T) {
 	drainCh(notifyCh)
 
 	// Re-Add same service
-	err = state.AddService(&structs.NodeService{
+	err = state.AddServiceWithChecks(&structs.NodeService{
 		Service: "web",
 		Port:    4444,
-	}, "fake-token-web")
+	}, nil, "fake-token-web")
 	require.NoError(t, err)
 
 	// Should have a notification
@@ -2261,9 +2261,9 @@ func TestState_Notify(t *testing.T) {
 	state.StopNotify(notifyCh)
 
 	// Add a service
-	err = state.AddService(&structs.NodeService{
+	err = state.AddServiceWithChecks(&structs.NodeService{
 		Service: "web",
-	}, "fake-token-web")
+	}, nil, "fake-token-web")
 	require.NoError(t, err)
 
 	// Should NOT have a notification
@@ -2293,7 +2293,7 @@ func TestAliasNotifications_local(t *testing.T) {
 		Address: "127.0.0.10",
 		Port:    8080,
 	}
-	a.State.AddService(srv, "")
+	a.State.AddServiceWithChecks(srv, nil, "")
 
 	scID := "socat-sidecar-proxy"
 	sc := &structs.NodeService{
@@ -2303,7 +2303,7 @@ func TestAliasNotifications_local(t *testing.T) {
 		Address: "127.0.0.10",
 		Port:    9090,
 	}
-	a.State.AddService(sc, "")
+	a.State.AddServiceWithChecks(sc, nil, "")
 
 	tcpID := types.CheckID("service:socat-tcp")
 	chk0 := &structs.HealthCheck{

agent/proxycfg-sources/catalog/config_source_test.go

@@ -116,7 +116,7 @@ func TestConfigSource_LocallyManagedService(t *testing.T) {
 	token := "token"
 
 	localState := testLocalState(t)
-	localState.AddService(&structs.NodeService{ID: serviceID.ID}, "")
+	localState.AddServiceWithChecks(&structs.NodeService{ID: serviceID.ID}, nil, "")
 
 	localWatcher := NewMockWatcher(t)
 	localWatcher.On("Watch", serviceID, nodeName, token).

agent/proxycfg-sources/local/sync_test.go

@@ -29,10 +29,10 @@ func TestSync(t *testing.T) {
 	state := local.NewState(local.Config{}, hclog.NewNullLogger(), tokens)
 	state.TriggerSyncChanges = func() {}
 
-	state.AddService(&structs.NodeService{
+	state.AddServiceWithChecks(&structs.NodeService{
 		ID:   serviceID,
 		Kind: structs.ServiceKindConnectProxy,
-	}, serviceToken)
+	}, nil, serviceToken)
 
 	cfgMgr := NewMockConfigManager(t)
 
@@ -96,10 +96,10 @@ func TestSync(t *testing.T) {
 		Return([]proxycfg.ProxyID{}).
 		Maybe()
 
-	state.AddService(&structs.NodeService{
+	state.AddServiceWithChecks(&structs.NodeService{
 		ID:   serviceID,
 		Kind: structs.ServiceKindConnectProxy,
-	}, "")
+	}, nil, "")
 
 	select {
 	case reg := <-registerCh:

agent/user_event_test.go

@@ -64,7 +64,7 @@ func TestShouldProcessUserEvent(t *testing.T) {
 		Tags:    []string{"test", "foo", "bar", "primary"},
 		Port:    5000,
 	}
-	a.State.AddService(srv1, "")
+	a.State.AddServiceWithChecks(srv1, nil, "")
 
 	p := &UserEvent{}
 	if !a.shouldProcessUserEvent(p) {
@@ -172,7 +172,7 @@ func TestFireReceiveEvent(t *testing.T) {
 		Tags:    []string{"test", "foo", "bar", "primary"},
 		Port:    5000,
 	}
-	a.State.AddService(srv1, "")
+	a.State.AddServiceWithChecks(srv1, nil, "")
 
 	p1 := &UserEvent{Name: "deploy", ServiceFilter: "web"}
 	err := a.UserEvent("dc1", "root", p1)