From 9a44453d592bc08bbef944433e7bbee7278cdd49 Mon Sep 17 00:00:00 2001 From: Yerken Date: Fri, 7 Apr 2017 16:00:24 +0200 Subject: [PATCH] define registry interface (#120) * define registry interface * init in-memory registry * remove ununsed variable, added comments * add inmemory registry tests * introduce DNSRecord struct * use noop registry * remove zone from registry fields * replace provider with registry in controller * move noop registry interface check to test * remove ownerid from noop registry * fix: remove dangling empty line * return provider records directly with noop * adjust according to pr review * fix noop tests --- controller/controller.go | 8 +- controller/controller_test.go | 5 +- main.go | 8 +- registry/noop.go | 45 ++++++++++ registry/noop_test.go | 150 ++++++++++++++++++++++++++++++++++ registry/registry.go | 31 +++++++ 6 files changed, 241 insertions(+), 6 deletions(-) create mode 100644 registry/noop.go create mode 100644 registry/noop_test.go create mode 100644 registry/registry.go diff --git a/controller/controller.go b/controller/controller.go index d153503a5..caf5042bf 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -22,7 +22,7 @@ import ( log "github.com/Sirupsen/logrus" "github.com/kubernetes-incubator/external-dns/plan" - "github.com/kubernetes-incubator/external-dns/provider" + "github.com/kubernetes-incubator/external-dns/registry" "github.com/kubernetes-incubator/external-dns/source" ) @@ -36,14 +36,14 @@ type Controller struct { Zone string Source source.Source - Provider provider.Provider + Registry registry.Registry // The interval between individual synchronizations Interval time.Duration } // RunOnce runs a single iteration of a reconciliation loop. func (c *Controller) RunOnce() error { - records, err := c.Provider.Records(c.Zone) + records, err := c.Registry.Records(c.Zone) if err != nil { return err } @@ -60,7 +60,7 @@ func (c *Controller) RunOnce() error { plan = plan.Calculate() - return c.Provider.ApplyChanges(c.Zone, &plan.Changes) + return c.Registry.ApplyChanges(c.Zone, &plan.Changes) } // Run runs RunOnce in a loop with a delay until stopChan receives a value. diff --git a/controller/controller_test.go b/controller/controller_test.go index 34e944e21..b4b95ecf0 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -23,6 +23,7 @@ import ( "github.com/kubernetes-incubator/external-dns/endpoint" "github.com/kubernetes-incubator/external-dns/plan" "github.com/kubernetes-incubator/external-dns/provider" + "github.com/kubernetes-incubator/external-dns/registry" "github.com/kubernetes-incubator/external-dns/source" ) @@ -131,11 +132,13 @@ func TestRunOnce(t *testing.T) { }, ) + r, _ := registry.NewNoopRegistry(provider) + // Run our controller once to trigger the validation. ctrl := &Controller{ Zone: "test-zone", Source: source, - Provider: provider, + Registry: r, } err := ctrl.RunOnce() diff --git a/main.go b/main.go index 7b31d1dba..d71012bc7 100644 --- a/main.go +++ b/main.go @@ -34,6 +34,7 @@ import ( "github.com/kubernetes-incubator/external-dns/pkg/apis/externaldns" "github.com/kubernetes-incubator/external-dns/pkg/apis/externaldns/validation" "github.com/kubernetes-incubator/external-dns/provider" + "github.com/kubernetes-incubator/external-dns/registry" "github.com/kubernetes-incubator/external-dns/source" ) @@ -93,10 +94,15 @@ func main() { log.Fatal(err) } + r, err := registry.NewNoopRegistry(p) + if err != nil { + log.Fatal(err) + } + ctrl := controller.Controller{ Zone: cfg.Zone, Source: sources, - Provider: p, + Registry: r, Interval: cfg.Interval, } diff --git a/registry/noop.go b/registry/noop.go new file mode 100644 index 000000000..e33700f8c --- /dev/null +++ b/registry/noop.go @@ -0,0 +1,45 @@ +/* +Copyright 2017 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 registry + +import ( + "github.com/kubernetes-incubator/external-dns/endpoint" + "github.com/kubernetes-incubator/external-dns/plan" + "github.com/kubernetes-incubator/external-dns/provider" +) + +// NoopRegistry implements registry interface without ownership directly propagating changes to dns provider +type NoopRegistry struct { + provider provider.Provider +} + +// NewNoopRegistry returns new NoopRegistry object +func NewNoopRegistry(provider provider.Provider) (*NoopRegistry, error) { + return &NoopRegistry{ + provider: provider, + }, nil +} + +// Records returns the current records from the dns provider +func (im *NoopRegistry) Records(zone string) ([]*endpoint.Endpoint, error) { + return im.provider.Records(zone) +} + +// ApplyChanges propagates changes to the dns provider +func (im *NoopRegistry) ApplyChanges(zone string, changes *plan.Changes) error { + return im.provider.ApplyChanges(zone, changes) +} diff --git a/registry/noop_test.go b/registry/noop_test.go new file mode 100644 index 000000000..70bba1678 --- /dev/null +++ b/registry/noop_test.go @@ -0,0 +1,150 @@ +/* +Copyright 2017 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 registry + +import ( + "testing" + + "github.com/kubernetes-incubator/external-dns/endpoint" + "github.com/kubernetes-incubator/external-dns/internal/testutils" + "github.com/kubernetes-incubator/external-dns/plan" + "github.com/kubernetes-incubator/external-dns/provider" +) + +var _ Registry = &NoopRegistry{} + +func TestNoopRegistry(t *testing.T) { + t.Run("NewNoopRegistry", testNoopInit) + t.Run("Records", testNoopRecords) + t.Run("ApplyChanges", testNoopApplyChanges) +} + +func testNoopInit(t *testing.T) { + p := provider.NewInMemoryProvider() + r, err := NewNoopRegistry(p) + if err != nil { + t.Fatal(err) + } + if r.provider != p { + t.Error("noop registry incorrectly initialized") + } +} + +func testNoopRecords(t *testing.T) { + p := provider.NewInMemoryProvider() + p.CreateZone("zone") + providerRecords := []*endpoint.Endpoint{ + { + DNSName: "example.org", + Target: "example-lb.com", + }, + } + p.ApplyChanges("zone", &plan.Changes{ + Create: providerRecords, + }) + + r, _ := NewNoopRegistry(p) + _, err := r.Records("wrong-zone") + if err == nil { + t.Error("Should fail for wrong zone: wrong-zone") + } + + r, _ = NewNoopRegistry(p) + eps, err := r.Records("zone") + if err != nil { + t.Error(err) + } + if !testutils.SameEndpoints(eps, providerRecords) { + t.Error("incorrect result is returned") + } +} + +func testNoopApplyChanges(t *testing.T) { + // do some prep + p := provider.NewInMemoryProvider() + p.CreateZone("zone") + providerRecords := []*endpoint.Endpoint{ + { + DNSName: "example.org", + Target: "8.8.8.8", + }, + } + expectedUpdate := []*endpoint.Endpoint{ + { + DNSName: "example.org", + Target: "new-example-lb.com", + }, + { + DNSName: "new-record.org", + Target: "new-lb.org", + }, + } + + p.ApplyChanges("zone", &plan.Changes{ + Create: providerRecords, + }) + + // wrong zone + r, _ := NewNoopRegistry(p) + err := r.ApplyChanges("wrong-zone", &plan.Changes{}) + if err != provider.ErrZoneNotFound { + t.Error("should return zone not found for apply changes on wrong zone") + } + + // wrong changes + r, _ = NewNoopRegistry(p) + err = r.ApplyChanges("zone", &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Target: "lb.com", + }, + }, + }) + if err != provider.ErrRecordAlreadyExists { + t.Error("should return record already exists") + } + + //correct changes + err = r.ApplyChanges("zone", &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + DNSName: "new-record.org", + Target: "new-lb.org", + }, + }, + UpdateNew: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Target: "new-example-lb.com", + }, + }, + UpdateOld: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Target: "8.8.8.8", + }, + }, + }) + if err != nil { + t.Fatal(err) + } + res, _ := p.Records("zone") + if !testutils.SameEndpoints(res, expectedUpdate) { + t.Error("incorrectly updated dns provider") + } +} diff --git a/registry/registry.go b/registry/registry.go new file mode 100644 index 000000000..b30e52ed1 --- /dev/null +++ b/registry/registry.go @@ -0,0 +1,31 @@ +/* +Copyright 2017 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 registry + +import ( + "github.com/kubernetes-incubator/external-dns/endpoint" + "github.com/kubernetes-incubator/external-dns/plan" +) + +// Registry is an interface which should enables ownership concept in external-dns +// Record(zone string) returns ALL records registered with DNS provider (TODO: for multi-zone support return all records) +// each entry includes owner information +// ApplyChanges(zone string, changes *plan.Changes) propagates the changes to the DNS Provider API and correspondingly updates ownership depending on type of registry being used +type Registry interface { + Records(zone string) ([]*endpoint.Endpoint, error) + ApplyChanges(zone string, changes *plan.Changes) error +}