Merge lp:~hloeung/ubuntu-repository-cache/metadata-sync-to-peers-less-juju into lp:ubuntu-repository-cache

[Haw Loeung]

Untie metadata sync from relying on Juju

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

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

[lp:~hloeung/ubuntu-repository-cache/metadata-sync-to-peers-less-juju]

384. By Haw Loeung on 2022-08-23

Fixed missing newline before EOF causing cron to fail to load our crontab

385. By Haw Loeung on 2022-08-23

Fixed here too

[James Simpson (jsimpso)]

LGTM, a couple of comments in-line but nothing worth blocking on!

review: Approve

[lp:~hloeung/ubuntu-repository-cache/metadata-sync-to-peers-less-juju]

386. By Haw Loeung on 2022-08-24

Add missing metric, also rename ubuntu_repository_cache_metadata_sync_total_duration to ubuntu_repository_cache_metadata_sync_duration making it consistent

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

Change successfully merged at revision 383

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2022-07-22 06:33:43 +0000
+++ hooks/hooks.py	2022-08-24 00:09:04 +0000
@@ -9,7 +9,6 @@
 from charmhelpers.core import hookenv
 
 from lib.ubuntu_repository_cache import (
-    metadata_sync,
     mirror,
     service,
     util,
@@ -51,9 +50,10 @@
     metadata is ready."""
 
     LOG('Notifying peers of metadata update')
+    urc_config = parse_config()
     # We can't use hookenv.config() here because the persistent config
     # permissions are restricted to root:root 0600 - LP:1828658
-    apache_root = parse_config()['APACHE_ROOT']
+    apache_root = urc_config['APACHE_ROOT']
     apache_data = '/'.join((apache_root, 'data'))
 
     meta_ver = sys.argv[1]
@@ -62,42 +62,38 @@
         return
 
     link_next = '/'.join((apache_data, 'ubuntu_next'))
-
     meta_dir = os.path.realpath(link_next)
 
+    peers = urc_config['PEERS'].split()
+    LOG('Peers: {}'.format(' '.join(peers)))
+
     start_time = time.time()
 
     # Push the metadata to peers
-    if hookenv.is_relation_made('cluster'):
-        dest = '/'.join((apache_data, meta_ver))
-        link_dir = '/'.join((apache_data, 'ubuntu_active'))
-        LOG('Pushing updates to peers', hookenv.DEBUG)
-        source = meta_dir + '/'
-        successful_peers = mirror.rsync_to_peers(source, dest, link_dir=link_dir)
-
-        # Push metaversion_good canary to indicate a complete sync
-        canary = '_'.join((meta_dir, 'good'))
-        LOG('Pushing canary files to peers', hookenv.DEBUG)
-        mirror.rsync_to_peers(canary, apache_data, peers=successful_peers)
-
-        # Update 'meta_version' relation setting
-        LOG('', hookenv.DEBUG)
-        ids = hookenv.relation_ids('cluster') or []
-        LOG('Updating meta_version for ids %s' % ids, hookenv.DEBUG)
-        for r_id in ids:
-            LOG('Setting meta_version=%s' % meta_ver, hookenv.DEBUG)
-            hookenv.relation_set(relation_id=r_id, meta_version=meta_ver)
+    dest = '/'.join((apache_data, meta_ver))
+    link_dir = '/'.join((apache_data, 'ubuntu_active'))
+    LOG('Pushing updates to peers', hookenv.DEBUG)
+    source = meta_dir + '/'
+    successful_peers = mirror.rsync_to_peers(source, dest, link_dir=link_dir, peers=peers)
+
+    if sorted(successful_peers) != sorted(peers):
+        LOG('Failed to sync latest metadata to all peers, aborting')
+        return
+
+    # Push metaversion_good canary to indicate a complete sync
+    canary = '_'.join((meta_dir, 'good'))
+    LOG('Pushing canary files to peers', hookenv.DEBUG)
+    mirror.rsync_to_peers(canary, apache_data, peers=successful_peers)
+
+    # Update ubuntu_active/ symlink
+    LOG('Updating ubuntu_active symlink', hookenv.DEBUG)
+    util.update_active_symlink(os.path.basename(meta_dir), apache_root)
+    mirror.rsync_to_peers(link_dir, apache_data, peers=successful_peers)
 
     duration = time.time() - start_time
     metric_name = 'ubuntu_repository_cache_metadata_sync_peers_duration'
     util.send_to_influx(util.render_influx(metric_name, '', duration))
 
-    util.update_active_symlink(os.path.basename(meta_dir), apache_root)
-    metadata_sync.clean_metadata_dir(apache_root, LOG)
-    # Sync doesn't change apache2 configs so doesn't require
-    # graceful. Just start it up if it isn't running.
-    service.start(apache2_start_only=True)
-
 
 if __name__ == "__main__":
     # execute a hook based on the name the program is called by

=== added file 'lib/ubuntu_repository_cache/metadata_cleanup.py'
--- lib/ubuntu_repository_cache/metadata_cleanup.py	1970-01-01 00:00:00 +0000
+++ lib/ubuntu_repository_cache/metadata_cleanup.py	2022-08-24 00:09:04 +0000
@@ -0,0 +1,60 @@
+#! /usr/bin/python3
+
+import os
+import time
+
+from .metadata_sync import (
+    clean_metadata_dir,
+)
+from .util import (
+    FileSemaphore,
+    SemaphoreExistsError,
+    render_influx,
+    send_to_influx,
+)
+
+
+# Log file and directory for the repository sync cron job
+REPO_SYNC_DIR = '/var/log/ubuntu-repository-cache'
+REPO_SYNC_LOG = os.path.join(REPO_SYNC_DIR, 'metadata-cleanup.log')
+
+
+SEMAPHORE_FILE = '/tmp/metadata-cleanup-running'
+
+
+def main(environment, log):
+    apache_root = environment['APACHE_ROOT']
+    clean_metadata_dir(apache_root, log)
+
+
+if __name__ == "__main__":
+
+    # Use the logging infrastructure to print messages with timestamps to
+    # stdout. This is a cheap way to get good timestamps. Ideally we should
+    # move to leverage the logging infrastructure better. ~tribaal
+    import logging
+
+    file_handler = logging.FileHandler(REPO_SYNC_LOG)
+    file_handler.setLevel(logging.INFO)
+
+    basic_formatter = logging.Formatter('%(asctime)s - %(message)s')
+    file_handler.setFormatter(basic_formatter)
+
+    cache_logger = logging.getLogger('ubuntu-repository-cache')
+    cache_logger.setLevel(logging.INFO)
+    cache_logger.addHandler(file_handler)
+
+    environment = os.environ
+
+    start_time = time.time()
+    try:
+        with FileSemaphore(SEMAPHORE_FILE):
+            cache_logger.info("Metadata cleanup started from metadata_cleanup.py:__main__")
+            main(environment, cache_logger.info)
+            cache_logger.info("Metadata cleanup from __main__ ended.")
+
+        duration = time.time() - start_time
+        metric_name = 'ubuntu_repository_cache_metadata_cleanup_duration'
+        send_to_influx(render_influx(metric_name, '', duration))
+    except SemaphoreExistsError:
+        cache_logger.info("Can't acquire semaphore: {} already exists.".format(SEMAPHORE_FILE))

=== modified file 'lib/ubuntu_repository_cache/metadata_sync.py'
--- lib/ubuntu_repository_cache/metadata_sync.py	2022-08-22 04:17:09 +0000
+++ lib/ubuntu_repository_cache/metadata_sync.py	2022-08-24 00:09:04 +0000
@@ -621,7 +621,7 @@
             cache_logger.info("Metadata sync from __main__ ended.")
 
         duration = time.time() - start_time
-        metric_name = 'ubuntu_repository_cache_metadata_sync_total_duration'
+        metric_name = 'ubuntu_repository_cache_metadata_sync_duration'
         send_to_influx(render_influx(metric_name, 'upstream_host={}'.format(environment["SYNC_HOST"]), duration))
     except SemaphoreExistsError:
         cache_logger.info("Can't acquire semaphore: {} already exists.".format(SEMAPHORE_FILE))

=== modified file 'lib/ubuntu_repository_cache/mirror.py'
--- lib/ubuntu_repository_cache/mirror.py	2022-08-22 04:00:31 +0000
+++ lib/ubuntu_repository_cache/mirror.py	2022-08-24 00:09:04 +0000
@@ -11,7 +11,6 @@
     CalledProcessError,
 )
 
-from charmhelpers.contrib import unison
 from charmhelpers.contrib.hahelpers import cluster
 
 from charmhelpers.core import (
@@ -76,24 +75,19 @@
     """
     Use rsync to push a directory to all configured peers.
 
-    Peers are configured in the 'cluster' relationship using
-    charmhelpers.contrib.unison.ssh_authorized_peers(); this list can be
-    overridden with the peers parameter.
-
     A list of the peers whose transfers were successful will be
     returned.
     """
 
-    if peers is None:
-        peers = unison.collect_authed_hosts(peer_interface='cluster')
-
     successful_peers = []
-    for peer in peers:
-        rsync_dest = '{}@{}:{}'.format(user, peer, dest_dir)
-        LOG('Syncing {} to {}.'.format(source_dir, rsync_dest))
+    for sync_peer in peers:
+        (peer_unit, peer_addr) = sync_peer.split(':')
+
+        rsync_dest = '{}@{}:{}'.format(user, peer_addr, dest_dir)
+        LOG('Syncing {} to {} ({}) {}.'.format(source_dir, peer_unit, peer_addr, rsync_dest))
 
         metric_name = 'ubuntu_repository_cache_metadata_sync_peers_failures'
-        label = 'peer={}'.format(peer)
+        label = 'peer={}'.format(peer_unit)
         failures = 0
 
         errmsgs = []
@@ -109,7 +103,7 @@
                 # Save the exact exception and reuse when we need to
                 # reraise it after exhausting all available attempts.
                 exp = e
-                msg = 'Sync to {} failed'.format(peer)
+                msg = 'Sync to {} ({}) failed'.format(peer_unit, peer_addr)
                 LOG(msg)
                 LOG(e.output.decode())
                 errmsgs.append("{}: {}".format(datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S"), msg))
@@ -118,13 +112,13 @@
                 sleep = random.randint(attempt, 10 * attempt)
                 time.sleep(sleep)
             else:
-                successful_peers.append(peer)
-                LOG('Sync to {} complete.'.format(peer))
+                successful_peers.append(sync_peer)
+                LOG('Sync to {} ({}) complete.'.format(peer_unit, peer_addr))
                 break
         else:
             util.send_to_influx(util.render_influx(metric_name, label, max_retries))
             print('\n'.join(errmsgs))
-            msg = "Failed after all available attempts ({}) with peer {}".format(max_retries, peer)
+            msg = "Failed after all available attempts ({}) with peer {} ({})".format(max_retries, peer_unit, peer_addr)
             LOG(msg)
             print("{}: {}".format(datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S"), msg))
             sys.stdout.flush()

=== modified file 'lib/ubuntu_repository_cache/service.py'
--- lib/ubuntu_repository_cache/service.py	2022-08-22 04:17:09 +0000
+++ lib/ubuntu_repository_cache/service.py	2022-08-24 00:09:04 +0000
@@ -265,36 +265,40 @@
 
     local_unit = hookenv.local_unit()
 
-    # Cron job for metadata update on leader
-    cron_filename = '/etc/cron.d/ubuntu-repository-cache_rsync'
+    # Clean up old file
+    old_cron_filename = '/etc/cron.d/ubuntu-repository-cache_rsync'
+    if os.path.exists(old_cron_filename):
+        os.unlink(old_cron_filename)
+
+    # Cron job for metadata update
+    cron_filename = '/etc/cron.d/ubuntu-repository-cache'
+    cron_context = {}
+    if not unitdata.kv().get('rsync-minutes'):
+        # Don't schedule an rsync within 15 minutes of startup
+        minutes = (random.randint(0, 43) + time.gmtime().tm_min + 15) % 60
+        unitdata.kv().set('rsync-minutes', minutes)
+    cron_context['ApacheRoot'] = unitdata.kv().get('apache-root')
+    cron_context['SyncHost'] = config['sync-host']
+    cron_context['PathBase'] = config['path-base']
+    cron_context['RsyncModule'] = config['rsync-module']
+    cron_context['Minutes'] = unitdata.kv().get('rsync-minutes')
+    cron_context['UnitId'] = hookenv.local_unit()
+    cron_context['UnitPath'] = '-'.join(('unit', hookenv.local_unit().replace('/', '-')))
+    cron_context['MirrorSeries'] = config['mirror-series'].strip()
+    cron_context['ConfigPath'] = urc_config_path
+    cron_context['CharmEnvPath'] = os.path.join(hookenv.charm_dir(), 'bin', 'charm-env')
+    # Add Juju model name  and AZ so it's easier to work out which region sent out cron emails.
+    cron_context['JujuModelName'] = os.environ.get('JUJU_MODEL_NAME')
+    cron_context['JujuAvailabilityZone'] = os.environ.get('JUJU_AVAILABILITY_ZONE')
+    peers = []
     if (config['leader_unit'] == '' and cluster.is_elected_leader(None)) or (local_unit == config['leader_unit']):
-        cron_context = {}
-        if not unitdata.kv().get('rsync-minutes'):
-            # Don't schedule an rsync within 15 minutes of startup
-            minutes = (random.randint(0, 43) + time.gmtime().tm_min + 15) % 60
-            unitdata.kv().set('rsync-minutes', minutes)
-        cron_context['ApacheRoot'] = unitdata.kv().get('apache-root')
-        cron_context['SyncHost'] = config['sync-host']
-        cron_context['PathBase'] = config['path-base']
-        cron_context['RsyncModule'] = config['rsync-module']
-        cron_context['Minutes'] = unitdata.kv().get('rsync-minutes')
-        cron_context['UnitId'] = hookenv.local_unit()
-        cron_context['UnitPath'] = '-'.join(('unit', hookenv.local_unit().replace('/', '-')))
-        cron_context['MirrorSeries'] = config['mirror-series'].strip()
-        cron_context['ConfigPath'] = urc_config_path
-        cron_context['CharmEnvPath'] = os.path.join(hookenv.charm_dir(), 'bin', 'charm-env')
-        # Add Juju model name  and AZ so it's easier to work out which region sent out cron emails.
-        cron_context['JujuModelName'] = os.environ.get('JUJU_MODEL_NAME')
-        cron_context['JujuAvailabilityZone'] = os.environ.get('JUJU_AVAILABILITY_ZONE')
-        templating.render('cron/ubuntu-repository-cache_rsync.cron', cron_filename, cron_context)
-        templating.render('cron/ubuntu-repository-cache-config', urc_config_path, cron_context)
-    else:
-        try:
-            os.stat(cron_filename)
-        except OSError:
-            pass
-        else:
-            os.remove(cron_filename)
+        for u in hookenv.iter_units_for_relation_name('cluster'):
+            unit_addr = hookenv.ingress_address(rid=u.rid, unit=u.unit)
+            peers.append('{}:{}'.format(u.unit, unit_addr))
+    cron_context['Peers'] = ' '.join(peers)
+
+    templating.render('cron/ubuntu-repository-cache-cron', cron_filename, cron_context)
+    templating.render('cron/ubuntu-repository-cache-config', urc_config_path, cron_context)
 
 
 def update_checks():

=== modified file 'templates/cron/ubuntu-repository-cache-config'
--- templates/cron/ubuntu-repository-cache-config	2022-08-22 04:22:38 +0000
+++ templates/cron/ubuntu-repository-cache-config	2022-08-24 00:09:04 +0000
@@ -7,7 +7,8 @@
 export MIRROR_SERIES="{{ MirrorSeries }}"
 export PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
 export PATH_BASE={{ PathBase }}
-export PYTHONPATH=/var/lib/juju/agents/{{ UnitPath }}/charm/lib
+export PEERS="{{ Peers }}"
+export PYTHONPATH=/var/lib/juju/agents/{{ UnitPath }}/charm
 export RSYNC_MODULE={{ RsyncModule }}
 export SYNC_HOST={{ SyncHost }}
 export UNIT_PATH={{ UnitPath }}

=== renamed file 'templates/cron/ubuntu-repository-cache_rsync.cron' => 'templates/cron/ubuntu-repository-cache-cron'
--- templates/cron/ubuntu-repository-cache_rsync.cron	2022-08-08 23:08:37 +0000
+++ templates/cron/ubuntu-repository-cache-cron	2022-08-24 00:09:04 +0000
@@ -2,7 +2,11 @@
 {%- if JujuAvailabilityZone %}
 JUJU_AVAILABILITY_ZONE={{ JujuAvailabilityZone }}
 {%- endif %}
+{%- if Peers != '' %}
 # This cronjob will make the leader sync its view of the metadata with upstream
-# It will then trigger a juju-run to let its peers synchronise.
 {{ Minutes }} * * * * root . {{ ConfigPath }} && run-one {{ CharmEnvPath }} --charm ubuntu-repository-cache python3 -m ubuntu_repository_cache.metadata_sync  # {{ JujuModelName }} {% if JujuAvailabilityZone %}{{ JujuAvailabilityZone }}{% endif %}
 
+{% else %}
+{{ Minutes }} * * * * root . {{ ConfigPath }} && run-one {{ CharmEnvPath }} --charm ubuntu-repository-cache python3 -m ubuntu_repository_cache.metadata_cleanup  # {{ JujuModelName }} {% if JujuAvailabilityZone %}{{ JujuAvailabilityZone }}{% endif %}
+
+{% endif %}

=== modified file 'tests/110-multi_unit.simple'
--- tests/110-multi_unit.simple	2017-04-06 21:02:01 +0000
+++ tests/110-multi_unit.simple	2022-08-24 00:09:04 +0000
@@ -108,14 +108,14 @@
 
 # Test that leader has cron job for metadata rsync and peer does not
 try:
-    content = leader.file('/etc/cron.d/ubuntu-repository-cache_rsync')
+    content = leader.file('/etc/cron.d/ubuntu-repository-cache')
 except:
     msg = 'Leader {} missing rsync cron job'.format(unit_name(leader))
     amulet.raise_status(amulet.FAIL, msg=msg)
 log('PASS: Leader {} has the rsync cron job'.format(unit_name(leader)))
 
 try:
-    peer.file('/etc/cron.d/ubuntu-repository-cache_rsync')
+    peer.file('/etc/cron.d/ubuntu-repository-cache')
 except IOError:
     pass
 else:

=== modified file 'tests/140-failover.simple'
--- tests/140-failover.simple	2017-04-06 21:02:01 +0000
+++ tests/140-failover.simple	2022-08-24 00:09:04 +0000
@@ -130,7 +130,7 @@
 
 # Test that leader has cron job for metadata rsync and peer does not
 try:
-    content = leader.file('/etc/cron.d/ubuntu-repository-cache_rsync')
+    content = leader.file('/etc/cron.d/ubuntu-repository-cache')
 except:
     msg = 'Leader {} missing rsync cron job'.format(unit_name(leader))
     amulet.raise_status(amulet.FAIL, msg=msg)
@@ -138,7 +138,7 @@
 
 for peer in peers:
     try:
-        content = peer.file('/etc/cron.d/ubuntu-repository-cache_rsync')
+        content = peer.file('/etc/cron.d/ubuntu-repository-cache')
     except IOError:
         pass
     else:
@@ -153,7 +153,7 @@
 
 # Failover should cause only one peer unit to generate the cron job
 # This also confirms that the new leader has the cron job
-new_leader = wait_for_file(peers, '/etc/cron.d/ubuntu-repository-cache_rsync',
+new_leader = wait_for_file(peers, '/etc/cron.d/ubuntu-repository-cache',
                            RSYNC_TIMEOUT, 'Neither peer become leader')
 log('PASS: New leader {} has the rsync cron job'.format(unit_name(new_leader)))
 
@@ -171,7 +171,7 @@
 peers.remove(new_leader)
 remaining_peer = peers[0]
 try:
-    remaining_peer.file('/etc/cron.d/ubuntu-repository-cache_rsync')
+    remaining_peer.file('/etc/cron.d/ubuntu-repository-cache')
 except IOError:
     pass
 else:

=== modified file 'tests/util.py'
--- tests/util.py	2021-02-09 08:54:35 +0000
+++ tests/util.py	2022-08-24 00:09:04 +0000
@@ -12,7 +12,7 @@
 SERVICE = 'ubuntu-repository-cache'
 TIMEOUT = (40 + 60) * 60
 
-CRON_FILE = '/etc/cron.d/ubuntu-repository-cache_rsync'
+CRON_FILE = '/etc/cron.d/ubuntu-repository-cache'
 LOG_DIR = '/var/log/ubuntu-repository-cache'
 LOG_NAME = 'repository-sync-cron.log'
 LOG_FILE = os.path.join(LOG_DIR, LOG_NAME)