Fix startup data corruption on persistent rootfs

deploy/demo.sh

@@ -172,6 +172,9 @@ function demo::fix-mounts {
     docker exec "${virtlet_node}" mount --make-shared /boot
   fi
   docker exec "${virtlet_node}" mount --make-shared /sys/fs/cgroup
+  demo::step "Bind-mounting /var/lib/virtlet from a docker volume"
+  docker exec "${virtlet_node}" mkdir -p /dind/virtlet /var/lib/virtlet
+  docker exec "${virtlet_node}" mount --bind /var/lib/virtlet /dind/virtlet
 }
 
 function demo::inject-local-image {

pkg/libvirttools/persistentroot_volumesource.go

@@ -21,8 +21,10 @@ import (
 	"encoding/hex"
 	"fmt"
 
+	"github.com/golang/glog"
 	libvirtxml "github.com/libvirt/libvirt-go-xml"
 	digest "github.com/opencontainers/go-digest"
+	"golang.org/x/sys/unix"
 
 	"github.com/Mirantis/virtlet/pkg/blockdev"
 	"github.com/Mirantis/virtlet/pkg/metadata/types"
@@ -54,24 +56,33 @@ func (v *persistentRootVolume) dmPath() string {
 }
 
 func (v *persistentRootVolume) copyImageToDev(imagePath string) error {
-	_, err := v.owner.Commander().Command("qemu-img", "convert", "-O", "raw", imagePath, v.dmPath()).Run(nil)
-	return err
+	syncFiles(imagePath, v.dev.HostPath, v.dmPath())
+	if _, err := v.owner.Commander().Command("qemu-img", "convert", "-O", "raw", imagePath, v.dmPath()).Run(nil); err != nil {
+		return err
+	}
+	syncFiles(v.dmPath())
+	return nil
 }
 
 func (v *persistentRootVolume) Setup() (*libvirtxml.DomainDisk, *libvirtxml.DomainFilesystem, error) {
+	glog.V(4).Infof("Persistent rootfs setup on %q", v.dev.HostPath)
 	imagePath, imageDigest, imageSize, err := v.owner.ImageManager().GetImagePathDigestAndVirtualSize(v.config.Image)
 	if err != nil {
+		glog.V(4).Infof("Persistent rootfs setup on %q: image info error: %v", v.dev.HostPath, err)
 		return nil, nil, err
 	}
 
 	if imageDigest.Algorithm() != digest.SHA256 {
+		glog.V(4).Infof("Persistent rootfs setup on %q: image info error: %v", v.dev.HostPath, err)
 		return nil, nil, fmt.Errorf("unsupported digest algorithm %q", imageDigest.Algorithm())
 	}
 	imageHash, err := hex.DecodeString(imageDigest.Hex())
 	if err != nil {
+		glog.V(4).Infof("Persistent rootfs setup on %q: bad digest hex: %q", v.dev.HostPath, imageDigest.Hex())
 		return nil, nil, fmt.Errorf("bad digest hex: %q", imageDigest.Hex())
 	}
 	if len(imageHash) != sha256.Size {
+		glog.V(4).Infof("Persistent rootfs setup on %q: bad digest size: %q", v.dev.HostPath, imageDigest.Hex())
 		return nil, nil, fmt.Errorf("bad digest size: %q", imageDigest.Hex())
 	}
 
@@ -81,14 +92,21 @@ func (v *persistentRootVolume) Setup() (*libvirtxml.DomainDisk, *libvirtxml.Doma
 	headerMatches, err := ldh.EnsureDevHeaderMatches(v.dev.HostPath, hash)
 
 	if err == nil {
+		glog.V(4).Infof("Persistent rootfs setup on %q: headerMatches: %v", v.dev.HostPath, headerMatches)
 		err = ldh.Map(v.dev.HostPath, v.dmName(), imageSize)
 	}
 
-	if err == nil && !headerMatches {
-		err = v.copyImageToDev(imagePath)
+	if err == nil {
+		if headerMatches {
+			glog.V(4).Infof("Persistent rootfs setup on %q: header matches image %q, not overwriting", v.dev.HostPath, imagePath)
+		} else {
+			glog.V(4).Infof("Persistent rootfs setup on %q: writing image from %q", v.dev.HostPath, imagePath)
+			err = v.copyImageToDev(imagePath)
+		}
 	}
 
 	if err != nil {
+		glog.V(4).Infof("Persistent rootfs setup on %q: error: %v", v.dev.HostPath, err)
 		return nil, nil, err
 	}
 
@@ -102,3 +120,22 @@ func (v *persistentRootVolume) Setup() (*libvirtxml.DomainDisk, *libvirtxml.Doma
 func (v *persistentRootVolume) Teardown() error {
 	return v.devHandler().Unmap(v.dmName())
 }
+
+func syncFiles(paths ...string) error {
+	// https://www.redhat.com/archives/libguestfs/2012-July/msg00009.html
+	unix.Sync()
+	for _, p := range paths {
+		fd, err := unix.Open(p, unix.O_RDWR|unix.O_SYNC, 0)
+		if err != nil {
+			return err
+		}
+		if err := unix.Fsync(fd); err != nil {
+			unix.Close(fd)
+			return err
+		}
+		if err := unix.Close(fd); err != nil {
+			return err
+		}
+	}
+	return nil
+}

tests/e2e/block_pv_test.go

@@ -50,8 +50,6 @@ var _ = Describe("Block PVs", func() {
 			devPath         string
 		)
 
-		withLoopbackBlockDevice(&virtletNodeName, &devPath)
-
 		AfterEach(func() {
 			if ssh != nil {
 				ssh.Close()
@@ -61,32 +59,38 @@ var _ = Describe("Block PVs", func() {
 			}
 		})
 
-		It("Should be accessible from within the VM", func() {
-			vm = makeVMWithMountAndSymlinkScript(virtletNodeName, []framework.PVCSpec{
-				{
-					Name:          "block-pv",
-					Size:          "10M",
-					NodeName:      virtletNodeName,
-					Block:         true,
-					LocalPath:     devPath,
-					ContainerPath: "/dev/testpvc",
-				},
-			}, nil)
-			ssh = waitSSH(vm)
-			expectToBeUsableForFilesystem(ssh, "/dev/testpvc")
+		Context("[Non-root]", func() {
+			withLoopbackBlockDevice(&virtletNodeName, &devPath, true)
+			It("Should be accessible from within the VM", func() {
+				vm = makeVMWithMountAndSymlinkScript(virtletNodeName, []framework.PVCSpec{
+					{
+						Name:          "block-pv",
+						Size:          "10M",
+						NodeName:      virtletNodeName,
+						Block:         true,
+						LocalPath:     devPath,
+						ContainerPath: "/dev/testpvc",
+					},
+				}, nil)
+				ssh = waitSSH(vm)
+				expectToBeUsableForFilesystem(ssh, "/dev/testpvc")
+			})
 		})
 
-		describePersistentRootfs(func() {
-			vm = makeVMWithMountAndSymlinkScript(virtletNodeName, []framework.PVCSpec{
-				{
-					Name:          "block-pv",
-					Size:          "10M",
-					NodeName:      virtletNodeName,
-					Block:         true,
-					LocalPath:     devPath,
-					ContainerPath: "/",
-				},
-			}, nil)
+		Context("[Root]", func() {
+			withLoopbackBlockDevice(&virtletNodeName, &devPath, false)
+			describePersistentRootfs(func() {
+				vm = makeVMWithMountAndSymlinkScript(virtletNodeName, []framework.PVCSpec{
+					{
+						Name:          "block-pv",
+						Size:          "10M",
+						NodeName:      virtletNodeName,
+						Block:         true,
+						LocalPath:     devPath,
+						ContainerPath: "/",
+					},
+				}, nil)
+			})
 		})
 	})
 

tests/e2e/common.go

@@ -19,6 +19,7 @@ package e2e
 import (
 	"flag"
 	"fmt"
+	"os"
 	"regexp"
 	"strconv"
 	"strings"
@@ -34,7 +35,11 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const cephContainerName = "ceph_cluster"
+const (
+	cephContainerName = "ceph_cluster"
+	// avoid having the loop device on top of overlay2/aufs when using k-d-c
+	loopDeviceTestDir = "/dind/virtlet-e2e-tests"
+)
 
 var (
 	vmImageLocation       = flag.String("image", defaultVMImageLocation, "VM image URL (*without http(s)://*")
@@ -180,35 +185,47 @@ func includeUnsafe() {
 	}
 }
 
-func withLoopbackBlockDevice(virtletNodeName, devPath *string) {
+func withLoopbackBlockDevice(virtletNodeName, devPath *string, mkfs bool) {
 	var nodeExecutor framework.Executor
-	BeforeAll(func() {
+	var filename string
+	BeforeEach(func() {
 		var err error
 		*virtletNodeName, err = controller.VirtletNodeName()
 		Expect(err).NotTo(HaveOccurred())
 		nodeExecutor, err = controller.DinDNodeExecutor(*virtletNodeName)
 		Expect(err).NotTo(HaveOccurred())
 
-		_, err = framework.RunSimple(nodeExecutor, "dd", "if=/dev/zero", "of=/rawdevtest", "bs=1M", "count=1000")
+		_, err = framework.RunSimple(nodeExecutor, "mkdir", "-p", loopDeviceTestDir)
 		Expect(err).NotTo(HaveOccurred())
-		// We use mkfs.ext3 here because mkfs.ext4 on
-		// the node may be too new for CirrOS, causing
-		// errors like this in VM's dmesg:
-		// [    1.316395] EXT3-fs (vdb): error: couldn't mount because of unsupported optional features (2c0)
-		// [    1.320222] EXT4-fs (vdb): couldn't mount RDWR because of unsupported optional features (400)
-		// [    1.339594] EXT3-fs (vdc1): error: couldn't mount because of unsupported optional features (240)
-		// [    1.342850] EXT4-fs (vdc1): mounted filesystem with ordered data mode. Opts: (null)
-		_, err = framework.RunSimple(nodeExecutor, "mkfs.ext3", "/rawdevtest")
+
+		filename, err = framework.RunSimple(nodeExecutor, "tempfile", "-d", loopDeviceTestDir, "--prefix", "ve2e-")
 		Expect(err).NotTo(HaveOccurred())
-		*devPath, err = framework.RunSimple(nodeExecutor, "losetup", "-f", "/rawdevtest", "--show")
+
+		_, err = framework.RunSimple(nodeExecutor, "dd", "if=/dev/zero", "of="+filename, "bs=1M", "count=1000")
+		Expect(err).NotTo(HaveOccurred())
+		if mkfs {
+			// We use mkfs.ext3 here because mkfs.ext4 on
+			// the node may be too new for CirrOS, causing
+			// errors like this in VM's dmesg:
+			// [    1.316395] EXT3-fs (vdb): error: couldn't mount because of unsupported optional features (2c0)
+			// [    1.320222] EXT4-fs (vdb): couldn't mount RDWR because of unsupported optional features (400)
+			// [    1.339594] EXT3-fs (vdc1): error: couldn't mount because of unsupported optional features (240)
+			// [    1.342850] EXT4-fs (vdc1): mounted filesystem with ordered data mode. Opts: (null)
+			_, err = framework.RunSimple(nodeExecutor, "mkfs.ext3", filename)
+			Expect(err).NotTo(HaveOccurred())
+		}
+		_, err = framework.RunSimple(nodeExecutor, "sync")
+		Expect(err).NotTo(HaveOccurred())
+		*devPath, err = framework.RunSimple(nodeExecutor, "losetup", "-f", filename, "--show")
 		Expect(err).NotTo(HaveOccurred())
 	})
 
-	AfterAll(func() {
+	AfterEach(func() {
 		// The loopback device is detached by itself upon
 		// success (TODO: check why it happens), so we
 		// ignore errors here
 		framework.RunSimple(nodeExecutor, "losetup", "-d", *devPath)
+		Expect(os.RemoveAll(loopDeviceTestDir)).To(Succeed())
 	})
 }
 

tests/e2e/volume_mount_test.go

@@ -36,7 +36,7 @@ var _ = Describe("Container volume mounts", func() {
 			ssh             framework.Executor
 		)
 
-		withLoopbackBlockDevice(&virtletNodeName, &devPath)
+		withLoopbackBlockDevice(&virtletNodeName, &devPath, true)
 
 		AfterEach(func() {
 			if ssh != nil {