Merge lp:~hloeung/ubuntu-repository-cache/dont-disable-unit-on-failover-or-pause into lp:ubuntu-repository-cache

Proposed by Haw Loeung
Status: Merged
Approved by: Haw Loeung
Approved revision: 326
Merged at revision: 326
Proposed branch: lp:~hloeung/ubuntu-repository-cache/dont-disable-unit-on-failover-or-pause
Merge into: lp:ubuntu-repository-cache
Diff against target: 99 lines (+16/-24)
2 files modified
lib/ubuntu_repository_cache/apache.py (+0/-5)
tests/unit/test_apache.py (+16/-19)
To merge this branch: bzr merge lp:~hloeung/ubuntu-repository-cache/dont-disable-unit-on-failover-or-pause
Reviewer Review Type Date Requested Status
Paul Collins lgtm Approve
Canonical IS Reviewers Pending
Review via email: mp+399254@code.launchpad.net

Commit message

Don't set health-check-disabled flag in apache.create_metadata_site()

On non-leader units, metadata sync calls service.pause() which then
calls this. This causes a brief period where the unit is disabled and
not serving requests. It's useful for initial unit provisioning but
not so on a fully functional unit where metadata points to an existing
copy on disk.

NOTE: The setting of health-check-disabled flag was introduced in r309 (MP:398100).

Description of the change

Seems only happening in gce-us-central1 where it has a large no. of u-r-c units compared to all the others (that or metadata sync between them takes longer).

See https://code.launchpad.net/~hloeung/ubuntu-repository-cache/health-check-handle-initial-sync/+merge/398100

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

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

Revision history for this message
Paul Collins (pjdc) :
review: Approve (lgtm)
Revision history for this message
Canonical IS Mergebot (canonical-is-mergebot) wrote :

Change successfully merged at revision 326

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/ubuntu_repository_cache/apache.py'
2--- lib/ubuntu_repository_cache/apache.py 2021-02-22 22:51:22 +0000
3+++ lib/ubuntu_repository_cache/apache.py 2021-03-07 22:12:50 +0000
4@@ -212,7 +212,6 @@
5 apache_context['Path_Base'] = config['path-base']
6 metaproxy_conf = '# No failover configured'
7 failover_host = util.get_failover_host()
8- healthcheck_disabled = os.path.join(unitdata.kv().get('apache-root'), 'health-check', 'health-check-disabled.flag')
9 if util.in_failover() and failover_host:
10 uri = 'http://%s:80' % failover_host
11 metaproxy_conf = (
12@@ -220,10 +219,6 @@
13 '\tProxyPass / http://{disp}/\n'
14 '\tProxyPassReverse / http://{disp}/'
15 ).format(uri=uri, disp=config['display-host'])
16- if not os.path.exists(healthcheck_disabled):
17- os.mknod(healthcheck_disabled)
18- elif os.path.exists(healthcheck_disabled):
19- os.unlink(healthcheck_disabled)
20 apache_context['MetadataProxy'] = metaproxy_conf
21 apache_context['Enable_Healthcheck'] = config['enable_healthcheck']
22
23
24=== modified file 'tests/unit/test_apache.py'
25--- tests/unit/test_apache.py 2021-02-22 03:43:03 +0000
26+++ tests/unit/test_apache.py 2021-03-07 22:12:50 +0000
27@@ -112,9 +112,8 @@
28 @mock.patch('lib.ubuntu_repository_cache.apache.start')
29 @mock.patch('lib.ubuntu_repository_cache.util.get_failover_host')
30 @mock.patch('lib.ubuntu_repository_cache.util.in_failover')
31- @mock.patch('os.mknod')
32 @mock.patch('subprocess.call')
33- def test_create_metadata_site_in_failover(self, call, mknod, in_failover, get_failover_host, start, render):
34+ def test_create_metadata_site_in_failover(self, call, in_failover, get_failover_host, start, render):
35 config = hookenv.Config(
36 {
37 'display-host': 'archive.ubuntu.com',
38@@ -126,13 +125,9 @@
39 get_failover_host.return_value = '10.1.1.1'
40 in_failover.return_value = True
41
42- health_check_path = os.path.join(self.tmpdir, 'apache', 'health-check')
43- disabled_flag = os.path.join(health_check_path, 'health-check-disabled.flag')
44 os.mkdir(os.path.join(self.tmpdir, 'apache'))
45- os.mkdir(health_check_path)
46
47 apache.create_metadata_site(config)
48- mknod.assert_called_with(disabled_flag)
49 render.assert_called_with(
50 'apache2/archive_ubuntu_com.conf',
51 '/etc/apache2/sites-available/archive_ubuntu_com.conf',
52@@ -149,18 +144,11 @@
53 )
54 call.assert_called_with(['a2ensite', 'archive_ubuntu_com'])
55
56- # If the disabled flag exists, no need to create it.
57- mknod.reset_mock()
58- open(disabled_flag, 'a').close()
59- apache.create_metadata_site(config)
60- mknod.assert_not_called()
61-
62 @mock.patch('charmhelpers.core.templating.render')
63 @mock.patch('lib.ubuntu_repository_cache.apache.start')
64 @mock.patch('lib.ubuntu_repository_cache.util.get_failover_host')
65- @mock.patch('os.unlink')
66 @mock.patch('subprocess.call')
67- def test_create_metadata_site_no_failover(self, call, unlink, get_failover_host, start, render):
68+ def test_create_metadata_site_no_failover(self, call, get_failover_host, start, render):
69 config = hookenv.Config(
70 {
71 'display-host': 'archive.ubuntu.com',
72@@ -170,13 +158,22 @@
73 }
74 )
75
76- health_check_path = os.path.join(self.tmpdir, 'apache', 'health-check')
77- disabled_flag = os.path.join(health_check_path, 'health-check-disabled.flag')
78 os.mkdir(os.path.join(self.tmpdir, 'apache'))
79- os.mkdir(health_check_path)
80- os.mknod(disabled_flag)
81+
82 apache.create_metadata_site(config)
83- unlink.assert_called_with(disabled_flag)
84+ render.assert_called_with(
85+ 'apache2/archive_ubuntu_com.conf',
86+ '/etc/apache2/sites-available/archive_ubuntu_com.conf',
87+ {
88+ 'DocumentRoot': os.path.join(self.tmpdir, 'apache'),
89+ 'Sync_Host': 'us.archive.ubuntu.com',
90+ 'Display_Host': 'archive.ubuntu.com',
91+ 'Path_Base': 'ubuntu',
92+ 'MetadataProxy': '# No failover configured',
93+ 'Enable_Healthcheck': True,
94+ },
95+ )
96+ call.assert_called_with(['a2ensite', 'archive_ubuntu_com'])
97
98 @mock.patch('charmhelpers.core.host.service_start')
99 @mock.patch('charmhelpers.core.host.service_reload')

Subscribers

People subscribed via source and target branches