Merge ~hloeung/content-cache-charm:master into content-cache-charm:master

[Haw Loeung]

Allow overriding default load balancing algorithm used by HAProxy - LP:1891263

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Barry Price (barryprice)]

We could consider adding safety checks to handle a typo or just a plain wrong setting via the haproxy_load_balancing_algorithm config item. If someone sets it to e.g. "xyzzy", will haproxy start? Should it?

On the other hand, if an operator is determined to break things, there's only so much the charms can do to prevent this. Other than that, LGTM - approving on that basis.

review: Approve

[Joel Sing (jsing)]

LGTM, minor comments inline.

review: Approve (+1)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 67f7f1384a4c1f788d2117dc31c377cc29a2051e

diff --git a/config.yaml b/config.yaml
index 12560a6..e634fe2 100644
--- a/config.yaml
+++ b/config.yaml
@@ -45,6 +45,12 @@ options:
     description: >
       Tune HAProxy's hard-stop-after to prevent lingering HAProxy processes
       (LP:1874386).
+  haproxy_load_balancing_algorithm:
+    default: "leastconn"
+    type: string
+    description: >
+      Change the load balancing algorithm used by HAProxy for the
+      backends.
   haproxy_processes:
     default: 0
     type: int
diff --git a/lib/haproxy.py b/lib/haproxy.py
index c3f3a76..cdb5721 100644
--- a/lib/haproxy.py
+++ b/lib/haproxy.py
@@ -11,15 +11,21 @@ from lib import utils
 
 
 HAPROXY_BASE_PATH = '/etc/haproxy'
+HAPROXY_LOAD_BALANCING_ALGORITHM = 'leastconn'
 INDENT = ' ' * 4
 TLS_CIPHER_SUITES = 'ECDHE+AESGCM:ECDHE+AES256:ECDHE+AES128:!SSLv3:!TLSv1'
 
 
 class HAProxyConf:
-    def __init__(self, conf_path=HAPROXY_BASE_PATH, max_connections=0, hard_stop_after='5m'):
+    def __init__(
+        self, conf_path=HAPROXY_BASE_PATH, max_connections=0, hard_stop_after='5m', load_balancing_algorithm=None
+    ):
         self._conf_path = conf_path
         self.max_connections = int(max_connections)
         self.hard_stop_after = hard_stop_after
+        self.load_balancing_algorithm = HAPROXY_LOAD_BALANCING_ALGORITHM
+        if load_balancing_algorithm:
+            self.load_balancing_algorithm = load_balancing_algorithm
 
     @property
     def conf_path(self):
@@ -212,7 +218,7 @@ listen {name}
 backend backend-{name}
 {options}{indent}{httpchk}
 {indent}http-request set-header Host {site_name}
-{indent}balance leastconn
+{indent}balance {load_balancing_algorithm}
 {backends}
 """
         rendered_output = []
@@ -293,6 +299,7 @@ backend backend-{name}
                     site=site,
                     site_name=site_name,
                     httpchk=httpchk,
+                    load_balancing_algorithm=self.load_balancing_algorithm,
                     backends='\n'.join(backend_confs),
                     options=options,
                     indent=INDENT,
diff --git a/reactive/content_cache.py b/reactive/content_cache.py
index 5b15f8a..5d4460b 100644
--- a/reactive/content_cache.py
+++ b/reactive/content_cache.py
@@ -208,7 +208,12 @@ def configure_haproxy():  # NOQA: C901 LP#1825084
 
     max_connections = config.get('max_connections', 0)
     hard_stop_after = config.get('haproxy_hard_stop_after')
-    haproxy = HAProxy.HAProxyConf(max_connections=max_connections, hard_stop_after=hard_stop_after)
+    load_balancing_algorithm = config.get('haproxy_load_balancing_algorithm')
+    haproxy = HAProxy.HAProxyConf(
+        max_connections=max_connections,
+        hard_stop_after=hard_stop_after,
+        load_balancing_algorithm=load_balancing_algorithm,
+    )
     sites_secrets = secrets_from_config(config.get('sites_secrets'))
     blacklist_ports = [int(x.strip()) for x in config.get('blacklist_ports', '').split(',') if x.strip()]
     sites = sites_from_config(config.get('sites'), sites_secrets, blacklist_ports=blacklist_ports)
diff --git a/tests/unit/files/content_cache_rendered_haproxy_test_output_load_balancing_algorithm.txt b/tests/unit/files/content_cache_rendered_haproxy_test_output_load_balancing_algorithm.txt
new file mode 100644
index 0000000..b0c6370
--- /dev/null
+++ b/tests/unit/files/content_cache_rendered_haproxy_test_output_load_balancing_algorithm.txt
@@ -0,0 +1,267 @@
+global
+	nbthread 4
+	maxconn 106496
+	log /dev/log	local0
+	log /dev/log	local1 notice
+	chroot /var/lib/haproxy
+	stats socket /run/haproxy/admin.sock mode 660 level admin expose-fd listeners
+	stats timeout 30s
+	user haproxy
+	group haproxy
+	daemon
+
+	# LP#1874386: Work around lingering HAProxy processes as per LP:1874386
+	# and kill them off.
+	hard-stop-after 15m
+
+	# Default SSL material locations
+	ca-base /etc/ssl/certs
+	crt-base /etc/ssl/private
+
+	# Default ciphers to use on SSL-enabled listening sockets.
+	# For more information, see ciphers(1SSL). This list is from:
+	#  https://hynek.me/articles/hardening-your-web-servers-ssl-ciphers/
+	# An alternative list with additional directives can be obtained from
+	#  https://mozilla.github.io/server-side-tls/ssl-config-generator/?server=haproxy
+	ssl-default-bind-ciphers ECDHE+AESGCM:ECDHE+AES256:ECDHE+AES128:!SSLv3:!TLSv1
+	ssl-default-bind-options no-sslv3 no-tlsv10 no-tlsv11
+	# We'll eventually disable DHE (LP#1825321), but for now, bump DH params
+	tune.ssl.default-dh-param 2048
+
+	# Increase the SSL/TLS session cache from the default 20k. But
+	# rather than hardcode values, let's just set it to match
+	# global_max_connections (which by default is calculated using
+	# num. of CPU cores and num. of configured sites). Each entry
+	# requires ~200 bytes so on a host with say 32 CPUs, 10 sites,
+	# each with 2000 max conns will only consume around 122 Mbytes
+	# (32 * 10 * 2000 * 200), which is not much.
+	tune.ssl.cachesize 106496
+
+defaults
+	log	global
+	maxconn 8192
+	mode	http
+	option	httplog
+	option	dontlognull
+	timeout connect 5s
+	timeout client  50s
+	timeout server  50s
+	errorfile 400 /etc/haproxy/errors/400.http
+	errorfile 403 /etc/haproxy/errors/403.http
+	errorfile 408 /etc/haproxy/errors/408.http
+	errorfile 500 /etc/haproxy/errors/500.http
+	errorfile 502 /etc/haproxy/errors/502.http
+	errorfile 503 /etc/haproxy/errors/503.http
+	errorfile 504 /etc/haproxy/errors/504.http
+
+resolvers dns
+	nameserver dns1 127.0.0.53:53
+	resolve_retries 3
+	timeout resolve 3s
+	timeout retry   3s
+	accepted_payload_size 8192
+
+listen  stats
+	bind 127.0.0.1:10000
+	acl allowed_cidr src 127.0.0.0/8
+	http-request deny unless allowed_cidr
+
+	mode http
+	stats enable
+	stats uri /
+	stats realm Haproxy\ Statistics
+	stats auth haproxy:biometricsarenotsecret
+	stats refresh 3
+
+
+listen combined-80
+    bind 0.0.0.0:80
+    bind :::80
+    redirect scheme https code 301 if { hdr(Host) -i site2.local } !{ ssl_fc }
+    use_backend backend-cached-site1-local if { hdr(Host) -i site1.local }
+    use_backend backend-cached-site3-local if { hdr(Host) -i site3.local }
+    use_backend backend-cached-site4-local if { hdr(Host) -i site4.local }
+    use_backend backend-cached-site5 if { hdr(Host) -i site5.local }
+    use_backend backend-cached-site6-local if { hdr(Host) -i site6.local }
+    use_backend backend-cached-site9-local if { hdr(Host) -i site9.local }
+    default_backend backend-cached-site3-local
+
+listen site1-local
+    bind 127.0.0.1:8080
+    default_backend backend-site1-local
+
+listen cached-site2-local
+    bind 0.0.0.0:443 ssl crt /etc/haproxy/site2-bundle.crt alpn h2,http/1.1
+    bind :::443 ssl crt /etc/haproxy/site2-bundle.crt alpn h2,http/1.1
+    default_backend backend-cached-site2-local
+
+listen site2-local
+    bind 127.0.0.1:8081
+    default_backend backend-site2-local
+
+listen site3-local
+    bind 127.0.0.1:8082
+    default_backend backend-site3-local
+
+listen site5
+    bind 127.0.0.1:8083
+    default_backend backend-site5
+
+listen site5-2
+    bind 127.0.0.1:8084
+    default_backend backend-site5-2
+
+listen site6-local
+    bind 127.0.0.1:8085
+    default_backend backend-site6-local
+
+listen combined-444
+    bind 0.0.0.0:444 ssl crt /etc/haproxy/site7-bundle.crt crt /etc/haproxy/site8-bundle.crt alpn h2,http/1.1
+    bind :::444 ssl crt /etc/haproxy/site7-bundle.crt crt /etc/haproxy/site8-bundle.crt alpn h2,http/1.1
+    use_backend backend-cached-site7-local if { hdr(Host) -i site7.local }
+    use_backend backend-cached-site8-local if { hdr(Host) -i site8.local }
+
+listen site7-local
+    bind 127.0.0.1:8086
+    default_backend backend-site7-local
+
+listen site8-local
+    bind 127.0.0.1:8087
+    default_backend backend-site8-local
+
+listen site8-local-2
+    bind 127.0.0.1:8088
+    default_backend backend-site8-local-2
+
+listen site9-local
+    bind 127.0.0.1:8089
+    default_backend backend-site9-local
+
+backend backend-cached-site1-local
+    option forwardfor
+    option httpchk HEAD /?token=1861920000_f3e404e205ed44749e942d481f7a7bec57c5e78a HTTP/1.0\r\nHost:\ site1.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site1.local
+    balance roundrobin
+    server server_1 127.0.0.1:6080 check inter 2s rise 2 fall 60 maxconn 2048
+
+backend backend-site1-local
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site1.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site1.local
+    balance roundrobin
+    server server_1 127.0.1.10:80 check inter 5s rise 2 fall 5 maxconn 2048
+    server server_2 127.0.1.11:80 check inter 5s rise 2 fall 5 maxconn 2048
+    server server_3 127.0.1.12:80 check inter 5s rise 2 fall 5 maxconn 2048
+
+backend backend-cached-site2-local
+    option forwardfor
+    option httpchk GET /check/ HTTP/1.0\r\nHost:\ site2.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site2.local
+    balance roundrobin
+    server server_1 127.0.0.1:6081 check inter 2s rise 2 fall 60 maxconn 2048
+
+backend backend-site2-local
+    option httpchk GET /check/ HTTP/1.0\r\nHost:\ site2.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site2.local
+    balance roundrobin
+    server server_1 127.0.1.10:443 check inter 5s rise 2 fall 5 maxconn 1024 ssl sni str(site2.local) check-sni site2.local verify required ca-file ca-certificates.crt
+    server server_2 127.0.1.11:443 check inter 5s rise 2 fall 5 maxconn 1024 ssl sni str(site2.local) check-sni site2.local verify required ca-file ca-certificates.crt
+    server server_3 127.0.1.12:443 check inter 5s rise 2 fall 5 maxconn 1024 ssl sni str(site2.local) check-sni site2.local verify required ca-file ca-certificates.crt
+
+backend backend-cached-site3-local
+    option forwardfor
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site3.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site3.local
+    balance roundrobin
+    server server_1 127.0.0.1:6082 check inter 2s rise 2 fall 60 maxconn 4096
+
+backend backend-site3-local
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site3.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site3.local
+    balance roundrobin
+    server server_1 127.0.1.10:80 check inter 5s rise 2 fall 5 maxconn 2048
+    server server_2 127.0.1.11:80 check inter 5s rise 2 fall 5 maxconn 2048
+    server server_3 127.0.1.12:80 check inter 5s rise 2 fall 5 maxconn 2048
+
+backend backend-cached-site4-local
+    option forwardfor
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site4.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site4.local
+    balance roundrobin
+    server server_1 127.0.0.1:6083 check inter 2s rise 2 fall 60 maxconn 2048
+
+backend backend-cached-site5
+    option forwardfor
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site5.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site5.local
+    balance roundrobin
+    server server_1 127.0.0.1:6084 check inter 2s rise 2 fall 60 maxconn 2048
+
+backend backend-site5
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site5.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site5.local
+    balance roundrobin
+    server server_1 127.0.1.10:80 check inter 5s rise 2 fall 5 maxconn 2048
+
+backend backend-site5-2
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site5.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site5.local
+    balance roundrobin
+    server server_1 127.0.1.11:80 check inter 5s rise 2 fall 5 maxconn 2048
+
+backend backend-cached-site6-local
+    option forwardfor
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site6.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site6.local
+    balance roundrobin
+    server server_1 127.0.0.1:6085 check inter 2s rise 2 fall 60 maxconn 2048
+
+backend backend-site6-local
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site6.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site6.local
+    balance roundrobin
+    server server_1 127.0.1.10:443 check inter 5s rise 2 fall 5 maxconn 2048 ssl sni str(site6.local) check-sni site6.local verify required ca-file ca-certificates.crt
+
+backend backend-cached-site7-local
+    option forwardfor
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site7.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site7.local
+    balance roundrobin
+    server server_1 127.0.0.1:6086 check inter 2s rise 2 fall 60 maxconn 2048
+
+backend backend-site7-local
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site7.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site7.local
+    balance roundrobin
+    server server_1 127.0.1.10:80 check inter 5s rise 2 fall 5 maxconn 2048
+
+backend backend-cached-site8-local
+    option forwardfor
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site8.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site8.local
+    balance roundrobin
+    server server_1 127.0.0.1:6087 check inter 2s rise 2 fall 60 maxconn 2048
+
+backend backend-site8-local
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site8.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site8.local
+    balance roundrobin
+    server server_1 127.0.1.10:80 check inter 5s rise 2 fall 5 maxconn 2048
+
+backend backend-site8-local-2
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ auth.site8.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host auth.site8.local
+    balance roundrobin
+    server server_1 127.0.1.10:443 check inter 5s rise 2 fall 5 maxconn 2048 ssl sni str(auth.site8.local) check-sni auth.site8.local verify required ca-file ca-certificates.crt
+
+backend backend-cached-site9-local
+    option forwardfor
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site9.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site9.local
+    balance roundrobin
+    server server_1 127.0.0.1:6088 check inter 2s rise 2 fall 60 maxconn 2048
+
+backend backend-site9-local
+    option httpchk HEAD / HTTP/1.0\r\nHost:\ site9.local\r\nUser-Agent:\ haproxy/httpchk\r\nCache-Control:\ no-cache
+    http-request set-header Host site9.local
+    balance roundrobin
+    server server_1 127.0.1.15:80 check inter 1m rise 2 fall 5 maxconn 2048
diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
index 3b62cbb..561cd53 100644
--- a/tests/unit/test_content_cache.py
+++ b/tests/unit/test_content_cache.py
@@ -485,6 +485,35 @@ site1.local:
     @freezegun.freeze_time("2019-03-22", tz_offset=0)
     @mock.patch('charmhelpers.core.hookenv.opened_ports')
     @mock.patch('charms.reactive.set_flag')
+    @mock.patch('reactive.content_cache.update_logrotate')
+    def test_configure_haproxy_sites_load_balancing_algorithm(self, logrotation, set_flag, opened_ports):
+        with open('tests/unit/files/config_test_config.txt', 'r', encoding='utf-8') as f:
+            config = f.read()
+        self.mock_config.return_value = {
+            'haproxy_hard_stop_after': '15m',
+            'haproxy_load_balancing_algorithm': 'roundrobin',
+            'max_connections': 8192,
+            'sites': config,
+        }
+
+        with mock.patch('lib.haproxy.HAProxyConf.conf_file', new_callable=mock.PropertyMock) as mock_conf_file:
+            mock_conf_file.return_value = os.path.join(self.tmpdir, 'haproxy.cfg')
+            opened_ports.return_value = ['443/tcp']
+            content_cache.configure_haproxy()
+
+            with open(
+                'tests/unit/files/content_cache_rendered_haproxy_test_output_load_balancing_algorithm.txt',
+                'r',
+                encoding='utf-8',
+            ) as f:
+                want = f.read()
+            with open(os.path.join(self.tmpdir, 'haproxy.cfg'), 'r', encoding='utf-8') as f:
+                got = f.read()
+            self.assertEqual(got, want)
+
+    @freezegun.freeze_time("2019-03-22", tz_offset=0)
+    @mock.patch('charmhelpers.core.hookenv.opened_ports')
+    @mock.patch('charms.reactive.set_flag')
     @mock.patch('lib.utils.package_version')
     @mock.patch('reactive.content_cache.update_logrotate')
     def test_configure_haproxy_processes_and_threads(self, logrotation, package_version, set_flag, opened_ports):