From 8823065ed991a08ce121655cf5f9ec197edf7dc0 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Mon, 28 Sep 2026 11:33:20 -0300 Subject: [PATCH] fix(xcat-dep): publish and finalize take locks of cells they do not own The apt publish took only the run lock of the host arch, and then read the staging of every expected arch. A ppc64el run could refill its staging while the publish assembled from it. Finalize ran on one host and took the cell locks of both arches. If it died, only that host could reclaim the locks of the other arch's cells, and the next builds of those cells waited on them and failed. A publish now takes the run lock of every expected arch, in name order, before the publish lock, and waits for them up to --publish-lock-wait. --finalize-arch limits finalize to the cells of one arch: it writes, locks, re-indexes and verifies only those, and reads the other arches. Without the option, finalize writes both arches as before. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- MockBuildUtils.pm | 13 +++++++++++-- mockbuild-all.pl | 15 ++++++++++++--- sbuild-all.pl | 22 +++++++++++++++------- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/MockBuildUtils.pm b/MockBuildUtils.pm index 791f5f1..3f2bf85 100644 --- a/MockBuildUtils.pm +++ b/MockBuildUtils.pm @@ -549,6 +549,15 @@ sub finalize_xcat_dep { my ($x86_64_repo, $ppc64le_repo, %opt) = @_; my $sign = $opt{sign}; my $reindex = $opt{reindex}; + # The arches whose cells this run writes. The others are read only: each host finalizes the + # cells it deploys, so it never holds a cell lock that only another host could reclaim. + my %known = map { $_->{arch} => 1 } @GENESIS_ARCHES; + my @only = @{ $opt{only} // [ map { $_->{arch} } @GENESIS_ARCHES ] }; + for my $a (@only) { + die "FATAL: [finalize] no cross-arch genesis for arch '$a'\n" unless $known{$a}; + } + my %write = map { $_ => 1 } @only; + my @dst_arches = grep { $write{ $_->{arch} } } @GENESIS_ARCHES; print_step('Finalize xcat-dep: cross-arch genesis-base provisioning (issue #7610)'); print "x86_64-repo: $x86_64_repo\n"; print "ppc64le-repo: $ppc64le_repo\n"; @@ -589,7 +598,7 @@ sub finalize_xcat_dep { # N-way cross-copy: put each arch's genesis into EVERY other arch's repo dir. my @summary; for my $src (@GENESIS_ARCHES) { - for my $dst (@GENESIS_ARCHES) { + for my $dst (@dst_arches) { next if $src->{arch} eq $dst->{arch}; my $n = cross_copy_genesis($adir{$src->{arch}}, $adir{$dst->{arch}}, $src->{tarch}, $sign); push @summary, "$n $src->{tarch} -> $dst->{arch}"; @@ -600,7 +609,7 @@ sub finalize_xcat_dep { # rpm on disk (so cross_copy_genesis now returns 0) yet ABSENT from repomd.xml -- which no # signature gate catches. Re-indexing is cheap (tiny repos) and idempotent, and heals that # partial state; skipped only when no signer/indexer was injected. - if ($reindex) { $reindex->($adir{$_->{arch}}) for @GENESIS_ARCHES; } + if ($reindex) { $reindex->($adir{$_->{arch}}) for @dst_arches; } print "[finalize] $osdir: " . join(', ', @summary) . "\n"; $pairs++; } diff --git a/mockbuild-all.pl b/mockbuild-all.pl index f7957a2..01374ec 100755 --- a/mockbuild-all.pl +++ b/mockbuild-all.pl @@ -136,6 +136,7 @@ my $try_unlock_timeout = 0; # --finalize-xcat-dep: post-build cross-arch genesis provisioning (issue #7610). Takes the two # per-arch repo roots and cross-populates the noarch xCAT-genesis-base between them. my $finalize_xcat_dep = 0; +my @finalize_arch; my $x86_64_repo = ''; my $ppc64le_repo = ''; # --verify-repo=: standalone, build-free completeness + signature gate over one already-built @@ -163,6 +164,7 @@ GetOptions( 'gpg-home=s' => \$gpg_home, 'try-unlock-timeout=i' => \$try_unlock_timeout, 'finalize-xcat-dep!' => \$finalize_xcat_dep, + 'finalize-arch=s' => \@finalize_arch, 'x86_64-repo=s' => \$x86_64_repo, 'ppc64le-repo=s' => \$ppc64le_repo, 'verify-repo=s' => \$verify_repo, @@ -256,15 +258,19 @@ if ($finalize_xcat_dep) { my $ppc = abs_path($ppc64le_repo) or die "--ppc64le-repo '$ppc64le_repo' not found\n"; die "--x86_64-repo '$x86' is not a directory\n" if !-d $x86; die "--ppc64le-repo '$ppc' is not a directory\n" if !-d $ppc; - # finalize rewrites the per-arch cells a build deploys, so it takes the same cell locks. + # finalize rewrites the per-arch cells a build deploys, so it takes the same cell locks. With + # --finalize-arch it writes, and locks, only the cells of those arches. + @finalize_arch = map { split /[\s,]+/ } @finalize_arch; + @finalize_arch = qw(x86_64 ppc64le) unless @finalize_arch; my %cell; for my $root ($x86, $ppc) { - $cell{ abs_path($_) } = 1 for grep { -d } (glob("$root/*/x86_64"), glob("$root/*/ppc64le")); + $cell{ abs_path($_) } = 1 for grep { -d } map { glob("$root/*/$_") } @finalize_arch; } take_lock(cell_lock_path($_), 'repository cell lock') for sort keys %cell; # Inject the per-rpm gpg re-sign and the repo re-index as callbacks so the finalize logic in # MockBuildUtils stays free of this script's gpg/createrepo state. finalize_xcat_dep($x86, $ppc, + only => \@finalize_arch, sign => ($gpg_sign ? sub { my ($rpm) = @_; local $ENV{GNUPGHOME} = $gpg_home if $gpg_home; @@ -281,7 +287,7 @@ if ($finalize_xcat_dep) { unless ($no_verify_repo) { my %seen; for my $root ($x86, $ppc) { - my @cells = (glob("$root/rh*/x86_64"), glob("$root/rh*/ppc64le")); + my @cells = map { glob("$root/rh*/$_") } @finalize_arch; for my $d (sort @cells) { next unless -d $d; my $abs = abs_path($d); @@ -1459,6 +1465,9 @@ Options: ppc64le repo (dropping any stale foreign-arch genesis), then re-indexes + re-signs. Restores the 2.17 cross-arch genesis (issue #7610). Honors --gpg-sign/--gpg-key-name/--gpg-home. Use alone. + --finalize-arch ARCH (finalize) Write, lock and verify only the cells of ARCH (x86_64 or + ppc64le; repeatable). Run it on the host that builds ARCH, so a + dead finalize leaves locks that host can reclaim. Default: both --x86_64-repo PATH (finalize) x86_64 repo root holding /x86_64 (e.g. rh9/x86_64) --ppc64le-repo PATH (finalize) ppc64le repo root holding /ppc64le --verify-repo PATH Standalone completeness + signature gate over the per-target repo at PATH diff --git a/sbuild-all.pl b/sbuild-all.pl index 1300281..132b086 100755 --- a/sbuild-all.pl +++ b/sbuild-all.pl @@ -113,7 +113,7 @@ my @genesis_debs; # native xcat-genesis-base- deb(s): p # better than failing the run. my $PUBLISH_LOCK_WAIT = 1800; # The per-arch run lock, held until the END block releases it. -my $RUN_LOCK; +my @RUN_LOCKS; # Builder map: manifest binary-package name -> the in-tree package dir that carries /sbuild.pl # and the maintained debian/. (goconserver's dir == its binary name.) @@ -339,13 +339,21 @@ for my $cn (@dist_list) { my $staging = "$output_root/staging"; unless ($dry_run) { make_path($staging); } -# Fail-fast PER-ARCH run lock. The amd64 and ppc64el stages of one run build concurrently on their -# own hosts against one --output-root, so the lock is per arch: a second run of the same arch stops. -# flock on the shared tree fails with ENOTSUPP through the NFS re-export, so this is an -# XCAT::NFSLock. Not taken under --dry-run. +# PER-ARCH run locks. The amd64 and ppc64el stages of one run build concurrently on their own hosts +# against one --output-root, so the lock is per arch: a second run of the same arch stops. A publish +# reads the staging of every expected arch, so it also waits for the run lock of each one. The locks +# are taken in name order. flock on the shared tree fails with ENOTSUPP through the NFS re-export, so +# these are XCAT::NFSLock. Not taken under --dry-run. unless ($dry_run) { make_path($output_root); - $RUN_LOCK = XCAT::NFSLock->acquire("$output_root/.sbuild-all.$arch.nfslock", label => "sbuild-all ($arch) run lock"); + my %lock_arch = ($arch => 1); + if ($publish) { $lock_arch{$_} = 1 for @{ resolve_expect_arches('publish', $apt_dir) }; } + for my $a (sort keys %lock_arch) { + # A build of this arch fails fast. A publish queues behind the builders it reads from. + my $wait = ($publish && !($a eq $arch && !$skip_build)) ? $PUBLISH_LOCK_WAIT : 0; + push(@RUN_LOCKS, XCAT::NFSLock->acquire("$output_root/.sbuild-all.$a.nfslock", + timeout => $wait, label => "sbuild-all ($a) run lock")); + } } # The per-package builders are separate processes with their own CLI, so the bound travels to them in @@ -1024,7 +1032,7 @@ sub acquire_publish_lock { END { $PUBLISH_LOCK->release if $PUBLISH_LOCK; - $RUN_LOCK->release if $RUN_LOCK; + $_->release for reverse(@RUN_LOCKS); } # assemble_into($dir, $expect_arches): (re)assemble every --dists codename inside $dir from the