From ee9c681f31a373e7e2c904a31ec71ab572b013e0 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Mon, 21 Oct 2024 12:28:35 -0600 Subject: [PATCH 1/6] Buildah treadmill: improve wording in test-fail instructions Clarify, expand, fix a typo. These are the instructions shown when the **patching** step fails, typically when buildah's helpers.bash is changed in a way that conflicts with our make-it-work-in-podman patches. Signed-off-by: Ed Santiago --- test/buildah-bud/run-buildah-bud-tests | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/buildah-bud/run-buildah-bud-tests b/test/buildah-bud/run-buildah-bud-tests index 35dd3ae88f..3561c3f117 100755 --- a/test/buildah-bud/run-buildah-bud-tests +++ b/test/buildah-bud/run-buildah-bud-tests @@ -199,11 +199,20 @@ if [[ -n $do_checkout ]]; then Error applying patch file. This can happen when you vendor in a new buildah. You will want to: - - look for 'test/*.rej' + *** START A NEW TERMINAL WINDOW! *** + *** ...so you can refer to these instructions *** + + - cd test-buildah-* (into the buildah-bud test directory) + + - look for 'tests/*.rej' - resolve conflicts manually - - git add test/helpers.bash + - git add tests/helpers.bash - git am --continue - ./make-new-buildah-diffs + + - cd .. (back to podman source dir) + +...and git-commit the new .diff file as part of your podman PR. " (set -x;git am --reject <$PATCHES) From ba8375c9e41435f5bc0ada391af86c6e86dd741b Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Mon, 21 Oct 2024 12:31:59 -0600 Subject: [PATCH 2/6] Buildah treadmill: improve test-failure instructions This time, in the vendor script itself. Signed-off-by: Ed Santiago --- hack/buildah-vendor-treadmill | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/hack/buildah-vendor-treadmill b/hack/buildah-vendor-treadmill index e2160ffa0c..dfb8e0ce82 100755 --- a/hack/buildah-vendor-treadmill +++ b/hack/buildah-vendor-treadmill @@ -457,21 +457,14 @@ sub build_and_check_podman { warn <<"END_WARN"; $ME: Errors found. I have to stop now for you to fix them. Your best bet now is: - 1) Find and fix whatever needs to be fixed; then - 2) git commit -am'fixme-fixme'; then - 3) git rebase -i main: - a) you are now in an editor window - b) move the new fixme-fixme commit up a line, to between the - 'buildah vendor treadmill' and 'vendor in buildah @ ...' lines - c) change 'pick' to 'squash' (or just 's') - d) save & quit to continue the rebase - e) back to a new editor window - f) change the commit message: remove fixme-fixme, add a description - of what you actually fixed. If possible, reference the PR (buildah - or podman) that introduced the failure - g) save & quit to continue the rebase + 1) START A NEW TERMINAL! So you can refer back here. - Now, for good measure, rerun this script. + 2) Find and fix whatever needs to be fixed. There may + be hint messages above. You may need to cd to the + new `test-buildah-SOMETHING` directory + 3) git commit --amend ANY-FILES-THAT-YOU-CHANGED + + Rerun this script, possibly with `--force-testing` For full documentation, refer to From a925c9f83125f655284e1fb258e9dd12fc8539aa Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Mon, 21 Oct 2024 12:33:02 -0600 Subject: [PATCH 3/6] Buildah treadmill: more allow-empty options Handle the condition where the second commit (the one making podman-specific changes) is empty. Signed-off-by: Ed Santiago --- hack/buildah-vendor-treadmill | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/buildah-vendor-treadmill b/hack/buildah-vendor-treadmill index dfb8e0ce82..de91bed960 100755 --- a/hack/buildah-vendor-treadmill +++ b/hack/buildah-vendor-treadmill @@ -285,7 +285,7 @@ END_FAIL_INSTRUCTIONS exit 1; }; progress('Reapplying treadmill patches'); - git('cherry-pick', '--allow-empty', $treadmill_commit); + git('cherry-pick', '--allow-empty', '--empty=keep', $treadmill_commit); # It worked! Clean up: remove our local die() handler and the saved branch undef $SIG{__DIE__}; From 9db04e87b6fca020d343e7542ec3bef6c05f849c Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Mon, 21 Oct 2024 12:38:15 -0600 Subject: [PATCH 4/6] Buildah treadmill: redo the .cirrus.yml tweaks Initial purpose of treadmill PR was to run buildah-bud tests early, and not run anything else if they fail. This was to catch vendoring problems and not be distracted by flakes. This was done by inspecting and massaging .cirrus.yml. As of #21639 this code was a silent NOP because the entire CI tree was overhauled. Here we make that work again. Also, in #20947 I enhanced this script to run rootless bud tests but neglected to updated the comments. Do so now. Signed-off-by: Ed Santiago --- hack/buildah-vendor-treadmill | 48 ++++++++++++++--------------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/hack/buildah-vendor-treadmill b/hack/buildah-vendor-treadmill index de91bed960..4a3ca7ca4f 100755 --- a/hack/buildah-vendor-treadmill +++ b/hack/buildah-vendor-treadmill @@ -338,14 +338,19 @@ sub pull_main { } ############################# -# tweak_cirrus_test_order # Run bud tests first, to fail fast & early +# tweak_cirrus_test_order # Two edits to Cirrus tasks ############################# +# +# 1. Run bud tests first, to fail fast & early +# 2. Run rootless bud tests. These don't run in regular CI, +# so this is our only chance to catch problems. +# sub tweak_cirrus_test_order { my $cirrus_yml = '.cirrus.yml'; my $tmpfile = "$cirrus_yml.tmp.$$"; unlink $tmpfile; - progress("Tweaking test order in $cirrus_yml to run bud tests early"); + progress("Tweaking test order in $cirrus_yml"); open my $in, '<', $cirrus_yml or do { warn "$ME: Cannot read $cirrus_yml: $!\n"; @@ -355,39 +360,22 @@ sub tweak_cirrus_test_order { open my $out, '>'. $tmpfile or die "$ME: Cannot create $tmpfile: $!\n"; my $current_task = ''; - my $in_depend; while (my $line = <$in>) { chomp $line; if ($line =~ /^(\S+)_task:$/) { $current_task = $1; - undef $in_depend; } - elsif ($line =~ /^(\s+)depends_on:$/) { - $in_depend = $1; - } - elsif ($in_depend && $line =~ /^($in_depend\s+-\s+)(\S+)/) { - my $indent = $1; - - # Run the buildah-bud tests early: that's the entire point + elsif ($line =~ /^(\s+)depends_on:\s+\*build$/) { + # Run the buildah-bud tests early: that's much of the point # of the treadmill PR. Here we switch Cirrus task dependencies - # such that bud tests run as early as possible. - if ($current_task =~ /buildah_bud_test/) { - # Buildah bud now depends only on validate... - $line = "${indent}validate"; - } - elsif ($2 eq 'validate' && $current_task ne 'success') { - # ...and all other tests that relied on validate now rely on - # bud tests instead. The point of the treadmill PR is to - # run the bud tests and only then, if everything passes, - # run normal tests. (Reason: bud tests are the only ones - # likely to fail on a buildah revendor, and we want to see - # failures early). - $line = "${indent}buildah_bud_test"; + # such that bud tests run as early as possible, and all other + # tests will only run if bud tests pass. + my $indent = $1; + if ($current_task !~ /buildah_bud_test/) { + $line = "${indent}depends_on:\n${indent} - buildah_bud_test"; } } else { - undef $in_depend; - # FIXME THIS IS HORRIBLE! # Add rootless jobs to the buildah bud test matrix. # This is incredibly fragile; it relies on the fact @@ -914,8 +902,10 @@ sub assert_buildah_vendor_commit { my @deltas = git('diff', '--name-only', "$ref^", $ref); - # It's OK if there are no deltas, e.g. immediately after a buildah vendor PR - return if !@deltas; + # It's OK if there are no deltas or if the only delta is to Cirrus. + # This is expected immediately after buildah gets vendored and + # there is nothing new to vendor in. + return if !@deltas || "@deltas" eq ".cirrus.yml"; # It's OK if there are more modified files than just these. # It's not OK if any of these are missing. @@ -934,7 +924,7 @@ sub assert_buildah_vendor_commit { return if !@missing; warn "$ME: $ref does not look like a buildah vendor commit:\n"; - warn "$ME: - $_\n" for @missing; + warn " - $_\n" for @missing; die "$ME: Cannot continue\n"; } From 825eed4bde227df6d4b0157353c27dfbec6c38e3 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Tue, 22 Oct 2024 12:43:52 -0600 Subject: [PATCH 5/6] new showrun() for displaying and running shell commands Equivalent to print() + system(). Shows individual commands being run, which may help a developer understand and replicate actions if they fail. Signed-off-by: Ed Santiago --- hack/buildah-vendor-treadmill | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/hack/buildah-vendor-treadmill b/hack/buildah-vendor-treadmill index 4a3ca7ca4f..2bd8e550c0 100755 --- a/hack/buildah-vendor-treadmill +++ b/hack/buildah-vendor-treadmill @@ -41,12 +41,14 @@ our $API_URL = 'https://api.github.com/graphql'; # Use colors if available and if stdout is a tty our $C_Highlight = ''; our $C_Warning = ''; +our $C_LogCmd = ''; our $C_Reset = ''; eval ' use Term::ANSIColor; if (-t 1) { $C_Highlight = color("green"); $C_Warning = color("bold red"); + $C_LogCmd = color("blue"); $C_Reset = color("reset"); } @@ -234,9 +236,9 @@ END_FAIL_INSTRUCTIONS # This does have a high possibility of failing. progress("Vendoring in buildah..."); - system('go', 'mod', 'edit', '--require' => "${Buildah}\@main") == 0 + showrun('go', 'mod', 'edit', '--require' => "${Buildah}\@main") == 0 or die "$ME: go mod edit failed"; - system('make', 'vendor') == 0 + showrun('make', 'vendor') == 0 or die "$ME: make vendor failed"; my $buildah_new = vendored_buildah(); print "-> buildah new = $buildah_new\n"; @@ -418,7 +420,7 @@ sub build_and_check_podman { # Confirm that we can still build podman progress("Running 'make' to confirm that podman builds cleanly..."); - system('make') == 0 + showrun('make') == 0 or die "$ME: 'make' failed with new buildah. Cannot continue.\n"; # See if any new options need man pages. (C_Warning will highlight errs) @@ -431,14 +433,14 @@ sub build_and_check_podman { # the name of the directory created by the bud-tests script. progress("Confirming that buildah-bud-tests patches still apply..."); system('rm -rf test-buildah-*'); - if (system('test/buildah-bud/run-buildah-bud-tests', '--no-test')) { + if (showrun('test/buildah-bud/run-buildah-bud-tests', '--no-test')) { # Error ++$errs; warn "$ME: Leaving test-buildah- directory for you to investigate\n"; } else { # Patches apply cleanly. Clean up - system('rm -rf test-buildah-*'); + showrun('rm -rf test-buildah-*'); } return if !$errs; @@ -894,6 +896,14 @@ sub git { return wantarray ? @results : join("\n", @results); } +############# +# showrun # Given a command, print it then run it. Return value is system() +############# +sub showrun { + print "\$ ${C_LogCmd}@_${C_Reset}\n"; + system(@_); +} + ################################## # assert_buildah_vendor_commit # Fails if input arg is not a buildah vendor ################################## From d2ba730f33bfd95808ac4727df06cdd4c65baf5c Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Tue, 22 Oct 2024 13:08:56 -0600 Subject: [PATCH 6/6] buildah version display: use progress() ...to make it stand out just a little more. Signed-off-by: Ed Santiago --- hack/buildah-vendor-treadmill | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/buildah-vendor-treadmill b/hack/buildah-vendor-treadmill index 2bd8e550c0..bd5a5abe76 100755 --- a/hack/buildah-vendor-treadmill +++ b/hack/buildah-vendor-treadmill @@ -180,7 +180,7 @@ sub do_sync { # Looks good so far. my $buildah_old = vendored_buildah(); - print "-> buildah old = $buildah_old\n"; + progress("::: buildah OLD = $buildah_old :::\n"); # Pull main, and pivot back to this branch pull_main(); @@ -241,7 +241,7 @@ END_FAIL_INSTRUCTIONS showrun('make', 'vendor') == 0 or die "$ME: make vendor failed"; my $buildah_new = vendored_buildah(); - print "-> buildah new = $buildah_new\n"; + progress("::: buildah NEW = $buildah_new :::\n"); # Tweak .cirrus.yml so we run bud tests first in CI (to fail fast). tweak_cirrus_test_order();