On Thu, Aug 22, 2013 at 10:16 AM, Helga Velroyen <[email protected]> wrote:
> This patch factors out the function that checks the DRBD > usermode helper in bootstrap (cluster init) in order to make > it more testable. It also contains the unit tests. Otherwise, > not functional changes. > s/not/no/ > > Signed-off-by: Helga Velroyen <[email protected]> > --- > lib/bootstrap.py | 38 > ++++++++++++++++++++++++------------ > test/py/ganeti.bootstrap_unittest.py | 32 ++++++++++++++++++++++++++++++ > 2 files changed, 57 insertions(+), 13 deletions(-) > > diff --git a/lib/bootstrap.py b/lib/bootstrap.py > index 1b862c5..fe5a1f4 100644 > --- a/lib/bootstrap.py > +++ b/lib/bootstrap.py > @@ -472,6 +472,30 @@ def _RestrictIpolicyToEnabledDiskTemplates(ipolicy, > enabled_disk_templates): > ipolicy[constants.IPOLICY_DTS] = restricted_disk_templates > > > +def _InitCheckDrbdHelper(drbd_helper, > + get_helper_fn=drbd.DRBD8.GetUsermodeHelper): > + """Checks the DRBD usermode helper. > + > + @type drbd_helper: string > + @param drbd_helper: name of the DRBD usermode helper that the system > should > + use > + > + """ > + if drbd_helper is not None: > + try: > + curr_helper = get_helper_fn() > + except errors.BlockDeviceError, err: > + raise errors.OpPrereqError("Error while checking drbd helper" > + " (specify --no-drbd-storage if you are > not" > + " using drbd): %s" % str(err), > + errors.ECODE_ENVIRON) > + if drbd_helper != curr_helper: > + raise errors.OpPrereqError("Error: requiring %s as drbd helper but > %s" > + " is the current helper" % (drbd_helper, > + curr_helper), > + errors.ECODE_INVAL) > + > + > def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914 > master_netmask, master_netdev, file_storage_dir, > shared_file_storage_dir, candidate_pool_size, > secondary_ip=None, > @@ -569,19 +593,7 @@ def InitCluster(cluster_name, mac_prefix, # pylint: > disable=R0913, R0914 > if vgstatus: > raise errors.OpPrereqError("Error: %s" % vgstatus, > errors.ECODE_INVAL) > > - if drbd_helper is not None: > - try: > - curr_helper = drbd.DRBD8.GetUsermodeHelper() > - except errors.BlockDeviceError, err: > - raise errors.OpPrereqError("Error while checking drbd helper" > - " (specify --no-drbd-storage if you are > not" > - " using drbd): %s" % str(err), > - errors.ECODE_ENVIRON) > - if drbd_helper != curr_helper: > - raise errors.OpPrereqError("Error: requiring %s as drbd helper but > %s" > - " is the current helper" % (drbd_helper, > - curr_helper), > - errors.ECODE_INVAL) > + _InitCheckDrbdHelper(drbd_helper) > > logging.debug("Stopping daemons (if any are running)") > result = utils.RunCmd([pathutils.DAEMON_UTIL, "stop-all"]) > diff --git a/test/py/ganeti.bootstrap_unittest.py b/test/py/ > ganeti.bootstrap_unittest.py > index 82c4a16..a0e5135 100755 > --- a/test/py/ganeti.bootstrap_unittest.py > +++ b/test/py/ganeti.bootstrap_unittest.py > @@ -130,5 +130,37 @@ class > TestRestrictIpolicyToEnabledDiskTemplates(unittest.TestCase): > self.assertEqual(ipolicy[constants.IPOLICY_DTS], [constants.DT_PLAIN]) > > > +class TestInitCheckDrbdHelper(unittest.TestCase): > + > + def testHelperNone(self): > + current_helper = "/bin/helper" > + drbd_helper = None > + mock_helper_fn = mock.Mock(return_value=current_helper) > + bootstrap._InitCheckDrbdHelper(drbd_helper, > get_helper_fn=mock_helper_fn) > + > + def testHelperOk(self): > + current_helper = "/bin/helper" > + drbd_helper = "/bin/helper" > + mock_helper_fn = mock.Mock(return_value=current_helper) > + bootstrap._InitCheckDrbdHelper(drbd_helper, > get_helper_fn=mock_helper_fn) > + > + def testWrongHelper(self): > + current_helper = "/bin/otherhelper" > + drbd_helper = "/bin/helper" > + mock_helper_fn = mock.Mock(return_value=current_helper) > + self.assertRaises(errors.OpPrereqError, > + bootstrap._InitCheckDrbdHelper, > + drbd_helper, get_helper_fn=mock_helper_fn) > + > + def testHelperCheckFails(self): > + current_helper = "/bin/otherhelper" > + drbd_helper = "/bin/helper" > + mock_helper_fn = mock.Mock(return_value=current_helper) > + mock_helper_fn.side_effect = errors.BlockDeviceError > + self.assertRaises(errors.OpPrereqError, > + bootstrap._InitCheckDrbdHelper, > + drbd_helper, get_helper_fn=mock_helper_fn) > + > + > if __name__ == "__main__": > testutils.GanetiTestProgram() > -- > 1.8.3 > > Rest LGTM. Note that you could also use @testutils.patch_object instead of introducing a new parameter, as is it done (incidentally for the drbd.DRBD8 class as well) in ganeti.storage.drbd_unittest.py:440. Thanks, Thomas -- Thomas Thrainer | Software Engineer | [email protected] | Google Germany GmbH Dienerstr. 12 80331 München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores
