Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cosalib/metal.py: migrate cmd-buildextend-metal to Python #1024

Closed
wants to merge 3 commits into from
Closed

cosalib/metal.py: migrate cmd-buildextend-metal to Python #1024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
febca2a
schema: add dasd disk definition
Feb 11, 2020
e1fdde7
REFACTOR: convert cmd-buildextend-{metal,qemu,dasd} to Python
Feb 12, 2020
6c0cdef
Update src/cosalib/metal.py
Feb 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/cmd-artifact-disk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ sys.path.insert(0, cosa_dir)

from cosalib.build import BuildExistsError
from cosalib.cli import BuildCli
import cosalib.metal as Metal
darkmuggle marked this conversation as resolved.
Show resolved Hide resolved
import cosalib.qemuvariants as QVariants
import cosalib.vmware as VmwareOVA

Expand All @@ -26,8 +27,13 @@ def get_builder(imgtype, build_root, build="latest", force=False, schema=None):
return QVariants.QemuVariantImage(*args, **kargs)

if imgtype in VmwareOVA.VARIANTS:
log.info(f"Target '{imgtype.upper()}' is a VMWare OVA Variant image")
return VmwareOVA.VmwareOVA(*args, **kargs)

if imgtype in Metal.VARIANTS:
log.info(f"Target '{imgtype.upper()}' is a Metal/Base Variant image")
return Metal.MetalVariantImage(*args, **kargs)

raise Exception(f"{imgtype} is not supported by this command")


Expand All @@ -39,6 +45,7 @@ def artifact_cli():

targets = list(QVariants.VARIANTS.keys())
targets.extend(VmwareOVA.VARIANTS.keys())
targets.extend(Metal.VARIANTS.keys())
targets.append("manual")

parser = BuildCli()
Expand Down
2 changes: 1 addition & 1 deletion src/cmd-buildextend-dasd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
cmd-buildextend-metal
cmd-artifact-disk
266 changes: 0 additions & 266 deletions src/cmd-buildextend-metal
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions src/cmd-buildextend-metal
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
cmd-artifact-disk
2 changes: 1 addition & 1 deletion src/cmd-buildextend-qemu
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
cmd-buildextend-metal
cmd-artifact-disk
7 changes: 6 additions & 1 deletion src/cmdlib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@
set -euo pipefail
# Shared shell script library

DIR=$(dirname "$0")
my_name="$0"
if [ "${BASH_SOURCE[0]}x" != "x" ]; then
# Allow this library to be sourced
my_name="${BASH_SOURCE[0]}"
fi
DIR="$(dirname "$(readlink -f "${my_name}")")"
RFC3339="%Y-%m-%dT%H:%M:%SZ"

# Detect what platform we are on
Expand Down
2 changes: 1 addition & 1 deletion src/cosalib/build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ def get_artifact_meta(self, fname=None):
log.info(f"Calculating metadata for {fname}")
return {
"path": fname,
"sha256sum": sha256sum_file(fpath),
"sha256": sha256sum_file(fpath),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was invalid per the schema.

"size": int(fsize)
}

Expand Down
28 changes: 21 additions & 7 deletions src/cosalib/cmdlib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ def run_verbose(args, **kwargs):
:type kwargs: dict
:raises: CalledProcessError
"""
print("+ {}".format(subprocess.list2cmdline(args)))

# default to throwing exception
if 'check' not in kwargs.keys():
kwargs['check'] = True
Expand All @@ -57,11 +55,27 @@ def run_verbose(args, **kwargs):
kwargs['stdout'] = subprocess.PIPE
kwargs['stderr'] = subprocess.PIPE

try:
process = subprocess.run(args, **kwargs)
except subprocess.CalledProcessError:
fatal("Error running command " + args[0])
return process
print("+ {}".format(subprocess.list2cmdline(args)))
with tempfile.NamedTemporaryFile(prefix="cmdlib-cmd") as t:
cosa_path = os.path.dirname(os.path.dirname(__file__))

# when cmdlib_sh=True, execute the command in a wrapper
# that sources the cmdlib.sh functions to allow using
# cmdlib.sh features like "runvm".
if kwargs.pop('cmdlib_sh', False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fairly magical, in extending the subprocess API with a custom keyword.

Given how few things need this, maybe better done outside-in as:

def run_cmdlib(...):
   with tempfile:
      run_verbose(tmpshell)

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, esp. given that the cmdlib hack should really be a temporary thing until we move it all to Python.

wrapper = f"""
. {cosa_path}/cmdlib.sh
{' '.join(args)}
"""
t.write(str.encode(wrapper))
t.flush()
args = ["/bin/bash", "-xe", f"{t.name}"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do slightly better here by just feeding the commands to bash via stdin :) Then there's no chance of leaving files around if we're killed.


try:
process = subprocess.run(args, **kwargs)
except subprocess.CalledProcessError:
fatal("Error running command " + args[0])
return process


def write_json(path, data):
Expand Down
Loading