Merge branch 'release/2.0.3'

.coveragerc

@@ -1,14 +1,22 @@
 # .coveragerc to control coverage.py
 [run]
 branch = True
+source =
+    COT
 omit =
     */tests/*
     COT/_version.py
+    COT/commands/demo_logging.py
     setup.py
     versioneer.py
-    .tox/*
     .eggs/*
     *easy_install*
     */pypy/*
     */lib_pypy/*
     */distutils/*
+
+[paths]
+source =
+    COT
+    .tox/*/lib/python*/site-packages/COT
+    .tox/pypy/site-packages/COT

CHANGELOG.rst

@@ -3,6 +3,16 @@ Change Log
 All notable changes to the COT project will be documented in this file.
 This project adheres to `Semantic Versioning`_.
 
+`2.0.3`_ - 2017-04-03
+---------------------
+
+**Fixed**
+
+- Fixed issue where UnboundLocalError would be raised during COT's
+  attempt to clean up after a qemu-img error occurring while trying to
+  convert a disk to VMDK (`#67`_).
+- Fixed incorrect invocation of 'sudo mkdir' on Mac OS X.
+
 `2.0.2`_ - 2017-03-20
 ---------------------
 
@@ -763,6 +773,7 @@ Initial public release.
 .. _#64: https://github.com/glennmatthews/cot/issues/64
 .. _#65: https://github.com/glennmatthews/cot/issues/65
 .. _#66: https://github.com/glennmatthews/cot/issues/66
+.. _#67: https://github.com/glennmatthews/cot/issues/67
 
 .. _Semantic Versioning: http://semver.org/
 .. _`PEP 8`: https://www.python.org/dev/peps/pep-0008/
@@ -795,6 +806,7 @@ Initial public release.
 .. _verboselogs: https://verboselogs.readthedocs.io/en/latest/
 
 .. _Unreleased: https://github.com/glennmatthews/cot/compare/master...develop
+.. _2.0.3: https://github.com/glennmatthews/cot/compare/v2.0.2...v2.0.3
 .. _2.0.2: https://github.com/glennmatthews/cot/compare/v2.0.1...v2.0.2
 .. _2.0.1: https://github.com/glennmatthews/cot/compare/v2.0.0...v2.0.1
 .. _2.0.0: https://github.com/glennmatthews/cot/compare/v1.9.1...v2.0.0

COT/commands/install_helpers.py

@@ -271,34 +271,7 @@ def create_subparser(self):
                  "``--ignore-errors``.",
                  """
 > cot install-helpers
-    INFO: Installing 'fatdisk'...
-    INFO: Compiling 'fatdisk'
-    INFO: Calling './RUNME'...
 (...)
-    INFO: ...done
-    INFO: Compilation complete, installing to /usr/local/bin
-    INFO: Successfully installed 'fatdisk'
-    INFO: Calling 'fatdisk --version' and capturing its output...
-    INFO: ...done
-    INFO: Installing 'vmdktool'...
-    INFO: vmdktool requires 'zlib'... installing 'zlib'
-    INFO: Calling 'dpkg -s zlib1g-dev' and capturing its output...
-    INFO: ...done
-    INFO: Compiling 'vmdktool'
-    INFO: Calling 'make CFLAGS="-D_GNU_SOURCE -g -O -pipe"'...
-(...)
-    INFO: ...done
-    INFO: Compilation complete, installing to /usr/local
-    INFO: Calling 'make install'...
-install -s vmdktool /usr/local/bin/
-install vmdktool.8 /usr/local/man/man8/
-    INFO: ...done
-    INFO: Successfully installed 'vmdktool'
-    INFO: Calling 'vmdktool -V' and capturing its output...
-    INFO: ...done
-    INFO: Copying cot-add-disk.1 to /usr/share/man/man1/cot-add-disk.1
-(...)
-    INFO: Copying cot.1 to /usr/share/man/man1/cot.1
 Results:
 -------------
 COT manpages: successfully installed to /usr/share/man
@@ -311,7 +284,7 @@ def create_subparser(self):
 qemu-img:     present at /usr/bin/qemu-img
 vmdktool:     successfully installed to /usr/local/bin/vmdktool
 
-Unable to install some helpers""".strip())]),
+[Errno 1] Unable to install some helpers""".strip())]),
             formatter_class=argparse.RawDescriptionHelpFormatter)
 
         group = parser.add_mutually_exclusive_group()

COT/disks/tests/test_vmdk.py

@@ -100,8 +100,10 @@ def setUp(self):
         super(TestVMDKConversion, self).setUp()
         self.input_image_paths = {}
         self.input_disks = {}
+        input_dir = os.path.join(self.temp_dir, "disks")
+        os.makedirs(input_dir)
         for disk_format in ["raw", "qcow2", "vmdk"]:
-            temp_disk = os.path.join(self.temp_dir,
+            temp_disk = os.path.join(input_dir,
                                      "foo.{0}".format(disk_format))
             helpers['qemu-img'].call(['create', '-f', disk_format,
                                       temp_disk, "16M"])
@@ -117,6 +119,12 @@ def other_format_to_vmdk_test(self, disk_format,
         self.assertEqual(vmdk.disk_format, 'vmdk')
         self.assertEqual(vmdk.disk_subformat, output_subformat)
 
+        # With older versions of QEMU, COT may convert the input image to a RAW
+        # file as an intermediate step. Make sure it's cleaned up afterwards!
+        temp_image = os.path.join(self.temp_dir, 'foo.img')
+        self.assertFalse(os.path.exists(temp_image),
+                         "Temporary image {0} not deleted".format(temp_image))
+
     @mock.patch('COT.helpers.qemu_img.QEMUImg.version',
                 new_callable=mock.PropertyMock,
                 return_value=StrictVersion("1.2.0"))
@@ -134,13 +142,46 @@ def test_disk_conversion_old_qemu(self,
         for disk_format in ["raw", "qcow2", "vmdk"]:
             self.other_format_to_vmdk_test(disk_format)
 
-            for call_args in mock_qemu_call.call_args_list:
-                self.assertNotIn('convert', call_args)
+            if disk_format != "raw":
+                # use qemu-img to convert to raw
+                mock_qemu_call.assert_called_once_with(
+                    ['convert', '-O', 'raw',
+                     self.input_disks[disk_format].path, mock.ANY])
+            else:
+                mock_qemu_call.assert_not_called()
+
             mock_vmdktool_call.assert_called_once()
 
             mock_qemu_call.reset_mock()
             mock_vmdktool_call.reset_mock()
 
+    @mock.patch('COT.helpers.qemu_img.QEMUImg.version',
+                new_callable=mock.PropertyMock,
+                return_value=StrictVersion("1.2.0"))
+    def test_disk_conversion_old_qemu_error(self, *_):
+        """Error recovery/cleanup during multi-step conversion with vmdktool.
+
+        https://github.com/glennmatthews/cot/issues/67
+        """
+        # Error in conversion from qcow2 to raw
+        with mock.patch('COT.helpers.qemu_img.QEMUImg.call',
+                        side_effect=HelperError):
+            self.assertRaises(HelperError,
+                              VMDK.from_other_image,
+                              self.input_disks['qcow2'], self.temp_dir)
+
+        # Error in conversion from raw to vmdk
+        with mock.patch('COT.helpers.vmdktool.VMDKTool.call',
+                        side_effect=HelperError):
+            self.assertRaises(HelperError,
+                              VMDK.from_other_image,
+                              self.input_disks['qcow2'], self.temp_dir)
+
+        # Make sure we didn't leave the temporary image behind
+        temp_image = os.path.join(self.temp_dir, 'foo.img')
+        self.assertFalse(os.path.exists(temp_image),
+                         "Temporary image {0} not deleted".format(temp_image))
+
     @mock.patch('COT.helpers.qemu_img.QEMUImg.version',
                 new_callable=mock.PropertyMock,
                 return_value=StrictVersion("2.1.0"))
@@ -157,12 +198,17 @@ def test_disk_conversion_med_qemu(self,
         we still prefer vmdktool, but fall back to qemu-img with a warning
         if vmdktool is not available.
         """
-        # First, with vmdktool, same as previous test case
+        # First, with vmdktool, same as test_disk_conversion_old_qemu
         for disk_format in ["raw", "qcow2", "vmdk"]:
             self.other_format_to_vmdk_test(disk_format)
 
-            for call_args in mock_qemu_call.call_args_list:
-                self.assertNotIn('convert', call_args)
+            if disk_format != "raw":
+                # use qemu-img to convert to raw
+                mock_qemu_call.assert_called_once_with(
+                    ['convert', '-O', 'raw',
+                     self.input_disks[disk_format].path, mock.ANY])
+            else:
+                mock_qemu_call.assert_not_called()
             mock_vmdktool_call.assert_called_once()
 
             mock_qemu_call.reset_mock()

COT/disks/vmdk.py

@@ -102,14 +102,17 @@ def from_other_image(cls, input_image, output_dir,
                 if input_image.disk_format != 'raw':
                     # vmdktool needs a raw image as input
                     from COT.disks import RAW
+                    temp_image = None
                     try:
                         temp_image = RAW.from_other_image(input_image,
                                                           output_dir)
                         return cls.from_other_image(temp_image,
                                                     output_dir,
                                                     output_subformat)
                     finally:
-                        os.remove(temp_image.path)
+                        if temp_image is not None:
+                            os.remove(temp_image.path)
+                            temp_image = None
 
                 # Note that vmdktool takes its arguments in unusual order -
                 # output file comes before input file

COT/helpers/helper.py

@@ -466,7 +466,8 @@ def mkdir(directory, permissions=493):    # 493 == 0o755
                          directory)
             try:
                 check_call(['sudo', 'mkdir', '-p',
-                            '--mode=%o' % permissions,
+                            # We previously used '--mode' but OS X lacks it.
+                            '-m', '%o' % permissions,
                             directory])
             except HelperError:
                 # That failed too - re-raise the original exception
@@ -582,6 +583,15 @@ def check_call(args, require_success=True, retry_with_sudo=False, **kwargs):
                        retry_with_sudo=False,
                        **kwargs)
             return
+        # In Travis CI container environment, 'sudo' is disallowed.
+        # For some reason, recently (4/2017) it's changed from failing with
+        # EPERM "sudo: must be setuid root"
+        # to:
+        # ENOEXEC "Exec format error"
+        # We shouldn't see ENOEXEC otherwise, so we special case this.
+        if (exc.errno == errno.ENOEXEC and
+                args[0] == 'sudo'):    # pragma: no cover
+            raise HelperError(exc.errno, "The 'sudo' command is unavailable")
         if exc.errno != errno.ENOENT:
             raise
         raise HelperNotFoundError(exc.errno,

COT/helpers/tests/test_fatdisk.py

@@ -92,7 +92,7 @@ def test_install_apt_get(self,
                 ['apt-get', '-q', 'install', 'make'],
                 ['apt-get', '-q', 'install', 'gcc'],
                 ['./RUNME'],
-                ['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/bin'],
+                ['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/bin'],
             ])
         self.assertTrue(re.search("/fatdisk$", mock_copy.call_args[0][0]))
         self.assertEqual('/usr/local/bin', mock_copy.call_args[0][1])
@@ -119,7 +119,7 @@ def test_install_apt_get(self,
             mock_check_call,
             [
                 ['./RUNME'],
-                ['sudo', 'mkdir', '-p', '--mode=755',
+                ['sudo', 'mkdir', '-p', '-m', '755',
                  '/home/cot/opt/local/bin'],
             ])
         self.assertTrue(re.search("/fatdisk$", mock_copy.call_args[0][0]))
@@ -156,7 +156,7 @@ def test_install_yum(self,
                 ['yum', '--quiet', 'install', 'make'],
                 ['yum', '--quiet', 'install', 'gcc'],
                 ['./RUNME'],
-                ['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/bin'],
+                ['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/bin'],
             ])
         self.assertTrue(re.search("/fatdisk$", mock_copy.call_args[0][0]))
         self.assertEqual('/usr/local/bin', mock_copy.call_args[0][1])

COT/helpers/tests/test_helper.py

@@ -500,7 +500,7 @@ def test_need_sudo(self, mock_isdir, mock_exists,
         mock_exists.assert_called_with('/foo/bar')
         mock_makedirs.assert_called_with('/foo/bar', 493)  # 493 == 0o755
         mock_check_call.assert_called_with(
-            ['sudo', 'mkdir', '-p', '--mode=755', '/foo/bar'])
+            ['sudo', 'mkdir', '-p', '-m', '755', '/foo/bar'])
 
     def test_nondefault_permissions(self, mock_isdir, mock_exists,
                                     mock_makedirs, mock_check_call):
@@ -518,7 +518,7 @@ def test_nondefault_permissions(self, mock_isdir, mock_exists,
         self.assertTrue(Helper.mkdir('/foo/bar', 511))  # 511 == 0o777
         mock_makedirs.assert_called_with('/foo/bar', 511)
         mock_check_call.assert_called_with(
-            ['sudo', 'mkdir', '-p', '--mode=777', '/foo/bar'])
+            ['sudo', 'mkdir', '-p', '-m', '777', '/foo/bar'])
 
 
 @mock.patch('COT.helpers.helper.check_call')

COT/helpers/tests/test_vmdktool.py

@@ -84,8 +84,8 @@ def test_install_helper_apt_get(self,
                 ['apt-get', '-q', 'install', 'make'],
                 ['apt-get', '-q', 'install', 'zlib1g-dev'],
                 ['make', 'CFLAGS="-D_GNU_SOURCE -g -O -pipe"'],
-                ['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/man/man8'],
-                ['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/bin'],
+                ['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/man/man8'],
+                ['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/bin'],
                 ['make', 'install', 'PREFIX=/usr/local'],
             ])
         self.assertAptUpdated()
@@ -111,9 +111,9 @@ def test_install_helper_apt_get(self,
             mock_check_call,
             [
                 ['make', 'CFLAGS="-D_GNU_SOURCE -g -O -pipe"'],
-                ['sudo', 'mkdir', '-p', '--mode=755',
+                ['sudo', 'mkdir', '-p', '-m', '755',
                  '/home/cot/opt/local/man/man8'],
-                ['sudo', 'mkdir', '-p', '--mode=755',
+                ['sudo', 'mkdir', '-p', '-m', '755',
                  '/home/cot/opt/local/bin'],
                 ['make', 'install', 'PREFIX=/opt/local', 'DESTDIR=/home/cot'],
             ])
@@ -146,8 +146,8 @@ def test_install_helper_yum(self,
                 ['yum', '--quiet', 'install', 'make'],
                 ['yum', '--quiet', 'install', 'zlib-devel'],
                 ['make', 'CFLAGS="-D_GNU_SOURCE -g -O -pipe"'],
-                ['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/man/man8'],
-                ['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/bin'],
+                ['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/man/man8'],
+                ['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/bin'],
                 ['make', 'install', 'PREFIX=/usr/local'],
             ])
 

tox.ini

@@ -31,7 +31,7 @@ PyPy = setup,                pypy,       stats
 passenv = PREFIX
 deps =
     -rrequirements.txt
-    coverage==4.1
+    coverage==4.3.4
     mock
     sphinx>=1.3
     sphinx_rtd_theme
@@ -47,7 +47,6 @@ commands =
 
 [testenv:stats]
 commands =
-    coverage combine
     coverage report -i
     coverage html -i