From 07c1f18e5116f6f511609055b28bd7b013f8232c Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Tue, 28 Aug 2018 12:02:06 +0800 Subject: [PATCH] factory: start proxy after create new VM The PR moves ahead the start of proxy process for vm factory so that it waits for both vm and proxy to be up at the same time. This saves about 300ms for new container creation in my local test machine. Fixes: #683 Signed-off-by: Peng Tao --- virtcontainers/factory/base/base.go | 2 +- virtcontainers/factory/cache/cache.go | 4 +- virtcontainers/factory/cache/cache_test.go | 3 +- virtcontainers/factory/direct/direct.go | 4 +- virtcontainers/factory/direct/direct_test.go | 3 +- virtcontainers/factory/factory.go | 44 ++++--- virtcontainers/factory/factory_test.go | 15 +-- virtcontainers/factory/template/template.go | 35 +++-- .../factory/template/template_test.go | 18 ++- virtcontainers/noop_proxy_test.go | 26 ++++ virtcontainers/sandbox.go | 14 +- virtcontainers/vm.go | 122 +++++++++++++++--- virtcontainers/vm_test.go | 1 + 13 files changed, 226 insertions(+), 65 deletions(-) create mode 100644 virtcontainers/noop_proxy_test.go diff --git a/virtcontainers/factory/base/base.go b/virtcontainers/factory/base/base.go index 98f5b27678..cdd05dde91 100644 --- a/virtcontainers/factory/base/base.go +++ b/virtcontainers/factory/base/base.go @@ -21,7 +21,7 @@ type FactoryBase interface { Config() vc.VMConfig // GetBaseVM returns a paused VM created by the base factory. - GetBaseVM(ctx context.Context) (*vc.VM, error) + GetBaseVM(ctx context.Context, config vc.VMConfig) (*vc.VM, error) // CloseFactory closes the base factory. CloseFactory(ctx context.Context) diff --git a/virtcontainers/factory/cache/cache.go b/virtcontainers/factory/cache/cache.go index edc130223e..7a4a093fad 100644 --- a/virtcontainers/factory/cache/cache.go +++ b/virtcontainers/factory/cache/cache.go @@ -37,7 +37,7 @@ func New(ctx context.Context, count uint, b base.FactoryBase) base.FactoryBase { c.wg.Add(1) go func() { for { - vm, err := b.GetBaseVM(ctx) + vm, err := b.GetBaseVM(ctx, c.Config()) if err != nil { c.wg.Done() c.CloseFactory(ctx) @@ -63,7 +63,7 @@ func (c *cache) Config() vc.VMConfig { } // GetBaseVM returns a base VM from cache factory's base factory. -func (c *cache) GetBaseVM(ctx context.Context) (*vc.VM, error) { +func (c *cache) GetBaseVM(ctx context.Context, config vc.VMConfig) (*vc.VM, error) { vm, ok := <-c.cacheCh if ok { return vm, nil diff --git a/virtcontainers/factory/cache/cache_test.go b/virtcontainers/factory/cache/cache_test.go index 800d3eb1b6..4139f6930c 100644 --- a/virtcontainers/factory/cache/cache_test.go +++ b/virtcontainers/factory/cache/cache_test.go @@ -27,6 +27,7 @@ func TestTemplateFactory(t *testing.T) { vmConfig := vc.VMConfig{ HypervisorType: vc.MockHypervisor, AgentType: vc.NoopAgentType, + ProxyType: vc.NoopProxyType, HypervisorConfig: hyperConfig, } @@ -39,7 +40,7 @@ func TestTemplateFactory(t *testing.T) { assert.Equal(f.Config(), vmConfig) // GetBaseVM - _, err := f.GetBaseVM(ctx) + _, err := f.GetBaseVM(ctx, vmConfig) assert.Nil(err) // CloseFactory diff --git a/virtcontainers/factory/direct/direct.go b/virtcontainers/factory/direct/direct.go index 1cc59f52e5..6ae891679b 100644 --- a/virtcontainers/factory/direct/direct.go +++ b/virtcontainers/factory/direct/direct.go @@ -28,8 +28,8 @@ func (d *direct) Config() vc.VMConfig { } // GetBaseVM create a new VM directly. -func (d *direct) GetBaseVM(ctx context.Context) (*vc.VM, error) { - vm, err := vc.NewVM(ctx, d.config) +func (d *direct) GetBaseVM(ctx context.Context, config vc.VMConfig) (*vc.VM, error) { + vm, err := vc.NewVM(ctx, config) if err != nil { return nil, err } diff --git a/virtcontainers/factory/direct/direct_test.go b/virtcontainers/factory/direct/direct_test.go index 58be358a09..20a2548747 100644 --- a/virtcontainers/factory/direct/direct_test.go +++ b/virtcontainers/factory/direct/direct_test.go @@ -26,6 +26,7 @@ func TestTemplateFactory(t *testing.T) { vmConfig := vc.VMConfig{ HypervisorType: vc.MockHypervisor, AgentType: vc.NoopAgentType, + ProxyType: vc.NoopProxyType, HypervisorConfig: hyperConfig, } @@ -38,7 +39,7 @@ func TestTemplateFactory(t *testing.T) { assert.Equal(f.Config(), vmConfig) // GetBaseVM - _, err := f.GetBaseVM(ctx) + _, err := f.GetBaseVM(ctx, vmConfig) assert.Nil(err) // CloseFactory diff --git a/virtcontainers/factory/factory.go b/virtcontainers/factory/factory.go index 94b6359580..b0d308f4e8 100644 --- a/virtcontainers/factory/factory.go +++ b/virtcontainers/factory/factory.go @@ -29,10 +29,6 @@ type Config struct { VMConfig vc.VMConfig } -func (f *Config) validate() error { - return f.VMConfig.Valid() -} - type factory struct { base base.FactoryBase } @@ -50,7 +46,7 @@ func NewFactory(ctx context.Context, config Config, fetchOnly bool) (vc.Factory, span, _ := trace(ctx, "NewFactory") defer span.Finish() - err := config.validate() + err := config.VMConfig.Valid() if err != nil { return nil, err } @@ -93,13 +89,15 @@ func (f *factory) log() *logrus.Entry { return factoryLogger.WithField("subsystem", "factory") } -func resetHypervisorConfig(config *vc.HypervisorConfig) { - config.NumVCPUs = 0 - config.MemorySize = 0 - config.BootToBeTemplate = false - config.BootFromTemplate = false - config.MemoryPath = "" - config.DevicesStatePath = "" +func resetHypervisorConfig(config *vc.VMConfig) { + config.HypervisorConfig.NumVCPUs = 0 + config.HypervisorConfig.MemorySize = 0 + config.HypervisorConfig.BootToBeTemplate = false + config.HypervisorConfig.BootFromTemplate = false + config.HypervisorConfig.MemoryPath = "" + config.HypervisorConfig.DevicesStatePath = "" + config.ProxyType = vc.NoopProxyType + config.ProxyConfig = vc.ProxyConfig{} } // It's important that baseConfig and newConfig are passed by value! @@ -113,8 +111,8 @@ func checkVMConfig(config1, config2 vc.VMConfig) error { } // check hypervisor config details - resetHypervisorConfig(&config1.HypervisorConfig) - resetHypervisorConfig(&config2.HypervisorConfig) + resetHypervisorConfig(&config1) + resetHypervisorConfig(&config2) if !reflect.DeepEqual(config1, config2) { return fmt.Errorf("hypervisor config does not match, base: %+v. new: %+v", config1, config2) @@ -129,13 +127,25 @@ func (f *factory) checkConfig(config vc.VMConfig) error { return checkVMConfig(config, baseConfig) } +func (f *factory) validateNewVMConfig(config vc.VMConfig) error { + if len(config.AgentType.String()) == 0 { + return fmt.Errorf("Missing agent type") + } + + if len(config.ProxyType.String()) == 0 { + return fmt.Errorf("Missing proxy type") + } + + return config.Valid() +} + // GetVM returns a working blank VM created by the factory. func (f *factory) GetVM(ctx context.Context, config vc.VMConfig) (*vc.VM, error) { span, _ := trace(ctx, "GetVM") defer span.Finish() hypervisorConfig := config.HypervisorConfig - err := config.Valid() + err := f.validateNewVMConfig(config) if err != nil { f.log().WithError(err).Error("invalid hypervisor config") return nil, err @@ -144,11 +154,11 @@ func (f *factory) GetVM(ctx context.Context, config vc.VMConfig) (*vc.VM, error) err = f.checkConfig(config) if err != nil { f.log().WithError(err).Info("fallback to direct factory vm") - return direct.New(ctx, config).GetBaseVM(ctx) + return direct.New(ctx, config).GetBaseVM(ctx, config) } f.log().Info("get base VM") - vm, err := f.base.GetBaseVM(ctx) + vm, err := f.base.GetBaseVM(ctx, config) if err != nil { f.log().WithError(err).Error("failed to get base VM") return nil, err diff --git a/virtcontainers/factory/factory_test.go b/virtcontainers/factory/factory_test.go index c20f1f40a8..6340b22ae8 100644 --- a/virtcontainers/factory/factory_test.go +++ b/virtcontainers/factory/factory_test.go @@ -94,23 +94,21 @@ func TestFactorySetLogger(t *testing.T) { func TestVMConfigValid(t *testing.T) { assert := assert.New(t) - config := Config{} - - err := config.validate() - assert.Error(err) - testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") - config.VMConfig = vc.VMConfig{ + config := vc.VMConfig{ HypervisorType: vc.MockHypervisor, AgentType: vc.NoopAgentType, + ProxyType: vc.NoopProxyType, HypervisorConfig: vc.HypervisorConfig{ KernelPath: testDir, ImagePath: testDir, }, } - err = config.validate() + f := factory{} + + err := f.validateNewVMConfig(config) assert.Nil(err) } @@ -165,8 +163,9 @@ func TestFactoryGetVM(t *testing.T) { } vmConfig := vc.VMConfig{ HypervisorType: vc.MockHypervisor, - AgentType: vc.NoopAgentType, HypervisorConfig: hyperConfig, + AgentType: vc.NoopAgentType, + ProxyType: vc.NoopProxyType, } ctx := context.Background() diff --git a/virtcontainers/factory/template/template.go b/virtcontainers/factory/template/template.go index 7daf8a76f9..beea512d23 100644 --- a/virtcontainers/factory/template/template.go +++ b/virtcontainers/factory/template/template.go @@ -23,6 +23,10 @@ type template struct { config vc.VMConfig } +var templateProxyType = vc.KataBuiltInProxyType +var templateWaitForAgent = 2 * time.Second +var templateWaitForMigration = 1 * time.Second + // Fetch finds and returns a pre-built template factory. // TODO: save template metadata and fetch from storage. func Fetch(config vc.VMConfig) (base.FactoryBase, error) { @@ -63,8 +67,8 @@ func (t *template) Config() vc.VMConfig { } // GetBaseVM creates a new paused VM from the template VM. -func (t *template) GetBaseVM(ctx context.Context) (*vc.VM, error) { - return t.createFromTemplateVM(ctx) +func (t *template) GetBaseVM(ctx context.Context, config vc.VMConfig) (*vc.VM, error) { + return t.createFromTemplateVM(ctx, config) } // CloseFactory cleans up the template VM. @@ -100,6 +104,8 @@ func (t *template) createTemplateVM(ctx context.Context) error { config.HypervisorConfig.BootFromTemplate = false config.HypervisorConfig.MemoryPath = t.statePath + "/memory" config.HypervisorConfig.DevicesStatePath = t.statePath + "/state" + // template vm uses builtin proxy + config.ProxyType = templateProxyType vm, err := vc.NewVM(ctx, config) if err != nil { @@ -107,28 +113,41 @@ func (t *template) createTemplateVM(ctx context.Context) error { } defer vm.Stop() - err = vm.Pause() - if err != nil { + if err = vm.Disconnect(); err != nil { return err } - err = vm.Save() - if err != nil { + // Sleep a bit to let the agent grpc server clean up + // When we close connection to the agent, it needs sometime to cleanup + // and restart listening on the communication( serial or vsock) port. + // That time can be saved if we sleep a bit to wait for the agent to + // come around and start listening again. The sleep is only done when + // creating new vm templates and saves time for every new vm that are + // created from template, so it worth the invest. + time.Sleep(templateWaitForAgent) + + if err = vm.Pause(); err != nil { + return err + } + + if err = vm.Save(); err != nil { return err } // qemu QMP does not wait for migration to finish... - time.Sleep(1 * time.Second) + time.Sleep(templateWaitForMigration) return nil } -func (t *template) createFromTemplateVM(ctx context.Context) (*vc.VM, error) { +func (t *template) createFromTemplateVM(ctx context.Context, c vc.VMConfig) (*vc.VM, error) { config := t.config config.HypervisorConfig.BootToBeTemplate = false config.HypervisorConfig.BootFromTemplate = true config.HypervisorConfig.MemoryPath = t.statePath + "/memory" config.HypervisorConfig.DevicesStatePath = t.statePath + "/state" + config.ProxyType = c.ProxyType + config.ProxyConfig = c.ProxyConfig return vc.NewVM(ctx, config) } diff --git a/virtcontainers/factory/template/template_test.go b/virtcontainers/factory/template/template_test.go index 21e23a48ba..09e9cd325a 100644 --- a/virtcontainers/factory/template/template_test.go +++ b/virtcontainers/factory/template/template_test.go @@ -10,6 +10,7 @@ import ( "io/ioutil" "os" "testing" + "time" "github.com/stretchr/testify/assert" @@ -19,6 +20,9 @@ import ( func TestTemplateFactory(t *testing.T) { assert := assert.New(t) + templateWaitForMigration = 1 * time.Microsecond + templateWaitForAgent = 1 * time.Microsecond + testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") hyperConfig := vc.HypervisorConfig{ KernelPath: testDir, @@ -26,8 +30,9 @@ func TestTemplateFactory(t *testing.T) { } vmConfig := vc.VMConfig{ HypervisorType: vc.MockHypervisor, - AgentType: vc.NoopAgentType, HypervisorConfig: hyperConfig, + AgentType: vc.NoopAgentType, + ProxyType: vc.NoopProxyType, } ctx := context.Background() @@ -39,7 +44,7 @@ func TestTemplateFactory(t *testing.T) { assert.Equal(f.Config(), vmConfig) // GetBaseVM - _, err := f.GetBaseVM(ctx) + _, err := f.GetBaseVM(ctx, vmConfig) assert.Nil(err) // Fetch @@ -62,9 +67,16 @@ func TestTemplateFactory(t *testing.T) { assert.Nil(err) err = tt.createTemplateVM(ctx) + assert.Error(err) + + _, err = f.GetBaseVM(ctx, vmConfig) assert.Nil(err) - _, err = tt.GetBaseVM(ctx) + templateProxyType = vc.NoopProxyType + err = tt.createTemplateVM(ctx) + assert.Nil(err) + + _, err = f.GetBaseVM(ctx, vmConfig) assert.Nil(err) // CloseFactory diff --git a/virtcontainers/noop_proxy_test.go b/virtcontainers/noop_proxy_test.go new file mode 100644 index 0000000000..6801a81d4f --- /dev/null +++ b/virtcontainers/noop_proxy_test.go @@ -0,0 +1,26 @@ +// Copyright (c) 2018 HyperHQ Inc. +// +// SPDX-License-Identifier: Apache-2.0 +// + +package virtcontainers + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNoopProxy(t *testing.T) { + n := &noopProxy{} + assert := assert.New(t) + + _, url, err := n.start(proxyParams{}) + assert.Nil(err) + assert.Equal(url, noopProxyURL) + + err = n.stop(0) + assert.Nil(err) + + assert.False(n.consoleWatched()) +} diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index d9aed435b8..50ae41490c 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -1104,6 +1104,8 @@ func (s *Sandbox) startVM() error { HypervisorConfig: s.config.HypervisorConfig, AgentType: s.config.AgentType, AgentConfig: s.config.AgentConfig, + ProxyType: s.config.ProxyType, + ProxyConfig: s.config.ProxyConfig, }) if err != nil { return err @@ -1593,9 +1595,9 @@ func (s *Sandbox) HotplugAddDevice(device api.Device, devType config.DeviceType) if _, err := s.hypervisor.hotplugAddDevice(dev, vfioDev); err != nil { s.Logger(). WithFields(logrus.Fields{ - "sandboxid": s.id, - "vfio device ID": dev.ID, - "vfio device BDF": dev.BDF, + "sandbox": s.id, + "vfio-device-ID": dev.ID, + "vfio-device-BDF": dev.BDF, }).WithError(err).Error("failed to hotplug VFIO device") return err } @@ -1630,9 +1632,9 @@ func (s *Sandbox) HotplugRemoveDevice(device api.Device, devType config.DeviceTy if _, err := s.hypervisor.hotplugRemoveDevice(dev, vfioDev); err != nil { s.Logger().WithError(err). WithFields(logrus.Fields{ - "sandboxid": s.id, - "vfio device ID": dev.ID, - "vfio device BDF": dev.BDF, + "sandbox": s.id, + "vfio-device-ID": dev.ID, + "vfio-device-BDF": dev.BDF, }).Error("failed to hot unplug VFIO device") return err } diff --git a/virtcontainers/vm.go b/virtcontainers/vm.go index 2c8bf6bc66..dd3dc67a31 100644 --- a/virtcontainers/vm.go +++ b/virtcontainers/vm.go @@ -21,6 +21,10 @@ type VM struct { hypervisor hypervisor agent agent + proxy proxy + proxyPid int + proxyURL string + cpu uint32 memory uint32 @@ -34,6 +38,9 @@ type VMConfig struct { AgentType AgentType AgentConfig interface{} + + ProxyType ProxyType + ProxyConfig ProxyConfig } // Valid check VMConfig validity. @@ -41,8 +48,56 @@ func (c *VMConfig) Valid() error { return c.HypervisorConfig.valid() } +func setupProxy(h hypervisor, agent agent, config VMConfig, id string) (int, string, proxy, error) { + consoleURL, err := h.getSandboxConsole(id) + if err != nil { + return -1, "", nil, err + } + agentURL, err := agent.getAgentURL() + if err != nil { + return -1, "", nil, err + } + + // default to kata builtin proxy + proxyType := config.ProxyType + if len(proxyType.String()) == 0 { + proxyType = KataBuiltInProxyType + } + proxy, err := newProxy(proxyType) + if err != nil { + return -1, "", nil, err + } + + proxyParams := proxyParams{ + id: id, + path: config.ProxyConfig.Path, + agentURL: agentURL, + consoleURL: consoleURL, + logger: virtLog.WithField("vm", id), + debug: config.ProxyConfig.Debug, + } + pid, url, err := proxy.start(proxyParams) + if err != nil { + virtLog.WithFields(logrus.Fields{ + "vm": id, + "proxy type": config.ProxyType, + "params": proxyParams, + }).WithError(err).Error("failed to start proxy") + return -1, "", nil, err + } + + return pid, url, proxy, nil +} + // NewVM creates a new VM based on provided VMConfig. func NewVM(ctx context.Context, config VMConfig) (*VM, error) { + var ( + proxy proxy + pid int + url string + ) + + // 1. setup hypervisor hypervisor, err := newHypervisor(config.HypervisorType) if err != nil { return nil, err @@ -54,11 +109,11 @@ func NewVM(ctx context.Context, config VMConfig) (*VM, error) { id := uuid.Generate().String() - virtLog.WithField("vm id", id).WithField("config", config).Info("create new vm") + virtLog.WithField("vm", id).WithField("config", config).Info("create new vm") defer func() { if err != nil { - virtLog.WithField("vm id", id).WithError(err).Error("failed to create new vm") + virtLog.WithField("vm", id).WithError(err).Error("failed to create new vm") } }() @@ -70,37 +125,48 @@ func NewVM(ctx context.Context, config VMConfig) (*VM, error) { return nil, err } + // 2. setup agent agent := newAgent(config.AgentType) - agentConfig := newAgentConfig(config.AgentType, config.AgentConfig) - // do not keep connection for temp agent - if c, ok := agentConfig.(KataAgentConfig); ok { - c.LongLiveConn = false - } vmSharePath := buildVMSharePath(id) - err = agent.configure(hypervisor, id, vmSharePath, true, agentConfig) + err = agent.configure(hypervisor, id, vmSharePath, isProxyBuiltIn(config.ProxyType), config.AgentConfig) if err != nil { return nil, err } + // 3. boot up guest vm if err = hypervisor.startSandbox(); err != nil { return nil, err } + if err = hypervisor.waitSandbox(vmStartTimeout); err != nil { + return nil, err + } defer func() { if err != nil { - virtLog.WithField("vm id", id).WithError(err).Info("clean up vm") + virtLog.WithField("vm", id).WithError(err).Info("clean up vm") hypervisor.stopSandbox() } }() + // 4. setup proxy + pid, url, proxy, err = setupProxy(hypervisor, agent, config, id) + if err != nil { + return nil, err + } + defer func() { + if err != nil { + virtLog.WithField("vm", id).WithError(err).Info("clean up proxy") + proxy.stop(pid) + } + }() + if err = agent.setProxy(nil, proxy, pid, url); err != nil { + return nil, err + } + + // 5. check agent aliveness // VMs booted from template are paused, do not check if !config.HypervisorConfig.BootFromTemplate { - err = hypervisor.waitSandbox(vmStartTimeout) - if err != nil { - return nil, err - } - - virtLog.WithField("vm id", id).Info("check agent status") + virtLog.WithField("vm", id).Info("check agent status") err = agent.check() if err != nil { return nil, err @@ -111,6 +177,9 @@ func NewVM(ctx context.Context, config VMConfig) (*VM, error) { id: id, hypervisor: hypervisor, agent: agent, + proxy: proxy, + proxyPid: pid, + proxyURL: url, cpu: config.HypervisorConfig.NumVCPUs, memory: config.HypervisorConfig.MemorySize, }, nil @@ -121,7 +190,7 @@ func buildVMSharePath(id string) string { } func (v *VM) logger() logrus.FieldLogger { - return virtLog.WithField("vm id", v.id) + return virtLog.WithField("vm", v.id) } // Pause pauses a VM. @@ -148,9 +217,24 @@ func (v *VM) Start() error { return v.hypervisor.startSandbox() } +// Disconnect agent and proxy connections to a VM +func (v *VM) Disconnect() error { + v.logger().Info("kill vm") + + if err := v.agent.disconnect(); err != nil { + v.logger().WithError(err).Error("failed to disconnect agent") + } + if err := v.proxy.stop(v.proxyPid); err != nil { + v.logger().WithError(err).Error("failed to stop proxy") + } + + return nil +} + // Stop stops a VM process. func (v *VM) Stop() error { v.logger().Info("kill vm") + return v.hypervisor.stopSandbox() } @@ -227,8 +311,14 @@ func (v *VM) assignSandbox(s *Sandbox) error { "vmSockDir": vmSockDir, "sbSharePath": sbSharePath, "sbSockDir": sbSockDir, + "proxy-pid": v.proxyPid, + "proxy-url": v.proxyURL, }).Infof("assign vm to sandbox %s", s.id) + if err := s.agent.setProxy(s, v.proxy, v.proxyPid, v.proxyURL); err != nil { + return err + } + // First make sure the symlinks do not exist os.RemoveAll(sbSharePath) os.RemoveAll(sbSockDir) diff --git a/virtcontainers/vm_test.go b/virtcontainers/vm_test.go index 8ecbd5c7da..fa5894c850 100644 --- a/virtcontainers/vm_test.go +++ b/virtcontainers/vm_test.go @@ -20,6 +20,7 @@ func TestNewVM(t *testing.T) { config := VMConfig{ HypervisorType: MockHypervisor, AgentType: NoopAgentType, + ProxyType: NoopProxyType, } hyperConfig := HypervisorConfig{ KernelPath: testDir,