Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rename DNSProvider to just Provider #101

Merged
merged 1 commit into from
Mar 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import (

log "github.com/Sirupsen/logrus"

"github.com/kubernetes-incubator/external-dns/dnsprovider"
"github.com/kubernetes-incubator/external-dns/plan"
"github.com/kubernetes-incubator/external-dns/provider"
"github.com/kubernetes-incubator/external-dns/source"
)

Expand All @@ -35,13 +35,13 @@ import (
type Controller struct {
Zone string

Source source.Source
DNSProvider dnsprovider.DNSProvider
Source source.Source
Provider provider.Provider
}

// RunOnce runs a single iteration of a reconciliation loop.
func (c *Controller) RunOnce() error {
records, err := c.DNSProvider.Records(c.Zone)
records, err := c.Provider.Records(c.Zone)
if err != nil {
return err
}
Expand All @@ -58,7 +58,7 @@ func (c *Controller) RunOnce() error {

plan = plan.Calculate()

return c.DNSProvider.ApplyChanges(c.Zone, &plan.Changes)
return c.Provider.ApplyChanges(c.Zone, &plan.Changes)
}

// Run runs RunOnce in a loop with a delay until stopChan receives a value.
Expand Down
24 changes: 12 additions & 12 deletions controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,26 @@ import (
"errors"
"testing"

"github.com/kubernetes-incubator/external-dns/dnsprovider"
"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/source"
)

// mockDNSProvider returns mock endpoints and validates changes.
type mockDNSProvider struct {
// mockProvider returns mock endpoints and validates changes.
type mockProvider struct {
RecordsStore []endpoint.Endpoint
ExpectZone string
ExpectChanges *plan.Changes
}

// Records returns the desired mock endpoints.
func (p *mockDNSProvider) Records(zone string) ([]endpoint.Endpoint, error) {
func (p *mockProvider) Records(zone string) ([]endpoint.Endpoint, error) {
return p.RecordsStore, nil
}

// ApplyChanges validates that the passed in changes satisfy the assumtions.
func (p *mockDNSProvider) ApplyChanges(zone string, changes *plan.Changes) error {
func (p *mockProvider) ApplyChanges(zone string, changes *plan.Changes) error {
if zone != p.ExpectZone {
return errors.New("zone is incorrect")
}
Expand Down Expand Up @@ -75,9 +75,9 @@ func (p *mockDNSProvider) ApplyChanges(zone string, changes *plan.Changes) error
return nil
}

// newMockDNSProvider creates a new mockDNSProvider returning the given endpoints and validating the desired changes.
func newMockDNSProvider(endpoints []endpoint.Endpoint, zone string, changes *plan.Changes) dnsprovider.DNSProvider {
dnsProvider := &mockDNSProvider{
// newMockProvider creates a new mockProvider returning the given endpoints and validating the desired changes.
func newMockProvider(endpoints []endpoint.Endpoint, zone string, changes *plan.Changes) provider.Provider {
dnsProvider := &mockProvider{
RecordsStore: endpoints,
ExpectZone: zone,
ExpectChanges: changes,
Expand All @@ -103,7 +103,7 @@ func TestRunOnce(t *testing.T) {
)

// Fake some existing records in our DNS provider and validate some desired changes.
provider := newMockDNSProvider(
provider := newMockProvider(
[]endpoint.Endpoint{
{
DNSName: "update-record",
Expand Down Expand Up @@ -133,9 +133,9 @@ func TestRunOnce(t *testing.T) {

// Run our controller once to trigger the validation.
ctrl := &Controller{
Zone: "test-zone",
Source: source,
DNSProvider: provider,
Zone: "test-zone",
Source: source,
Provider: provider,
}

err := ctrl.RunOnce()
Expand Down
2 changes: 1 addition & 1 deletion docs/legacy/kops-dns-controller.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ The DNS record mappings try to "do the right thing", but what this means is diff
We consult the `Status.LoadBalancer.Ingress` records on the ingress. For each one, we create a record.
If the record is an IP address, we add an A record. If the record is a hostname (AWS ELB), we use a CNAME.

We would like to use an ALIAS, but we have not yet done this because of limitations of the dnsprovider.
We would like to use an ALIAS, but we have not yet done this because of limitations of the DNS provider.

### Pods

Expand Down
2 changes: 1 addition & 1 deletion docs/project-structure.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ external-dns --in-cluster=false --dnsprovider=aws --source=ingress --source=serv
aws.go
google.go
fake.go
dnsprovider.go - interface
provider.go - interface
./pkg/apis/externaldns - types that we will want to be subject to apimachinery (e.g. configuration)
./pkg/apis/externaldns/validation - validation for our types
./source/ - list of sources
Expand Down
38 changes: 19 additions & 19 deletions docs/proposal/storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,22 @@ Initial discussion - https://github.com/kubernetes-incubator/external-dns/issues

## Purpose

One should not be afraid to use external-dns, because it can delete or overwrite the records preexisting in the DNS provider.
One should not be afraid to use external-dns, because it can delete or overwrite the records preexisting in the DNS provider.

**Why we need it?**

DNS provider (AWS Route53, Google DNS, etc.) stores dns records which are created via various means. Integration of External-DNS should be safe and should not delete or overwrite the records which it is not responsible for. Moreover, it should certainly be possible for multiple kubernetes clusters to share the same hosted zone within the dns provider, additionally multiple external-dns instances inside the same cluster should be able to co-exist without messing with the same set of records.
DNS provider (AWS Route53, Google DNS, etc.) stores dns records which are created via various means. Integration of External-DNS should be safe and should not delete or overwrite the records which it is not responsible for. Moreover, it should certainly be possible for multiple kubernetes clusters to share the same hosted zone within the dns provider, additionally multiple external-dns instances inside the same cluster should be able to co-exist without messing with the same set of records.

Storage provides a persistent storage with information to track the records created by the external-dns.

This proposal introduces multiple possible implementation with the details depending on the setup.
This proposal introduces multiple possible implementation with the details depending on the setup.

## Requirements and assumptions

1. Pre-existing records should not be modified by external-dns
2. External-dns instance only creates/modifies/deletes records which are created by this instance
3. It should be possible to transfer the ownership to another external-dns instance
4. Any integrated dns-provider should provide at least a single way to implement the storage
4. Any integrated DNS provider should provide at least a single way to implement the storage
5. Lifetime of the records should not be limited to lifetime of external-dns
6. External-dns should have its identifier for marking the managed records - **`storage-id`**

Expand All @@ -31,7 +31,7 @@ The following presents two ways to implement the storage, and we are planning to

This implementation idea is borrowed from [Mate](https://github.com/zalando-incubator/mate/)

Each record created by external-dns is accompanied by the TXT record, which internally stores the external-dns identifier. For example, if external dns with `storage-id="external-dns-1"` record to be created with dns name `foo.zone.org`, external-dns will create a TXT record with the same dns name `foo.zone.org` and injected value of `"external-dns-1"`. The transfer of ownership can be done by modifying the value of the TXT record. If no TXT record exists for the record or the value does not match its own `storage-id`, then external-dns will simply ignore it.
Each record created by external-dns is accompanied by the TXT record, which internally stores the external-dns identifier. For example, if external dns with `storage-id="external-dns-1"` record to be created with dns name `foo.zone.org`, external-dns will create a TXT record with the same dns name `foo.zone.org` and injected value of `"external-dns-1"`. The transfer of ownership can be done by modifying the value of the TXT record. If no TXT record exists for the record or the value does not match its own `storage-id`, then external-dns will simply ignore it.


#### Goods
Expand All @@ -47,7 +47,7 @@ Each record created by external-dns is accompanied by the TXT record, which inte

### ConfigMap

Store the state in the configmap. ConfigMap is created and managed by each external-dns individually, i.e. external-dns with **`storage-id=external-dns-1`** will create and operate on `extern-dns-1-storage` ConfigMap. ConfigMap will store **all** the records present in the DNS provider as serialized JSON. For example:
Store the state in the configmap. ConfigMap is created and managed by each external-dns individually, i.e. external-dns with **`storage-id=external-dns-1`** will create and operate on `extern-dns-1-storage` ConfigMap. ConfigMap will store **all** the records present in the DNS provider as serialized JSON. For example:

```
kind: ConfigMap
Expand Down Expand Up @@ -93,35 +93,35 @@ ConfigMap will be periodically resynced with the dns provider by fetching the dn

## Component integration

Components:
Components:
* Source - all endpoints ( collection of ingress, service[type=LoadBalancer] etc.)
* [Plan](https://github.com/kubernetes-incubator/external-dns/issues/13) - object responsible for the create of change lists in external-dns
* DNSProvider - interface to access the DNS provider API
* [Plan](https://github.com/kubernetes-incubator/external-dns/issues/13) - object responsible for the create of change lists in external-dns
* Provider - interface to access the DNS provider API

A single loop iteration of external-dns operation:
A single loop iteration of external-dns operation:

1. Get all endpoints ( collection ingress, service[type=LoadBalancer] etc.) into collection of `endpoints`
2. Get storage `Records()`
1. Get all endpoints ( collection ingress, service[type=LoadBalancer] etc.) into collection of `endpoints`
2. Get storage `Records()`
3. Pass `Records` (including ownership information) and list of endpoints to `Plan` to do the calculation
4. Make a call to DNS provider with `Plan` provided change list
5. If call succeeded pass the change list to storage `Assign()` to mark the records that are created
5. If call succeeded pass the change list to storage `Assign()` to mark the records that are created

Storage gets updated all the time via `Poll`. `Plan` does not call DNS provider directly. Good value of the `Poll` is to have simple rate limiting mechanism on DNS provider.

#### Notes:

1. DNS Provider should use batch operations
2. DNS Provider should be called with CREATE operation (not UPSERT!) when the record does not yet exist!
3. Storage does not need to be in complete sync with DNS provider due to #2. Hence resolving the potential caveats of ConfigMap implementation
2. DNS Provider should be called with CREATE operation (not UPSERT!) when the record does not yet exist!
3. Storage does not need to be in complete sync with DNS provider due to #2. Hence resolving the potential caveats of ConfigMap implementation


## Implementation

Basic implementation of the storage interface:
Basic implementation of the storage interface:

1. Storage has the dnsprovider object to retrieve the list of records, but it never makes the call to modify the records (think layer to help out with endpoint filtering)
2. Record() - returns whatever is stored in the storage
3. Assign(endpoints) - called when the records are registered with dns provider - hence storage need to mark its ownership. **Therefore DNSProvider serves as a safe-guard from race conditions**
3. Assign(endpoints) - called when the records are registered with dns provider - hence storage need to mark its ownership. **Therefore Provider serves as a safe-guard from race conditions**
4. WaitForSync() - called in the beginning to populate the storage, in case of configmap would be the configmap creation and fetching the dns provider records
5. Poll() - resync loop to stay-up-to-date with dns provider state

Expand All @@ -132,9 +132,9 @@ We will provide `InMemoryStorage` non-persistent storage which should help us wi

```
type InMemoryStorage struct {
registry dnsprovider.DNSProvider
registry provider.Provider
zone string
owner string
owner string
cache map[string]*SharedEndpoint
sync.Mutex
}
Expand Down
2 changes: 1 addition & 1 deletion docs/tutorials/gke.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ spec:
- --zone=external-dns-test-gcp-zalan-do
- --source=service
- --source=ingress
- --dns-provider=google
- --provider=google
- --google-project=zalando-external-dns-test
- --dry-run=false
```
Expand Down
18 changes: 9 additions & 9 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ import (
"k8s.io/client-go/tools/clientcmd"

"github.com/kubernetes-incubator/external-dns/controller"
"github.com/kubernetes-incubator/external-dns/dnsprovider"
"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/source"
)

Expand Down Expand Up @@ -79,23 +79,23 @@ func main() {

sources := source.NewMultiSource(source.LookupMultiple(cfg.Sources...)...)

var provider dnsprovider.DNSProvider
switch cfg.DNSProvider {
var p provider.Provider
switch cfg.Provider {
case "google":
provider, err = dnsprovider.NewGoogleProvider(cfg.GoogleProject, cfg.DryRun)
p, err = provider.NewGoogleProvider(cfg.GoogleProject, cfg.DryRun)
case "aws":
provider, err = dnsprovider.NewAWSProvider(cfg.DryRun)
p, err = provider.NewAWSProvider(cfg.DryRun)
default:
log.Fatalf("unknown dns provider: %s", cfg.DNSProvider)
log.Fatalf("unknown dns provider: %s", cfg.Provider)
}
if err != nil {
log.Fatal(err)
}

ctrl := controller.Controller{
Zone: cfg.Zone,
Source: sources,
DNSProvider: provider,
Zone: cfg.Zone,
Source: sources,
Provider: p,
}

if cfg.Once {
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/externaldns/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type Config struct {
Namespace string
Zone string
Sources []string
DNSProvider string
Provider string
GoogleProject string
HealthPort string
Once bool
Expand All @@ -57,7 +57,7 @@ func (cfg *Config) ParseFlags(args []string) error {
flags.StringVar(&cfg.Namespace, "namespace", v1.NamespaceAll, "the namespace to look for endpoints; all namespaces by default")
flags.StringVar(&cfg.Zone, "zone", "", "the ID of the hosted zone to target")
flags.StringArrayVar(&cfg.Sources, "source", nil, "the sources to gather endpoints from")
flags.StringVar(&cfg.DNSProvider, "dns-provider", "", "the DNS provider to materialize the records in")
flags.StringVar(&cfg.Provider, "provider", "", "the DNS provider to materialize the records in")
flags.StringVar(&cfg.GoogleProject, "google-project", "", "gcloud project to target")
flags.StringVar(&cfg.HealthPort, "health-port", defaultHealthPort, "health port to listen on")
flags.StringVar(&cfg.LogFormat, "log-format", defaultLogFormat, "log format output. options: [\"text\", \"json\"]")
Expand Down
12 changes: 6 additions & 6 deletions pkg/apis/externaldns/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestParseFlags(t *testing.T) {
Namespace: "",
Zone: "",
Sources: nil,
DNSProvider: "",
Provider: "",
GoogleProject: "",
HealthPort: defaultHealthPort,
DryRun: true,
Expand All @@ -55,7 +55,7 @@ func TestParseFlags(t *testing.T) {
Namespace: "",
Zone: "",
Sources: nil,
DNSProvider: "",
Provider: "",
GoogleProject: "",
HealthPort: defaultHealthPort,
DryRun: true,
Expand All @@ -73,7 +73,7 @@ func TestParseFlags(t *testing.T) {
Namespace: "",
Zone: "",
Sources: nil,
DNSProvider: "",
Provider: "",
GoogleProject: "",
HealthPort: defaultHealthPort,
DryRun: true,
Expand All @@ -96,7 +96,7 @@ func TestParseFlags(t *testing.T) {
Namespace: "",
Zone: "",
Sources: nil,
DNSProvider: "",
Provider: "",
GoogleProject: "",
HealthPort: defaultHealthPort,
DryRun: true,
Expand All @@ -113,7 +113,7 @@ func TestParseFlags(t *testing.T) {
"--namespace", "namespace",
"--zone", "zone",
"--source", "source",
"--dns-provider", "provider",
"--provider", "provider",
"--google-project", "project",
"--health-port", "1234",
"--dry-run", "true",
Expand All @@ -125,7 +125,7 @@ func TestParseFlags(t *testing.T) {
Namespace: "namespace",
Zone: "zone",
Sources: []string{"source"},
DNSProvider: "provider",
Provider: "provider",
GoogleProject: "project",
HealthPort: "1234",
DryRun: true,
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/externaldns/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func ValidateConfig(cfg *externaldns.Config) error {
if len(cfg.Sources) == 0 {
return errors.New("no sources specified")
}
if cfg.DNSProvider == "" {
if cfg.Provider == "" {
return errors.New("no provider specified")
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/externaldns/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestValidateFlags(t *testing.T) {
}

cfg = newValidConfig(t)
cfg.DNSProvider = ""
cfg.Provider = ""
if err := ValidateConfig(cfg); err == nil {
t.Error("missing provider should fail")
}
Expand All @@ -68,7 +68,7 @@ func newValidConfig(t *testing.T) *externaldns.Config {
cfg.LogFormat = "json"
cfg.Zone = "test-zone"
cfg.Sources = []string{"test-source"}
cfg.DNSProvider = "test-provider"
cfg.Provider = "test-provider"

if err := ValidateConfig(cfg); err != nil {
t.Fatalf("newValidConfig should return valid config")
Expand Down
Loading