konflux: Decouple lockfile generation and merging

Refactor `konflux-rpm-lockfile` to support parallel, per-arch builds.
The script is now split into `generate` and `merge` subcommands.

`cmd-fetch` now calls `generate` to create arch-specific lockfile
(e.g., `x86_64.rpms.lock.yaml`). It also look for an existing lockfile
and call generate in that case.

The `merge` subcommand combines these files in one, while injecting
overrides. The override mechanism allow developpers to add extra RPMs
needed in the build, e.g. in `buildroot-prep` [1]

The pipeline will call merge after running `generate` on each builder.

[1] coreos/fedora-coreos-config@fb167ed
See: #4298 (comment)

Author: jbtrystram

src/cmd-fetch

@@ -40,6 +40,7 @@ Usage: coreos-assembler fetch --help
   --with-cosa-overrides    Don't ignore cosa overrides in `overrides/rpm`
   --autolock=VERSION       If no base lockfile used, create one from any arch build of `VERSION`
   --konflux                Generate hermeto lockfile for Konflux derived from the rpm-ostree lockfiles.
+                           Auto enabled if `rpms.lock.yaml` is found in the config directory.
 EOF
 }
 
@@ -109,6 +110,7 @@ fi
 
 prepare_build
 
+
 lock_args=
 extra_args=
 
@@ -181,9 +183,10 @@ if [ -n "${UPDATE_LOCKFILE}" ]; then
     echo "Wrote out lockfile ${outfile}"
 fi
 
-if [ -n "${KONFLUX}" ]; then
+KONFLUX_LOCKFILE=rpms.lock.yaml
+if [ -n "${KONFLUX}" ] || [ -f "${configdir}/${KONFLUX_LOCKFILE}" ]; then
    echo "Generating hermeto lockfile..."
-   /usr/lib/coreos-assembler/konflux-rpm-lockfile "${flattened_manifest}" --context "${configdir}" --output "${tmprepo}/tmp/rpms.lock.yaml" --arch all
-   (cd "${workdir}" && mv -f "${tmprepo}/tmp/rpms.lock.yaml" "konflux-rpms-lock.yaml")
-   echo "Wrote out hermeto lockfile: konflux-rpms-lock.yaml"
+   /usr/lib/coreos-assembler/konflux-rpm-lockfile generate "${flattened_manifest}" --context "${configdir}" --output "${tmprepo}/tmp/${arch}.${KONFLUX_LOCKFILE}"
+   mv -f "${tmprepo}/tmp/${arch}.${KONFLUX_LOCKFILE}" "${configdir}/${arch}.${KONFLUX_LOCKFILE}"
+   echo "Wrote out hermeto (konflux) lockfile: ${configdir}/${arch}.${KONFLUX_LOCKFILE}"
 fi

src/cosalib/cmdlib.py

@@ -576,28 +576,3 @@ def ncpu():
     '''Return the number of usable CPUs we have for parallelism.'''
     return int(subprocess.check_output(['kola', 'ncpu']))
 
-
-def get_locked_nevras(srcdir, arch=None):
-    """
-    Gathers all locked packages from the manifest-lock files.
-    The return format can be a dictionary of {pkgname: evr}
-    """
-    if not arch:
-        arch = get_basearch()
-    lockfile_path = os.path.join(srcdir, f"manifest-lock.{arch}.json")
-    overrides_path = os.path.join(srcdir, "manifest-lock.overrides.yaml")
-    overrides_arch_path = os.path.join(srcdir, f"manifest-lock.overrides.{arch}.json")
-
-    locks = {}
-    for path in [lockfile_path, overrides_path, overrides_arch_path]:
-        if os.path.exists(path):
-            with open(path, encoding='utf-8') as f:
-                if path.endswith('.yaml'):
-                    data = yaml.safe_load(f)
-                else:
-                    data = json.load(f)
-                # this essentially re-implements the merge semantics of rpm-ostree
-                locks.update({pkgname: v.get('evra') or v.get('evr')
-                              for (pkgname, v) in data['packages'].items()})
-
-    return locks

src/konflux-rpm-lockfile

@@ -8,8 +8,7 @@ import subprocess
 import yaml
 
 sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
-from cosalib.cmdlib import get_locked_nevras, get_basearch
-
+from cosalib.cmdlib import get_basearch
 
 def format_packages_with_repoid(pkgs, repos):
     """
@@ -32,10 +31,8 @@ def format_packages_with_repoid(pkgs, repos):
     # This is needed because rpm-ostree uses the full list of repos to
     # resolve packages and errors out if a repository is missing.
     repo_numbers = len(local_repos)
-    i = 0
-    for pkg in pkgs:
+    for i, pkg in enumerate(pkgs):
         packages.append({"url": pkg, "repoid": local_repos[i % repo_numbers]})
-        i += 1
     return packages
 
 
@@ -62,33 +59,38 @@ def write_hermeto_lockfile(arch_packages, repos):
     return lockfile
 
 
-def merge_lockfiles(base_lockfile, override_lockfile):
+def merge_lockfiles(base_lockfile, next_lockfile, override=False):
     """
-    Merges an override lockfile into a base lockfile.
+    Merges a lockfile into a base lockfile.
+
+    If is_override is True, it will only add packages to existing
+    architectures. Otherwise, it will add new architectures.
     """
-    if not override_lockfile:
+    if not next_lockfile:
         return base_lockfile
 
     # Create a dictionary for base arches for easy lookup
     base_arches = {arch['arch']: arch for arch in base_lockfile.get('arches', [])}
 
-    override = override_lockfile.get('arches', [])
-    if not override:
+    next_arches_list = next_lockfile.get('arches', [])
+    if not next_arches_list:
         return base_lockfile
 
-    for override_entry in override:
-        # override_entry is a dict like {'arch': x86_64','packages': [...]}
-        if not isinstance(override_entry, dict):
+    for next_arch_entry in next_arches_list:
+        if not isinstance(next_arch_entry, dict):
+            continue
+        arch = next_arch_entry.get('arch', None)
+        if not arch:
             continue
-        arch = override_entry.get('arch', None)
-        override_packages = override_entry.get('packages', [])
+
+        next_packages = next_arch_entry.get('packages', [])
         if arch in base_arches:
-            # Merge packages
+            # Arch exists, merge packages
             base_packages = base_arches[arch].get('packages', [])
-            base_packages += override_packages
-        else:
-            # Add the arch from the override file
-            base_arches[arch] = override_packages
+            base_packages += next_packages
+        elif not override:
+            # Arch is new and this is not an override, so add it
+            base_arches[arch] = next_arch_entry
 
     # Reconstruct the arches list
     base_lockfile['arches'] = list(base_arches.values())
@@ -103,7 +105,7 @@ def query_packages_location(locks, repoquery_args):
     if not locks:
         return pkg_urls
 
-    locked_nevras = [f'{k}-{v}' for (k, v) in locks.items()]
+    locked_nevras = [f'{k}-{v.get('evra', '')}' for (k, v) in locks.items()]
     cmd = ['dnf', 'repoquery'] + locked_nevras + repoquery_args
     result = subprocess.check_output(cmd, text=True)
 
@@ -119,21 +121,37 @@ def query_packages_location(locks, repoquery_args):
             if name not in processed_urls:
                 processed_urls[name] = url
     pkg_urls = list(processed_urls.values())
-    # sanity check all the packages got resolved
-    if len(pkg_urls) < len(locked_nevras):
+    # sanity check all the locked packages got resolved
+    if len(pkg_urls) != len(locked_nevras):
         print("Some packages from the lockfile could not be resolved. The rpm-ostree lockfile is probably out of date.")
-        for name in locks.keys():
-            if name not in processed_urls:
-                print(f"could not resolve package {name}")
         sys.exit(1)
 
+    print(f"Done. Resolved location for {len(pkg_urls)} packages.")
     return pkg_urls
 
+def get_locked_nevras(srcdir, arch):
+
+    path = os.path.join(srcdir, f"manifest-lock.{arch}.json")
 
-def generate_lockfile(contextdir, manifest, output_path, arches):
+    data = {}
+    if os.path.exists(path):
+        with open(path, encoding='utf-8') as f:
+            data = json.load(f)
+    else:
+        print(f"rpm-ostree lockfile not found at {path}")
+
+    return data.get('packages', [])
+
+
+def generate_main(args):
     """
     Generates the cachi2/hermeto RPM lock file.
     """
+    contextdir = args.context
+    manifest = os.path.abspath(args.manifest)
+    output_path = args.output
+    arches = args.arch
+
     if not arches:
         arches_to_resolve = [get_basearch()]
     elif 'all' in arches:
@@ -151,7 +169,7 @@ def generate_lockfile(contextdir, manifest, output_path, arches):
     repos = manifest_data.get('repos', [])
     repos += manifest_data.get('lockfile-repos', [])
 
-    repoquery_args = ["--queryformat", "%{name} %{location}\n", "--disablerepo=*", "--refresh"]
+    repoquery_args = ["--queryformat", "%{name} %{location}\n", "--disablerepo=*", "--refresh", "--quiet"]
     # Tell dnf to load repos files from $contextdir
     repoquery_args.extend([f"--setopt=reposdir={contextdir}"])
 
@@ -162,67 +180,126 @@ def generate_lockfile(contextdir, manifest, output_path, arches):
     for arch in arches_to_resolve:
         locks = get_locked_nevras(contextdir, arch)
         if not locks:
-            print(f"This tool derive the konflux lockfile from rpm-ostree lockfiles. No manifest-lock exist for {arch} in {contextdir}")
+            print(f"This tool derive the konflux lockfile from rpm-ostree lockfiles. Empty manifest-lock for {arch} in {contextdir}")
             sys.exit(1)
         print(f"Resolving packages for {arch}...")
-        # append noarch as well because otherwise tose packages get excluded from results
-        # We use --forcearch here because otherwise dnf still respect the system basearch
-        # we have to specify both --arch and --forcearch to get both result for $arch and $noarch
-        args_arch = ['--forcearch', arch, '--arch', arch, '--arch', 'noarch']
-        pkg_urls = query_packages_location(locks, repoquery_args + args_arch)
+        arch_args = []
+        if arch is not get_basearch():
+            # append noarch as well because otherwise those packages get excluded from results
+            # We use --forcearch here because otherwise dnf still respect the system basearch
+            # we have to specify both --arch and --forcearch to get both result for $arch and $noarch
+            arch_args = ['--forcearch', arch, '--arch', arch, '--arch', 'noarch']
+        pkg_urls = query_packages_location(locks, repoquery_args + arch_args)
         packages.append({'arch': arch, 'packages': pkg_urls})
 
     lockfile = write_hermeto_lockfile(packages, repos)
 
-    override_path = os.path.join(contextdir, 'konflux-lockfile-override.yaml')
-    if os.path.exists(override_path):
+    try:
+        with open(output_path, 'w', encoding='utf-8') as f:
+            yaml.safe_dump(lockfile, f, default_flow_style=False)
+    except IOError as e:
+        print(f"\u274c Error: Could not write to output file '{output_path}'. Reason: {e}")
+        sys.exit(1)
+
+
+def merge_main(args):
+    """
+    Merges multiple lockfiles into one, optionally applying an override file.
+    """
+    if not args.input:
+        print("Error: at least one input file is required for merging.", file=sys.stderr)
+        sys.exit(1)
+
+    try:
+        with open(args.input[0], 'r', encoding='utf-8') as f:
+            base_lockfile = yaml.safe_load(f)
+    except (IOError, yaml.YAMLError) as e:
+        print(f"Error reading base lockfile {args.input[0]}: {e}", file=sys.stderr)
+        sys.exit(1)
+
+    for subsequent_file in args.input[1:]:
         try:
-            with open(override_path, 'r', encoding="utf8") as f:
+            with open(subsequent_file, 'r', encoding='utf-8') as f:
+                next_lockfile = yaml.safe_load(f)
+            base_lockfile = merge_lockfiles(base_lockfile, next_lockfile)
+        except (IOError, yaml.YAMLError) as e:
+            print(f"Error reading or merging {subsequent_file}: {e}", file=sys.stderr)
+            sys.exit(1)
+
+    if os.path.exists(args.override):
+        try:
+            with open(args.override, 'r', encoding="utf8") as f:
                 override_data = yaml.safe_load(f)
-            print(f"Merging override from {override_path}")
-            lockfile = merge_lockfiles(lockfile, override_data)
+            print(f"Merging override from {args.override}")
+            base_lockfile = merge_lockfiles(base_lockfile, override_data, override=True)
         except (IOError, yaml.YAMLError) as e:
-            print(f"\u274c Error: Could not read or parse override file '{override_path}'. Reason: {e}")
+            print(f"Error reading or parsing override file '{args.override}': {e}", file=sys.stderr)
             sys.exit(1)
 
     try:
-        with open(output_path, 'w', encoding='utf-8') as f:
-            yaml.safe_dump(lockfile, f, default_flow_style=False)
+        with open(args.output, 'w', encoding='utf-8') as f:
+            yaml.safe_dump(base_lockfile, f, default_flow_style=False)
+        print(f"Successfully merged lockfiles to {args.output}")
     except IOError as e:
-        print(f"\u274c Error: Could not write to output file '{output_path}'. Reason: {e}")
+        print(f"Error writing to output file '{args.output}': {e}", file=sys.stderr)
         sys.exit(1)
 
 
 if __name__ == "__main__":
     parser = argparse.ArgumentParser(
-        description="Generate hermeto lock files."
+        description="Generate and merge hermeto lock files."
     )
+    subparsers = parser.add_subparsers(dest='command', required=True)
 
-    parser.add_argument(
+    # GENERATE command
+    parser_generate = subparsers.add_parser(
+        'generate',
+        help='Resolve RPMs and generate a lockfile for one or more architectures.'
+    )
+    parser_generate.add_argument(
         'manifest',
         help='Path to the flattened rpm-ostree manifest (e.g., tmp/manifest.json)'
     )
-
-    parser.add_argument(
+    parser_generate.add_argument(
         '--context',
         default='.',
         help="Path to the directory containing repofiles and lockfiles. (default: '.')"
     )
-
-    parser.add_argument(
+    parser_generate.add_argument(
         '--output',
         default='./rpms.lock.yaml',
         help="Path for the hermeto lockfile. (default: './rpms.lock.yaml')"
     )
-
-    parser.add_argument(
+    parser_generate.add_argument(
         '--arch',
         action='append',
         choices=['x86_64', 'aarch64', 's390x', 'ppc64le', 'all'],
         help="The architecture to resolve. Can be specified multiple times. 'all' resolves all architectures."
     )
+    parser_generate.set_defaults(func=generate_main)
 
-    args = parser.parse_args()
+    # MERGE command
+    parser_merge = subparsers.add_parser(
+        'merge',
+        help='Merge multiple architecture-specific lockfiles into a single file.'
+    )
+    parser_merge.add_argument(
+        '--input',
+        nargs='+',
+        required=True,
+        help='One or more input lockfiles to merge.'
+    )
+    parser_merge.add_argument(
+        '--output',
+        default='./rpms.lock.yaml',
+        help="Path for the merged lockfile. (default: './rpms.lock.yaml')"
+    )
+    parser_merge.add_argument(
+        '--override',
+        default='konflux-lockfile-override.yaml',
+        help="Path to an override file. (default: 'konflux-lockfile-override.yaml')"
+    )
+    parser_merge.set_defaults(func=merge_main)
 
-    manifest_abs_path = os.path.abspath(args.manifest)
-    generate_lockfile(args.context, manifest_abs_path, args.output, args.arch)
+    args = parser.parse_args()
+    args.func(args)