Skip to content

Commit

Permalink
Merge pull request #776 from Mirantis/ivan4th/fix-another-persistent-…
Browse files Browse the repository at this point in the history 
…rootfs-cache-issue

Fix startup data corruption on persistent rootfs
  • Loading branch information
@jellonek
jellonek authored Oct 12, 2018
2 parents ded8cef + f1c883f commit 4d925c7
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 45 deletions.
3 changes: 3 additions & 0 deletions deploy/demo.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,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 {
Expand Down
45 changes: 41 additions & 4 deletions pkg/libvirttools/persistentroot_volumesource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
}

Expand All @@ -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
}

Expand All @@ -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
}
56 changes: 30 additions & 26 deletions tests/e2e/block_pv_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ var _ = Describe("Block PVs", func() {
devPath string
)

withLoopbackBlockDevice(&virtletNodeName, &devPath)

AfterEach(func() {
if ssh != nil {
ssh.Close()
Expand All @@ -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)
})
})
})

Expand Down
45 changes: 31 additions & 14 deletions tests/e2e/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package e2e
import (
"flag"
"fmt"
"os"
"regexp"
"strconv"
"strings"
Expand All @@ -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)://*")
Expand Down Expand Up @@ -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())
})
}

Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/volume_mount_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 4d925c7

Please sign in to comment.