Content Cache Charm

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

  1. Git
  2. lp:~hloeung/content-cache-charm
  3. master
  4. Merge into master
Proposed by Haw Loeung
Status: Merged
Approved by: Haw Loeung
Approved revision: 507191d96dbfdfa5e01b8a2e6135975cce830f4f
Merged at revision: 4a1022d99ea0ce2d5c875f385a40a8018b329c02
Proposed branch: ~hloeung/content-cache-charm:master
Merge into: content-cache-charm:master
Diff against target: 186 lines (+42/-31)
2 files modified
reactive/content_cache.py (+23/-12)
tests/unit/test_content_cache.py (+19/-19)
Reviewer Review Type Date Requested Status
Joel Sing (community) +1 Approve
Canonical IS Reviewers Pending
Review via email: mp+380260@code.launchpad.net

Commit message

Reload Nginx and HAProxy together and immediately after - LP:1865945

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
Haw Loeung (hloeung) wrote :

Where commit 8c67e60 is https://code.launchpad.net/~hloeung/content-cache-charm/+git/content-cache-charm/+merge/378217

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

LGTM

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

Change successfully merged at revision 4a1022d99ea0ce2d5c875f385a40a8018b329c02

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/reactive/content_cache.py b/reactive/content_cache.py
2index be27cf0..428b3d1 100644
3--- a/reactive/content_cache.py
4+++ b/reactive/content_cache.py
5@@ -66,16 +66,27 @@ def set_active():
6 reactive.set_flag('content_cache.active')
7
8
9-def service_start_or_reload(name):
10- if host.service_running(name):
11- random.seed()
12- rnd = (random.random() * 100) % 20
13- status.maintenance('Reloading {} in {}s...'.format(name, int(rnd)))
14- time.sleep(rnd)
15- host.service_reload(name)
16- else:
17- status.maintenance('Starting {}...'.format(name))
18- host.service_start(name)
19+@reactive.when_any('content_cache.haproxy.restart-required', 'content_cache.nginx.restart-required')
20+def service_start_or_reload():
21+ services = ['haproxy', 'nginx']
22+
23+ # Immediately start up services if they're not running.
24+ for name in services:
25+ if not host.service_running(name):
26+ status.maintenance('Starting {}...'.format(name))
27+ host.service_start(name)
28+ reactive.clear_flag('content_cache.{}.restart-required'.format(name))
29+
30+ random.seed()
31+ rnd = (random.random() * 100) % 20
32+ status.maintenance('Reloading services in {}s...'.format(int(rnd)))
33+ time.sleep(rnd)
34+
35+ for name in services:
36+ if reactive.is_flag_set('content_cache.{}.restart-required'.format(name)):
37+ status.maintenance('Reloading {}...'.format(name))
38+ host.service_reload(name)
39+ reactive.clear_flag('content_cache.{}.restart-required'.format(name))
40
41
42 def configure_nginx_metrics(ngx_conf, enable_prometheus_metrics):
43@@ -163,7 +174,7 @@ def configure_nginx(conf_path=None):
44 changed = True
45
46 if changed:
47- service_start_or_reload('nginx')
48+ reactive.set_flag('content_cache.nginx.restart-required')
49
50 update_logrotate('nginx', retention=config.get('log_retention'))
51 reactive.set_flag('content_cache.nginx.configured')
52@@ -291,7 +302,7 @@ def configure_haproxy(): # NOQA: C901 LP#1825084
53 )
54
55 if haproxy.write(rendered_config):
56- service_start_or_reload('haproxy')
57+ reactive.set_flag('content_cache.haproxy.restart-required')
58
59 update_logrotate('haproxy', retention=config.get('log_retention'))
60 reactive.set_flag('content_cache.haproxy.configured')
61diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
62index 71cc27a..dfac47a 100644
63--- a/tests/unit/test_content_cache.py
64+++ b/tests/unit/test_content_cache.py
65@@ -125,10 +125,10 @@ class TestCharm(unittest.TestCase):
66 def test_service_start_or_reload_running(self, service_start, service_reload, service_running):
67 '''Test service restarted when already running'''
68 service_running.return_value = True
69- content_cache.service_start_or_reload('someservice')
70+ content_cache.service_start_or_reload()
71 status.maintenance.assert_called()
72 service_start.assert_not_called()
73- service_reload.assert_called_once_with('someservice')
74+ service_reload.assert_called_with('nginx')
75
76 @mock.patch('charmhelpers.core.host.service_running')
77 @mock.patch('charmhelpers.core.host.service_reload')
78@@ -136,9 +136,9 @@ class TestCharm(unittest.TestCase):
79 def test_service_start_or_reload_stopped(self, service_start, service_reload, service_running):
80 '''Test service started up when not running/stopped'''
81 service_running.return_value = False
82- content_cache.service_start_or_reload('someservice')
83+ content_cache.service_start_or_reload()
84 status.maintenance.assert_called()
85- service_start.assert_called_once_with('someservice')
86+ service_start.assert_called_with('nginx')
87 service_reload.assert_not_called()
88
89 @mock.patch('charms.reactive.clear_flag')
90@@ -156,11 +156,11 @@ class TestCharm(unittest.TestCase):
91 status.blocked.assert_called()
92 clear_flag.assert_called_once_with('content_cache.active')
93
94+ @mock.patch('charms.reactive.set_flag')
95 @mock.patch('charmhelpers.core.hookenv.close_port')
96 @mock.patch('charmhelpers.core.hookenv.opened_ports')
97- @mock.patch('reactive.content_cache.service_start_or_reload')
98 @mock.patch('reactive.content_cache.update_logrotate')
99- def test_configure_nginx_sites(self, logrotation, service_start_or_reload, opened_ports, close_port):
100+ def test_configure_nginx_sites(self, logrotation, opened_ports, close_port, set_flag):
101 '''Test configuration of Nginx sites'''
102 with open('tests/unit/files/config_test_config.txt', 'r', encoding='utf-8') as f:
103 ngx_config = f.read()
104@@ -185,15 +185,15 @@ class TestCharm(unittest.TestCase):
105
106 opened_ports.return_value = ['80/tcp', '{0}/tcp'.format(nginx.METRICS_PORT)]
107 content_cache.configure_nginx(self.tmpdir)
108- service_start_or_reload.assert_called_once_with('nginx')
109+ set_flag.assert_has_calls([mock.call('content_cache.nginx.restart-required')])
110 close_port.assert_called_once_with(nginx.METRICS_PORT, 'TCP')
111
112 # Re-run with same set of sites, no change so shouldn't need to restart Nginx
113- service_start_or_reload.reset_mock()
114+ set_flag.reset_mock()
115 close_port.reset_mock()
116 opened_ports.return_value = ['80/tcp']
117 content_cache.configure_nginx(self.tmpdir)
118- service_start_or_reload.assert_not_called()
119+ self.assertFalse(mock.call('content_cache.nginx.restart-required') in set_flag.call_args_list)
120 close_port.assert_not_called()
121
122 sites = [
123@@ -362,9 +362,9 @@ site1.local:
124 clear_flag.assert_called_once_with('content_cache.active')
125
126 @freezegun.freeze_time("2019-03-22", tz_offset=0)
127- @mock.patch('reactive.content_cache.service_start_or_reload')
128+ @mock.patch('charms.reactive.set_flag')
129 @mock.patch('reactive.content_cache.update_logrotate')
130- def test_configure_haproxy_sites(self, logrotation, service_start_or_reload):
131+ def test_configure_haproxy_sites(self, logrotation, set_flag):
132 with open('tests/unit/files/config_test_config.txt', 'r', encoding='utf-8') as f:
133 ngx_config = f.read()
134 self.mock_config.return_value = {'sites': ngx_config}
135@@ -375,15 +375,15 @@ site1.local:
136 'charmhelpers.core.hookenv.opened_ports', return_value=["443/tcp"]
137 ), mock.patch('charmhelpers.core.hookenv.open_port'), mock.patch('charmhelpers.core.hookenv.close_port'):
138 content_cache.configure_haproxy()
139- service_start_or_reload.assert_called_with('haproxy')
140+ set_flag.assert_has_calls([mock.call('content_cache.haproxy.restart-required')])
141
142 # Again, this time should be no change so no need to restart HAProxy
143- service_start_or_reload.reset_mock()
144+ set_flag.reset_mock()
145 with mock.patch('charmhelpers.core.hookenv.opened_ports', return_value=["443/tcp"]), mock.patch(
146 'charmhelpers.core.hookenv.open_port'
147 ), mock.patch('charmhelpers.core.hookenv.close_port'):
148 content_cache.configure_haproxy()
149- service_start_or_reload.assert_not_called()
150+ self.assertFalse(mock.call('content_cache.haproxy.restart-required') in set_flag.call_args_list)
151
152 with open('tests/unit/files/content_cache_rendered_haproxy_test_output.txt', 'r', encoding='utf-8') as f:
153 want = f.read()
154@@ -959,11 +959,11 @@ site1.local:
155 )
156 )
157
158+ @mock.patch('charms.reactive.set_flag')
159 @mock.patch('charmhelpers.core.hookenv.open_port')
160 @mock.patch('charmhelpers.core.hookenv.opened_ports')
161- @mock.patch('reactive.content_cache.service_start_or_reload')
162 @mock.patch('reactive.content_cache.update_logrotate')
163- def test_configure_nginx_metrics_sites(self, logrotation, service_start_or_reload, opened_ports, open_port):
164+ def test_configure_nginx_metrics_sites(self, logrotation, opened_ports, open_port, set_flag):
165 """Test configuration of Nginx sites with enable_prometheus_metrics activated."""
166 with open('tests/unit/files/config_test_basic_config.txt', 'r', encoding='utf-8') as f:
167 ngx_config = f.read()
168@@ -989,15 +989,15 @@ site1.local:
169
170 opened_ports.return_value = ['80/tcp', '443/tcp']
171 content_cache.configure_nginx(self.tmpdir)
172- service_start_or_reload.assert_called_once_with('nginx')
173+ set_flag.assert_has_calls([mock.call('content_cache.nginx.restart-required')])
174 open_port.assert_called_once_with(nginx.METRICS_PORT, 'TCP')
175
176 # Re-run with same set of sites, no change so shouldn't need to restart Nginx
177- service_start_or_reload.reset_mock()
178+ set_flag.reset_mock()
179 open_port.reset_mock()
180 opened_ports.return_value = ['80/tcp', '443/tcp', '{0}/tcp'.format(nginx.METRICS_PORT)]
181 content_cache.configure_nginx(self.tmpdir)
182- service_start_or_reload.assert_not_called()
183+ self.assertFalse(mock.call('content_cache.nginx.restart-required') in set_flag.call_args_list)
184 open_port.assert_not_called()
185
186 # Test the site with cache HIT logging

Subscribers

People subscribed via source and target branches