Tidy filehandles and files in case of exceptions

Signed-off-by: Robert Breker <[email protected]>

Author: Robert Breker

src/xscontainer/api_helper.py

@@ -253,12 +253,14 @@ def get_this_host_uuid():
     # ToDo: There must be a better way that also works with plugins?!?
     uuid = None
     filehandler = open("/etc/xensource-inventory", 'r')
-    for line in filehandler.readlines():
-        if line.startswith("INSTALLATION_UUID"):
-            uuid = line.split("'")[1]
-            filehandler.close()
-            return uuid
-
+    try:
+        for line in filehandler.readlines():
+            if line.startswith("INSTALLATION_UUID"):
+                uuid = line.split("'")[1]
+                break
+    finally:
+        filehandler.close()
+    return uuid
 
 def get_this_host_ref(session):
     host_uuid = get_this_host_uuid()

src/xscontainer/coreos.py

@@ -35,11 +35,16 @@ def install_vm(session, urlvhdbz2, sruuid,
     # @todo: pipe instead, so the file never actually touches Dom0
     cmd = ['curl', '-o', atempfile, urlvhdbz2]
     util.runlocal(cmd)
-    cmd = ['bzip2', '-d', atempfile]
-    util.runlocal(cmd)
-    vdiref = api_helper.import_disk(session, sruuid, atempfileunpacked, 'vhd',
-                                    'Disk')
-    os.remove(atempfileunpacked)
+    try:
+        cmd = ['bzip2', '-d', atempfile]
+        util.runlocal(cmd)
+        vdiref = api_helper.import_disk(session, sruuid, atempfileunpacked,
+                                        'vhd', 'Disk')
+    finally:
+        if os.path.exists(atempfile):
+            os.remove(atempfile)
+        if os.path.exists(atempfileunpacked):
+            os.remove(atempfileunpacked)
     templateref = session.xenapi.VM.get_by_name_label(templatename)[0]
     vmref = session.xenapi.VM.clone(templateref, vmname)
     vmuuid = session.xenapi.VM.get_record(vmref)['uuid']
@@ -113,9 +118,11 @@ def load_cloud_config_template(template_path=None):
 
     log.info("load_cloud_config_template from %s" % (template_path))
 
-    fh = open(template_path)
-    template_data = fh.read()
-    fh.close()
+    filehandle = open(template_path)
+    try:
+        template_data = filehandle.read()
+    finally:
+        filehandle.close()
 
     # Append template location to make it clear where it was loaded from.
     template_data = ("%s\n\n# Template loaded from %s"
@@ -130,55 +137,71 @@ def get_config_drive_default(session):
         userdata = filterxshinexists(userdata)
     return userdata
 
-
-def create_config_drive_iso(session, userdata, vmuuid):
-    log.info("create_config_drive_iso for vm %s" %(vmuuid))
-    tempisodir = tempfile.mkdtemp()
-    tempisofile = tempfile.mkstemp()[1]
-    openstackfolder = os.path.join(tempisodir, 'openstack')
-    latestfolder = os.path.join(openstackfolder, 'latest')
-    os.makedirs(latestfolder)
-    userdatafile = os.path.join(latestfolder, 'user_data')
-    userdata = customize_userdata(session, userdata, vmuuid)
-    util.write_file(userdatafile, userdata)
-    log.debug("Userdata: %s" % (userdata))
-    # Also add the Linux guest agent
-    temptoolsisodir = tempfile.mkdtemp()
-    # First find the latest tools ISO
+def find_latest_tools_iso_path():
     tools_iso_paths = glob.glob(XS_TOOLS_ISO_PATH)
     if len(tools_iso_paths) < 1:
         raise util.XSContainerException("Can't locate XS tools in %s."
                                         % (XS_TOOLS_ISO_PATH))
     tools_iso_paths.sort(key=distutils.version.LooseVersion)
     tools_iso_path = tools_iso_paths[-1]
-    # Then copy it
-    cmd = ['mount', '-o', 'loop',
-           tools_iso_path,  temptoolsisodir]
-    util.runlocal(cmd)
-    agentpath = os.path.join(tempisodir, 'agent')
-    os.makedirs(agentpath)
-    agentfiles = ['xe-daemon', 'xe-linux-distribution',
-                  'xe-linux-distribution.service', 'xe-update-guest-attrs',
-                  'xen-vcpu-hotplug.rules', 'install.sh',
-                  'versions.deb', 'versions.rpm']
-    for filename in agentfiles:
-        path = os.path.join(temptoolsisodir, 'Linux', filename)
-        shutil.copy(path, agentpath)
-    cmd = ['umount', temptoolsisodir]
-    util.runlocal(cmd)
-    os.rmdir(temptoolsisodir)
-    # Finally wrap up the iso
-    cmd = ['mkisofs', '-R', '-V', 'config-2', '-o', tempisofile, tempisodir]
-    util.runlocal(cmd)
-    # Tidy
-    os.remove(userdatafile)
-    os.rmdir(latestfolder)
-    os.rmdir(openstackfolder)
-    for filename in agentfiles:
-        path = os.path.join(agentpath, filename)
-        os.remove(path)
-    os.rmdir(agentpath)
-    os.rmdir(tempisodir)
+    return tools_iso_path
+
+def create_config_drive_iso(session, userdata, vmuuid):
+    log.info("create_config_drive_iso for vm %s" %(vmuuid))
+    umountrequired = False
+    temptoolsisodir = None
+    userdatafile = None
+    latestfolder = None
+    openstackfolder = None
+    agentfilepaths = []
+    agentpath = None
+    tempisodir = None
+    try:
+        tempisodir = tempfile.mkdtemp()
+        tempisofile = tempfile.mkstemp()[1]
+        # add the userdata-file
+        openstackfolder = os.path.join(tempisodir, 'openstack')
+        latestfolder = os.path.join(openstackfolder, 'latest')
+        os.makedirs(latestfolder)
+        userdatafile = os.path.join(latestfolder, 'user_data')
+        userdata = customize_userdata(session, userdata, vmuuid)
+        util.write_file(userdatafile, userdata)
+        log.debug("Userdata: %s" % (userdata))
+        # Also add the Linux guest agent
+        temptoolsisodir = tempfile.mkdtemp()
+        tools_iso_path = find_latest_tools_iso_path()
+        cmd = ['mount', '-o', 'loop',
+               tools_iso_path,  temptoolsisodir]
+        util.runlocal(cmd)
+        umountrequired = True
+        agentpath = os.path.join(tempisodir, 'agent')
+        os.makedirs(agentpath)
+        agentfiles = ['xe-daemon', 'xe-linux-distribution',
+                      'xe-linux-distribution.service', 'xe-update-guest-attrs',
+                      'xen-vcpu-hotplug.rules', 'install.sh',
+                      'versions.deb', 'versions.rpm']
+        for filename in agentfiles:
+            path = os.path.join(temptoolsisodir, 'Linux', filename)
+            shutil.copy(path, agentpath)
+            agentfilepaths.append(os.path.join(agentpath, filename))
+        # Finally wrap up the iso
+        cmd = ['mkisofs', '-R', '-V', 'config-2', '-o', tempisofile, tempisodir]
+        util.runlocal(cmd)
+    finally:
+        # And tidy
+        if umountrequired:
+            cmd = ['umount', temptoolsisodir]
+            util.runlocal(cmd)
+        for path in [temptoolsisodir, userdatafile, latestfolder,
+                     openstackfolder]+agentfilepaths+[agentpath, tempisodir]:
+            if path != None:
+                if os.path.isdir(path):
+                    os.rmdir(path)
+                elif os.path.isfile(path):
+                    os.remove(path)
+                else:
+                    log.debug("create_config_drive_iso: Not tidying %s because"
+                              " it could not be found" % (path))
     return tempisofile
 
 
@@ -203,12 +226,14 @@ def create_config_drive(session, vmuuid, sruuid, userdata):
     vmrecord = session.xenapi.VM.get_record(vmref)
     prepare_vm_for_config_drive(session, vmref, vmuuid)
     isofile = create_config_drive_iso(session, userdata, vmuuid)
-    configdisk_namelabel = 'Automatic Config Drive'
-    other_config_keys = {OTHER_CONFIG_CONFIG_DRIVE_KEY: 'True'}
-    vdiref = api_helper.import_disk(session, sruuid, isofile, 'raw',
-                                    configdisk_namelabel,
-                                    other_config_keys=other_config_keys)
-    os.remove(isofile)
+    try:
+        configdisk_namelabel = 'Automatic Config Drive'
+        other_config_keys = {OTHER_CONFIG_CONFIG_DRIVE_KEY: 'True'}
+        vdiref = api_helper.import_disk(session, sruuid, isofile, 'raw',
+                                        configdisk_namelabel,
+                                        other_config_keys=other_config_keys)
+    finally:
+        os.remove(isofile)
     remove_config_drive(session, vmrecord, configdisk_namelabel)
     vbdref = api_helper.create_vbd(session, vmref, vdiref, 'ro', False)
     if vmrecord['power_state'] == 'Running':
@@ -223,14 +248,21 @@ def create_config_drive(session, vmuuid, sruuid, userdata):
 
 def get_config_drive_configuration(session, vdiuuid):
     log.info("get_config_drive_configuration from vdi %s" % (vdiuuid))
+    tempdir = None
+    umountrequired = False
     filename = api_helper.export_disk(session, vdiuuid)
-    tempdir = tempfile.mkdtemp()
-    cmd = ['mount', '-o', 'loop', '-t', 'iso9660', filename, tempdir]
-    util.runlocal(cmd)
-    userdatapath = os.path.join(tempdir, 'openstack', 'latest', 'user_data')
-    content = util.read_file(userdatapath)
-    cmd = ['umount', tempdir]
-    util.runlocal(cmd)
-    os.rmdir(tempdir)
-    os.remove(filename)
+    try:
+        tempdir = tempfile.mkdtemp()
+        cmd = ['mount', '-o', 'loop', '-t', 'iso9660', filename, tempdir]
+        util.runlocal(cmd)
+        umountrequired = True
+        userdatapath = os.path.join(tempdir, 'openstack', 'latest', 'user_data')
+        content = util.read_file(userdatapath)
+    finally:
+        os.remove(filename)
+        if umountrequired:
+            cmd = ['umount', tempdir]
+            util.runlocal(cmd)
+        if tempdir:
+            os.rmdir(tempdir)
     return content