Merge pull request #684 from bergwolf/vm-proxy

factory: start proxy before assign vm to a sandbox

virtcontainers/agent.go

@@ -145,6 +145,12 @@ type agent interface {
 	// start the proxy
 	startProxy(sandbox *Sandbox) error
 
+	// set to use an existing proxy
+	setProxy(sandbox *Sandbox, proxy proxy, pid int, url string) error
+
+	// get agent url
+	getAgentURL() (string, error)
+
 	// createSandbox will tell the agent to perform necessary setup for a Sandbox.
 	createSandbox(sandbox *Sandbox) error
 

virtcontainers/cc_proxy.go

@@ -5,28 +5,27 @@
 
 package virtcontainers
 
-import (
-	"os/exec"
-)
+import "os/exec"
 
 type ccProxy struct {
 }
 
 // start is the proxy start implementation for ccProxy.
-func (p *ccProxy) start(sandbox *Sandbox, params proxyParams) (int, string, error) {
-	config, err := newProxyConfig(sandbox.config)
-	if err != nil {
+func (p *ccProxy) start(params proxyParams) (int, string, error) {
+	if err := validateProxyParams(params); err != nil {
 		return -1, "", err
 	}
 
+	params.logger.Info("Starting cc proxy")
+
 	// construct the socket path the proxy instance will use
-	proxyURL, err := defaultProxyURL(sandbox, SocketTypeUNIX)
+	proxyURL, err := defaultProxyURL(params.id, SocketTypeUNIX)
 	if err != nil {
 		return -1, "", err
 	}
 
-	args := []string{config.Path, "-uri", proxyURL}
-	if config.Debug {
+	args := []string{params.path, "-uri", proxyURL}
+	if params.debug {
 		args = append(args, "-log", "debug")
 	}
 
@@ -38,7 +37,7 @@ func (p *ccProxy) start(sandbox *Sandbox, params proxyParams) (int, string, erro
 	return cmd.Process.Pid, proxyURL, nil
 }
 
-func (p *ccProxy) stop(sandbox *Sandbox, pid int) error {
+func (p *ccProxy) stop(pid int) error {
 	return nil
 }
 

virtcontainers/cc_proxy_test.go

@@ -7,10 +7,22 @@ package virtcontainers
 
 import (
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestCCProxyStart(t *testing.T) {
 	proxy := &ccProxy{}
 
-	testProxyStart(t, nil, proxy, CCProxyType)
+	testProxyStart(t, nil, proxy)
+}
+
+func TestCCProxy(t *testing.T) {
+	proxy := &ccProxy{}
+	assert := assert.New(t)
+
+	err := proxy.stop(0)
+	assert.Nil(err)
+
+	assert.False(proxy.consoleWatched())
 }

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) {
-	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
 	}

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

virtcontainers/factory/factory_test.go

@@ -94,23 +94,27 @@ 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,
 		HypervisorConfig: vc.HypervisorConfig{
 			KernelPath: testDir,
 			ImagePath:  testDir,
 		},
 	}
 
-	err = config.validate()
+	f := factory{}
+
+	err := f.validateNewVMConfig(config)
+	assert.NotNil(err)
+
+	config.AgentType = vc.NoopAgentType
+	err = f.validateNewVMConfig(config)
+	assert.NotNil(err)
+
+	config.ProxyType = vc.NoopProxyType
+	err = f.validateNewVMConfig(config)
 	assert.Nil(err)
 }
 
@@ -165,10 +169,14 @@ func TestFactoryGetVM(t *testing.T) {
 	}
 	vmConfig := vc.VMConfig{
 		HypervisorType:   vc.MockHypervisor,
-		AgentType:        vc.NoopAgentType,
 		HypervisorConfig: hyperConfig,
+		AgentType:        vc.NoopAgentType,
+		ProxyType:        vc.NoopProxyType,
 	}
 
+	err := vmConfig.Valid()
+	assert.Nil(err)
+
 	ctx := context.Background()
 
 	// direct factory

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