Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Commit

Permalink
vc: Revert "vc: change container rootfs to be a mount"
Browse files Browse the repository at this point in the history
This reverts commit 196661b.

Reverting because cri-o with devicemapper started
to fail after this commit was merged.

Fixes: #1574.
  • Loading branch information
chavafg committed Apr 23, 2019
1 parent 77309f4 commit 18080fd
Show file tree
Hide file tree
Showing 19 changed files with 212 additions and 83 deletions.
2 changes: 1 addition & 1 deletion cli/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func create(ctx context.Context, containerID, bundlePath, console, pidFilePath s
disableOutput := noNeedForOutput(detach, ociSpec.Process.Terminal)

//rootfs has been mounted by containerd shim
rootFs := vc.Mount{Mounted: true}
rootFs := vc.RootFs{Mounted: true}

var process vc.Process
switch containerType {
Expand Down
7 changes: 3 additions & 4 deletions containerd-shim-v2/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ package containerdshim
import (
"context"
"fmt"
"os"
"path/filepath"

"github.com/containerd/typeurl"
vc "github.com/kata-containers/runtime/virtcontainers"
"github.com/kata-containers/runtime/virtcontainers/pkg/oci"
"github.com/pkg/errors"
"os"
"path/filepath"

taskAPI "github.com/containerd/containerd/runtime/v2/task"

Expand All @@ -32,7 +31,7 @@ import (
)

func create(ctx context.Context, s *service, r *taskAPI.CreateTaskRequest, netns string) (*container, error) {
rootFs := vc.Mount{Mounted: s.mount}
rootFs := vc.RootFs{Mounted: s.mount}
if len(r.Rootfs) == 1 {
m := r.Rootfs[0]
rootFs.Source = m.Source
Expand Down
4 changes: 2 additions & 2 deletions pkg/katautils/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func SetEphemeralStorageType(ociSpec oci.CompatOCISpec) oci.CompatOCISpec {
}

// CreateSandbox create a sandbox container
func CreateSandbox(ctx context.Context, vci vc.VC, ociSpec oci.CompatOCISpec, runtimeConfig oci.RuntimeConfig, rootFs vc.Mount,
func CreateSandbox(ctx context.Context, vci vc.VC, ociSpec oci.CompatOCISpec, runtimeConfig oci.RuntimeConfig, rootFs vc.RootFs,
containerID, bundlePath, console string, disableOutput, systemdCgroup, builtIn bool) (vc.VCSandbox, vc.Process, error) {
span, ctx := Trace(ctx, "createSandbox")
defer span.Finish()
Expand Down Expand Up @@ -237,7 +237,7 @@ func CreateSandbox(ctx context.Context, vci vc.VC, ociSpec oci.CompatOCISpec, ru
}

// CreateContainer create a container
func CreateContainer(ctx context.Context, vci vc.VC, sandbox vc.VCSandbox, ociSpec oci.CompatOCISpec, rootFs vc.Mount, containerID, bundlePath, console string, disableOutput, builtIn bool) (vc.Process, error) {
func CreateContainer(ctx context.Context, vci vc.VC, sandbox vc.VCSandbox, ociSpec oci.CompatOCISpec, rootFs vc.RootFs, containerID, bundlePath, console string, disableOutput, builtIn bool) (vc.Process, error) {
var c vc.VCContainer

span, ctx := Trace(ctx, "createContainer")
Expand Down
10 changes: 5 additions & 5 deletions pkg/katautils/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ func TestCreateSandboxConfigFail(t *testing.T) {
Quota: &quota,
}

rootFs := vc.Mount{Mounted: true}
rootFs := vc.RootFs{Mounted: true}

_, _, err = CreateSandbox(context.Background(), testingImpl, spec, runtimeConfig, rootFs, testContainerID, bundlePath, testConsole, true, true, false)
assert.Error(err)
Expand Down Expand Up @@ -342,7 +342,7 @@ func TestCreateSandboxFail(t *testing.T) {
spec, err := readOCIConfigFile(ociConfigFile)
assert.NoError(err)

rootFs := vc.Mount{Mounted: true}
rootFs := vc.RootFs{Mounted: true}

_, _, err = CreateSandbox(context.Background(), testingImpl, spec, runtimeConfig, rootFs, testContainerID, bundlePath, testConsole, true, true, false)
assert.Error(err)
Expand Down Expand Up @@ -381,7 +381,7 @@ func TestCreateContainerContainerConfigFail(t *testing.T) {
err = writeOCIConfigFile(spec, ociConfigFile)
assert.NoError(err)

rootFs := vc.Mount{Mounted: true}
rootFs := vc.RootFs{Mounted: true}

for _, disableOutput := range []bool{true, false} {
_, err = CreateContainer(context.Background(), testingImpl, nil, spec, rootFs, testContainerID, bundlePath, testConsole, disableOutput, false)
Expand Down Expand Up @@ -424,7 +424,7 @@ func TestCreateContainerFail(t *testing.T) {
err = writeOCIConfigFile(spec, ociConfigFile)
assert.NoError(err)

rootFs := vc.Mount{Mounted: true}
rootFs := vc.RootFs{Mounted: true}

for _, disableOutput := range []bool{true, false} {
_, err = CreateContainer(context.Background(), testingImpl, nil, spec, rootFs, testContainerID, bundlePath, testConsole, disableOutput, false)
Expand Down Expand Up @@ -474,7 +474,7 @@ func TestCreateContainer(t *testing.T) {
err = writeOCIConfigFile(spec, ociConfigFile)
assert.NoError(err)

rootFs := vc.Mount{Mounted: true}
rootFs := vc.RootFs{Mounted: true}

for _, disableOutput := range []bool{true, false} {
_, err = CreateContainer(context.Background(), testingImpl, nil, spec, rootFs, testContainerID, bundlePath, testConsole, disableOutput, false)
Expand Down
2 changes: 1 addition & 1 deletion virtcontainers/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ func statusContainer(sandbox *Sandbox, containerID string) (ContainerStatus, err
State: container.state,
PID: container.process.Pid,
StartTime: container.process.StartTime,
RootFs: container.config.RootFs.Destination,
RootFs: container.config.RootFs.Target,
Annotations: container.config.Annotations,
}, nil
}
Expand Down
4 changes: 2 additions & 2 deletions virtcontainers/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func newTestSandboxConfigNoop() SandboxConfig {
// Define the container command and bundle.
container := ContainerConfig{
ID: containerID,
RootFs: Mount{Destination: filepath.Join(testDir, testBundle), Mounted: true},
RootFs: RootFs{Target: filepath.Join(testDir, testBundle), Mounted: true},
Cmd: newBasicTestCmd(),
Annotations: containerAnnotations,
}
Expand Down Expand Up @@ -751,7 +751,7 @@ func newTestContainerConfigNoop(contID string) ContainerConfig {
// Define the container command and bundle.
container := ContainerConfig{
ID: contID,
RootFs: Mount{Destination: filepath.Join(testDir, testBundle), Mounted: true},
RootFs: RootFs{Target: filepath.Join(testDir, testBundle), Mounted: true},
Cmd: newBasicTestCmd(),
Annotations: containerAnnotations,
}
Expand Down
65 changes: 48 additions & 17 deletions virtcontainers/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ type ContainerConfig struct {
ID string

// RootFs is the container workload image on the host.
RootFs Mount
RootFs RootFs

// ReadOnlyRootfs indicates if the rootfs should be mounted readonly
ReadonlyRootfs bool
Expand Down Expand Up @@ -272,13 +272,27 @@ type ContainerDevice struct {
GID uint32
}

// RootFs describes the container's rootfs.
type RootFs struct {
// Source specifies the BlockDevice path
Source string
// Target specify where the rootfs is mounted if it has been mounted
Target string
// Type specifies the type of filesystem to mount.
Type string
// Options specifies zero or more fstab style mount options.
Options []string
// Mounted specifies whether the rootfs has be mounted or not
Mounted bool
}

// Container is composed of a set of containers and a runtime environment.
// A Container can be created, deleted, started, stopped, listed, entered, paused and restored.
type Container struct {
id string
sandboxID string

rootFs Mount
rootFs RootFs

config *ContainerConfig

Expand Down Expand Up @@ -357,6 +371,19 @@ func (c *Container) SetPid(pid int) error {
return c.storeProcess()
}

func (c *Container) setStateFstype(fstype string) error {
c.state.Fstype = fstype

if !c.sandbox.supportNewStore() {
// experimental runtime use "persist.json" which doesn't need "state.json" anymore
if err := c.storeState(); err != nil {
return err
}
}

return nil
}

// GetAnnotations returns container's annotations
func (c *Container) GetAnnotations() map[string]string {
return c.config.Annotations
Expand Down Expand Up @@ -384,6 +411,10 @@ func (c *Container) storeDevices() error {
return c.store.Store(store.DeviceIDs, c.devices)
}

func (c *Container) storeState() error {
return c.store.Store(store.State, c.state)
}

func (c *Container) loadMounts() ([]Mount, error) {
var mounts []Mount
if err := c.store.Load(store.Mounts, &mounts); err != nil {
Expand Down Expand Up @@ -1152,14 +1183,6 @@ func (c *Container) resume() error {
}

func (c *Container) hotplugDrive() error {
if err := c.hotplugRootfsDrive(); err != nil {
return err
}

return nil
}

func (c *Container) hotplugRootfsDrive() error {
var dev device
var err error

Expand All @@ -1169,7 +1192,7 @@ func (c *Container) hotplugRootfsDrive() error {
// there is no "rootfs" dir on block device backed rootfs
c.rootfsSuffix = ""
} else {
dev, err = getDeviceForPath(c.rootFs.Destination)
dev, err = getDeviceForPath(c.rootFs.Target)
}

if err == errMountPointNotFound {
Expand Down Expand Up @@ -1198,15 +1221,14 @@ func (c *Container) hotplugRootfsDrive() error {
devicePath := c.rootFs.Source
fsType := c.rootFs.Type
if c.rootFs.Mounted {
if dev.mountPoint == c.rootFs.Destination {
if dev.mountPoint == c.rootFs.Target {
c.rootfsSuffix = ""
}
// If device mapper device, then fetch the full path of the device
devicePath, fsType, err = GetDevicePathAndFsType(dev.mountPoint)
if err != nil {
return err
}
c.rootFs.Type = fsType
}

devicePath, err = filepath.EvalSymlinks(devicePath)
Expand All @@ -1219,7 +1241,11 @@ func (c *Container) hotplugRootfsDrive() error {
"fs-type": fsType,
}).Info("Block device detected")

return c.plugDevice(devicePath)
if err = c.plugDevice(devicePath); err != nil {
return err
}

return c.setStateFstype(fsType)
}

func (c *Container) plugDevice(devicePath string) error {
Expand All @@ -1240,7 +1266,7 @@ func (c *Container) plugDevice(devicePath string) error {
return fmt.Errorf("device manager failed to create rootfs device for %q: %v", devicePath, err)
}

c.rootFs.BlockDeviceID = b.DeviceID()
c.state.BlockDeviceID = b.DeviceID()

// attach rootfs device
if err := c.sandbox.devManager.AttachDevice(b.DeviceID(), c.sandbox); err != nil {
Expand All @@ -1254,11 +1280,16 @@ func (c *Container) plugDevice(devicePath string) error {
return nil
}

// isDriveUsed checks if a drive has been used for container rootfs
func (c *Container) isDriveUsed() bool {
return !(c.state.Fstype == "")
}

func (c *Container) removeDrive() (err error) {
if c.rootFs.BlockDeviceID != "" {
if c.isDriveUsed() {
c.Logger().Info("unplugging block device")

devID := c.rootFs.BlockDeviceID
devID := c.state.BlockDeviceID
err := c.sandbox.devManager.DetachDevice(devID, c.sandbox)
if err != nil && err != manager.ErrDeviceNotAttached {
return err
Expand Down
16 changes: 8 additions & 8 deletions virtcontainers/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func TestContainerRemoveDrive(t *testing.T) {
id: "testContainer",
}

container.rootFs.Type = ""
container.state.Fstype = ""
err = container.removeDrive()

// hotplugRemoveDevice for hypervisor should not be called.
Expand All @@ -131,8 +131,8 @@ func TestContainerRemoveDrive(t *testing.T) {
err = sandbox.storeSandboxDevices()
assert.Nil(t, err)

container.rootFs.Type = "xfs"
container.rootFs.BlockDeviceID = device.DeviceID()
container.state.Fstype = "xfs"
container.state.BlockDeviceID = device.DeviceID()
err = container.removeDrive()
assert.Nil(t, err, "remove drive should succeed")
}
Expand Down Expand Up @@ -245,7 +245,7 @@ func TestContainerAddDriveDir(t *testing.T) {
container := Container{
sandbox: sandbox,
id: contID,
rootFs: Mount{Destination: fakeRootfs, Mounted: true},
rootFs: RootFs{Target: fakeRootfs, Mounted: true},
}

containerStore, err := store.NewVCContainerStore(sandbox.ctx, sandbox.id, container.id)
Expand All @@ -272,14 +272,14 @@ func TestContainerAddDriveDir(t *testing.T) {
checkStorageDriver = savedFunc
}()

container.rootFs.Type = "xfs"
container.state.Fstype = ""

err = container.hotplugDrive()
if err != nil {
t.Fatalf("Error with hotplugDrive :%v", err)
}

if container.rootFs.Type == "" {
if container.state.Fstype == "" {
t.Fatal()
}
}
Expand Down Expand Up @@ -315,7 +315,7 @@ func TestContainerRootfsPath(t *testing.T) {
container := Container{
id: "rootfstestcontainerid",
sandbox: sandbox,
rootFs: Mount{Destination: fakeRootfs, Mounted: true},
rootFs: RootFs{Target: fakeRootfs, Mounted: true},
rootfsSuffix: "rootfs",
}
cvcstore, err := store.NewVCContainerStore(context.Background(),
Expand All @@ -328,7 +328,7 @@ func TestContainerRootfsPath(t *testing.T) {
assert.Empty(t, container.rootfsSuffix)

// Reset the value to test the other case
container.rootFs = Mount{Destination: fakeRootfs + "/rootfs", Mounted: true}
container.rootFs = RootFs{Target: fakeRootfs + "/rootfs", Mounted: true}
container.rootfsSuffix = "rootfs"

container.hotplugDrive()
Expand Down
2 changes: 1 addition & 1 deletion virtcontainers/example_pod_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"github.com/kata-containers/runtime/virtcontainers/types"
)

var containerRootfs = vc.Mount{Destination: "/var/lib/container/bundle/", Mounted: true}
var containerRootfs = vc.RootFs{Target: "/var/lib/container/bundle/", Mounted: true}

// This example creates and starts a single container sandbox,
// using qemu as the hypervisor and kata as the VM agent.
Expand Down
17 changes: 8 additions & 9 deletions virtcontainers/kata_agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ func (k *kataAgent) rollbackFailingContainerCreation(c *Container) {
}

func (k *kataAgent) buildContainerRootfs(sandbox *Sandbox, c *Container, rootPathParent string) (*grpc.Storage, error) {
if c.rootFs.BlockDeviceID != "" {
if c.state.Fstype != "" && c.state.BlockDeviceID != "" {
// The rootfs storage volume represents the container rootfs
// mount point inside the guest.
// It can be a block based device (when using block based container
Expand All @@ -953,10 +953,10 @@ func (k *kataAgent) buildContainerRootfs(sandbox *Sandbox, c *Container, rootPat
rootfs := &grpc.Storage{}

// This is a block based device rootfs.
device := sandbox.devManager.GetDeviceByID(c.rootFs.BlockDeviceID)
device := sandbox.devManager.GetDeviceByID(c.state.BlockDeviceID)
if device == nil {
k.Logger().WithField("device", c.rootFs.BlockDeviceID).Error("failed to find device by id")
return nil, fmt.Errorf("failed to find device by id %q", c.rootFs.BlockDeviceID)
k.Logger().WithField("device", c.state.BlockDeviceID).Error("failed to find device by id")
return nil, fmt.Errorf("failed to find device by id %q", c.state.BlockDeviceID)
}

blockDrive, ok := device.GetDeviceInfo().(*config.BlockDrive)
Expand All @@ -976,11 +976,10 @@ func (k *kataAgent) buildContainerRootfs(sandbox *Sandbox, c *Container, rootPat
rootfs.Source = blockDrive.SCSIAddr
}
rootfs.MountPoint = rootPathParent
rootfs.Fstype = c.rootFs.Type
rootfs.Options = c.rootFs.Options
rootfs.Fstype = c.state.Fstype

if rootfs.Fstype == "xfs" {
rootfs.Options = append(rootfs.Options, "nouuid")
if c.state.Fstype == "xfs" {
rootfs.Options = []string{"nouuid"}
}

return rootfs, nil
Expand All @@ -994,7 +993,7 @@ func (k *kataAgent) buildContainerRootfs(sandbox *Sandbox, c *Container, rootPat
// (kataGuestSharedDir) is already mounted in the
// guest. We only need to mount the rootfs from
// the host and it will show up in the guest.
if err := bindMountContainerRootfs(k.ctx, kataHostSharedDir, sandbox.id, c.id, c.rootFs.Destination, false); err != nil {
if err := bindMountContainerRootfs(k.ctx, kataHostSharedDir, sandbox.id, c.id, c.rootFs.Target, false); err != nil {
return nil, err
}

Expand Down
Loading

0 comments on commit 18080fd

Please sign in to comment.