diff --git a/dns-controller/pkg/dns/dnscontroller.go b/dns-controller/pkg/dns/dnscontroller.go index 4bd74808f35e9..718f9af711809 100644 --- a/dns-controller/pkg/dns/dnscontroller.go +++ b/dns-controller/pkg/dns/dnscontroller.go @@ -17,6 +17,7 @@ limitations under the License. package dns import ( + "context" "fmt" "time" @@ -189,6 +190,8 @@ type recordKey struct { } func (c *DNSController) runOnce() error { + ctx := context.TODO() + snapshot := c.snapshotIfChangedAndReady() if snapshot == nil { // Unchanged / not ready @@ -302,8 +305,12 @@ func (c *DNSController) runOnce() error { } for key, changeset := range op.changesets { + if changeset.IsEmpty() { + continue + } + klog.V(2).Infof("applying DNS changeset for zone %s", key) - if err := changeset.Apply(); err != nil { + if err := changeset.Apply(ctx); err != nil { klog.Warningf("error applying DNS changeset for zone %s: %v", key, err) errors = append(errors, fmt.Errorf("error applying DNS changeset for zone %s: %v", key, err)) } @@ -321,6 +328,8 @@ func (c *DNSController) runOnce() error { } func (c *DNSController) RemoveRecordsImmediate(records []Record) error { + ctx := context.TODO() + op, err := newDNSOp(c.zoneRules, c.dnsCache) if err != nil { return err @@ -344,7 +353,7 @@ func (c *DNSController) RemoveRecordsImmediate(records []Record) error { for key, changeset := range op.changesets { klog.V(2).Infof("applying DNS changeset for zone %s", key) - if err := changeset.Apply(); err != nil { + if err := changeset.Apply(ctx); err != nil { klog.Warningf("error applying DNS changeset for zone %s: %v", key, err) errors = append(errors, fmt.Errorf("error applying DNS changeset for zone %s: %v", key, err)) } diff --git a/dnsprovider/pkg/dnsprovider/dns.go b/dnsprovider/pkg/dnsprovider/dns.go index fbf2925e85cbb..3fa5723c442ae 100644 --- a/dnsprovider/pkg/dnsprovider/dns.go +++ b/dnsprovider/pkg/dnsprovider/dns.go @@ -17,6 +17,7 @@ limitations under the License. package dnsprovider import ( + "context" "reflect" "k8s.io/kops/dnsprovider/pkg/dnsprovider/rrstype" @@ -79,7 +80,8 @@ type ResourceRecordChangeset interface { // If you have the pre-image, it will likely be more efficient to call Remove and Add. Upsert(ResourceRecordSet) ResourceRecordChangeset // Apply applies the accumulated operations to the Zone. - Apply() error + // Implementations should tolerate an empty changeset, and be a relatively quick no-op. + Apply(ctx context.Context) error // IsEmpty returns true if there are no accumulated operations. IsEmpty() bool // ResourceRecordSets returns the parent ResourceRecordSets diff --git a/dnsprovider/pkg/dnsprovider/providers/aws/route53/route53_test.go b/dnsprovider/pkg/dnsprovider/providers/aws/route53/route53_test.go index a85ed50e99e1b..ba165e9bc3c37 100644 --- a/dnsprovider/pkg/dnsprovider/providers/aws/route53/route53_test.go +++ b/dnsprovider/pkg/dnsprovider/providers/aws/route53/route53_test.go @@ -17,6 +17,7 @@ limitations under the License. package route53 import ( + "context" "flag" "fmt" "os" @@ -128,8 +129,8 @@ func getExampleRrs(zone dnsprovider.Zone) dnsprovider.ResourceRecordSet { return rrsets.New("www11."+zone.Name(), []string{"10.10.10.10", "169.20.20.20"}, 180, rrstype.A) } -func addRrsetOrFail(t *testing.T, rrsets dnsprovider.ResourceRecordSets, rrset dnsprovider.ResourceRecordSet) { - err := rrsets.StartChangeset().Add(rrset).Apply() +func addRrsetOrFail(ctx context.Context, t *testing.T, rrsets dnsprovider.ResourceRecordSets, rrset dnsprovider.ResourceRecordSet) { + err := rrsets.StartChangeset().Add(rrset).Apply(ctx) if err != nil { t.Fatalf("Failed to add recordsets: %v", err) } @@ -180,21 +181,25 @@ func TestResourceRecordSetsList(t *testing.T) { /* TestResourceRecordSetsAddSuccess verifies that addition of a valid RRS succeeds */ func TestResourceRecordSetsAddSuccess(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) set := getExampleRrs(zone) - addRrsetOrFail(t, sets, set) - defer sets.StartChangeset().Remove(set).Apply() + addRrsetOrFail(ctx, t, sets, set) + defer sets.StartChangeset().Remove(set).Apply(ctx) t.Logf("Successfully added resource record set: %v", set) } /* TestResourceRecordSetsAdditionVisible verifies that added RRS is visible after addition */ func TestResourceRecordSetsAdditionVisible(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - defer sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Logf("Successfully added resource record set: %v", rrset) found := false for _, record := range listRrsOrFail(t, sets) { @@ -208,18 +213,20 @@ func TestResourceRecordSetsAdditionVisible(t *testing.T) { } } -/* TestResourceRecordSetsAddDuplicateFail verifies that addition of a duplicate RRS fails */ +/* TestResourceRecordSetsAddDuplicateFailure verifies that addition of a duplicate RRS fails */ func TestResourceRecordSetsAddDuplicateFailure(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - defer sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Logf("Successfully added resource record set: %v", rrset) // Try to add it again, and verify that the call fails. - err := sets.StartChangeset().Add(rrset).Apply() + err := sets.StartChangeset().Add(rrset).Apply(ctx) if err == nil { - defer sets.StartChangeset().Remove(rrset).Apply() + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Errorf("Should have failed to add duplicate resource record %v, but succeeded instead.", rrset) } else { t.Logf("Correctly failed to add duplicate resource record %v: %v", rrset, err) @@ -228,14 +235,16 @@ func TestResourceRecordSetsAddDuplicateFailure(t *testing.T) { /* TestResourceRecordSetsRemove verifies that the removal of an existing RRS succeeds */ func TestResourceRecordSetsRemove(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - err := sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + err := sets.StartChangeset().Remove(rrset).Apply(ctx) if err != nil { // Try again to clean up. - defer sets.StartChangeset().Remove(rrset).Apply() + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Errorf("Failed to remove resource record set %v after adding", rrset) } else { t.Logf("Successfully removed resource set %v after adding", rrset) @@ -244,14 +253,16 @@ func TestResourceRecordSetsRemove(t *testing.T) { /* TestResourceRecordSetsRemoveGone verifies that a removed RRS no longer exists */ func TestResourceRecordSetsRemoveGone(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - err := sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + err := sets.StartChangeset().Remove(rrset).Apply(ctx) if err != nil { // Try again to clean up. - defer sets.StartChangeset().Remove(rrset).Apply() + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Errorf("Failed to remove resource record set %v after adding", rrset) } else { t.Logf("Successfully removed resource set %v after adding", rrset) @@ -287,3 +298,11 @@ func TestResourceRecordSetsDifferentTypes(t *testing.T) { zone := firstZone(t) tests.CommonTestResourceRecordSetsDifferentTypes(t, zone) } + +// TestContract verifies the general interface contract +func TestContract(t *testing.T) { + zone := firstZone(t) + sets := rrs(t, zone) + + tests.TestContract(t, sets) +} diff --git a/dnsprovider/pkg/dnsprovider/providers/aws/route53/rrchangeset.go b/dnsprovider/pkg/dnsprovider/providers/aws/route53/rrchangeset.go index 968698440f345..0b34a7b54f18c 100644 --- a/dnsprovider/pkg/dnsprovider/providers/aws/route53/rrchangeset.go +++ b/dnsprovider/pkg/dnsprovider/providers/aws/route53/rrchangeset.go @@ -18,6 +18,7 @@ package route53 import ( "bytes" + "context" "fmt" "github.com/aws/aws-sdk-go/aws" @@ -73,7 +74,12 @@ func buildChange(action string, rrs dnsprovider.ResourceRecordSet) *route53.Chan return change } -func (c *ResourceRecordChangeset) Apply() error { +func (c *ResourceRecordChangeset) Apply(ctx context.Context) error { + // Empty changesets should be a relatively quick no-op + if c.IsEmpty() { + return nil + } + hostedZoneID := c.zone.impl.Id removals := make(map[string]*route53.Change) diff --git a/dnsprovider/pkg/dnsprovider/providers/coredns/coredns_test.go b/dnsprovider/pkg/dnsprovider/providers/coredns/coredns_test.go index fb33610f1ead7..824cb861c7689 100644 --- a/dnsprovider/pkg/dnsprovider/providers/coredns/coredns_test.go +++ b/dnsprovider/pkg/dnsprovider/providers/coredns/coredns_test.go @@ -17,6 +17,7 @@ limitations under the License. package coredns import ( + "context" "flag" "fmt" "os" @@ -125,8 +126,8 @@ func getExampleRrs(zone dnsprovider.Zone) dnsprovider.ResourceRecordSet { return rrsets.New("www11."+zone.Name(), []string{"10.10.10.10", "169.20.20.20"}, 180, rrstype.A) } -func addRrsetOrFail(t *testing.T, rrsets dnsprovider.ResourceRecordSets, rrset dnsprovider.ResourceRecordSet) { - err := rrsets.StartChangeset().Add(rrset).Apply() +func addRrsetOrFail(ctx context.Context, t *testing.T, rrsets dnsprovider.ResourceRecordSets, rrset dnsprovider.ResourceRecordSet) { + err := rrsets.StartChangeset().Add(rrset).Apply(ctx) if err != nil { t.Fatalf("Failed to add recordsets: %v", err) } @@ -154,11 +155,13 @@ func TestResourceRecordSetsGet(t *testing.T) { /* TestResourceRecordSetsAddSuccess verifies that addition of a valid RRS succeeds */ func TestResourceRecordSetsAddSuccess(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) set := getExampleRrs(zone) - addRrsetOrFail(t, sets, set) - defer sets.StartChangeset().Remove(set).Apply() + addRrsetOrFail(ctx, t, sets, set) + defer sets.StartChangeset().Remove(set).Apply(ctx) t.Logf("Successfully added resource record set: %v", set) if sets.Zone().ID() != zone.ID() { t.Errorf("Zone for rrset does not match expected") @@ -167,11 +170,13 @@ func TestResourceRecordSetsAddSuccess(t *testing.T) { /* TestResourceRecordSetsAdditionVisible verifies that added RRS is visible after addition */ func TestResourceRecordSetsAdditionVisible(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - defer sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Logf("Successfully added resource record set: %v", rrset) record := getRrOrFail(t, sets, rrset.Name()) @@ -180,18 +185,20 @@ func TestResourceRecordSetsAdditionVisible(t *testing.T) { } } -/* TestResourceRecordSetsAddDuplicateFail verifies that addition of a duplicate RRS fails */ +/* TestResourceRecordSetsAddDuplicateFailure verifies that addition of a duplicate RRS fails */ func TestResourceRecordSetsAddDuplicateFailure(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - defer sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Logf("Successfully added resource record set: %v", rrset) // Try to add it again, and verify that the call fails. - err := sets.StartChangeset().Add(rrset).Apply() + err := sets.StartChangeset().Add(rrset).Apply(ctx) if err == nil { - defer sets.StartChangeset().Remove(rrset).Apply() + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Errorf("Should have failed to add duplicate resource record %v, but succeeded instead.", rrset) } else { t.Logf("Correctly failed to add duplicate resource record %v: %v", rrset, err) @@ -200,14 +207,16 @@ func TestResourceRecordSetsAddDuplicateFailure(t *testing.T) { /* TestResourceRecordSetsRemove verifies that the removal of an existing RRS succeeds */ func TestResourceRecordSetsRemove(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - err := sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + err := sets.StartChangeset().Remove(rrset).Apply(ctx) if err != nil { // Try again to clean up. - defer sets.StartChangeset().Remove(rrset).Apply() + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Errorf("Failed to remove resource record set %v after adding", rrset) } else { t.Logf("Successfully removed resource set %v after adding", rrset) @@ -216,14 +225,16 @@ func TestResourceRecordSetsRemove(t *testing.T) { /* TestResourceRecordSetsRemoveGone verifies that a removed RRS no longer exists */ func TestResourceRecordSetsRemoveGone(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - err := sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + err := sets.StartChangeset().Remove(rrset).Apply(ctx) if err != nil { // Try again to clean up. - defer sets.StartChangeset().Remove(rrset).Apply() + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Errorf("Failed to remove resource record set %v after adding", rrset) } else { t.Logf("Successfully removed resource set %v after adding", rrset) @@ -252,3 +263,11 @@ func TestResourceRecordSetsDifferentTypes(t *testing.T) { zone := firstZone(t) tests.CommonTestResourceRecordSetsDifferentTypes(t, zone) } + +// TestContract verifies the general interface contract +func TestContract(t *testing.T) { + zone := firstZone(t) + sets := rrs(t, zone) + + tests.TestContract(t, sets) +} diff --git a/dnsprovider/pkg/dnsprovider/providers/coredns/rrchangeset.go b/dnsprovider/pkg/dnsprovider/providers/coredns/rrchangeset.go index 60902024bc083..b5f490c6b84f6 100644 --- a/dnsprovider/pkg/dnsprovider/providers/coredns/rrchangeset.go +++ b/dnsprovider/pkg/dnsprovider/providers/coredns/rrchangeset.go @@ -69,8 +69,12 @@ func (c *ResourceRecordChangeset) Upsert(rrset dnsprovider.ResourceRecordSet) dn return c } -func (c *ResourceRecordChangeset) Apply() error { - ctx := context.Background() +func (c *ResourceRecordChangeset) Apply(ctx context.Context) error { + // Empty changesets should be a relatively quick no-op + if c.IsEmpty() { + return nil + } + etcdPathPrefix := c.zone.zones.intf.etcdPathPrefix getOpts := &etcdc.GetOptions{} setOpts := &etcdc.SetOptions{} diff --git a/dnsprovider/pkg/dnsprovider/providers/google/clouddns/clouddns_test.go b/dnsprovider/pkg/dnsprovider/providers/google/clouddns/clouddns_test.go index 17e84d027fdbe..c834083f4a0f4 100644 --- a/dnsprovider/pkg/dnsprovider/providers/google/clouddns/clouddns_test.go +++ b/dnsprovider/pkg/dnsprovider/providers/google/clouddns/clouddns_test.go @@ -17,6 +17,7 @@ limitations under the License. package clouddns import ( + "context" "flag" "fmt" "os" @@ -106,8 +107,8 @@ func getExampleRrs(zone dnsprovider.Zone) dnsprovider.ResourceRecordSet { return rrsets.New("www11."+zone.Name(), []string{"10.10.10.10", "169.20.20.20"}, 180, rrstype.A) } -func addRrsetOrFail(t *testing.T, rrsets dnsprovider.ResourceRecordSets, rrset dnsprovider.ResourceRecordSet) { - err := rrsets.StartChangeset().Add(rrset).Apply() +func addRrsetOrFail(ctx context.Context, t *testing.T, rrsets dnsprovider.ResourceRecordSets, rrset dnsprovider.ResourceRecordSet) { + err := rrsets.StartChangeset().Add(rrset).Apply(ctx) if err != nil { t.Fatalf("Failed to add recordsets: %v", err) } @@ -159,21 +160,23 @@ func TestResourceRecordSetsList(t *testing.T) { /* TestResourceRecordSetsAddSuccess verifies that addition of a valid RRS succeeds */ func TestResourceRecordSetsAddSuccess(t *testing.T) { + ctx := context.Background() zone := firstZone(t) sets := rrs(t, zone) set := getExampleRrs(zone) - addRrsetOrFail(t, sets, set) - defer sets.StartChangeset().Remove(set).Apply() + addRrsetOrFail(ctx, t, sets, set) + defer sets.StartChangeset().Remove(set).Apply(ctx) t.Logf("Successfully added resource record set: %v", set) } /* TestResourceRecordSetsAdditionVisible verifies that added RRS is visible after addition */ func TestResourceRecordSetsAdditionVisible(t *testing.T) { + ctx := context.Background() zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - defer sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Logf("Successfully added resource record set: %v", rrset) found := false for _, record := range listRrsOrFail(t, sets) { @@ -187,18 +190,20 @@ func TestResourceRecordSetsAdditionVisible(t *testing.T) { } } -/* TestResourceRecordSetsAddDuplicateFail verifies that addition of a duplicate RRS fails */ +/* TestResourceRecordSetsAddDuplicateFailure verifies that addition of a duplicate RRS fails */ func TestResourceRecordSetsAddDuplicateFailure(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - defer sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Logf("Successfully added resource record set: %v", rrset) // Try to add it again, and verify that the call fails. - err := sets.StartChangeset().Add(rrset).Apply() + err := sets.StartChangeset().Add(rrset).Apply(ctx) if err == nil { - defer sets.StartChangeset().Remove(rrset).Apply() + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Errorf("Should have failed to add duplicate resource record %v, but succeeded instead.", rrset) } else { t.Logf("Correctly failed to add duplicate resource record %v: %v", rrset, err) @@ -207,14 +212,16 @@ func TestResourceRecordSetsAddDuplicateFailure(t *testing.T) { /* TestResourceRecordSetsRemove verifies that the removal of an existing RRS succeeds */ func TestResourceRecordSetsRemove(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - err := sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + err := sets.StartChangeset().Remove(rrset).Apply(ctx) if err != nil { // Try again to clean up. - defer sets.StartChangeset().Remove(rrset).Apply() + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Errorf("Failed to remove resource record set %v after adding: %v", rrset, err) } else { t.Logf("Successfully removed resource set %v after adding", rrset) @@ -223,14 +230,16 @@ func TestResourceRecordSetsRemove(t *testing.T) { /* TestResourceRecordSetsRemoveGone verifies that a removed RRS no longer exists */ func TestResourceRecordSetsRemoveGone(t *testing.T) { + ctx := context.Background() + zone := firstZone(t) sets := rrs(t, zone) rrset := getExampleRrs(zone) - addRrsetOrFail(t, sets, rrset) - err := sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + err := sets.StartChangeset().Remove(rrset).Apply(ctx) if err != nil { // Try again to clean up. - defer sets.StartChangeset().Remove(rrset).Apply() + defer sets.StartChangeset().Remove(rrset).Apply(ctx) t.Errorf("Failed to remove resource record set %v after adding: %v", rrset, err) } else { t.Logf("Successfully removed resource set %v after adding", rrset) @@ -266,3 +275,11 @@ func TestResourceRecordSetsDifferentTypes(t *testing.T) { zone := firstZone(t) tests.CommonTestResourceRecordSetsDifferentTypes(t, zone) } + +// TestContract verifies the general interface contract +func TestContract(t *testing.T) { + zone := firstZone(t) + sets := rrs(t, zone) + + tests.TestContract(t, sets) +} diff --git a/dnsprovider/pkg/dnsprovider/providers/google/clouddns/rrchangeset.go b/dnsprovider/pkg/dnsprovider/providers/google/clouddns/rrchangeset.go index 6b6d95e5ad98a..2bb243477bc3f 100644 --- a/dnsprovider/pkg/dnsprovider/providers/google/clouddns/rrchangeset.go +++ b/dnsprovider/pkg/dnsprovider/providers/google/clouddns/rrchangeset.go @@ -17,6 +17,7 @@ limitations under the License. package clouddns import ( + "context" "fmt" "k8s.io/kops/dnsprovider/pkg/dnsprovider" @@ -49,7 +50,12 @@ func (c *ResourceRecordChangeset) Upsert(rrset dnsprovider.ResourceRecordSet) dn return c } -func (c *ResourceRecordChangeset) Apply() error { +func (c *ResourceRecordChangeset) Apply(ctx context.Context) error { + // Empty changesets should be a relatively quick no-op + if c.IsEmpty() { + return nil + } + rrsets := c.rrsets service := rrsets.zone.zones.interface_.service.Changes() @@ -115,7 +121,7 @@ func (c *ResourceRecordChangeset) Apply() error { } func (c *ResourceRecordChangeset) IsEmpty() bool { - return len(c.additions) == 0 && len(c.removals) == 0 + return len(c.additions) == 0 && len(c.removals) == 0 && len(c.upserts) == 0 } // ResourceRecordSets returns the parent ResourceRecordSets diff --git a/dnsprovider/pkg/dnsprovider/providers/openstack/designate/BUILD.bazel b/dnsprovider/pkg/dnsprovider/providers/openstack/designate/BUILD.bazel index 44ed7d83d8ccc..b88f90a04b0c6 100644 --- a/dnsprovider/pkg/dnsprovider/providers/openstack/designate/BUILD.bazel +++ b/dnsprovider/pkg/dnsprovider/providers/openstack/designate/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -25,3 +25,10 @@ go_library( "//vendor/k8s.io/klog:go_default_library", ], ) + +go_test( + name = "go_default_test", + srcs = ["designate_test.go"], + embed = [":go_default_library"], + deps = ["//dnsprovider/pkg/dnsprovider/tests:go_default_library"], +) diff --git a/dnsprovider/pkg/dnsprovider/providers/openstack/designate/designate_test.go b/dnsprovider/pkg/dnsprovider/providers/openstack/designate/designate_test.go new file mode 100644 index 0000000000000..8fb4a3bd1c83a --- /dev/null +++ b/dnsprovider/pkg/dnsprovider/providers/openstack/designate/designate_test.go @@ -0,0 +1,30 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package designate + +import ( + "testing" + + "k8s.io/kops/dnsprovider/pkg/dnsprovider/tests" +) + +// TestContract verifies the general interface contract +func TestContract(t *testing.T) { + sets := &ResourceRecordSets{} + + tests.TestContract(t, sets) +} diff --git a/dnsprovider/pkg/dnsprovider/providers/openstack/designate/rrchangeset.go b/dnsprovider/pkg/dnsprovider/providers/openstack/designate/rrchangeset.go index b533ca0ea6646..58d91b7c0dcbc 100644 --- a/dnsprovider/pkg/dnsprovider/providers/openstack/designate/rrchangeset.go +++ b/dnsprovider/pkg/dnsprovider/providers/openstack/designate/rrchangeset.go @@ -17,6 +17,7 @@ limitations under the License. package designate import ( + "context" "fmt" "github.com/gophercloud/gophercloud/openstack/dns/v2/recordsets" @@ -50,7 +51,12 @@ func (c *ResourceRecordChangeset) Upsert(rrset dnsprovider.ResourceRecordSet) dn return c } -func (c *ResourceRecordChangeset) Apply() error { +func (c *ResourceRecordChangeset) Apply(ctx context.Context) error { + // Empty changesets should be a relatively quick no-op + if c.IsEmpty() { + return nil + } + zoneID := c.zone.impl.ID for _, removal := range c.removals { diff --git a/dnsprovider/pkg/dnsprovider/tests/BUILD.bazel b/dnsprovider/pkg/dnsprovider/tests/BUILD.bazel index c89a65ce54462..16440c50e373c 100644 --- a/dnsprovider/pkg/dnsprovider/tests/BUILD.bazel +++ b/dnsprovider/pkg/dnsprovider/tests/BUILD.bazel @@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", - srcs = ["commontests.go"], + srcs = [ + "commontests.go", + "contract.go", + ], importpath = "k8s.io/kops/dnsprovider/pkg/dnsprovider/tests", visibility = ["//visibility:public"], deps = [ diff --git a/dnsprovider/pkg/dnsprovider/tests/commontests.go b/dnsprovider/pkg/dnsprovider/tests/commontests.go index 555f552235f99..c0babf49c9d01 100644 --- a/dnsprovider/pkg/dnsprovider/tests/commontests.go +++ b/dnsprovider/pkg/dnsprovider/tests/commontests.go @@ -17,6 +17,7 @@ limitations under the License. package tests import ( + "context" "reflect" "testing" @@ -26,20 +27,21 @@ import ( /* CommonTestResourceRecordSetsReplace verifies that replacing an RRS works */ func CommonTestResourceRecordSetsReplace(t *testing.T, zone dnsprovider.Zone) { + ctx := context.Background() rrsets, _ := zone.ResourceRecordSets() sets := rrs(t, zone) rrset := rrsets.New("alpha.test.com", []string{"8.8.4.4"}, 40, rrstype.A) - addRrsetOrFail(t, sets, rrset) - defer sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + defer sets.StartChangeset().Remove(rrset).Apply(ctx) // Replace the record (change ttl and rrdatas) newRrset := rrsets.New("alpha.test.com", []string{"8.8.8.8"}, 80, rrstype.A) - err := sets.StartChangeset().Add(newRrset).Remove(rrset).Apply() + err := sets.StartChangeset().Add(newRrset).Remove(rrset).Apply(ctx) if err != nil { t.Errorf("Failed to replace resource record set %v -> %v: %v", rrset, newRrset, err) } else { - defer sets.StartChangeset().Remove(newRrset).Apply() + defer sets.StartChangeset().Remove(newRrset).Apply(ctx) t.Logf("Correctly replaced resource record %v -> %v", rrset, newRrset) } @@ -49,21 +51,22 @@ func CommonTestResourceRecordSetsReplace(t *testing.T, zone dnsprovider.Zone) { /* CommonTestResourceRecordSetsReplaceAll verifies that we can remove an RRS and create one with a different name*/ func CommonTestResourceRecordSetsReplaceAll(t *testing.T, zone dnsprovider.Zone) { + ctx := context.Background() rrsets, _ := zone.ResourceRecordSets() sets := rrs(t, zone) rrset := rrsets.New("alpha.test.com", []string{"8.8.4.4"}, 40, rrstype.A) - addRrsetOrFail(t, sets, rrset) - defer sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + defer sets.StartChangeset().Remove(rrset).Apply(ctx) newRrset := rrsets.New("beta.test.com", []string{"8.8.8.8"}, 80, rrstype.A) // Try to add it again, and verify that the call fails. - err := sets.StartChangeset().Add(newRrset).Remove(rrset).Apply() + err := sets.StartChangeset().Add(newRrset).Remove(rrset).Apply(ctx) if err != nil { t.Errorf("Failed to replace resource record set %v -> %v: %v", rrset, newRrset, err) } else { - defer sets.StartChangeset().Remove(newRrset).Apply() + defer sets.StartChangeset().Remove(newRrset).Apply(ctx) t.Logf("Correctly replaced resource record %v -> %v", rrset, newRrset) } @@ -74,21 +77,22 @@ func CommonTestResourceRecordSetsReplaceAll(t *testing.T, zone dnsprovider.Zone) /* CommonTestResourceRecordSetsDifferentType verifies that we can add records of the same name but different types */ func CommonTestResourceRecordSetsDifferentTypes(t *testing.T, zone dnsprovider.Zone) { + ctx := context.Background() rrsets, _ := zone.ResourceRecordSets() sets := rrs(t, zone) rrset := rrsets.New("alpha.test.com", []string{"8.8.4.4"}, 40, rrstype.A) - addRrsetOrFail(t, sets, rrset) - defer sets.StartChangeset().Remove(rrset).Apply() + addRrsetOrFail(ctx, t, sets, rrset) + defer sets.StartChangeset().Remove(rrset).Apply(ctx) aaaaRrset := rrsets.New("alpha.test.com", []string{"2001:4860:4860::8888"}, 80, rrstype.AAAA) // Add the resource with the same name but different type - err := sets.StartChangeset().Add(aaaaRrset).Apply() + err := sets.StartChangeset().Add(aaaaRrset).Apply(ctx) if err != nil { t.Errorf("Failed to add resource record set %v: %v", aaaaRrset, err) } - defer sets.StartChangeset().Remove(aaaaRrset).Apply() + defer sets.StartChangeset().Remove(aaaaRrset).Apply(ctx) // Check that both records exist assertHasRecord(t, sets, aaaaRrset) @@ -184,8 +188,8 @@ func assertEquivalent(t *testing.T, l, r dnsprovider.ResourceRecordSet) { } } -func addRrsetOrFail(t *testing.T, rrsets dnsprovider.ResourceRecordSets, rrset dnsprovider.ResourceRecordSet) { - err := rrsets.StartChangeset().Add(rrset).Apply() +func addRrsetOrFail(ctx context.Context, t *testing.T, rrsets dnsprovider.ResourceRecordSets, rrset dnsprovider.ResourceRecordSet) { + err := rrsets.StartChangeset().Add(rrset).Apply(ctx) if err != nil { t.Fatalf("Failed to add recordset %v: %v", rrset, err) } else { diff --git a/dnsprovider/pkg/dnsprovider/tests/contract.go b/dnsprovider/pkg/dnsprovider/tests/contract.go new file mode 100644 index 0000000000000..113421a43d6f2 --- /dev/null +++ b/dnsprovider/pkg/dnsprovider/tests/contract.go @@ -0,0 +1,68 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tests + +import ( + "testing" + + "k8s.io/kops/dnsprovider/pkg/dnsprovider" + "k8s.io/kops/dnsprovider/pkg/dnsprovider/rrstype" +) + +// TestContract verifies the general ResourceRecordChangeset contract +func TestContract(t *testing.T, rrsets dnsprovider.ResourceRecordSets) { + + { + changeset := rrsets.StartChangeset() + if !changeset.IsEmpty() { + t.Fatalf("expected new changeset to be empty") + } + + rrs := changeset.ResourceRecordSets().New("foo", []string{"192.168.0.1"}, 1, rrstype.A) + changeset.Add(rrs) + if changeset.IsEmpty() { + t.Fatalf("expected changeset not to be empty after add") + } + } + + { + changeset := rrsets.StartChangeset() + if !changeset.IsEmpty() { + t.Fatalf("expected new changeset to be empty") + } + + rrs := changeset.ResourceRecordSets().New("foo", []string{"192.168.0.1"}, 1, rrstype.A) + changeset.Remove(rrs) + if changeset.IsEmpty() { + t.Fatalf("expected changeset not to be empty after remove") + } + } + + { + changeset := rrsets.StartChangeset() + if !changeset.IsEmpty() { + t.Fatalf("expected new changeset to be empty") + } + + rrs := changeset.ResourceRecordSets().New("foo", []string{"192.168.0.1"}, 1, rrstype.A) + changeset.Upsert(rrs) + if changeset.IsEmpty() { + t.Fatalf("expected changeset not to be empty after upsert") + } + } + +} diff --git a/pkg/resources/digitalocean/dns/dns.go b/pkg/resources/digitalocean/dns/dns.go index 1439d0cf91bec..980de0b8e929d 100644 --- a/pkg/resources/digitalocean/dns/dns.go +++ b/pkg/resources/digitalocean/dns/dns.go @@ -313,13 +313,15 @@ func (r *resourceRecordChangeset) Upsert(rrset dnsprovider.ResourceRecordSet) dn // Apply adds new records stored in r.additions, updates records stored // in r.upserts and deletes records stored in r.removals -func (r *resourceRecordChangeset) Apply() error { - klog.V(2).Info("applying changes in record change set") +func (r *resourceRecordChangeset) Apply(ctx context.Context) error { + // Empty changesets should be a relatively quick no-op if r.IsEmpty() { - klog.V(2).Info("record change set is empty") + klog.V(4).Info("record change set is empty") return nil } + klog.V(2).Info("applying changes in record change set") + if len(r.additions) > 0 { for _, rrset := range r.additions { err := r.applyResourceRecordSet(rrset) diff --git a/pkg/resources/digitalocean/dns/dns_test.go b/pkg/resources/digitalocean/dns/dns_test.go index 40b5aa1489747..a7fc68998ef5a 100644 --- a/pkg/resources/digitalocean/dns/dns_test.go +++ b/pkg/resources/digitalocean/dns/dns_test.go @@ -369,6 +369,8 @@ func TestNewResourceRecordSet(t *testing.T) { } func TestResourceRecordChangeset(t *testing.T) { + ctx := context.Background() + fake := &fakeDomainService{} fake.recordsFunc = func(ctx context.Context, domain string, listOpts *godo.ListOptions) ([]godo.DomainRecord, *godo.Response, error) { domainRecords := []godo.DomainRecord{ @@ -456,7 +458,7 @@ func TestResourceRecordChangeset(t *testing.T) { record = rrset.New("to-upsert", []string{"127.0.0.1"}, 3600, rrstype.A) changeset.Upsert(record) - err = changeset.Apply() + err = changeset.Apply(ctx) if err != nil { t.Errorf("error applying changeset: %v", err) } diff --git a/protokube/pkg/gossip/dns/provider/changeset.go b/protokube/pkg/gossip/dns/provider/changeset.go index afd0baf2888f5..f5ac1cd9944bc 100644 --- a/protokube/pkg/gossip/dns/provider/changeset.go +++ b/protokube/pkg/gossip/dns/provider/changeset.go @@ -17,6 +17,8 @@ limitations under the License. package provider import ( + "context" + "k8s.io/kops/dnsprovider/pkg/dnsprovider" ) @@ -52,7 +54,12 @@ func (c *resourceRecordChangeset) Upsert(rrs dnsprovider.ResourceRecordSet) dnsp } // Apply applies the accumulated operations to the Zone. -func (c *resourceRecordChangeset) Apply() error { +func (c *resourceRecordChangeset) Apply(ctx context.Context) error { + // Empty changesets should be a relatively quick no-op + if c.IsEmpty() { + return nil + } + return c.zone.applyChangeset(c) } diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go index d361789e49091..1e7e218ff4498 100644 --- a/upup/pkg/fi/cloudup/apply_cluster.go +++ b/upup/pkg/fi/cloudup/apply_cluster.go @@ -960,7 +960,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error { } if shouldPrecreateDNS { - if err := precreateDNS(cluster, cloud); err != nil { + if err := precreateDNS(ctx, cluster, cloud); err != nil { klog.Warningf("unable to pre-create DNS records - cluster startup may be slower: %v", err) } } diff --git a/upup/pkg/fi/cloudup/dns.go b/upup/pkg/fi/cloudup/dns.go index 45e88723f22fd..adb2b83ab362b 100644 --- a/upup/pkg/fi/cloudup/dns.go +++ b/upup/pkg/fi/cloudup/dns.go @@ -17,6 +17,7 @@ limitations under the License. package cloudup import ( + "context" "fmt" "net" "os" @@ -124,7 +125,7 @@ func validateDNS(cluster *kops.Cluster, cloud fi.Cloud) error { return nil } -func precreateDNS(cluster *kops.Cluster, cloud fi.Cloud) error { +func precreateDNS(ctx context.Context, cluster *kops.Cluster, cloud fi.Cloud) error { // TODO: Move to update if !featureflag.DNSPreCreate.Enabled() { klog.V(4).Infof("Skipping DNS record pre-creation because feature flag not enabled") @@ -240,7 +241,7 @@ func precreateDNS(cluster *kops.Cluster, cloud fi.Cloud) error { } if len(created) != 0 { - err := changeset.Apply() + err := changeset.Apply(ctx) if err != nil { return fmt.Errorf("error pre-creating DNS records: %v", err) }