Skip to content

Commit

Permalink
fix dynamic update of the default backend
Browse files Browse the repository at this point in the history
Since v0.11, all cluster updates try to be partially applied in the
in-memory model. This is addressed building an object tracking, followed
by purging the old state and recreating these same objects in the new
state. In short ingress objects track hostnames, services and secrets,
and services track endpoints.

Default backends wasn't being updated because it exposes a service
without using an ingress object, so endpoints are deployed on a full
synchronization, including de first one, and wasn't tracker anymore.
This update creates a dummy tracking point to bind the default backend
endpoints and the haproxy model.

This update also needed to manage an issue with the backend ID: the
default backend is renamed to `_default_backend` when assigned as such,
and changed back when unassiged. This unassignment happens when the
backend is purged in order to have its internal state updated, and such
asynchrony should be fixed in the dynupdate. Now dynupdate compares
backends based on an internal and immutable state.
  • Loading branch information
jcmoraisjr committed Oct 19, 2020
1 parent 0ca1760 commit 727c7db
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 11 deletions.
13 changes: 10 additions & 3 deletions pkg/converters/ingress/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ func NewIngressConverter(options *ingtypes.ConverterOptions, haproxy haproxy.Con
logger: options.Logger,
cache: options.Cache,
tracker: options.Tracker,
defaultBackSource: annotations.Source{Name: "<default-backend>", Type: "ingress"},
mapBuilder: annotations.NewMapBuilder(options.Logger, options.AnnotationPrefix+"/", defaultConfig),
updater: annotations.NewUpdater(haproxy, options),
globalConfig: annotations.NewMapBuilder(options.Logger, "", defaultConfig).NewMapper(),
Expand All @@ -82,6 +83,7 @@ type converter struct {
cache convtypes.Cache
tracker convtypes.Tracker
defaultCrt convtypes.CrtFile
defaultBackSource annotations.Source
mapBuilder *annotations.MapBuilder
updater annotations.Updater
globalConfig *annotations.Mapper
Expand All @@ -95,7 +97,6 @@ func (c *converter) Sync() {
c.haproxy.Clear()
}
c.syncDefaultCrt()
c.syncDefaultBackend()
if c.needFullSync {
c.syncFull()
} else {
Expand Down Expand Up @@ -150,8 +151,9 @@ func (c *converter) syncDefaultCrt() {

func (c *converter) syncDefaultBackend() {
if c.options.DefaultBackend != "" {
if backend, err := c.addBackend(&annotations.Source{}, hatypes.DefaultHost, "/", c.options.DefaultBackend, "", map[string]string{}); err == nil {
if backend, err := c.addBackend(&c.defaultBackSource, hatypes.DefaultHost, "/", c.options.DefaultBackend, "", map[string]string{}); err == nil {
c.haproxy.Backends().SetDefaultBackend(backend)
c.tracker.TrackHostname(convtypes.IngressType, c.defaultBackSource.FullName(), hatypes.DefaultHost)
} else {
c.logger.Error("error reading default service: %v", err)
}
Expand All @@ -165,6 +167,7 @@ func (c *converter) syncFull() {
return
}
sortIngress(ingList)
c.syncDefaultBackend()
for _, ing := range ingList {
c.syncIngress(ing)
}
Expand Down Expand Up @@ -266,7 +269,11 @@ func (c *converter) syncPartial() {
var err error
ing, err = c.cache.GetIngress(name)
if err != nil {
c.logger.Warn("ignoring ingress '%s': %v", name, err)
if name == c.defaultBackSource.FullName() {
c.syncDefaultBackend()
} else {
c.logger.Warn("ignoring ingress '%s': %v", name, err)
}
ing = nil
}
}
Expand Down
26 changes: 25 additions & 1 deletion pkg/converters/ingress/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1214,7 +1214,7 @@ WARN using default certificate due to an error reading secret 'default/tls1' on
{"default/echo1", "echo1:8080"},
},
svcAdd: svcDefault,
logging: `INFO-V(2) syncing 1 host(s) and 0 backend(s)`,
logging: `INFO-V(2) syncing 1 host(s) and 1 backend(s)`,
expDefaultFront: expDefaultFrontDefault,
expBack: `
- id: default_echo1_8080
Expand Down Expand Up @@ -1347,6 +1347,30 @@ WARN using default certificate due to an error reading secret 'default/tls1' on
}
}

func TestSyncPartialDefaultBackend(t *testing.T) {
c := setup(t)
defer c.teardown()

// first config, sync, commit, cleanup
c.Sync()
c.hconfig.Commit()
c.logger.Logging = []string{}

// the mock of the default backend is hardcoded to system/default:8080 at 172.17.0.99
_, ep := conv_helper.CreateService("system/default", "8080", "172.17.0.90")
c.cache.Changed.Endpoints = []*api.Endpoints{ep}
c.Sync()

c.compareConfigFront(`[]`)
c.compareConfigBack(`
- id: _default_backend
endpoints:
- ip: 172.17.0.90
port: 8080
`)
c.logger.CompareLogging(`INFO-V(2) syncing 1 host(s) and 1 backend(s)`)
}

/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
*
* ANNOTATIONS
Expand Down
8 changes: 5 additions & 3 deletions pkg/haproxy/dynupdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,13 @@ func (d *dynUpdater) checkConfigChange() bool {
// return false on new backend which cannot be dynamically created
backends := make(map[string]*backendPair, len(d.config.backends.ItemsDel()))
for _, backend := range d.config.backends.ItemsDel() {
backends[backend.ID] = &backendPair{old: backend}
backends[backend.BackendID().String()] = &backendPair{old: backend}
}
for _, backend := range d.config.backends.ItemsAdd() {
back, found := backends[backend.ID]
id := backend.BackendID().String()
back, found := backends[id]
if !found {
d.logger.InfoV(2, "added backend '%s'", backend.ID)
d.logger.InfoV(2, "added backend '%s'", id)
updated = false
} else {
back.cur = backend
Expand Down Expand Up @@ -133,6 +134,7 @@ func (d *dynUpdater) checkBackendPair(pair *backendPair) bool {

// check equality of everything but endpoints
oldBackCopy := *oldBack
oldBackCopy.ID = curBack.ID
oldBackCopy.Dynamic = curBack.Dynamic
oldBackCopy.Endpoints = curBack.Endpoints
if !reflect.DeepEqual(&oldBackCopy, curBack) {
Expand Down
27 changes: 24 additions & 3 deletions pkg/haproxy/dynupdate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,29 @@ set server default_app_8080/srv002 weight 1`,
},
dynamic: true,
},
// 23
{
doconfig1: func(c *testConfig) {
b := c.config.Backends().AcquireBackend("default", "app", "8080")
c.config.backends.SetDefaultBackend(b)
b.AcquireEndpoint("172.17.0.2", 8080, "")
},
doconfig2: func(c *testConfig) {
b := c.config.Backends().AcquireBackend("default", "app", "8080")
c.config.backends.SetDefaultBackend(b)
b.Dynamic.DynUpdate = true
b.AcquireEndpoint("172.17.0.3", 8080, "")
},
expected: []string{
"srv001:172.17.0.3:8080:1",
},
dynamic: true,
cmd: `
set server _default_backend/srv001 addr 172.17.0.3 port 8080
set server _default_backend/srv001 state ready
set server _default_backend/srv001 weight 1`,
logging: `INFO-V(2) updated endpoint '172.17.0.3:8080' weight '1' state 'ready' on backend/server '_default_backend/srv001'`,
},
}
for i, test := range testCases {
c := setup(t)
Expand All @@ -591,9 +614,7 @@ set server default_app_8080/srv002 weight 1`,
c.instance.config.Commit()
backendIDs := []types.BackendID{}
for _, backend := range c.config.Backends().Items() {
if backend != c.config.Backends().DefaultBackend() {
backendIDs = append(backendIDs, backend.BackendID())
}
backendIDs = append(backendIDs, backend.BackendID())
}
c.config.Backends().RemoveAll(backendIDs)
if test.doconfig2 != nil {
Expand Down
5 changes: 4 additions & 1 deletion pkg/haproxy/types/backends.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@ func (b *Backends) RemoveAll(backendID []BackendID) {
}
b.changedShards[item.shard] = true
b.itemsDel[id] = item
if item == b.defaultBackend {
b.SetDefaultBackend(nil)
}
delete(b.items, id)
}
}
Expand All @@ -254,7 +257,7 @@ func (b *Backends) SetDefaultBackend(defaultBackend *Backend) {

func (b BackendID) String() string {
if b.id == "" {
b.id = b.Namespace + "_" + b.Name + "_" + b.Port
b.id = buildID(b.Namespace, b.Name, b.Port)
}
return b.id
}
Expand Down

0 comments on commit 727c7db

Please sign in to comment.