collect_rpms copied every binary rpm a builder produced. The syslinux builder
also produces syslinux, syslinux-extlinux and their debug rpms for the chroot
it runs in, so a forcearch target that builds the noarch boot loaders in the
native x86_64 chroot would have published x86_64 rpms in the riscv64 cell.
The completeness gate checks names and pins only, so it would have passed.
Keep an rpm only when it is noarch or carries the cell's architecture, read
from the rpm header. The rule lives in MockBuildUtils as rpm_in_cell.
Two CD runs died at compile time inside XCAT::BuildUtils because a builder was
missing a Perl module the script loads: perl-File-Slurper on xcat-master-ub and
perl-IPC-Cmd on xcat-master-ppc. Both surfaced as "Can't locate ... in @INC" in
the middle of a build, and both were fixed by hand -- so the next unprovisioned
host fails the same way, and BUILD.md's install line can drift from what the code
actually requires.
--install-deps installs this host's prerequisites and exits: the toolchain plus
the modules, through dnf or zypper as the host's ID dictates. It then LOADS each
module and fails naming any that is still missing, rather than trusting the
package manager's exit code -- a package that installs cleanly but leaves the
module unusable is exactly the failure this exists to prevent.
The list and the command are pure functions in MockBuildUtils, so the decision is
unit-tested (package sets per family, the right installer non-interactively, and
the probe reporting only what genuinely cannot be loaded); the side effect stays
in the caller. Run on xcat-master and xcat-master-ppc: both report every module
present. xcat-master-suse is unreachable and still needs it.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Three review follow-ups on the repo validation gate:
1. Full EPOCH:VERSION-RELEASE validation. The gate compared only %{VERSION}, so
xCAT-genesis-base=2.* accepted a pre-2.18 (2.17.x) genesis even though
xCAT-genesis-scripts Requires >= 2:2.18.0, and it could not enforce release
floors like perl-IO-Stty >= 0.04-5. Manifest pins now also accept an EVR
constraint (>=, >, <=, <, = followed by [epoch:]version[-release]); the built
rpm's full EVR is compared with rpm's own algorithm (rpm.vercmp via the lua
binding, injected into the pure evr_cmp, which composes epoch/version/release).
genesis-base is pinned >= 2:2.18.0 and perl-IO-Stty >= 0.04-5. rpm_evr also
catches release-level stale-artifact accumulation that rpm_version (VERSION
dedup) missed.
2. RPM-native crypto verification. The per-rpm gate extracted the header signer
id but did not verify digests/signatures. It now also runs `rpmkeys --checksig`
against an isolated keyring holding only the signing key (exported from the gpg
home), so every rpm's header/payload digests AND the signature-by-this-key are
cryptographically verified; the signer-id origin check is kept alongside.
3. repo_gpgcheck. The generated xcat-dep.repo set only gpgcheck=1; add
repo_gpgcheck=1 (mirroring gpgcheck) so clients enforce the detached
repomd.xml.asc signature that sign_and_index_repo already produces.
Validated: unit tests for parse_evr/evr_constraint_ok (rpm's real vercmp; the
reviewer's 2.17.9-rejected, 0.04-4-rejected, epoch-enforced cases) + checksig
verdict; and `--verify-repo` over a real signed rh8/x86_64 cell passes
(12 packages EVR-satisfied, every rpm checksig-verified).
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Two review follow-ups:
- Cross-arch genesis matrix is now driven by a single @GENESIS_ARCHES array
(arch -> xCAT tarch) in MockBuildUtils. finalize_xcat_dep discovers, gates, and
N-way cross-copies over that list, so adding an arch later (aarch64, riscv64) is
one entry there plus wiring its repo root. No behaviour change for the current
x86_64/ppc64le pair (tests unchanged + a contract assertion on the array).
- PR #62 review #4: the repo completeness gate verified only the repomd.xml
signature. It now ALSO verifies every binary rpm's header signature is by the
configured signing key -- resolve the key's accepted id set (primary + subkey
ids) and check each rpm's RSAHEADER/DSAHEADER pgpsig key id, failing on any
unsigned or foreign-signed rpm. Runs per-target in deploy_target and per-cell
after --finalize-xcat-dep. A signed repomd over an unsigned rpm otherwise passed
the gate yet DNF rejects the package at install. Added pure fixtures.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
PR #62 review #2: finalize_xcat_dep() discovered OS dirs by grepping for an
x86_64 subdir under the x86_64 repo, so a ppc64le-only <os> (an rh<N> that built
for ppc but has no x86_64 sibling) was never iterated -- finalize never
cross-populated that cell's x86_64 genesis and still exited 0.
Discover the union of <os> dirs from both arch repos and require both arch peers
for every one, so an x86_64-only AND a ppc64le-only cell both die with a named
error instead of passing unnoticed. Add a symmetric fixture for the ppc64le-only
case (mirrors the existing x86_64-only peer test).
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
PR #62 review raised two build-side problems:
1. The child builders installed each freshly built RPM onto the build host
("dnf -y install" + a run smoke). Because mockbuild-all builds el8/el9/el10
on one host, this installs a foreign-EL RPM into the host RPM database and
corrupts it. The real install-and-run verification already happens in the CI
Test phase (cluster-test.pl boots a matching MN and installs xCAT + the
freshly built xcat-dep there), so the host-side smoke was redundant. Remove
the install/smoke step from every builder and the perl aggregate builder, and
drop the now-dead --skip-install flag (builders, mockbuild-all.pl, and the
pipeline invocations). The perl builder's --jobs 1 throttle existed only to
avoid host dnf-lock contention during that install, so it goes too (perl
packages build in parallel again). Also drop goconserver's now-unused run_rc.
2. build_mock_uniqueext truncated the run id by keeping the LAST 24 chars, which
for the 7-char "ppc64le" arch dropped the leading EL digit -- so
alma+epel-{8,9,10}-ppc64le collapsed to the same run part. goconserver
compiles every EL in the el10 chroot (build_cfg rewritten to -10-), so the
chroot name is identical across the three ELs and the uniqueext was the only
thing keeping their mock roots apart: with parallel targets the three ppc
goconserver builds raced in one root. Keep a readable leading token AND append
a short digest of the full id so distinct ids always yield distinct uniqueext.
Moved the helper into MockBuildUtils.pm and added fixtures (distinct per EL on
a long run id, both arches, determinism).
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Adds a real gate on the BUILT per-target repo, using packages-manifest.conf as the
single source of truth, layered so the decision is pure and unit-tested:
- MockBuildUtils::verify_repo_packages(\%expected,\%present) -- pure completeness
(MISSING / VERSION vs the manifest pins); verify_repo_signature(\%expected,\%observed)
-- pure signature identity (UNSIGNED / WRONGKEY). Both unit-tested (happy+sad).
- mockbuild-all.pl does the IO and calls both from one sub, verify_target_repo:
reads the manifest, enumerates the repo via rpm_version, resolves --gpg-key-name to a
primary-key fingerprint and extracts repomd.xml.asc's actual signer (VALIDSIG), then
merges the two pure results and dies listing every problem.
- Runs AUTOMATICALLY at the end of deploy_target (after sign+index), replacing the old
assert_required_deps + inline version-pin loop with one consolidated gate; suppressible
with --no-verify-repo. Also a standalone build-free '--verify-repo=<repo>' mode (manifest
from repo_root, gpg from --gpg-key-name/--gpg-home; target derived from the rh<N>/<arch>
path or --target).
prove t/mockbuild-all.t: 68/68 (was 50). gpg round-trip smoke-tested: right key -> OK,
wrong key -> WRONGKEY, missing .asc -> UNSIGNED.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Follow-up to the @viniciusferrao review of the EL matrix build:
- Rewrite the stale run_build_steps_parallel comment that still described the
removed 'tolerate genesis failure' workaround; the code is strict
zero-tolerance (xcat-core #7696 made buildrpms.pl exit 0 iff it built the
genesis rpm), so the comment now matches.
- sh_quote the operator-supplied --gpg-key-name at every rpmsign/gpg site
(was interpolated raw into the shell).
- Derive assert_required_deps' required set from the target's
packages-manifest.conf section (the single source of truth) instead of a
second hard-coded list that could drift.
- Move bump_dep_release_suffix into MockBuildUtils (pure, arg-driven) and add a
File::Temp fixture test (stamp, xcat-core prune, no-Release skip, idempotency)
-- the paths the review asked to cover. Its temp file now carries hostname+pid
so the two arch build hosts can't collide on the shared NFS tree.
prove t/mockbuild-all.t: 50/50.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Follow-up to 40feffc, addressing the nice-to-have / secondary points from @viniciusferrao's review.
1 (over-reach): the --build-number spec walk now PRUNES a nested `xcat-core`/`xcat-source-code`
checkout under $repo_root, so the legacy nested layout can no longer rewrite an xCAT-core spec
(e.g. xCAT-genesis-base.spec's dynamic Release). In the normal sibling layout nothing changes.
4 (validate %RELEASE): after the manifest %VERSION pins, when a CD --build-number bump is in effect
the run now also asserts the bump actually LANDED in each built dep/perl rpm's %RELEASE (genesis
excluded -- it is intentionally not bumped). Catches a silently un-bumped NVR that a Version-only
check misses. New MockBuildUtils::rpm_release helper.
6 (--max-parallel a real cap): the perl builder internally forks up to $effective_parallel_builds
mock jobs, so running it concurrently with the dep builders let live mock builds reach ~2x the
cap. Run the perl builder in its OWN phase, after the (quick) dep builders -- each phase then runs
at most $effective_parallel_builds mock builds, so --max-parallel holds, at a small bounded cost.
Tests: 45/45 in t/mockbuild-all.t (rpm_release added).
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Reviewed by @viniciusferrao. Each numbered point below is his; the code changes verify + fix it.
1. --build-number over-reach / dry-run / double-stamp
- The bump now runs ONLY on a real build: `--dry-run --build-number N` prints what it would
stamp and writes nothing (previously it rewrote every spec on disk during a dry run).
- Re-stamping is idempotent AND replacing: a re-run in a reused tree with a different
--build-number strips the prior .snap<ts>.<n> before applying the new one, instead of
accumulating a second stamp (…snap57.snap58). Extracted the per-line logic into the
unit-testable MockBuildUtils::restamp_release_line and covered it in t/mockbuild-all.t.
(The headline "rewrites xcat-core / xCAT-genesis-base.spec" does not occur in the real layout:
xcat-core is a sibling of $repo_root, and there is no genesis spec under the dep tree. The
legacy nested xcat-source-code case remains a non-CD layout; left as a follow-up.)
3. --finalize-xcat-dep idempotency
- cross_copy_genesis compared only SIGMD5 (content), which is blind to signature + index state.
It now also treats a same-content-but-UNSIGNED destination rpm as not-up-to-date (new
rpm_is_signed helper) so a crash between copy and sign heals on re-run.
- finalize_xcat_dep now re-indexes+signs BOTH repos of a touched pair every run, not only when
an rpm was copied, so a crash after copy+sign but before createrepo (rpm on disk, absent from
repomd) also heals.
4. Stale run-state can mask a failed build
- A real build now wipes its per-target $run_root first (run_id is derived from the deterministic
commit time, so re-runs reused the same tree). --skip-build keeps the tree; --dry-run writes nothing.
- mockbuild-perl-packages.pl clears each package's stale status.txt/error.txt BEFORE building, and
the aggregate now treats the child worker's exit code as authoritative: a package is PASS only if
its worker exited 0 AND wrote a PASS this run (a stale PASS in a reused log dir no longer counts).
5. --skip-build can publish the wrong artifacts
- --skip-build now REQUIRES an explicit --target (without it, all three EL targets collected the
same EL-agnostic roots and cross-published them).
- Collection is scoped to this target's own per-target $build_root (the same tree a normal build
populates), not the legacy build-output/list3/list5/list6 dirs.
- The manifest version-pin validation (and the "no manifest section" guard) now also run under
--skip-build, so a collection-only publish is validated exactly like a fresh build.
2. goconserver bypassed the CD bump (minimal fix; hermetic rebuild deferred)
- goconserver/mockbuild.pl gains --release-suffix, appended to its generated `Release: 4.elN`,
and mockbuild-all.pl passes the CD suffix down -- so goconserver's NVR advances per run like
every other dep package (an additive publish is no longer a silent no-op on a frozen NVR).
- Pinned the clone to an immutable upstream commit instead of the moving `master` (0.3.3 is
unreleased -- newest tag is v0.3.2 -- so a SHA pin is required; clone now fetches by ref).
- The host build + `go mod tidy` hermeticity concern is a tracked follow-up, not in this change.
7. Docs
- BUILD.md: --target is a single value, not repeatable; conserver-xcat is built for every target
(not "not required"). POD: --parallel-targets default is 1 = serial, not "auto".
- Added a manifest<->docs consistency test (conserver-xcat present in every target section).
(6, --max-parallel not a true global cap, is a documented nice-to-have and is left as a follow-up.)
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Follow-up self-review hardening on top of the PR #62 review response:
- finalize_xcat_dep now treats a missing ppc64le PEER repo (not just missing
genesis rpms) as fatal instead of silently skipping the OS -- in the CD both
arches build every EL, so a missing peer is an incomplete input that would
otherwise leave the x86_64 repo without the ppc64 genesis and still exit 0.
- cross_copy_genesis treats an empty SIGMD5 (unreadable rpm) as "cannot confirm
identical" and refreshes, rather than risking a false up-to-date match when two
unreadable rpms both return an empty digest.
- rpm_version fails when a directory holds more than one distinct version of a
package (a stale artifact not cleaned before the build) instead of silently
returning the first sorted match, which a version pin could pass against while
the stale rpm still ships. Both arches share a Version for genesis, so a normal
x86_64+ppc64 pair is a single entry.
t/mockbuild-all.t: +4 cases (30 total) -- missing-peer fatal, rpm_sigmd5 on a
missing rpm, and rpm_version multi-version failure.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Review feedback (viniciusferrao):
1. --finalize-xcat-dep no longer succeeds with no genesis rpms and no longer treats
a shared filename as up to date when the content differs.
- finalize_xcat_dep now REQUIRES each arch's own genesis rpm for every repo pair
it processes (a pair with none is a hard FATAL, not a silent exit-0 no-op).
- cross_copy_genesis compares RPM identity by SIGMD5 (header+payload digest,
independent of the GPG signature), so a stale rpm that merely shares a basename
is refreshed instead of being mistaken for up to date.
2. Remove the genesis workaround. xcat-core #7696 is merged, so buildrpms.pl now
exits 0 iff it produced the genesis rpm; the zero-tolerance check no longer
ignores a genesis failure when a matching (possibly stale) rpm already exists --
any failed build step, genesis included, fails the run.
3. Skip modes work. required_pkgs() drops the packages whose builder was skipped, and
both the version-pin check and assert_required_deps use it, so a clean
--skip-genesis / --skip-xcat-dep / --skip-perl run no longer fails validating
packages it deliberately did not build.
4. Tests. The reusable, side-effect-free helpers are factored into MockBuildUtils.pm
(cross_copy_genesis and finalize_xcat_dep take injected sign/reindex callbacks so
they carry no gpg/createrepo state) and t/mockbuild-all.t adds focused fixture
tests for all of the above: skip-mode selection, version-pin globs, SIGMD5-based
RPM-identity comparison + cross_copy refresh/idempotency, and the finalize
require-inputs guard. Run with `prove t/`.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Vinícius Ferrão
Daniel Hilst