Merge lp:~danilo/charms/trusty/glance-simplestreams-sync/leader-only into lp:~landscape/charms/trusty/glance-simplestreams-sync/landscape

[Данило Шеган]

Make glance-simplestreams-sync.py script no-op when not run on the leader unit

To avoid changing the structure too much, we do not set-up and destroy cronjobs in leader-change hooks, but instead simply make the cronjob script no-op when not run on the leader unit. Cronjob setup is a bit intricate and involved (there's a per-minute cronjob set initially, along with the cronjob depending on the frequency configured for the service).

Testing instructions:

This charm has been published at cs:~danilo/trusty/glance-simplestreams-sync-21

See Landscape branch that integrates this charm https://code.launchpad.net/~danilo/landscape/ha-image-refresh/+merge/293751

[lp:~danilo/charms/trusty/glance-simplestreams-sync/leader-only]

70. By Данило Шеган on 2016-05-04

Strip the returned value from is-leader.

[Chris Glass (tribaal)]

Looks good, one nitpick inline.

review: Approve

[lp:~danilo/charms/trusty/glance-simplestreams-sync/leader-only]

71. By Данило Шеган on 2016-05-04

Return explicit False (instead of implicit None) in is_leader() on exception.

[Bogdana Vereha (bogdana)]

+1

review: Approve

[lp:~danilo/charms/trusty/glance-simplestreams-sync/leader-only]

72. By Данило Шеган on 2016-05-04

Add leader hooks.

73. By Данило Шеган on 2016-05-05

Merge landscape charm tip.

74. By Данило Шеган on 2016-05-05

Do not drop the glance-simplestreams-sync per-minute cronjob when not leader inside the script, but manage that from leader-related hooks.

[Данило Шеган (danilo)]

As discussed with David, we'll be shelving this work for now, waiting for glance-simplestreams-sync stuff to be integrated into glance itself, when we'll get HA as well.

Unmerged revisions

74. By Данило Шеган on 2016-05-05

Do not drop the glance-simplestreams-sync per-minute cronjob when not leader inside the script, but manage that from leader-related hooks.

73. By Данило Шеган on 2016-05-05

Merge landscape charm tip.

72. By Данило Шеган on 2016-05-04

Add leader hooks.

71. By Данило Шеган on 2016-05-04

Return explicit False (instead of implicit None) in is_leader() on exception.

70. By Данило Шеган on 2016-05-04

Strip the returned value from is-leader.

69. By Данило Шеган on 2016-05-04

Merge lint fixes from trunk.

68. By Данило Шеган on 2016-05-04

Lint fix.

67. By Данило Шеган on 2016-05-04

Revert accidental change.

66. By Данило Шеган on 2016-04-29

Use a better check for is-leader.

65. By Данило Шеган on 2016-04-28

Do not validate full configuration when getting unit_name for juju_run_cmd().

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2016-04-14 07:03:35 +0000
+++ hooks/hooks.py	2016-05-05 09:04:26 +0000
@@ -246,6 +246,18 @@
     config.save()
 
 
[e-mail]('leader-elected')
+def leader_elected():
+    hookenv.log('elected as leader.')
+    install_cron_poll()
+
+
[e-mail]('leader-settings-changed')
+def leader_settings_changed():
+    hookenv.log('leader settings changed.')
+    uninstall_cron_poll()
+
+
 @hooks.hook('upgrade-charm')
 def upgrade_charm():
     install()

=== modified file 'scripts/glance-simplestreams-sync.py'
--- scripts/glance-simplestreams-sync.py	2016-04-27 13:04:08 +0000
+++ scripts/glance-simplestreams-sync.py	2016-05-05 09:04:26 +0000
@@ -133,7 +133,7 @@
     return confobj
 
 
-def get_conf():
+def get_conf(validate=True):
     conf_files = [ID_CONF_FILE_NAME, CHARM_CONF_FILE_NAME]
     for conf_file_name in conf_files:
         if not os.path.exists(conf_file_name):
@@ -141,12 +141,12 @@
             sys.exit(1)
 
     id_conf = read_conf(ID_CONF_FILE_NAME)
-    if None in id_conf.values():
+    if validate and None in id_conf.values():
         log.info("Configuration value missing in {}:\n"
                  "{}".format(ID_CONF_FILE_NAME, id_conf))
         sys.exit(1)
     charm_conf = read_conf(CHARM_CONF_FILE_NAME)
-    if None in charm_conf.values():
+    if validate and None in charm_conf.values():
         log.info("Configuration value missing in {}:\n"
                  "{}".format(CHARM_CONF_FILE_NAME, charm_conf))
         sys.exit(1)
@@ -271,7 +271,7 @@
 
 def juju_run_cmd(cmd):
     '''Execute the passed commands under the local unit context'''
-    id_conf, _ = get_conf()
+    id_conf, _ = get_conf(validate=False)
     unit_name = id_conf['unit_name']
     _cmd = ['juju-run', unit_name, ' '.join(cmd)]
     log.info("Executing command: {}".format(_cmd))
@@ -286,6 +286,16 @@
         log.info(message)
 
 
+def is_leader():
+    """Returns True if the script is running on the leader unit."""
+    try:
+        value = juju_run_cmd(['is-leader']).strip()
+        return value == 'True'
+    except subprocess.CalledProcessError as e:
+        log.info(e.message)
+        return False
+
+
 def update_endpoint_urls(region, publicurl, adminurl, internalurl):
     # Notify keystone via the identity service relation about
     # and endpoint changes
@@ -381,8 +391,21 @@
             raise e
 
 
+def remove_initial_cronjob(should_delete_cron_poll=False):
+    """
+    Remove the initial per-minute cron job if should_delete_cron_poll is True.
+    """
+    if os.path.exists(CRON_POLL_FILENAME) and should_delete_cron_poll:
+        os.unlink(CRON_POLL_FILENAME)
+        log.info("Initial sync attempt done. every-minute cronjob removed.")
+
+
 def main():
 
+    if not is_leader():
+        log.info("glance-simplestreams-sync can only run on the leader unit.")
+        sys.exit(0)
+
     log.info("glance-simplestreams-sync started.")
 
     lockfile = open(SYNC_RUNNING_FLAG_FILE_NAME, 'w')
@@ -476,9 +499,7 @@
 
     status_exchange.close()
 
-    if os.path.exists(CRON_POLL_FILENAME) and should_delete_cron_poll:
-        os.unlink(CRON_POLL_FILENAME)
-        log.info("Initial sync attempt done. every-minute cronjob removed.")
+    remove_initial_cronjob(should_delete_cron_poll)
     log.info("sync done.")