Merge pull request #631 from kzys/volume-runc

Make volume package usable from non-Firecracker runtimes

Author: kzys

runtime/volume_integ_test.go

@@ -20,6 +20,7 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/containerd/containerd"
@@ -39,11 +40,22 @@ const mib = 1024 * 1024
 func TestVolumes_Isolated(t *testing.T) {
 	integtest.Prepare(t)
 
+	runtimes := []string{firecrackerRuntime, "io.containerd.runc.v2"}
+
+	for _, rt := range runtimes {
+		t.Run(rt, func(t *testing.T) {
+			testVolumes(t, rt)
+		})
+	}
+}
+
+func testVolumes(t *testing.T, runtime string) {
 	const vmID = 0
+	testName := strings.ReplaceAll(t.Name(), "/", "_")
 
 	ctx := namespaces.WithNamespace(context.Background(), "default")
 
-	client, err := containerd.New(containerdSockPath, containerd.WithDefaultRuntime(firecrackerRuntime))
+	client, err := containerd.New(containerdSockPath, containerd.WithDefaultRuntime(runtime))
 	require.NoError(t, err, "unable to create client to containerd service at %s, is containerd running?", containerdSockPath)
 	defer client.Close()
 
@@ -54,7 +66,7 @@ func TestVolumes_Isolated(t *testing.T) {
 	require.NoError(t, err, "failed to create fccontrol client")
 
 	// Make volumes.
-	path, err := os.MkdirTemp("", t.Name())
+	path, err := os.MkdirTemp("", testName)
 	require.NoError(t, err)
 
 	f, err := os.Create(filepath.Join(path, "hello.txt"))
@@ -64,22 +76,27 @@ func TestVolumes_Isolated(t *testing.T) {
 	require.NoError(t, err)
 
 	const volName = "volume1"
-	vs := volume.NewSet()
+	vs := volume.NewSet(runtime)
 	vs.Add(volume.FromHost(volName, path))
 
-	// Since CreateVM doesn't take functional options, we need to explicitly create
-	// a FirecrackerDriveMount
-	mount, err := vs.PrepareDriveMount(ctx, 10*mib)
-	require.NoError(t, err)
-
 	containers := []string{"c1", "c2"}
 
-	_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{
-		VMID:           strconv.Itoa(vmID),
-		ContainerCount: int32(len(containers)),
-		DriveMounts:    []*proto.FirecrackerDriveMount{mount},
-	})
-	require.NoError(t, err, "failed to create VM")
+	if runtime == firecrackerRuntime {
+		// Since CreateVM doesn't take functional options, we need to explicitly create
+		// a FirecrackerDriveMount
+		mount, err := vs.PrepareDriveMount(ctx, 10*mib)
+		require.NoError(t, err)
+
+		_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{
+			VMID:           strconv.Itoa(vmID),
+			ContainerCount: int32(len(containers)),
+			DriveMounts:    []*proto.FirecrackerDriveMount{mount},
+		})
+		require.NoError(t, err, "failed to create VM")
+	} else {
+		err := vs.PrepareDirectory(ctx)
+		require.NoError(t, err)
+	}
 
 	// Make containers with the volume.
 	dir := "/path/in/container"
@@ -103,6 +120,8 @@ func TestVolumes_Isolated(t *testing.T) {
 		)
 		require.NoError(t, err, "failed to create container %s", name)
 
+		defer container.Delete(ctx, containerd.WithSnapshotCleanup)
+
 		var stdout, stderr bytes.Buffer
 
 		task, err := container.NewTask(ctx, cio.NewCreator(cio.WithStreams(nil, &stdout, &stderr)))
@@ -135,6 +154,7 @@ func TestVolumes_Isolated(t *testing.T) {
 		),
 	)
 	require.NoError(t, err, "failed to create container %s", name)
+	defer container.Delete(ctx, containerd.WithSnapshotCleanup)
 
 	var stdout, stderr bytes.Buffer
 	task, err := container.NewTask(ctx, cio.NewCreator(cio.WithStreams(nil, &stdout, &stderr)))

tools/docker/Dockerfile.integ-test

@@ -28,6 +28,10 @@ RUN mkdir -p \
     ${FICD_LOG_DIR} \
     /etc/cni/net.d
 
+RUN wget https://github.com/containerd/containerd/releases/download/v1.6.4/containerd-1.6.4-linux-amd64.tar.gz && \
+  tar zxvf containerd-1.6.4-linux-amd64.tar.gz -C /tmp/ && \
+  install -D -o root -g root -m755 -t /usr/local/bin /tmp/bin/containerd-shim-runc-v2 && \
+  rm -rf containerd-1.6.4-linux-amd64.tar.gz /tmp/bin
 
 # Pull the images the tests need into the content store so we don't need internet
 # access during the tests themselves. This runs as a seperate step before the other

volume/set.go

@@ -36,18 +36,20 @@ const (
 
 // Set is a set of volumes.
 type Set struct {
-	volumes map[string]*Volume
-	tempDir string
+	volumes   map[string]*Volume
+	tempDir   string
+	runtime   string
+	volumeDir string
 }
 
 // NewSet returns a new volume set.
-func NewSet() *Set {
-	return NewSetWithTempDir(os.TempDir())
+func NewSet(runtime string) *Set {
+	return NewSetWithTempDir(runtime, os.TempDir())
 }
 
 // NewSetWithTempDir returns a new volume set and creates all temporary files under the tempDir.
-func NewSetWithTempDir(tempDir string) *Set {
-	return &Set{volumes: make(map[string]*Volume), tempDir: tempDir}
+func NewSetWithTempDir(runtime, tempDir string) *Set {
+	return &Set{runtime: runtime, volumes: make(map[string]*Volume), tempDir: tempDir}
 }
 
 // Add a volume to the set.
@@ -96,6 +98,38 @@ func mountDiskImage(source, target string) error {
 	return mount.All([]mount.Mount{{Type: fsType, Source: source, Options: []string{"loop"}}}, target)
 }
 
+// PrepareDirectory creates a directory that have volumes.
+func (vs *Set) PrepareDirectory(ctx context.Context) (retErr error) {
+	dir, err := os.MkdirTemp(vs.tempDir, "Prepare")
+	if err != nil {
+		retErr = err
+		return
+	}
+	defer func() {
+		if retErr != nil {
+			err := os.Remove(dir)
+			if err != nil {
+				retErr = multierror.Append(retErr, err)
+			}
+		}
+	}()
+
+	for _, v := range vs.volumes {
+		path := filepath.Join(dir, v.name)
+		if v.hostPath == "" {
+			continue
+		}
+		err := fs.CopyDir(path, v.hostPath)
+		if err != nil {
+			retErr = fmt.Errorf("failed to copy volume %q: %w", v.name, err)
+			return
+		}
+	}
+
+	vs.volumeDir = dir
+	return
+}
+
 // PrepareDriveMount returns a FirecrackerDriveMount that could be used with CreateVM.
 func (vs *Set) PrepareDriveMount(ctx context.Context, size int64) (dm *proto.FirecrackerDriveMount, retErr error) {
 	path, err := vs.createDiskImage(ctx, size)
@@ -155,6 +189,7 @@ func (vs *Set) PrepareDriveMount(ctx context.Context, size int64) (dm *proto.Fir
 		FilesystemType: fsType,
 		IsWritable:     true,
 	}
+	vs.volumeDir = vmVolumePath
 	return
 }
 
@@ -186,7 +221,7 @@ func (vs *Set) WithMounts(mountpoints []Mount) (oci.SpecOpts, error) {
 		mounts = append(mounts, specs.Mount{
 			// TODO: for volumes that are provided by the guest (e.g. in-VM snapshotters)
 			// We may be able to have bind-mounts from in-VM snapshotters' mount points.
-			Source:      filepath.Join(vmVolumePath, v.name),
+			Source:      filepath.Join(vs.volumeDir, v.name),
 			Destination: mp.Destination,
 			Type:        "bind",
 			Options:     options,