On Tue, Oct 15, 2013 at 12:01 AM, Thomas Thrainer <[email protected]>wrote:

> This patch adds unit tests for LUBackupExport.
>
> Signed-off-by: Thomas Thrainer <[email protected]>
> ---
>  lib/cmdlib/backup.py              |   2 +-
>  test/py/cmdlib/backup_unittest.py | 114
> +++++++++++++++++++++++++++++++++++++-
>  2 files changed, 113 insertions(+), 3 deletions(-)
>
> diff --git a/lib/cmdlib/backup.py b/lib/cmdlib/backup.py
> index 06920f5..79e7d27 100644
> --- a/lib/cmdlib/backup.py
> +++ b/lib/cmdlib/backup.py
> @@ -396,7 +396,7 @@ class LUBackupExport(LogicalUnit):
>      activate_disks = not self.instance.disks_active
>
>      if activate_disks:
> -      # Activate the instance disks if we'exporting a stopped instance
> +      # Activate the instance disks if we're exporting a stopped instance
>        feedback_fn("Activating disks for %s" % self.instance.name)
>        StartInstanceDisks(self, self.instance, None)
>
> diff --git a/test/py/cmdlib/backup_unittest.py
> b/test/py/cmdlib/backup_unittest.py
> index 3843071..db938f2 100644
> --- a/test/py/cmdlib/backup_unittest.py
> +++ b/test/py/cmdlib/backup_unittest.py
> @@ -21,9 +21,8 @@
>
>  """Tests for LUBackup*"""
>
> -import mock
> -
>  from ganeti import constants
> +from ganeti import objects
>  from ganeti import opcodes
>  from ganeti import query
>
> @@ -110,5 +109,116 @@ qRNQR5h9LJfZv7zZZSI=
>      self.ExecOpCode(op)
>
>
> +class TestLUBackupExportBase(CmdlibTestCase):
> +  def setUp(self):
> +    super(TestLUBackupExportBase, self).setUp()
> +
> +    self.rpc.call_instance_start.return_value = \
> +      self.RpcResultsBuilder() \
> +        .CreateSuccessfulNodeResult(self.master, True)
> +
> +    self.rpc.call_blockdev_assemble.return_value = \
> +      self.RpcResultsBuilder() \
> +        .CreateSuccessfulNodeResult(self.master, None)
> +
> +    self.rpc.call_blockdev_shutdown.return_value = \
> +      self.RpcResultsBuilder() \
> +        .CreateSuccessfulNodeResult(self.master, None)
> +
> +    self.rpc.call_blockdev_snapshot.return_value = \
> +      self.RpcResultsBuilder() \
> +        .CreateSuccessfulNodeResult(self.master, ("mock_vg", "mock_id"))
> +
> +    self.rpc.call_blockdev_remove.return_value = \
> +      self.RpcResultsBuilder() \
> +        .CreateSuccessfulNodeResult(self.master, None)
> +
> +    self.rpc.call_export_start.return_value = \
> +      self.RpcResultsBuilder() \
> +        .CreateSuccessfulNodeResult(self.master, "export_daemon")
> +
> +    def ImpExpStatus(node_uuid, name):
> +      return self.RpcResultsBuilder() \
> +               .CreateSuccessfulNodeResult(node_uuid,
> +                                           [objects.ImportExportStatus(
> +                                             exit_status=0
> +                                           )])
> +    self.rpc.call_impexp_status.side_effect = ImpExpStatus
> +
> +    def ImpExpCleanup(node_uuid, name):
> +      return self.RpcResultsBuilder() \
> +               .CreateSuccessfulNodeResult(node_uuid)
> +    self.rpc.call_impexp_cleanup.side_effect = ImpExpCleanup
> +
> +    self.rpc.call_finalize_export.return_value = \
> +      self.RpcResultsBuilder() \
> +        .CreateSuccessfulNodeResult(self.master, None)
> +
> +  def testRemoveRunningInstanceWithoutShutdown(self):
> +    inst = self.cfg.AddNewInstance(admin_state=constants.ADMINST_UP)
> +    op = opcodes.OpBackupExport(instance_name=inst.name,
> +                                target_node=self.master.name,
> +                                shutdown=False,
> +                                remove_instance=True)
> +    self.ExecOpCodeExpectOpPrereqError(
> +      op, "Can not remove instance without shutting it down before")
> +
> +  def testUnsupportedDiskTemplate(self):
> +    inst = self.cfg.AddNewInstance(disk_template=constants.DT_FILE)
> +    op = opcodes.OpBackupExport(instance_name=inst.name,
> +                                target_node=self.master.name)
> +    self.ExecOpCodeExpectOpPrereqError(
> +      op, "Export not supported for instances with file-based disks")
>

I find it a bit odd to include a unittest that does not test the
requirements, but tests for an error message which is only a (poor)
workaround of an issue that needs to be fixed. Fixing the issue would
actually break the test then.


> +
> +
> +class TestLUBackupExportLocalExport(TestLUBackupExportBase):
> +  def setUp(self):
> +    super(TestLUBackupExportLocalExport, self).setUp()
> +
> +    self.inst = self.cfg.AddNewInstance()
> +    self.target_node = self.cfg.AddNewNode()
> +    self.op = opcodes.OpBackupExport(mode=constants.EXPORT_MODE_LOCAL,
> +                                     instance_name=self.inst.name,
> +                                     target_node=self.target_node.name)
> +
> +    self.rpc.call_import_start.return_value = \
> +      self.RpcResultsBuilder() \
> +        .CreateSuccessfulNodeResult(self.target_node, "import_daemon")
> +
> +  def testExportWithShutdown(self):
> +    inst = self.cfg.AddNewInstance(admin_state=constants.ADMINST_UP)
> +    op = self.CopyOpCode(self.op, instance_name=inst.name, shutdown=True)
> +    self.ExecOpCode(op)
> +
> +  def testExportDeactivatedDisks(self):
> +    self.ExecOpCode(self.op)
> +
> +  def testExportRemoveInstance(self):
> +    op = self.CopyOpCode(self.op, remove_instance=True)
> +    self.ExecOpCode(op)
> +
> +
> +class TestLUBackupExportRemoteExport(TestLUBackupExportBase):
> +  def setUp(self):
> +    super(TestLUBackupExportRemoteExport, self).setUp()
> +
> +    self.inst = self.cfg.AddNewInstance()
> +    self.op = opcodes.OpBackupExport(mode=constants.EXPORT_MODE_REMOTE,
> +                                     instance_name=self.inst.name,
> +                                     target_node=[],
> +                                     x509_key_name=["mock_key_name"],
> +                                     destination_x509_ca="mock_dest_ca")
> +
> +  def testRemoteExportWithoutX509KeyName(self):
> +    op = self.CopyOpCode(self.op, x509_key_name=self.REMOVE)
> +    self.ExecOpCodeExpectOpPrereqError(op,
> +                                       "Missing X509 key name for
> encryption")
> +
> +  def testRemoteExportWithoutX509DestCa(self):
> +    op = self.CopyOpCode(self.op, destination_x509_ca=self.REMOVE)
> +    self.ExecOpCodeExpectOpPrereqError(op,
> +                                       "Missing destination X509 CA")
> +
> +
>  if __name__ == "__main__":
>    testutils.GanetiTestProgram()
> --
> 1.8.4
>
>
LGTM, thanks


-- 
-- 
Helga Velroyen | 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

Reply via email to