containers: Don't descend into mounts on destroy.

This tells the sad tale of @the-kenny who had bind-mounted his home
directory into a container. After doing `nixos-container destroy` he
discovered that his home directory went from "full of precious data" to
"no more data".

We want to avoid having similar sad tales in the future, so this now also
check this in the containers VM test.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>

2 files changed, 50 insertions, 3 deletions

diff --git a/nixos/modules/virtualisation/nixos-container.pl b/nixos/modules/virtualisation/nixos-container.pl
index 7403a42f0f14..9ae5331786cc 100644
--- a/nixos/modules/virtualisation/nixos-container.pl
+++ b/nixos/modules/virtualisation/nixos-container.pl
@@ -201,15 +201,32 @@ sub runInContainer {
     die "cannot run ‘nsenter’: $!\n";
 }
 
+# Remove a directory while recursively unmounting all mounted filesystems within
+# that directory and unmounting/removing that directory afterwards as well.
+#
+# NOTE: If the specified path is a mountpoint, its contents will be removed,
+#       only mountpoints underneath that path will be unmounted properly.
+sub safeRemoveTree {
+    my ($path) = @_;
+    system("find", $path, "-mindepth", "1", "-xdev",
+           "(", "-type", "d", "-exec", "mountpoint", "-q", "{}", ";", ")",
+           "-exec", "umount", "-fR", "{}", "+");
+    system("rm", "--one-file-system", "-rf", $path);
+    if (-e $path) {
+        system("umount", "-fR", $path);
+        system("rm", "--one-file-system", "-rf", $path);
+    }
+}
+
 if ($action eq "destroy") {
     die "$0: cannot destroy declarative container (remove it from your configuration.nix instead)\n"
         unless POSIX::access($confFile, &POSIX::W_OK);
 
     stopContainer if isContainerRunning;
 
-    rmtree($profileDir) if -e $profileDir;
-    rmtree($gcRootsDir) if -e $gcRootsDir;
-    rmtree($root) if -e $root;
+    safeRemoveTree($profileDir) if -e $profileDir;
+    safeRemoveTree($gcRootsDir) if -e $gcRootsDir;
+    safeRemoveTree($root) if -e $root;
     unlink($confFile) or die;
 }
 
diff --git a/nixos/tests/containers.nix b/nixos/tests/containers.nix
index 13d98d742075..331324139a1a 100644
--- a/nixos/tests/containers.nix
+++ b/nixos/tests/containers.nix
@@ -56,12 +56,35 @@ import ./make-test.nix {
 
       die if $id1 eq $id2;
 
+      # Put the root of $id2 into a bind mount.
+      $machine->succeed(
+        "mv /var/lib/containers/$id2 /id2-bindmount",
+        "mount --bind /id2-bindmount /var/lib/containers/$id1"
+      );
+
       my $ip1 = $machine->succeed("nixos-container show-ip $id1");
       chomp $ip1;
       my $ip2 = $machine->succeed("nixos-container show-ip $id2");
       chomp $ip2;
       die if $ip1 eq $ip2;
 
+      # Create a directory and a file we can later check if it still exists
+      # after destruction of the container.
+      $machine->succeed(
+        "mkdir /nested-bindmount",
+        "echo important data > /nested-bindmount/dummy",
+      );
+
+      # Create a directory with a dummy file and bind-mount it into both
+      # containers.
+      foreach ($id1, $id2) {
+        my $importantPath = "/var/lib/containers/$_/very/important/data";
+        $machine->succeed(
+          "mkdir -p $importantPath",
+          "mount --bind /nested-bindmount $importantPath"
+        );
+      }
+
       # Start one of them.
       $machine->succeed("nixos-container start $id1");
 
@@ -72,6 +95,13 @@ import ./make-test.nix {
       $machine->succeed("nixos-container destroy $id1");
       $machine->succeed("nixos-container destroy $id2");
 
+      $machine->succeed(
+        # Check whether destruction of any container has killed important data
+        "grep -qF 'important data' /nested-bindmount/dummy",
+        # Ensure that the container path is gone
+        "test ! -e /var/lib/containers/$id1"
+      );
+
       # Destroying a declarative container should fail.
       $machine->fail("nixos-container destroy webserver");
     '';