From 27a92f94c8bdaa660b230c364479442534cdd5b9 Mon Sep 17 00:00:00 2001 From: Ganesh Maharaj Mahalingam Date: Tue, 5 Mar 2019 13:23:00 -0800 Subject: [PATCH 1/2] runtime: Fix rootfs mount assumptions This patch fixes the issue where various version of snapshotters, overlay, block based graphdriver, containerd-shim-v2 overlay, block based snapshotters mount & create rootfs differently and kata should be able to handle them all. The current version of the code always assumes that a folder named 'rootfs' exists within the mount device and that is the path the container should start at. This patch checks the existing mount point and if it is the same as the rootFs passed to the container, we no longer add a suffix to the container's rootfs path. Fixes: #1325 Signed-off-by: Ganesh Maharaj Mahalingam Co-Authored-by: Manohar Castelino --- virtcontainers/container.go | 6 ++++++ virtcontainers/kata_agent.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 5408d858b7..f11fe997b8 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -276,6 +276,7 @@ type Container struct { runPath string configPath string containerPath string + rootfsSuffix string state types.State @@ -640,6 +641,7 @@ func newContainer(sandbox *Sandbox, contConfig ContainerConfig) (*Container, err runPath: store.ContainerRuntimeRootPath(sandbox.id, contConfig.ID), configPath: store.ContainerConfigurationRootPath(sandbox.id, contConfig.ID), containerPath: filepath.Join(sandbox.id, contConfig.ID), + rootfsSuffix: "rootfs", state: types.State{}, process: Process{}, mounts: contConfig.Mounts, @@ -1131,6 +1133,10 @@ func (c *Container) hotplugDrive() error { return nil } + if dev.mountPoint == c.rootFs { + c.rootfsSuffix = "" + } + // If device mapper device, then fetch the full path of the device devicePath, fsType, err := getDevicePathAndFsType(dev.mountPoint) if err != nil { diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index f07b891068..bec20373e5 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -986,7 +986,7 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process, // This is the guest absolute root path for that container. rootPathParent := filepath.Join(kataGuestSharedDir, c.id) - rootPath := filepath.Join(rootPathParent, rootfsDir) + rootPath := filepath.Join(rootPathParent, c.rootfsSuffix) // In case the container creation fails, the following defer statement // takes care of rolling back actions previously performed. From 45fe8700b8e984fece021aac3e3bb9ddbe3c7aa3 Mon Sep 17 00:00:00 2001 From: Ganesh Maharaj Mahalingam Date: Thu, 7 Mar 2019 15:57:06 -0800 Subject: [PATCH 2/2] runtime: Add unit tests Add unit tests for the rootfs patch Signed-off-by: Ganesh Maharaj Mahalingam --- virtcontainers/container_test.go | 51 ++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/virtcontainers/container_test.go b/virtcontainers/container_test.go index e97d699215..28a56be775 100644 --- a/virtcontainers/container_test.go +++ b/virtcontainers/container_test.go @@ -275,6 +275,57 @@ func TestContainerAddDriveDir(t *testing.T) { } } +func TestContainerRootfsPath(t *testing.T) { + + testRawFile, loopDev, fakeRootfs, err := testSetupFakeRootfs(t) + defer cleanupFakeRootfsSetup(testRawFile, loopDev, fakeRootfs) + assert.Nil(t, err) + + truecheckstoragedriver := checkStorageDriver + checkStorageDriver = func(major, minor int) (bool, error) { + return true, nil + } + defer func() { + checkStorageDriver = truecheckstoragedriver + }() + + sandbox := &Sandbox{ + ctx: context.Background(), + id: "rootfstestsandbox", + agent: &noopAgent{}, + hypervisor: &mockHypervisor{}, + config: &SandboxConfig{ + HypervisorConfig: HypervisorConfig{ + DisableBlockDeviceUse: false, + }, + }, + } + vcstore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id) + sandbox.store = vcstore + assert.Nil(t, err) + container := Container{ + id: "rootfstestcontainerid", + sandbox: sandbox, + rootFs: fakeRootfs, + rootfsSuffix: "rootfs", + } + cvcstore, err := store.NewVCContainerStore(context.Background(), + sandbox.id, + container.id) + assert.Nil(t, err) + container.store = cvcstore + + container.hotplugDrive() + assert.Empty(t, container.rootfsSuffix) + + // Reset the value to test the other case + container.rootFs = fakeRootfs + "/rootfs" + container.rootfsSuffix = "rootfs" + + container.hotplugDrive() + assert.Equal(t, container.rootfsSuffix, "rootfs") +} + func TestCheckSandboxRunningEmptyCmdFailure(t *testing.T) { c := &Container{} err := c.checkSandboxRunning("")