Skip to content

Commit

Permalink
Merge pull request #1374 from tariq1890/rm_ctx_todo
Browse files Browse the repository at this point in the history
remove context.TODO()s in external-dns
  • Loading branch information
k8s-ci-robot authored Jan 21, 2020
2 parents 9353d12 + a5896c2 commit 4be51a9
Show file tree
Hide file tree
Showing 14 changed files with 199 additions and 170 deletions.
7 changes: 3 additions & 4 deletions controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,7 @@ type Controller struct {
}

// RunOnce runs a single iteration of a reconciliation loop.
func (c *Controller) RunOnce() error {
ctx := context.Background()
func (c *Controller) RunOnce(ctx context.Context) error {
records, err := c.Registry.Records(ctx)
if err != nil {
registryErrorsTotal.Inc()
Expand Down Expand Up @@ -141,11 +140,11 @@ func (c *Controller) RunOnce() error {
}

// Run runs RunOnce in a loop with a delay until stopChan receives a value.
func (c *Controller) Run(stopChan <-chan struct{}) {
func (c *Controller) Run(ctx context.Context, stopChan <-chan struct{}) {
ticker := time.NewTicker(c.Interval)
defer ticker.Stop()
for {
err := c.RunOnce()
err := c.RunOnce(ctx)
if err != nil {
log.Error(err)
}
Expand Down
2 changes: 1 addition & 1 deletion controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func TestRunOnce(t *testing.T) {
Policy: &plan.SyncPolicy{},
}

assert.NoError(t, ctrl.RunOnce())
assert.NoError(t, ctrl.RunOnce(context.Background()))

// Validate that the mock source was called.
source.AssertExpectations(t)
Expand Down
12 changes: 8 additions & 4 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package main

import (
"context"
"net/http"
"os"
"os/signal"
Expand Down Expand Up @@ -60,6 +61,8 @@ func main() {
}
log.SetLevel(ll)

ctx := context.Background()

stopChan := make(chan struct{}, 1)

go serveMetrics(cfg.MetricsAddress)
Expand Down Expand Up @@ -144,9 +147,9 @@ func main() {
case "rcodezero":
p, err = provider.NewRcodeZeroProvider(domainFilter, cfg.DryRun, cfg.RcodezeroTXTEncrypt)
case "google":
p, err = provider.NewGoogleProvider(cfg.GoogleProject, domainFilter, zoneIDFilter, cfg.GoogleBatchChangeSize, cfg.GoogleBatchChangeInterval, cfg.DryRun)
p, err = provider.NewGoogleProvider(ctx, cfg.GoogleProject, domainFilter, zoneIDFilter, cfg.GoogleBatchChangeSize, cfg.GoogleBatchChangeInterval, cfg.DryRun)
case "digitalocean":
p, err = provider.NewDigitalOceanProvider(domainFilter, cfg.DryRun)
p, err = provider.NewDigitalOceanProvider(ctx, domainFilter, cfg.DryRun)
case "linode":
p, err = provider.NewLinodeProvider(domainFilter, cfg.DryRun, externaldns.Version)
case "dnsimple":
Expand Down Expand Up @@ -197,6 +200,7 @@ func main() {
p, err = provider.NewDesignateProvider(domainFilter, cfg.DryRun)
case "pdns":
p, err = provider.NewPDNSProvider(
ctx,
provider.PDNSConfig{
DomainFilter: domainFilter,
DryRun: cfg.DryRun,
Expand Down Expand Up @@ -266,14 +270,14 @@ func main() {
}

if cfg.Once {
err := ctrl.RunOnce()
err := ctrl.RunOnce(ctx)
if err != nil {
log.Fatal(err)
}

os.Exit(0)
}
ctrl.Run(stopChan)
ctrl.Run(ctx, stopChan)
}

func handleSigterm(stopChan chan struct{}) {
Expand Down
11 changes: 5 additions & 6 deletions provider/cloudflare.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,8 @@ func NewCloudFlareProvider(domainFilter DomainFilter, zoneIDFilter ZoneIDFilter,
}

// Zones returns the list of hosted zones.
func (p *CloudFlareProvider) Zones() ([]cloudflare.Zone, error) {
func (p *CloudFlareProvider) Zones(ctx context.Context) ([]cloudflare.Zone, error) {
result := []cloudflare.Zone{}
ctx := context.TODO()
p.PaginationOptions.Page = 1

for {
Expand Down Expand Up @@ -177,7 +176,7 @@ func (p *CloudFlareProvider) Zones() ([]cloudflare.Zone, error) {

// Records returns the list of records.
func (p *CloudFlareProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
zones, err := p.Zones()
zones, err := p.Zones(ctx)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -208,17 +207,17 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha
combinedChanges = append(combinedChanges, newCloudFlareChanges(cloudFlareUpdate, changes.UpdateNew, proxiedByDefault)...)
combinedChanges = append(combinedChanges, newCloudFlareChanges(cloudFlareDelete, changes.Delete, proxiedByDefault)...)

return p.submitChanges(combinedChanges)
return p.submitChanges(ctx, combinedChanges)
}

// submitChanges takes a zone and a collection of Changes and sends them as a single transaction.
func (p *CloudFlareProvider) submitChanges(changes []*cloudFlareChange) error {
func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloudFlareChange) error {
// return early if there is nothing to change
if len(changes) == 0 {
return nil
}

zones, err := p.Zones()
zones, err := p.Zones(ctx)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion provider/cloudflare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ func TestCloudFlareZones(t *testing.T) {
zoneIDFilter: NewZoneIDFilter([]string{""}),
}

zones, err := provider.Zones()
zones, err := provider.Zones(context.Background())
if err != nil {
t.Fatal(err)
}
Expand Down
34 changes: 17 additions & 17 deletions provider/digital_ocean.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ type DigitalOceanChange struct {
}

// NewDigitalOceanProvider initializes a new DigitalOcean DNS based Provider.
func NewDigitalOceanProvider(domainFilter DomainFilter, dryRun bool) (*DigitalOceanProvider, error) {
func NewDigitalOceanProvider(ctx context.Context, domainFilter DomainFilter, dryRun bool) (*DigitalOceanProvider, error) {
token, ok := os.LookupEnv("DO_TOKEN")
if !ok {
return nil, fmt.Errorf("No token found")
}
oauthClient := oauth2.NewClient(context.TODO(), oauth2.StaticTokenSource(&oauth2.Token{
oauthClient := oauth2.NewClient(ctx, oauth2.StaticTokenSource(&oauth2.Token{
AccessToken: token,
}))
client := godo.NewClient(oauthClient)
Expand All @@ -76,10 +76,10 @@ func NewDigitalOceanProvider(domainFilter DomainFilter, dryRun bool) (*DigitalOc
}

// Zones returns the list of hosted zones.
func (p *DigitalOceanProvider) Zones() ([]godo.Domain, error) {
func (p *DigitalOceanProvider) Zones(ctx context.Context) ([]godo.Domain, error) {
result := []godo.Domain{}

zones, err := p.fetchZones()
zones, err := p.fetchZones(ctx)
if err != nil {
return nil, err
}
Expand All @@ -95,13 +95,13 @@ func (p *DigitalOceanProvider) Zones() ([]godo.Domain, error) {

// Records returns the list of records in a given zone.
func (p *DigitalOceanProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
zones, err := p.Zones()
zones, err := p.Zones(ctx)
if err != nil {
return nil, err
}
endpoints := []*endpoint.Endpoint{}
for _, zone := range zones {
records, err := p.fetchRecords(zone.Name)
records, err := p.fetchRecords(ctx, zone.Name)
if err != nil {
return nil, err
}
Expand All @@ -124,11 +124,11 @@ func (p *DigitalOceanProvider) Records(ctx context.Context) ([]*endpoint.Endpoin
return endpoints, nil
}

func (p *DigitalOceanProvider) fetchRecords(zoneName string) ([]godo.DomainRecord, error) {
func (p *DigitalOceanProvider) fetchRecords(ctx context.Context, zoneName string) ([]godo.DomainRecord, error) {
allRecords := []godo.DomainRecord{}
listOptions := &godo.ListOptions{}
for {
records, resp, err := p.Client.Records(context.TODO(), zoneName, listOptions)
records, resp, err := p.Client.Records(ctx, zoneName, listOptions)
if err != nil {
return nil, err
}
Expand All @@ -149,11 +149,11 @@ func (p *DigitalOceanProvider) fetchRecords(zoneName string) ([]godo.DomainRecor
return allRecords, nil
}

func (p *DigitalOceanProvider) fetchZones() ([]godo.Domain, error) {
func (p *DigitalOceanProvider) fetchZones(ctx context.Context) ([]godo.Domain, error) {
allZones := []godo.Domain{}
listOptions := &godo.ListOptions{}
for {
zones, resp, err := p.Client.List(context.TODO(), listOptions)
zones, resp, err := p.Client.List(ctx, listOptions)
if err != nil {
return nil, err
}
Expand All @@ -175,21 +175,21 @@ func (p *DigitalOceanProvider) fetchZones() ([]godo.Domain, error) {
}

// submitChanges takes a zone and a collection of Changes and sends them as a single transaction.
func (p *DigitalOceanProvider) submitChanges(changes []*DigitalOceanChange) error {
func (p *DigitalOceanProvider) submitChanges(ctx context.Context, changes []*DigitalOceanChange) error {
// return early if there is nothing to change
if len(changes) == 0 {
return nil
}

zones, err := p.Zones()
zones, err := p.Zones(ctx)
if err != nil {
return err
}

// separate into per-zone change sets to be passed to the API.
changesByZone := digitalOceanChangesByZone(zones, changes)
for zoneName, changes := range changesByZone {
records, err := p.fetchRecords(zoneName)
records, err := p.fetchRecords(ctx, zoneName)
if err != nil {
log.Errorf("Failed to list records in the zone: %s", zoneName)
continue
Expand Down Expand Up @@ -225,7 +225,7 @@ func (p *DigitalOceanProvider) submitChanges(changes []*DigitalOceanChange) erro

switch change.Action {
case DigitalOceanCreate:
_, _, err = p.Client.CreateRecord(context.TODO(), zoneName,
_, _, err = p.Client.CreateRecord(ctx, zoneName,
&godo.DomainRecordEditRequest{
Data: change.ResourceRecordSet.Data,
Name: change.ResourceRecordSet.Name,
Expand All @@ -237,13 +237,13 @@ func (p *DigitalOceanProvider) submitChanges(changes []*DigitalOceanChange) erro
}
case DigitalOceanDelete:
recordID := p.getRecordID(records, change.ResourceRecordSet)
_, err = p.Client.DeleteRecord(context.TODO(), zoneName, recordID)
_, err = p.Client.DeleteRecord(ctx, zoneName, recordID)
if err != nil {
return err
}
case DigitalOceanUpdate:
recordID := p.getRecordID(records, change.ResourceRecordSet)
_, _, err = p.Client.EditRecord(context.TODO(), zoneName, recordID,
_, _, err = p.Client.EditRecord(ctx, zoneName, recordID,
&godo.DomainRecordEditRequest{
Data: change.ResourceRecordSet.Data,
Name: change.ResourceRecordSet.Name,
Expand All @@ -267,7 +267,7 @@ func (p *DigitalOceanProvider) ApplyChanges(ctx context.Context, changes *plan.C
combinedChanges = append(combinedChanges, newDigitalOceanChanges(DigitalOceanUpdate, changes.UpdateNew)...)
combinedChanges = append(combinedChanges, newDigitalOceanChanges(DigitalOceanDelete, changes.Delete)...)

return p.submitChanges(combinedChanges)
return p.submitChanges(ctx, combinedChanges)
}

// newDigitalOceanChanges returns a collection of Changes based on the given records and action.
Expand Down
8 changes: 4 additions & 4 deletions provider/digital_ocean_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ func TestDigitalOceanZones(t *testing.T) {
domainFilter: NewDomainFilter([]string{"com"}),
}

zones, err := provider.Zones()
zones, err := provider.Zones(context.Background())
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -445,12 +445,12 @@ func TestDigitalOceanApplyChanges(t *testing.T) {

func TestNewDigitalOceanProvider(t *testing.T) {
_ = os.Setenv("DO_TOKEN", "xxxxxxxxxxxxxxxxx")
_, err := NewDigitalOceanProvider(NewDomainFilter([]string{"ext-dns-test.zalando.to."}), true)
_, err := NewDigitalOceanProvider(context.Background(), NewDomainFilter([]string{"ext-dns-test.zalando.to."}), true)
if err != nil {
t.Errorf("should not fail, %s", err)
}
_ = os.Unsetenv("DO_TOKEN")
_, err = NewDigitalOceanProvider(NewDomainFilter([]string{"ext-dns-test.zalando.to."}), true)
_, err = NewDigitalOceanProvider(context.Background(), NewDomainFilter([]string{"ext-dns-test.zalando.to."}), true)
if err == nil {
t.Errorf("expected to fail")
}
Expand Down Expand Up @@ -494,7 +494,7 @@ func TestDigitalOceanRecord(t *testing.T) {
Client: &mockDigitalOceanClient{},
}

records, err := provider.fetchRecords("example.com")
records, err := provider.fetchRecords(context.Background(), "example.com")
if err != nil {
t.Fatal(err)
}
Expand Down
4 changes: 2 additions & 2 deletions provider/exoscale.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,13 @@ func (ep *ExoscaleProvider) ApplyChanges(ctx context.Context, changes *plan.Chan
func (ep *ExoscaleProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
endpoints := make([]*endpoint.Endpoint, 0)

domains, err := ep.client.GetDomains(context.TODO())
domains, err := ep.client.GetDomains(ctx)
if err != nil {
return nil, err
}

for _, d := range domains {
record, err := ep.client.GetRecords(context.TODO(), d.Name)
record, err := ep.client.GetRecords(ctx, d.Name)
if err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit 4be51a9

Please sign in to comment.