Add logging setup network

cni/vmconf/vmconf.go

@@ -12,29 +12,31 @@
 // permissions and limitations under the License.
 
 /*
- Package vmconf defines an interface for converting particular CNI invocation
- results to networking configuration usable by a VM. It expects the CNI result
- to have the following properties:
- * The results should contain an interface for a tap device, which will be used
-   as the VM's tap device.
- * The results should contain an interface with the same name as the tap device
-   but with sandbox ID set to the containerID provided during CNI invocation.
-   This should be a "pseudo-interface", not one that has actually been created.
-   It represents the configuration that should be applied to the VM internally.
-   The CNI "containerID" is, in this case, used more as a "vmID" to represent
-   the VM's internal network interface.
-     * If the CNI results specify an IP associated with this interface, that IP
-       should be used to statically configure the VM's internal network interface.
+Package vmconf defines an interface for converting particular CNI invocation
+results to networking configuration usable by a VM. It expects the CNI result
+to have the following properties:
+  - The results should contain an interface for a tap device, which will be used
+    as the VM's tap device.
+  - The results should contain an interface with the same name as the tap device
+    but with sandbox ID set to the containerID provided during CNI invocation.
+    This should be a "pseudo-interface", not one that has actually been created.
+    It represents the configuration that should be applied to the VM internally.
+    The CNI "containerID" is, in this case, used more as a "vmID" to represent
+    the VM's internal network interface.
+  - If the CNI results specify an IP associated with this interface, that IP
+    should be used to statically configure the VM's internal network interface.
 */
 package vmconf
 
 import (
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/containernetworking/cni/pkg/types"
 	current "github.com/containernetworking/cni/pkg/types/100"
 	"github.com/containernetworking/plugins/pkg/ns"
+	log "github.com/sirupsen/logrus"
 
 	"github.com/firecracker-microvm/firecracker-go-sdk/cni/internal"
 )
@@ -88,13 +90,13 @@ type StaticNetworkConf struct {
 //
 // Due to the limitation of "ip=", not all configuration specified in StaticNetworkConf can be
 // applied automatically. In particular:
-// * The MacAddr and MTU cannot be applied
-// * The only routes created will match what's specified in VMIPConfig; VMRoutes will be ignored.
-// * Only up to two namesevers can be supplied. If VMNameservers is has more than 2 entries, only
-//   the first two in the slice will be applied in the VM.
-// * VMDomain, VMSearchDomains and VMResolverOptions will be ignored
-// * Nameserver settings are also only set in /proc/net/pnp. Most applications will thus require
-//   /etc/resolv.conf to be a symlink to /proc/net/pnp in order to resolve names as expected.
+//   - The MacAddr and MTU cannot be applied
+//   - The only routes created will match what's specified in VMIPConfig; VMRoutes will be ignored.
+//   - Only up to two namesevers can be supplied. If VMNameservers is has more than 2 entries, only
+//     the first two in the slice will be applied in the VM.
+//   - VMDomain, VMSearchDomains and VMResolverOptions will be ignored
+//   - Nameserver settings are also only set in /proc/net/pnp. Most applications will thus require
+//     /etc/resolv.conf to be a symlink to /proc/net/pnp in order to resolve names as expected.
 func (c StaticNetworkConf) IPBootParam() string {
 	// See "ip=" section of kernel linked above for details on each field listed below.
 
@@ -149,7 +151,7 @@ func (c StaticNetworkConf) IPBootParam() string {
 // StaticNetworkConfFrom takes the result of a CNI invocation that conforms to the specification
 // in this package's docstring and converts it to a StaticNetworkConf object that the caller
 // can use to configure their VM with.
-func StaticNetworkConfFrom(result types.Result, containerID string) (*StaticNetworkConf, error) {
+func StaticNetworkConfFrom(result types.Result, containerID string, logger *log.Entry) (*StaticNetworkConf, error) {
 	currentResult, err := current.NewResultFromResult(result)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse cni result: %w", err)
@@ -161,28 +163,37 @@ func StaticNetworkConfFrom(result types.Result, containerID string) (*StaticNetw
 	// internal network device.
 	vmIfaceSandbox := containerID
 
+	beforeVMTapPair := time.Now()
 	vmIface, tapIface, err := internal.VMTapPair(currentResult, vmIfaceSandbox)
 	if err != nil {
 		return nil, err
 	}
+	logger.Infof("VMTapPair took %s", time.Since(beforeVMTapPair))
 
 	// find the IP associated with the VM iface
+	beforeInterfaceIPs := time.Now()
 	vmIPs := internal.InterfaceIPs(currentResult, vmIface.Name, vmIface.Sandbox)
 	if len(vmIPs) != 1 {
 		return nil, fmt.Errorf("expected to find 1 IP for vm interface %q, but instead found %+v",
 			vmIface.Name, vmIPs)
 	}
+	logger.Infof("InterfaceIPs took %s", time.Since(beforeInterfaceIPs))
+
 	vmIP := vmIPs[0]
 
+	beforeGetNS := time.Now()
 	netNS, err := ns.GetNS(tapIface.Sandbox)
 	if err != nil {
 		return nil, fmt.Errorf("failed to find netns at path %q: %w", tapIface.Sandbox, err)
 	}
+	logger.Infof("GetNS took %s", time.Since(beforeGetNS))
 
+	beforeMTU := time.Now()
 	tapMTU, err := mtuOf(tapIface.Name, netNS, internal.DefaultNetlinkOps())
 	if err != nil {
 		return nil, err
 	}
+	logger.Infof("MTUOf took %s", time.Since(beforeMTU))
 
 	return &StaticNetworkConf{
 		TapName:           tapIface.Name,

handlers.go

@@ -109,10 +109,6 @@ var JailerConfigValidationHandler = Handler{
 			return fmt.Errorf("UID must be specified when using jailer mode")
 		}
 
-		if m.Cfg.JailerCfg.NumaNode == nil {
-			return fmt.Errorf("ID must be specified when using jailer mode")
-		}
-
 		return nil
 	},
 }

jailer.go

@@ -86,6 +86,9 @@ type JailerConfig struct {
 	// CgroupVersion is the version of the cgroup filesystem to use.
 	CgroupVersion string
 
+	// CgroupArgs are the cgroup arguments to pass to the jailer.
+	CgroupArgs []string
+
 	// Stdout specifies the IO writer for STDOUT to use when spawning the jailer.
 	Stdout io.Writer
 	// Stderr specifies the IO writer for STDERR to use when spawning the jailer.
@@ -102,14 +105,15 @@ type JailerCommandBuilder struct {
 	uid      int
 	gid      int
 	execFile string
-	node     int
 
 	// optional params
 	chrootBaseDir   string
 	netNS           string
 	daemonize       bool
 	firecrackerArgs []string
 	cgroupVersion   string
+	cgroupArgs      []string
+	node            *int
 
 	stdin  io.Reader
 	stdout io.Writer
@@ -139,9 +143,15 @@ func (b JailerCommandBuilder) Args() []string {
 	args = append(args, "--gid", strconv.Itoa(b.gid))
 	args = append(args, "--exec-file", b.execFile)
 
-	if cpulist := getNumaCpuset(b.node); len(cpulist) > 0 {
-		args = append(args, "--cgroup", fmt.Sprintf("cpuset.mems=%d", b.node))
-		args = append(args, "--cgroup", fmt.Sprintf("cpuset.cpus=%s", cpulist))
+	for _, cgroupArg := range b.cgroupArgs {
+		args = append(args, "--cgroup", cgroupArg)
+	}
+
+	if b.node != nil {
+		if cpulist := getNumaCpuset(*b.node); len(cpulist) > 0 {
+			args = append(args, "--cgroup", fmt.Sprintf("cpuset.mems=%d", *b.node))
+			args = append(args, "--cgroup", fmt.Sprintf("cpuset.cpus=%s", cpulist))
+		}
 	}
 
 	if len(b.cgroupVersion) > 0 {
@@ -208,7 +218,14 @@ func (b JailerCommandBuilder) WithExecFile(path string) JailerCommandBuilder {
 // WithNumaNode uses the specfied node for the jailer. This represents the numa
 // node that the process will get assigned to.
 func (b JailerCommandBuilder) WithNumaNode(node int) JailerCommandBuilder {
-	b.node = node
+	b.node = &node
+	return b
+}
+
+// WithCgroupArgs will set the specified cgroup args to the builder. This
+// represents the cgroup settings that the process will get assigned.
+func (b JailerCommandBuilder) WithCgroupArgs(cgroupArgs ...string) JailerCommandBuilder {
+	b.cgroupArgs = cgroupArgs
 	return b
 }
 
@@ -344,11 +361,11 @@ func jail(ctx context.Context, m *Machine, cfg *Config) error {
 		WithID(cfg.JailerCfg.ID).
 		WithUID(*cfg.JailerCfg.UID).
 		WithGID(*cfg.JailerCfg.GID).
-		WithNumaNode(*cfg.JailerCfg.NumaNode).
 		WithExecFile(cfg.JailerCfg.ExecFile).
 		WithChrootBaseDir(cfg.JailerCfg.ChrootBaseDir).
 		WithDaemonize(cfg.JailerCfg.Daemonize).
 		WithCgroupVersion(cfg.JailerCfg.CgroupVersion).
+		WithCgroupArgs(cfg.JailerCfg.CgroupArgs...).
 		WithFirecrackerArgs(fcArgs...).
 		WithStdout(stdout).
 		WithStderr(stderr)
@@ -365,6 +382,10 @@ func jail(ctx context.Context, m *Machine, cfg *Config) error {
 		builder = builder.WithStdin(stdin)
 	}
 
+	if cfg.JailerCfg.NumaNode != nil {
+		builder = builder.WithNumaNode(*cfg.JailerCfg.NumaNode)
+	}
+
 	m.cmd = builder.Build(ctx)
 
 	if err := cfg.JailerCfg.ChrootStrategy.AdaptHandlers(&m.Handlers); err != nil {

jailer_test.go

@@ -34,7 +34,6 @@ func TestJailerBuilder(t *testing.T) {
 				ID:             "my-test-id",
 				UID:            Int(123),
 				GID:            Int(100),
-				NumaNode:       Int(0),
 				ChrootStrategy: NewNaiveChrootStrategy("kernel-image-path"),
 				ExecFile:       "/path/to/firecracker",
 			},
@@ -48,10 +47,6 @@ func TestJailerBuilder(t *testing.T) {
 				"100",
 				"--exec-file",
 				"/path/to/firecracker",
-				"--cgroup",
-				"cpuset.mems=0",
-				"--cgroup",
-				fmt.Sprintf("cpuset.cpus=%s", getNumaCpuset(0)),
 			},
 			expectedSockPath: filepath.Join(
 				defaultJailerPath,
@@ -67,7 +62,6 @@ func TestJailerBuilder(t *testing.T) {
 				ID:             "my-test-id",
 				UID:            Int(123),
 				GID:            Int(100),
-				NumaNode:       Int(0),
 				ChrootStrategy: NewNaiveChrootStrategy("kernel-image-path"),
 				ExecFile:       "/path/to/firecracker",
 				JailerBinary:   "imprisoner",
@@ -82,10 +76,6 @@ func TestJailerBuilder(t *testing.T) {
 				"100",
 				"--exec-file",
 				"/path/to/firecracker",
-				"--cgroup",
-				"cpuset.mems=0",
-				"--cgroup",
-				fmt.Sprintf("cpuset.cpus=%s", getNumaCpuset(0)),
 			},
 			expectedSockPath: filepath.Join(
 				defaultJailerPath,
@@ -145,10 +135,13 @@ func TestJailerBuilder(t *testing.T) {
 				WithID(c.jailerCfg.ID).
 				WithUID(IntValue(c.jailerCfg.UID)).
 				WithGID(IntValue(c.jailerCfg.GID)).
-				WithNumaNode(IntValue(c.jailerCfg.NumaNode)).
 				WithCgroupVersion(c.jailerCfg.CgroupVersion).
 				WithExecFile(c.jailerCfg.ExecFile)
 
+			if c.jailerCfg.NumaNode != nil {
+				b = b.WithNumaNode(IntValue(c.jailerCfg.NumaNode))
+			}
+
 			if len(c.jailerCfg.JailerBinary) > 0 {
 				b = b.WithBin(c.jailerCfg.JailerBinary)
 			}
@@ -188,7 +181,6 @@ func TestJail(t *testing.T) {
 				ID:             "my-test-id",
 				UID:            Int(123),
 				GID:            Int(100),
-				NumaNode:       Int(0),
 				ChrootStrategy: NewNaiveChrootStrategy("kernel-image-path"),
 				ExecFile:       "/path/to/firecracker",
 			},
@@ -202,10 +194,6 @@ func TestJail(t *testing.T) {
 				"100",
 				"--exec-file",
 				"/path/to/firecracker",
-				"--cgroup",
-				"cpuset.mems=0",
-				"--cgroup",
-				fmt.Sprintf("cpuset.cpus=%s", getNumaCpuset(0)),
 				"--",
 				"--no-seccomp",
 				"--api-sock",
@@ -225,7 +213,6 @@ func TestJail(t *testing.T) {
 				ID:             "my-test-id",
 				UID:            Int(123),
 				GID:            Int(100),
-				NumaNode:       Int(0),
 				ChrootStrategy: NewNaiveChrootStrategy("kernel-image-path"),
 				ExecFile:       "/path/to/firecracker",
 				JailerBinary:   "imprisoner",
@@ -240,10 +227,6 @@ func TestJail(t *testing.T) {
 				"100",
 				"--exec-file",
 				"/path/to/firecracker",
-				"--cgroup",
-				"cpuset.mems=0",
-				"--cgroup",
-				fmt.Sprintf("cpuset.cpus=%s", getNumaCpuset(0)),
 				"--",
 				"--no-seccomp",
 				"--api-sock",