Moves NetNS to Config from JailerConfig

This change moves the NetNS field from the JailerConfig to the Config.
This makes the most sense since net namespaces are not subject to only
jailers, but can be used generally as well.

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

Author: xibz

jailer.go

@@ -86,10 +86,6 @@ type JailerConfig struct {
 	// default is /srv/jailer
 	ChrootBaseDir string
 
-	// NetNS represents the path to a network namespace handle. If present, the
-	// jailer will use this to join the associated network namespace
-	NetNS string
-
 	//  Daemonize is set to true, call setsid() and redirect STDIN, STDOUT, and
 	//  STDERR to /dev/null
 	Daemonize bool
@@ -114,13 +110,6 @@ type JailerConfig struct {
 	Stdin io.Reader
 }
 
-func (jailerCfg *JailerConfig) netNSPath() string {
-	if jailerCfg == nil {
-		return ""
-	}
-	return jailerCfg.NetNS
-}
-
 // JailerCommandBuilder will build a jailer command. This can be used to
 // specify that a jailed firecracker executable wants to be run on the Machine.
 type JailerCommandBuilder struct {
@@ -348,8 +337,8 @@ func jail(ctx context.Context, m *Machine, cfg *Config) error {
 		builder = builder.WithBin(jailerBinary)
 	}
 
-	if netNS := cfg.JailerCfg.NetNS; netNS != "" {
-		builder = builder.WithNetNS(netNS)
+	if cfg.NetNS != "" {
+		builder = builder.WithNetNS(cfg.NetNS)
 	}
 
 	if stdin := cfg.JailerCfg.Stdin; stdin != nil {

jailer_test.go

@@ -11,6 +11,7 @@ var testCases = []struct {
 	name             string
 	jailerCfg        JailerConfig
 	expectedArgs     []string
+	netns            string
 	expectedSockPath string
 }{
 	{
@@ -69,15 +70,15 @@ var testCases = []struct {
 		expectedSockPath: filepath.Join(defaultJailerPath, "my-test-id", rootfsFolderName, "api.socket"),
 	},
 	{
-		name: "optional fields",
+		name:  "optional fields",
+		netns: "/path/to/netns",
 		jailerCfg: JailerConfig{
 			ID:             "my-test-id",
 			UID:            Int(123),
 			GID:            Int(100),
 			NumaNode:       Int(1),
 			ChrootStrategy: NewNaiveChrootStrategy("path", "kernel-image-path"),
 			ExecFile:       "/path/to/firecracker",
-			NetNS:          "/net/namespace",
 			ChrootBaseDir:  "/tmp",
 			SeccompLevel:   SeccompLevelAdvanced,
 			JailerBinary:   "/path/to/the/jailer",
@@ -97,7 +98,7 @@ var testCases = []struct {
 			"--chroot-base-dir",
 			"/tmp",
 			"--netns",
-			"/net/namespace",
+			"/path/to/netns",
 			"--seccomp-level",
 			"2",
 		},
@@ -124,8 +125,8 @@ func TestJailerBuilder(t *testing.T) {
 				b = b.WithChrootBaseDir(c.jailerCfg.ChrootBaseDir)
 			}
 
-			if len(c.jailerCfg.NetNS) > 0 {
-				b = b.WithNetNS(c.jailerCfg.NetNS)
+			if c.netns != "" {
+				b = b.WithNetNS(c.netns)
 			}
 
 			if c.jailerCfg.Daemonize {
@@ -150,6 +151,7 @@ func TestJail(t *testing.T) {
 			}
 			cfg := &Config{
 				JailerCfg: &c.jailerCfg,
+				NetNS:     c.netns,
 			}
 			jail(context.Background(), m, cfg)
 

machine.go

@@ -108,6 +108,10 @@ type Config struct {
 	// set the CNI ContainerID and create a network namespace path if
 	// CNI configuration is provided as part of NetworkInterfaces
 	VMID string
+
+	// NetNS represents the path to a network namespace handle. If present, the
+	// application will use this to join the associated network namespace
+	NetNS string
 }
 
 // Validate will ensure that the required fields are set and that
@@ -152,6 +156,7 @@ func (cfg *Config) Validate() error {
 	return nil
 }
 
+// ValidateNetwork .
 func (cfg *Config) ValidateNetwork() error {
 	if cfg.DisableValidation {
 		return nil
@@ -297,6 +302,10 @@ func NewMachine(ctx context.Context, cfg Config, opts ...Opt) (*Machine, error)
 	m.machineConfig = cfg.MachineCfg
 	m.Cfg = cfg
 
+	if cfg.NetNS == "" && cfg.NetworkInterfaces.cniInterface() != nil {
+		m.Cfg.NetNS = m.defaultNetNSPath()
+	}
+
 	m.logger.Debug("Called NewMachine()")
 	return m, nil
 }
@@ -354,24 +363,8 @@ func (m *Machine) Wait(ctx context.Context) error {
 	}
 }
 
-func (m *Machine) netNSPath() string {
-	// If the jailer specifies a netns, use that
-	if jailerNetNS := m.Cfg.JailerCfg.netNSPath(); jailerNetNS != "" {
-		return jailerNetNS
-	}
-
-	// If there isn't a jailer netns but there is a network
-	// interface with CNI configuration, use a default netns path
-	if m.Cfg.NetworkInterfaces.cniInterface() != nil {
-		return filepath.Join(defaultNetNSDir, m.Cfg.VMID)
-	}
-
-	// else, just don't use a netns for the VM
-	return ""
-}
-
 func (m *Machine) setupNetwork(ctx context.Context) error {
-	err, cleanupFuncs := m.Cfg.NetworkInterfaces.setupNetwork(ctx, m.Cfg.VMID, m.netNSPath(), m.logger)
+	err, cleanupFuncs := m.Cfg.NetworkInterfaces.setupNetwork(ctx, m.Cfg.VMID, m.Cfg.NetNS, m.logger)
 	m.cleanupFuncs = append(m.cleanupFuncs, cleanupFuncs...)
 	return err
 }
@@ -421,19 +414,20 @@ func (m *Machine) attachDrives(ctx context.Context, drives ...models.Drive) erro
 	return nil
 }
 
+func (m *Machine) defaultNetNSPath() string {
+	return filepath.Join(defaultNetNSDir, m.Cfg.VMID)
+}
+
 // startVMM starts the firecracker vmm process and configures logging.
 func (m *Machine) startVMM(ctx context.Context) error {
 	m.logger.Printf("Called startVMM(), setting up a VMM on %s", m.Cfg.SocketPath)
-
-	hasNetNS := m.netNSPath() != ""
-	jailerProvidedNetNS := m.Cfg.JailerCfg.netNSPath() != ""
 	startCmd := m.cmd.Start
 
 	var err error
-	if hasNetNS && !jailerProvidedNetNS {
+	if m.Cfg.NetNS != "" && m.Cfg.JailerCfg == nil {
 		// If the VM needs to be started in a netns but no jailer netns was configured,
 		// start the vmm child process in the netns directly here.
-		err = ns.WithNetNSPath(m.netNSPath(), func(_ ns.NetNS) error {
+		err = ns.WithNetNSPath(m.Cfg.NetNS, func(_ ns.NetNS) error {
 			return startCmd()
 		})
 	} else {