Merge pull request #630 from jcmoraisjr/jm-shrink-config

shrink list of changed hosts and backends

pkg/haproxy/config.go

@@ -40,6 +40,7 @@ type Config interface {
 	Backends() *hatypes.Backends
 	Userlists() *hatypes.Userlists
 	Clear()
+	Shrink()
 	Commit()
 }
 
@@ -360,6 +361,11 @@ func (c *config) Clear() {
 	*c = *config
 }
 
+func (c *config) Shrink() {
+	c.hosts.Shrink()
+	c.backends.Shrink()
+}
+
 func (c *config) Commit() {
 	if !reflect.DeepEqual(c.globalOld, c.global) {
 		// globals still uses the old deepCopy+fullParsing+deepEqual strategy

pkg/haproxy/instance.go

@@ -21,6 +21,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -238,6 +239,7 @@ func (i *instance) haproxyUpdate(timer *utils.Timer) {
 	//
 	defer i.config.Commit()
 	i.config.SyncConfig()
+	i.config.Shrink()
 	if err := i.config.WriteFrontendMaps(); err != nil {
 		i.logger.Error("error building frontend maps: %v", err)
 		i.metrics.IncUpdateNoop()
@@ -248,9 +250,13 @@ func (i *instance) haproxyUpdate(timer *utils.Timer) {
 		i.metrics.IncUpdateNoop()
 		return
 	}
+	timer.Tick("write_maps")
+	if i.options.HAProxyCmd != "" {
+		// TODO update tests and remove `if cmd!=""` above
+		i.logChanged()
+	}
 	updater := i.newDynUpdater()
 	updated := updater.update()
-	timer.Tick("write_maps")
 	if !updated || updater.cmdCnt > 0 {
 		// only need to rewrtite config files if:
 		//   - !updated           - there are changes that cannot be dynamically applied
@@ -294,6 +300,43 @@ func (i *instance) haproxyUpdate(timer *utils.Timer) {
 	timer.Tick("reload_haproxy")
 }
 
+func (i *instance) logChanged() {
+	hostsAdd := i.config.Hosts().ItemsAdd()
+	if len(hostsAdd) < 100 {
+		hostsDel := i.config.Hosts().ItemsDel()
+		hosts := make([]string, 0, len(hostsAdd))
+		for host := range hostsAdd {
+			hosts = append(hosts, host)
+		}
+		for host := range hostsDel {
+			if _, found := hostsAdd[host]; !found {
+				hosts = append(hosts, host)
+			}
+		}
+		sort.Strings(hosts)
+		i.logger.InfoV(2, "updating %d host(s): %v", len(hosts), hosts)
+	} else {
+		i.logger.InfoV(2, "updating %d hosts", len(hostsAdd))
+	}
+	backsAdd := i.config.Backends().ItemsAdd()
+	if len(backsAdd) < 100 {
+		backsDel := i.config.Backends().ItemsDel()
+		backs := make([]string, 0, len(backsAdd))
+		for back := range backsAdd {
+			backs = append(backs, back)
+		}
+		for back := range backsDel {
+			if _, found := backsAdd[back]; !found {
+				backs = append(backs, back)
+			}
+		}
+		sort.Strings(backs)
+		i.logger.InfoV(2, "updating %d backend(s): %v", len(backs), backs)
+	} else {
+		i.logger.InfoV(2, "updating %d backends", len(backsAdd))
+	}
+}
+
 func (i *instance) writeConfig() (err error) {
 	//
 	// modsec template execution

pkg/haproxy/types/backends.go

@@ -18,6 +18,7 @@ package types
 
 import (
 	"crypto/md5"
+	"reflect"
 	"sort"
 )
 
@@ -51,6 +52,74 @@ func (b *Backends) ItemsDel() map[string]*Backend {
 	return b.itemsDel
 }
 
+// Shrink compares deleted and added backends with the same name - ie changed
+// objects - and remove both from the changing hashmap tracker when they match.
+func (b *Backends) Shrink() {
+	changed := false
+	for name, del := range b.itemsDel {
+		if add, found := b.itemsAdd[name]; found {
+			if backendsMatch(add, del) {
+				// Such changed backend, when removed from the tracking, need to
+				// be reincluded into the current state hashmap `items` and also
+				// into its shard hashmap when backend sharding is enabled.
+				if len(b.shards) > 0 {
+					b.shards[del.shard][del.ID] = del
+				}
+				b.items[name] = del
+				delete(b.itemsAdd, name)
+				delete(b.itemsDel, name)
+				changed = true
+			}
+		}
+	}
+	// Backends removed from the changing tracker might clean a shard state if it
+	// was the only one changed into the shard. Recalc changedShards if anything
+	// was changed.
+	if changed {
+		b.changedShards = map[int]bool{}
+		for _, back := range b.itemsAdd {
+			b.changedShards[back.shard] = true
+		}
+		for _, back := range b.itemsDel {
+			b.changedShards[back.shard] = true
+		}
+	}
+}
+
+// backendsMatch returns true if two backends match. This comparison
+// ignores empty endpoints and its order and it's cheaper than leave
+// the backend dirty.
+func backendsMatch(back1, back2 *Backend) bool {
+	if reflect.DeepEqual(back1, back2) {
+		return true
+	}
+	b1copy := *back1
+	b1copy.Endpoints = back2.Endpoints
+	if !reflect.DeepEqual(&b1copy, back2) {
+		return false
+	}
+	epmap := make(map[Endpoint]bool, len(back1.Endpoints))
+	for _, ep := range back1.Endpoints {
+		if !ep.IsEmpty() {
+			epmap[*ep] = false
+		}
+	}
+	for _, ep := range back2.Endpoints {
+		if !ep.IsEmpty() {
+			if _, found := epmap[*ep]; !found {
+				return false
+			}
+			epmap[*ep] = true
+		}
+	}
+	for _, found := range epmap {
+		if !found {
+			return false
+		}
+	}
+	return true
+}
+
 // Commit ...
 func (b *Backends) Commit() {
 	b.itemsAdd = map[string]*Backend{}

pkg/haproxy/types/backends_test.go

@@ -128,6 +128,155 @@ func TestBackendCrud(t *testing.T) {
 	}
 }
 
+func TestShrinkBackends(t *testing.T) {
+	ep0 := &Endpoint{IP: "127.0.0.1"}
+	ep11 := &Endpoint{IP: "192.168.0.11"}
+	ep21 := &Endpoint{IP: "192.168.0.21"}
+	app11 := &Backend{Name: "default_app1_8080", Endpoints: []*Endpoint{ep11}}
+	app12 := &Backend{Name: "default_app1_8080", Endpoints: []*Endpoint{ep11, ep0}}
+	app21 := &Backend{Name: "default_app2_8080", Endpoints: []*Endpoint{ep21}}
+	testCases := []struct {
+		add, del       []*Backend
+		expAdd, expDel []*Backend
+	}{
+		// 0
+		{},
+		// 1
+		{
+			add:    []*Backend{app11},
+			expAdd: []*Backend{app11},
+		},
+		// 2
+		{
+			add: []*Backend{app11},
+			del: []*Backend{app11},
+		},
+		// 3
+		{
+			add:    []*Backend{app11, app21},
+			del:    []*Backend{app21},
+			expAdd: []*Backend{app11},
+		},
+		// 4
+		{
+			add:    []*Backend{app11},
+			del:    []*Backend{app11, app21},
+			expDel: []*Backend{app21},
+		},
+		// 5
+		{
+			add: []*Backend{app11},
+			del: []*Backend{app12},
+		},
+	}
+	for i, test := range testCases {
+		c := setup(t)
+		b := CreateBackends(0)
+		for _, add := range test.add {
+			b.itemsAdd[add.Name] = add
+		}
+		for _, del := range test.del {
+			b.itemsDel[del.Name] = del
+		}
+		expAdd := map[string]*Backend{}
+		for _, add := range test.expAdd {
+			expAdd[add.Name] = add
+		}
+		expDel := map[string]*Backend{}
+		for _, del := range test.expDel {
+			expDel[del.Name] = del
+		}
+		b.Shrink()
+		c.compareObjects("add", i, b.itemsAdd, expAdd)
+		c.compareObjects("del", i, b.itemsDel, expDel)
+		c.teardown()
+	}
+}
+
+func TestBackendsMatch(t *testing.T) {
+	ep0_1 := &Endpoint{IP: "127.0.0.1"}
+	ep0_2 := &Endpoint{IP: "127.0.0.1"}
+	ep1_1 := &Endpoint{IP: "192.168.0.1"}
+	ep1_2 := &Endpoint{IP: "192.168.0.1"}
+	ep2_1 := &Endpoint{IP: "192.168.0.2"}
+	ep2_2 := &Endpoint{IP: "192.168.0.2"}
+	testCases := []struct {
+		back1, back2 *Backend
+		expected     bool
+	}{
+		// 0
+		{
+			expected: true,
+		},
+		// 1
+		{
+			back1:    &Backend{},
+			back2:    &Backend{},
+			expected: true,
+		},
+		// 2
+		{
+			back1:    &Backend{Endpoints: []*Endpoint{ep0_1}},
+			back2:    &Backend{Endpoints: []*Endpoint{ep0_2}},
+			expected: true,
+		},
+		// 3
+		{
+			back1:    &Backend{CustomConfig: []string{"http-request"}, Endpoints: []*Endpoint{ep0_1}},
+			back2:    &Backend{CustomConfig: []string{"http-response"}, Endpoints: []*Endpoint{ep0_2}},
+			expected: false,
+		},
+		// 4
+		{
+			back1:    &Backend{Endpoints: []*Endpoint{ep0_1, ep1_1}},
+			back2:    &Backend{Endpoints: []*Endpoint{ep0_2, ep1_2}},
+			expected: true,
+		},
+		// 5
+		{
+			back1:    &Backend{Endpoints: []*Endpoint{ep0_1, ep1_1}},
+			back2:    &Backend{Endpoints: []*Endpoint{ep0_2, ep2_2}},
+			expected: false,
+		},
+		// 6
+		{
+			back1:    &Backend{Endpoints: []*Endpoint{ep0_1, ep1_1, ep2_1}},
+			back2:    &Backend{Endpoints: []*Endpoint{ep0_2, ep2_2, ep1_2}},
+			expected: true,
+		},
+		// 7
+		{
+			back1:    &Backend{Endpoints: []*Endpoint{ep2_1, ep1_1}},
+			back2:    &Backend{Endpoints: []*Endpoint{ep1_2, ep2_2}},
+			expected: true,
+		},
+		// 8
+		{
+			back1:    &Backend{Endpoints: []*Endpoint{ep2_1, ep0_1, ep1_1}},
+			back2:    &Backend{Endpoints: []*Endpoint{ep1_2, ep2_2, ep0_2}},
+			expected: true,
+		},
+		// 9
+		{
+			back1:    &Backend{Endpoints: []*Endpoint{ep2_1, ep0_1}},
+			back2:    &Backend{Endpoints: []*Endpoint{ep1_2, ep2_2, ep0_2}},
+			expected: false,
+		},
+		// 10
+		{
+			back1:    &Backend{Endpoints: []*Endpoint{ep2_1, ep0_1, ep1_1}},
+			back2:    &Backend{Endpoints: []*Endpoint{ep1_2, ep0_2}},
+			expected: false,
+		},
+	}
+	for i, test := range testCases {
+		c := setup(t)
+		result := backendsMatch(test.back1, test.back2)
+		c.compareObjects("match", i, result, test.expected)
+		c.teardown()
+	}
+}
+
 func TestBuildID(t *testing.T) {
 	testCases := []struct {
 		namespace string

pkg/haproxy/types/host.go

@@ -64,6 +64,21 @@ func (h *Hosts) RemoveAll(hostnames []string) {
 	}
 }
 
+// Shrink removes matching added and deleted hosts from the changing hashmap
+// tracker that has the same content. A matching added+deleted pair means
+// that a hostname was reparsed but its content wasn't changed.
+func (h *Hosts) Shrink() {
+	for name, del := range h.itemsDel {
+		if add, found := h.itemsAdd[name]; found {
+			if reflect.DeepEqual(add, del) {
+				h.items[name] = del
+				delete(h.itemsAdd, name)
+				delete(h.itemsDel, name)
+			}
+		}
+	}
+}
+
 // Commit ...
 func (h *Hosts) Commit() {
 	h.itemsAdd = map[string]*Host{}
@@ -72,7 +87,7 @@ func (h *Hosts) Commit() {
 
 // Changed ...
 func (h *Hosts) Changed() bool {
-	return !reflect.DeepEqual(h.itemsAdd, h.itemsDel)
+	return len(h.itemsAdd) > 0 || len(h.itemsDel) > 0
 }
 
 func (h *Hosts) createHost(hostname string) *Host {