Adding netns to jailer

This adds netns specification per create vm request. In the event that
no netns was found in the runc config, we will then make sure of what
was passed into the create vm request

Signed-off-by: xibz <[email protected]>

Author: xibz

runtime/firecracker-runc-config.json.example

@@ -96,9 +96,6 @@
             {
                 "type": "pid"
             },
-            {
-                "type": "network"
-            },
             {
                 "type": "ipc"
             },

runtime/jailer.go

@@ -73,5 +73,5 @@ func newJailer(
 	}
 
 	l := logger.WithField("jailer", "runc")
-	return newRuncJailer(ctx, l, ociBundlePath, service.config.JailerConfig.RuncBinaryPath, jailerUID, jailerGID)
+	return newRuncJailer(ctx, l, ociBundlePath, service.config.JailerConfig.RuncBinaryPath, request)
 }

runtime/runc_jailer.go

@@ -33,6 +33,7 @@ import (
 
 	"github.com/firecracker-microvm/firecracker-containerd/internal"
 	"github.com/firecracker-microvm/firecracker-containerd/internal/vm"
+	"github.com/firecracker-microvm/firecracker-containerd/proto"
 )
 
 // runcJailer uses runc to set up a jailed environment for the Firecracker VM.
@@ -46,19 +47,24 @@ type runcJailer struct {
 	runcBinaryPath string
 	uid            uint32
 	gid            uint32
+	netns          string
 }
 
-func newRuncJailer(ctx context.Context, logger *logrus.Entry, ociBundlePath, runcBinPath string, uid, gid uint32) (*runcJailer, error) {
+func newRuncJailer(ctx context.Context, logger *logrus.Entry, ociBundlePath, runcBinPath string, req *proto.CreateVMRequest) (*runcJailer, error) {
 	l := logger.WithField("ociBundlePath", ociBundlePath).
 		WithField("runcBinaryPath", runcBinPath)
+	if req.JailerConfig == nil {
+		return nil, fmt.Errorf("no jailer configuration specified")
+	}
 
 	j := &runcJailer{
 		ctx:            ctx,
 		logger:         l,
 		ociBundlePath:  ociBundlePath,
 		runcBinaryPath: runcBinPath,
-		uid:            uid,
-		gid:            gid,
+		uid:            jailerUID,
+		gid:            jailerGID,
+		netns:          req.JailerConfig.NetworkNamespace,
 	}
 
 	rootPath := j.RootPath()
@@ -347,8 +353,6 @@ func (j runcJailer) jailerCommand(containerName string) *exec.Cmd {
 }
 
 // overwriteConfig will set the proper default values if a field had not been set.
-//
-// TODO: Add netns
 func (j runcJailer) overwriteConfig(cfg *Config, socketPath, configPath string) error {
 	spec := specs.Spec{}
 	configBytes, err := ioutil.ReadFile(configPath)
@@ -376,6 +380,13 @@ func (j runcJailer) overwriteConfig(cfg *Config, socketPath, configPath string)
 	spec.Process.User.UID = j.uid
 	spec.Process.User.GID = j.gid
 
+	if len(j.netns) > 0 && !hasNetworkNamespace(spec.Linux.Namespaces) {
+		spec.Linux.Namespaces = append(spec.Linux.Namespaces, specs.LinuxNamespace{
+			Type: "network",
+			Path: j.netns,
+		})
+	}
+
 	configBytes, err = json.Marshal(&spec)
 	if err != nil {
 		return err
@@ -442,3 +453,13 @@ func mkdirAllWithPermissions(path string, mode os.FileMode, uid, gid uint32) err
 
 	return nil
 }
+
+func hasNetworkNamespace(namespaces []specs.LinuxNamespace) bool {
+	for _, namespace := range namespaces {
+		if namespace.Type == specs.LinuxNamespaceType("network") {
+			return true
+		}
+	}
+
+	return false
+}

runtime/runc_jailer_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/firecracker-microvm/firecracker-containerd/internal"
+	"github.com/firecracker-microvm/firecracker-containerd/proto"
 )
 
 func TestBuildJailedRootHandler_Isolated(t *testing.T) {
@@ -53,7 +54,9 @@ func TestBuildJailedRootHandler_Isolated(t *testing.T) {
 	defer firecrackerFd.Close()
 
 	l := logrus.NewEntry(logrus.New())
-	jailer, err := newRuncJailer(context.Background(), l, dir, "bin-path", 123, 456)
+	jailer, err := newRuncJailer(context.Background(), l, dir, "bin-path", &proto.CreateVMRequest{
+		JailerConfig: &proto.JailerConfig{},
+	})
 	require.NoError(t, err, "failed to create runc jailer")
 
 	cfg := Config{

runtime/service_integ_test.go

@@ -37,6 +37,7 @@ import (
 	"github.com/containerd/containerd/pkg/ttrpcutil"
 	"github.com/containerd/containerd/runtime"
 	"github.com/containerd/typeurl"
+	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/shirou/gopsutil/process"
@@ -225,6 +226,8 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 		cfg.JailerConfig.RuncBinaryPath = "/usr/local/bin/runc"
 	})
 
+	netns, err := ns.GetCurrentNS()
+
 	cases := []struct {
 		MaxContainers int32
 		JailerConfig  *proto.JailerConfig
@@ -240,11 +243,15 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 		},
 		{
 			MaxContainers: 3,
-			JailerConfig:  &proto.JailerConfig{},
+			JailerConfig: &proto.JailerConfig{
+				NetworkNamespace: netns.Path(),
+			},
 		},
 		{
 			MaxContainers: 3,
-			JailerConfig:  &proto.JailerConfig{},
+			JailerConfig: &proto.JailerConfig{
+				NetworkNamespace: netns.Path(),
+			},
 		},
 	}
 
@@ -273,6 +280,7 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 
 			tapName := fmt.Sprintf("tap%d", vmID)
 			err = createTapDevice(ctx, tapName)
+
 			require.NoError(t, err, "failed to create tap device for vm %d", vmID)
 
 			rootfsPath := defaultVMRootfsPath
@@ -299,10 +307,6 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 				JailerConfig:   jailerConfig,
 			}
 
-			if jailerConfig != nil {
-				req.NetworkInterfaces = nil
-			}
-
 			_, err = fcClient.CreateVM(ctx, req)
 			require.NoError(t, err, "failed to create vm")
 
@@ -319,13 +323,6 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 						fmt.Sprintf("/bin/sleep %d", testTimeout/time.Second),
 					}, " && "))
 
-					if jailerConfig != nil {
-						// TODO: this if statement block can go away once we add netns
-						processArgs = oci.WithProcessArgs("/bin/sh", "-c", strings.Join([]string{
-							fmt.Sprintf("/bin/sleep %d", testTimeout/time.Second),
-						}, " && "))
-					}
-
 					// spawn a container that just prints the VM's eth0 mac address (which we have set uniquely per VM)
 					newContainer, err := client.NewContainer(ctx,
 						containerName,
@@ -454,20 +451,13 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 
 						stdoutLines := strings.Split(strings.TrimSpace(taskStdout.String()), "\n")
 						lines := 2
-						if jailerConfig != nil {
-							lines = 1
-						}
 						require.Len(t, stdoutLines, lines)
 
 						printedVMID := strings.TrimSpace(stdoutLines[0])
-						// TODO: Remove this if statement once we can add a netns which
-						// will allow firecracker to have visibility of the tap devices.
-						if jailerConfig == nil {
-							require.Equal(t, vmIDtoMacAddr(uint(vmID)), printedVMID, "unexpected VMID output from container %q", containerName)
+						require.Equal(t, vmIDtoMacAddr(uint(vmID)), printedVMID, "unexpected VMID output from container %q", containerName)
 
-							taskMntNS := strings.TrimSpace(stdoutLines[1])
-							require.Equal(t, execMntNS, taskMntNS, "unexpected mnt NS output from container %q", containerName)
-						}
+						taskMntNS := strings.TrimSpace(stdoutLines[1])
+						require.Equal(t, execMntNS, taskMntNS, "unexpected mnt NS output from container %q", containerName)
 
 					case <-ctx.Done():
 						require.Fail(t, "context cancelled",