cosalib: further consolidate run() functions

I then found run_verbose in cmdlib.py that is already in wide
use. Let's consolidate it with runcmd from src/cosalib/utils.py
and delete src/cosalib/utils.py.

I also deleted bespoke `run_verbose()` function definitions and
uses from other files throughout the codebase.

Author: dustymabe

src/cmd-buildextend-extensions

@@ -88,7 +88,7 @@ def run_rpmostree(workdir, commit, treefile, extensions):
 
 
 def create_yumrepo(repodir):
-    cmdlib.run_verbose(['createrepo_c', repodir])
+    cmdlib.runcmd_verbose(['createrepo_c', repodir])
     # we could also have rpm-ostree output the pkglist for us, but meh... we
     # need to run createrepo_c anyway and it's nice that we're using it as the
     # source of truth, since that's what rpm-ostree clients will also use
@@ -116,7 +116,7 @@ def create_tarball(buildmeta, srcdir, destdir):
     destdir = os.path.abspath(destdir)
     basearch = buildmeta['coreos-assembler.basearch']
     tarfile = f'{destdir}/{buildmeta["name"]}-{buildmeta["buildid"]}-extensions.{basearch}.tar'
-    cmdlib.run_verbose(['tar', '-cf', tarfile, '.'], cwd=srcdir)
+    cmdlib.runcmd_verbose(['tar', '-cf', tarfile, '.'], cwd=srcdir)
     return tarfile
 
 

src/cmd-buildextend-live

@@ -18,7 +18,7 @@ import time
 
 sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
 from cosalib.builds import Builds
-from cosalib.cmdlib import run_verbose, sha256sum_file
+from cosalib.cmdlib import runcmd_verbose, sha256sum_file
 from cosalib.cmdlib import import_ostree_commit, get_basearch, ensure_glob
 from cosalib.meta import GenericBuildMeta
 
@@ -137,18 +137,18 @@ def align_initrd_for_uncompressed_append(destf):
 def get_os_features():
     features = {}
 
-    f = run_verbose(['/usr/bin/ostree', 'cat', '--repo', repo,
-            buildmeta_commit, '/usr/libexec/coreos-installer-service'],
-            capture_output=True).stdout.decode()
+    f = runcmd_verbose(['/usr/bin/ostree', 'cat', '--repo', repo,
+                       buildmeta_commit, '/usr/libexec/coreos-installer-service'],
+                       capture_output=True).stdout.decode()
     # eventually we can assume coreos-installer >= 0.12.0, delete this check,
     # and hardcode the feature flag
     if '--config-file' in f:
         features['installer-config'] = True
 
-    f = run_verbose(['/usr/bin/ostree', 'cat', '--repo', repo,
-            buildmeta_commit,
-            '/usr/lib/dracut/modules.d/35coreos-network/coreos-copy-firstboot-network.sh'],
-            capture_output=True).stdout.decode()
+    f = runcmd_verbose(['/usr/bin/ostree', 'cat', '--repo', repo,
+                       buildmeta_commit,
+                       '/usr/lib/dracut/modules.d/35coreos-network/coreos-copy-firstboot-network.sh'],
+                       capture_output=True).stdout.decode()
     # eventually we can assume /etc/coreos-firstboot-network support and
     # hardcode the feature flag
     if 'etc_firstboot_network_dir' in f:
@@ -239,17 +239,17 @@ def generate_iso():
     # Find the directory under `/usr/lib/modules/<kver>` where the
     # kernel/initrd live. It will be the 2nd entity output by
     # `ostree ls <commit> /usr/lib/modules`
-    process = run_verbose(['/usr/bin/ostree', 'ls', '--repo', repo,
-                           '--nul-filenames-only', f"{buildmeta_commit}",
-                           '/usr/lib/modules'], capture_output=True)
+    process = runcmd_verbose(['/usr/bin/ostree', 'ls', '--repo', repo,
+                             '--nul-filenames-only', f"{buildmeta_commit}",
+                             '/usr/lib/modules'], capture_output=True)
     moduledir = process.stdout.decode().split('\0')[1]
 
     # copy those files out of the ostree into the iso root dir
     initramfs_img = 'initramfs.img'
     for file in [kernel_img, initramfs_img]:
-        run_verbose(['/usr/bin/ostree', 'checkout', '--force-copy', '--repo', repo,
-                     '--user-mode', '--subpath', os.path.join(moduledir, file),
-                     f"{buildmeta_commit}", tmpisoimagespxe])
+        runcmd_verbose(['/usr/bin/ostree', 'checkout', '--force-copy', '--repo', repo,
+                       '--user-mode', '--subpath', os.path.join(moduledir, file),
+                       f"{buildmeta_commit}", tmpisoimagespxe])
         # initramfs isn't world readable by default so let's open up perms
         os.chmod(os.path.join(tmpisoimagespxe, file), 0o644)
         if file == initramfs_img:
@@ -297,23 +297,23 @@ def generate_iso():
     # Add osmet files
     tmp_osmet = os.path.join(tmpinitrd_rootfs, img_metal_obj['path'] + '.osmet')
     print('Generating osmet file for 512b metal image')
-    run_verbose(['/usr/lib/coreos-assembler/osmet-pack',
-                 img_metal, '512', tmp_osmet, img_metal_checksum,
-                 'fast' if args.fast else 'normal'])
+    runcmd_verbose(['/usr/lib/coreos-assembler/osmet-pack',
+                   img_metal, '512', tmp_osmet, img_metal_checksum,
+                   'fast' if args.fast else 'normal'])
     if img_metal4k_obj:
         tmp_osmet4k = os.path.join(tmpinitrd_rootfs, img_metal4k_obj['path'] + '.osmet')
         print('Generating osmet file for 4k metal image')
-        run_verbose(['/usr/lib/coreos-assembler/osmet-pack',
-                     img_metal4k, '4096', tmp_osmet4k, img_metal4k_checksum,
-                     'fast' if args.fast else 'normal'])
+        runcmd_verbose(['/usr/lib/coreos-assembler/osmet-pack',
+                       img_metal4k, '4096', tmp_osmet4k, img_metal4k_checksum,
+                       'fast' if args.fast else 'normal'])
 
     # Generate root squashfs
     print(f'Compressing squashfs with {squashfs_compression}')
     # Name must be exactly "root.squashfs" because the 20live dracut module
     # makes assumptions about the length of the name in sysroot.mount
     tmp_squashfs = os.path.join(tmpinitrd_rootfs, 'root.squashfs')
-    run_verbose(['/usr/lib/coreos-assembler/gf-mksquashfs',
-                img_metal, tmp_squashfs, squashfs_compression])
+    runcmd_verbose(['/usr/lib/coreos-assembler/gf-mksquashfs',
+                   img_metal, tmp_squashfs, squashfs_compression])
 
     # Generate rootfs image
     iso_rootfs = os.path.join(tmpisoimagespxe, rootfs_img)
@@ -340,8 +340,8 @@ def generate_iso():
     cp_reflink(iso_initramfs, pxe_initramfs)
 
     # Read and filter kernel arguments for substituting into ISO bootloader
-    result = run_verbose(['/usr/lib/coreos-assembler/gf-get-kargs',
-            img_metal], stdout=subprocess.PIPE, text=True)
+    result = runcmd_verbose(['/usr/lib/coreos-assembler/gf-get-kargs',
+                            img_metal], stdout=subprocess.PIPE, text=True)
     kargs_array = [karg for karg in result.stdout.split()
             if karg.split('=')[0] not in live_exclude_kargs]
     kargs_array.append(f"coreos.liveiso={volid}")
@@ -445,7 +445,7 @@ def generate_iso():
 
         # safely remove things we don't need in the final ISO tree
         for d in ['EFI', 'isolinux', 'zipl.prm']:
-            run_verbose(['rm', '-rf', os.path.join(tmpisoroot, d)])
+            runcmd_verbose(['rm', '-rf', os.path.join(tmpisoroot, d)])
 
         # grub2-mkrescue is a wrapper around xorriso
         genisoargs = ['grub2-mkrescue', '-volid', volid]
@@ -472,11 +472,11 @@ def generate_iso():
         kernel_img = 'kernel.img'
 
         # combine kernel, initramfs and cmdline using the mk-s390image tool
-        run_verbose(['/usr/bin/mk-s390image',
-                     kernel_dest,
-                     os.path.join(tmpisoimages, 'cdboot.img'),
-                     '-r', iso_initramfs,
-                     '-p', os.path.join(tmpisoimages, 'cdboot.prm')])
+        runcmd_verbose(['/usr/bin/mk-s390image',
+                       kernel_dest,
+                       os.path.join(tmpisoimages, 'cdboot.img'),
+                       '-r', iso_initramfs,
+                       '-p', os.path.join(tmpisoimages, 'cdboot.prm')])
         # generate .addrsize file for LPAR
         with open(os.path.join(tmpisoimages, 'initrd.addrsize'), 'wb') as addrsize:
             addrsize_data = struct.pack(">iiii", 0, int(INITRD_ADDRESS, 16), 0,
@@ -485,7 +485,7 @@ def generate_iso():
 
         # safely remove things we don't need in the final ISO tree
         for d in ['EFI', 'isolinux', 'zipl.prm']:
-            run_verbose(['rm', '-rf', os.path.join(tmpisoroot, d)])
+            runcmd_verbose(['rm', '-rf', os.path.join(tmpisoroot, d)])
 
         genisoargs = ['/usr/bin/xorrisofs', '-verbose',
                       '-volid', volid,
@@ -514,10 +514,10 @@ def generate_iso():
                 return tarinfo
 
             tmpimageefidir = os.path.join(tmpdir, "efi")
-            run_verbose(['/usr/bin/ostree', 'checkout', '--repo', repo,
-                         '--user-mode', '--subpath',
-                         "/usr/lib/bootupd/updates/EFI",
-                         buildmeta_commit, tmpimageefidir])
+            runcmd_verbose(['/usr/bin/ostree', 'checkout', '--repo', repo,
+                           '--user-mode', '--subpath',
+                           "/usr/lib/bootupd/updates/EFI",
+                           buildmeta_commit, tmpimageefidir])
 
             # Find name of vendor directory
             vendor_ids = [n for n in os.listdir(tmpimageefidir) if n != "BOOT"]
@@ -586,8 +586,8 @@ boot
             # so set EFI-SYSTEM for consistency with the metal image.
             # This should not be needed on Fedora or RHEL 9, but seems like
             # a good thing to do anyway.
-            run_verbose(['virt-make-fs', '--type=vfat', '--label=EFI-SYSTEM',
-                         efitarfile.name, efibootfile])
+            runcmd_verbose(['virt-make-fs', '--type=vfat', '--label=EFI-SYSTEM',
+                           efitarfile.name, efibootfile])
 
         genisoargs += ['-eltorito-alt-boot',
                        '-efi-boot', 'images/efiboot.img',
@@ -634,12 +634,12 @@ boot
         for f in ensure_glob(os.path.join(tmpisoisolinux, '*.msg')):
             os.unlink(f)
 
-    run_verbose(genisoargs_final)
+    runcmd_verbose(genisoargs_final)
 
     # Add MBR, and GPT with ESP, for x86_64 BIOS/UEFI boot when ISO is
     # copied to a USB stick
     if basearch == "x86_64":
-        run_verbose(['/usr/bin/isohybrid', '--uefi', tmpisofile])
+        runcmd_verbose(['/usr/bin/isohybrid', '--uefi', tmpisofile])
 
     genisoargs_minimal = genisoargs + ['-o', f'{tmpisofile}.minimal', tmpisoroot]
     # The only difference with the miniso is that we drop these two files.
@@ -649,12 +649,12 @@ boot
     # coreos-installer takes care of removing it.
     os.unlink(iso_rootfs)
     os.unlink(miniso_data)
-    run_verbose(genisoargs_minimal)
+    runcmd_verbose(genisoargs_minimal)
     if basearch == "x86_64":
-        run_verbose(['/usr/bin/isohybrid', '--uefi', f'{tmpisofile}.minimal'])
+        runcmd_verbose(['/usr/bin/isohybrid', '--uefi', f'{tmpisofile}.minimal'])
     # this consumes the minimal image
-    run_verbose(['coreos-installer', 'pack', 'minimal-iso',
-                 tmpisofile, f'{tmpisofile}.minimal', "--consume"])
+    runcmd_verbose(['coreos-installer', 'pack', 'minimal-iso',
+                   tmpisofile, f'{tmpisofile}.minimal', "--consume"])
 
     buildmeta['images'].update({
         'live-iso': {

src/cmd-compress

@@ -14,7 +14,7 @@ from cosalib.builds import Builds
 from cosalib.cmdlib import (
     ncpu,
     rm_allow_noent,
-    run_verbose,
+    runcmd_verbose,
     sha256sum_file,
     write_json)
 
@@ -121,9 +121,9 @@ def compress_one_builddir(builddir):
             with open(tmpfile, 'wb') as f:
                 if args.compressor == 'xz':
                     t = ncpu()
-                    run_verbose(['xz', '-c9', f'-T{t}', filepath], stdout=f)
+                    runcmd_verbose(['xz', '-c9', f'-T{t}', filepath], stdout=f)
                 else:
-                    run_verbose(['gzip', f'-{gzip_level}', '-c', filepath], stdout=f)
+                    runcmd_verbose(['gzip', f'-{gzip_level}', '-c', filepath], stdout=f)
             file_with_ext = file + ext
             filepath_with_ext = filepath + ext
             compressed_size = os.path.getsize(tmpfile)
@@ -199,9 +199,9 @@ def uncompress_one_builddir(builddir):
             with open(tmpfile, 'wb') as f:
                 if file.endswith('xz'):
                     t = ncpu()
-                    run_verbose(['xz', '-dc', f'-T{t}', filepath], stdout=f)
+                    runcmd_verbose(['xz', '-dc', f'-T{t}', filepath], stdout=f)
                 elif file.endswith('gz'):
-                    run_verbose(['gzip', '-dc', filepath], stdout=f)
+                    runcmd_verbose(['gzip', '-dc', filepath], stdout=f)
                 else:
                     print(f"Unknown sufix of file {file}")
             file_without_ext = strip_ext(file)

src/cmd-oc-adm-release

@@ -23,7 +23,7 @@ sys.path.insert(0, COSA_PATH)
 from cosalib.meta import GenericBuildMeta as Meta
 from cosalib.cmdlib import (
     get_basearch,
-    run_verbose
+    runcmd_verbose
 )
 
 os.environ["PATH"] = f"{os.getcwd()}:{COSA_PATH}:{os.environ.get('PATH')}"
@@ -62,7 +62,7 @@ def release_stream(meta, args, ocp_ver):
     if args.authfile != "":
         tag_cmd.extend(["--authfile", args.authfile])
     tag_cmd.extend([f"docker://{args.from_url}"])
-    tags = json.loads(run_verbose(tag_cmd, capture_output=True).stdout)
+    tags = json.loads(runcmd_verbose(tag_cmd, capture_output=True).stdout)
 
     log.info(f"Looking for latest release tag for {ocp_ver}")
     ctags = []
@@ -132,5 +132,5 @@ if __name__ == '__main__':
 
     log.info(f"Command: {' '.join(cmd)}")
     if not args.dry_run:
-        run_verbose(cmd)
+        runcmd_verbose(cmd)
         log.info(f"Payload released to: {payload_dest}")

src/cmd-remote-build-container

@@ -5,7 +5,7 @@ import logging
 import sys
 import tenacity
 
-from cosalib.utils import runcmd
+from cosalib.cmdlib import runcmd
 from os import environ
 
 # Set up logging
@@ -23,7 +23,7 @@ def build_container_image(labels, gitURL, gitRef, repo, tag):
     for label in labels:
         cmd.extend([f"--label={label}"])
     # Long running command. Send output to stdout for logging
-    runcmd(cmd, capture_output=False)
+    runcmd(cmd)
 
 def push_container_image(repo, tag):
     '''
@@ -33,7 +33,7 @@ def push_container_image(repo, tag):
     '''
     cmd = ["podman", "--remote", "push", f"{repo}:{tag}"]
     # Long running command. Send output to stdout for logging
-    runcmd(cmd, capture_output=False)
+    runcmd(cmd)
     # Quay seems to take some time to publish images in some occasions.
     # After the push let's wait for it to show up in the registry
     # before moving on.
@@ -58,7 +58,7 @@ def is_tag_in_podman_storage(repo, tag):
     @param tag  str image tag
     '''
     cmd = ["podman", "--remote", "image", "exists", f"{repo}:{tag}"]
-    return runcmd(cmd, check=False).returncode == 0
+    return runcmd(cmd, check=False, capture_output=True).returncode == 0
 
 def is_tag_in_registry(repo, tag):
     '''
@@ -67,7 +67,8 @@ def is_tag_in_registry(repo, tag):
     @param tag  str image tag
     '''
     # Podman remote doesn't allow push using digestfile. That's why the tag check is done
-    tags = runcmd(["podman", "search", "--list-tags", repo]).stdout.strip().decode()
+    cmd = ["podman", "search", "--list-tags", repo]
+    tags = runcmd(cmd, capture_output=True).stdout.strip().decode()
     if (tag in str(tags)):
         return True
     return False
@@ -82,7 +83,7 @@ def main():
     # short commit hash
     if not args.tag:
         cmd = ["git", "ls-remote", args.git_url, args.git_ref]
-        commit = runcmd(cmd).stdout.strip().decode()[0:7]
+        commit = runcmd(cmd, capture_output=True).stdout.strip().decode()[0:7]
         args.tag = f"{args.arch}-{commit}"
     logging.info(f"Targetting a container image for {args.repo}:{args.tag}")
     # Sanity check the registry if asked to push to a registry

src/cmd-upload-oscontainer

@@ -46,7 +46,7 @@ with open(metapath) as f:
 # for backcompat, we auto-build extensions if they're missing
 if os.path.exists('src/config/extensions.yaml'):
     if 'extensions' not in meta:
-        cmdlib.run_verbose(['coreos-assembler', 'buildextend-extensions'])
+        cmdlib.runcmd_verbose(['coreos-assembler', 'buildextend-extensions'])
         with open(metapath) as f:
             meta = json.load(f)
     assert 'extensions' in meta
@@ -82,9 +82,9 @@ if not os.path.exists(tmprepo):
 
 tmp_osreleasedir = 'tmp/usrlib-osrelease'
 subprocess.check_call(['rm', '-rf', tmp_osreleasedir])
-cmdlib.run_verbose(['/usr/bin/ostree', 'checkout', '--repo', tmprepo,
-                    '--user-mode', '--subpath=/usr/lib/os-release', ostree_commit,
-                    tmp_osreleasedir])
+cmdlib.runcmd_verbose(['/usr/bin/ostree', 'checkout', '--repo', tmprepo,
+                      '--user-mode', '--subpath=/usr/lib/os-release', ostree_commit,
+                      tmp_osreleasedir])
 display_name = None
 with open(os.path.join(tmp_osreleasedir, "os-release")) as f:
     display_name = subprocess.check_output(['/bin/sh', '-c', 'set -euo pipefail; . /proc/self/fd/0 && echo $NAME'], stdin=f, encoding='UTF-8').strip()

src/cmd-virt-install

@@ -127,6 +127,6 @@ basevinstall_args = ['virt-install', f"--connect={args.connect}",
                      f'--name={domname}', '--os-type=linux', '--os-variant=rhel8-unknown',
                      f'--qemu-commandline={qemu_args}',
                      '--noautoconsole']
-cmdlib.run_verbose(basevinstall_args + vinstall_args)
+cmdlib.runcmd_verbose(basevinstall_args + vinstall_args)
 if args.console:
     os.execlp("virsh", "virsh", "console", domname)

src/cosalib/aliyun.py

@@ -2,7 +2,7 @@
 import logging as log
 import json
 import sys
-from cosalib.cmdlib import run_verbose
+from cosalib.cmdlib import runcmd_verbose
 from tenacity import (
     retry,
     stop_after_attempt
@@ -12,7 +12,7 @@
 def remove_aliyun_image(aliyun_id, region):
     print(f"aliyun: removing image {aliyun_id} in {region}")
     try:
-        run_verbose([
+        runcmd_verbose([
             'ore',
             'aliyun', '--log-level', 'debug', 'delete-image',
             '--id', aliyun_id,

src/cosalib/azure.py

@@ -1,6 +1,6 @@
 import os
 import urllib
-from cosalib.cmdlib import run_verbose
+from cosalib.cmdlib import runcmd_verbose
 from tenacity import (
     retry,
     stop_after_attempt
@@ -11,7 +11,7 @@
 def remove_azure_image(image, resource_group, auth, profile):
     print(f"Azure: removing image {image}")
     try:
-        run_verbose([
+        runcmd_verbose([
             'ore', 'azure',
             '--azure-auth', auth,
             '--azure-profile', profile,
@@ -51,7 +51,7 @@ def azure_run_ore(build, args):
     ]
     if args.force:
         ore_args.append('--overwrite')
-    run_verbose(ore_args)
+    runcmd_verbose(ore_args)
 
     url_path = urllib.parse.quote((
         f"{args.storage_account}.blob.core.windows.net/"