Merge pull request #23989 from edsantiago/enable-bats-parallel

CI: system tests: enable parallel tests

.cirrus.yml

@@ -836,8 +836,8 @@ local_system_test_task: &local_system_test_task
         (changesInclude('**/*.go', '**/*.c', '**/*.h') && !changesIncludeOnly('test/**', 'pkg/machine/e2e/**'))
     depends_on: *build
     matrix: *platform_axis
-    gce_instance: *standardvm
-    timeout_in: 35m
+    gce_instance: *fastvm
+    timeout_in: 25m
     env:
         TEST_FLAVOR: sys
     clone_script: *get_gosrc
@@ -890,8 +890,8 @@ rootless_remote_system_test_task:
               CTR_FQIN: ${FEDORA_CONTAINER_FQIN}
     <<: *local_system_test_task
     alias: rootless_remote_system_test
-    gce_instance: *standardvm
-    timeout_in: 35m
+    gce_instance: *fastvm
+    timeout_in: 25m
     env:
         TEST_FLAVOR: sys
         PODBIN_NAME: remote
@@ -905,8 +905,8 @@ rootless_system_test_task:
     only_if: *only_if_system_test
     depends_on: *build
     matrix: *platform_axis
-    gce_instance: *standardvm
-    timeout_in: 35m
+    gce_instance: *fastvm
+    timeout_in: 25m
     env:
         TEST_FLAVOR: sys
         PRIV_NAME: rootless

Makefile

@@ -688,7 +688,8 @@ localmachine:
 localsystem:
 	# Wipe existing config, database, and cache: start with clean slate.
 	$(RM) -rf ${HOME}/.local/share/containers ${HOME}/.config/containers
-	if timeout -v 1 true; then PODMAN=$(CURDIR)/bin/podman QUADLET=$(CURDIR)/bin/quadlet bats -T test/system/; else echo "Skipping $@: 'timeout -v' unavailable'"; fi
+	PODMAN=$(CURDIR)/bin/podman QUADLET=$(CURDIR)/bin/quadlet bats -T --filter-tags '!ci:parallel' test/system/
+	PODMAN=$(CURDIR)/bin/podman QUADLET=$(CURDIR)/bin/quadlet bats -T --filter-tags ci:parallel -j $$(nproc) test/system/
 
 .PHONY: remotesystem
 remotesystem:
@@ -717,7 +718,8 @@ remotesystem:
 			echo "Error: ./bin/podman system service did not come up" >&2;\
 			exit 1;\
 		fi;\
-		env PODMAN="$(CURDIR)/bin/podman-remote" bats -T test/system/ ;\
+		env PODMAN="$(CURDIR)/bin/podman-remote" bats -T --filter-tags '!ci:parallel' test/system/ ;\
+		env PODMAN="$(CURDIR)/bin/podman-remote" bats -T --filter-tags ci:parallel -j $$(nproc) test/system/ ;\
 		rc=$$?;\
 		kill %1;\
 	else \

contrib/cirrus/logformatter

@@ -349,7 +349,7 @@ END_HTML
         # BATS handling. This will recognize num_tests both at start and end
         if ($line =~ /^1\.\.(\d+)$/) {
             $looks_like_bats = 1;
-            $bats_count{expected_total} = $1;
+            $bats_count{expected_total} += $1;
             undef $looks_like_python;
         }
         # Since the number of tests can't always be predicted, recognize
@@ -445,9 +445,14 @@ END_HTML
             if ($css) {
                 # Make it linkable, e.g. foo.html#t--00001
                 if ($line =~ /^(not\s+)?ok\s+(\d+)\s+(.*)/) {
-                    $line = sprintf("<a name='t--%05d'>%s</a>", $2, $line);
+                    my ($notok, $num, $name) = ($1, $2, $3);
+                    # Parallel system tests are shown as |nnn| instead of [nnn]
+                    my $p = '';
+                    $p = 'p' if $name =~ /^\s*\|\d+\|/;
+
+                    $line = sprintf("<a name='t--%05d%s'>%s</a>", $num, $p, $line);
 
-                    push @{$bats_count{__fail_list}}, [ $2, $3 ] if $1;
+                    push @{$bats_count{__fail_list}}, [ $num, $name ] if $notok;
                 }
                 $bats_count{$css}++;
                 $css = "bats-$css";

hack/bats

@@ -83,7 +83,10 @@ for i;do
         --rootless) TEST_ROOT= ;;
         --remote)   REMOTE=remote ;;
         --ts|-T)    bats_opts+=("-T") ;;
-        --tag=*)    bats_filter=("--filter-tags" "$value") ;;
+        --tag=*)    bats_filter=("--filter-tags" "$value")
+                    if [[ "$value" = "ci:parallel" ]]; then
+                        bats_opts+=("--jobs" $(nproc))
+                    fi;;
         */*.bats)   TESTS=$i ;;
         *)
             if [[ $i =~ : ]]; then
@@ -130,7 +133,7 @@ export PODMAN_BATS_LEAK_CHECK=1
 
 # Root
 if [[ "$TEST_ROOT" ]]; then
-    echo "# bats ${bats_filter[*]} $TESTS"
+    echo "# bats ${bats_opts[*]} ${bats_filter[*]} $TESTS"
     sudo    --preserve-env=PODMAN \
             --preserve-env=QUADLET \
             --preserve-env=PODMAN_TEST_DEBUG \
@@ -144,7 +147,7 @@ fi
 # Rootless. (Only if we're not already root)
 if [[ "$TEST_ROOTLESS" && "$(id -u)" != 0 ]]; then
     echo "--------------------------------------------------"
-    echo "\$ bats ${bats_filter[*]} $TESTS"
+    echo "\$ bats ${bats_opts[*]} ${bats_filter[*]} $TESTS"
     bats "${bats_opts[@]}" "${bats_filter[@]}" $TESTS
     rc=$((rc | $?))
 fi

test/system/000-TEMPLATE

@@ -11,6 +11,22 @@ load helpers
     args="some sort of argument list"
     run_podman subcmd $args
     assert "$output" == "what we expect"   "output from 'podman subcmd $args'"
+
+    # safename() provides a lower-case string with both the BATS test number
+    # and a pseudorandom element. Its use is strongly encouraged for container
+    # names, volumes, networks, images, everything, because the test number
+    # makes it possible to find leaked elements.
+    cname="c-$(safename)"
+
+    # Run "top" in a detached container with a safe name
+    run_podman run -d --name $cname --this-option --that $IMAGE top
+
+    # A number ("17") as the first run_podman arg means "expect this exit code".
+    # By default, run_podman expects success, and will barf on nonzero status.
+    run_podman 17 run --this-option --that $IMAGE exit 17
+
+    # Give a hoot, don't pollute
+    run_podman rm -f -t0 $cname
 }
 
 # vim: filetype=sh
@@ -110,5 +126,31 @@ size      | -\\\?[0-9]\\\+
     done < <(parse_table "$tests")
 }
 
+# Whenever possible, please add the ci:parallel tag to new tests,
+# not only for speed but for stress testing.
+#
+# This is an example of what NOT to do when enabling parallel tests.
+#
+# bats test_tags=ci:parallel
+@test "this test is completely broken in parallel" {
+    # Never use "--name HARDCODED". Define 'cname=c-$(safename)' instead.
+    # Not only does that guarantee uniqueness, it also gives the test number
+    # in the name, so if there's a leak (at end of tests) it will be possible
+    # to identify the culprit.
+    run_podman --name mycontainer $IMAGE top
+
+    # Same thing for build and -t
+    run_podman build -t myimage ...
+
+    # Never assume exact output from podman ps! Many other containers may be running.
+    run_podman ps
+    assert "$output" = "mycontainer"
+
+    # Never use "-l". It is meaningless when other processes are running.
+    run_podman container inspect -l
+
+    # Never 'rm -a'!!! OMG like seriously just don't.
+    run_podman rm -f -a
+}
 
 # vim: filetype=sh

test/system/030-run.bats

@@ -874,8 +874,9 @@ RUN rm /etc/mtab
 EOF
     expected="'/etc/mtab' -> '/proc/mounts'"
 
+    # --layers=false needed to work around buildah#5674 parallel flake
     local iname=nomtab-$(safename)
-    run_podman build -t $iname $tmpdir
+    run_podman build -t $iname --layers=false $tmpdir
     run_podman run --rm $iname stat -c %N /etc/mtab
     is "$output" "$expected" "/etc/mtab should be created"
 
@@ -1080,8 +1081,9 @@ echo -e "#!/bin/sh\nfalse" >> /usr/bin/nsenter; \
 chmod +x /usr/bin/nsenter
 EOF
 
+    # --layers=false needed to work around buildah#5674 parallel flake
     test_image="cve_2022_1227_test-$(safename)"
-    run_podman build -t $test_image $tmpbuilddir
+    run_podman build -t $test_image --layers=false $tmpbuilddir
     run_podman run -d ${keepid} $test_image top
     ctr="$output"
     run_podman top $ctr huser,user

test/system/055-rm.bats

@@ -127,10 +127,10 @@ function __run_healthcheck_container() {
     run_podman run -d --name $1 \
                --health-cmd /bin/false \
                --health-interval 1s \
-               --health-retries 1 \
+               --health-retries 2 \
                --health-timeout 1s \
                --health-on-failure=stop \
-               --stop-timeout=1 \
+               --stop-timeout=2 \
                --health-start-period 0 \
                --stop-signal SIGTERM \
                $IMAGE sleep infinity

test/system/060-mount.bats

@@ -409,8 +409,9 @@ FROM $IMAGE
 RUN mkdir /mountroot && echo ${datacontent[img]} > /mountroot/data
 EOF
 
+    # --layers=false needed to work around buildah#5674 parallel flake
     img="localhost/img-$(safename)"
-    run_podman build -t $img -f $dockerfile
+    run_podman build -t $img --layers=false -f $dockerfile
 
     # Each test is set up in exactly the same way:
     #
@@ -554,7 +555,7 @@ glob | /*    | /mountroot/           | in
     echo "$datafile_contents" > $workdir/$datafile
     ln -s $datafile $workdir/link
 
-    run_podman create --mount type=glob,src=$workdir/*,dst=/mountroot/,no-dereference --privileged $img sh -c "stat -c '%N' /mountroot/link; cat /mountroot/link"
+    run_podman create --mount type=glob,src=$workdir/*,dst=/mountroot/,no-dereference --privileged $img sh -c "stat -c '%N' /mountroot/link; cat /mountroot/link; ls -l /mountroot"
     cid="$output"
     run_podman start -a $cid
     assert "${lines[0]}" = "'/mountroot/link' -> '$datafile'" "symlink is preserved, on start"

test/system/220-healthcheck.bats

@@ -18,7 +18,12 @@ function _check_health {
     local hc_status="$5"
 
     # Loop-wait (up to a few seconds) for healthcheck event (#20342)
+    # Allow a margin when running parallel, because of system load
     local timeout=5
+    if [[ -n "$PARALLEL_JOBSLOT" ]]; then
+        timeout=$((timeout + 3))
+    fi
+
     while :; do
         run_podman events --filter container=$ctrname --filter event=health_status \
                    --since "$since" --stream=false --format "{{.HealthStatus}}"
@@ -157,7 +162,7 @@ Log[-1].Output   | \"Uh-oh on stdout!\\\nUh-oh on stderr!\\\n\"
 
         # Wait for the container in the background and create the $wait_file to
         # signal the specified wait condition was met.
-        (timeout --foreground -v --kill=5 5 $PODMAN wait --condition=$condition $ctr && touch $wait_file) &
+        (timeout --foreground -v --kill=5 10 $PODMAN wait --condition=$condition $ctr && touch $wait_file) &
 
         # Sleep 1 second to make sure above commands are running
         sleep 1

test/system/252-quadlet.bats

@@ -428,12 +428,14 @@ EOF
 
 # A quadlet container depends on a named quadlet volume
 @test "quadlet - named volume dependency" {
+    local volume_name="v-$(safename)"
+
     # Save the unit name to use as the volume for the container
     local quadlet_vol_unit=dep_$(safename).volume
     local quadlet_vol_file=$PODMAN_TMPDIR/${quadlet_vol_unit}
     cat > $quadlet_vol_file <<EOF
 [Volume]
-VolumeName=foo
+VolumeName=$volume_name
 EOF
 
     # Have quadlet create the systemd unit file for the volume unit
@@ -442,7 +444,6 @@ EOF
 
     # Save the volume service name since the variable will be overwritten
     local vol_service=$QUADLET_SERVICE_NAME
-    local volume_name="foo"
 
     local quadlet_file=$PODMAN_TMPDIR/user_$(safename).container
     cat > $quadlet_file <<EOF
@@ -975,6 +976,7 @@ EOF
 
       service_setup $QUADLET_SERVICE_NAME
 
+      # FIXME: log.91: Starting, not Started
       # Ensure we have output. Output is synced via sd-notify (socat in Exec)
       run journalctl "--since=$STARTED_TIME" --unit="$QUADLET_SERVICE_NAME"
       is "$output" '.*Started.*\.service.*'

test/system/500-networking.bats

@@ -169,18 +169,25 @@ load helpers.network
     local con2_name=con2-$(safename)
     run_podman pod create --name $pod_name  --infra-name $infra_name
     pid="$output"
-    run_podman run --pod $pod_name --name $con1_name $IMAGE cat /etc/hosts
-    is "$output" ".*\s$pod_name $infra_name.*" "Pod hostname in /etc/hosts"
-    is "$output" ".*127.0.0.1\s$con1_name.*" "Container1 name in /etc/hosts"
+    run_podman run --rm --pod $pod_name --name $con1_name $IMAGE cat /etc/hosts
+    assert "$output" =~ ".*\s$pod_name $infra_name.*" \
+           "Pod hostname in /etc/hosts"
+    assert "$output" =~ ".*127.0.0.1\s$con1_name.*" \
+           "Container1 name in /etc/hosts"
     # get the length of the hosts file
     old_lines=${#lines[@]}
 
     # since the first container should be cleaned up now we should only see the
     # new host entry and the old one should be removed (lines check)
     run_podman run --pod $pod_name --name $con2_name $IMAGE cat /etc/hosts
-    is "$output" ".*\s$pod_name $infra_name.*" "Pod hostname in /etc/hosts"
-    is "$output" ".*127.0.0.1\s$con2_name.*" "Container2 name in /etc/hosts"
-    is "${#lines[@]}" "$old_lines" "Number of hosts lines is equal"
+    assert "$output" =~ ".*\s$pod_name $infra_name.*" \
+           "Pod hostname in /etc/hosts"
+    assert "$output" =~ ".*127.0.0.1\s$con2_name.*" \
+           "Container2 name in /etc/hosts"
+    assert "$output" !~ "$con1_name" \
+           "Container1 name should not be in /etc/hosts"
+    is "${#lines[@]}" "$old_lines" \
+       "Number of hosts lines is equal"
 
     run_podman run --pod $pod_name  $IMAGE sh -c  "hostname && cat /etc/hostname"
     is "${lines[0]}" "$pod_name" "hostname is the pod hostname"

test/system/README.md

@@ -5,11 +5,10 @@ debug failures.
 Quick Start
 ===========
 
-Look at [030-run.bats](030-run.bats) for a simple but packed example.
+Look at [000-TEMPLATE](000-TEMPLATE) for a simple starting point.
 This introduces the basic set of helper functions:
 
-* `setup` (implicit) - resets container storage so there's
-one and only one (standard) image, and no running containers.
+* `setup` (implicit) - establishes a test environment.
 
 * `parse_table` - you can define tables of inputs and expected results,
 then read those in a `while` loop. This makes it easy to add new tests.
@@ -21,7 +20,7 @@ examples of how to deal with the more typical such issues.
 but could also be './bin/podman' or 'podman-remote'), with a timeout.
 Checks its exit status.
 
-* `is` - compare actual vs expected output. Emits a useful diagnostic
+* `assert` - compare actual vs expected output. Emits a useful diagnostic
 on failure.
 
 * `die` - output a properly-formatted message to stderr, and fail test
@@ -30,7 +29,13 @@ on failure.
 
 * `skip_if_remote` - like the above, but skip if testing `podman-remote`
 
-* `random_string` - returns a pseudorandom alphanumeric string
+* `safename` - generates a pseudorandom lower-case string suitable
+for use in names for containers, images, volumes, any object. String
+includes the BATS test number, making it possible to identify the
+source of leaks (failure to clean up) at the end of tests.
+
+* `random_string` - returns a pseudorandom alphanumeric string suitable
+for verifying I/O.
 
 Test files are of the form `NNN-name.bats` where NNN is a three-digit
 number. Please preserve this convention, it simplifies viewing the
@@ -59,7 +64,7 @@ commands, their output and exit codes. In a normal run you will never
 see this, but BATS will display it on failure. The goal here is to
 give you everything you need to diagnose without having to rerun tests.
 
-The `is` comparison function is designed to emit useful diagnostics,
+The `assert` comparison function is designed to emit useful diagnostics,
 in particular, the actual and expected strings. Please do not use
 the horrible BATS standard of `[ x = y ]`; that's nearly useless
 for tracking down failures.