From 86a63e69b64db3635b1bfaa28c1a92bb5b5b154c Mon Sep 17 00:00:00 2001 From: Ivan Shvedunov Date: Tue, 9 Oct 2018 19:55:06 +0300 Subject: [PATCH 1/4] Fix possible persistent rootfs data corruption --- .../persistentroot_volumesource.go | 45 +++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/pkg/libvirttools/persistentroot_volumesource.go b/pkg/libvirttools/persistentroot_volumesource.go index f91c64497..18f4464eb 100644 --- a/pkg/libvirttools/persistentroot_volumesource.go +++ b/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 +} From 6c6031f3378f5ab173f46e6e0a7c33a4d2f40563 Mon Sep 17 00:00:00 2001 From: Ivan Shvedunov Date: Tue, 9 Oct 2018 19:57:02 +0300 Subject: [PATCH 2/4] Don't reuse the same file for loopback blockdevs --- tests/e2e/block_pv_test.go | 56 ++++++++++++++++++---------------- tests/e2e/common.go | 44 +++++++++++++++++--------- tests/e2e/volume_mount_test.go | 2 +- 3 files changed, 61 insertions(+), 41 deletions(-) diff --git a/tests/e2e/block_pv_test.go b/tests/e2e/block_pv_test.go index 298d2efb7..0a86373ac 100644 --- a/tests/e2e/block_pv_test.go +++ b/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) + }) }) }) diff --git a/tests/e2e/common.go b/tests/e2e/common.go index 8302f29c6..a28846187 100644 --- a/tests/e2e/common.go +++ b/tests/e2e/common.go @@ -19,6 +19,7 @@ package e2e import ( "flag" "fmt" + "os" "regexp" "strconv" "strings" @@ -34,7 +35,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -const cephContainerName = "ceph_cluster" +const ( + cephContainerName = "ceph_cluster" + loopDeviceTestDir = "/virtlet-e2e-tests" +) var ( vmImageLocation = flag.String("image", defaultVMImageLocation, "VM image URL (*without http(s)://*") @@ -180,35 +184,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()) }) } diff --git a/tests/e2e/volume_mount_test.go b/tests/e2e/volume_mount_test.go index 614531bcc..f4e2bb27b 100644 --- a/tests/e2e/volume_mount_test.go +++ b/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 { From 4b797360315737706ca89cb2d7967681a1cf2fe8 Mon Sep 17 00:00:00 2001 From: Ivan Shvedunov Date: Wed, 10 Oct 2018 22:42:40 +0300 Subject: [PATCH 3/4] Avoid loop file on top of aufs/overlay2 in e2e --- tests/e2e/common.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/e2e/common.go b/tests/e2e/common.go index a28846187..1a31416ff 100644 --- a/tests/e2e/common.go +++ b/tests/e2e/common.go @@ -37,7 +37,8 @@ import ( const ( cephContainerName = "ceph_cluster" - loopDeviceTestDir = "/virtlet-e2e-tests" + // avoid having the loop device on top of overlay2/aufs when using k-d-c + loopDeviceTestDir = "/dind/virtlet-e2e-tests" ) var ( From f1c883f8c3ba60f4eefc5ef47b94c9598f3ae28c Mon Sep 17 00:00:00 2001 From: Ivan Shvedunov Date: Thu, 11 Oct 2018 18:16:49 +0300 Subject: [PATCH 4/4] Move /var/lib/virtlet to a docker volume in demo --- deploy/demo.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/deploy/demo.sh b/deploy/demo.sh index 46d7598a4..5e148a827 100755 --- a/deploy/demo.sh +++ b/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 {