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 <bergwolf@gmail.com>

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)

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

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

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) {
+func (d *direct) GetBaseVM(ctx context.Context, config vc.VMConfig) (*vc.VM, error) {
-	vm, err := vc.NewVM(ctx, d.config)
+	vm, err := vc.NewVM(ctx, config)
 	if err != nil {
 		return nil, err
 	}

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

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) {
+func resetHypervisorConfig(config *vc.VMConfig) {
-	config.NumVCPUs = 0
+	config.HypervisorConfig.NumVCPUs = 0
-	config.MemorySize = 0
+	config.HypervisorConfig.MemorySize = 0
-	config.BootToBeTemplate = false
+	config.HypervisorConfig.BootToBeTemplate = false
-	config.BootFromTemplate = false
+	config.HypervisorConfig.BootFromTemplate = false
-	config.MemoryPath = ""
+	config.HypervisorConfig.MemoryPath = ""
-	config.DevicesStatePath = ""
+	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(&config1)
-	resetHypervisorConfig(&config2.HypervisorConfig)
+	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

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()

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) {
+func (t *template) GetBaseVM(ctx context.Context, config vc.VMConfig) (*vc.VM, error) {
-	return t.createFromTemplateVM(ctx)
+	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 = vm.Disconnect(); err != nil {
-	if err != nil {
 		return err
 	}
 
-	err = vm.Save()
+	// Sleep a bit to let the agent grpc server clean up
-	if err != nil {
+	// 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)
 }

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

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())
+}

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,
+						"sandbox":         s.id,
-						"vfio device ID":  dev.ID,
+						"vfio-device-ID":  dev.ID,
-						"vfio device BDF": dev.BDF,
+						"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,
+						"sandbox":         s.id,
-						"vfio device ID":  dev.ID,
+						"vfio-device-ID":  dev.ID,
-						"vfio device BDF": dev.BDF,
+						"vfio-device-BDF": dev.BDF,
 					}).Error("failed to hot unplug VFIO device")
 				return err
 			}

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)
+		virtLog.WithField("vm", id).Info("check agent status")
-		if err != nil {
-			return nil, err
-		}
-
-		virtLog.WithField("vm id", 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)

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,