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
diff --git a/config.yaml b/config.yaml
index 7f6d083..cdc2408 100644
--- a/config.yaml
+++ b/config.yaml
@@ -40,3 +40,9 @@ options:
40 origin-headers:40 origin-headers:
41 X-Origin-Key: my-origin-secret-key41 X-Origin-Key: my-origin-secret-key
42 signed-url-hmac-key: my-signed-url-secret-key42 signed-url-hmac-key: my-signed-url-secret-key
43 enable_prometheus_metrics:
44 default: true
45 type: boolean
46 description: |
47 Export metrics for the number of requests and the number of cache hits.
48 Prometheus metrics are exposed at /metrics on port 9145.
diff --git a/lib/nginx.py b/lib/nginx.py
index 055acc8..48ef4a9 100644
--- a/lib/nginx.py
+++ b/lib/nginx.py
@@ -6,8 +6,10 @@ import jinja2
6from lib import utils6from lib import utils
77
88
9NGINX_BASE_PATH = '/etc/nginx'
10INDENT = ' ' * 49INDENT = ' ' * 4
10METRICS_PORT = 9145
11METRICS_SITE = 'nginx_metrics'
12NGINX_BASE_PATH = '/etc/nginx'
11# Subset of http://nginx.org/en/docs/http/ngx_http_proxy_module.html13# Subset of http://nginx.org/en/docs/http/ngx_http_proxy_module.html
12PROXY_CACHE_DEFAULTS = {14PROXY_CACHE_DEFAULTS = {
13 'background-update': 'on',15 'background-update': 'on',
@@ -23,8 +25,17 @@ class NginxConf:
23 def __init__(self, conf_path=None):25 def __init__(self, conf_path=None):
24 if not conf_path:26 if not conf_path:
25 conf_path = NGINX_BASE_PATH27 conf_path = NGINX_BASE_PATH
26 self._conf_path = os.path.join(conf_path, 'conf.d')28 self._base_path = conf_path
27 self._sites_path = os.path.join(conf_path, 'sites-available')29 self._conf_path = os.path.join(self.base_path, 'conf.d')
30 self._sites_path = os.path.join(self.base_path, 'sites-available')
31 script_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
32 self.jinja_env = jinja2.Environment(loader=jinja2.FileSystemLoader(script_dir))
33
34 # Expose base_path as a property to allow mocking in indirect calls to
35 # this class.
36 @property
37 def base_path(self):
38 return self._base_path
2839
29 # Expose conf_path as a property to allow mocking in indirect calls to40 # Expose conf_path as a property to allow mocking in indirect calls to
30 # this class.41 # this class.
@@ -103,13 +114,71 @@ class NginxConf:
103 data = {114 data = {
104 'address': conf['listen_address'],115 'address': conf['listen_address'],
105 'cache_max_size': conf['cache_max_size'],116 'cache_max_size': conf['cache_max_size'],
117 'enable_prometheus_metrics': conf['enable_prometheus_metrics'],
106 'cache_path': conf['cache_path'],118 'cache_path': conf['cache_path'],
107 'locations': self._process_locations(conf['locations']),119 'locations': self._process_locations(conf['locations']),
108 'name': self._generate_name(conf['site']),120 'name': self._generate_name(conf['site']),
109 'port': conf['listen_port'],121 'port': conf['listen_port'],
110 'site': conf['site'],122 'site': conf['site'],
111 }123 }
112 base = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))124 template = self.jinja_env.get_template('templates/nginx_cfg.tmpl')
113 env = jinja2.Environment(loader=jinja2.FileSystemLoader(base))
114 template = env.get_template('templates/nginx_cfg.tmpl')
115 return template.render(data)125 return template.render(data)
126
127 def _remove_metrics_site(self, available, enabled):
128 """Remove the configuration exposing metrics.
129
130 :param str available: Path of the "available" site exposing the metrics
131 :param str enabled: Path of the "enabled" symlink to the "available" configuration
132 :returns: True if any change was made, False otherwise
133 :rtype: bool
134 """
135 changed = False
136 try:
137 os.remove(available)
138 changed = True
139 except FileNotFoundError:
140 pass
141 try:
142 os.remove(enabled)
143 changed = True
144 except FileNotFoundError:
145 pass
146
147 return changed
148
149 def toggle_metrics_site(self, enable_prometheus_metrics):
150 """Create/delete the metrics site configuration and links.
151
152 :param bool enable_prometheus_metrics: True if metrics are exposed to prometheus
153 :returns: True if any change was made, False otherwise
154 :rtype: bool
155 """
156 changed = False
157 metrics_site_conf = '{0}.conf'.format(METRICS_SITE)
158 available = os.path.join(self.sites_path, metrics_site_conf)
159 enabled = os.path.join(self.base_path, 'sites-enabled', metrics_site_conf)
160 # If no cache metrics, remove the site
161 if not enable_prometheus_metrics:
162 return self._remove_metrics_site(available, enabled)
163 template = self.jinja_env.get_template('templates/nginx_metrics_cfg.tmpl')
164 content = template.render({'nginx_conf_path': self.conf_path, 'port': METRICS_PORT})
165 # Check if contents changed
166 try:
167 with open(available, 'r', encoding='utf-8') as f:
168 current = f.read()
169 except FileNotFoundError:
170 current = ''
171 if content != current:
172 with open(available, 'w', encoding='utf-8') as f:
173 f.write(content)
174 changed = True
175 os.listdir(self.sites_path)
176 if not os.path.exists(enabled):
177 os.symlink(available, enabled)
178 changed = True
179 if os.path.realpath(available) != os.path.realpath(enabled):
180 os.remove(enabled)
181 os.symlink(available, enabled)
182 changed = True
183
184 return changed
diff --git a/nginx_config_rendered_test_output-nginx_metrics.txt b/nginx_config_rendered_test_output-nginx_metrics.txt
116new file mode 100644185new file mode 100644
index 0000000..a58a258
--- /dev/null
+++ b/nginx_config_rendered_test_output-nginx_metrics.txt
@@ -0,0 +1,32 @@
1proxy_cache_path /var/lib/nginx/proxy/nginx_metrics use_temp_path=off levels=1:2 keys_zone=nginx_metrics-cache:10m max_size=1g;
2
3server {
4 server_name nginx_metrics;
5 listen 127.0.0.1:6080;
6
7 port_in_redirect off;
8 absolute_redirect off;
9
10 location / {
11 proxy_pass http://localhost:8080;
12 proxy_set_header Host "nginx_metrics";
13 proxy_cache nginx_metrics-cache;
14 proxy_cache_background_update on;
15 proxy_cache_lock on;
16 proxy_cache_min_uses 1;
17 proxy_cache_revalidate on;
18 proxy_cache_use_stale error timeout updating http_500 http_502 http_503 http_504;
19 proxy_cache_valid 200 1d;
20
21 log_by_lua_block {
22 http_request_total:inc(1, {ngx.var.server_name, ngx.var.status})
23 if ngx.var.upstream_cache_status == "HIT" then
24 cache_request_hit_total:inc(1, {ngx.var.server_name})
25 end
26 }
27 }
28
29
30 access_log /var/log/nginx/nginx_metrics-access.log content_cache;
31 error_log /var/log/nginx/nginx_metrics-error.log;
32}
diff --git a/reactive/content_cache.py b/reactive/content_cache.py
index 03cd7bf..ae90822 100644
--- a/reactive/content_cache.py
+++ b/reactive/content_cache.py
@@ -65,6 +65,32 @@ def service_start_or_restart(name):
65 host.service_start(name)65 host.service_start(name)
6666
6767
68def configure_nginx_metrics(ngx_conf, enable_prometheus_metrics):
69 """Configure nginx to expose metrics.
70
71 Create the dedicated server exposing the metrics and add the logging of the cache hits for the other sites.
72
73 :param bool enable_prometheus_metrics: True is the metrics should be exposed, False otherwise
74 :returns: True if any change was made, False otherwise
75 :rtype: bool
76 """
77 changed = False
78 if copy_file('files/prometheus.lua', os.path.join(ngx_conf.conf_path, 'prometheus.lua')):
79 changed = True
80 if ngx_conf.toggle_metrics_site(enable_prometheus_metrics):
81 changed = True
82 old_ports = [int(port.split('/')[0]) for port in hookenv.opened_ports()]
83 hookenv.log("Current opened ports: {}".format(old_ports))
84 if enable_prometheus_metrics and nginx.METRICS_PORT not in old_ports:
85 hookenv.log("Opening port {0}".format(nginx.METRICS_PORT))
86 hookenv.open_port(nginx.METRICS_PORT, 'TCP')
87 elif not enable_prometheus_metrics and nginx.METRICS_PORT in old_ports:
88 hookenv.log("Closing port {0}".format(nginx.METRICS_PORT))
89 hookenv.close_port(nginx.METRICS_PORT, 'TCP')
90
91 return changed
92
93
68@reactive.when_not('content_cache.nginx.configured')94@reactive.when_not('content_cache.nginx.configured')
69def configure_nginx(conf_path=None):95def configure_nginx(conf_path=None):
70 status.maintenance('setting up Nginx as caching layer')96 status.maintenance('setting up Nginx as caching layer')
@@ -76,6 +102,8 @@ def configure_nginx(conf_path=None):
76 status.blocked('requires list of sites to configure')102 status.blocked('requires list of sites to configure')
77 return103 return
78104
105 enable_prometheus_metrics = config.get('enable_prometheus_metrics', False)
106
79 ngx_conf = nginx.NginxConf(conf_path)107 ngx_conf = nginx.NginxConf(conf_path)
80 sites_secrets = secrets_from_config(config.get('sites_secrets'))108 sites_secrets = secrets_from_config(config.get('sites_secrets'))
81 sites = sites_from_config(config.get('sites'), sites_secrets)109 sites = sites_from_config(config.get('sites'), sites_secrets)
@@ -95,11 +123,19 @@ def configure_nginx(conf_path=None):
95 conf['site'] = site_conf.get('site-name') or site123 conf['site'] = site_conf.get('site-name') or site
96 conf['listen_port'] = site_conf['cache_port']124 conf['listen_port'] = site_conf['cache_port']
97 conf['locations'] = site_conf.get('locations', {})125 conf['locations'] = site_conf.get('locations', {})
126 conf['enable_prometheus_metrics'] = enable_prometheus_metrics
98127
99 if ngx_conf.write_site(site, ngx_conf.render(conf)):128 if ngx_conf.write_site(site, ngx_conf.render(conf)):
100 hookenv.log('Wrote out new configs for site: {}'.format(site))129 hookenv.log('Wrote out new configs for site: {}'.format(site))
101 changed = True130 changed = True
102131
132 if configure_nginx_metrics(ngx_conf, enable_prometheus_metrics):
133 hookenv.log('nginx metrics exposed to prometheus')
134 changed = True
135
136 # Include the site exposing metrics if needed
137 if enable_prometheus_metrics:
138 sites[nginx.METRICS_SITE] = None
103 if ngx_conf.sync_sites(sites.keys()):139 if ngx_conf.sync_sites(sites.keys()):
104 hookenv.log('Enabled sites: {}'.format(' '.join(sites.keys())))140 hookenv.log('Enabled sites: {}'.format(' '.join(sites.keys())))
105 changed = True141 changed = True
diff --git a/templates/nginx_cfg.tmpl b/templates/nginx_cfg.tmpl
index 18d5488..031b3e0 100644
--- a/templates/nginx_cfg.tmpl
+++ b/templates/nginx_cfg.tmpl
@@ -100,7 +100,15 @@ server {
100 args["token"] = nil100 args["token"] = nil
101 ngx.req.set_uri_args(args)101 ngx.req.set_uri_args(args)
102 }102 }
103{%- endif %}
104{%- if enable_prometheus_metrics %}
103105
106 log_by_lua_block {
107 http_request_total:inc(1, {ngx.var.server_name, ngx.var.status})
108 if ngx.var.upstream_cache_status == "HIT" then
109 cache_request_hit_total:inc(1, {ngx.var.server_name})
110 end
111 }
104{%- endif %}112{%- endif %}
105 }113 }
106{% endfor %}114{% endfor %}
diff --git a/templates/nginx_metrics_cfg.tmpl b/templates/nginx_metrics_cfg.tmpl
107new file mode 100644115new file mode 100644
index 0000000..e048ffc
--- /dev/null
+++ b/templates/nginx_metrics_cfg.tmpl
@@ -0,0 +1,23 @@
1lua_shared_dict prometheus_metrics 10M;
2lua_package_path "{{ nginx_conf_path }}/?.lua";
3init_by_lua_block {
4 prometheus = require("prometheus").init("prometheus_metrics")
5 http_request_total = prometheus:counter(
6 "nginx_http_request_total", "Number of HTTP requests per site", {"host", "status"})
7 cache_request_hit_total = prometheus:counter(
8 "nginx_cache_request_hit_total", "Number of cache hits per site", {"host"})
9}
10
11server {
12 listen {{ port }};
13 allow all;
14 location /metrics {
15 content_by_lua_block {
16 prometheus:collect()
17 }
18 }
19
20 access_log /var/log/nginx/nginx_metrics-access.log;
21 error_log /var/log/nginx/nginx_metrics-error.log;
22}
23
diff --git a/tests/unit/files/config_test_basic_config.txt b/tests/unit/files/config_test_basic_config.txt
0new file mode 10064424new file mode 100644
index 0000000..6707563
--- /dev/null
+++ b/tests/unit/files/config_test_basic_config.txt
@@ -0,0 +1,6 @@
1basic_site:
2 port: 80
3 locations:
4 /:
5 backends:
6 - 127.0.1.10:80
diff --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
0new file mode 1006447new file mode 100644
index 0000000..90693e5
--- /dev/null
+++ b/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt
@@ -0,0 +1,32 @@
1proxy_cache_path /var/lib/nginx/proxy/basic_site use_temp_path=off levels=1:2 keys_zone=basic_site-cache:10m max_size=1g;
2
3server {
4 server_name basic_site;
5 listen 127.0.0.1:6080;
6
7 port_in_redirect off;
8 absolute_redirect off;
9
10 location / {
11 proxy_pass http://localhost:8080;
12 proxy_set_header Host "basic_site";
13 proxy_cache basic_site-cache;
14 proxy_cache_background_update on;
15 proxy_cache_lock on;
16 proxy_cache_min_uses 1;
17 proxy_cache_revalidate on;
18 proxy_cache_use_stale error timeout updating http_500 http_502 http_503 http_504;
19 proxy_cache_valid 200 1d;
20
21 log_by_lua_block {
22 http_request_total:inc(1, {ngx.var.server_name, ngx.var.status})
23 if ngx.var.upstream_cache_status == "HIT" then
24 cache_request_hit_total:inc(1, {ngx.var.server_name})
25 end
26 }
27 }
28
29
30 access_log /var/log/nginx/basic_site-access.log content_cache;
31 error_log /var/log/nginx/basic_site-error.log;
32}
diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
index 3ac3749..c071022 100644
--- a/tests/unit/test_content_cache.py
+++ b/tests/unit/test_content_cache.py
@@ -6,6 +6,7 @@ import unittest
6from unittest import mock6from unittest import mock
77
8import freezegun8import freezegun
9import jinja2
910
10# We also need to mock up charms.layer so we can run unit tests without having11# We also need to mock up charms.layer so we can run unit tests without having
11# to build the charm and pull in layers such as layer-status.12# to build the charm and pull in layers such as layer-status.
@@ -16,6 +17,7 @@ from charms.layer import status # NOQA: E402
16# Add path to where our reactive layer lives and import.17# Add path to where our reactive layer lives and import.
17sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))))18sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))))
18from reactive import content_cache # NOQA: E40219from reactive import content_cache # NOQA: E402
20from lib import nginx # NOQA: E402
1921
2022
21class TestCharm(unittest.TestCase):23class TestCharm(unittest.TestCase):
@@ -131,14 +133,17 @@ class TestCharm(unittest.TestCase):
131 self.assertFalse(status.blocked.assert_called())133 self.assertFalse(status.blocked.assert_called())
132 self.assertFalse(clear_flag.assert_called_once_with('content_cache.active'))134 self.assertFalse(clear_flag.assert_called_once_with('content_cache.active'))
133135
136 @mock.patch('charmhelpers.core.hookenv.close_port')
137 @mock.patch('charmhelpers.core.hookenv.opened_ports')
134 @mock.patch('reactive.content_cache.service_start_or_restart')138 @mock.patch('reactive.content_cache.service_start_or_restart')
135 def test_configure_nginx_sites(self, service_start_or_restart):139 def test_configure_nginx_sites(self, service_start_or_restart, opened_ports, close_port):
136 '''Test configuration of Nginx sites'''140 '''Test configuration of Nginx sites'''
137 with open('tests/unit/files/config_test_config.txt', 'r', encoding='utf-8') as f:141 with open('tests/unit/files/config_test_config.txt', 'r', encoding='utf-8') as f:
138 ngx_config = f.read()142 ngx_config = f.read()
139 self.mock_config.return_value = {143 self.mock_config.return_value = {
140 'cache_max_size': '1g',144 'cache_max_size': '1g',
141 'cache_path': '/var/lib/nginx/proxy',145 'cache_path': '/var/lib/nginx/proxy',
146 'enable_prometheus_metrics': False,
142 'sites': ngx_config,147 'sites': ngx_config,
143 }148 }
144149
@@ -149,13 +154,18 @@ class TestCharm(unittest.TestCase):
149 os.mkdir(os.path.join(self.tmpdir, 'conf.d'))154 os.mkdir(os.path.join(self.tmpdir, 'conf.d'))
150 os.mkdir(os.path.join(self.tmpdir, 'sites-available'))155 os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
151 os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))156 os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
157 opened_ports.return_value = ['80/tcp', '{0}/tcp'.format(nginx.METRICS_PORT)]
152 content_cache.configure_nginx(self.tmpdir)158 content_cache.configure_nginx(self.tmpdir)
153 self.assertFalse(service_start_or_restart.assert_called_once_with('nginx'))159 self.assertFalse(service_start_or_restart.assert_called_once_with('nginx'))
160 close_port.assert_called_once_with(nginx.METRICS_PORT, 'TCP')
154161
155 # Re-run with same set of sites, no change so shouldn't need to restart Nginx162 # Re-run with same set of sites, no change so shouldn't need to restart Nginx
156 service_start_or_restart.reset_mock()163 service_start_or_restart.reset_mock()
164 close_port.reset_mock()
165 opened_ports.return_value = ['80/tcp']
157 content_cache.configure_nginx(self.tmpdir)166 content_cache.configure_nginx(self.tmpdir)
158 self.assertFalse(service_start_or_restart.assert_not_called())167 self.assertFalse(service_start_or_restart.assert_not_called())
168 close_port.assert_not_called()
159169
160 for site in [170 for site in [
161 'site1.local',171 'site1.local',
@@ -165,6 +175,7 @@ class TestCharm(unittest.TestCase):
165 'site5',175 'site5',
166 'site6.local',176 'site6.local',
167 'site7.local',177 'site7.local',
178 'site8.local',
168 ]:179 ]:
169 with open(180 with open(
170 'tests/unit/files/nginx_config_rendered_test_output-{}.txt'.format(site), 'r', encoding='utf-8'181 'tests/unit/files/nginx_config_rendered_test_output-{}.txt'.format(site), 'r', encoding='utf-8'
@@ -183,8 +194,10 @@ class TestCharm(unittest.TestCase):
183 got = f.read()194 got = f.read()
184 self.assertEqual(got, want)195 self.assertEqual(got, want)
185196
197 @mock.patch('charmhelpers.core.hookenv.close_port')
198 @mock.patch('charmhelpers.core.hookenv.opened_ports')
186 @mock.patch('reactive.content_cache.service_start_or_restart')199 @mock.patch('reactive.content_cache.service_start_or_restart')
187 def test_configure_nginx_sites_secrets(self, service_start_or_restart):200 def test_configure_nginx_sites_secrets(self, service_start_or_restart, opened_ports, close_port):
188 with open('tests/unit/files/config_test_secrets.txt', 'r', encoding='utf-8') as f:201 with open('tests/unit/files/config_test_secrets.txt', 'r', encoding='utf-8') as f:
189 secrets = f.read()202 secrets = f.read()
190 config = '''203 config = '''
@@ -227,9 +240,11 @@ site1.local:
227 got = f.read()240 got = f.read()
228 self.assertEqual(got, want)241 self.assertEqual(got, want)
229242
243 @mock.patch('charmhelpers.core.hookenv.close_port')
244 @mock.patch('charmhelpers.core.hookenv.opened_ports')
230 @mock.patch('shutil.disk_usage')245 @mock.patch('shutil.disk_usage')
231 @mock.patch('reactive.content_cache.service_start_or_restart')246 @mock.patch('reactive.content_cache.service_start_or_restart')
232 def test_configure_nginx_cache_config(self, service_start_or_restart, disk_usage):247 def test_configure_nginx_cache_config(self, service_start_or_restart, disk_usage, opened_ports, close_port):
233 config = '''248 config = '''
234site1.local:249site1.local:
235 locations:250 locations:
@@ -671,3 +686,72 @@ site1.local:
671 content='test content\n', group='somegroup', owner='somedude', path=dest, perms=444686 content='test content\n', group='somegroup', owner='somedude', path=dest, perms=444
672 )687 )
673 )688 )
689
690 @mock.patch('charmhelpers.core.hookenv.open_port')
691 @mock.patch('charmhelpers.core.hookenv.opened_ports')
692 @mock.patch('reactive.content_cache.service_start_or_restart')
693 def test_configure_nginx_metrics_sites(self, service_start_or_restart, opened_ports, open_port):
694 """Test configuration of Nginx sites with enable_prometheus_metrics activated."""
695 with open('tests/unit/files/config_test_basic_config.txt', 'r', encoding='utf-8') as f:
696 ngx_config = f.read()
697 self.mock_config.return_value = {
698 'cache_max_size': '1g',
699 'enable_prometheus_metrics': True,
700 'cache_path': '/var/lib/nginx/proxy',
701 'sites': ngx_config,
702 }
703
704 with mock.patch.multiple(
705 'lib.nginx.NginxConf',
706 sites_path=os.path.join(self.tmpdir, 'sites-available'),
707 base_path=self.tmpdir,
708 ) as nginxconf_mock: # noqa: F841
709 # conf.d, sites-available, and sites-enabled won't exist in our
710 # temporary directory.
711 os.mkdir(os.path.join(self.tmpdir, 'conf.d'))
712 os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
713 os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
714 opened_ports.return_value = ['80/tcp', '443/tcp']
715 content_cache.configure_nginx(self.tmpdir)
716 self.assertFalse(service_start_or_restart.assert_called_once_with('nginx'))
717 open_port.assert_called_once_with(nginx.METRICS_PORT, 'TCP')
718
719 # Re-run with same set of sites, no change so shouldn't need to restart Nginx
720 service_start_or_restart.reset_mock()
721 open_port.reset_mock()
722 opened_ports.return_value = ['80/tcp', '443/tcp', '{0}/tcp'.format(nginx.METRICS_PORT)]
723 content_cache.configure_nginx(self.tmpdir)
724 self.assertFalse(service_start_or_restart.assert_not_called())
725 open_port.assert_not_called()
726
727 # Test the site with cache HIT logging
728 site = 'basic_site'
729 test_file = 'tests/unit/files/nginx_config_rendered_test_output-{0}.txt'.format(site)
730 with open(test_file, 'r', encoding='utf-8') as f:
731 want = f.read()
732
733 test_file = os.path.join(self.tmpdir, 'sites-available/{0}.conf'.format(site))
734 with open(test_file, 'r', encoding='utf-8') as f:
735 got = f.read()
736 self.assertEqual(got, want)
737
738 # Test the site exposing the metrics
739 site = 'nginx_metrics'
740 script_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..')
741 jinja_env = jinja2.Environment(loader=jinja2.FileSystemLoader(script_dir))
742 template = jinja_env.get_template('templates/nginx_metrics_cfg.tmpl')
743 content = template.render({
744 'nginx_conf_path': os.path.join(self.tmpdir, 'conf.d'),
745 'port': nginx.METRICS_PORT})
746 want = content
747 test_file = os.path.join(self.tmpdir, 'sites-available/{0}.conf'.format(nginx.METRICS_SITE))
748 with open(test_file, 'r', encoding='utf-8') as f:
749 got = f.read()
750 self.assertEqual(got, want)
751
752 # Prometheus.lua library
753 with open('files/prometheus.lua', 'r') as f:
754 want = f.read()
755 with open(os.path.join(self.tmpdir, 'conf.d', 'prometheus.lua'), 'r') as f:
756 got = f.read()
757 self.assertEqual(got, want)
diff --git a/tests/unit/test_nginx.py b/tests/unit/test_nginx.py
index e9ec2f1..7097f7e 100644
--- a/tests/unit/test_nginx.py
+++ b/tests/unit/test_nginx.py
@@ -5,6 +5,8 @@ import tempfile
5import unittest5import unittest
6import yaml6import yaml
77
8import jinja2
9
8sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))))10sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))))
9from lib import nginx # NOQA: E40211from lib import nginx # NOQA: E402
1012
@@ -21,10 +23,13 @@ class TestLibNginx(unittest.TestCase):
2123
22 def test_nginx_config_conf_path(self):24 def test_nginx_config_conf_path(self):
23 conf_path = '/etc/nginx/conf.d'25 conf_path = '/etc/nginx/conf.d'
26 alternate_conf_path = '/var/lib/snap/nginx/etc'
24 ngx_conf = nginx.NginxConf()27 ngx_conf = nginx.NginxConf()
25 self.assertEqual(ngx_conf.conf_path, conf_path)28 self.assertEqual(ngx_conf.conf_path, conf_path)
26 ngx_conf = nginx.NginxConf(None)29 ngx_conf = nginx.NginxConf(None)
27 self.assertEqual(ngx_conf.conf_path, conf_path)30 self.assertEqual(ngx_conf.conf_path, conf_path)
31 ngx_conf = nginx.NginxConf(alternate_conf_path)
32 self.assertEqual(ngx_conf.base_path, alternate_conf_path)
2833
29 def test_nginx_config_sites_path(self):34 def test_nginx_config_sites_path(self):
30 sites_path = '/etc/nginx/sites-available'35 sites_path = '/etc/nginx/sites-available'
@@ -34,7 +39,7 @@ class TestLibNginx(unittest.TestCase):
34 self.assertEqual(ngx_conf.sites_path, sites_path)39 self.assertEqual(ngx_conf.sites_path, sites_path)
3540
36 def test_nginx_config_render(self):41 def test_nginx_config_render(self):
37 '''Test parsing a YAML-formatted list of sites'''42 """Test parsing a YAML-formatted list of sites."""
38 ngx_conf = nginx.NginxConf()43 ngx_conf = nginx.NginxConf()
3944
40 with open('tests/unit/files/config_test_config.txt', 'r', encoding='utf-8') as f:45 with open('tests/unit/files/config_test_config.txt', 'r', encoding='utf-8') as f:
@@ -42,6 +47,7 @@ class TestLibNginx(unittest.TestCase):
4247
43 conf = {}48 conf = {}
44 conf['cache_max_size'] = '1g'49 conf['cache_max_size'] = '1g'
50 conf['enable_prometheus_metrics'] = False
45 conf['cache_path'] = '/var/lib/nginx/proxy'51 conf['cache_path'] = '/var/lib/nginx/proxy'
46 conf['listen_address'] = '127.0.0.1'52 conf['listen_address'] = '127.0.0.1'
47 # From the given YAML-formatted list of sites, check that each individual53 # From the given YAML-formatted list of sites, check that each individual
@@ -66,7 +72,7 @@ class TestLibNginx(unittest.TestCase):
66 self.assertEqual(ngx_conf.render(conf), output)72 self.assertEqual(ngx_conf.render(conf), output)
6773
68 def test_nginx_config_write_sites(self):74 def test_nginx_config_write_sites(self):
69 '''Test writing out sites to individual Nginx site config files'''75 """Test writing out sites to individual Nginx site config files."""
70 ngx_conf = nginx.NginxConf(self.tmpdir)76 ngx_conf = nginx.NginxConf(self.tmpdir)
71 os.mkdir(os.path.join(self.tmpdir, 'sites-available'))77 os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
72 os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))78 os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
@@ -113,3 +119,78 @@ class TestLibNginx(unittest.TestCase):
113119
114 # Re-run, no change this time.120 # Re-run, no change this time.
115 self.assertFalse(ngx_conf.sync_sites(['site1.local', 'site2.local']))121 self.assertFalse(ngx_conf.sync_sites(['site1.local', 'site2.local']))
122
123 def test_nginx_config_render_with_metrics(self):
124 """Test rendering with metrics exposed."""
125 ngx_conf = nginx.NginxConf()
126
127 with open('tests/unit/files/config_test_basic_config.txt', 'r', encoding='utf-8') as f:
128 sites = yaml.safe_load(f.read())
129
130 conf = {
131 'cache_max_size': '1g',
132 'cache_path': '/var/lib/nginx/proxy',
133 'listen_address': '127.0.0.1',
134 'enable_prometheus_metrics': True,
135 }
136 for site, site_conf in sites.items():
137 conf['site'] = site_conf.get('site-name') or site
138 conf['listen_port'] = BASE_LISTEN_PORT
139 conf['locations'] = site_conf.get('locations', {})
140
141 for location, loc_conf in conf['locations'].items():
142 if loc_conf.get('backends'):
143 loc_conf['backend_port'] = BASE_BACKEND_PORT
144
145 output_file = 'tests/unit/files/nginx_config_rendered_test_output-{}.txt'.format(site)
146 with open(output_file, 'r', encoding='utf-8') as f:
147 output = f.read()
148
149 self.assertEqual(ngx_conf.render(conf), output)
150
151 def test_nginx_config_toggle_metrics_site(self):
152 """Test the metrics site.
153
154 Check that the activation fo the cache metrics activate the dedicated site for exposing prometheus metrics.
155 """
156 ngx_conf = nginx.NginxConf(self.tmpdir)
157 os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
158 os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
159
160 metrics_site_conf = '{0}.conf'.format(nginx.METRICS_SITE)
161
162 script_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..')
163 env = jinja2.Environment(loader=jinja2.FileSystemLoader(script_dir))
164 template = env.get_template('templates/nginx_metrics_cfg.tmpl')
165 content = template.render({
166 'nginx_conf_path': os.path.join(self.tmpdir, 'conf.d'),
167 'port': nginx.METRICS_PORT})
168 nginx_metrics_conf = content
169
170 metrics_site_available = os.path.join(self.tmpdir, 'sites-available', metrics_site_conf)
171 metrics_site_enabled = os.path.join(self.tmpdir, 'sites-enabled', metrics_site_conf)
172 self.assertTrue(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=True))
173 # Write again with same contents, this time it should return 'False'
174 # as there's no change, thus no need to restart/reload Nginx.
175 self.assertFalse(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=True))
176 self.assertTrue(os.path.exists(metrics_site_available))
177
178 # Compare what's been written out matches what's in tests/unit/files.
179 with open(metrics_site_available, 'r', encoding='utf-8') as f:
180 output = f.read()
181 self.assertEqual(nginx_metrics_conf, output)
182
183 # Check link existence
184 self.assertTrue(os.path.islink(metrics_site_enabled))
185
186 # Mess up with the target of the link and check that a toggle fixes it
187 os.remove(metrics_site_enabled)
188 os.symlink('/dev/null', metrics_site_enabled)
189 self.assertTrue(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=True))
190 self.assertTrue(os.path.realpath(metrics_site_available) == os.path.realpath(metrics_site_enabled))
191
192 # Remove the site
193 self.assertTrue(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=False))
194 self.assertFalse(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=False))
195 self.assertFalse(os.path.exists(metrics_site_available))
196 self.assertFalse(os.path.exists(metrics_site_enabled))

Subscribers

People subscribed via source and target branches