Skip to content

Commit

Permalink
Multi target plan ( Page Not Found ) (kubernetes-sigs#404)
Browse files Browse the repository at this point in the history
* Make suitableType() be Endpoint method

With this change it becomes possible to work with endpoint
of empty type in packages other than "provider". Also
it seems logical for a smart property getter without side effects
to be a method rather than a function in different package

* Make plan computation work correctly with multi-target domains

* fix drawing

* drop comments

* fix boilerplate header

* fix comment

* fix the bug with empty map

* rework registry to support random lables

*  serialize->serializeLabel function rename

* golint for err variable naming

* add additional test

* add tests for current case where one resource can generate multiple endpoints

* make labels have its own type, add serialization as a method

* add comment for exported error

* use greater rather than not equal zero

* update changelog
  • Loading branch information
Yerken authored and linki committed Dec 14, 2017
1 parent 874ec1a commit 2ce9625
Show file tree
Hide file tree
Showing 17 changed files with 980 additions and 261 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
- Every record managed by External DNS is now mapped to a kubernetes resource (service/ingress) @ideahitme
- New field is stored in TXT DNS record which reflects which kubernetes resource has acquired the DNS name
- Target of DNS record is changed only if corresponding kubernetes resource target changes
- If kubernetes resource is deleted, then another resource may acquire DNS name
- "Flapping" target issue is resolved by providing a consistent and defined mechanism for choosing a target

## v0.4.8 - 2017-11-22

- Allow filtering by source annotation via `--annotation-filter` (#354) @khrisrichardson
Expand Down
15 changes: 2 additions & 13 deletions endpoint/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (
)

const (
// OwnerLabelKey is the name of the label that defines the owner of an Endpoint.
OwnerLabelKey = "owner"
// RecordTypeA is a RecordType enum value
RecordTypeA = "A"
// RecordTypeCNAME is a RecordType enum value
Expand Down Expand Up @@ -51,7 +49,7 @@ type Endpoint struct {
// TTL for the record
RecordTTL TTL
// Labels stores labels defined for the Endpoint
Labels map[string]string
Labels Labels
}

// NewEndpoint initialization method to be used to create an endpoint
Expand All @@ -65,20 +63,11 @@ func NewEndpointWithTTL(dnsName, target, recordType string, ttl TTL) *Endpoint {
DNSName: strings.TrimSuffix(dnsName, "."),
Target: strings.TrimSuffix(target, "."),
RecordType: recordType,
Labels: map[string]string{},
Labels: NewLabels(),
RecordTTL: ttl,
}
}

// MergeLabels adds keys to labels if not defined for the endpoint
func (e *Endpoint) MergeLabels(labels map[string]string) {
for k, v := range labels {
if e.Labels[k] == "" {
e.Labels[k] = v
}
}
}

func (e *Endpoint) String() string {
return fmt.Sprintf("%s %d IN %s %s", e.DNSName, e.RecordTTL, e.RecordType, e.Target)
}
12 changes: 0 additions & 12 deletions endpoint/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package endpoint

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestNewEndpoint(t *testing.T) {
Expand All @@ -36,13 +34,3 @@ func TestNewEndpoint(t *testing.T) {
t.Error("endpoint is not initialized correctly")
}
}

func TestMergeLabels(t *testing.T) {
e := NewEndpoint("abc.com", "1.2.3.4", "A")
e.Labels = map[string]string{
"foo": "bar",
"baz": "qux",
}
e.MergeLabels(map[string]string{"baz": "baz", "new": "fox"})
assert.Equal(t, map[string]string{"foo": "bar", "baz": "qux", "new": "fox"}, e.Labels)
}
99 changes: 99 additions & 0 deletions endpoint/labels.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
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 endpoint

import (
"errors"
"fmt"
"sort"
"strings"
)

var (
// ErrInvalidHeritage is returned when heritage was not found, or different heritage is found
ErrInvalidHeritage = errors.New("heritage is unknown or not found")
)

const (
heritage = "external-dns"
// OwnerLabelKey is the name of the label that defines the owner of an Endpoint.
OwnerLabelKey = "owner"
// ResourceLabelKey is the name of the label that identifies k8s resource which wants to acquire the DNS name
ResourceLabelKey = "resource"
)

// Labels store metadata related to the endpoint
// it is then stored in a persistent storage via serialization
type Labels map[string]string

// NewLabels returns empty Labels
func NewLabels() Labels {
return map[string]string{}
}

// NewLabelsFromString constructs endpoints labels from a provided format string
// if heritage set to another value is found then error is returned
// no heritage automatically assumes is not owned by external-dns and returns invalidHeritage error
func NewLabelsFromString(labelText string) (Labels, error) {
endpointLabels := map[string]string{}
labelText = strings.Trim(labelText, "\"") // drop quotes
tokens := strings.Split(labelText, ",")
foundExternalDNSHeritage := false
for _, token := range tokens {
if len(strings.Split(token, "=")) != 2 {
continue
}
key := strings.Split(token, "=")[0]
val := strings.Split(token, "=")[1]
if key == "heritage" && val != heritage {
return nil, ErrInvalidHeritage
}
if key == "heritage" {
foundExternalDNSHeritage = true
continue
}
if strings.HasPrefix(key, heritage) {
endpointLabels[strings.TrimPrefix(key, heritage+"/")] = val
}
}

if !foundExternalDNSHeritage {
return nil, ErrInvalidHeritage
}

return endpointLabels, nil
}

// Serialize transforms endpoints labels into a external-dns recognizable format string
// withQuotes adds additional quotes
func (l Labels) Serialize(withQuotes bool) string {
var tokens []string
tokens = append(tokens, fmt.Sprintf("heritage=%s", heritage))
var keys []string
for key := range l {
keys = append(keys, key)
}
sort.Strings(keys) // sort for consistency

for _, key := range keys {
tokens = append(tokens, fmt.Sprintf("%s/%s=%s", heritage, key, l[key]))
}
if withQuotes {
return fmt.Sprintf("\"%s\"", strings.Join(tokens, ","))
}
return strings.Join(tokens, ",")
}
92 changes: 92 additions & 0 deletions endpoint/labels_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
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 endpoint

import (
"fmt"
"testing"

"github.com/stretchr/testify/suite"
)

type LabelsSuite struct {
suite.Suite
foo Labels
fooAsText string
fooAsTextWithQuotes string
barText string
barTextAsMap Labels
noHeritageText string
noHeritageAsMap Labels
wrongHeritageText string
multipleHeritageText string //considered invalid
}

func (suite *LabelsSuite) SetupTest() {
suite.foo = map[string]string{
"owner": "foo-owner",
"resource": "foo-resource",
}
suite.fooAsText = "heritage=external-dns,external-dns/owner=foo-owner,external-dns/resource=foo-resource"
suite.fooAsTextWithQuotes = fmt.Sprintf(`"%s"`, suite.fooAsText)

suite.barTextAsMap = map[string]string{
"owner": "bar-owner",
"resource": "bar-resource",
"new-key": "bar-new-key",
}
suite.barText = "heritage=external-dns,,external-dns/owner=bar-owner,external-dns/resource=bar-resource,external-dns/new-key=bar-new-key,random=stuff,no-equal-sign,," //also has some random gibberish

suite.noHeritageText = "external-dns/owner=random-owner"
suite.wrongHeritageText = "heritage=mate,external-dns/owner=random-owner"
suite.multipleHeritageText = "heritage=mate,heritage=external-dns,external-dns/owner=random-owner"
}

func (suite *LabelsSuite) TestSerialize() {
suite.Equal(suite.fooAsText, suite.foo.Serialize(false), "should serializeLabel")
suite.Equal(suite.fooAsTextWithQuotes, suite.foo.Serialize(true), "should serializeLabel")
}

func (suite *LabelsSuite) TestDeserialize() {
foo, err := NewLabelsFromString(suite.fooAsText)
suite.NoError(err, "should succeed for valid label text")
suite.Equal(suite.foo, foo, "should reconstruct original label map")

foo, err = NewLabelsFromString(suite.fooAsTextWithQuotes)
suite.NoError(err, "should succeed for valid label text")
suite.Equal(suite.foo, foo, "should reconstruct original label map")

bar, err := NewLabelsFromString(suite.barText)
suite.NoError(err, "should succeed for valid label text")
suite.Equal(suite.barTextAsMap, bar, "should reconstruct original label map")

noHeritage, err := NewLabelsFromString(suite.noHeritageText)
suite.Equal(ErrInvalidHeritage, err, "should fail if no heritage is found")
suite.Nil(noHeritage, "should return nil")

wrongHeritage, err := NewLabelsFromString(suite.wrongHeritageText)
suite.Equal(ErrInvalidHeritage, err, "should fail if wrong heritage is found")
suite.Nil(wrongHeritage, "if error should return nil")

multipleHeritage, err := NewLabelsFromString(suite.multipleHeritageText)
suite.Equal(ErrInvalidHeritage, err, "should fail if multiple heritage is found")
suite.Nil(multipleHeritage, "if error should return nil")
}

func TestLabels(t *testing.T) {
suite.Run(t, new(LabelsSuite))
}
10 changes: 7 additions & 3 deletions internal/testutils/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ limitations under the License.

package testutils

import "github.com/kubernetes-incubator/external-dns/endpoint"
import "sort"
import (
"sort"

"github.com/kubernetes-incubator/external-dns/endpoint"
)

/** test utility functions for endpoints verifications */

Expand Down Expand Up @@ -45,7 +48,8 @@ func (b byAllFields) Less(i, j int) bool {
// considers example.org. and example.org DNSName/Target as different endpoints
func SameEndpoint(a, b *endpoint.Endpoint) bool {
return a.DNSName == b.DNSName && a.Target == b.Target && a.RecordType == b.RecordType &&
a.Labels[endpoint.OwnerLabelKey] == b.Labels[endpoint.OwnerLabelKey] && a.RecordTTL == b.RecordTTL
a.Labels[endpoint.OwnerLabelKey] == b.Labels[endpoint.OwnerLabelKey] && a.RecordTTL == b.RecordTTL &&
a.Labels[endpoint.ResourceLabelKey] == b.Labels[endpoint.ResourceLabelKey]
}

// SameEndpoints compares two slices of endpoints regardless of order
Expand Down
70 changes: 70 additions & 0 deletions plan/conflict.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
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 plan

import (
"sort"

"github.com/kubernetes-incubator/external-dns/endpoint"
)

// ConflictResolver is used to make a decision in case of two or more different kubernetes resources
// are trying to acquire same DNS name
type ConflictResolver interface {
ResolveCreate(candidates []*endpoint.Endpoint) *endpoint.Endpoint
ResolveUpdate(current *endpoint.Endpoint, candidates []*endpoint.Endpoint) *endpoint.Endpoint
}

// PerResource allows only one resource to own a given dns name
type PerResource struct{}

// ResolveCreate is invoked when dns name is not owned by any resource
// ResolveCreate takes "minimal" (string comparison of Target) endpoint to acquire the DNS record
func (s PerResource) ResolveCreate(candidates []*endpoint.Endpoint) *endpoint.Endpoint {
var min *endpoint.Endpoint
for _, ep := range candidates {
if min == nil || s.less(ep, min) {
min = ep
}
}
return min
}

// ResolveUpdate is invoked when dns name is already owned by "current" endpoint
// ResolveUpdate uses "current" record as base and updates it accordingly with new version of same resource
// if it doesn't exist then pick min
func (s PerResource) ResolveUpdate(current *endpoint.Endpoint, candidates []*endpoint.Endpoint) *endpoint.Endpoint {
currentResource := current.Labels[endpoint.ResourceLabelKey] // resource which has already acquired the DNS
// TODO: sort candidates only needed because we can still have two endpoints from same resource here. We sort for consistency
// TODO: remove once single endpoint can have multiple targets
sort.SliceStable(candidates, func(i, j int) bool {
return s.less(candidates[i], candidates[j])
})
for _, ep := range candidates {
if ep.Labels[endpoint.ResourceLabelKey] == currentResource {
return ep
}
}
return s.ResolveCreate(candidates)
}

// less returns true if endpoint x is less than y
func (s PerResource) less(x, y *endpoint.Endpoint) bool {
return x.Target < y.Target
}

// TODO: with cross-resource/cross-cluster setup alternative variations of ConflictResolver can be used
Loading

0 comments on commit 2ce9625

Please sign in to comment.