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 @@ -61,10 +66,17 @@ func TestTemplateFactory(t *testing.T) { err = tt.checkTemplateVM() assert.Nil(err) + err = tt.createTemplateVM(ctx) + assert.Error(err) + + _, err = f.GetBaseVM(ctx, vmConfig) + assert.Nil(err) + + templateProxyType = vc.NoopProxyType err = tt.createTemplateVM(ctx) assert.Nil(err) - _, err = tt.GetBaseVM(ctx) + _, 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() } }() - // VMs booted from template are paused, do not check - if !config.HypervisorConfig.BootFromTemplate { - err = hypervisor.waitSandbox(vmStartTimeout) + // 4. setup proxy + pid, url, proxy, err = setupProxy(hypervisor, agent, config, id) + if err != nil { + return nil, err + } + defer func() { if err != nil { - return nil, err + 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 + } - virtLog.WithField("vm id", id).Info("check agent status") + // 5. check agent aliveness + // VMs booted from template are paused, do not check + if !config.HypervisorConfig.BootFromTemplate { + 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,