From e03dcb10680d5900a1a5219ee385efecab2c7a81 Mon Sep 17 00:00:00 2001 From: Ben Steffen Date: Sun, 24 Nov 2019 21:54:32 -0500 Subject: [PATCH] Fully switch over to Python 3 Use #!/usr/bin/env python3 shebang Remove verbosity argument and just use exceptions to display error conditions Use subprocess.run instead of Popen to throw exceptions upon command failure Use modern string formatting instead of % Clean up formatting according to what pylint recommends --- debian/changelog | 6 ++ debian/control | 3 +- invirt-images | 240 ++++++++++++++++++------------------------------------ 3 files changed, 87 insertions(+), 162 deletions(-) diff --git a/debian/changelog b/debian/changelog index 1dd7308..8cc6288 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +invirt-images (0.0.7) precise; urgency=medium + + * Port to Python 3 + + -- Ben Steffen Sun, 24 Nov 2019 22:03:30 -0500 + invirt-images (0.0.6) precise; urgency=low * Fixing images for new lv setup to deactivate volumes before modifying. diff --git a/debian/control b/debian/control index 9c6a9d9..5f7dea1 100644 --- a/debian/control +++ b/debian/control @@ -4,10 +4,11 @@ Priority: extra Maintainer: Invirt project Build-Depends: debhelper (>= 7) Standards-Version: 3.9.3 +X-Python3-Version: >= 3.6 Package: invirt-images Architecture: all -Depends: ${shlibs:Depends}, ${misc:Depends}, python, invirt-database +Depends: ${shlibs:Depends}, ${misc:Depends}, ${python3:Depends}, invirt-database Description: Invirt's tools for managing disk images for VMs These install the tools for managing disk images for Invirt VMs. Images can share common mirrors. The tools include mechanisms diff --git a/invirt-images b/invirt-images index 93e4f6e..eeda870 100755 --- a/invirt-images +++ b/invirt-images @@ -1,164 +1,90 @@ -#!/usr/bin/python - -from __future__ import print_function -from future import standard_library -standard_library.install_aliases() -from builtins import map -from builtins import zip -from builtins import range -from invirt import database +#!/usr/bin/env python3 + import os -import sys import subprocess import random import string import tempfile -import urllib.request, urllib.parse, urllib.error +import urllib.request import math -import optparse as op - -class InvirtImageException(Exception): - pass +import optparse -# verbosity = 0 means no output from the actual commands -# verbosity = 1 means only errors from the actual commands -# verbosity = 2 means all output from the actual commands -verbosity = 0 +from invirt import database -def getOutput(): - global verbosity - return { - 'stdout': subprocess.PIPE if verbosity < 2 else None, - 'stderr': subprocess.PIPE if verbosity < 1 else None - } def lvcreate(name, size): - lvc = subprocess.Popen(['lvcreate', '-L', size, '-n', name, 'xenvg'], - stderr=subprocess.PIPE, - stdout=getOutput()['stdout']) - if not lvc.wait(): - return 0 - stderr = lvc.stderr.read() - if 'already exists in volume group' in stderr: - return 5 - else: - if verbosity > 0: - print(stderr) - return 6 + subprocess.run(['lvcreate', '-L', size, '-n', name, 'xenvg'], + check_output=True, encoding='utf-8', check=True) def lvrename(dest, src): - lvr = subprocess.Popen(['lvchange', '-an', "xenvg/%s" % (src,)], - stderr=subprocess.PIPE, - stdout=getOutput()['stdout']) - ret = lvr.wait() - - if ret: - if verbosity > 0: - print(lvr.stderr.read()) - return ret - - lvr = subprocess.Popen(['lvrename', "xenvg/%s" % (src,), "xenvg/%s" % (dest,)], - stderr=subprocess.PIPE, - stdout=getOutput()['stdout']) - ret = lvr.wait() + subprocess.run(['lvchange', '-an', f'xenvg/{src}'], + check_output=True, encoding='utf-8', check=True) - if ret: - stderr = lvr.stderr.read() - if not ('not found in volume group' in stderr): - if verbosity > 0: - print(stderr) - return ret - - subprocess.Popen(['lvchange', '-ay', "xenvg/%s" % (dest,)], - stderr=subprocess.PIPE, - stdout=getOutput()['stdout']) - ret = lvr.wait() - - if ret: - if verbosity > 0: - print(lvr.stderr.read()) - - return ret + subprocess.run(['lvrename', f'xenvg/{src}', f'xenvg/{dest}'], + check_output=True, encoding='utf-8', check=True) + subprocess.run(['lvchange', '-ay', f'xenvg/{dest}'], + check_output=True, encoding='utf-8', check=True) def lv_random(func, pattern, *args): """ - Run some LVM-related command, optionally with a random string in - the LV name. - + Run some LVM-related command with a random string in the LV name. + func takes an LV name plus whatever's in *args and returns the return code of some LVM command, such as lvcreate or lvrename - - pattern can contain at most one '%s' pattern, which will be - replaced by a 6-character random string. - - If pattern contains a '%s', the script will attempt to re-run - itself if the error code indicates that the destination already - exists + + pattern must contain one '{}' pattern, which will be replaced + by a 6-character random string. + + the script will attempt to re-run itself if the error code + indicates that the destination already exists """ + # Keep trying until it works while True: - rand_string = ''.join(random.choice(string.ascii_letters) \ - for i in range(6)) - if '%s' in pattern: - name = pattern % rand_string - else: - name = pattern - ret = func(name, *args) - if ret == 0: - return name - # 5 is the return code if the destination already exists - elif '%s' not in pattern or ret != 5: - raise InvirtImageException('E: Error running %s with args %s' % (func.__name__, args)) + letters = (random.choice(string.ascii_letters) for _ in range(6)) + rand_string = ''.join(letters) -def lvcreate_random(pattern, size): - """ - Creates an LV, optionally with a random string in the name. - - Call with a string formatting pattern with a single '%s' to use as - a pattern for the name of the new LV. - """ - return lv_random(lvcreate, pattern, size) + name = pattern.format(rand_string) -def lvrename_random(src, pattern): - """ - Rename an LV to a new name with a random string incorporated. - - Call with a string formatting pattern with a single '%s' to use as - a pattern for the name of the new LV - """ - return lv_random(lvrename, pattern, src) + try: + func(name, *args) + except subprocess.CalledProcessError as e: + # 5 is the return code if the destination already exists + if e.returncode != 5: + raise + else: + return name def fetch_image(cdrom): """ Download a cdrom from a URI, shelling out to rsync if appropriate and otherwise trying to use urllib """ + full_uri = os.path.join(cdrom.mirror.uri_prefix, cdrom.uri_suffix) temp_file = tempfile.mkstemp()[1] - if verbosity > 0: - print("Fetching image %s from %s to %s" % (cdrom.cdrom_id, full_uri, temp_file), file=sys.stderr) + print(f'Fetching image {cdrom.cdrom_id} from {full_uri} to {temp_file}') try: if full_uri.startswith('rsync://'): - if subprocess.call(['rsync', '--no-motd', '-tLP', full_uri, temp_file], - **getOutput()): - raise InvirtImageException("E: Unable to download '%s'" % full_uri) + subprocess.run(['rsync', '--no-motd', '-tLP', full_uri, temp_file], + check_output=True, encoding='utf-8', check=True) else: # I'm not going to look for errors here, because I bet it'll # throw its own exceptions urllib.request.urlretrieve(full_uri, temp_file) return temp_file except: - os.unlink(temp_file) + os.remove(temp_file) raise def copy_file(src, dest): """ Copy a file from one location to another using dd """ - if subprocess.call(['dd', 'if=%s' % src, 'of=%s' % dest, 'bs=1M'], - **getOutput()): - raise InvirtImageException('E: Unable to transfer %s into %s' % (src, dest)) + + subprocess.run(['dd', f'if={src}', f'of={dest}', 'bs=1M'], + check_output=True, encoding='utf-8', check=True) def load_image(cdrom): """ @@ -166,56 +92,49 @@ def load_image(cdrom): transferring it into an LV, moving the old LV out of the way and the new LV into place """ + if cdrom.mirror_id is None: return - try: - temp_file = fetch_image(cdrom) - except InvirtImageException as e: - print('ERROR: %s. Skipping.' % e, file=sys.stderr) - return + + temp_file = fetch_image(cdrom) + + st_size = os.stat(temp_file).st_size + + assert st_size > 0, 'CD-ROM image size is 0' + + megabytes = math.ceil((float(st_size) / (1024 * 1024))) + cdrom_size = f'{megabytes}M' try: - st_size = os.stat(temp_file).st_size - if not st_size: - print("Failed to fetch %s" % cdrom.cdrom_id, file=sys.stderr) - return - cdrom_size = '%sM' % math.ceil((float(st_size) / (1024 * 1024))) - new_lv = lvcreate_random('image-new_%s_%%s' % cdrom.cdrom_id, cdrom_size) - copy_file(temp_file, '/dev/xenvg/%s' % new_lv) - lvrename_random('image_%s' % cdrom.cdrom_id, 'image-old_%s_%%s' % cdrom.cdrom_id) - lvrename_random(new_lv, 'image_%s' % cdrom.cdrom_id) + new_lv = lv_random(lvcreate, f'image-new_{cdrom.cdrom_id}' + '_{}', cdrom_size) + copy_file(temp_file, f'/dev/xenvg/{new_lv}') + lv_random(lvrename, f'image-old_{cdrom.cdrom_id}' + '_{}', 'image_{cdrom.cdrom_id}') + lv_random(lvrename, f'image_{cdrom.cdrom_id}', new_lv) reap_images() finally: - os.unlink(temp_file) + os.remove(temp_file) def reap_images(): """ Remove stale cdrom images that are no longer in use - + load_image doesn't attempt to remove the old image because it might still be in use. reap_images attempts to delete any LVs starting with 'image-old_', but ignores errors, in case they're still being used. """ - lvm_list = subprocess.Popen(['lvs', '-o', 'lv_name', '--noheadings'], - stdout=subprocess.PIPE, - stdin=subprocess.PIPE) - lvm_list.wait() - - for lv in map(str.strip, lvm_list.stdout.read().splitlines()): + + lvm_list = subprocess.run(['lvs', '-o', 'lv_name', '--noheadings'], + check_output=True, encoding='utf-8', check=True) + + for lv in (s.strip() for s in lvm_list.stdout.readlines()): if lv.startswith('image-old_'): - subprocess.call(['lvchange', '-a', 'n', '/dev/xenvg/%s' % lv], - **getOutput()) - subprocess.call(['lvchange', '-a', 'n', '/dev/xenvg/%s' % lv], - **getOutput()) - subprocess.call(['lvchange', '-a', 'ey', '/dev/xenvg/%s' % lv], - **getOutput()) - subprocess.call(['lvremove', '--force', '/dev/xenvg/%s' % lv], - **getOutput()) + subprocess.run(['lvchange', '-a', 'n', f'/dev/xenvg/{lv}']) + subprocess.run(['lvchange', '-a', 'n', f'/dev/xenvg/{lv}']) + subprocess.run(['lvchange', '-a', 'ey', f'/dev/xenvg/{lv}']) + subprocess.run(['lvremove', '--force', f'/dev/xenvg/{lv}']) def main(): - global verbosity - database.connect() database.session.begin() @@ -224,23 +143,23 @@ def main(): %prog [options] --update [short_name1 [short_name2 ...]] %prog [options] --reap""" - - parser = op.OptionParser(usage=usage) + + parser = optparse.OptionParser(usage=usage) parser.set_defaults(verbosity=0, item='cdrom') - + parser.add_option('-a', '--add', action='store_const', dest='action', const='add', help='Add a new item to the database') - + parser.add_option('-u', '--update', action='store_const', dest='action', const='update', help='Update all cdrom images in the database with the latest version') parser.add_option('-r', '--reap', action='store_const', dest='action', const='reap', help='Reap stale cdrom images that are no longer in use') - - a_group = op.OptionGroup(parser, 'Adding new items') + + a_group = optparse.OptionGroup(parser, 'Adding new items') a_group.add_option('-c', '--cdrom', action='store_const', dest='item', const='cdrom', help='Add a new cdrom to the database') @@ -248,8 +167,8 @@ def main(): dest='item', const='mirror', help='Add a new mirror to the database') parser.add_option_group(a_group) - - v_group = op.OptionGroup(parser, "Verbosity levels") + + v_group = optparse.OptionGroup(parser, "Verbosity levels") v_group.add_option("-q", "--quiet", action='store_const', dest='verbosity', const=0, help='Show no output from commands this script runs (default)') @@ -260,24 +179,23 @@ def main(): dest='verbosity', const=2, help='Show all output from commands this script runs') parser.add_option_group(v_group) - + (options, args) = parser.parse_args() - verbosity = options.verbosity if options.action is None: print(parser.format_help()) elif options.action == 'add': if options.item == 'cdrom': attrs = dict(list(zip(('cdrom_id', 'description', 'mirror_id', 'uri_suffix'), - args))) + args))) cdrom = database.CDROM(**attrs) database.session.add(cdrom) database.session.commit() - + load_image(cdrom) - + elif options.item == 'mirror': attrs = dict(list(zip(('mirror_id', 'uri_prefix'), - args))) + args))) mirror = database.Mirror(**attrs) database.session.add(mirror) database.session.commit() -- 1.7.9.5