Implemented some integration tests

.github/workflows/build_and_maybe_release.yml

@@ -8,13 +8,13 @@ jobs:
   build:
     env:
       RELEASE_OS: ubuntu-24.04
-      RELEASE_GOVER: "1.22"
+      RELEASE_GOVER: "1.25"
 
     strategy:
       fail-fast: false
       matrix:
-        os: [ ubuntu-24.04, ubuntu-22.04, ubuntu-20.04 ]
-        gover: [ "1.22", "1.21", "1.20" ]
+        os: [ ubuntu-24.04, ubuntu-22.04 ]
+        gover: [ "1.25", "1.24", "1.23", "1.22", "1.21", "1.20" ]
 
     runs-on: ${{ matrix.os }}
 
@@ -23,8 +23,17 @@ jobs:
       - uses: actions/setup-go@v5
         with:
           go-version: ${{ matrix.gover }}
+      - name: Setup Bats
+        uses: bats-core/[email protected]
       - name: Build the project
         run: go build .
+      - name: Run integration tests
+        env:
+          # For `bats -F pretty`
+          TERM: xterm-256color
+        run: |
+          sudo -b ./docker-on-top
+          bats -F pretty tests/
       - name: Save built binary
         if: startsWith(github.ref, 'refs/tags/') && matrix.os == env.RELEASE_OS && matrix.gover == env.RELEASE_GOVER
         uses: actions/upload-artifact@v4

README.md

@@ -69,6 +69,12 @@ sudo systemctl enable docker-on-top.service  # If you want the plugin to be auto
 That's it. After these actions you can manage the plugin as a systemd service with
 commands like `systemctl start`, `systemctl stop`, etc.
 
+## Integration tests
+
+To run integration tests, set up [bats](https://github.com/bats-core/bats-core), start
+the version of docker-on-top that you want to test (e.g., build from source and start),
+then, from the project root directory, run `bats tests/`.
+
 ## Volatile volumes
 
 (note: volatile volumes have nothing to do with overlayfs's "volatile mount")

go.mod

@@ -8,10 +8,8 @@ require (
 )
 
 require (
-	github.com/Microsoft/go-winio v0.6.1 // indirect
+	github.com/Microsoft/go-winio v0.6.2 // indirect
 	github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf // indirect
 	github.com/docker/go-connections v0.5.0 // indirect
-	golang.org/x/mod v0.8.0 // indirect
-	golang.org/x/sys v0.5.0 // indirect
-	golang.org/x/tools v0.6.0 // indirect
+	golang.org/x/sys v0.10.0 // indirect
 )

go.sum

@@ -1,5 +1,5 @@
-github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow=
-github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM=
+github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=
+github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=
 github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf h1:iW4rZ826su+pqaw19uhpSCzhj44qo35pNgKFGqzDKkU=
 github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4=
 github.com/docker/go-connections v0.5.0 h1:USnMq7hx7gwdVZq1L49hLXaFtUdTADjXGp+uj1Br63c=
@@ -8,10 +8,5 @@ github.com/docker/go-plugins-helpers v0.0.0-20211224144127-6eecb7beb651 h1:YcvzL
 github.com/docker/go-plugins-helpers v0.0.0-20211224144127-6eecb7beb651/go.mod h1:LFyLie6XcDbyKGeVK6bHe+9aJTYCxWLBg5IrJZOaXKA=
 github.com/op/go-logging v0.0.0-20160315200505-970db520ece7 h1:lDH9UUVJtmYCjyT0CI4q8xvlXPxeZ0gYCVvWbmPlp88=
 github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWErDVzsI23lYNej1Htcns9BCg93Dk0bBINWk=
-golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8=
-golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
-golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
-golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
-golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM=
-golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
+golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA=
+golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

tests/basic_usage.bats

@@ -0,0 +1,50 @@
+#!/usr/bin/env bats
+
+# For `CONTAINER_CMD_*`
+load common.sh
+
+basic_test() {
+	# Setup
+	BASE="$(mktemp --directory)"
+	NAME="$(basename "$BASE")"
+	if [ -z "$1" ]; then
+		VOLATILE=false
+	else
+		VOLATILE=true
+	fi
+	docker volume create --driver docker-on-top "$NAME" -o base="$BASE" -o volatile="$VOLATILE"
+
+	# Deferred cleanup
+	trap 'rm -rf "$BASE"; docker volume rm "$NAME"; trap - RETURN' RETURN
+
+	echo 123 > "$BASE"/a
+	echo 456 > "$BASE"/b
+
+	docker run --rm -v "$NAME":/dot alpine:latest \
+		sh -e -c "
+			$CONTAINER_CMD_CHECK_INITIAL_DATA
+
+			$CONTAINER_CMD_MAKE_AND_CHECK_CHANGES
+		"
+
+	# Changes are not visible from the host
+	[ "$(cat "$BASE"/a)" = 123 ]
+	[ "$(cat "$BASE"/b)" = 456 ]
+	[ ! -e "$BASE"/c ]
+
+	if $VOLATILE; then
+		# For a volatile volume, changes should not be visible
+		[ "$(docker run --rm -v "$NAME":/dot alpine:latest sh -c 'cat /dot/*')" = "$(echo 123; echo 456)" ]
+	else
+		# For a regular volume, changes should remain
+		[ "$(docker run --rm -v "$NAME":/dot alpine:latest sh -c 'cat /dot/*')" = "$(echo 789; echo etc)" ]
+	fi
+}
+
+@test "Test basic usage" {
+	basic_test
+}
+
+@test "Test volatile basic usage" {
+	basic_test volatile
+}

tests/common.sh

@@ -0,0 +1,27 @@
+#!/bin/false
+
+CONTAINER_CMD_CHECK_INITIAL_DATA='
+	# Check that the initial data is as expected
+	[ "$(cat /dot/a)" = 123 ]
+	[ "$(cat /dot/b)" = 456 ]
+'
+
+CONTAINER_CMD_MAKE_AND_CHECK_CHANGES='
+	# My file removal is visible to me
+	rm -f /dot/a
+	[ ! -e /dot/a ]
+
+	# My changes are visible to me
+	echo 789 > /dot/b
+	[ "$(cat /dot/b)" = 789 ]
+
+	# My new files are visible to me
+	echo etc > /dot/c
+	[ "$(cat /dot/c)" = etc ]
+'
+
+CONTAINER_CMD_CHECK_ALREADY_MODIFIED_DATA='
+	[ ! -e /dot/a ]
+	[ "$(cat /dot/b)" = 789 ]
+	[ "$(cat /dot/c)" = etc ]
+'

tests/invalid_input.bats

@@ -0,0 +1,21 @@
+#!/usr/bin/env bats
+
+@test "invalid volume name" {
+  # Special character as the first character is not allowed
+  ! docker volume create --driver docker-on-top _invalidname
+
+  # Only dots, hyphens, and underscores are allowed as special characters
+  ! docker volume create --driver docker-on-top invalid\*name
+}
+
+@test "invalid base path" {
+  # Only absolute paths are allowed as base paths
+  ! docker volume create --driver docker-on-top valid-name -o base=\\
+  ! docker volume create --driver docker-on-top valid-name -o base=a/b
+
+  # Base directory shall exist
+  ! docker volume create --driver docker-on-top valid-name -o base=/does/not/exist
+
+  # TODO: if the base directory points to a file, that should be an error
+  # ! docker volume create --driver docker-on-top valid-name -o base="$(realpath "$0")"
+}

tests/mounted_volume_state_safety.bats

@@ -0,0 +1,197 @@
+#!/usr/bin/env bats
+
+# TODO: move the following explanation from here to the source code?
+
+# Under normal operation, docker-on-top keeps all of its volumes in a
+# _valid_ state. When interfered with (e.g., when the user messes with
+# our internal state or in case we were terminated abruptly or whatever
+# other reason causes our crucial syscalls to fail),
+# it may not be possible to maintain a _valid_ state, but we still want to
+# leave a volume in a state that is _safe_, that is, on the next interaction
+# with the volume, docker-on-top can tell that something went wrong last time.
+
+# For example, if a volume is unmounted improperly, such that `activemountsdir`
+# was cleaned properly but we failed to unmount `mountpointdir`, that's unfortunate
+# (as we leave a mount hanging) but tolerable (as the next attempt to mount it will
+# succeed as if the volume was unmounted cleanly).
+#
+# On the contrary, if a volume is unmounted improperly, such that `mountpointdir`
+# was unmounted but `activemountsdir` remains non-empty, that's a horror. On the
+# next attempt to mount that volume, docker-on-top will see that it is already mounted
+# and will try to use `mountpoindir` outright but, as it is not actually mounted, that
+# will lead to a completely broken behavior.
+
+# The following tests try to simulate that either mount/unmount system calls, or
+# activemounts file creation/removal system calls fail, and tests that docker-on-top
+# is fine.
+
+
+# For `CONTAINER_CMD_*`
+load common.sh
+
+# Ask for password early
+sudo -v
+
+# Test template
+
+template_cleanup() {
+  $3 "$BASE" "$NAME"
+  $5 "$BASE" "$NAME"
+  sudo umount /var/lib/docker-on-top/"$NAME"/mountpoint || :
+  sudo rmdir /var/lib/docker-on-top/"$NAME"/mountpoint  || :
+  sudo rm /var/lib/docker-on-top/"$NAME"/activemounts/* || :
+  docker rm "$CONTAINER_ID"
+  docker volume rm "$NAME"
+  rm -rf "$BASE"
+}
+
+# $1 is empty if volatile, non-empty if non-volatile;
+# $2 is the command (messing with the state) to perform before mounting;
+# $3 shall undo the effect of $2;
+# $4 is the command (messing with the state) to perform before unmounting;
+# $5 shall undo the effect of $4.
+template() {
+  # Setup
+  BASE="$(mktemp --directory)"
+  NAME="$(basename "$BASE")"
+  if [ -z "$1" ]; then
+    VOLATILE=false
+  else
+    VOLATILE=true
+  fi
+  docker volume create --driver docker-on-top "$NAME" -o base="$BASE" -o volatile="$VOLATILE"
+
+  echo 123 > "$BASE"/a
+  echo 456 > "$BASE"/b
+
+  # Run the breaking action before mount
+  $2 "$BASE" "$NAME"
+
+  # Start the container (might fail)
+  if ! CONTAINER_ID=$(
+      docker run -d -v "$NAME":/dot alpine:latest \
+        sh -e -c "
+          sleep 1  # Give as a moment to break everything
+
+          $CONTAINER_CMD_CHECK_INITIAL_DATA
+
+          $CONTAINER_CMD_MAKE_AND_CHECK_CHANGES
+        "
+    ); then
+
+    # This is the best case: docker-on-top has detected the problem very early
+    # and did not mount anything in the first place
+    template_cleanup "$@"
+    return
+  fi
+
+  # Undo the breaking action
+  $3 "$BASE" "$NAME"
+
+  # Run the breaking action before unmount
+  $4 "$BASE" "$NAME"
+
+  if [ 0 -ne "$(docker wait "$CONTAINER_ID")" ]; then
+    # Container has failed
+    template_cleanup "$@"
+    false
+    return
+  fi
+
+  # Undo the breaking action
+  $5 "$BASE" "$NAME"
+
+  # Now that we are fixed, we expect that (I) either the container doesn't start at all:
+  if ! docker run --rm -v "$NAME":/dot alpine:latest true; then
+    template_cleanup "$@"
+    return  # The test is successful: docker-on-top detected the error and the mount is refused
+  fi
+
+  # (II) Or the container starts and runs as expected:
+  if $VOLATILE; then
+    docker run --rm -v "$NAME":/dot alpine:latest \
+      sh -e -c "
+        $CONTAINER_CMD_CHECK_INITIAL_DATA
+
+        $CONTAINER_CMD_MAKE_AND_CHECK_CHANGES
+      "
+  else
+    docker run --rm -v "$NAME":/dot alpine:latest \
+      sh -e -c "
+        $CONTAINER_CMD_CHECK_ALREADY_MODIFIED_DATA
+
+        $CONTAINER_CMD_MAKE_AND_CHECK_CHANGES
+      "
+  fi
+
+  template_cleanup "$@"
+}
+
+
+# I use "activate"/"deactivate" as shorts for "create/remove a file at `activemountsdir`"
+
+# Break/unbreak functions receive $BASE as $1 and $NAME as $2
+
+break_deactivate() {
+  # Immutable file can not be removed
+  sudo chattr +i /var/lib/docker-on-top/"$2"/activemounts/*
+}
+
+unbreak_deactivate() {
+  sudo chattr -i /var/lib/docker-on-top/"$2"/activemounts/*
+}
+
+
+@test "Mount=fine, activate=fine, unmount=fine, deactivate=broken" {
+  template "" : : break_deactivate unbreak_deactivate
+
+  template "volatile" : : break_deactivate unbreak_deactivate
+}
+
+break_unmount() {
+  # The mountpoint will be kept busy with the following process that has a file on it open
+  sleep 1.5 < /var/lib/docker-on-top/"$2"/mountpoint/b &
+}
+
+unbreak_unmount() {
+  # Just wait for the previous process to finish
+  sleep 1.5
+}
+
+
+@test "Mount=fine, activate=fine, unmount=broken, deactivate=fine" {
+  skip "This is not working yet, needs to be fixed"
+
+  template "" : : break_unmount unbreak_unmount
+
+  template "volatile" : : break_unmount unbreak_unmount
+}
+
+break_activate() {
+  sudo chattr +i /var/lib/docker-on-top/"$2"/activemounts
+}
+
+unbreak_activate() {
+  sudo chattr -i /var/lib/docker-on-top/"$2"/activemounts
+}
+
+@test "Mount=fine, activate=broken, unmount=fine, deactivate=fine" {
+  template "" break_activate unbreak_activate : :
+
+  template "volatile" break_activate unbreak_activate : :
+}
+
+break_mount() {
+  # If the base directory does not exist, the overlayfs mount shall fail
+  rm -r "$1"
+}
+
+unbreak_mount() {
+  mkdir "$1"
+}
+
+@test "Mount=broken, activate=fine, unmount=fine, deactivate=fine" {
+  template "" break_mount unbreak_mount : :
+
+  template "volatile" break_mount unbreak_mount : :
+}