From 7dd45ad7843487b303d018b8ca52b259473f3d78 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Fri, 29 Nov 2024 12:55:08 -0500 Subject: [PATCH 01/15] gitignore: generalize the .build files ignore rule Signed-off-by: John Mulligan --- .gitignore | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 2dc9b698..3addb4ef 100644 --- a/.gitignore +++ b/.gitignore @@ -1,13 +1,4 @@ *~ -.build.server -.build.opensuse-server -.build.ad-server -.build.opensuse-ad-server -.build.client -.build.opensuse-client -.build.toolbox -.build.opensuse-toolbox -.build.nightly.server -.build.nightly.ad-server +.build.* .common .bin From 98fe5cf78bf941da24142ed5ce029183dd12c8c1 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Fri, 29 Nov 2024 13:00:47 -0500 Subject: [PATCH 02/15] hack/build-image: apply black style formatting Some `black` and `flake8` fixes. Signed-off-by: John Mulligan --- hack/build-image | 77 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 54 insertions(+), 23 deletions(-) diff --git a/hack/build-image b/hack/build-image index 34e8530a..caafb898 100755 --- a/hack/build-image +++ b/hack/build-image @@ -102,9 +102,9 @@ DEVBUILDS = "devbuilds" PACKAGE_SOURCES = [DEFAULT, NIGHTLY, DEVBUILDS] PACKAGES_FROM = { - DEFAULT: '', - NIGHTLY: 'samba-nightly', - DEVBUILDS: 'devbuilds', + DEFAULT: "", + NIGHTLY: "samba-nightly", + DEVBUILDS: "devbuilds", } # SOURCE_DIRS - image source paths @@ -121,7 +121,7 @@ DEFAULT_DISTRO_BASES = [FEDORA] LATEST = "latest" QUAL_NONE = "unqualified" QUAL_DISTRO = "distro-qualified" -QUAL_FQIN = 'fqin' +QUAL_FQIN = "fqin" _DISCOVERED_CONTAINER_ENGINES = [] @@ -207,45 +207,71 @@ def container_build(cli, target): tasks = [] # For docker cross-builds we need to use buildx - if "docker" in eng and target.arch != host_arch(): + if "docker" in eng and target.arch != host_arch(): args = [eng, "buildx"] # Docker's default builder only supports the host architecture. # Therefore, we need to create a new builder to support other # architectures, and we must ensure we start with a fresh builder # that does not contain any images from previous builds. - tasks.append(lambda : run(cli, args + ["rm", target.flat_name()], check=False)) - tasks.append(lambda : run(cli, args + ["create", f"--name={target.flat_name()}"], check=True)) + tasks.append( + lambda: run(cli, args + ["rm", target.flat_name()], check=False) + ) + tasks.append( + lambda: run( + cli, + args + ["create", f"--name={target.flat_name()}"], + check=True, + ) + ) - tasks.append(lambda : run(cli, args + [ - "build", - f"--builder={target.flat_name()}", - f"--platform=linux/{target.arch}", - "--load"] + create_common_container_engine_args(cli, target), check=True)) + tasks.append( + lambda: run( + cli, + args + + [ + "build", + f"--builder={target.flat_name()}", + f"--platform=linux/{target.arch}", + "--load", + ] + + create_common_container_engine_args(cli, target), + check=True, + ) + ) - tasks.append(lambda : run(cli, args + ["rm", target.flat_name()], check=True)) + tasks.append( + lambda: run(cli, args + ["rm", target.flat_name()], check=True) + ) else: args = [eng, "build"] if target.arch != host_arch() or FORCE_ARCH_FLAG: - # We've noticed a few small quirks when using podman with the --arch - # option. The main issue is that building the client image works - # but then the toolbox image fails because it somehow doesn't see - # the image we just built as usable. This doesn't happen when + # We've noticed a few small quirks when using podman with the + # --arch option. The main issue is that building the client image + # works but then the toolbox image fails because it somehow doesn't + # see the image we just built as usable. This doesn't happen when # --arch is not provided. So if the target arch and the host_arch # are the same, skip passing the extra argument. args += [f"--arch={target.arch}"] - tasks.append(lambda : run(cli, args + create_common_container_engine_args(cli, target), check=True)) + tasks.append( + lambda: run( + cli, + args + create_common_container_engine_args(cli, target), + check=True, + ) + ) for task in tasks: task() + def create_common_container_engine_args(cli, target): args = [] pkgs_from = PACKAGES_FROM[target.pkg_source] if pkgs_from: args.append(f"--build-arg=INSTALL_PACKAGES_FROM={pkgs_from}") - + if cli.extra_build_arg: args.extend(cli.extra_build_arg) @@ -258,6 +284,7 @@ def create_common_container_engine_args(cli, target): args.append(kind_source_dir(target.name)) return [str(a) for a in args] + def container_push(cli, push_name): """Construct and execute a command to push a container image.""" args = [container_engine(cli), "push", push_name] @@ -306,7 +333,9 @@ def kind_source_dir(kind): def target_containerfile(target): """Return the path to a containerfile given an image target.""" - return str(kind_source_dir(target.name) / f"Containerfile.{target.distro}") + return str( + kind_source_dir(target.name) / f"Containerfile.{target.distro}" + ) def host_arch(): @@ -659,7 +688,7 @@ def main(): parser.add_argument( "--push-selected-tags", type=QMatcher, - help=QMatcher.__doc__ + help=QMatcher.__doc__, ) parser.add_argument( "--buildfile-prefix", @@ -724,8 +753,10 @@ def main(): action="store_const", dest="main_action", const=retag, - help=("Regenerate any short (unqualified) tags expected to exist" - " for a given FQIN. Requires FQIN to already exist locally."), + help=( + "Regenerate any short (unqualified) tags expected to exist" + " for a given FQIN. Requires FQIN to already exist locally." + ), ) cli = parser.parse_args() From 0cb61dfbca3d0fa066e2105708adab5937d48a6e Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Fri, 8 Nov 2024 12:28:08 -0500 Subject: [PATCH 03/15] hack/build-image: use constants already defined in the code Signed-off-by: John Mulligan --- hack/build-image | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/build-image b/hack/build-image index caafb898..6591f636 100755 --- a/hack/build-image +++ b/hack/build-image @@ -463,9 +463,9 @@ def add_special_tags(img, distro_qualified=True): img.additional_tags.append((NIGHTLY, QUAL_NONE)) if not distro_qualified: return # skip creating "distro qualified" tags - if img.arch == host_arch() and img.pkg_source == "default": + if img.arch == host_arch() and img.pkg_source == DEFAULT: img.additional_tags.append((f"{img.distro}-{LATEST}", QUAL_DISTRO)) - if img.arch == host_arch() and img.pkg_source == "nightly": + if img.arch == host_arch() and img.pkg_source == NIGHTLY: img.additional_tags.append((f"{img.distro}-{NIGHTLY}", QUAL_DISTRO)) From 55e970a4fb615c69d9fdce7d0c4387186017ebfb Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Fri, 8 Nov 2024 12:51:56 -0500 Subject: [PATCH 04/15] hack/build-image: don't call same function many times Don't call the same function many times when that function's value is never expected to change at this scope. Signed-off-by: John Mulligan --- hack/build-image | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/hack/build-image b/hack/build-image index 6591f636..f530e172 100755 --- a/hack/build-image +++ b/hack/build-image @@ -456,16 +456,17 @@ def add_special_tags(img, distro_qualified=True): # that certain images deserve some extra special tags. Mostly this serves # to keep us compatible with older tagging schemes from earlier versions of # the project. + _host_arch = host_arch() if img.distro in [FEDORA, OPENSUSE]: - if img.arch == host_arch() and img.pkg_source == DEFAULT: + if img.arch == _host_arch and img.pkg_source == DEFAULT: img.additional_tags.append((LATEST, QUAL_NONE)) - if img.arch == host_arch() and img.pkg_source == NIGHTLY: + if img.arch == _host_arch and img.pkg_source == NIGHTLY: img.additional_tags.append((NIGHTLY, QUAL_NONE)) if not distro_qualified: return # skip creating "distro qualified" tags - if img.arch == host_arch() and img.pkg_source == DEFAULT: + if img.arch == _host_arch and img.pkg_source == DEFAULT: img.additional_tags.append((f"{img.distro}-{LATEST}", QUAL_DISTRO)) - if img.arch == host_arch() and img.pkg_source == NIGHTLY: + if img.arch == _host_arch and img.pkg_source == NIGHTLY: img.additional_tags.append((f"{img.distro}-{NIGHTLY}", QUAL_DISTRO)) From 63ea60820e90a789c023b014619a0b39c9416f1e Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Fri, 29 Nov 2024 13:22:49 -0500 Subject: [PATCH 05/15] hack/build-image: remove unnecessary uses of lambda I find lambda to be a bit of an unpleasant thing and try to remove it when possible. Since all uses of lambda in this code are calling the same function we can remove the function (and unchanging) argument and have the variable section map to the run function's remaining kwargs. Signed-off-by: John Mulligan --- hack/build-image | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/hack/build-image b/hack/build-image index f530e172..29d09153 100755 --- a/hack/build-image +++ b/hack/build-image @@ -215,20 +215,18 @@ def container_build(cli, target): # architectures, and we must ensure we start with a fresh builder # that does not contain any images from previous builds. tasks.append( - lambda: run(cli, args + ["rm", target.flat_name()], check=False) + {"cmd": args + ["rm", target.flat_name()], "check": False} ) tasks.append( - lambda: run( - cli, - args + ["create", f"--name={target.flat_name()}"], - check=True, - ) + { + "cmd": args + ["create", f"--name={target.flat_name()}"], + "check": True, + } ) tasks.append( - lambda: run( - cli, - args + { + "cmd": args + [ "build", f"--builder={target.flat_name()}", @@ -236,12 +234,12 @@ def container_build(cli, target): "--load", ] + create_common_container_engine_args(cli, target), - check=True, - ) + "check": True, + } ) tasks.append( - lambda: run(cli, args + ["rm", target.flat_name()], check=True) + {"cmd": args + ["rm", target.flat_name()], "check": True} ) else: args = [eng, "build"] @@ -255,15 +253,15 @@ def container_build(cli, target): args += [f"--arch={target.arch}"] tasks.append( - lambda: run( - cli, - args + create_common_container_engine_args(cli, target), - check=True, - ) + { + "cmd": args + + create_common_container_engine_args(cli, target), + "check": True, + } ) for task in tasks: - task() + run(cli, **task) def create_common_container_engine_args(cli, target): From 18535a20ca8a1518fc144039b6219593956ea027 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Fri, 29 Nov 2024 13:09:20 -0500 Subject: [PATCH 06/15] hack/build-image: add --archive action Add an --archive=LOCATION command line argument that is somewhat like push but instead uses the archive tarballs. This could be use in a situation where you are building on a fast machine or different arch than the machine you will ultimately be pushing from. Signed-off-by: John Mulligan --- hack/build-image | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/hack/build-image b/hack/build-image index 29d09153..0896296c 100755 --- a/hack/build-image +++ b/hack/build-image @@ -36,6 +36,7 @@ Usage: """ import argparse +import functools import logging import os import pathlib @@ -304,6 +305,16 @@ def container_id(cli, target): return res.stdout.decode("utf8").strip() +def maybe_container_id(cli, target): + """Same as container_id but returns None if no match for the target was + found. + """ + try: + return container_id(cli, target) + except subprocess.CalledProcessError: + return None + + def container_tag(cli, target, tag, *tags): """Add additional tags to the existing target image.""" if isinstance(target, str): @@ -552,6 +563,20 @@ def push(cli, target): container_push(cli, push_name) +def archive(cli, target, location): + """Write tarballs to archive location.""" + # reuse push_stage flag. TODO rename flag + if cli.push_state == "rebuild" or ( + cli.push_state == "exists" and not maybe_container_id(cli, target) + ): + build(cli, target) + + eng = container_engine(cli) + fname = pathlib.Path(location) / f"{target.flat_name()}.tar" + args = [eng, "save", f"-o{fname}", target.image_name()] + run(cli, args, check=True) + + def retag(cli, target): """Command to regenerate any missing unqualified tags.""" cid = container_id(cli, target) @@ -590,6 +615,16 @@ def print_tags(cli, target): print(f"{prefix}{name}") +def _kwbind(func, key, conversion=None): + """Attach an argument value to a command-linked argument.""" + + def _capture(arg_value): + value = conversion(arg_value) if conversion else arg_value + return functools.partial(func, **{key: value}) + + return _capture + + def main(): parser = argparse.ArgumentParser() parser.add_argument( @@ -757,6 +792,13 @@ def main(): " for a given FQIN. Requires FQIN to already exist locally." ), ) + behaviors.add_argument( + "--archive", + metavar="LOCATION", + dest="main_action", + type=_kwbind(archive, "location"), + help="Write archive files (tarballs) to a specified location", + ) cli = parser.parse_args() if os.environ.get("BUILD_IMAGE_DEBUG") in ("1", "yes"): From 8f5598eb365fea27158567e15071610f34fb2e3e Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Mon, 6 Jan 2025 14:59:30 -0500 Subject: [PATCH 07/15] hack/build-image: add --load action Add a --load=LOCATION command line argument that mirrors the new --archive command but imports images from the archive dir. This could be used in a situation where you are building images on one machine but then need to update or push images from a different machine. Signed-off-by: John Mulligan --- hack/build-image | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/hack/build-image b/hack/build-image index 0896296c..433c3db4 100755 --- a/hack/build-image +++ b/hack/build-image @@ -577,6 +577,16 @@ def archive(cli, target, location): run(cli, args, check=True) +def load_archived(cli, target, location): + """Load tarballs from archive location.""" + fname = pathlib.Path(location) / f"{target.flat_name()}.tar" + logger.info("Loading from: %s", fname) + + eng = container_engine(cli) + args = [eng, "load", f"-i{fname}"] + run(cli, args, check=True) + + def retag(cli, target): """Command to regenerate any missing unqualified tags.""" cid = container_id(cli, target) @@ -799,6 +809,13 @@ def main(): type=_kwbind(archive, "location"), help="Write archive files (tarballs) to a specified location", ) + behaviors.add_argument( + "--load", + metavar="LOCATION", + dest="main_action", + type=_kwbind(load_archived, "location"), + help="Read in archive files (tarballs) from a specified location", + ) cli = parser.parse_args() if os.environ.get("BUILD_IMAGE_DEBUG") in ("1", "yes"): From 9fc6bbe5b59199d690410de67293aa1f747b494c Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Mon, 6 Jan 2025 15:10:20 -0500 Subject: [PATCH 08/15] hack/build-image: add _split_img method Split the existing function used to break up a container name into parts into a smaller reusable function. Signed-off-by: John Mulligan --- hack/build-image | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/hack/build-image b/hack/build-image index 433c3db4..30563907 100755 --- a/hack/build-image +++ b/hack/build-image @@ -366,6 +366,17 @@ class RepoConfig: return self.distro_map.get(distro, self.default) +def _split_img(image_name, max_tag_split=3): + if "/" in image_name: + base, rest = image_name.rsplit("/", 1) + else: + base = "" + rest = image_name + iname, tag = rest.split(":", 1) + tparts = tag.split("-", max_tag_split) + return base, iname, tparts + + class TargetImage: def __init__( self, name, pkg_source, distro, arch, extra_tag="", *, repo_base="" @@ -414,13 +425,7 @@ class TargetImage: @classmethod def parse(cls, image_name): - if "/" in image_name: - base, rest = image_name.rsplit("/", 1) - else: - base = "" - rest = image_name - iname, tag = rest.split(":", 1) - tparts = tag.split("-", 3) + base, iname, tparts = _split_img(image_name) if len(tparts) < 3: raise ValueError(f"too few tag components: {tag!r}") return cls( From 4ce5ebe7c8146700891e3c1cc30862dfd9c53d86 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Tue, 7 Jan 2025 10:47:57 -0500 Subject: [PATCH 09/15] hack/build-image: add --combined option for building indexes Indexes (aka manifests) allow for images to be built for different architectures and the associated with one tag (or set of shared tags). The build script enables indexes/manifests when supplied with the --combined flag. The `build` function is changed a bit and no longer always builds an image. If the image is already present locally, the build will be skipped - this is done because (re)building an image of a different architecture can be very slow. This new behavior tries to avoid costly unintentional rebuilds. A future change will add a new command to force a rebuild. Signed-off-by: John Mulligan --- hack/build-image | 271 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 232 insertions(+), 39 deletions(-) diff --git a/hack/build-image b/hack/build-image index 30563907..0ff64423 100755 --- a/hack/build-image +++ b/hack/build-image @@ -37,6 +37,7 @@ Usage: import argparse import functools +import json import logging import os import pathlib @@ -265,6 +266,59 @@ def container_build(cli, target): run(cli, **task) +def index_build(cli, target): + """Construct a new index or manifest.""" + logger.debug("Building index: %s", target) + eng = container_engine(cli) + args = [eng, "manifest", "create", target.image_name()] + run(cli, args, check=True) + # add images to index + for img in target.contents: + add_args = [eng, "manifest", "add"] + # Currently annotations can't be used for much, but perhaps in the + # future podman/docker/etc will let you filter on annotations. Then + # you could choose base distro, etc using a single big index with + # annotations indicating various features. For now, it's mostly + # acedemic and for practice. + add_args.append( + "--annotation=org.samba.samba-container.pkg-source=" + f"{img.pkg_source}" + ) + add_args.append( + "--annotation=org.samba.samba-container.distro=" f"{img.distro}" + ) + add_args += [target.image_name(), img.image_name()] + run(cli, add_args, check=True) + # apply additional tag names + for tname in target.all_names(baseless=cli.without_repo_bases): + tag_args = [eng, "tag", target.image_name(), tname] + if target.image_name() != tname: + run(cli, tag_args, check=True) + # verfication step + inspect_args = [eng, "manifest", "inspect", target.image_name()] + res = run(cli, inspect_args, check=True, capture_output=True) + idx_info = json.loads(res.stdout) + if len(idx_info["manifests"]) != len(target.contents): + logger.error("unexpected index info: %r", idx_info) + logger.error( + "saw %d entries, expected %d (%r)", + len(idx_info["manifests"]), + len(target.contents), + target.contents, + ) + raise ValueError("unexpected number of manifest entries") + return target.image_name() + + +def image_build(cli, target): + if isinstance(target, TargetIndex): + logger.debug("target is an index (or manifest)") + return index_build(cli, target) + else: + logger.debug("target is a container image") + return container_build(cli, target) + + def create_common_container_engine_args(cli, target): args = [] pkgs_from = PACKAGES_FROM[target.pkg_source] @@ -294,6 +348,15 @@ def container_id(cli, target): """Construct and run a command to fetch a hexidecimal id for a container image. """ + if isinstance(target, TargetIndex): + args = [ + container_engine(cli), + "manifest", + "exists", + target.image_name(), + ] + run(cli, args, check=True) + return target.image_name() args = [ container_engine(cli), "inspect", @@ -377,24 +440,36 @@ def _split_img(image_name, max_tag_split=3): return base, iname, tparts -class TargetImage: +class Target: def __init__( - self, name, pkg_source, distro, arch, extra_tag="", *, repo_base="" + self, name, *, pkg_source, distro, extra_tag="", repo_base="" ): self.name = name self.pkg_source = pkg_source self.distro = distro - self.arch = arch self.extra_tag = extra_tag self.repo_base = repo_base self.additional_tags = [] + def all_names(self, baseless=False): + yield self.image_name() + for tag, _ in self.additional_tags: + yield self.image_name(tag=tag) + if self.repo_base and baseless: + yield self.image_name(repo_base="") + for tag, qual in self.additional_tags: + if qual == QUAL_NONE: + continue + yield self.image_name(tag=tag, repo_base="") + + def supports_arch(self, arch): + return False + + def flat_name(self): + return f"{self.name}.{self.tag_name()}" + def tag_name(self): - tag_parts = [self.pkg_source, self.distro, self.arch] - if self.extra_tag: - tag_parts.append(self.extra_tag) - tag = "-".join(tag_parts) - return tag + raise NotImplementedError() def image_name(self, *, tag=None, repo_base=None): if not tag: @@ -406,22 +481,32 @@ class TargetImage: image_name = f"{repo_base}/{image_name}" return image_name - def flat_name(self): - return f"{self.name}.{self.tag_name()}" - def __str__(self): return self.image_name() - def all_names(self, baseless=False): - yield self.image_name() - for tag, _ in self.additional_tags: - yield self.image_name(tag=tag) - if self.repo_base and baseless: - yield self.image_name(repo_base="") - for tag, qual in self.additional_tags: - if qual == QUAL_NONE: - continue - yield self.image_name(tag=tag, repo_base="") + +class TargetImage(Target): + def __init__( + self, name, pkg_source, distro, arch, extra_tag="", *, repo_base="" + ): + super().__init__( + name, + pkg_source=pkg_source, + distro=distro, + extra_tag=extra_tag, + repo_base=repo_base, + ) + self.arch = arch + + def tag_name(self): + tag_parts = [self.pkg_source, self.distro, self.arch] + if self.extra_tag: + tag_parts.append(self.extra_tag) + tag = "-".join(tag_parts) + return tag + + def supports_arch(self, arch): + return arch == self.arch @classmethod def parse(cls, image_name): @@ -438,9 +523,98 @@ class TargetImage: ) -def generate_images(cli): - """Given full image names or a matrix of kind/pkg_source/distro_base/arch - values generate a list of target images to build/process. +class TargetIndex(Target): + def __init__( + self, + name, + *, + pkg_source, + distro, + contents=None, + extra_tag="", + repo_base="", + ): + super().__init__( + name, + pkg_source=pkg_source, + distro=distro, + extra_tag=extra_tag, + repo_base=repo_base, + ) + self.contents = contents or [] + + def key(self): + return (self.name, self.pkg_source, self.distro) + + def tag_name(self): + tag_parts = [self.pkg_source, self.distro] + if self.extra_tag: + tag_parts.append(self.extra_tag) + tag = "-".join(tag_parts) + return tag + + def merge(self, other): + assert self.name == other.name + assert self.pkg_source == other.pkg_source + assert self.distro == other.distro + self.contents.extend(other.contents) + + def supports_arch(self, arch): + return True + + @classmethod + def from_image(cls, img): + return cls( + img.name, + pkg_source=img.pkg_source, + distro=img.distro, + contents=[img], + repo_base=img.repo_base or "", + ) + + +class BuildRequest: + def __init__(self, images=None, indexes=None): + self.images = list(images or []) + self.indexes = list(indexes or []) + + def __bool__(self): + return bool(self.images or self.indexes) + + def expanded(self, indexes=False, distro_qualified=True): + new_req = self.__class__(self.images, self.indexes) + if indexes: + new_req._build_indexes() + new_req._expand_special_tags(distro_qualified=distro_qualified) + return new_req + + def _expand_special_tags(self, distro_qualified=True): + if self.indexes: + for image in self.indexes: + # distro qualified is redundant with the default tag of an + # index/manifest as well as mainly needed for backwards + # compatibility something we don't want for indexes. + add_special_tags(image, distro_qualified=False) + else: + for image in self.images: + add_special_tags(image, distro_qualified=distro_qualified) + + def _build_indexes(self): + _indexes = {} + for image in self.images: + image_index = TargetIndex.from_image(image) + key = image_index.key() + if key in _indexes: + _indexes[key].merge(image_index) + else: + _indexes[key] = image_index + self.indexes = list(_indexes.values()) + + +def generate_request(cli): + """Given command line parameters with full image names or a matrix of + kind/pkg_source/distro_base/arch values generate request object containing + the target images or indexes to build and/or otherwise process. """ images = {} for img in cli.image or []: @@ -459,7 +633,9 @@ def generate_images(cli): repo_base=rc.find_base(distro_base), ) images[str(timg)] = timg - return list(images.values()) + return BuildRequest(images=images.values()).expanded( + indexes=cli.combined, distro_qualified=cli.distro_qualified + ) def add_special_tags(img, distro_qualified=True): @@ -471,28 +647,32 @@ def add_special_tags(img, distro_qualified=True): # to keep us compatible with older tagging schemes from earlier versions of # the project. _host_arch = host_arch() + arch_ok = img.supports_arch(_host_arch) if img.distro in [FEDORA, OPENSUSE]: - if img.arch == _host_arch and img.pkg_source == DEFAULT: + if arch_ok and img.pkg_source == DEFAULT: img.additional_tags.append((LATEST, QUAL_NONE)) - if img.arch == _host_arch and img.pkg_source == NIGHTLY: + if arch_ok and img.pkg_source == NIGHTLY: img.additional_tags.append((NIGHTLY, QUAL_NONE)) if not distro_qualified: return # skip creating "distro qualified" tags - if img.arch == _host_arch and img.pkg_source == DEFAULT: + if arch_ok and img.pkg_source == DEFAULT: img.additional_tags.append((f"{img.distro}-{LATEST}", QUAL_DISTRO)) - if img.arch == _host_arch and img.pkg_source == NIGHTLY: + if arch_ok and img.pkg_source == NIGHTLY: img.additional_tags.append((f"{img.distro}-{NIGHTLY}", QUAL_DISTRO)) -def build(cli, target): +def build(cli, target, rebuild=False): """Command to build images.""" build_file = pathlib.Path(f"{cli.buildfile_prefix}{target.flat_name()}") common_src = "./images/common" common_dst = str(kind_source_dir(target.name) / ".common") - logger.debug("Copying common tree: %r -> %r", common_src, common_dst) - shutil.copytree(common_src, common_dst, dirs_exist_ok=True) - container_build(cli, target) - cid = container_id(cli, target) + cid = maybe_container_id(cli, target) + logger.debug("target: %s, cid=%s, rebuild=%s", target, cid, rebuild) + if not cid or rebuild: + logger.debug("Copying common tree: %r -> %r", common_src, common_dst) + shutil.copytree(common_src, common_dst, dirs_exist_ok=True) + image_build(cli, target) + cid = container_id(cli, target) with open(build_file, "w") as fh: fh.write(f"{cid} {target.image_name()}\n") @@ -768,6 +948,13 @@ def main(): " will be created." ), ) + parser.add_argument( + "--combined", + action=argparse.BooleanOptionalAction, + default=False, + help=("Specify if manifests/image indexes should be created."), + ) + behaviors = parser.add_mutually_exclusive_group() behaviors.add_argument( "--push", @@ -828,17 +1015,23 @@ def main(): logging.basicConfig(level=cli.log_level) _action = cli.main_action if cli.main_action else build - imgs = [] + req = None try: - imgs = generate_images(cli) - for img in imgs: - add_special_tags(img, cli.distro_qualified) + req = generate_request(cli) + for img in req.images: logger.info("Image %s, extra tags: %s", img, img.additional_tags) _action(cli, img) + for index in req.indexes: + logger.info( + "Index (Manifest) %s, extra tags: %s", + index, + index.additional_tags, + ) + _action(cli, index) except subprocess.CalledProcessError as err: logger.error("Failed command: %s", _cmd_to_str(err.cmd)) sys.exit(err.returncode) - if not imgs: + if not req: logger.error("No images or image kinds supplied") sys.exit(2) From a7d8204f048bf4b818038792b2ae87694dc2488e Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Tue, 7 Jan 2025 10:56:21 -0500 Subject: [PATCH 10/15] hack/build-image: add rebuild command Add a --rebuild command to force building of images that already exist in local storage. Signed-off-by: John Mulligan --- hack/build-image | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/hack/build-image b/hack/build-image index 0ff64423..47d37771 100755 --- a/hack/build-image +++ b/hack/build-image @@ -677,6 +677,10 @@ def build(cli, target, rebuild=False): fh.write(f"{cid} {target.image_name()}\n") +def rebuild(cli, target): + return build(cli, target, rebuild=True) + + class QMatcher: """Push only tags that meet the specified criteria: all - all tags; @@ -994,6 +998,13 @@ def main(): " for a given FQIN. Requires FQIN to already exist locally." ), ) + behaviors.add_argument( + "--rebuild", + action="store_const", + dest="main_action", + const=rebuild, + help="Always rebuild images even if matching images exist locally.", + ) behaviors.add_argument( "--archive", metavar="LOCATION", From 508f51a77ff9ebe49dd8a120c2421b224a62ac77 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Tue, 7 Jan 2025 14:57:35 -0500 Subject: [PATCH 11/15] hack/build-image: container index/manifest push fixes Fix/improve the behavior of the script when --push is combined with --combined. Signed-off-by: John Mulligan --- hack/build-image | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/hack/build-image b/hack/build-image index 47d37771..5230d353 100755 --- a/hack/build-image +++ b/hack/build-image @@ -338,9 +338,14 @@ def create_common_container_engine_args(cli, target): return [str(a) for a in args] -def container_push(cli, push_name): +def container_push(cli, push_name, manifest=False): """Construct and execute a command to push a container image.""" - args = [container_engine(cli), "push", push_name] + eng = container_engine(cli) + if manifest: + args = [eng, "manifest", "push", "--all"] + else: + args = [eng, "push"] + args.append(push_name) run(cli, args, check=True) @@ -747,9 +752,13 @@ def push(cli, target): to_push.append((target.image_name(tag=tag), qual)) to_push.append((push_name, QUAL_FQIN)) qmatcher = cli.push_selected_tags or QMatcher("") + manifest = isinstance(target, TargetIndex) # is it a manifest push? for push_name, tag_qual in to_push: if qmatcher(tag_qual): - container_push(cli, push_name) + logger.debug( + "pushing named object: %s (manifest=%s)", push_name, manifest + ) + container_push(cli, push_name, manifest=manifest) def archive(cli, target, location): From 997aa749167419cd65dc470afa4b0eadd9546dd9 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Tue, 7 Jan 2025 15:14:48 -0500 Subject: [PATCH 12/15] hack/build-image: clean up logic around auto-building on push & archive The description of `exists` didn't match the code and now we can extend and clean things up in the presence of indexes. Signed-off-by: John Mulligan --- hack/build-image | 54 +++++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/hack/build-image b/hack/build-image index 5230d353..09864d97 100755 --- a/hack/build-image +++ b/hack/build-image @@ -125,6 +125,12 @@ QUAL_NONE = "unqualified" QUAL_DISTRO = "distro-qualified" QUAL_FQIN = "fqin" +# conditions +EXISTS = "exists" +AUTO = "auto" +AUTO_INDEX = "auto-index" +REBUILD = "rebuild" + _DISCOVERED_CONTAINER_ENGINES = [] @@ -733,15 +739,25 @@ class QMatcher: return False +def _autobuild(cli, target, is_index=None): + if is_index is None: + is_index = isinstance(target, TargetIndex) + if cli.condition == REBUILD: + rebuild(cli, target) + elif maybe_container_id(cli, target): + logger.debug("target item exists: %s", target) + elif cli.condition == AUTO or (cli.condition == AUTO_INDEX and is_index): + logger.debug("target item auto build: %s", target) + build(cli, target) + else: + log.error("no existing image or index: %s", target) + raise ValueError("not present", target) + + def push(cli, target): """Command to push images.""" - if cli.push_state == "rebuild": - build(cli, target) - if cli.push_state == "exists": - try: - container_id(cli, target) - except subprocess.CalledProcessError: - build(cli, target) + is_index = isinstance(target, TargetIndex) # is it a manifest push? + _autobuild(cli, target, is_index=is_index) to_push = [] push_name = target.image_name() @@ -752,22 +768,17 @@ def push(cli, target): to_push.append((target.image_name(tag=tag), qual)) to_push.append((push_name, QUAL_FQIN)) qmatcher = cli.push_selected_tags or QMatcher("") - manifest = isinstance(target, TargetIndex) # is it a manifest push? for push_name, tag_qual in to_push: if qmatcher(tag_qual): logger.debug( - "pushing named object: %s (manifest=%s)", push_name, manifest + "pushing named object: %s (manifest=%s)", push_name, is_index ) - container_push(cli, push_name, manifest=manifest) + container_push(cli, push_name, manifest=is_index) def archive(cli, target, location): """Write tarballs to archive location.""" - # reuse push_stage flag. TODO rename flag - if cli.push_state == "rebuild" or ( - cli.push_state == "exists" and not maybe_container_id(cli, target) - ): - build(cli, target) + _autobuild(cli, target) eng = container_engine(cli) fname = pathlib.Path(location) / f"{target.flat_name()}.tar" @@ -920,11 +931,16 @@ def main(): ) parser.add_argument( "--push-state", - choices=("exists", "rebuild"), - default="exists", + "--image-condition", + dest="condition", + choices=(EXISTS, AUTO, AUTO_INDEX, REBUILD), + default=EXISTS, help=( - "Only push if a state is met:" - "exists - image exists; rebuild - image must be rebuilt." + "Image state must be met before continuing:" + " exists - item already exists;" + " auto - automatically build missing image;" + " auto-index - automatically build missing indexes only;" + " rebuild - image must be rebuilt." ), ) parser.add_argument( From dd7ae7404ebd4755f2f65b1261979b5697f08df6 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Tue, 7 Jan 2025 15:15:25 -0500 Subject: [PATCH 13/15] hack/build-image: disable archive command for indexes Ignore index targets if `--combined` is passed along with `--archive=`. Signed-off-by: John Mulligan --- hack/build-image | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hack/build-image b/hack/build-image index 09864d97..5da59ae9 100755 --- a/hack/build-image +++ b/hack/build-image @@ -778,6 +778,8 @@ def push(cli, target): def archive(cli, target, location): """Write tarballs to archive location.""" + if isinstance(target, TargetIndex): + return # ignore indexes/manifests _autobuild(cli, target) eng = container_engine(cli) From 0ca5d8d3d636af7ea7255a0a54b3e2a860747b32 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Tue, 7 Jan 2025 16:51:18 -0500 Subject: [PATCH 14/15] hack/build-image: option to create images without unqualified tags Allow the script to skip creating unqualified tags on images (like 'nightly' or 'latest'). Signed-off-by: John Mulligan --- hack/build-image | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/hack/build-image b/hack/build-image index 5da59ae9..80b84b05 100755 --- a/hack/build-image +++ b/hack/build-image @@ -592,23 +592,33 @@ class BuildRequest: def __bool__(self): return bool(self.images or self.indexes) - def expanded(self, indexes=False, distro_qualified=True): + def expanded( + self, indexes=False, distro_qualified=True, unqualified=True + ): new_req = self.__class__(self.images, self.indexes) if indexes: new_req._build_indexes() - new_req._expand_special_tags(distro_qualified=distro_qualified) + new_req._expand_special_tags( + distro_qualified=distro_qualified, unqualified=unqualified + ) return new_req - def _expand_special_tags(self, distro_qualified=True): + def _expand_special_tags(self, distro_qualified=True, unqualified=True): if self.indexes: for image in self.indexes: # distro qualified is redundant with the default tag of an # index/manifest as well as mainly needed for backwards # compatibility something we don't want for indexes. - add_special_tags(image, distro_qualified=False) + add_special_tags( + image, distro_qualified=False, unqualified=unqualified + ) else: for image in self.images: - add_special_tags(image, distro_qualified=distro_qualified) + add_special_tags( + image, + distro_qualified=distro_qualified, + unqualified=unqualified, + ) def _build_indexes(self): _indexes = {} @@ -645,11 +655,13 @@ def generate_request(cli): ) images[str(timg)] = timg return BuildRequest(images=images.values()).expanded( - indexes=cli.combined, distro_qualified=cli.distro_qualified + indexes=cli.combined, + distro_qualified=cli.distro_qualified, + unqualified=cli.unqualified, ) -def add_special_tags(img, distro_qualified=True): +def add_special_tags(img, distro_qualified=True, unqualified=True): """Certain images have special tags. Given an image, add general (non-FQIN) tags to that image. """ @@ -659,7 +671,7 @@ def add_special_tags(img, distro_qualified=True): # the project. _host_arch = host_arch() arch_ok = img.supports_arch(_host_arch) - if img.distro in [FEDORA, OPENSUSE]: + if unqualified and img.distro in [FEDORA, OPENSUSE]: if arch_ok and img.pkg_source == DEFAULT: img.additional_tags.append((LATEST, QUAL_NONE)) if arch_ok and img.pkg_source == NIGHTLY: @@ -975,10 +987,18 @@ def main(): action=argparse.BooleanOptionalAction, default=True, help=( - "Specify if image tags like fedora-nightly or centos-latest" + "Specify if image tags like 'fedora-nightly' or 'centos-latest'" " will be created." ), ) + parser.add_argument( + "--unqualified", + action=argparse.BooleanOptionalAction, + default=True, + help=( + "Specify if image tags like 'nightly' or 'latest' will be created." + ), + ) parser.add_argument( "--combined", action=argparse.BooleanOptionalAction, From df6452858931f08ea5efdf56ecd5badb1d3fb0e8 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Tue, 7 Jan 2025 17:06:38 -0500 Subject: [PATCH 15/15] hack/build-image: workaround weird podman manifest push issue Signed-off-by: John Mulligan --- hack/build-image | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hack/build-image b/hack/build-image index 80b84b05..3f3145e1 100755 --- a/hack/build-image +++ b/hack/build-image @@ -351,6 +351,10 @@ def container_push(cli, push_name, manifest=False): args = [eng, "manifest", "push", "--all"] else: args = [eng, "push"] + if "podman" in eng: + # very strange bug? sometimes it fails to push without the format + # option. but I swear it also does work without it some times + args.append("--format=oci") args.append(push_name) run(cli, args, check=True)