From 727c7db0d16cfa61dd80a91ba2ed237f87d38ef6 Mon Sep 17 00:00:00 2001 From: Joao Morais Date: Mon, 19 Oct 2020 15:41:01 -0300 Subject: [PATCH] fix dynamic update of the default backend 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. --- pkg/converters/ingress/ingress.go | 13 ++++++++++--- pkg/converters/ingress/ingress_test.go | 26 ++++++++++++++++++++++++- pkg/haproxy/dynupdate.go | 8 +++++--- pkg/haproxy/dynupdate_test.go | 27 +++++++++++++++++++++++--- pkg/haproxy/types/backends.go | 5 ++++- 5 files changed, 68 insertions(+), 11 deletions(-) diff --git a/pkg/converters/ingress/ingress.go b/pkg/converters/ingress/ingress.go index 301fa20cf..6d71ebeb2 100644 --- a/pkg/converters/ingress/ingress.go +++ b/pkg/converters/ingress/ingress.go @@ -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: "", Type: "ingress"}, mapBuilder: annotations.NewMapBuilder(options.Logger, options.AnnotationPrefix+"/", defaultConfig), updater: annotations.NewUpdater(haproxy, options), globalConfig: annotations.NewMapBuilder(options.Logger, "", defaultConfig).NewMapper(), @@ -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 @@ -95,7 +97,6 @@ func (c *converter) Sync() { c.haproxy.Clear() } c.syncDefaultCrt() - c.syncDefaultBackend() if c.needFullSync { c.syncFull() } else { @@ -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) } @@ -165,6 +167,7 @@ func (c *converter) syncFull() { return } sortIngress(ingList) + c.syncDefaultBackend() for _, ing := range ingList { c.syncIngress(ing) } @@ -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 } } diff --git a/pkg/converters/ingress/ingress_test.go b/pkg/converters/ingress/ingress_test.go index ef7143ff0..399066277 100644 --- a/pkg/converters/ingress/ingress_test.go +++ b/pkg/converters/ingress/ingress_test.go @@ -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 @@ -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 diff --git a/pkg/haproxy/dynupdate.go b/pkg/haproxy/dynupdate.go index 65ea7ff7c..8a1714127 100644 --- a/pkg/haproxy/dynupdate.go +++ b/pkg/haproxy/dynupdate.go @@ -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 @@ -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) { diff --git a/pkg/haproxy/dynupdate_test.go b/pkg/haproxy/dynupdate_test.go index b23c78cf5..18ac72f2d 100644 --- a/pkg/haproxy/dynupdate_test.go +++ b/pkg/haproxy/dynupdate_test.go @@ -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) @@ -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 { diff --git a/pkg/haproxy/types/backends.go b/pkg/haproxy/types/backends.go index c7bbac9a2..12165cf14 100644 --- a/pkg/haproxy/types/backends.go +++ b/pkg/haproxy/types/backends.go @@ -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) } } @@ -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 }