From 5abf038ddc0abf511cdc1eb3ee3605f292e97956 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Tue, 15 Oct 2024 18:04:53 -0400 Subject: [PATCH 1/2] Integration tests: run git daemon on a random-but-bind()able port Use a listener helper to bind to an available-according-to-the-kernel listening port and run a command with its stdio more or less tied to the connection instead of trying to launch a git daemon directly using a port number that we can only guess is available. Signed-off-by: Nalin Dahyabhai --- Makefile | 5 +- rpm/buildah.spec | 3 + tests/helpers.bash | 14 +++- tests/inet/inet.go | 180 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 tests/inet/inet.go diff --git a/Makefile b/Makefile index be8c6424cee..37dbeb43e98 100644 --- a/Makefile +++ b/Makefile @@ -53,7 +53,7 @@ endif # Note: Uses the -N -l go compiler options to disable compiler optimizations # and inlining. Using these build options allows you to subsequently # use source debugging tools like delve. -all: bin/buildah bin/imgtype bin/copy bin/tutorial docs +all: bin/buildah bin/imgtype bin/copy bin/inet bin/tutorial docs # Update nix/nixpkgs.json its latest stable commit .PHONY: nixpkgs @@ -110,6 +110,9 @@ bin/copy: $(SOURCES) tests/copy/copy.go bin/tutorial: $(SOURCES) tests/tutorial/tutorial.go $(GO_BUILD) $(BUILDAH_LDFLAGS) -o $@ $(BUILDFLAGS) ./tests/tutorial/tutorial.go +bin/inet: tests/inet/inet.go + $(GO_BUILD) $(BUILDAH_LDFLAGS) -o $@ $(BUILDFLAGS) ./tests/inet/inet.go + .PHONY: clean clean: $(RM) -r bin tests/testreport/testreport diff --git a/rpm/buildah.spec b/rpm/buildah.spec index 13070adc69f..975a61051bd 100644 --- a/rpm/buildah.spec +++ b/rpm/buildah.spec @@ -133,6 +133,7 @@ export BUILDTAGS+=" btrfs_noversion exclude_graphdriver_btrfs" %gobuild -o bin/imgtype ./tests/imgtype %gobuild -o bin/copy ./tests/copy %gobuild -o bin/tutorial ./tests/tutorial +%gobuild -o bin/inet ./tests/inet %{__make} docs %install @@ -143,6 +144,7 @@ cp -pav tests/. %{buildroot}/%{_datadir}/%{name}/test/system cp bin/imgtype %{buildroot}/%{_bindir}/%{name}-imgtype cp bin/copy %{buildroot}/%{_bindir}/%{name}-copy cp bin/tutorial %{buildroot}/%{_bindir}/%{name}-tutorial +cp bin/inet %{buildroot}/%{_bindir}/%{name}-inet rm %{buildroot}%{_datadir}/%{name}/test/system/tools/build/* @@ -163,6 +165,7 @@ rm %{buildroot}%{_datadir}/%{name}/test/system/tools/build/* %{_bindir}/%{name}-imgtype %{_bindir}/%{name}-copy %{_bindir}/%{name}-tutorial +%{_bindir}/%{name}-inet %{_datadir}/%{name}/test %changelog diff --git a/tests/helpers.bash b/tests/helpers.bash index 20f0787ffb4..53f1e6601ca 100644 --- a/tests/helpers.bash +++ b/tests/helpers.bash @@ -7,6 +7,7 @@ BUILDAH_BINARY=${BUILDAH_BINARY:-$TEST_SOURCES/../bin/buildah} IMGTYPE_BINARY=${IMGTYPE_BINARY:-$TEST_SOURCES/../bin/imgtype} COPY_BINARY=${COPY_BINARY:-$TEST_SOURCES/../bin/copy} TUTORIAL_BINARY=${TUTORIAL_BINARY:-$TEST_SOURCES/../bin/tutorial} +INET_BINARY=${INET_BINARY:-$TEST_SOURCES/../bin/inet} STORAGE_DRIVER=${STORAGE_DRIVER:-vfs} PATH=$(dirname ${BASH_SOURCE})/../bin:${PATH} OCI=${CI_DESIRED_RUNTIME:-$(${BUILDAH_BINARY} info --format '{{.host.OCIRuntime}}' || command -v runc || command -v crun)} @@ -683,8 +684,17 @@ function start_git_daemon() { chown -R root:root ${daemondir}/repo fi - GITPORT=$(($RANDOM + 32768)) - git daemon --detach --pid-file=${TEST_SCRATCH_DIR}/git-daemon/pid --reuseaddr --port=${GITPORT} --base-path=${daemondir} ${daemondir} + ${INET_BINARY} -port-file ${TEST_SCRATCH_DIR}/git-daemon/port -pid-file=${TEST_SCRATCH_DIR}/git-daemon/pid -- git daemon --inetd --base-path=${daemondir} ${daemondir} & + + local waited=0 + while ! test -s ${TEST_SCRATCH_DIR}/git-daemon/pid ; do + sleep 0.1 + if test $((++waited)) -ge 300 ; then + echo test git server did not write pid file within timeout + exit 1 + fi + done + GITPORT=$(cat ${TEST_SCRATCH_DIR}/git-daemon/port) } function stop_git_daemon() { diff --git a/tests/inet/inet.go b/tests/inet/inet.go new file mode 100644 index 00000000000..71d65b112ae --- /dev/null +++ b/tests/inet/inet.go @@ -0,0 +1,180 @@ +package main + +import ( + "errors" + "flag" + "fmt" + "io" + "net" + "os" + "os/exec" + "path/filepath" + "strconv" + "syscall" + + multierror "github.com/hashicorp/go-multierror" + "github.com/sirupsen/logrus" +) + +// This is similar to netcat's listen mode, except it logs which port it's +// assigned if it's told to attempt to bind to port 0. Or it's similar to +// inetd, if it wasn't a daemon, and it wasn't as well-written. +func main() { + pidFile := "" + portFile := "" + detach := false + flag.BoolVar(&detach, "detach", false, "detach from terminal") + flag.StringVar(&portFile, "port-file", "", "file to write listening port number") + flag.StringVar(&pidFile, "pid-file", "", "file to write process ID to") + flag.Parse() + args := flag.Args() + if len(args) < 1 { + fmt.Printf("Usage: %s [-port-file filename] [-pid-file filename] command ...\n", filepath.Base(os.Args[0])) + os.Exit(1) + } + // Start listening without specifying a port number. + ln, err := net.ListenTCP("tcp", &net.TCPAddr{}) + if err != nil { + logrus.Fatalf("listening: %v", err) + } + // Retrieve the address we ended up bound to and write the port number + // part to the specified file, if one was specified. + addrString := ln.Addr().String() + _, portString, err := net.SplitHostPort(addrString) + if err != nil { + logrus.Fatalf("finding the port number in %q: %v", addrString, err) + } + if portFile != "" { + if err := os.WriteFile(portFile, []byte(portString), 0o644); err != nil { + logrus.Fatalf("writing listening port to %q: %v", portFile, err) + } + defer os.Remove(portFile) + } + // Write our process ID to the specified file, if one was specified. + if pidFile != "" { + pid := strconv.Itoa(os.Getpid()) + if err := os.WriteFile(pidFile, []byte(pid), 0o644); err != nil { + logrus.Fatalf("writing pid %d to %q: %v", os.Getpid(), pidFile, err) + } + defer os.Remove(pidFile) + } + // Now we can log which port we're listening on. + fmt.Printf("process %d listening on port %s\n", os.Getpid(), portString) + closeCloser := func(closer io.Closer) { + if err := closer.Close(); err != nil { + logrus.Errorf("closing: %v", err) + } + } + // Helper function to shuttle data between a reader and a writer. + relay := func(reader io.Reader, writer io.Writer) error { + buffer := make([]byte, 1024) + for { + nr, err := reader.Read(buffer) + if err != nil { + if errors.Is(err, io.EOF) { + return nil + } + return err + } + if nr == 0 { + return nil + } + if nr < 0 { + // no error? + break + } + nw, err := writer.Write(buffer[:nr]) + if err != nil { + return nil + } + if nw != nr { + return fmt.Errorf("short write: %d != %d", nw, nr) + } + } + return nil + } + for { + // Accept the next incoming connection. + conn, err := ln.AcceptTCP() + if err != nil { + logrus.Errorf("accepting new connection: %v", err) + continue + } + if conn == nil { + logrus.Error("no new connection?") + continue + } + go func() { + defer closeCloser(conn) + rawConn, err := conn.SyscallConn() + if err != nil { + logrus.Errorf("getting underlying connection: %v", err) + return + } + var setNonblockError error + if err := rawConn.Control(func(fd uintptr) { + setNonblockError = syscall.SetNonblock(int(fd), true) + }); err != nil { + logrus.Errorf("marking connection nonblocking (outer): %v", err) + return + } + if setNonblockError != nil { + logrus.Errorf("marking connection nonblocking (inner): %v", setNonblockError) + return + } + // Create pipes for the subprocess's stdio. + stdinReader, stdinWriter, err := os.Pipe() + if err != nil { + logrus.Errorf("opening pipe for stdin: %v", err) + return + } + defer closeCloser(stdinWriter) + stdoutReader, stdoutWriter, err := os.Pipe() + if err != nil { + logrus.Errorf("opening pipe for stdout: %v", err) + closeCloser(stdinReader) + return + } + defer closeCloser(stdoutReader) + if err := syscall.SetNonblock(int(stdoutReader.Fd()), true); err != nil { + logrus.Errorf("marking stdout reader nonblocking: %v", err) + closeCloser(stdinReader) + closeCloser(stdoutWriter) + return + } + // Start the subprocess. + cmd := exec.Command(args[0], args[1:]...) + cmd.Stdin = stdinReader + cmd.Stdout = stdoutWriter + cmd.Stderr = os.Stderr + if err := cmd.Start(); err != nil { + logrus.Errorf("starting %v: %v", args, err) + closeCloser(stdinReader) + closeCloser(stdoutWriter) + return + } + // Process the subprocess's stdio and wait for it to exit, + // presumably when it runs out of data. + var relayGroup multierror.Group + relayGroup.Go(func() error { + err := relay(conn, stdinWriter) + closeCloser(stdinWriter) + return err + }) + relayGroup.Go(func() error { + err := relay(stdoutReader, conn) + closeCloser(stdoutReader) + return err + }) + relayGroup.Go(func() error { + err := cmd.Wait() + closeCloser(conn) + return err + }) + merr := relayGroup.Wait() + if merr != nil && merr.ErrorOrNil() != nil { + logrus.Errorf("%v\n", merr) + } + }() + } +} From 855ec0f0c5e1d9614e78a95878e91413a2963bee Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Fri, 18 Oct 2024 15:25:51 -0400 Subject: [PATCH 2/2] tests/test_runner.sh: remove some redundancies This wrapper doesn't need to load anything from helpers.bash, because the various .bats files already do so on their own. Signed-off-by: Nalin Dahyabhai --- tests/test_runner.sh | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/test_runner.sh b/tests/test_runner.sh index a93654b3721..8f89e73c716 100755 --- a/tests/test_runner.sh +++ b/tests/test_runner.sh @@ -7,16 +7,10 @@ cd "$(dirname "$(readlink -f "$BASH_SOURCE")")" # labels than /tmp, which is often on tmpfs. export TMPDIR=${TMPDIR:-/var/tmp} -# Load the helpers. -. helpers.bash - function execute() { >&2 echo "++ $@" eval "$@" } -# Tests to run. Defaults to all. -TESTS=${@:-.} - # Run the tests. -execute time bats --tap $TESTS +execute time bats --tap "${@:-.}"