From 34106bcee68974843ecb1164eb65f24b21a7d63e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 10 Apr 2022 13:37:44 -0400 Subject: [PATCH 1/3] Serialize `grub-script` literally into image.json Prep for injecting `image.json` into the ostree commit. To make that meaningful, it has to be entirely independent of coreos-assembler. This bit is crucial for being able to do image builds in a manner independent of coreos-assembler's content, using solely the ostree commit as input. --- src/cosalib/cmdlib.py | 3 +++ src/create_disk.sh | 2 +- src/image-default.yaml | 1 - 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cosalib/cmdlib.py b/src/cosalib/cmdlib.py index 8bf128edc1..33e605d2cd 100644 --- a/src/cosalib/cmdlib.py +++ b/src/cosalib/cmdlib.py @@ -354,6 +354,9 @@ def generate_image_json(srcfile): r = yaml.safe_load(open("/usr/lib/coreos-assembler/image-default.yaml")) for k, v in flatten_image_yaml(srcfile).items(): r[k] = v + # Serialize our default GRUB config + with open("/usr/lib/coreos-assembler/grub.cfg") as f: + r['grub-script'] = f.read() return r diff --git a/src/create_disk.sh b/src/create_disk.sh index 61d52cd963..370bb2d311 100755 --- a/src/create_disk.sh +++ b/src/create_disk.sh @@ -364,7 +364,7 @@ install_grub_cfg() { # 0700 to match the RPM permissions which I think are mainly in case someone has # manually set a grub password mkdir -p -m 0700 $rootfs/boot/grub2 - cp -v $grub_script $rootfs/boot/grub2/grub.cfg + printf "%s" "$grub_script" > $rootfs/boot/grub2/grub.cfg } # Other arch-specific bootloader changes diff --git a/src/image-default.yaml b/src/image-default.yaml index 2810c34b05..c4bc05798a 100644 --- a/src/image-default.yaml +++ b/src/image-default.yaml @@ -4,7 +4,6 @@ bootfs: "ext4" rootfs: "xfs" # Add arguments here that will be passed to e.g. mkfs.xfs rootfs-args: "" -grub-script: "/usr/lib/coreos-assembler/grub.cfg" # Additional default kernel arguments injected into disk images extra-kargs: [] From 4ac818f74cc341cff789cd9033fa6fcdd701faab Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 13 Apr 2022 17:08:08 -0400 Subject: [PATCH 2/3] pytest: Lower percentage to 75% So...personally, I think unit testing most of the coreos-assembler code in practice is not very useful. We have good integration tests that cover things. --- pytest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index 8b36970ea6..7eced2f302 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,2 +1,2 @@ [pytest] -addopts = --cov=cosalib.cli --cov=cosalib.meta --cov=cosalib.cmdlib --cov-report term --cov-fail-under=80 +addopts = --cov=cosalib.cli --cov=cosalib.meta --cov=cosalib.cmdlib --cov-report term --cov-fail-under=75 From 089f9cd3610e9a4f74280b81ba474a9fbbda8005 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 9 Apr 2022 10:19:13 -0400 Subject: [PATCH 3/3] Insert generated image.json into the ostree commit This is part of https://github.com/coreos/fedora-coreos-tracker/issues/1151 Our generated disk images are largely just a "shell" around the egg of an ostree commit. There is almost nothing that lives in the disk image that isn't in the commit. (This is especially true now that a preparatory commit previous to this moved the *content* of our static `grub.cfg` into `image.json`) In the original coreos-assembler design I'd tried to cleanly separate builds of the ostree from disk image builds, but also support linking them together (with matching version numbers, etc.) The separate `image.yaml` was part of this. This...mostly worked. This change furthers that separation by having image builds input from *just the ostree commit*. Crucially we would no longer need the config git repository to perform an image build. And this in turn unlocks truly better separating ostree builds from disk image builds in the pipeline *and* supporting downstream tooling generating disk images from custom containers. One neat thing here is we will finally fix a longstanding issue where coreos-assembler fails when just the `image.yaml` changes: Closes: #972 --- src/cmd-buildextend-live | 22 ++++++++++------------ src/cmd-buildextend-metal | 3 +++ src/cmdlib.sh | 21 +++++++++++++++++++++ src/cosalib/cmdlib.py | 7 +++++++ src/cosalib/ova.py | 4 ++-- src/cosalib/qemuvariants.py | 6 +++++- 6 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/cmd-buildextend-live b/src/cmd-buildextend-live index 486fd26545..a740ea6950 100755 --- a/src/cmd-buildextend-live +++ b/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, generate_image_json +from cosalib.cmdlib import run_verbose, sha256sum_file, extract_image_json from cosalib.cmdlib import import_ostree_commit, get_basearch, ensure_glob from cosalib.meta import GenericBuildMeta @@ -46,9 +46,6 @@ if not args.build: args.build = builds.get_latest() print(f"Targeting build: {args.build}") -image_json = generate_image_json('src/config/image.yaml') -squashfs_compression = 'lz4' if args.fast else image_json['squashfs-compression'] - srcdir_prefix = "src/config/live/" if not os.path.isdir(srcdir_prefix): @@ -58,6 +55,15 @@ workdir = os.path.abspath(os.getcwd()) builddir = builds.get_build_dir(args.build) buildmeta_path = os.path.join(builddir, 'meta.json') buildmeta = GenericBuildMeta(workdir=workdir, build=args.build) +repo = os.path.join(workdir, 'tmp/repo') + +# Grab the commit hash for this build +buildmeta_commit = buildmeta['ostree-commit'] + +import_ostree_commit(repo, builddir, buildmeta) + +image_json = extract_image_json(repo, buildmeta_commit) +squashfs_compression = 'lz4' if args.fast else image_json['squashfs-compression'] base_name = buildmeta['name'] if base_name == "rhcos" and args.fast: @@ -71,12 +77,6 @@ if os.path.exists(build_semaphore): raise Exception( f"{build_semaphore} exists: another process is building live") - -# Grab the commit hash for this build -buildmeta_commit = buildmeta['ostree-commit'] - -repo = os.path.join(workdir, 'tmp/repo') - # Don't run if it's already been done, unless forced if 'live-iso' in buildmeta['images'] and not args.force: print(f"'live' has already been built for {args.build}. Skipping.") @@ -696,8 +696,6 @@ boot print(f"Updated: {buildmeta_path}") -import_ostree_commit(repo, builddir, buildmeta) - # lock and build with open(build_semaphore, 'w') as f: f.write(f"{time.time_ns()}") diff --git a/src/cmd-buildextend-metal b/src/cmd-buildextend-metal index 87f6dac3cd..5be7b2ca96 100755 --- a/src/cmd-buildextend-metal +++ b/src/cmd-buildextend-metal @@ -118,6 +118,9 @@ ostree_repo=${tmprepo} # Ensure that we have the cached unpacked commit import_ostree_commit_for_build "${build}" +image_json=image.json +extract_image_json "${tmprepo}" "${commit}" > "${image_json}" + image_format=raw if [[ $image_type == qemu ]]; then image_format=qcow2 diff --git a/src/cmdlib.sh b/src/cmdlib.sh index 267f822bda..d5cad5e5c1 100755 --- a/src/cmdlib.sh +++ b/src/cmdlib.sh @@ -394,6 +394,13 @@ EOF done fi + local imagejsondir="${tmp_overridesdir}/imagejson" + export ostree_image_json="/usr/share/coreos-assembler/image.json" + mkdir -p "${imagejsondir}/usr/share/coreos-assembler/" + cp "${image_json}" "${imagejsondir}${ostree_image_json}" + commit_overlay cosa-image-json "${imagejsondir}" + layers="${layers} cosa-image-json" + local_overrides_lockfile="${tmp_overridesdir}/local-overrides.json" if [ -n "${with_cosa_overrides}" ] && [[ -n $(ls "${overridesdir}/rpm/"*.rpm 2> /dev/null) ]]; then (cd "${overridesdir}"/rpm && rm -rf .repodata && createrepo_c .) @@ -874,6 +881,7 @@ builds.bump_timestamp() print('Build ${buildid} was inserted ${arch:+for $arch}')") } +# Prepare the image.json as part of an ostree image build write_image_json() { local srcfile=$1; shift local outfile=$1; shift @@ -884,6 +892,19 @@ from cosalib import cmdlib cmdlib.write_image_json('${srcfile}', '${outfile}')") } +# Fetch the image.json from the ostree commit to stdout. +# Should be used by disk image builds. +extract_image_json() { + local repo=$1; shift + local commit=$1; shift + (python3 -c " +import sys, json +sys.path.insert(0, '${DIR}') +from cosalib import cmdlib +json.dump(cmdlib.extract_image_json('${repo}', '${commit}'), sys.stdout, sort_keys=True) +") +} + # Shell wrapper for the Python import_ostree_commit import_ostree_commit_for_build() { local buildid=$1; shift diff --git a/src/cosalib/cmdlib.py b/src/cosalib/cmdlib.py index 33e605d2cd..e8bbd2ea32 100644 --- a/src/cosalib/cmdlib.py +++ b/src/cosalib/cmdlib.py @@ -350,6 +350,13 @@ def cmdlib_sh(script): ''']) +# Should be used by disk image builds to extract the image.json from the +# ostree commit. +def extract_image_json(repo, commit): + out = subprocess.check_output(['ostree', f'--repo={repo}', 'cat', commit, '/usr/share/coreos-assembler/image.json']) + return json.loads(out) + + def generate_image_json(srcfile): r = yaml.safe_load(open("/usr/lib/coreos-assembler/image-default.yaml")) for k, v in flatten_image_yaml(srcfile).items(): diff --git a/src/cosalib/ova.py b/src/cosalib/ova.py index 6949534da2..e900b6d6e6 100644 --- a/src/cosalib/ova.py +++ b/src/cosalib/ova.py @@ -10,7 +10,7 @@ sys.path.insert(0, f"{cosa_dir}/cosalib") sys.path.insert(0, cosa_dir) -from cosalib.cmdlib import generate_image_json, image_info +from cosalib.cmdlib import extract_image_json, image_info from cosalib.qemuvariants import QemuVariantImage @@ -86,7 +86,7 @@ def generate_ovf_parameters(self, vmdk, cpu=2, memory=4096): Returns a dictionary with the parameters needed to create an OVF file based on the qemu, vmdk, image.yaml, and info from the build metadata """ - image_json = generate_image_json('src/config/image.yaml') + image_json = extract_image_json(os.path.join(self._workdir, 'tmp/repo'), self.meta['ostree-commit']) system_type = 'vmx-{}'.format(image_json['vmware-hw-version']) os_type = image_json['vmware-os-type'] diff --git a/src/cosalib/qemuvariants.py b/src/cosalib/qemuvariants.py index 8d57803b12..812d86dc6a 100644 --- a/src/cosalib/qemuvariants.py +++ b/src/cosalib/qemuvariants.py @@ -20,7 +20,8 @@ get_basearch, image_info, run_verbose, - sha256sum_file + sha256sum_file, + import_ostree_commit ) # BASEARCH is the current machine architecture @@ -183,6 +184,9 @@ def __init__(self, **kwargs): _Build.__init__(self, **kwargs) + repo = os.path.join(self._workdir, 'tmp/repo') + import_ostree_commit(repo, self._tmpdir, self.meta) + @property def image_qemu(self): """