Skip to content

Commit 0481540

Browse files
farodin91farodin91
authored
Merge pull request #26 from GDATASoftwareAG/feat-coredns-pass-context-to-etcd-client
feat(coredns): pass context to etcd client
2 parents df5d462 + 8d4f9e5 commit 0481540

File tree

2 files changed

+38
-48
lines changed

2 files changed

+38
-48
lines changed

‎coredns.go‎

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ type CoreDNSConfig struct {
5555

5656
// coreDNSClient is an interface to work with CoreDNS service records in etcd
5757
type coreDNSClient interface {
58-
GetServices(prefix string) ([]*Service, error)
59-
SaveService(value *Service) error
60-
DeleteService(key string) error
58+
GetServices(ctx context.Context, prefix string) ([]*Service, error)
59+
SaveService(ctx context.Context, value *Service) error
60+
DeleteService(ctx context.Context, key string) error
6161
}
6262

6363
type coreDNSProvider struct {
@@ -98,16 +98,15 @@ type Service struct {
9898

9999
type etcdClient struct {
100100
client *etcdcv3.Client
101-
ctx context.Context
102101
managedBy string
103102
ignoreEmptyManagedBy bool
104103
}
105104

106105
var _ coreDNSClient = etcdClient{}
107106

108107
// GetServices return all Service records stored in etcd stored anywhere under the given key (recursively)
109-
func (c etcdClient) GetServices(prefix string) ([]*Service, error) {
110-
ctx, cancel := context.WithTimeout(c.ctx, etcdTimeout)
108+
func (c etcdClient) GetServices(ctx context.Context, prefix string) ([]*Service, error) {
109+
ctx, cancel := context.WithTimeout(ctx, etcdTimeout)
111110
defer cancel()
112111

113112
path := prefix
@@ -157,14 +156,14 @@ func (c etcdClient) GetServices(prefix string) ([]*Service, error) {
157156
}
158157

159158
// SaveService persists service data into etcd
160-
func (c etcdClient) SaveService(service *Service) error {
161-
ctx, cancel := context.WithTimeout(c.ctx, etcdTimeout)
159+
func (c etcdClient) SaveService(ctx context.Context, service *Service) error {
160+
ctx, cancel := context.WithTimeout(ctx, etcdTimeout)
162161
defer cancel()
163162

164163
if c.managedBy != "" {
165164
service.ManagedBy = c.managedBy
166165
}
167-
if ownedBy, err := c.IsOwnedBy(service.Key); err != nil {
166+
if ownedBy, err := c.IsOwnedBy(ctx, service.Key); err != nil {
168167
return err
169168
} else if !ownedBy {
170169
return fmt.Errorf("key %q is not owned by this service", service.Key)
@@ -181,8 +180,8 @@ func (c etcdClient) SaveService(service *Service) error {
181180
return nil
182181
}
183182

184-
func (c etcdClient) IsOwnedBy(key string) (bool, error) {
185-
ctx, cancel := context.WithTimeout(c.ctx, etcdTimeout)
183+
func (c etcdClient) IsOwnedBy(ctx context.Context, key string) (bool, error) {
184+
ctx, cancel := context.WithTimeout(ctx, etcdTimeout)
186185
defer cancel()
187186

188187
if c.managedBy == "" {
@@ -217,11 +216,11 @@ func (c etcdClient) IsOwnedBy(key string) (bool, error) {
217216
}
218217

219218
// DeleteService deletes service record from etcd
220-
func (c etcdClient) DeleteService(key string) error {
221-
ctx, cancel := context.WithTimeout(c.ctx, etcdTimeout)
219+
func (c etcdClient) DeleteService(ctx context.Context, key string) error {
220+
ctx, cancel := context.WithTimeout(ctx, etcdTimeout)
222221
defer cancel()
223222

224-
if owned, err := c.IsOwnedBy(key); err != nil {
223+
if owned, err := c.IsOwnedBy(ctx, key); err != nil {
225224
return err
226225
} else if !owned {
227226
return fmt.Errorf("key %q is not owned by this service", key)
@@ -270,7 +269,7 @@ func newETCDClient(managedBy string, ignoreEmptyManagedBy bool) (coreDNSClient,
270269
if err != nil {
271270
return nil, err
272271
}
273-
return etcdClient{c, context.Background(), managedBy, ignoreEmptyManagedBy}, nil
272+
return etcdClient{c, managedBy, ignoreEmptyManagedBy}, nil
274273
}
275274

276275
// NewCoreDNSProvider is a CoreDNS provider constructor
@@ -311,9 +310,9 @@ func findLabelInTargets(targets []string, label string) (string, bool) {
311310

312311
// Records returns all DNS records found in CoreDNS etcd backend. Depending on the record fields
313312
// it may be mapped to one or two records of type A, CNAME, TXT, A+TXT, CNAME+TXT
314-
func (p coreDNSProvider) Records(_ context.Context) ([]*endpoint.Endpoint, error) {
313+
func (p coreDNSProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
315314
var result []*endpoint.Endpoint
316-
services, err := p.client.GetServices(p.coreDNSPrefix)
315+
services, err := p.client.GetServices(ctx, p.coreDNSPrefix)
317316
if err != nil {
318317
return nil, err
319318
}
@@ -361,20 +360,20 @@ func (p coreDNSProvider) Records(_ context.Context) ([]*endpoint.Endpoint, error
361360
return result, nil
362361
}
363362

364-
func (p coreDNSProvider) ApplyChanges(_ context.Context, changes *plan.Changes) error {
363+
func (p coreDNSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error {
365364
grouped := p.groupEndpoints(changes)
366365

367366
for dnsName, group := range grouped {
368367
if !p.domainFilter.Match(dnsName) {
369368
log.Debugf("Skipping record %q due to domain filter", dnsName)
370369
continue
371370
}
372-
if err := p.applyGroup(dnsName, group); err != nil {
371+
if err := p.applyGroup(ctx, dnsName, group); err != nil {
373372
return err
374373
}
375374
}
376375

377-
return p.deleteEndpoints(changes.Delete)
376+
return p.deleteEndpoints(ctx, changes.Delete)
378377
}
379378

380379
func (p coreDNSProvider) groupEndpoints(changes *plan.Changes) map[string][]*endpoint.Endpoint {
@@ -390,12 +389,12 @@ func (p coreDNSProvider) groupEndpoints(changes *plan.Changes) map[string][]*end
390389
return grouped
391390
}
392391

393-
func (p coreDNSProvider) applyGroup(dnsName string, group []*endpoint.Endpoint) error {
392+
func (p coreDNSProvider) applyGroup(ctx context.Context, dnsName string, group []*endpoint.Endpoint) error {
394393
var services []*Service
395394

396395
for _, ep := range group {
397396
if ep.RecordType != endpoint.RecordTypeTXT {
398-
srvs, err := p.createServicesForEndpoint(dnsName, ep)
397+
srvs, err := p.createServicesForEndpoint(ctx, dnsName, ep)
399398
if err != nil {
400399
return err
401400
}
@@ -410,15 +409,15 @@ func (p coreDNSProvider) applyGroup(dnsName string, group []*endpoint.Endpoint)
410409
if p.dryRun {
411410
continue
412411
}
413-
if err := p.client.SaveService(service); err != nil {
412+
if err := p.client.SaveService(ctx, service); err != nil {
414413
return err
415414
}
416415
}
417416

418417
return nil
419418
}
420419

421-
func (p coreDNSProvider) createServicesForEndpoint(dnsName string, ep *endpoint.Endpoint) ([]*Service, error) {
420+
func (p coreDNSProvider) createServicesForEndpoint(ctx context.Context, dnsName string, ep *endpoint.Endpoint) ([]*Service, error) {
422421
var services []*Service
423422

424423
for _, target := range ep.Targets {
@@ -455,7 +454,7 @@ func (p coreDNSProvider) createServicesForEndpoint(dnsName string, ep *endpoint.
455454
if p.dryRun {
456455
continue
457456
}
458-
if err := p.client.DeleteService(key); err != nil {
457+
if err := p.client.DeleteService(ctx, key); err != nil {
459458
return nil, err
460459
}
461460
}
@@ -497,7 +496,7 @@ func (p coreDNSProvider) updateTXTRecords(dnsName string, group []*endpoint.Endp
497496
return services
498497
}
499498

500-
func (p coreDNSProvider) deleteEndpoints(endpoints []*endpoint.Endpoint) error {
499+
func (p coreDNSProvider) deleteEndpoints(ctx context.Context, endpoints []*endpoint.Endpoint) error {
501500
for _, ep := range endpoints {
502501
dnsName := ep.DNSName
503502
if ep.Labels[randomPrefixLabel] != "" {
@@ -508,7 +507,7 @@ func (p coreDNSProvider) deleteEndpoints(endpoints []*endpoint.Endpoint) error {
508507
if p.dryRun {
509508
continue
510509
}
511-
if err := p.client.DeleteService(key); err != nil {
510+
if err := p.client.DeleteService(ctx, key); err != nil {
512511
return err
513512
}
514513
}

‎coredns_test.go‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ type fakeETCDClient struct {
4242
services map[string]Service
4343
}
4444

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

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

62-
func (c fakeETCDClient) DeleteService(key string) error {
62+
func (c fakeETCDClient) DeleteService(_ context.Context, key string) error {
6363
delete(c.services, key)
6464
return nil
6565
}
@@ -569,10 +569,9 @@ func TestGetServices_Success(t *testing.T) {
569569
client: &etcdcv3.Client{
570570
KV: mockKV,
571571
},
572-
ctx: context.TODO(),
573572
}
574573

575-
result, err := c.GetServices("/prefix")
574+
result, err := c.GetServices(context.Background(), "/prefix")
576575
assert.NoError(t, err)
577576
assert.Len(t, result, 1)
578577
assert.Equal(t, "example.com", result[0].Host)
@@ -584,7 +583,6 @@ func TestGetServices_Duplicate(t *testing.T) {
584583
client: &etcdcv3.Client{
585584
KV: mockKV,
586585
},
587-
ctx: context.TODO(),
588586
}
589587

590588
svc := Service{Host: "example.com", Port: 80, Priority: 1, Weight: 10, Text: "hello"}
@@ -604,7 +602,7 @@ func TestGetServices_Duplicate(t *testing.T) {
604602
},
605603
}, nil)
606604

607-
result, err := c.GetServices("/prefix")
605+
result, err := c.GetServices(context.Background(), "/prefix")
608606
assert.NoError(t, err)
609607
assert.Len(t, result, 1)
610608
}
@@ -615,7 +613,6 @@ func TestGetServices_Multiple(t *testing.T) {
615613
client: &etcdcv3.Client{
616614
KV: mockKV,
617615
},
618-
ctx: context.TODO(),
619616
}
620617

621618
svc := Service{Host: "example.com", Port: 80, Priority: 1, Weight: 10, Text: "hello"}
@@ -638,7 +635,7 @@ func TestGetServices_Multiple(t *testing.T) {
638635
},
639636
}, nil)
640637

641-
result, err := c.GetServices("/prefix")
638+
result, err := c.GetServices(context.Background(), "/prefix")
642639
assert.NoError(t, err)
643640
assert.Len(t, result, 2)
644641
assert.Equal(t, priority, result[1].Priority)
@@ -650,7 +647,6 @@ func TestGetServices_FilterOutOtherServicesWithDifferentManager(t *testing.T) {
650647
client: &etcdcv3.Client{
651648
KV: mockKV,
652649
},
653-
ctx: context.TODO(),
654650
managedBy: "managed-by",
655651
ignoreEmptyManagedBy: false,
656652
}
@@ -682,7 +678,7 @@ func TestGetServices_FilterOutOtherServicesWithDifferentManager(t *testing.T) {
682678
},
683679
}, nil)
684680

685-
result, err := c.GetServices("/prefix")
681+
result, err := c.GetServices(context.Background(), "/prefix")
686682
assert.NoError(t, err)
687683
assert.Len(t, result, 2)
688684
assert.Equal(t, "managed-by", result[0].ManagedBy)
@@ -695,7 +691,6 @@ func TestGetServices_FilterOutOtherServicesWithDifferentManagerAndIgnoreEmpty(t
695691
client: &etcdcv3.Client{
696692
KV: mockKV,
697693
},
698-
ctx: context.TODO(),
699694
managedBy: "managed-by",
700695
ignoreEmptyManagedBy: true,
701696
}
@@ -727,7 +722,7 @@ func TestGetServices_FilterOutOtherServicesWithDifferentManagerAndIgnoreEmpty(t
727722
},
728723
}, nil)
729724

730-
result, err := c.GetServices("/prefix")
725+
result, err := c.GetServices(context.Background(), "/prefix")
731726
assert.NoError(t, err)
732727
assert.Len(t, result, 1)
733728
assert.Equal(t, "managed-by", result[0].ManagedBy)
@@ -739,7 +734,6 @@ func TestGetServices_UnmarshalError(t *testing.T) {
739734
client: &etcdcv3.Client{
740735
KV: mockKV,
741736
},
742-
ctx: context.TODO(),
743737
}
744738

745739
mockKV.On("Get", mock.Anything, "/prefix").Return(&etcdcv3.GetResponse{
@@ -755,7 +749,7 @@ func TestGetServices_UnmarshalError(t *testing.T) {
755749
},
756750
}, nil)
757751

758-
_, err := c.GetServices("/prefix")
752+
_, err := c.GetServices(context.Background(), "/prefix")
759753
assert.Error(t, err)
760754
assert.Contains(t, err.Error(), "/prefix/1")
761755
}
@@ -766,12 +760,11 @@ func TestGetServices_GetError(t *testing.T) {
766760
client: &etcdcv3.Client{
767761
KV: mockKV,
768762
},
769-
ctx: context.TODO(),
770763
}
771764

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

774-
_, err := c.GetServices("/prefix")
767+
_, err := c.GetServices(context.Background(), "/prefix")
775768
assert.Error(t, err)
776769
assert.EqualError(t, err, "etcd failure")
777770
}
@@ -885,11 +878,10 @@ func TestDeleteService(t *testing.T) {
885878
client: &etcdcv3.Client{
886879
KV: mockKV,
887880
},
888-
ctx: context.Background(),
889881
managedBy: tt.managedBy,
890882
}
891883

892-
err = c.DeleteService(tt.key)
884+
err = c.DeleteService(context.Background(), tt.key)
893885

894886
if tt.wantErr {
895887
require.Error(t, err)
@@ -1061,11 +1053,10 @@ func TestSaveService(t *testing.T) {
10611053
client: &etcdcv3.Client{
10621054
KV: mockKV,
10631055
},
1064-
ctx: context.TODO(),
10651056
managedBy: tt.managedBy,
10661057
}
10671058

1068-
err = c.SaveService(tt.service)
1059+
err = c.SaveService(context.Background(), tt.service)
10691060
if tt.wantErr {
10701061
assert.Error(t, err)
10711062
} else {

0 commit comments

Comments
 (0)