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 <[email protected]>
kata-containers · Sep 14, 2018 · 07c1f18 · 07c1f18
1 parent 4738d4e
commit 07c1f18
Show file tree

Hide file tree

Showing 13 changed files with 225 additions and 64 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,35 +104,50 @@ 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 {
 		return err
 	}
 	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
@@ -10,6 +10,7 @@ import (
 	"io/ioutil"
 	"os"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 
@@ -19,15 +20,19 @@ 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,
 		ImagePath:  testDir,
 	}
 	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
@@ -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())
+}