Skip to content

Commit 58191bf

Browse files
astaticvoidastaticvoid
authored
Replace ListInstances API calls with DiscoverInstances (#70)
1 parent 865f9d3 commit 58191bf

File tree

11 files changed

+114
-97
lines changed

11 files changed

+114
-97
lines changed

‎integration/janitor/janitor.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func (j *cloudMapJanitor) Cleanup(ctx context.Context, nsName string) {
6464

6565
for _, svc := range svcs {
6666
fmt.Printf("found service to clean: %s\n", svc.Id)
67-
j.deregisterInstances(ctx, svc.Id)
67+
j.deregisterInstances(ctx, nsName, svc.Name, svc.Id)
6868

6969
delSvcErr := j.sdApi.DeleteService(ctx, svc.Id)
7070
j.checkOrFail(delSvcErr, "service deleted", "could not cleanup service")
@@ -78,15 +78,15 @@ func (j *cloudMapJanitor) Cleanup(ctx context.Context, nsName string) {
7878
j.checkOrFail(err, "clean up successful", "could not cleanup namespace")
7979
}
8080

81-
func (j *cloudMapJanitor) deregisterInstances(ctx context.Context, svcId string) {
82-
insts, err := j.sdApi.ListInstances(ctx, svcId)
81+
func (j *cloudMapJanitor) deregisterInstances(ctx context.Context, nsName string, svcName string, svcId string) {
82+
insts, err := j.sdApi.DiscoverInstances(ctx, nsName, svcName)
8383
j.checkOrFail(err,
8484
fmt.Sprintf("service has %d instances to clean", len(insts)),
8585
"could not list instances to cleanup")
8686

8787
opColl := cloudmap.NewOperationCollector()
8888
for _, inst := range insts {
89-
instId := aws.ToString(inst.Id)
89+
instId := aws.ToString(inst.InstanceId)
9090
fmt.Printf("found instance to clean: %s\n", instId)
9191
opColl.Add(func() (opId string, err error) {
9292
return j.sdApi.DeregisterInstance(ctx, svcId, instId)

‎integration/janitor/janitor_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ func TestCleanupHappyCase(t *testing.T) {
3131
Return([]*model.Namespace{{Id: test.NsId, Name: test.NsName}}, nil)
3232
tj.mockApi.EXPECT().ListServices(context.TODO(), test.NsId).
3333
Return([]*model.Resource{{Id: test.SvcId, Name: test.SvcName}}, nil)
34-
tj.mockApi.EXPECT().ListInstances(context.TODO(), test.SvcId).
35-
Return([]types.InstanceSummary{{Id: aws.String(test.EndptId1)}}, nil)
34+
tj.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.NsName, test.SvcName).
35+
Return([]types.HttpInstanceSummary{{InstanceId: aws.String(test.EndptId1)}}, nil)
3636

3737
tj.mockApi.EXPECT().DeregisterInstance(context.TODO(), test.SvcId, test.EndptId1).
3838
Return(test.OpId1, nil)

‎pkg/cloudmap/api.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ type ServiceDiscoveryApi interface {
2626
// ListServices returns a list of services for a given namespace.
2727
ListServices(ctx context.Context, namespaceId string) (services []*model.Resource, err error)
2828

29-
// ListInstances returns a list of service instances registered to a given service.
30-
ListInstances(ctx context.Context, serviceId string) ([]types.InstanceSummary, error)
29+
// DiscoverInstances returns a list of service instances registered to a given service.
30+
DiscoverInstances(ctx context.Context, nsName string, svcName string) (insts []types.HttpInstanceSummary, err error)
3131

3232
// ListOperations returns a map of operations to their status matching a list of filters.
3333
ListOperations(ctx context.Context, opFilters []types.OperationFilter) (operationStatusMap map[string]types.OperationStatus, err error)
@@ -113,19 +113,19 @@ func (sdApi *serviceDiscoveryApi) ListServices(ctx context.Context, nsId string)
113113
return svcs, nil
114114
}
115115

116-
func (sdApi *serviceDiscoveryApi) ListInstances(ctx context.Context, svcId string) (insts []types.InstanceSummary, err error) {
117-
pages := sd.NewListInstancesPaginator(sdApi.awsFacade, &sd.ListInstancesInput{ServiceId: &svcId})
118-
119-
for pages.HasMorePages() {
120-
output, err := pages.NextPage(ctx)
121-
if err != nil {
122-
return insts, err
123-
}
116+
func (sdApi *serviceDiscoveryApi) DiscoverInstances(ctx context.Context, nsName string, svcName string) (insts []types.HttpInstanceSummary, err error) {
117+
out, err := sdApi.awsFacade.DiscoverInstances(ctx, &sd.DiscoverInstancesInput{
118+
NamespaceName: aws.String(nsName),
119+
ServiceName: aws.String(svcName),
120+
HealthStatus: types.HealthStatusFilterAll,
121+
MaxResults: aws.Int32(1000),
122+
})
124123

125-
insts = append(insts, output.Instances...)
124+
if err != nil {
125+
return insts, err
126126
}
127127

128-
return insts, nil
128+
return out.Instances, nil
129129
}
130130

131131
func (sdApi *serviceDiscoveryApi) ListOperations(ctx context.Context, opFilters []types.OperationFilter) (opStatusMap map[string]types.OperationStatus, err error) {

‎pkg/cloudmap/api_test.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,26 +85,32 @@ func TestServiceDiscoveryApi_ListServices_HappyCase(t *testing.T) {
8585
assert.Equal(t, svcs[0], &model.Resource{Id: test.SvcId, Name: test.SvcName})
8686
}
8787

88-
func TestServiceDiscoveryApi_ListInstances_HappyCase(t *testing.T) {
88+
func TestServiceDiscoveryApi_DiscoverInstances_HappyCase(t *testing.T) {
8989
mockController := gomock.NewController(t)
9090
defer mockController.Finish()
9191

9292
awsFacade := cloudmap.NewMockAwsFacade(mockController)
9393
sdApi := getServiceDiscoveryApi(t, awsFacade)
9494

95-
awsFacade.EXPECT().ListInstances(context.TODO(), gomock.Any()).
96-
Return(&sd.ListInstancesOutput{
97-
Instances: []types.InstanceSummary{
98-
{Id: aws.String(test.EndptId1)},
99-
{Id: aws.String(test.EndptId2)},
95+
awsFacade.EXPECT().DiscoverInstances(context.TODO(),
96+
&sd.DiscoverInstancesInput{
97+
NamespaceName: aws.String(test.NsName),
98+
ServiceName: aws.String(test.SvcName),
99+
HealthStatus: types.HealthStatusFilterAll,
100+
MaxResults: aws.Int32(1000),
101+
}).
102+
Return(&sd.DiscoverInstancesOutput{
103+
Instances: []types.HttpInstanceSummary{
104+
{InstanceId: aws.String(test.EndptId1)},
105+
{InstanceId: aws.String(test.EndptId2)},
100106
},
101107
}, nil)
102108

103-
insts, err := sdApi.ListInstances(context.TODO(), test.SvcId)
109+
insts, err := sdApi.DiscoverInstances(context.TODO(), test.NsName, test.SvcName)
104110
assert.Nil(t, err, "No error for happy case")
105111
assert.True(t, len(insts) == 2)
106-
assert.Equal(t, test.EndptId1, *insts[0].Id)
107-
assert.Equal(t, test.EndptId2, *insts[1].Id)
112+
assert.Equal(t, test.EndptId1, *insts[0].InstanceId)
113+
assert.Equal(t, test.EndptId2, *insts[1].InstanceId)
108114
}
109115

110116
func TestServiceDiscoveryApi_ListOperations_HappyCase(t *testing.T) {

‎pkg/cloudmap/aws_facade.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@ type AwsFacade interface {
1515
// ListServices provides ServiceDiscovery ListServices wrapper interface for paginator.
1616
ListServices(context.Context, *sd.ListServicesInput, ...func(options *sd.Options)) (*sd.ListServicesOutput, error)
1717

18-
// ListInstances provides ServiceDiscovery ListInstances wrapper interface for paginator.
19-
ListInstances(context.Context, *sd.ListInstancesInput, ...func(*sd.Options)) (*sd.ListInstancesOutput, error)
20-
2118
// ListOperations provides ServiceDiscovery ListOperations wrapper interface for paginator.
2219
ListOperations(context.Context, *sd.ListOperationsInput, ...func(*sd.Options)) (*sd.ListOperationsOutput, error)
2320

@@ -35,6 +32,9 @@ type AwsFacade interface {
3532

3633
// DeregisterInstance provides ServiceDiscovery DeregisterInstance wrapper interface.
3734
DeregisterInstance(context.Context, *sd.DeregisterInstanceInput, ...func(*sd.Options)) (*sd.DeregisterInstanceOutput, error)
35+
36+
// DiscoverInstances provides ServiceDiscovery DiscoverInstances wrapper interface.
37+
DiscoverInstances(context.Context, *sd.DiscoverInstancesInput, ...func(*sd.Options)) (*sd.DiscoverInstancesOutput, error)
3838
}
3939

4040
type awsFacade struct {

‎pkg/cloudmap/cache.go

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ type ServiceDiscoveryClientCache interface {
2727
CacheNilNamespace(namespaceName string)
2828
GetServiceId(namespaceName string, serviceName string) (serviceId string, found bool)
2929
CacheServiceId(namespaceName string, serviceName string, serviceId string)
30-
GetEndpoints(serviceId string) (endpoints []*model.Endpoint, found bool)
31-
CacheEndpoints(serviceId string, endpoints []*model.Endpoint)
32-
EvictEndpoints(serviceId string)
30+
GetEndpoints(namespaceName string, serviceName string) (endpoints []*model.Endpoint, found bool)
31+
CacheEndpoints(namespaceName string, serviceName string, endpoints []*model.Endpoint)
32+
EvictEndpoints(namespaceName string, serviceName string)
3333
}
3434

3535
type sdCache struct {
@@ -109,30 +109,31 @@ func (sdCache *sdCache) CacheServiceId(nsName string, svcName string, svcId stri
109109
sdCache.cache.Add(key, svcId, sdCache.config.svcTTL)
110110
}
111111

112-
func (sdCache *sdCache) GetEndpoints(svcId string) (endpts []*model.Endpoint, found bool) {
113-
key := sdCache.buildEndptsKey(svcId)
112+
func (sdCache *sdCache) GetEndpoints(nsName string, svcName string) (endpts []*model.Endpoint, found bool) {
113+
key := sdCache.buildEndptsKey(nsName, svcName)
114114
entry, exists := sdCache.cache.Get(key)
115115
if !exists {
116116
return nil, false
117117
}
118118

119119
endpts, ok := entry.([]*model.Endpoint)
120120
if !ok {
121-
sdCache.log.Error(errors.New("failed to retrieve endpoints from cache"), "", "svcId", svcId)
121+
sdCache.log.Error(errors.New("failed to retrieve endpoints from cache"), "",
122+
"ns", "nsName", "svc", svcName)
122123
sdCache.cache.Remove(key)
123124
return nil, false
124125
}
125126

126127
return endpts, true
127128
}
128129

129-
func (sdCache *sdCache) CacheEndpoints(svcId string, endpts []*model.Endpoint) {
130-
key := sdCache.buildEndptsKey(svcId)
130+
func (sdCache *sdCache) CacheEndpoints(nsName string, svcName string, endpts []*model.Endpoint) {
131+
key := sdCache.buildEndptsKey(nsName, svcName)
131132
sdCache.cache.Add(key, endpts, sdCache.config.endptTTL)
132133
}
133134

134-
func (sdCache *sdCache) EvictEndpoints(svcId string) {
135-
key := sdCache.buildEndptsKey(svcId)
135+
func (sdCache *sdCache) EvictEndpoints(nsName string, svcName string) {
136+
key := sdCache.buildEndptsKey(nsName, svcName)
136137
sdCache.cache.Remove(key)
137138
}
138139

@@ -144,6 +145,6 @@ func (sdCache *sdCache) buildSvcKey(nsName string, svcName string) (cacheKey str
144145
return fmt.Sprintf("%s:%s:%s", svcKeyPrefix, nsName, svcName)
145146
}
146147

147-
func (sdCache *sdCache) buildEndptsKey(svcId string) string {
148-
return fmt.Sprintf("%s:%s", endptKeyPrefix, svcId)
148+
func (sdCache *sdCache) buildEndptsKey(nsName string, svcName string) string {
149+
return fmt.Sprintf("%s:%s:%s", endptKeyPrefix, nsName, svcName)
149150
}

‎pkg/cloudmap/cache_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,36 +79,36 @@ func TestServiceDiscoveryClientCacheGetServiceId_Corrupt(t *testing.T) {
7979

8080
func TestServiceDiscoveryClientCacheGetEndpoints_Found(t *testing.T) {
8181
sdc := NewDefaultServiceDiscoveryClientCache()
82-
sdc.CacheEndpoints(test.SvcId, []*model.Endpoint{test.GetTestEndpoint(), test.GetTestEndpoint2()})
82+
sdc.CacheEndpoints(test.NsName, test.SvcName, []*model.Endpoint{test.GetTestEndpoint(), test.GetTestEndpoint2()})
8383

84-
endpts, found := sdc.GetEndpoints(test.SvcId)
84+
endpts, found := sdc.GetEndpoints(test.NsName, test.SvcName)
8585
assert.True(t, found)
8686
assert.Equal(t, []*model.Endpoint{test.GetTestEndpoint(), test.GetTestEndpoint2()}, endpts)
8787
}
8888

8989
func TestServiceDiscoveryClientCacheGetEndpoints_NotFound(t *testing.T) {
9090
sdc := NewDefaultServiceDiscoveryClientCache()
9191

92-
endpts, found := sdc.GetEndpoints(test.SvcId)
92+
endpts, found := sdc.GetEndpoints(test.NsName, test.SvcName)
9393
assert.False(t, found)
9494
assert.Nil(t, endpts)
9595
}
9696

9797
func TestServiceDiscoveryClientCacheGetEndpoints_Corrupt(t *testing.T) {
9898
sdc := NewDefaultServiceDiscoveryClientCache().(*sdCache)
9999

100-
sdc.cache.Add(sdc.buildEndptsKey(test.SvcId), &model.Resource{}, time.Minute)
101-
endpts, found := sdc.GetEndpoints(test.SvcId)
100+
sdc.cache.Add(sdc.buildEndptsKey(test.NsName, test.SvcName), &model.Resource{}, time.Minute)
101+
endpts, found := sdc.GetEndpoints(test.NsName, test.SvcName)
102102
assert.False(t, found)
103103
assert.Nil(t, endpts)
104104
}
105105

106106
func TestServiceDiscoveryClientEvictEndpoints(t *testing.T) {
107107
sdc := NewDefaultServiceDiscoveryClientCache()
108-
sdc.CacheEndpoints(test.SvcId, []*model.Endpoint{test.GetTestEndpoint(), test.GetTestEndpoint2()})
109-
sdc.EvictEndpoints(test.SvcId)
108+
sdc.CacheEndpoints(test.NsName, test.SvcName, []*model.Endpoint{test.GetTestEndpoint(), test.GetTestEndpoint2()})
109+
sdc.EvictEndpoints(test.NsName, test.SvcName)
110110

111-
endpts, found := sdc.GetEndpoints(test.SvcId)
111+
endpts, found := sdc.GetEndpoints(test.NsName, test.SvcName)
112112
assert.False(t, found)
113113
assert.Nil(t, endpts)
114114
}

‎pkg/cloudmap/client.go

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (sdc *serviceDiscoveryClient) ListServices(ctx context.Context, nsName stri
5858
for _, svcSum := range svcSums {
5959
sdc.cache.CacheServiceId(nsName, svcSum.Name, svcSum.Id)
6060

61-
endpts, endptsErr := sdc.listEndpoints(ctx, svcSum.Id)
61+
endpts, endptsErr := sdc.listEndpoints(ctx, nsName, svcSum.Name)
6262
if endptsErr != nil {
6363
return svcs, endptsErr
6464
}
@@ -101,6 +101,15 @@ func (sdc *serviceDiscoveryClient) CreateService(ctx context.Context, nsName str
101101

102102
func (sdc *serviceDiscoveryClient) GetService(ctx context.Context, nsName string, svcName string) (svc *model.Service, err error) {
103103
sdc.log.Info("fetching a service", "namespace", nsName, "name", svcName)
104+
endpts, cacheHit := sdc.cache.GetEndpoints(nsName, svcName)
105+
106+
if cacheHit {
107+
return &model.Service{
108+
Namespace: nsName,
109+
Name: svcName,
110+
Endpoints: endpts,
111+
}, nil
112+
}
104113

105114
svcId, err := sdc.getServiceId(ctx, nsName, svcName)
106115

@@ -112,19 +121,17 @@ func (sdc *serviceDiscoveryClient) GetService(ctx context.Context, nsName string
112121
return nil, nil
113122
}
114123

115-
endpts, err := sdc.listEndpoints(ctx, svcId)
124+
endpts, err = sdc.listEndpoints(ctx, nsName, svcName)
116125

117126
if err != nil {
118127
return nil, err
119128
}
120129

121-
svc = &model.Service{
130+
return &model.Service{
122131
Namespace: nsName,
123132
Name: svcName,
124133
Endpoints: endpts,
125-
}
126-
127-
return svc, nil
134+
}, nil
128135
}
129136

130137
func (sdc *serviceDiscoveryClient) RegisterEndpoints(ctx context.Context, nsName string, svcName string, endpts []*model.Endpoint) (err error) {
@@ -153,7 +160,7 @@ func (sdc *serviceDiscoveryClient) RegisterEndpoints(ctx context.Context, nsName
153160
err = NewRegisterInstancePoller(sdc.sdApi, svcId, opCollector.Collect(), opCollector.GetStartTime()).Poll(ctx)
154161

155162
// Evict cache entry so next list call reflects changes
156-
sdc.cache.EvictEndpoints(svcId)
163+
sdc.cache.EvictEndpoints(nsName, svcName)
157164

158165
if err != nil {
159166
return err
@@ -192,7 +199,7 @@ func (sdc *serviceDiscoveryClient) DeleteEndpoints(ctx context.Context, nsName s
192199
err = NewDeregisterInstancePoller(sdc.sdApi, svcId, opCollector.Collect(), opCollector.GetStartTime()).Poll(ctx)
193200

194201
// Evict cache entry so next list call reflects changes
195-
sdc.cache.EvictEndpoints(svcId)
202+
sdc.cache.EvictEndpoints(nsName, svcName)
196203
if err != nil {
197204
return err
198205
}
@@ -204,26 +211,26 @@ func (sdc *serviceDiscoveryClient) DeleteEndpoints(ctx context.Context, nsName s
204211
return nil
205212
}
206213

207-
func (sdc *serviceDiscoveryClient) listEndpoints(ctx context.Context, serviceId string) (endpts []*model.Endpoint, err error) {
208-
if endpts, found := sdc.cache.GetEndpoints(serviceId); found {
214+
func (sdc *serviceDiscoveryClient) listEndpoints(ctx context.Context, nsName string, svcName string) (endpts []*model.Endpoint, err error) {
215+
if endpts, found := sdc.cache.GetEndpoints(nsName, svcName); found {
209216
return endpts, nil
210217
}
211218

212-
insts, err := sdc.sdApi.ListInstances(ctx, serviceId)
219+
insts, err := sdc.sdApi.DiscoverInstances(ctx, nsName, svcName)
213220
if err != nil {
214221
return nil, err
215222
}
216223

217224
for _, inst := range insts {
218225
endpt, endptErr := model.NewEndpointFromInstance(&inst)
219226
if endptErr != nil {
220-
sdc.log.Info(fmt.Sprintf("skipping instance %s to endpoint conversion: %s", *inst.Id, endptErr.Error()))
227+
sdc.log.Info(fmt.Sprintf("skipping instance %s to endpoint conversion: %s", *inst.InstanceId, endptErr.Error()))
221228
continue
222229
}
223230
endpts = append(endpts, endpt)
224231
}
225232

226-
sdc.cache.CacheEndpoints(serviceId, endpts)
233+
sdc.cache.CacheEndpoints(nsName, svcName, endpts)
227234

228235
return endpts, nil
229236
}

0 commit comments

Comments
 (0)