PXE driver should rmtree directories it created

Baremetal PXE driver was failing to delete the per-instance tftpboot and
image directories which it created when the instance was deleted. This
happened partly because of dangling files within the directory, and
partly because 'unlink' does not remove directories and the error was
squelched.

Now, when destroy()ing an instance, PXE driver will call shutil.rmtree()
on the per-instance directories which it created.

Fixes bug 1101048.

Change-Id: I682d0264288add488ea23e5d5200357b7be52dd9

nova/tests/baremetal/test_pxe.py

@@ -414,10 +414,11 @@ def test_cache_images(self):
 
     def test_destroy_images(self):
         self._create_node()
-        self.mox.StubOutWithMock(os, 'unlink')
+        self.mox.StubOutWithMock(bm_utils, 'unlink_without_raise')
+        self.mox.StubOutWithMock(bm_utils, 'rmtree_without_raise')
 
-        os.unlink(pxe.get_image_file_path(self.instance))
-        os.unlink(pxe.get_image_dir_path(self.instance))
+        bm_utils.unlink_without_raise(pxe.get_image_file_path(self.instance))
+        bm_utils.rmtree_without_raise(pxe.get_image_dir_path(self.instance))
         self.mox.ReplayAll()
 
         self.driver.destroy_images(self.context, self.node, self.instance)
@@ -482,6 +483,7 @@ def test_deactivate_bootloader(self):
         pxe_path = pxe.get_pxe_config_file_path(self.instance)
 
         self.mox.StubOutWithMock(bm_utils, 'unlink_without_raise')
+        self.mox.StubOutWithMock(bm_utils, 'rmtree_without_raise')
         self.mox.StubOutWithMock(pxe, 'get_tftp_image_info')
         self.mox.StubOutWithMock(self.driver, '_collect_mac_addresses')
 
@@ -493,7 +495,7 @@ def test_deactivate_bootloader(self):
                 AndReturn(macs)
         for mac in macs:
             bm_utils.unlink_without_raise(pxe.get_pxe_mac_path(mac))
-        bm_utils.unlink_without_raise(
+        bm_utils.rmtree_without_raise(
                 os.path.join(CONF.baremetal.tftp_root, 'fake-uuid'))
         self.mox.ReplayAll()
 
@@ -516,6 +518,7 @@ def test_deactivate_bootloader_for_nonexistent_instance(self):
         pxe_path = pxe.get_pxe_config_file_path(self.instance)
 
         self.mox.StubOutWithMock(bm_utils, 'unlink_without_raise')
+        self.mox.StubOutWithMock(bm_utils, 'rmtree_without_raise')
         self.mox.StubOutWithMock(pxe, 'get_tftp_image_info')
         self.mox.StubOutWithMock(self.driver, '_collect_mac_addresses')
 
@@ -524,7 +527,7 @@ def test_deactivate_bootloader_for_nonexistent_instance(self):
         bm_utils.unlink_without_raise(pxe_path)
         self.driver._collect_mac_addresses(self.context, self.node).\
                 AndRaise(exception.DBError)
-        bm_utils.unlink_without_raise(
+        bm_utils.rmtree_without_raise(
                 os.path.join(CONF.baremetal.tftp_root, 'fake-uuid'))
         self.mox.ReplayAll()
 

nova/virt/baremetal/pxe.py

@@ -21,7 +21,6 @@
 """
 
 import os
-import shutil
 
 from nova.compute import instance_types
 from nova import exception
@@ -344,7 +343,7 @@ def cache_images(self, context, node, instance,
     def destroy_images(self, context, node, instance):
         """Delete instance's image file."""
         bm_utils.unlink_without_raise(get_image_file_path(instance))
-        bm_utils.unlink_without_raise(get_image_dir_path(instance))
+        bm_utils.rmtree_without_raise(get_image_dir_path(instance))
 
     def activate_bootloader(self, context, node, instance):
         """Configure PXE boot loader for an instance
@@ -419,7 +418,7 @@ def deactivate_bootloader(self, context, node, instance):
             for mac in macs:
                 bm_utils.unlink_without_raise(get_pxe_mac_path(mac))
 
-        bm_utils.unlink_without_raise(
+        bm_utils.rmtree_without_raise(
                 os.path.join(CONF.baremetal.tftp_root, instance['uuid']))
 
     def activate_node(self, context, node, instance):

nova/virt/baremetal/utils.py

@@ -16,6 +16,7 @@
 #    under the License.
 
 import os
+import shutil
 
 from nova.openstack.common import log as logging
 from nova.virt.disk import api as disk_api
@@ -47,6 +48,14 @@ def unlink_without_raise(path):
         LOG.exception(_("Failed to unlink %s") % path)
 
 
+def rmtree_without_raise(path):
+    try:
+        if os.path.isdir(path):
+            shutil.rmtree(path)
+    except OSError, e:
+        LOG.warn(_("Failed to remove dir %(path)s, error: %(e)s") % locals())
+
+
 def write_to_file(path, contents):
     with open(path, 'w') as f:
         f.write(contents)