Content Cache Charm

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

  1. Git
  2. lp:~hloeung/content-cache-charm
  3. haproxy-config
  4. Merge into master
Proposed by Haw Loeung
Status: Merged
Approved by: Haw Loeung
Approved revision: c585dd9259543aba3ba7eb7fb7de4a9f688249f7
Merged at revision: dce9fba4659c564faca0045c339547e0e308d754
Proposed branch: ~hloeung/content-cache-charm:haproxy-config
Merge into: content-cache-charm:master
Diff against target: 338 lines (+49/-10)
9 files modified
config.yaml (+6/-0)
lib/haproxy.py (+5/-2)
reactive/content_cache.py (+3/-1)
templates/haproxy_cfg.tmpl (+1/-0)
tests/unit/files/content_cache_rendered_haproxy_test_output.txt (+14/-0)
tests/unit/files/haproxy_config_rendered_listen_stanzas_test_output.txt (+3/-0)
tests/unit/files/haproxy_config_rendered_test_output.txt (+4/-0)
tests/unit/test_content_cache.py (+11/-5)
tests/unit/test_haproxy.py (+2/-2)
Reviewer Review Type Date Requested Status
Joel Sing (community) +1 Approve
Canonical IS Reviewers Pending
Review via email: mp+380256@code.launchpad.net

Commit message

Allow overriding HAProxy's maxconns - LP:1866036

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

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

Revision history for this message
Joel Sing (jsing) wrote :

Change looks fine, questions re the numbers. Also, seems we're only testing the default value and no other configuration - would be good to address (either now or in later change).

review: Needs Information
Revision history for this message
Haw Loeung (hloeung) :
Revision history for this message
Joel Sing (jsing) wrote :

LGTM, but see nit inline.

review: Approve (+1)
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision dce9fba4659c564faca0045c339547e0e308d754

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/config.yaml b/config.yaml
2index abd045a..5e0133f 100644
3--- a/config.yaml
4+++ b/config.yaml
5@@ -30,6 +30,12 @@ options:
6 default: 30
7 description: >
8 Number of log files to retain during rotation.
9+ max_connections:
10+ type: int
11+ default: 8192
12+ description: >
13+ Configure maximum number of connections on frontend HAProxy. (8192 for now,
14+ the default will automatically be calculated in the future)
15 nagios_context:
16 type: string
17 default: "juju"
18diff --git a/lib/haproxy.py b/lib/haproxy.py
19index aa1af24..cd92dbe 100644
20--- a/lib/haproxy.py
21+++ b/lib/haproxy.py
22@@ -14,8 +14,9 @@ TLS_CIPHER_SUITES = 'ECDHE+AESGCM:ECDHE+AES256:ECDHE+AES128:!SSLv3:!TLSv1'
23
24
25 class HAProxyConf:
26- def __init__(self, conf_path=HAPROXY_BASE_PATH):
27+ def __init__(self, conf_path=HAPROXY_BASE_PATH, max_connections=2000):
28 self._conf_path = conf_path
29+ self.max_connections = max_connections
30
31 @property
32 def conf_path(self):
33@@ -98,6 +99,7 @@ class HAProxyConf:
34 listen_stanza = """
35 listen {name}
36 {bind_config}
37+ maxconn {max_connections}
38 {backend_config}"""
39 backend_conf = '{indent}use_backend backend-{backend} if {{ hdr(Host) -i {site_name} }}\n'
40 redirect_conf = '{indent}redirect scheme https code 301 if {{ hdr(Host) -i {site_name} }} !{{ ssl_fc }}\n'
41@@ -151,7 +153,7 @@ listen {name}
42 if address == '0.0.0.0':
43 bind_config += '\n{indent}bind :::{port}{tls}'.format(port=port, tls=tls_config, indent=INDENT)
44 output = listen_stanza.format(
45- name=name, backend_config=''.join(backend_config), bind_config=bind_config, indent=INDENT
46+ name=name, max_connections=self.max_connections, backend_config=''.join(backend_config), bind_config=bind_config, indent=INDENT
47 )
48 rendered_output.append(output)
49 return rendered_output
50@@ -256,6 +258,7 @@ backend backend-{name}
51 'listen': self.render_stanza_listen(config),
52 'backend': self.render_stanza_backend(config),
53 'num_threads': num_threads,
54+ 'max_connections': self.max_connections,
55 'monitoring_password': monitoring_password or self.monitoring_password,
56 'tls_cipher_suites': tls_cipher_suites,
57 }
58diff --git a/reactive/content_cache.py b/reactive/content_cache.py
59index 428b3d1..b778711 100644
60--- a/reactive/content_cache.py
61+++ b/reactive/content_cache.py
62@@ -191,7 +191,9 @@ def configure_haproxy(): # NOQA: C901 LP#1825084
63 status.blocked('requires list of sites to configure')
64 return
65
66- haproxy = HAProxy.HAProxyConf()
67+ # TODO: Calculate max connections if none specified. Likely use configured
68+ # nbthreads (2000 * nbthreads). Or maybe even per site.
69+ haproxy = HAProxy.HAProxyConf(max_connections=config['max_connections'])
70 sites_secrets = secrets_from_config(config.get('sites_secrets'))
71 blacklist_ports = [int(x.strip()) for x in config.get('blacklist_ports', '').split(',') if x.strip()]
72 sites = sites_from_config(config.get('sites'), sites_secrets, blacklist_ports=blacklist_ports)
73diff --git a/templates/haproxy_cfg.tmpl b/templates/haproxy_cfg.tmpl
74index 16da8de..a4ec3be 100644
75--- a/templates/haproxy_cfg.tmpl
76+++ b/templates/haproxy_cfg.tmpl
77@@ -1,5 +1,6 @@
78 global
79 nbthread {{num_threads}}
80+ maxconn {{max_connections}}
81 log /dev/log local0
82 log /dev/log local1 notice
83 chroot /var/lib/haproxy
84diff --git a/tests/unit/files/content_cache_rendered_haproxy_test_output.txt b/tests/unit/files/content_cache_rendered_haproxy_test_output.txt
85index e3542b3..b715f9d 100644
86--- a/tests/unit/files/content_cache_rendered_haproxy_test_output.txt
87+++ b/tests/unit/files/content_cache_rendered_haproxy_test_output.txt
88@@ -1,5 +1,6 @@
89 global
90 nbthread 4
91+ maxconn 8192
92 log /dev/log local0
93 log /dev/log local1 notice
94 chroot /var/lib/haproxy
95@@ -55,6 +56,7 @@ listen stats
96 listen combined-80
97 bind 0.0.0.0:80
98 bind :::80
99+ maxconn 8192
100 use_backend backend-cached-site1-local if { hdr(Host) -i site1.local }
101 redirect scheme https code 301 if { hdr(Host) -i site2.local } !{ ssl_fc }
102 use_backend backend-cached-site3-local if { hdr(Host) -i site3.local }
103@@ -65,53 +67,65 @@ listen combined-80
104
105 listen site1-local
106 bind 127.0.0.1:8080
107+ maxconn 8192
108 default_backend backend-site1-local
109
110 listen cached-site2-local
111 bind 0.0.0.0:443 ssl crt /etc/haproxy/site2-bundle.crt
112 bind :::443 ssl crt /etc/haproxy/site2-bundle.crt
113+ maxconn 8192
114 default_backend backend-cached-site2-local
115
116 listen site2-local
117 bind 127.0.0.1:8081
118+ maxconn 8192
119 default_backend backend-site2-local
120
121 listen site3-local
122 bind 127.0.0.1:8082
123+ maxconn 8192
124 default_backend backend-site3-local
125
126 listen site5
127 bind 127.0.0.1:8083
128+ maxconn 8192
129 default_backend backend-site5
130
131 listen site5-2
132 bind 127.0.0.1:8084
133+ maxconn 8192
134 default_backend backend-site5
135
136 listen site6-local
137 bind 127.0.0.1:8085
138+ maxconn 8192
139 default_backend backend-site6-local
140
141 listen combined-444
142 bind 0.0.0.0:444 ssl crt /etc/haproxy/site7-bundle.crt crt /etc/haproxy/site8-bundle.crt
143 bind :::444 ssl crt /etc/haproxy/site7-bundle.crt crt /etc/haproxy/site8-bundle.crt
144+ maxconn 8192
145 use_backend backend-cached-site7-local if { hdr(Host) -i site7.local }
146 use_backend backend-cached-site8-local if { hdr(Host) -i site8.local }
147
148 listen site7-local
149 bind 127.0.0.1:8086
150+ maxconn 8192
151 default_backend backend-site7-local
152
153 listen site8-local
154 bind 127.0.0.1:8087
155+ maxconn 8192
156 default_backend backend-site8-local
157
158 listen site8-local-2
159 bind 127.0.0.1:8088
160+ maxconn 8192
161 default_backend backend-site8-local
162
163 listen site9-local
164 bind 127.0.0.1:8089
165+ maxconn 8192
166 default_backend backend-site9-local
167
168 backend backend-cached-site1-local
169diff --git a/tests/unit/files/haproxy_config_rendered_listen_stanzas_test_output.txt b/tests/unit/files/haproxy_config_rendered_listen_stanzas_test_output.txt
170index e98c5fe..57884f0 100644
171--- a/tests/unit/files/haproxy_config_rendered_listen_stanzas_test_output.txt
172+++ b/tests/unit/files/haproxy_config_rendered_listen_stanzas_test_output.txt
173@@ -2,6 +2,7 @@
174 listen combined-80
175 bind 0.0.0.0:80
176 bind :::80
177+ maxconn 8192
178 use_backend backend-site1-local if { hdr(Host) -i site1.local }
179 redirect scheme https code 301 if { hdr(Host) -i site2.local } !{ ssl_fc }
180 use_backend backend-site3-local if { hdr(Host) -i site3.local }
181@@ -13,10 +14,12 @@ listen combined-80
182 listen site2-local
183 bind 0.0.0.0:443 ssl crt /etc/haproxy/site2-bundle.crt
184 bind :::443 ssl crt /etc/haproxy/site2-bundle.crt
185+ maxconn 8192
186 default_backend backend-site2-local
187
188 listen combined-444
189 bind 0.0.0.0:444 ssl crt /etc/haproxy/site7-bundle.crt crt /etc/haproxy/site8-bundle.crt
190 bind :::444 ssl crt /etc/haproxy/site7-bundle.crt crt /etc/haproxy/site8-bundle.crt
191+ maxconn 8192
192 use_backend backend-site7-local if { hdr(Host) -i site7.local }
193 use_backend backend-site8-local if { hdr(Host) -i site8.local }
194diff --git a/tests/unit/files/haproxy_config_rendered_test_output.txt b/tests/unit/files/haproxy_config_rendered_test_output.txt
195index 851eeaa..e3132ec 100644
196--- a/tests/unit/files/haproxy_config_rendered_test_output.txt
197+++ b/tests/unit/files/haproxy_config_rendered_test_output.txt
198@@ -1,5 +1,6 @@
199 global
200 nbthread 4
201+ maxconn 8192
202 log /dev/log local0
203 log /dev/log local1 notice
204 chroot /var/lib/haproxy
205@@ -55,6 +56,7 @@ listen stats
206 listen combined-80
207 bind 0.0.0.0:80
208 bind :::80
209+ maxconn 8192
210 use_backend backend-site1-local if { hdr(Host) -i site1.local }
211 redirect scheme https code 301 if { hdr(Host) -i site2.local } !{ ssl_fc }
212 use_backend backend-site3-local if { hdr(Host) -i site3.local }
213@@ -66,11 +68,13 @@ listen combined-80
214 listen site2-local
215 bind 0.0.0.0:443 ssl crt /etc/haproxy/site2-bundle.crt
216 bind :::443 ssl crt /etc/haproxy/site2-bundle.crt
217+ maxconn 8192
218 default_backend backend-site2-local
219
220 listen combined-444
221 bind 0.0.0.0:444 ssl crt /etc/haproxy/site7-bundle.crt crt /etc/haproxy/site8-bundle.crt
222 bind :::444 ssl crt /etc/haproxy/site7-bundle.crt crt /etc/haproxy/site8-bundle.crt
223+ maxconn 8192
224 use_backend backend-site7-local if { hdr(Host) -i site7.local }
225 use_backend backend-site8-local if { hdr(Host) -i site8.local }
226
227diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
228index dfac47a..bf1c4b8 100644
229--- a/tests/unit/test_content_cache.py
230+++ b/tests/unit/test_content_cache.py
231@@ -169,6 +169,7 @@ class TestCharm(unittest.TestCase):
232 'cache_max_size': '1g',
233 'cache_path': '/var/lib/nginx/proxy',
234 'enable_prometheus_metrics': False,
235+ 'max_connections': 8192,
236 'sites': ngx_config,
237 'worker_connections': 768,
238 'worker_processes': 0,
239@@ -247,6 +248,7 @@ site1.local:
240 'cache_inactive_time': '',
241 'cache_max_size': '1g',
242 'cache_path': '/var/lib/nginx/proxy',
243+ 'max_connections': 8192,
244 'sites': config,
245 'sites_secrets': secrets,
246 'worker_connections': 768,
247@@ -299,6 +301,7 @@ site1.local:
248 'cache_inactive_time': '2h',
249 'cache_max_size': '1g',
250 'cache_path': '/var/lib/nginx/proxy',
251+ 'max_connections': 8192,
252 'sites': config,
253 'worker_connections': 768,
254 'worker_processes': 0,
255@@ -316,6 +319,7 @@ site1.local:
256 'cache_inactive_time': '2h',
257 'cache_max_size': '20g',
258 'cache_path': '/srv/cache',
259+ 'max_connections': 8192,
260 'sites': config,
261 'worker_connections': 768,
262 'worker_processes': 0,
263@@ -334,6 +338,7 @@ site1.local:
264 'cache_inactive_time': '2h',
265 'cache_max_size': '',
266 'cache_path': '/srv/cache',
267+ 'max_connections': 8192,
268 'sites': config,
269 'worker_connections': 768,
270 'worker_processes': 0,
271@@ -356,7 +361,7 @@ site1.local:
272
273 status.reset_mock()
274 clear_flag.reset_mock()
275- self.mock_config.return_value = {'sites': 'site1:'}
276+ self.mock_config.return_value = {'max_connections': 8192, 'sites': 'site1:'}
277 content_cache.configure_haproxy()
278 status.blocked.assert_called()
279 clear_flag.assert_called_once_with('content_cache.active')
280@@ -367,7 +372,7 @@ site1.local:
281 def test_configure_haproxy_sites(self, logrotation, set_flag):
282 with open('tests/unit/files/config_test_config.txt', 'r', encoding='utf-8') as f:
283 ngx_config = f.read()
284- self.mock_config.return_value = {'sites': ngx_config}
285+ self.mock_config.return_value = {'max_connections': 8192, 'sites': ngx_config}
286
287 with mock.patch('lib.haproxy.HAProxyConf.conf_file', new_callable=mock.PropertyMock) as mock_conf_file:
288 mock_conf_file.return_value = os.path.join(self.tmpdir, 'haproxy.cfg')
289@@ -970,8 +975,9 @@ site1.local:
290 self.mock_config.return_value = {
291 'cache_inactive_time': '2h',
292 'cache_max_size': '1g',
293- 'enable_prometheus_metrics': True,
294 'cache_path': '/var/lib/nginx/proxy',
295+ 'enable_prometheus_metrics': True,
296+ 'max_connections': 8192,
297 'sites': ngx_config,
298 'worker_connections': 768,
299 'worker_processes': 0,
300@@ -1046,7 +1052,7 @@ site1.local:
301 ngx_config = f.read()
302
303 # Test that haproxy calls close_port with the nginx.METRIC_PORT when enable_prometheus_metrics is False
304- self.mock_config.return_value = {'enable_prometheus_metrics': False, 'sites': ngx_config}
305+ self.mock_config.return_value = {'enable_prometheus_metrics': False, 'max_connections': 8192, 'sites': ngx_config}
306 opened_ports.return_value = {"80/tcp", "{0}/tcp".format(nginx.METRICS_PORT)}
307 content_cache.configure_haproxy()
308 close_port.assert_called_once_with(nginx.METRICS_PORT)
309@@ -1054,6 +1060,6 @@ site1.local:
310 # Test that haproxy calls open_port with the nginx.METRIC_PORT when enable_prometheus_metrics is True
311 close_port.reset_mock()
312 open_port.reset_mock()
313- self.mock_config.return_value = {'enable_prometheus_metrics': True, 'sites': ngx_config}
314+ self.mock_config.return_value = {'enable_prometheus_metrics': True, 'max_connections': 8192, 'sites': ngx_config}
315 content_cache.configure_haproxy()
316 close_port.assert_not_called()
317diff --git a/tests/unit/test_haproxy.py b/tests/unit/test_haproxy.py
318index bd2fe7f..f8a8a07 100644
319--- a/tests/unit/test_haproxy.py
320+++ b/tests/unit/test_haproxy.py
321@@ -70,7 +70,7 @@ class TestLibHAProxy(unittest.TestCase):
322 )
323
324 def test_haproxy_config_rendered_listen_stanzas(self):
325- haproxy = HAProxy.HAProxyConf(self.tmpdir)
326+ haproxy = HAProxy.HAProxyConf(self.tmpdir, max_connections=8192)
327 config = self.site_config
328 output = 'tests/unit/files/haproxy_config_rendered_listen_stanzas_test_output.txt'
329 with open(output, 'r', encoding='utf-8') as f:
330@@ -102,7 +102,7 @@ class TestLibHAProxy(unittest.TestCase):
331
332 @freezegun.freeze_time("2019-03-22", tz_offset=0)
333 def test_haproxy_config_rendered_full_config(self):
334- haproxy = HAProxy.HAProxyConf(self.tmpdir)
335+ haproxy = HAProxy.HAProxyConf(self.tmpdir, max_connections=8192)
336 config = self.site_config
337 num_threads = 4
338 tls_cipher_suites = 'ECDH+AESGCM:!aNULL:!MD5:!DSS'

Subscribers

People subscribed via source and target branches