Skip to content

Commit 8f34a87

Browse files
farodin91farodin91
authored
feat(coredns): pass context to etcd client (kubernetes-sigs#5915)
Signed-off-by: Jan Jansen <[email protected]>
1 parent d9a20d1 commit 8f34a87

File tree

2 files changed

+32
-40
lines changed

2 files changed

+32
-40
lines changed

‎provider/coredns/coredns.go‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ const (
4747

4848
// coreDNSClient is an interface to work with CoreDNS service records in etcd
4949
type coreDNSClient interface {
50-
GetServices(prefix string) ([]*Service, error)
51-
SaveService(value *Service) error
52-
DeleteService(key string) error
50+
GetServices(ctx context.Context, prefix string) ([]*Service, error)
51+
SaveService(ctx context.Context, value *Service) error
52+
DeleteService(ctx context.Context, key string) error
5353
}
5454

5555
type coreDNSProvider struct {
@@ -88,14 +88,13 @@ type Service struct {
8888

8989
type etcdClient struct {
9090
client *etcdcv3.Client
91-
ctx context.Context
9291
}
9392

9493
var _ coreDNSClient = etcdClient{}
9594

9695
// GetServices GetService return all Service records stored in etcd stored anywhere under the given key (recursively)
97-
func (c etcdClient) GetServices(prefix string) ([]*Service, error) {
98-
ctx, cancel := context.WithTimeout(c.ctx, etcdTimeout)
96+
func (c etcdClient) GetServices(ctx context.Context, prefix string) ([]*Service, error) {
97+
ctx, cancel := context.WithTimeout(ctx, etcdTimeout)
9998
defer cancel()
10099

101100
path := prefix
@@ -129,8 +128,8 @@ func (c etcdClient) GetServices(prefix string) ([]*Service, error) {
129128
}
130129

131130
// SaveService persists service data into etcd
132-
func (c etcdClient) SaveService(service *Service) error {
133-
ctx, cancel := context.WithTimeout(c.ctx, etcdTimeout)
131+
func (c etcdClient) SaveService(ctx context.Context, service *Service) error {
132+
ctx, cancel := context.WithTimeout(ctx, etcdTimeout)
134133
defer cancel()
135134

136135
value, err := json.Marshal(&service)
@@ -145,8 +144,8 @@ func (c etcdClient) SaveService(service *Service) error {
145144
}
146145

147146
// DeleteService deletes service record from etcd
148-
func (c etcdClient) DeleteService(key string) error {
149-
ctx, cancel := context.WithTimeout(c.ctx, etcdTimeout)
147+
func (c etcdClient) DeleteService(ctx context.Context, key string) error {
148+
ctx, cancel := context.WithTimeout(ctx, etcdTimeout)
150149
defer cancel()
151150

152151
_, err := c.client.Delete(ctx, key, etcdcv3.WithPrefix())
@@ -193,7 +192,7 @@ func newETCDClient() (coreDNSClient, error) {
193192
if err != nil {
194193
return nil, err
195194
}
196-
return etcdClient{c, context.Background()}, nil
195+
return etcdClient{c}, nil
197196
}
198197

199198
// NewCoreDNSProvider is a CoreDNS provider constructor
@@ -235,9 +234,9 @@ func findLabelInTargets(targets []string, label string) (string, bool) {
235234

236235
// Records returns all DNS records found in CoreDNS etcd backend. Depending on the record fields
237236
// it may be mapped to one or two records of type A, CNAME, TXT, A+TXT, CNAME+TXT
238-
func (p coreDNSProvider) Records(_ context.Context) ([]*endpoint.Endpoint, error) {
237+
func (p coreDNSProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
239238
var result []*endpoint.Endpoint
240-
services, err := p.client.GetServices(p.coreDNSPrefix)
239+
services, err := p.client.GetServices(ctx, p.coreDNSPrefix)
241240
if err != nil {
242241
return nil, err
243242
}
@@ -285,20 +284,20 @@ func (p coreDNSProvider) Records(_ context.Context) ([]*endpoint.Endpoint, error
285284
return result, nil
286285
}
287286

288-
func (p coreDNSProvider) ApplyChanges(_ context.Context, changes *plan.Changes) error {
287+
func (p coreDNSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error {
289288
grouped := p.groupEndpoints(changes)
290289

291290
for dnsName, group := range grouped {
292291
if !p.domainFilter.Match(dnsName) {
293292
log.Debugf("Skipping record %q due to domain filter", dnsName)
294293
continue
295294
}
296-
if err := p.applyGroup(dnsName, group); err != nil {
295+
if err := p.applyGroup(ctx, dnsName, group); err != nil {
297296
return err
298297
}
299298
}
300299

301-
return p.deleteEndpoints(changes.Delete)
300+
return p.deleteEndpoints(ctx, changes.Delete)
302301
}
303302

304303
func (p coreDNSProvider) groupEndpoints(changes *plan.Changes) map[string][]*endpoint.Endpoint {
@@ -314,12 +313,12 @@ func (p coreDNSProvider) groupEndpoints(changes *plan.Changes) map[string][]*end
314313
return grouped
315314
}
316315

317-
func (p coreDNSProvider) applyGroup(dnsName string, group []*endpoint.Endpoint) error {
316+
func (p coreDNSProvider) applyGroup(ctx context.Context, dnsName string, group []*endpoint.Endpoint) error {
318317
var services []*Service
319318

320319
for _, ep := range group {
321320
if ep.RecordType != endpoint.RecordTypeTXT {
322-
srvs, err := p.createServicesForEndpoint(dnsName, ep)
321+
srvs, err := p.createServicesForEndpoint(ctx, dnsName, ep)
323322
if err != nil {
324323
return err
325324
}
@@ -334,15 +333,15 @@ func (p coreDNSProvider) applyGroup(dnsName string, group []*endpoint.Endpoint)
334333
if p.dryRun {
335334
continue
336335
}
337-
if err := p.client.SaveService(service); err != nil {
336+
if err := p.client.SaveService(ctx, service); err != nil {
338337
return err
339338
}
340339
}
341340

342341
return nil
343342
}
344343

345-
func (p coreDNSProvider) createServicesForEndpoint(dnsName string, ep *endpoint.Endpoint) ([]*Service, error) {
344+
func (p coreDNSProvider) createServicesForEndpoint(ctx context.Context, dnsName string, ep *endpoint.Endpoint) ([]*Service, error) {
346345
var services []*Service
347346

348347
for _, target := range ep.Targets {
@@ -378,7 +377,7 @@ func (p coreDNSProvider) createServicesForEndpoint(dnsName string, ep *endpoint.
378377
if p.dryRun {
379378
continue
380379
}
381-
if err := p.client.DeleteService(key); err != nil {
380+
if err := p.client.DeleteService(ctx, key); err != nil {
382381
return nil, err
383382
}
384383
}
@@ -420,7 +419,7 @@ func (p coreDNSProvider) updateTXTRecords(dnsName string, group []*endpoint.Endp
420419
return services
421420
}
422421

423-
func (p coreDNSProvider) deleteEndpoints(endpoints []*endpoint.Endpoint) error {
422+
func (p coreDNSProvider) deleteEndpoints(ctx context.Context, endpoints []*endpoint.Endpoint) error {
424423
for _, ep := range endpoints {
425424
dnsName := ep.DNSName
426425
if ep.Labels[randomPrefixLabel] != "" {
@@ -431,7 +430,7 @@ func (p coreDNSProvider) deleteEndpoints(endpoints []*endpoint.Endpoint) error {
431430
if p.dryRun {
432431
continue
433432
}
434-
if err := p.client.DeleteService(key); err != nil {
433+
if err := p.client.DeleteService(ctx, key); err != nil {
435434
return err
436435
}
437436
}

‎provider/coredns/coredns_test.go‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type fakeETCDClient struct {
4343
services map[string]Service
4444
}
4545

46-
func (c fakeETCDClient) GetServices(prefix string) ([]*Service, error) {
46+
func (c fakeETCDClient) GetServices(_ context.Context, prefix string) ([]*Service, error) {
4747
var result []*Service
4848
for key, value := range c.services {
4949
if strings.HasPrefix(key, prefix) {
@@ -55,12 +55,12 @@ func (c fakeETCDClient) GetServices(prefix string) ([]*Service, error) {
5555
return result, nil
5656
}
5757

58-
func (c fakeETCDClient) SaveService(service *Service) error {
58+
func (c fakeETCDClient) SaveService(_ context.Context, service *Service) error {
5959
c.services[service.Key] = *service
6060
return nil
6161
}
6262

63-
func (c fakeETCDClient) DeleteService(key string) error {
63+
func (c fakeETCDClient) DeleteService(_ context.Context, key string) error {
6464
delete(c.services, key)
6565
return nil
6666
}
@@ -507,10 +507,9 @@ func TestGetServices_Success(t *testing.T) {
507507
client: &etcdcv3.Client{
508508
KV: mockKV,
509509
},
510-
ctx: context.TODO(),
511510
}
512511

513-
result, err := c.GetServices("/prefix")
512+
result, err := c.GetServices(context.Background(), "/prefix")
514513
assert.NoError(t, err)
515514
assert.Len(t, result, 1)
516515
assert.Equal(t, "example.com", result[0].Host)
@@ -522,7 +521,6 @@ func TestGetServices_Duplicate(t *testing.T) {
522521
client: &etcdcv3.Client{
523522
KV: mockKV,
524523
},
525-
ctx: context.TODO(),
526524
}
527525

528526
svc := Service{Host: "example.com", Port: 80, Priority: 1, Weight: 10, Text: "hello"}
@@ -542,7 +540,7 @@ func TestGetServices_Duplicate(t *testing.T) {
542540
},
543541
}, nil)
544542

545-
result, err := c.GetServices("/prefix")
543+
result, err := c.GetServices(context.Background(), "/prefix")
546544
assert.NoError(t, err)
547545
assert.Len(t, result, 1)
548546
}
@@ -553,7 +551,6 @@ func TestGetServices_Multiple(t *testing.T) {
553551
client: &etcdcv3.Client{
554552
KV: mockKV,
555553
},
556-
ctx: context.TODO(),
557554
}
558555

559556
svc := Service{Host: "example.com", Port: 80, Priority: 1, Weight: 10, Text: "hello"}
@@ -576,7 +573,7 @@ func TestGetServices_Multiple(t *testing.T) {
576573
},
577574
}, nil)
578575

579-
result, err := c.GetServices("/prefix")
576+
result, err := c.GetServices(context.Background(), "/prefix")
580577
assert.NoError(t, err)
581578
assert.Len(t, result, 2)
582579
assert.Equal(t, priority, result[1].Priority)
@@ -588,7 +585,6 @@ func TestGetServices_UnmarshalError(t *testing.T) {
588585
client: &etcdcv3.Client{
589586
KV: mockKV,
590587
},
591-
ctx: context.TODO(),
592588
}
593589

594590
mockKV.On("Get", mock.Anything, "/prefix").Return(&etcdcv3.GetResponse{
@@ -604,7 +600,7 @@ func TestGetServices_UnmarshalError(t *testing.T) {
604600
},
605601
}, nil)
606602

607-
_, err := c.GetServices("/prefix")
603+
_, err := c.GetServices(context.Background(), "/prefix")
608604
assert.Error(t, err)
609605
assert.Contains(t, err.Error(), "/prefix/1")
610606
}
@@ -615,12 +611,11 @@ func TestGetServices_GetError(t *testing.T) {
615611
client: &etcdcv3.Client{
616612
KV: mockKV,
617613
},
618-
ctx: context.TODO(),
619614
}
620615

621616
mockKV.On("Get", mock.Anything, "/prefix").Return(&etcdcv3.GetResponse{}, errors.New("etcd failure"))
622617

623-
_, err := c.GetServices("/prefix")
618+
_, err := c.GetServices(context.Background(), "/prefix")
624619
assert.Error(t, err)
625620
assert.EqualError(t, err, "etcd failure")
626621
}
@@ -654,10 +649,9 @@ func TestDeleteService(t *testing.T) {
654649
client: &etcdcv3.Client{
655650
KV: mockKV,
656651
},
657-
ctx: context.Background(),
658652
}
659653

660-
err := c.DeleteService(tt.key)
654+
err := c.DeleteService(context.Background(), tt.key)
661655

662656
if tt.wantErr {
663657
require.Error(t, err)
@@ -712,10 +706,9 @@ func TestSaveService(t *testing.T) {
712706
client: &etcdcv3.Client{
713707
KV: mockKV,
714708
},
715-
ctx: context.TODO(),
716709
}
717710

718-
err = c.SaveService(tt.service)
711+
err = c.SaveService(context.Background(), tt.service)
719712
if tt.wantErr {
720713
assert.Error(t, err)
721714
} else {

0 commit comments

Comments
 (0)