diff --git a/build-utils/lib/XCAT/BuildUtils.pm b/build-utils/lib/XCAT/BuildUtils.pm index cdc558137..9bf034c1e 100644 --- a/build-utils/lib/XCAT/BuildUtils.pm +++ b/build-utils/lib/XCAT/BuildUtils.pm @@ -20,6 +20,7 @@ use File::Path qw(make_path remove_tree); use File::Temp qw(tempdir tempfile); use File::Slurper qw(read_text write_text); use POSIX qw(strftime); +use Sys::Hostname (); use Pod::Usage qw(pod2usage); use feature 'say'; @@ -32,7 +33,8 @@ our @EXPORT_OK = qw( pin_control_version rewrite_changelog_header reprepro_distributions reprepro_options lock_id_for take_build_lock - build_sources_dir prepare_build_sources_dir + build_sources_base build_sources_dir prepare_build_sources_dir + remove_build_sources_dir sweep_build_sources_dirs sh_quote clean_debian_residue git_revision backup_file restore_file sh sh_or_die usage @@ -972,44 +974,65 @@ sub genesis_log_errors { #------------------------------------------------------------------------------- -=head3 build_sources_dir +=head3 build_sources_base -Descriptions: The rpmbuild SOURCES directory for one package and one target. +Descriptions: The directory that holds the per-process rpmbuild SOURCES directories. Arguments: - $package - the package being built, e.g. xCAT - $target - the mock target, e.g. openeuler-24.03sp4-x86_64 - $uniqueext - the mock uniqueext, or undef - $home - the home directory; defaults to $ENV{HOME} + $home - the home directory; defaults to $ENV{HOME} + +Returns: the absolute path of the base directory. + +=cut + +#------------------------------------------------------------------------------- +sub build_sources_base { + my ($home) = @_; + $home = $ENV{HOME} unless defined $home && length $home; + die "build_sources_base: no home directory\n" unless defined $home && length $home; + return "$home/rpmbuild/sources"; +} + +#------------------------------------------------------------------------------- + +=head3 build_sources_dir + +Descriptions: The rpmbuild SOURCES directory of one build process. + +Arguments: + $base - the directory from build_sources_base + $pid - the pid of the process that stages; defaults to $$ + $host - the host name; defaults to this host Returns: the absolute path of the staging directory. =cut #------------------------------------------------------------------------------- -# Keyed like the mock chroot. buildrpms.pl forks a child per package and target, and -# mock --sources copies the whole directory, so one shared directory lets a peer's tar -# truncate an archive mid-copy. xCAT and xCATsn both write etc.tar.gz. +# Keyed by the process that stages. buildrpms.pl forks a child per package and target, +# and mock --sources copies the whole directory, so a shared directory lets a peer's tar +# truncate an archive mid-copy. A pid is unique only on one host, and $HOME can be on NFS. sub build_sources_dir { - my ($package, $target, $uniqueext, $home) = @_; + my ($base, $pid, $host) = @_; + die "build_sources_dir: no base directory\n" unless defined $base && length $base; + $pid = $$ unless defined $pid; + die "build_sources_dir: pid '$pid' is not a number\n" unless $pid =~ /^[0-9]+$/; + $host = _sources_host($host); + return "$base/SOURCES.$host.$pid"; +} - # An empty key names the shared directory again. - die "build_sources_dir: package is required\n" unless defined $package && length $package; - die "build_sources_dir: target is required\n" unless defined $target && length $target; - - $home = $ENV{HOME} unless defined $home && length $home; - die "build_sources_dir: no home directory\n" unless defined $home && length $home; - - my $key = "$package-$target"; - $key .= "-$uniqueext" if defined $uniqueext && length $uniqueext; - return "$home/rpmbuild/$key/SOURCES"; +sub _sources_host { + my ($host) = @_; + $host = Sys::Hostname::hostname() unless defined $host && length $host; + $host =~ s/[^A-Za-z0-9._-]/-/g; + return $host; } #------------------------------------------------------------------------------- =head3 prepare_build_sources_dir -Descriptions: Create the staging directory for one package and target, and return it. +Descriptions: Create the staging directory of one build process, empty, and return it. Arguments: the same as build_sources_dir. @@ -1020,9 +1043,75 @@ Returns: the absolute path of the staging directory. #------------------------------------------------------------------------------- sub prepare_build_sources_dir { my $dir = build_sources_dir(@_); + # A directory with this name belongs to a dead process that had the same pid. + remove_tree($dir) if -e $dir; make_path($dir); die "build_sources_dir: $dir was not created\n" unless -d $dir; return $dir; } +#------------------------------------------------------------------------------- + +=head3 remove_build_sources_dir + +Descriptions: Delete the staging directory of one build process. + +Arguments: the same as build_sources_dir. The caller makes sure the process has exited. + +Returns: true when the directory is gone. + +=cut + +#------------------------------------------------------------------------------- +sub remove_build_sources_dir { + my $dir = build_sources_dir(@_); + remove_tree($dir) if -e $dir; + return !-e $dir; +} + +#------------------------------------------------------------------------------- + +=head3 sweep_build_sources_dirs + +Descriptions: Delete the staging directories that processes on this host left behind + when they were killed before their parent could delete them. + +Arguments: + $base - the directory from build_sources_base + $host - the host name; defaults to this host + $alive - a sub that takes a pid and returns true when that process exists; + defaults to kill 0 + +Returns: the list of directories deleted. + +=cut + +#------------------------------------------------------------------------------- +sub sweep_build_sources_dirs { + my ($base, $host, $alive) = @_; + $host = _sources_host($host); + $alive //= \&_pid_alive; + opendir(my $dh, $base) or return (); + my @entries = readdir $dh; + closedir $dh; + + my @removed; + for my $entry (sort @entries) { + # Another host's pids say nothing about this host's processes. + my ($pid) = $entry =~ /^SOURCES\.\Q$host\E\.([0-9]+)$/ or next; + next if $alive->($pid); + remove_tree("$base/$entry"); + push @removed, "$base/$entry" unless -e "$base/$entry"; + } + return @removed; +} + +# EPERM means the process exists and belongs to another user. An unreaped zombie +# also answers here, and its parent deletes the directory when it reaps it. +sub _pid_alive { + my ($pid) = @_; + return 1 if kill 0, $pid; + return $!{EPERM} ? 1 : 0; +} + 1; diff --git a/buildrpms.pl b/buildrpms.pl index 22a1542bd..c350d0351 100755 --- a/buildrpms.pl +++ b/buildrpms.pl @@ -55,7 +55,8 @@ use File::Temp qw(tempdir tempfile); use FindBin qw($Bin); use lib "$Bin/build-utils/lib"; use XCAT::BuildUtils qw(git_revision source_date_epoch sh sh_or_die usage buildinfo_text - prepare_build_sources_dir stage_xcat_probe_sources stage_xcatsn_templates + build_sources_base prepare_build_sources_dir remove_build_sources_dir + sweep_build_sources_dirs stage_xcat_probe_sources stage_xcatsn_templates stage_genesis_base_sources write_script read_line targetarch_from_target openeuler_build_target openeuler_repo_subdir); @@ -69,8 +70,9 @@ use autodie; use autodie qw(cp); -# Set per package and target in buildall, so each forked child stages alone. +# Set in buildall to a directory named for the child's pid, so each forked child stages alone. my $SOURCES = ''; +my $SOURCES_BASE = build_sources_base(); # Ensure the rpmbuild tree exists. buildrpms stages source tarballs into $SOURCES, but it only # runs rpmdev-setuptree in the one-time env-setup path -- so on a host where that never ran (or # $HOME/rpmbuild was cleaned) source staging fails with "SOURCES/...: No such file or directory", @@ -527,8 +529,8 @@ EOF sub buildall { my ($pkg, $target) = @_; - # This process is the child for one pair, so the assignment cannot reach a peer. - $SOURCES = prepare_build_sources_dir($pkg, $target, $opts{mock_uniqueext}); + # The parent deletes this directory when it reaps the child. + $SOURCES = prepare_build_sources_dir($SOURCES_BASE, $$); createmockconfig($pkg, $target); buildsources($pkg, $target); buildspkgs($pkg, $target); @@ -938,7 +940,8 @@ sub abort_builds { warn "\n[buildrpms] caught SIG$sig: aborting -- signalling mock to self-clean...\n"; my @chroots = values %MOCK_INFLIGHT; kill 'TERM', mock_pids(@chroots); # mock unmounts + orphanKills itself - kill 'TERM', keys %MOCK_INFLIGHT; # unwind the ForkManager builders too + my @builders = keys %MOCK_INFLIGHT; + kill 'TERM', @builders; # unwind the ForkManager builders too my @mock; for (1 .. 30) { # wait for mock to finish its own cleanup @mock = mock_pids(@chroots); @@ -951,6 +954,17 @@ sub abort_builds { select undef, undef, undef, 2; sweep_mock_mounts(@chroots); } + # Delete the staging directory of each builder that has exited. A builder that is + # still alive keeps its directory, and the sweep in the next run deletes it. + for my $pid (@builders) { + my $reaped = 0; + for (1 .. 10) { + $reaped = waitpid($pid, POSIX::WNOHANG()); + last if $reaped; + select undef, undef, undef, 0.5; + } + remove_build_sources_dir($SOURCES_BASE, $pid) if $reaped == $pid; + } warn "[buildrpms] abort cleanup done\n"; # The re-raise below kills this process by signal, and END blocks do not run then. Release the # build lock here or a killed build strands it for every later run. @@ -999,6 +1013,9 @@ sub main { } } + # A builder killed with SIGKILL together with this parent leaves its directory behind. + sweep_build_sources_dirs($SOURCES_BASE); + my @rpms = product($opts{packages}, $opts{targets}); my $pm = Parallel::ForkManager->new($opts{nproc}); @@ -1009,6 +1026,7 @@ sub main { $pm->run_on_finish(sub { my ($pid, $exit_code, $ident, $exit_signal, $core_dump) = @_; delete $MOCK_INFLIGHT{$pid}; + remove_build_sources_dir($SOURCES_BASE, $pid); # A child that die()s exits non-zero; one killed by a SIGNAL (SIGKILL / OOM-killer) is reaped # with $exit_code==0 but $exit_signal!=0 (and maybe $core_dump). Checking $exit_code alone # would let an OOM-killed worker through and the parent would index+sign a partial repository diff --git a/xCAT-test/unit/buildrpms_source_only.t b/xCAT-test/unit/buildrpms_source_only.t index 9af6f850a..54ebe5c27 100644 --- a/xCAT-test/unit/buildrpms_source_only.t +++ b/xCAT-test/unit/buildrpms_source_only.t @@ -52,7 +52,7 @@ our @STAGES; sub buildsources { push @main::STAGES, 'buildsources'; } sub buildspkgs { push @main::STAGES, 'buildspkgs'; } sub buildpkgs { push @main::STAGES, 'buildpkgs'; } - # buildall assigns a staging directory per package and target. + # buildall assigns a staging directory per build process. sub prepare_build_sources_dir { return '/nonexistent/rpmbuild/stub/SOURCES'; } } @@ -64,6 +64,7 @@ my $harness = join "\n", 'use strict; use warnings;', 'our %opts;', 'our $SOURCES = \'\';', + 'our $SOURCES_BASE = \'/nonexistent/rpmbuild/sources\';', 'sub say { }', # write_repo_metadata_dir does real work past the guard; stop it there so the # test observes the guard and nothing else. diff --git a/xCAT-test/unit/buildrpms_sources_per_package.t b/xCAT-test/unit/buildrpms_sources_per_package.t index ce1f43268..0a97d9380 100644 --- a/xCAT-test/unit/buildrpms_sources_per_package.t +++ b/xCAT-test/unit/buildrpms_sources_per_package.t @@ -1,63 +1,92 @@ #!/usr/bin/env perl -# buildrpms.pl staged every source tarball into one $HOME/rpmbuild/SOURCES while forking a child -# per package and target, so a build could read a truncated archive: -# -# error: File /builddir/build/SOURCES/xCAT-test-2.20.0.tar.gz is smaller than 13 bytes -# -# Two packages of one target collide the same way: xCAT and xCATsn both write etc.tar.gz. +# buildrpms.pl forks a child per package and target. Each child stages its source +# tarballs into a directory named for its pid, and the parent deletes it after the child. use strict; use warnings; -use File::Spec (); +use File::Path qw(make_path); +use File::Slurper qw(write_text); use File::Temp qw(tempdir); use FindBin; use lib "$FindBin::Bin/../../build-utils/lib"; use Test::More; -use XCAT::BuildUtils qw(build_sources_dir prepare_build_sources_dir); +use XCAT::BuildUtils qw(build_sources_base build_sources_dir prepare_build_sources_dir + remove_build_sources_dir sweep_build_sources_dirs); my $home = tempdir(CLEANUP => 1); +my $base = build_sources_base($home); +is($base, "$home/rpmbuild/sources", 'the base directory is under the rpmbuild tree'); -# The key is the package AND the target. Either one alone leaves a pair sharing a directory. -my $a = build_sources_dir('xCAT', 'openeuler-24.03sp4-x86_64', 'ci', $home); -my $b = build_sources_dir('xCATsn', 'openeuler-24.03sp4-x86_64', 'ci', $home); -my $c = build_sources_dir('xCAT', 'openeuler-22.03sp4-x86_64', 'ci', $home); -isnt($a, $b, 'two packages of one target stage into different directories'); -isnt($a, $c, 'two targets of one package stage into different directories'); +# Two live processes on one host never have the same pid. The host name keeps two +# hosts apart when $HOME is on NFS. +my $a = build_sources_dir($base, 101, 'hosta'); +my $b = build_sources_dir($base, 102, 'hosta'); +my $c = build_sources_dir($base, 101, 'hostb'); +isnt($a, $b, 'two pids stage into different directories'); +isnt($a, $c, 'one pid on two hosts stages into different directories'); +is($a, "$base/SOURCES.hosta.101", 'the directory name carries the host and the pid'); +is(build_sources_dir($base, undef, 'hosta'), "$base/SOURCES.hosta.$$", + 'the pid defaults to the calling process'); -# Keyed like the mock chroot, so the staging directory and the chroot cannot disagree about -# which build owns which files. -is($a, "$home/rpmbuild/xCAT-openeuler-24.03sp4-x86_64-ci/SOURCES", - 'the directory is keyed like the mock chroot: package, target, uniqueext'); -is(build_sources_dir('xCAT', 'openeuler-24.03sp4-x86_64', '', $home), - "$home/rpmbuild/xCAT-openeuler-24.03sp4-x86_64/SOURCES", - 'an empty uniqueext adds no trailing separator'); -isnt(build_sources_dir('xCAT', 'el9-x86_64', 'ci', $home), - build_sources_dir('xCAT', 'el9-x86_64', 'other', $home), - 'two uniqueexts of one pair do not share, so two runs on one host cannot collide'); +ok(!eval { build_sources_dir($base, 'x1', 'hosta'); 1 }, 'a pid that is not a number is fatal'); +like($@, qr/build_sources_dir: pid 'x1' is not a number/, 'and the message names the pid'); +ok(!eval { build_sources_dir('', 101, 'hosta'); 1 }, 'an empty base is fatal'); +like($@, qr/build_sources_dir: no base directory/, 'and the message names the base'); -# A missing argument must not collapse the key back to the one shared directory. -for my $case (['package', undef, 'el9-x86_64'], ['target', 'xCAT', undef], - ['empty package', '', 'el9-x86_64'], ['empty target', 'xCAT', '']) { - my ($what, $pkg, $tg) = @$case; - my $died = eval { build_sources_dir($pkg, $tg, 'ci', $home); 1 } ? 0 : 1; - ok($died, "a missing $what is fatal rather than a shared directory"); - like($@, qr/build_sources_dir: (?:package|target) is required/, - "the message names the missing argument for $what"); +# A directory with the same name is left by a dead process that had this pid. +make_path($a); +write_text("$a/stale.tar.gz", "old\n"); +my $made = prepare_build_sources_dir($base, 101, 'hosta'); +is($made, $a, 'prepare returns the directory of that pid'); +ok(-d $made, 'the staging directory is created'); +opendir(my $dh, $made) or die "cannot read $made: $!\n"; +my @left = grep { !/^\.\.?$/ } readdir $dh; +closedir $dh; +is_deeply(\@left, [], 'the staging directory is created empty'); +remove_build_sources_dir($base, 101, 'hosta'); + +# The parent calls remove_build_sources_dir when it reaps a child, whatever its exit. +for my $how ('success', 'failure') { + my $dir = prepare_build_sources_dir($base, 201, 'hosta'); + write_text("$dir/xCAT-test-2.20.0.tar.gz", "payload\n"); + my $pid = fork // die "fork: $!\n"; + if ($pid == 0) { + require POSIX; + POSIX::_exit($how eq 'success' ? 0 : 1); + } + waitpid($pid, 0); + is($? >> 8, $how eq 'success' ? 0 : 1, "the child exits for $how"); + ok(remove_build_sources_dir($base, 201, 'hosta'), "remove reports the directory gone after $how"); + ok(!-e $dir, "the directory is deleted after $how"); } +ok(remove_build_sources_dir($base, 202, 'hosta'), 'removing a directory that does not exist is not an error'); -# prepare_build_sources_dir is what buildall calls: it must CREATE the directory, because -# staging into a path that does not exist is how the shared tree came to be created up front. -my $made = prepare_build_sources_dir('xCAT-test', 'openeuler-24.03sp4-x86_64', 'ci', $home); -ok(-d $made, 'the staging directory is created, not merely named'); -my $again = prepare_build_sources_dir('xCAT-test', 'openeuler-24.03sp4-x86_64', 'ci', $home); -is($again, $made, 'a second call is idempotent and returns the same directory'); +# The sweep deletes the directories of dead pids on this host only. +my %alive = (301 => 1); +my $is_alive = sub { $alive{ $_[0] } }; +my $dead = prepare_build_sources_dir($base, 300, 'hosta'); +my $live = prepare_build_sources_dir($base, 301, 'hosta'); +my $other = prepare_build_sources_dir($base, 300, 'hostb'); +make_path("$base/unrelated"); +my @swept = sweep_build_sources_dirs($base, 'hosta', $is_alive); +is_deeply(\@swept, [$dead], 'the sweep reports the directory of the dead pid'); +ok(!-e $dead, 'the directory of a dead pid is deleted'); +ok(-d $live, 'the directory of a live pid is kept'); +ok(-d $other, 'the directory of another host is kept'); +ok(-d "$base/unrelated", 'a directory with another name is kept'); -# Nothing writes beside the per-pair directories: a path that escaped $home/rpmbuild would put -# one build's sources where another build reads them. -my @dirs = map { build_sources_dir($_, 'el9-x86_64', 'ci', $home) } qw(xCAT xCATsn xCAT-test); -for my $d (@dirs) { - like($d, qr{^\Q$home/rpmbuild/\E[^/]+/SOURCES$}, "$d is one level under rpmbuild"); -} +# The default liveness check: this process is alive, a reaped child is not. +my $own = prepare_build_sources_dir($base, $$, 'hosta'); +my $gone = fork // die "fork: $!\n"; +if ($gone == 0) { require POSIX; POSIX::_exit(0) } +waitpid($gone, 0); +my $reaped = prepare_build_sources_dir($base, $gone, 'hosta'); +sweep_build_sources_dirs($base, 'hosta'); +ok(-d $own, 'the default check keeps the directory of a running process'); +ok(!-e $reaped, 'the default check deletes the directory of a reaped process'); + +is_deeply([sweep_build_sources_dirs("$home/absent", 'hosta', $is_alive)], [], + 'a missing base directory is not an error'); done_testing;