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

3 files changed, 19 insertions, 8 deletions

diff --git a/nova/tests/baremetal/test_pxe.py b/nova/tests/baremetal/test_pxe.py
index 73ef8caa3..6d0462fa4 100644
--- a/nova/tests/baremetal/test_pxe.py
+++ b/nova/tests/baremetal/test_pxe.py
@@ -414,10 +414,11 @@ class PXEPublicMethodsTestCase(BareMetalPXETestCase):
 
     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 @@ class PXEPublicMethodsTestCase(BareMetalPXETestCase):
         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 @@ class PXEPublicMethodsTestCase(BareMetalPXETestCase):
                 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 @@ class PXEPublicMethodsTestCase(BareMetalPXETestCase):
         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 @@ class PXEPublicMethodsTestCase(BareMetalPXETestCase):
         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()
 
diff --git a/nova/virt/baremetal/pxe.py b/nova/virt/baremetal/pxe.py
index 0daac1d46..4322439cf 100644
--- a/nova/virt/baremetal/pxe.py
+++ b/nova/virt/baremetal/pxe.py
@@ -21,7 +21,6 @@ Class for PXE bare-metal nodes.
 """
 
 import os
-import shutil
 
 from nova.compute import instance_types
 from nova import exception
@@ -344,7 +343,7 @@ class PXE(base.NodeDriver):
     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 @@ class PXE(base.NodeDriver):
             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):
diff --git a/nova/virt/baremetal/utils.py b/nova/virt/baremetal/utils.py
index 0842ae201..efc5199db 100644
--- a/nova/virt/baremetal/utils.py
+++ b/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)