Merge pull request #677 from jcmoraisjr/jm-sort-backends

Implement sort-backends

CHANGELOG.md

@@ -453,6 +453,7 @@ Note: A new configuration parser and HAProxy config builder is in place. Despite
 * `dynamic-scaling` configuration key changed the default value from `false` to `true`
 * `nbthread` configuration key changed the default value from `1` to `2`
 * `reload-strategy` command-line option changed  the default value from `native` to `reusesocket`
+* A missing `--sort-backends` command-line option does not shuffle endpoints anymore
 
 The `--v07-controller=true` command-line option can be used to revert to the old controller and behavior. Note that in this case the `*-v07.tmpl` templates will be used instead. This option will be removed on v0.10.
 

docs/content/en/docs/configuration/command-line.md

@@ -234,10 +234,18 @@ describes how it works.
 
 ## --sort-backends
 
-Ingress will randomly shuffle backends and server endpoints on each reload in order to avoid
-requesting always the same backends just after reloads, depending on the balancing algorithm.
-Use `--sort-backends` to avoid this behavior and always declare backends and upstream servers
-in the same order.
+Defines if backend's endpoints should be sorted by name. Since v0.8 the endpoints will stay in the
+same order found in the Kubernetes' endpoint objects if `--sort-backends` is missing.
+
+In v0.7 and older version, if `--sort-backends` is missing, HAProxy Ingress randomly shuffle endpoints
+on each reload in order to avoid requesting always the same backends just after haproxy reloads.
+
+Sorting backends by name has a real effect only if using a distinct [backend-server-naming]({{% relref "keys#backend-server-naming" %}})
+option, because the default value builds the server name using a numeric sequence.
+
+See also:
+
+* [backend-server-naming]({{% relref "keys#backend-server-naming" %}}) configuration key
 
 ---
 

pkg/common/ingress/controller/launch.go

@@ -169,7 +169,7 @@ func NewIngressController(backend ingress.Controller) *GenericController {
 			`Defines how much files should be used to configure the haproxy backends`)
 
 		sortBackends = flags.Bool("sort-backends", false,
-			`Defines if backends and it's endpoints should be sorted`)
+			`Defines if backend's endpoints should be sorted by name. It uses the same k8s endpoint order if missing`)
 
 		useNodeInternalIP = flags.Bool("report-node-internal-ip-address", false,
 			`Defines if the nodes IP address to be returned in the ingress status should be the internal instead of the external IP address`)

pkg/controller/controller.go

@@ -130,6 +130,7 @@ func (hc *HAProxyController) configController() {
 		Metrics:           hc.metrics,
 		ReloadStrategy:    *hc.reloadStrategy,
 		MaxOldConfigFiles: *hc.maxOldConfigFiles,
+		SortBackends:      hc.cfg.SortBackends,
 		ValidateConfig:    *hc.validateConfig,
 	}
 	hc.instance = haproxy.CreateInstance(hc.logger, instanceOptions)

pkg/haproxy/dynupdate.go

@@ -223,7 +223,6 @@ func (d *dynUpdater) checkBackendPair(pair *backendPair) bool {
 	for i := len(added); i < len(empty); i++ {
 		curBack.AddEmptyEndpoint().Name = empty[i].Name
 	}
-	curBack.SortEndpoints()
 
 	return updated
 }

pkg/haproxy/dynupdate_test.go

@@ -113,8 +113,8 @@ set server default_app_8080/srv002 weight 1`,
 				b.AcquireEndpoint("172.17.0.4", 8080, "")
 			},
 			expected: []string{
-				"srv001:172.17.0.4:8080:1",
 				"srv002:172.17.0.3:8080:1",
+				"srv001:172.17.0.4:8080:1",
 			},
 			dynamic: true,
 			cmd: `
@@ -162,8 +162,8 @@ set server default_app_8080/srv001 weight 1`,
 				b.AcquireEndpoint("172.17.0.3", 8080, "")
 			},
 			expected: []string{
-				"srv001:127.0.0.1:1023:1",
 				"srv002:172.17.0.3:8080:1",
+				"srv001:127.0.0.1:1023:1",
 			},
 			dynamic: true,
 			cmd: `
@@ -235,8 +235,8 @@ set server default_app_8080/srv001 weight 1
 				b.AcquireEndpoint("172.17.0.3", 8080, "")
 			},
 			expected: []string{
-				"srv001:172.17.0.3:8080:1",
 				"srv002:172.17.0.2:8080:1",
+				"srv001:172.17.0.3:8080:1",
 			},
 			dynamic: true,
 			cmd: `
@@ -266,12 +266,12 @@ set server default_app_8080/srv001 weight 1
 				b.AcquireEndpoint("172.17.0.7", 8080, "")
 			},
 			expected: []string{
+				"srv004:172.17.0.5:8080:1",
+				"srv006:172.17.0.7:8080:1",
 				"srv001:127.0.0.1:1023:1",
 				"srv002:127.0.0.1:1023:1",
 				"srv003:127.0.0.1:1023:1",
-				"srv004:172.17.0.5:8080:1",
 				"srv005:127.0.0.1:1023:1",
-				"srv006:172.17.0.7:8080:1",
 				"srv007:127.0.0.1:1023:1",
 				"srv008:127.0.0.1:1023:1",
 			},

pkg/haproxy/instance.go

@@ -44,6 +44,7 @@ type InstanceOptions struct {
 	MaxOldConfigFiles int
 	Metrics           types.Metrics
 	ReloadStrategy    string
+	SortBackends      bool
 	ValidateConfig    bool
 	// TODO Fake is used to skip real haproxy calls. Use a mock instead.
 	fake bool
@@ -257,6 +258,9 @@ func (i *instance) haproxyUpdate(timer *utils.Timer) {
 	}
 	updater := i.newDynUpdater()
 	updated := updater.update()
+	if i.options.SortBackends {
+		i.config.Backends().SortChangedEndpoints()
+	}
 	if !updated || updater.cmdCnt > 0 {
 		// only need to rewrtite config files if:
 		//   - !updated           - there are changes that cannot be dynamically applied

pkg/haproxy/types/backend.go

@@ -92,8 +92,7 @@ func (b *Backend) addEndpoint(ip string, port int, targetRef string) *Endpoint {
 	return endpoint
 }
 
-// SortEndpoints ...
-func (b *Backend) SortEndpoints() {
+func (b *Backend) sortEndpoints() {
 	sort.SliceStable(b.Endpoints, func(i, j int) bool {
 		return b.Endpoints[i].Name < b.Endpoints[j].Name
 	})

pkg/haproxy/types/backends.go

@@ -146,6 +146,13 @@ func (b *Backends) ChangedShards() []int {
 	return changed
 }
 
+// SortChangedEndpoints ...
+func (b *Backends) SortChangedEndpoints() {
+	for _, backend := range b.itemsAdd {
+		backend.sortEndpoints()
+	}
+}
+
 // BuildSortedItems ...
 func (b *Backends) BuildSortedItems() []*Backend {
 	// TODO BuildSortedItems() is currently used only by the backend template.