Tighten up the leader_set(..) usage to handle any errors

This stops leader_set from throwing backtraces, and instead logs the
error and continues as though the charm is not the leader (which is the
case when leader_set(...) fails).

Changed the py35 tox job to invoke py3 (to allow it to also test under
py3.6 on artful+).

Note that the pep8 check is still py27, so the additional # NOQA is
to handle Py2 not having FileNotFoundError.

Change-Id: Ic25d29983db9a0738d83e66de4673bb50594b599

hooks/ceph_hooks.py

@@ -205,25 +205,33 @@ def config_changed():
                     mon_secret = config('monitor-secret')
                 else:
                     mon_secret = "{}".format(ceph.generate_monitor_secret())
-                status_set('maintenance', 'Creating FSID and Monitor Secret')
                 opts = {
                     'fsid': fsid,
                     'monitor-secret': mon_secret,
                 }
-                log("Settings for the cluster are: {}".format(opts))
+                try:
-                leader_set(opts)
+                    leader_set(opts)
-        elif cfg.changed('no-bootstrap') and \
+                    status_set('maintenance',
-                is_relation_made('bootstrap-source'):
+                               'Created FSID and Monitor Secret')
+                    log("Settings for the cluster are: {}".format(opts))
+                except Exception as e:
+                    # we're probably not the leader an exception occured
+                    # let's log it anyway.
+                    log("leader_set failed: {}".format(str(e)))
+        elif (cfg.changed('no-bootstrap') and
+              is_relation_made('bootstrap-source')):
             # User changed the no-bootstrap config option, we're the leader,
             # and the bootstrap-source relation has been made. The charm should
             # be in a blocked state indicating that the no-bootstrap option
             # must be set. This block is invoked when the user is trying to
             # get out of that scenario by enabling no-bootstrap.
             bootstrap_source_relation_changed()
-    elif leader_get('fsid') is None or leader_get('monitor-secret') is None:
+    # unconditionally verify that the fsid and monitor-secret are set now
+    # otherwise we exit until a leader does this.
+    if leader_get('fsid') is None or leader_get('monitor-secret') is None:
             log('still waiting for leader to setup keys')
             status_set('waiting', 'Waiting for leader to setup keys')
-            sys.exit(0)
+            return
 
     emit_cephconf()
 
@@ -231,7 +239,12 @@ def config_changed():
     if (not ceph.is_bootstrapped() and int(config('monitor-count')) == 1 and
             is_leader()):
         status_set('maintenance', 'Bootstrapping single Ceph MON')
-        ceph.bootstrap_monitor_cluster(leader_get('monitor-secret'))
+        # the following call raises an exception if it can't add the keyring
+        try:
+            ceph.bootstrap_monitor_cluster(leader_get('monitor-secret'))
+        except FileNotFoundError as e:  # NOQA -- PEP8 is still PY2
+            log("Couldn't bootstrap the monitor yet: {}".format(str(e)))
+            return
         ceph.wait_for_bootstrap()
         if cmp_pkgrevno('ceph', '12.0.0') >= 0:
             status_set('maintenance', 'Bootstrapping single Ceph MGR')
@@ -323,15 +336,18 @@ def bootstrap_source_relation_changed():
                 assert curr_secret == mon_secret, \
                     "bootstrap secret '{}' != current secret '{}'".format(
                         mon_secret, curr_secret)
-
             opts = {
                 'fsid': fsid,
                 'monitor-secret': mon_secret,
             }
-
+            try:
-            log('Updating leader settings for fsid and monitor-secret '
+                leader_set(opts)
-                'from remote relation data: {}'.format(opts))
+                log('Updating leader settings for fsid and monitor-secret '
-            leader_set(opts)
+                    'from remote relation data: {}'.format(opts))
+            except Exception as e:
+                # we're probably not the leader an exception occured
+                # let's log it anyway.
+                log("leader_set failed: {}".format(str(e)))
 
     # The leader unit needs to bootstrap itself as it won't receive the
     # leader-settings-changed hook elsewhere.
@@ -353,7 +369,12 @@ def mon_relation():
     moncount = int(config('monitor-count'))
     if len(get_mon_hosts()) >= moncount:
         status_set('maintenance', 'Bootstrapping MON cluster')
-        ceph.bootstrap_monitor_cluster(leader_get('monitor-secret'))
+        # the following call raises an exception if it can't add the keyring
+        try:
+            ceph.bootstrap_monitor_cluster(leader_get('monitor-secret'))
+        except FileNotFoundError as e:  # NOQA -- PEP8 is still PY2
+            log("Couldn't bootstrap the monitor yet: {}".format(str(e)))
+            exit(0)
         ceph.wait_for_bootstrap()
         ceph.wait_for_quorum()
         if cmp_pkgrevno('ceph', '12.0.0') >= 0:

tox.ini

@@ -2,9 +2,10 @@
 # This file is managed centrally by release-tools and should not be modified
 # within individual charm repos.
 [tox]
-envlist = pep8,py27,py35,py36
+;envlist = pep8,py27,py35,py36
+envlist = pep8,py27,py35
 skipsdist = True
-skip_missing_interpreters = True
+;skip_missing_interpreters = True
 
 [testenv]
 setenv = VIRTUAL_ENV={envdir}
@@ -24,13 +25,9 @@ deps = -r{toxinidir}/requirements.txt
 # temporarily disable py27
 commands = /bin/true
 
+; keep zuul happy until we change the py35 job
 [testenv:py35]
-basepython = python3.5
+basepython = python3
-deps = -r{toxinidir}/requirements.txt
-       -r{toxinidir}/test-requirements.txt
-
-[testenv:py36]
-basepython = python3.6
 deps = -r{toxinidir}/requirements.txt
        -r{toxinidir}/test-requirements.txt
 

unit_tests/test_utils.py

@@ -17,8 +17,7 @@ import unittest
 import os
 import yaml
 
-from contextlib import contextmanager
+from mock import patch
-from mock import patch, MagicMock
 
 
 def load_config():
@@ -145,21 +144,3 @@ class TestLeaderSettings(object):
         elif attr in self.settings:
             return self.settings[attr]
         return None
-
-
-@contextmanager
-def patch_open():
-    '''Patch open() to allow mocking both open() itself and the file that is
-    yielded.
-
-    Yields the mock for "open" and "file", respectively.'''
-    mock_open = MagicMock(spec=open)
-    mock_file = MagicMock(spec=file)
-
-    @contextmanager
-    def stub_open(*args, **kwargs):
-        mock_open(*args, **kwargs)
-        yield mock_file
-
-    with patch('__builtin__.open', stub_open):
-        yield mock_open, mock_file