Merge ~ballot/content-cache-charm/+git/content-cache-charm:add_lua into content-cache-charm:master

Proposed by Benjamin Allot
Status: Merged
Approved by: Benjamin Allot
Approved revision: a388316ee133b9454a3298c9d464a6df9386c9f0
Merged at revision: 3fa2dbc5cd782969d2b2bd3011f50275f4871dae
Proposed branch: ~ballot/content-cache-charm/+git/content-cache-charm:add_lua
Merge into: content-cache-charm:master
Diff against target: 624 lines (+388/-11)
10 files modified
config.yaml (+6/-0)
lib/nginx.py (+75/-6)
nginx_config_rendered_test_output-nginx_metrics.txt (+32/-0)
reactive/content_cache.py (+36/-0)
templates/nginx_cfg.tmpl (+8/-0)
templates/nginx_metrics_cfg.tmpl (+23/-0)
tests/unit/files/config_test_basic_config.txt (+6/-0)
tests/unit/files/nginx_config_rendered_test_output-basic_site.txt (+32/-0)
tests/unit/test_content_cache.py (+87/-3)
tests/unit/test_nginx.py (+83/-2)
Reviewer Review Type Date Requested Status
Joel Sing (community) +1 Approve
Canonical IS Reviewers Pending
Review via email: mp+372309@code.launchpad.net

Commit message

Add nginx metrics for cache hits

A new option `enable_prometheus_metrics` can be set to True to expose nginx metrics about cache hits and HTTP requests in prometheus format.
This metrics are available on port 9145.

Update tests

All the tests have been adapted with the new bits of configuration and
new tests have been developed for the new feature.

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) :
review: Needs Fixing
Revision history for this message
Benjamin Allot (ballot) wrote :

Ah damn, I was rereading yesterday night but I missed quite a lot of consistency/debug stnzas. My bad.

About the separated tests:
I had done so in my previous configuration. However, it had his share of probem:
* a lot lot of files needed to be modified without adding any value (listen, backend haproxy ones notably)
* I had introduced a specific clause based on the name of the key to inject the enable_prometheus_metrics boolean to True. I found that error prone
After weighting the pros and cons, I ended up doing a separate file to test only the part relevant to the metrics exposure.

About the noqa, I had an issue using the form
```
patcher = mock.patch.multiple(....
patcher.start()
.
.
patcher.stop()
```

For some reason, the patcher.stop() was not removing the patch and I was ending with others tests failing.
I spent time trying to figure out why but could not find out so I use the context manager since it was "working".
I'm still looking for a more elegant solution to handle this but I figured it should not block the progress.

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

LGTM - per previous comment, before landing please rename config_test_nginx_metrics_config.txt (and site_with_nginx_metrics).

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

Change successfully merged at revision 3fa2dbc5cd782969d2b2bd3011f50275f4871dae

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 7f6d083..cdc2408 100644
3--- a/config.yaml
4+++ b/config.yaml
5@@ -40,3 +40,9 @@ options:
6 origin-headers:
7 X-Origin-Key: my-origin-secret-key
8 signed-url-hmac-key: my-signed-url-secret-key
9+ enable_prometheus_metrics:
10+ default: true
11+ type: boolean
12+ description: |
13+ Export metrics for the number of requests and the number of cache hits.
14+ Prometheus metrics are exposed at /metrics on port 9145.
15diff --git a/lib/nginx.py b/lib/nginx.py
16index 055acc8..48ef4a9 100644
17--- a/lib/nginx.py
18+++ b/lib/nginx.py
19@@ -6,8 +6,10 @@ import jinja2
20 from lib import utils
21
22
23-NGINX_BASE_PATH = '/etc/nginx'
24 INDENT = ' ' * 4
25+METRICS_PORT = 9145
26+METRICS_SITE = 'nginx_metrics'
27+NGINX_BASE_PATH = '/etc/nginx'
28 # Subset of http://nginx.org/en/docs/http/ngx_http_proxy_module.html
29 PROXY_CACHE_DEFAULTS = {
30 'background-update': 'on',
31@@ -23,8 +25,17 @@ class NginxConf:
32 def __init__(self, conf_path=None):
33 if not conf_path:
34 conf_path = NGINX_BASE_PATH
35- self._conf_path = os.path.join(conf_path, 'conf.d')
36- self._sites_path = os.path.join(conf_path, 'sites-available')
37+ self._base_path = conf_path
38+ self._conf_path = os.path.join(self.base_path, 'conf.d')
39+ self._sites_path = os.path.join(self.base_path, 'sites-available')
40+ script_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
41+ self.jinja_env = jinja2.Environment(loader=jinja2.FileSystemLoader(script_dir))
42+
43+ # Expose base_path as a property to allow mocking in indirect calls to
44+ # this class.
45+ @property
46+ def base_path(self):
47+ return self._base_path
48
49 # Expose conf_path as a property to allow mocking in indirect calls to
50 # this class.
51@@ -103,13 +114,71 @@ class NginxConf:
52 data = {
53 'address': conf['listen_address'],
54 'cache_max_size': conf['cache_max_size'],
55+ 'enable_prometheus_metrics': conf['enable_prometheus_metrics'],
56 'cache_path': conf['cache_path'],
57 'locations': self._process_locations(conf['locations']),
58 'name': self._generate_name(conf['site']),
59 'port': conf['listen_port'],
60 'site': conf['site'],
61 }
62- base = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
63- env = jinja2.Environment(loader=jinja2.FileSystemLoader(base))
64- template = env.get_template('templates/nginx_cfg.tmpl')
65+ template = self.jinja_env.get_template('templates/nginx_cfg.tmpl')
66 return template.render(data)
67+
68+ def _remove_metrics_site(self, available, enabled):
69+ """Remove the configuration exposing metrics.
70+
71+ :param str available: Path of the "available" site exposing the metrics
72+ :param str enabled: Path of the "enabled" symlink to the "available" configuration
73+ :returns: True if any change was made, False otherwise
74+ :rtype: bool
75+ """
76+ changed = False
77+ try:
78+ os.remove(available)
79+ changed = True
80+ except FileNotFoundError:
81+ pass
82+ try:
83+ os.remove(enabled)
84+ changed = True
85+ except FileNotFoundError:
86+ pass
87+
88+ return changed
89+
90+ def toggle_metrics_site(self, enable_prometheus_metrics):
91+ """Create/delete the metrics site configuration and links.
92+
93+ :param bool enable_prometheus_metrics: True if metrics are exposed to prometheus
94+ :returns: True if any change was made, False otherwise
95+ :rtype: bool
96+ """
97+ changed = False
98+ metrics_site_conf = '{0}.conf'.format(METRICS_SITE)
99+ available = os.path.join(self.sites_path, metrics_site_conf)
100+ enabled = os.path.join(self.base_path, 'sites-enabled', metrics_site_conf)
101+ # If no cache metrics, remove the site
102+ if not enable_prometheus_metrics:
103+ return self._remove_metrics_site(available, enabled)
104+ template = self.jinja_env.get_template('templates/nginx_metrics_cfg.tmpl')
105+ content = template.render({'nginx_conf_path': self.conf_path, 'port': METRICS_PORT})
106+ # Check if contents changed
107+ try:
108+ with open(available, 'r', encoding='utf-8') as f:
109+ current = f.read()
110+ except FileNotFoundError:
111+ current = ''
112+ if content != current:
113+ with open(available, 'w', encoding='utf-8') as f:
114+ f.write(content)
115+ changed = True
116+ os.listdir(self.sites_path)
117+ if not os.path.exists(enabled):
118+ os.symlink(available, enabled)
119+ changed = True
120+ if os.path.realpath(available) != os.path.realpath(enabled):
121+ os.remove(enabled)
122+ os.symlink(available, enabled)
123+ changed = True
124+
125+ return changed
126diff --git a/nginx_config_rendered_test_output-nginx_metrics.txt b/nginx_config_rendered_test_output-nginx_metrics.txt
127new file mode 100644
128index 0000000..a58a258
129--- /dev/null
130+++ b/nginx_config_rendered_test_output-nginx_metrics.txt
131@@ -0,0 +1,32 @@
132+proxy_cache_path /var/lib/nginx/proxy/nginx_metrics use_temp_path=off levels=1:2 keys_zone=nginx_metrics-cache:10m max_size=1g;
133+
134+server {
135+ server_name nginx_metrics;
136+ listen 127.0.0.1:6080;
137+
138+ port_in_redirect off;
139+ absolute_redirect off;
140+
141+ location / {
142+ proxy_pass http://localhost:8080;
143+ proxy_set_header Host "nginx_metrics";
144+ proxy_cache nginx_metrics-cache;
145+ proxy_cache_background_update on;
146+ proxy_cache_lock on;
147+ proxy_cache_min_uses 1;
148+ proxy_cache_revalidate on;
149+ proxy_cache_use_stale error timeout updating http_500 http_502 http_503 http_504;
150+ proxy_cache_valid 200 1d;
151+
152+ log_by_lua_block {
153+ http_request_total:inc(1, {ngx.var.server_name, ngx.var.status})
154+ if ngx.var.upstream_cache_status == "HIT" then
155+ cache_request_hit_total:inc(1, {ngx.var.server_name})
156+ end
157+ }
158+ }
159+
160+
161+ access_log /var/log/nginx/nginx_metrics-access.log content_cache;
162+ error_log /var/log/nginx/nginx_metrics-error.log;
163+}
164diff --git a/reactive/content_cache.py b/reactive/content_cache.py
165index 03cd7bf..ae90822 100644
166--- a/reactive/content_cache.py
167+++ b/reactive/content_cache.py
168@@ -65,6 +65,32 @@ def service_start_or_restart(name):
169 host.service_start(name)
170
171
172+def configure_nginx_metrics(ngx_conf, enable_prometheus_metrics):
173+ """Configure nginx to expose metrics.
174+
175+ Create the dedicated server exposing the metrics and add the logging of the cache hits for the other sites.
176+
177+ :param bool enable_prometheus_metrics: True is the metrics should be exposed, False otherwise
178+ :returns: True if any change was made, False otherwise
179+ :rtype: bool
180+ """
181+ changed = False
182+ if copy_file('files/prometheus.lua', os.path.join(ngx_conf.conf_path, 'prometheus.lua')):
183+ changed = True
184+ if ngx_conf.toggle_metrics_site(enable_prometheus_metrics):
185+ changed = True
186+ old_ports = [int(port.split('/')[0]) for port in hookenv.opened_ports()]
187+ hookenv.log("Current opened ports: {}".format(old_ports))
188+ if enable_prometheus_metrics and nginx.METRICS_PORT not in old_ports:
189+ hookenv.log("Opening port {0}".format(nginx.METRICS_PORT))
190+ hookenv.open_port(nginx.METRICS_PORT, 'TCP')
191+ elif not enable_prometheus_metrics and nginx.METRICS_PORT in old_ports:
192+ hookenv.log("Closing port {0}".format(nginx.METRICS_PORT))
193+ hookenv.close_port(nginx.METRICS_PORT, 'TCP')
194+
195+ return changed
196+
197+
198 @reactive.when_not('content_cache.nginx.configured')
199 def configure_nginx(conf_path=None):
200 status.maintenance('setting up Nginx as caching layer')
201@@ -76,6 +102,8 @@ def configure_nginx(conf_path=None):
202 status.blocked('requires list of sites to configure')
203 return
204
205+ enable_prometheus_metrics = config.get('enable_prometheus_metrics', False)
206+
207 ngx_conf = nginx.NginxConf(conf_path)
208 sites_secrets = secrets_from_config(config.get('sites_secrets'))
209 sites = sites_from_config(config.get('sites'), sites_secrets)
210@@ -95,11 +123,19 @@ def configure_nginx(conf_path=None):
211 conf['site'] = site_conf.get('site-name') or site
212 conf['listen_port'] = site_conf['cache_port']
213 conf['locations'] = site_conf.get('locations', {})
214+ conf['enable_prometheus_metrics'] = enable_prometheus_metrics
215
216 if ngx_conf.write_site(site, ngx_conf.render(conf)):
217 hookenv.log('Wrote out new configs for site: {}'.format(site))
218 changed = True
219
220+ if configure_nginx_metrics(ngx_conf, enable_prometheus_metrics):
221+ hookenv.log('nginx metrics exposed to prometheus')
222+ changed = True
223+
224+ # Include the site exposing metrics if needed
225+ if enable_prometheus_metrics:
226+ sites[nginx.METRICS_SITE] = None
227 if ngx_conf.sync_sites(sites.keys()):
228 hookenv.log('Enabled sites: {}'.format(' '.join(sites.keys())))
229 changed = True
230diff --git a/templates/nginx_cfg.tmpl b/templates/nginx_cfg.tmpl
231index 18d5488..031b3e0 100644
232--- a/templates/nginx_cfg.tmpl
233+++ b/templates/nginx_cfg.tmpl
234@@ -100,7 +100,15 @@ server {
235 args["token"] = nil
236 ngx.req.set_uri_args(args)
237 }
238+{%- endif %}
239+{%- if enable_prometheus_metrics %}
240
241+ log_by_lua_block {
242+ http_request_total:inc(1, {ngx.var.server_name, ngx.var.status})
243+ if ngx.var.upstream_cache_status == "HIT" then
244+ cache_request_hit_total:inc(1, {ngx.var.server_name})
245+ end
246+ }
247 {%- endif %}
248 }
249 {% endfor %}
250diff --git a/templates/nginx_metrics_cfg.tmpl b/templates/nginx_metrics_cfg.tmpl
251new file mode 100644
252index 0000000..e048ffc
253--- /dev/null
254+++ b/templates/nginx_metrics_cfg.tmpl
255@@ -0,0 +1,23 @@
256+lua_shared_dict prometheus_metrics 10M;
257+lua_package_path "{{ nginx_conf_path }}/?.lua";
258+init_by_lua_block {
259+ prometheus = require("prometheus").init("prometheus_metrics")
260+ http_request_total = prometheus:counter(
261+ "nginx_http_request_total", "Number of HTTP requests per site", {"host", "status"})
262+ cache_request_hit_total = prometheus:counter(
263+ "nginx_cache_request_hit_total", "Number of cache hits per site", {"host"})
264+}
265+
266+server {
267+ listen {{ port }};
268+ allow all;
269+ location /metrics {
270+ content_by_lua_block {
271+ prometheus:collect()
272+ }
273+ }
274+
275+ access_log /var/log/nginx/nginx_metrics-access.log;
276+ error_log /var/log/nginx/nginx_metrics-error.log;
277+}
278+
279diff --git a/tests/unit/files/config_test_basic_config.txt b/tests/unit/files/config_test_basic_config.txt
280new file mode 100644
281index 0000000..6707563
282--- /dev/null
283+++ b/tests/unit/files/config_test_basic_config.txt
284@@ -0,0 +1,6 @@
285+basic_site:
286+ port: 80
287+ locations:
288+ /:
289+ backends:
290+ - 127.0.1.10:80
291diff --git a/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt b/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt
292new file mode 100644
293index 0000000..90693e5
294--- /dev/null
295+++ b/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt
296@@ -0,0 +1,32 @@
297+proxy_cache_path /var/lib/nginx/proxy/basic_site use_temp_path=off levels=1:2 keys_zone=basic_site-cache:10m max_size=1g;
298+
299+server {
300+ server_name basic_site;
301+ listen 127.0.0.1:6080;
302+
303+ port_in_redirect off;
304+ absolute_redirect off;
305+
306+ location / {
307+ proxy_pass http://localhost:8080;
308+ proxy_set_header Host "basic_site";
309+ proxy_cache basic_site-cache;
310+ proxy_cache_background_update on;
311+ proxy_cache_lock on;
312+ proxy_cache_min_uses 1;
313+ proxy_cache_revalidate on;
314+ proxy_cache_use_stale error timeout updating http_500 http_502 http_503 http_504;
315+ proxy_cache_valid 200 1d;
316+
317+ log_by_lua_block {
318+ http_request_total:inc(1, {ngx.var.server_name, ngx.var.status})
319+ if ngx.var.upstream_cache_status == "HIT" then
320+ cache_request_hit_total:inc(1, {ngx.var.server_name})
321+ end
322+ }
323+ }
324+
325+
326+ access_log /var/log/nginx/basic_site-access.log content_cache;
327+ error_log /var/log/nginx/basic_site-error.log;
328+}
329diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
330index 3ac3749..c071022 100644
331--- a/tests/unit/test_content_cache.py
332+++ b/tests/unit/test_content_cache.py
333@@ -6,6 +6,7 @@ import unittest
334 from unittest import mock
335
336 import freezegun
337+import jinja2
338
339 # We also need to mock up charms.layer so we can run unit tests without having
340 # to build the charm and pull in layers such as layer-status.
341@@ -16,6 +17,7 @@ from charms.layer import status # NOQA: E402
342 # Add path to where our reactive layer lives and import.
343 sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))))
344 from reactive import content_cache # NOQA: E402
345+from lib import nginx # NOQA: E402
346
347
348 class TestCharm(unittest.TestCase):
349@@ -131,14 +133,17 @@ class TestCharm(unittest.TestCase):
350 self.assertFalse(status.blocked.assert_called())
351 self.assertFalse(clear_flag.assert_called_once_with('content_cache.active'))
352
353+ @mock.patch('charmhelpers.core.hookenv.close_port')
354+ @mock.patch('charmhelpers.core.hookenv.opened_ports')
355 @mock.patch('reactive.content_cache.service_start_or_restart')
356- def test_configure_nginx_sites(self, service_start_or_restart):
357+ def test_configure_nginx_sites(self, service_start_or_restart, opened_ports, close_port):
358 '''Test configuration of Nginx sites'''
359 with open('tests/unit/files/config_test_config.txt', 'r', encoding='utf-8') as f:
360 ngx_config = f.read()
361 self.mock_config.return_value = {
362 'cache_max_size': '1g',
363 'cache_path': '/var/lib/nginx/proxy',
364+ 'enable_prometheus_metrics': False,
365 'sites': ngx_config,
366 }
367
368@@ -149,13 +154,18 @@ class TestCharm(unittest.TestCase):
369 os.mkdir(os.path.join(self.tmpdir, 'conf.d'))
370 os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
371 os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
372+ opened_ports.return_value = ['80/tcp', '{0}/tcp'.format(nginx.METRICS_PORT)]
373 content_cache.configure_nginx(self.tmpdir)
374 self.assertFalse(service_start_or_restart.assert_called_once_with('nginx'))
375+ close_port.assert_called_once_with(nginx.METRICS_PORT, 'TCP')
376
377 # Re-run with same set of sites, no change so shouldn't need to restart Nginx
378 service_start_or_restart.reset_mock()
379+ close_port.reset_mock()
380+ opened_ports.return_value = ['80/tcp']
381 content_cache.configure_nginx(self.tmpdir)
382 self.assertFalse(service_start_or_restart.assert_not_called())
383+ close_port.assert_not_called()
384
385 for site in [
386 'site1.local',
387@@ -165,6 +175,7 @@ class TestCharm(unittest.TestCase):
388 'site5',
389 'site6.local',
390 'site7.local',
391+ 'site8.local',
392 ]:
393 with open(
394 'tests/unit/files/nginx_config_rendered_test_output-{}.txt'.format(site), 'r', encoding='utf-8'
395@@ -183,8 +194,10 @@ class TestCharm(unittest.TestCase):
396 got = f.read()
397 self.assertEqual(got, want)
398
399+ @mock.patch('charmhelpers.core.hookenv.close_port')
400+ @mock.patch('charmhelpers.core.hookenv.opened_ports')
401 @mock.patch('reactive.content_cache.service_start_or_restart')
402- def test_configure_nginx_sites_secrets(self, service_start_or_restart):
403+ def test_configure_nginx_sites_secrets(self, service_start_or_restart, opened_ports, close_port):
404 with open('tests/unit/files/config_test_secrets.txt', 'r', encoding='utf-8') as f:
405 secrets = f.read()
406 config = '''
407@@ -227,9 +240,11 @@ site1.local:
408 got = f.read()
409 self.assertEqual(got, want)
410
411+ @mock.patch('charmhelpers.core.hookenv.close_port')
412+ @mock.patch('charmhelpers.core.hookenv.opened_ports')
413 @mock.patch('shutil.disk_usage')
414 @mock.patch('reactive.content_cache.service_start_or_restart')
415- def test_configure_nginx_cache_config(self, service_start_or_restart, disk_usage):
416+ def test_configure_nginx_cache_config(self, service_start_or_restart, disk_usage, opened_ports, close_port):
417 config = '''
418 site1.local:
419 locations:
420@@ -671,3 +686,72 @@ site1.local:
421 content='test content\n', group='somegroup', owner='somedude', path=dest, perms=444
422 )
423 )
424+
425+ @mock.patch('charmhelpers.core.hookenv.open_port')
426+ @mock.patch('charmhelpers.core.hookenv.opened_ports')
427+ @mock.patch('reactive.content_cache.service_start_or_restart')
428+ def test_configure_nginx_metrics_sites(self, service_start_or_restart, opened_ports, open_port):
429+ """Test configuration of Nginx sites with enable_prometheus_metrics activated."""
430+ with open('tests/unit/files/config_test_basic_config.txt', 'r', encoding='utf-8') as f:
431+ ngx_config = f.read()
432+ self.mock_config.return_value = {
433+ 'cache_max_size': '1g',
434+ 'enable_prometheus_metrics': True,
435+ 'cache_path': '/var/lib/nginx/proxy',
436+ 'sites': ngx_config,
437+ }
438+
439+ with mock.patch.multiple(
440+ 'lib.nginx.NginxConf',
441+ sites_path=os.path.join(self.tmpdir, 'sites-available'),
442+ base_path=self.tmpdir,
443+ ) as nginxconf_mock: # noqa: F841
444+ # conf.d, sites-available, and sites-enabled won't exist in our
445+ # temporary directory.
446+ os.mkdir(os.path.join(self.tmpdir, 'conf.d'))
447+ os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
448+ os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
449+ opened_ports.return_value = ['80/tcp', '443/tcp']
450+ content_cache.configure_nginx(self.tmpdir)
451+ self.assertFalse(service_start_or_restart.assert_called_once_with('nginx'))
452+ open_port.assert_called_once_with(nginx.METRICS_PORT, 'TCP')
453+
454+ # Re-run with same set of sites, no change so shouldn't need to restart Nginx
455+ service_start_or_restart.reset_mock()
456+ open_port.reset_mock()
457+ opened_ports.return_value = ['80/tcp', '443/tcp', '{0}/tcp'.format(nginx.METRICS_PORT)]
458+ content_cache.configure_nginx(self.tmpdir)
459+ self.assertFalse(service_start_or_restart.assert_not_called())
460+ open_port.assert_not_called()
461+
462+ # Test the site with cache HIT logging
463+ site = 'basic_site'
464+ test_file = 'tests/unit/files/nginx_config_rendered_test_output-{0}.txt'.format(site)
465+ with open(test_file, 'r', encoding='utf-8') as f:
466+ want = f.read()
467+
468+ test_file = os.path.join(self.tmpdir, 'sites-available/{0}.conf'.format(site))
469+ with open(test_file, 'r', encoding='utf-8') as f:
470+ got = f.read()
471+ self.assertEqual(got, want)
472+
473+ # Test the site exposing the metrics
474+ site = 'nginx_metrics'
475+ script_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..')
476+ jinja_env = jinja2.Environment(loader=jinja2.FileSystemLoader(script_dir))
477+ template = jinja_env.get_template('templates/nginx_metrics_cfg.tmpl')
478+ content = template.render({
479+ 'nginx_conf_path': os.path.join(self.tmpdir, 'conf.d'),
480+ 'port': nginx.METRICS_PORT})
481+ want = content
482+ test_file = os.path.join(self.tmpdir, 'sites-available/{0}.conf'.format(nginx.METRICS_SITE))
483+ with open(test_file, 'r', encoding='utf-8') as f:
484+ got = f.read()
485+ self.assertEqual(got, want)
486+
487+ # Prometheus.lua library
488+ with open('files/prometheus.lua', 'r') as f:
489+ want = f.read()
490+ with open(os.path.join(self.tmpdir, 'conf.d', 'prometheus.lua'), 'r') as f:
491+ got = f.read()
492+ self.assertEqual(got, want)
493diff --git a/tests/unit/test_nginx.py b/tests/unit/test_nginx.py
494index e9ec2f1..7097f7e 100644
495--- a/tests/unit/test_nginx.py
496+++ b/tests/unit/test_nginx.py
497@@ -5,6 +5,8 @@ import tempfile
498 import unittest
499 import yaml
500
501+import jinja2
502+
503 sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))))
504 from lib import nginx # NOQA: E402
505
506@@ -21,10 +23,13 @@ class TestLibNginx(unittest.TestCase):
507
508 def test_nginx_config_conf_path(self):
509 conf_path = '/etc/nginx/conf.d'
510+ alternate_conf_path = '/var/lib/snap/nginx/etc'
511 ngx_conf = nginx.NginxConf()
512 self.assertEqual(ngx_conf.conf_path, conf_path)
513 ngx_conf = nginx.NginxConf(None)
514 self.assertEqual(ngx_conf.conf_path, conf_path)
515+ ngx_conf = nginx.NginxConf(alternate_conf_path)
516+ self.assertEqual(ngx_conf.base_path, alternate_conf_path)
517
518 def test_nginx_config_sites_path(self):
519 sites_path = '/etc/nginx/sites-available'
520@@ -34,7 +39,7 @@ class TestLibNginx(unittest.TestCase):
521 self.assertEqual(ngx_conf.sites_path, sites_path)
522
523 def test_nginx_config_render(self):
524- '''Test parsing a YAML-formatted list of sites'''
525+ """Test parsing a YAML-formatted list of sites."""
526 ngx_conf = nginx.NginxConf()
527
528 with open('tests/unit/files/config_test_config.txt', 'r', encoding='utf-8') as f:
529@@ -42,6 +47,7 @@ class TestLibNginx(unittest.TestCase):
530
531 conf = {}
532 conf['cache_max_size'] = '1g'
533+ conf['enable_prometheus_metrics'] = False
534 conf['cache_path'] = '/var/lib/nginx/proxy'
535 conf['listen_address'] = '127.0.0.1'
536 # From the given YAML-formatted list of sites, check that each individual
537@@ -66,7 +72,7 @@ class TestLibNginx(unittest.TestCase):
538 self.assertEqual(ngx_conf.render(conf), output)
539
540 def test_nginx_config_write_sites(self):
541- '''Test writing out sites to individual Nginx site config files'''
542+ """Test writing out sites to individual Nginx site config files."""
543 ngx_conf = nginx.NginxConf(self.tmpdir)
544 os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
545 os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
546@@ -113,3 +119,78 @@ class TestLibNginx(unittest.TestCase):
547
548 # Re-run, no change this time.
549 self.assertFalse(ngx_conf.sync_sites(['site1.local', 'site2.local']))
550+
551+ def test_nginx_config_render_with_metrics(self):
552+ """Test rendering with metrics exposed."""
553+ ngx_conf = nginx.NginxConf()
554+
555+ with open('tests/unit/files/config_test_basic_config.txt', 'r', encoding='utf-8') as f:
556+ sites = yaml.safe_load(f.read())
557+
558+ conf = {
559+ 'cache_max_size': '1g',
560+ 'cache_path': '/var/lib/nginx/proxy',
561+ 'listen_address': '127.0.0.1',
562+ 'enable_prometheus_metrics': True,
563+ }
564+ for site, site_conf in sites.items():
565+ conf['site'] = site_conf.get('site-name') or site
566+ conf['listen_port'] = BASE_LISTEN_PORT
567+ conf['locations'] = site_conf.get('locations', {})
568+
569+ for location, loc_conf in conf['locations'].items():
570+ if loc_conf.get('backends'):
571+ loc_conf['backend_port'] = BASE_BACKEND_PORT
572+
573+ output_file = 'tests/unit/files/nginx_config_rendered_test_output-{}.txt'.format(site)
574+ with open(output_file, 'r', encoding='utf-8') as f:
575+ output = f.read()
576+
577+ self.assertEqual(ngx_conf.render(conf), output)
578+
579+ def test_nginx_config_toggle_metrics_site(self):
580+ """Test the metrics site.
581+
582+ Check that the activation fo the cache metrics activate the dedicated site for exposing prometheus metrics.
583+ """
584+ ngx_conf = nginx.NginxConf(self.tmpdir)
585+ os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
586+ os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
587+
588+ metrics_site_conf = '{0}.conf'.format(nginx.METRICS_SITE)
589+
590+ script_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..')
591+ env = jinja2.Environment(loader=jinja2.FileSystemLoader(script_dir))
592+ template = env.get_template('templates/nginx_metrics_cfg.tmpl')
593+ content = template.render({
594+ 'nginx_conf_path': os.path.join(self.tmpdir, 'conf.d'),
595+ 'port': nginx.METRICS_PORT})
596+ nginx_metrics_conf = content
597+
598+ metrics_site_available = os.path.join(self.tmpdir, 'sites-available', metrics_site_conf)
599+ metrics_site_enabled = os.path.join(self.tmpdir, 'sites-enabled', metrics_site_conf)
600+ self.assertTrue(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=True))
601+ # Write again with same contents, this time it should return 'False'
602+ # as there's no change, thus no need to restart/reload Nginx.
603+ self.assertFalse(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=True))
604+ self.assertTrue(os.path.exists(metrics_site_available))
605+
606+ # Compare what's been written out matches what's in tests/unit/files.
607+ with open(metrics_site_available, 'r', encoding='utf-8') as f:
608+ output = f.read()
609+ self.assertEqual(nginx_metrics_conf, output)
610+
611+ # Check link existence
612+ self.assertTrue(os.path.islink(metrics_site_enabled))
613+
614+ # Mess up with the target of the link and check that a toggle fixes it
615+ os.remove(metrics_site_enabled)
616+ os.symlink('/dev/null', metrics_site_enabled)
617+ self.assertTrue(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=True))
618+ self.assertTrue(os.path.realpath(metrics_site_available) == os.path.realpath(metrics_site_enabled))
619+
620+ # Remove the site
621+ self.assertTrue(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=False))
622+ self.assertFalse(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=False))
623+ self.assertFalse(os.path.exists(metrics_site_available))
624+ self.assertFalse(os.path.exists(metrics_site_enabled))

Subscribers

People subscribed via source and target branches