From f24bcda2798e550b9747a74c2ee4554bbf41a911 Mon Sep 17 00:00:00 2001 From: xibz Date: Tue, 5 Nov 2019 01:52:18 +0000 Subject: [PATCH] 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 --- jailer.go | 15 ++------------- jailer_test.go | 12 +++++++----- machine.go | 37 +++++++++++++++---------------------- 3 files changed, 24 insertions(+), 40 deletions(-) diff --git a/jailer.go b/jailer.go index 0dfcc637..5e61c5ec 100644 --- a/jailer.go +++ b/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 { diff --git a/jailer_test.go b/jailer_test.go index ad2f40cd..cb4214c4 100644 --- a/jailer_test.go +++ b/jailer_test.go @@ -11,6 +11,7 @@ var testCases = []struct { name string jailerCfg JailerConfig expectedArgs []string + netns string expectedSockPath string }{ { @@ -69,7 +70,8 @@ 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), @@ -77,7 +79,6 @@ var testCases = []struct { 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) diff --git a/machine.go b/machine.go index bd9cd7b0..c6ba6604 100644 --- a/machine.go +++ b/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 @@ -297,6 +301,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 +362,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 +413,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 {