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

[Haw Loeung]

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).

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

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

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

[Paul Collins (pjdc)]

review: Approve (lgtm)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 326

=== modified file 'lib/ubuntu_repository_cache/apache.py'
--- lib/ubuntu_repository_cache/apache.py	2021-02-22 22:51:22 +0000
+++ lib/ubuntu_repository_cache/apache.py	2021-03-07 22:12:50 +0000
@@ -212,7 +212,6 @@
     apache_context['Path_Base'] = config['path-base']
     metaproxy_conf = '# No failover configured'
     failover_host = util.get_failover_host()
-    healthcheck_disabled = os.path.join(unitdata.kv().get('apache-root'), 'health-check', 'health-check-disabled.flag')
     if util.in_failover() and failover_host:
         uri = 'http://%s:80' % failover_host
         metaproxy_conf = (
@@ -220,10 +219,6 @@
             '\tProxyPass        / http://{disp}/\n'
             '\tProxyPassReverse / http://{disp}/'
         ).format(uri=uri, disp=config['display-host'])
-        if not os.path.exists(healthcheck_disabled):
-            os.mknod(healthcheck_disabled)
-    elif os.path.exists(healthcheck_disabled):
-        os.unlink(healthcheck_disabled)
     apache_context['MetadataProxy'] = metaproxy_conf
     apache_context['Enable_Healthcheck'] = config['enable_healthcheck']
 
 
=== modified file 'tests/unit/test_apache.py'
--- tests/unit/test_apache.py	2021-02-22 03:43:03 +0000
+++ tests/unit/test_apache.py	2021-03-07 22:12:50 +0000
@@ -112,9 +112,8 @@
     @mock.patch('lib.ubuntu_repository_cache.apache.start')
     @mock.patch('lib.ubuntu_repository_cache.util.get_failover_host')
     @mock.patch('lib.ubuntu_repository_cache.util.in_failover')
-    @mock.patch('os.mknod')
     @mock.patch('subprocess.call')
-    def test_create_metadata_site_in_failover(self, call, mknod, in_failover, get_failover_host, start, render):
+    def test_create_metadata_site_in_failover(self, call, in_failover, get_failover_host, start, render):
         config = hookenv.Config(
             {
                 'display-host': 'archive.ubuntu.com',
@@ -126,13 +125,9 @@
         get_failover_host.return_value = '10.1.1.1'
         in_failover.return_value = True
 
-        health_check_path = os.path.join(self.tmpdir, 'apache', 'health-check')
-        disabled_flag = os.path.join(health_check_path, 'health-check-disabled.flag')
         os.mkdir(os.path.join(self.tmpdir, 'apache'))
-        os.mkdir(health_check_path)
 
         apache.create_metadata_site(config)
-        mknod.assert_called_with(disabled_flag)
         render.assert_called_with(
             'apache2/archive_ubuntu_com.conf',
             '/etc/apache2/sites-available/archive_ubuntu_com.conf',
@@ -149,18 +144,11 @@
         )
         call.assert_called_with(['a2ensite', 'archive_ubuntu_com'])
 
-        # If the disabled flag exists, no need to create it.
-        mknod.reset_mock()
-        open(disabled_flag, 'a').close()
-        apache.create_metadata_site(config)
-        mknod.assert_not_called()
-
     @mock.patch('charmhelpers.core.templating.render')
     @mock.patch('lib.ubuntu_repository_cache.apache.start')
     @mock.patch('lib.ubuntu_repository_cache.util.get_failover_host')
-    @mock.patch('os.unlink')
     @mock.patch('subprocess.call')
-    def test_create_metadata_site_no_failover(self, call, unlink, get_failover_host, start, render):
+    def test_create_metadata_site_no_failover(self, call, get_failover_host, start, render):
         config = hookenv.Config(
             {
                 'display-host': 'archive.ubuntu.com',
@@ -170,13 +158,22 @@
             }
         )
 
-        health_check_path = os.path.join(self.tmpdir, 'apache', 'health-check')
-        disabled_flag = os.path.join(health_check_path, 'health-check-disabled.flag')
         os.mkdir(os.path.join(self.tmpdir, 'apache'))
-        os.mkdir(health_check_path)
-        os.mknod(disabled_flag)
+
         apache.create_metadata_site(config)
-        unlink.assert_called_with(disabled_flag)
+        render.assert_called_with(
+            'apache2/archive_ubuntu_com.conf',
+            '/etc/apache2/sites-available/archive_ubuntu_com.conf',
+            {
+                'DocumentRoot': os.path.join(self.tmpdir, 'apache'),
+                'Sync_Host': 'us.archive.ubuntu.com',
+                'Display_Host': 'archive.ubuntu.com',
+                'Path_Base': 'ubuntu',
+                'MetadataProxy': '# No failover configured',
+                'Enable_Healthcheck': True,
+            },
+        )
+        call.assert_called_with(['a2ensite', 'archive_ubuntu_com'])
 
     @mock.patch('charmhelpers.core.host.service_start')
     @mock.patch('charmhelpers.core.host.service_reload')