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

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,10 +30,14 @@ func TestTemplateFactory(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()
 
 	// New
@@ -39,7 +47,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
@@ -48,6 +56,8 @@ func TestTemplateFactory(t *testing.T) {
 		config:    vmConfig,
 	}
 
+	assert.Equal(tt.Config(), vmConfig)
+
 	err = tt.checkTemplateVM()
 	assert.Error(err)
 
@@ -62,9 +72,22 @@ func TestTemplateFactory(t *testing.T) {
 	assert.Nil(err)
 
 	err = tt.createTemplateVM(ctx)
+	assert.Error(err)
+
+	templateProxyType = vc.NoopProxyType
+	_, err = tt.GetBaseVM(ctx, vmConfig)
 	assert.Nil(err)
 
-	_, err = tt.GetBaseVM(ctx)
+	_, err = f.GetBaseVM(ctx, vmConfig)
+	assert.Nil(err)
+
+	err = tt.createTemplateVM(ctx)
+	assert.Nil(err)
+
+	_, err = tt.GetBaseVM(ctx, vmConfig)
+	assert.Nil(err)
+
+	_, err = f.GetBaseVM(ctx, vmConfig)
 	assert.Nil(err)
 
 	// CloseFactory

virtcontainers/hyperstart_agent.go

@@ -389,16 +389,34 @@ func (h *hyper) startProxy(sandbox *Sandbox) error {
 		return nil
 	}
 
+	if h.state.URL != "" {
+		h.Logger().WithFields(logrus.Fields{
+			"sandbox":   sandbox.id,
+			"proxy-pid": h.state.ProxyPid,
+			"proxy-url": h.state.URL,
+		}).Infof("proxy already started")
+		return nil
+	}
+
 	// Start the proxy here
-	pid, uri, err := h.proxy.start(sandbox, proxyParams{})
+	pid, uri, err := h.proxy.start(proxyParams{
+		id:     sandbox.id,
+		path:   sandbox.config.ProxyConfig.Path,
+		debug:  sandbox.config.ProxyConfig.Debug,
+		logger: h.Logger(),
+	})
 	if err != nil {
 		return err
 	}
 
+	defer func() {
+		if err != nil {
+			h.proxy.stop(pid)
+		}
+	}()
+
 	// Fill agent state with proxy information, and store them.
-	h.state.ProxyPid = pid
-	h.state.URL = uri
-	if err := sandbox.storage.storeAgentState(sandbox.id, h.state); err != nil {
+	if err = h.setProxy(sandbox, h.proxy, pid, uri); err != nil {
 		return err
 	}
 
@@ -461,7 +479,18 @@ func (h *hyper) stopSandbox(sandbox *Sandbox) error {
 		return err
 	}
 
-	return h.proxy.stop(sandbox, h.state.ProxyPid)
+	if err := h.proxy.stop(h.state.ProxyPid); err != nil {
+		return err
+	}
+
+	h.state.ProxyPid = -1
+	h.state.URL = ""
+	if err := sandbox.storage.storeAgentState(sandbox.id, h.state); err != nil {
+		// ignore error
+		h.Logger().WithError(err).WithField("sandbox", sandbox.id).Error("failed to clean up agent state")
+	}
+
+	return nil
 }
 
 // handleBlockVolumes handles volumes that are block device files, by
@@ -932,3 +961,29 @@ func (h *hyper) reseedRNG(data []byte) error {
 	// hyperstart-agent does not support reseeding
 	return nil
 }
+
+func (h *hyper) getAgentURL() (string, error) {
+	// hyperstart-agent does not support getting agent url
+	return "", nil
+}
+
+func (h *hyper) setProxy(sandbox *Sandbox, proxy proxy, pid int, url string) error {
+	if url == "" {
+		return fmt.Errorf("invalid empty proxy url")
+	}
+
+	if h.state.URL != "" && h.state.URL != url {
+		h.proxy.stop(h.state.ProxyPid)
+	}
+
+	h.proxy = proxy
+	h.state.ProxyPid = pid
+	h.state.URL = url
+	if sandbox != nil {
+		if err := sandbox.storage.storeAgentState(sandbox.id, h.state); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}

virtcontainers/hyperstart_agent_test.go

@@ -238,3 +238,26 @@ func TestHyperListRoutes(t *testing.T) {
 	_, err := h.listRoutes()
 	assert.Nil(err)
 }
+
+func TestHyperSetProxy(t *testing.T) {
+	assert := assert.New(t)
+
+	h := &hyper{}
+	p := &ccProxy{}
+	s := &Sandbox{storage: &filesystem{}}
+
+	err := h.setProxy(s, p, 0, "")
+	assert.Error(err)
+
+	err = h.setProxy(s, p, 0, "foobar")
+	assert.Error(err)
+}
+
+func TestHyperGetAgentUrl(t *testing.T) {
+	assert := assert.New(t)
+	h := &hyper{}
+
+	url, err := h.getAgentURL()
+	assert.Nil(err)
+	assert.Empty(url)
+}

virtcontainers/kata_agent.go

@@ -471,19 +471,37 @@ func (k *kataAgent) startProxy(sandbox *Sandbox) error {
 		return nil
 	}
 
+	if k.state.URL != "" {
+		k.Logger().WithFields(logrus.Fields{
+			"sandbox":   sandbox.id,
+			"proxy-pid": k.state.ProxyPid,
+			"proxy-url": k.state.URL,
+		}).Infof("proxy already started")
+		return nil
+	}
+
 	// Get agent socket path to provide it to the proxy.
 	agentURL, err := k.agentURL()
 	if err != nil {
 		return err
 	}
 
+	consoleURL, err := sandbox.hypervisor.getSandboxConsole(sandbox.id)
+	if err != nil {
+		return err
+	}
+
 	proxyParams := proxyParams{
+		id:         sandbox.id,
+		path:       sandbox.config.ProxyConfig.Path,
 		agentURL:   agentURL,
+		consoleURL: consoleURL,
 		logger:     k.Logger().WithField("sandbox", sandbox.id),
+		debug:      sandbox.config.ProxyConfig.Debug,
 	}
 
 	// Start the proxy here
-	pid, uri, err := k.proxy.start(sandbox, proxyParams)
+	pid, uri, err := k.proxy.start(proxyParams)
 	if err != nil {
 		return err
 	}
@@ -491,15 +509,13 @@ func (k *kataAgent) startProxy(sandbox *Sandbox) error {
 	// If error occurs after kata-proxy process start,
 	// then rollback to kill kata-proxy process
 	defer func() {
-		if err != nil && pid > 0 {
-			k.proxy.stop(sandbox, pid)
+		if err != nil {
+			k.proxy.stop(pid)
 		}
 	}()
 
 	// Fill agent state with proxy information, and store them.
-	k.state.ProxyPid = pid
-	k.state.URL = uri
-	if err = sandbox.storage.storeAgentState(sandbox.id, k.state); err != nil {
+	if err = k.setProxy(sandbox, k.proxy, pid, uri); err != nil {
 		return err
 	}
 
@@ -512,6 +528,35 @@ func (k *kataAgent) startProxy(sandbox *Sandbox) error {
 	return nil
 }
 
+func (k *kataAgent) getAgentURL() (string, error) {
+	return k.agentURL()
+}
+
+func (k *kataAgent) setProxy(sandbox *Sandbox, proxy proxy, pid int, url string) error {
+	if url == "" {
+		var err error
+		if url, err = k.agentURL(); err != nil {
+			return err
+		}
+	}
+
+	// Are we setting the same proxy again?
+	if k.proxy != nil && k.state.URL != "" && k.state.URL != url {
+		k.proxy.stop(k.state.ProxyPid)
+	}
+
+	k.proxy = proxy
+	k.state.ProxyPid = pid
+	k.state.URL = url
+	if sandbox != nil {
+		if err := sandbox.storage.storeAgentState(sandbox.id, k.state); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func (k *kataAgent) startSandbox(sandbox *Sandbox) error {
 	span, _ := k.trace("startSandbox")
 	defer span.Finish()
@@ -602,7 +647,19 @@ func (k *kataAgent) stopSandbox(sandbox *Sandbox) error {
 		return err
 	}
 
-	return k.proxy.stop(sandbox, k.state.ProxyPid)
+	if err := k.proxy.stop(k.state.ProxyPid); err != nil {
+		return err
+	}
+
+	// clean up agent state
+	k.state.ProxyPid = -1
+	k.state.URL = ""
+	if err := sandbox.storage.storeAgentState(sandbox.id, k.state); err != nil {
+		// ignore error
+		k.Logger().WithError(err).WithField("sandbox", sandbox.id).Error("failed to clean up agent state")
+	}
+
+	return nil
 }
 
 func (k *kataAgent) cleanupSandbox(sandbox *Sandbox) error {

virtcontainers/kata_agent_test.go

@@ -795,3 +795,35 @@ func TestAgentNetworkOperation(t *testing.T) {
 	_, err = k.listRoutes()
 	assert.Nil(err)
 }
+
+func TestKataAgentSetProxy(t *testing.T) {
+	assert := assert.New(t)
+
+	k := &kataAgent{}
+	p := &kataBuiltInProxy{}
+	s := &Sandbox{storage: &filesystem{}}
+
+	err := k.setProxy(s, p, 0, "")
+	assert.Error(err)
+
+	err = k.setProxy(s, p, 0, "foobar")
+	assert.Error(err)
+}
+
+func TestKataGetAgentUrl(t *testing.T) {
+	assert := assert.New(t)
+
+	k := &kataAgent{}
+	err := k.generateVMSocket("foobar", KataAgentConfig{})
+	assert.Nil(err)
+	url, err := k.getAgentURL()
+	assert.Nil(err)
+	assert.NotEmpty(url)
+
+	err = k.generateVMSocket("foobar", KataAgentConfig{UseVSock: true})
+	assert.Nil(err)
+	url, err = k.getAgentURL()
+	assert.Nil(err)
+	assert.NotEmpty(url)
+
+}

virtcontainers/kata_builtin_proxy.go

@@ -14,6 +14,8 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+var buildinProxyConsoleProto = consoleProtoUnix
+
 // This is a kata builtin proxy implementation of the proxy interface. Kata proxy
 // functionality is implemented inside the virtcontainers library.
 type kataBuiltInProxy struct {
@@ -26,22 +28,36 @@ func (p *kataBuiltInProxy) consoleWatched() bool {
 	return p.conn != nil
 }
 
+func (p *kataBuiltInProxy) validateParams(params proxyParams) error {
+	if len(params.id) == 0 || len(params.agentURL) == 0 || len(params.consoleURL) == 0 {
+		return fmt.Errorf("Invalid proxy parameters %+v", params)
+	}
+
+	if params.logger == nil {
+		return fmt.Errorf("Invalid proxy parameter: proxy logger is not set")
+	}
+
+	return nil
+}
+
 // start is the proxy start implementation for kata builtin proxy.
 // It starts the console watcher for the guest.
 // It returns agentURL to let agent connect directly.
-func (p *kataBuiltInProxy) start(sandbox *Sandbox, params proxyParams) (int, string, error) {
-	if p.consoleWatched() {
-		return -1, "", fmt.Errorf("kata builtin proxy running for sandbox %s", p.sandboxID)
-	}
-
-	p.sandboxID = sandbox.id
-	console, err := sandbox.hypervisor.getSandboxConsole(sandbox.id)
-	if err != nil {
+func (p *kataBuiltInProxy) start(params proxyParams) (int, string, error) {
+	if err := p.validateParams(params); err != nil {
 		return -1, "", err
 	}
 
-	err = p.watchConsole(consoleProtoUnix, console, params.logger)
+	if p.consoleWatched() {
+		return -1, "", fmt.Errorf("kata builtin proxy running for sandbox %s", params.id)
+	}
+
+	params.logger.Info("Starting builtin kata proxy")
+
+	p.sandboxID = params.id
+	err := p.watchConsole(buildinProxyConsoleProto, params.consoleURL, params.logger)
 	if err != nil {
+		p.sandboxID = ""
 		return -1, "", err
 	}
 
@@ -49,7 +65,7 @@ func (p *kataBuiltInProxy) start(sandbox *Sandbox, params proxyParams) (int, str
 }
 
 // stop is the proxy stop implementation for kata builtin proxy.
-func (p *kataBuiltInProxy) stop(sandbox *Sandbox, pid int) error {
+func (p *kataBuiltInProxy) stop(pid int) error {
 	if p.conn != nil {
 		p.conn.Close()
 		p.conn = nil

virtcontainers/kata_builtin_proxy_test.go

@@ -0,0 +1,50 @@
+// Copyright (c) 2018 HyperHQ Inc.
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package virtcontainers
+
+import (
+	"testing"
+
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestKataBuiltinProxy(t *testing.T) {
+	assert := assert.New(t)
+
+	p := kataBuiltInProxy{}
+
+	params := proxyParams{}
+
+	err := p.validateParams(params)
+	assert.NotNil(err)
+
+	params.id = "foobarproxy"
+	err = p.validateParams(params)
+	assert.NotNil(err)
+
+	params.agentURL = "foobaragent"
+	err = p.validateParams(params)
+	assert.NotNil(err)
+
+	params.consoleURL = "foobarconsole"
+	err = p.validateParams(params)
+	assert.NotNil(err)
+
+	params.logger = logrus.WithField("proxy", params.id)
+	err = p.validateParams(params)
+	assert.Nil(err)
+
+	buildinProxyConsoleProto = "foobarproto"
+	_, _, err = p.start(params)
+	assert.NotNil(err)
+	assert.Empty(p.sandboxID)
+
+	err = p.stop(0)
+	assert.Nil(err)
+
+	assert.False(p.consoleWatched())
+}

virtcontainers/kata_proxy.go

@@ -6,7 +6,6 @@
 package virtcontainers
 
 import (
-	"fmt"
 	"os/exec"
 	"syscall"
 )
@@ -23,42 +22,28 @@ func (p *kataProxy) consoleWatched() bool {
 }
 
 // start is kataProxy start implementation for proxy interface.
-func (p *kataProxy) start(sandbox *Sandbox, params proxyParams) (int, string, error) {
-	sandbox.Logger().Info("Starting regular Kata proxy rather than built-in")
-	if sandbox.agent == nil {
-		return -1, "", fmt.Errorf("No agent")
-	}
-
-	if params.agentURL == "" {
-		return -1, "", fmt.Errorf("AgentURL cannot be empty")
-	}
-
-	config, err := newProxyConfig(sandbox.config)
-	if err != nil {
+func (p *kataProxy) start(params proxyParams) (int, string, error) {
+	if err := validateProxyParams(params); err != nil {
 		return -1, "", err
 	}
 
+	params.logger.Info("Starting regular Kata proxy rather than built-in")
+
 	// 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,
+		params.path,
 		"-listen-socket", proxyURL,
 		"-mux-socket", params.agentURL,
-		"-sandbox", sandbox.ID(),
+		"-sandbox", params.id,
 	}
 
-	if config.Debug {
-		args = append(args, "-log", "debug")
-		console, err := sandbox.hypervisor.getSandboxConsole(sandbox.id)
-		if err != nil {
-			return -1, "", err
-		}
-
-		args = append(args, "-agent-logs-socket", console)
+	if params.debug {
+		args = append(args, "-log", "debug", "-agent-logs-socket", params.consoleURL)
 	}
 
 	cmd := exec.Command(args[0], args[1:]...)
@@ -70,7 +55,7 @@ func (p *kataProxy) start(sandbox *Sandbox, params proxyParams) (int, string, er
 }
 
 // stop is kataProxy stop implementation for proxy interface.
-func (p *kataProxy) stop(sandbox *Sandbox, pid int) error {
+func (p *kataProxy) stop(pid int) error {
 	// Signal the proxy with SIGTERM.
 	return syscall.Kill(pid, syscall.SIGTERM)
 }

virtcontainers/kata_proxy_test.go

@@ -13,5 +13,5 @@ func TestKataProxyStart(t *testing.T) {
 	agent := &kataAgent{}
 	proxy := &kataProxy{}
 
-	testProxyStart(t, agent, proxy, KataProxyType)
+	testProxyStart(t, agent, proxy)
 }

virtcontainers/no_proxy.go

@@ -23,8 +23,13 @@ type noProxy struct {
 }
 
 // start is noProxy start implementation for proxy interface.
-func (p *noProxy) start(sandbox *Sandbox, params proxyParams) (int, string, error) {
-	sandbox.Logger().Info("No proxy started because of no-proxy implementation")
+func (p *noProxy) start(params proxyParams) (int, string, error) {
+	if params.logger == nil {
+		return -1, "", fmt.Errorf("proxy logger is not set")
+	}
+
+	params.logger.Info("No proxy started because of no-proxy implementation")
+
 	if params.agentURL == "" {
 		return -1, "", fmt.Errorf("AgentURL cannot be empty")
 	}
@@ -33,7 +38,7 @@ func (p *noProxy) start(sandbox *Sandbox, params proxyParams) (int, string, erro
 }
 
 // stop is noProxy stop implementation for proxy interface.
-func (p *noProxy) stop(sandbox *Sandbox, pid int) error {
+func (p *noProxy) stop(pid int) error {
 	return nil
 }
 

virtcontainers/no_proxy_test.go

@@ -7,34 +7,30 @@ package virtcontainers
 
 import (
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestNoProxyStart(t *testing.T) {
-	sandbox := &Sandbox{
-		agent: newAgent(NoopAgentType),
-	}
-
 	p := &noProxy{}
+	assert := assert.New(t)
 
 	agentURL := "agentURL"
-	pid, vmURL, err := p.start(sandbox, proxyParams{agentURL: agentURL})
-	if err != nil {
-		t.Fatal(err)
-	}
+	_, _, err := p.start(proxyParams{
+		agentURL: agentURL,
+	})
+	assert.NotNil(err)
 
-	if vmURL != agentURL {
-		t.Fatalf("Got URL %q, expecting %q", vmURL, agentURL)
-	}
+	pid, vmURL, err := p.start(proxyParams{
+		agentURL: agentURL,
+		logger:   testDefaultLogger,
+	})
+	assert.Nil(err)
+	assert.Equal(vmURL, agentURL)
+	assert.Equal(pid, 0)
 
-	if pid != 0 {
-		t.Fatal("Failure since returned PID should be 0")
-	}
-}
-
-func TestNoProxyStop(t *testing.T) {
-	p := &noProxy{}
-
-	if err := p.stop(&Sandbox{}, 0); err != nil {
-		t.Fatal(err)
-	}
+	err = p.stop(0)
+	assert.Nil(err)
+
+	assert.False(p.consoleWatched())
 }

virtcontainers/noop_agent.go

@@ -187,3 +187,13 @@ func (n *noopAgent) getSharePath(id string) string {
 func (n *noopAgent) reseedRNG(data []byte) error {
 	return nil
 }
+
+// getAgentURL is the Noop agent url getter. It returns nothing.
+func (n *noopAgent) getAgentURL() (string, error) {
+	return "", nil
+}
+
+// setProxy is the Noop agent proxy setter. It does nothing.
+func (n *noopAgent) setProxy(sandbox *Sandbox, proxy proxy, pid int, url string) error {
+	return nil
+}

virtcontainers/noop_agent_test.go

@@ -9,6 +9,8 @@ package virtcontainers
 import (
 	"context"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func testCreateNoopContainer() (*Sandbox, *Container, error) {
@@ -251,3 +253,22 @@ func TestNoopAgentListRoutes(t *testing.T) {
 		t.Fatal("listRoutes failed")
 	}
 }
+
+func TestNoopAgentRSetProxy(t *testing.T) {
+	n := &noopAgent{}
+	p := &noopProxy{}
+	s := &Sandbox{}
+	err := n.setProxy(s, p, 0, "")
+	if err != nil {
+		t.Fatal("set proxy failed")
+	}
+}
+
+func TestNoopGetAgentUrl(t *testing.T) {
+	assert := assert.New(t)
+	n := &noopAgent{}
+
+	url, err := n.getAgentURL()
+	assert.Nil(err)
+	assert.Empty(url)
+}

virtcontainers/noop_proxy.go

@@ -13,13 +13,13 @@ var noopProxyURL = "noopProxyURL"
 
 // register is the proxy start implementation for testing purpose.
 // It does nothing.
-func (p *noopProxy) start(sandbox *Sandbox, params proxyParams) (int, string, error) {
+func (p *noopProxy) start(params proxyParams) (int, string, error) {
 	return 0, noopProxyURL, nil
 }
 
 // stop is the proxy stop implementation for testing purpose.
 // It does nothing.
-func (p *noopProxy) stop(sandbox *Sandbox, pid int) error {
+func (p *noopProxy) stop(pid int) error {
 	return nil
 }
 

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/proxy.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"path/filepath"
 
-	"github.com/mitchellh/mapstructure"
 	"github.com/sirupsen/logrus"
 )
 
@@ -23,8 +22,12 @@ type ProxyConfig struct {
 // proxyParams is the structure providing specific parameters needed
 // for the execution of the proxy binary.
 type proxyParams struct {
+	id         string
+	path       string
 	agentURL   string
+	consoleURL string
 	logger     *logrus.Entry
+	debug      bool
 }
 
 // ProxyType describes a proxy type.
@@ -120,34 +123,30 @@ func newProxy(pType ProxyType) (proxy, error) {
 	}
 }
 
-// newProxyConfig returns a proxy config from a generic SandboxConfig handler,
-// after it properly checked the configuration was valid.
-func newProxyConfig(sandboxConfig *SandboxConfig) (ProxyConfig, error) {
-	if sandboxConfig == nil {
-		return ProxyConfig{}, fmt.Errorf("Sandbox config cannot be nil")
+func validateProxyParams(p proxyParams) error {
+	if len(p.path) == 0 || len(p.id) == 0 || len(p.agentURL) == 0 || len(p.consoleURL) == 0 {
+		return fmt.Errorf("Invalid proxy parameters %+v", p)
 	}
 
-	var config ProxyConfig
-	switch sandboxConfig.ProxyType {
-	case KataProxyType:
-		fallthrough
-	case CCProxyType:
-		if err := mapstructure.Decode(sandboxConfig.ProxyConfig, &config); err != nil {
-			return ProxyConfig{}, err
-		}
+	if p.logger == nil {
+		return fmt.Errorf("Invalid proxy parameter: proxy logger is not set")
 	}
 
-	if config.Path == "" {
-		return ProxyConfig{}, fmt.Errorf("Proxy path cannot be empty")
-	}
-
-	return config, nil
+	return nil
 }
 
-func defaultProxyURL(sandbox *Sandbox, socketType string) (string, error) {
+func validateProxyConfig(proxyConfig ProxyConfig) error {
+	if len(proxyConfig.Path) == 0 {
+		return fmt.Errorf("Proxy path cannot be empty")
+	}
+
+	return nil
+}
+
+func defaultProxyURL(id, socketType string) (string, error) {
 	switch socketType {
 	case SocketTypeUNIX:
-		socketPath := filepath.Join(runStoragePath, sandbox.id, "proxy.sock")
+		socketPath := filepath.Join(runStoragePath, id, "proxy.sock")
 		return fmt.Sprintf("unix://%s", socketPath), nil
 	case SocketTypeVSOCK:
 		// TODO Build the VSOCK default URL
@@ -163,13 +162,13 @@ func isProxyBuiltIn(pType ProxyType) bool {
 
 // proxy is the virtcontainers proxy interface.
 type proxy interface {
-	// start launches a proxy instance for the specified sandbox, returning
+	// start launches a proxy instance with specified parameters, returning
 	// the PID of the process and the URL used to connect to it.
-	start(sandbox *Sandbox, params proxyParams) (int, string, error)
+	start(params proxyParams) (int, string, error)
 
 	// stop terminates a proxy instance after all communications with the
 	// agent inside the VM have been properly stopped.
-	stop(sandbox *Sandbox, pid int) error
+	stop(pid int) error
 
 	//check if the proxy has watched the vm console.
 	consoleWatched() bool

virtcontainers/proxy_test.go

@@ -13,9 +13,12 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 )
 
+var testDefaultLogger = logrus.WithField("proxy", "test")
+
 func testSetProxyType(t *testing.T, value string, expected ProxyType) {
 	var proxyType ProxyType
 
@@ -154,15 +157,15 @@ func TestNewProxyFromUnknownProxyType(t *testing.T) {
 	}
 }
 
-func testNewProxyConfigFromSandboxConfig(t *testing.T, sandboxConfig SandboxConfig, expected ProxyConfig) {
-	result, err := newProxyConfig(&sandboxConfig)
-	if err != nil {
+func testNewProxyFromSandboxConfig(t *testing.T, sandboxConfig SandboxConfig) {
+	if _, err := newProxy(sandboxConfig.ProxyType); err != nil {
 		t.Fatal(err)
 	}
 
-	if reflect.DeepEqual(result, expected) == false {
-		t.Fatalf("Got %+v\nExpecting %+v", result, expected)
+	if err := validateProxyConfig(sandboxConfig.ProxyConfig); err != nil {
+		t.Fatal(err)
 	}
+
 }
 
 var testProxyPath = "proxy-path"
@@ -177,7 +180,7 @@ func TestNewProxyConfigFromCCProxySandboxConfig(t *testing.T) {
 		ProxyConfig: proxyConfig,
 	}
 
-	testNewProxyConfigFromSandboxConfig(t, sandboxConfig, proxyConfig)
+	testNewProxyFromSandboxConfig(t, sandboxConfig)
 }
 
 func TestNewProxyConfigFromKataProxySandboxConfig(t *testing.T) {
@@ -190,22 +193,11 @@ func TestNewProxyConfigFromKataProxySandboxConfig(t *testing.T) {
 		ProxyConfig: proxyConfig,
 	}
 
-	testNewProxyConfigFromSandboxConfig(t, sandboxConfig, proxyConfig)
-}
-
-func TestNewProxyConfigNilSandboxConfigFailure(t *testing.T) {
-	if _, err := newProxyConfig(nil); err == nil {
-		t.Fatal("Should fail because SandboxConfig provided is nil")
-	}
+	testNewProxyFromSandboxConfig(t, sandboxConfig)
 }
 
 func TestNewProxyConfigNoPathFailure(t *testing.T) {
-	sandboxConfig := &SandboxConfig{
-		ProxyType:   CCProxyType,
-		ProxyConfig: ProxyConfig{},
-	}
-
-	if _, err := newProxyConfig(sandboxConfig); err == nil {
+	if err := validateProxyConfig(ProxyConfig{}); err == nil {
 		t.Fatal("Should fail because ProxyConfig has no Path")
 	}
 }
@@ -217,7 +209,7 @@ func testDefaultProxyURL(expectedURL string, socketType string, sandboxID string
 		id: sandboxID,
 	}
 
-	url, err := defaultProxyURL(sandbox, socketType)
+	url, err := defaultProxyURL(sandbox.id, socketType)
 	if err != nil {
 		return err
 	}
@@ -253,7 +245,7 @@ func TestDefaultProxyURLUnknown(t *testing.T) {
 	}
 }
 
-func testProxyStart(t *testing.T, agent agent, proxy proxy, proxyType ProxyType) {
+func testProxyStart(t *testing.T, agent agent, proxy proxy) {
 	assert := assert.New(t)
 
 	assert.NotNil(proxy)
@@ -263,7 +255,6 @@ func testProxyStart(t *testing.T, agent agent, proxy proxy, proxyType ProxyType)
 	defer os.RemoveAll(tmpdir)
 
 	type testData struct {
-		sandbox     *Sandbox
 		params      proxyParams
 		expectedURI string
 		expectError bool
@@ -274,60 +265,43 @@ func testProxyStart(t *testing.T, agent agent, proxy proxy, proxyType ProxyType)
 	expectedURI := fmt.Sprintf("unix://%s", expectedSocketPath)
 
 	data := []testData{
-		{&Sandbox{}, proxyParams{}, "", true},
+		{proxyParams{}, "", true},
 		{
-			&Sandbox{
-				config: &SandboxConfig{
-					ProxyType: "invalid",
-				},
-			},
-			proxyParams{},
-			"", true,
-		},
-		{
-			&Sandbox{
-				config: &SandboxConfig{
-					ProxyType: proxyType,
-					// invalid - no path
-					ProxyConfig: ProxyConfig{},
-				},
-			},
-			proxyParams{},
-			"", true,
-		},
-		{
-			&Sandbox{
-				config: &SandboxConfig{
-					ProxyType: proxyType,
-					ProxyConfig: ProxyConfig{
-						Path: invalidPath,
-					},
-				},
-			},
-			proxyParams{},
-			"", true,
-		},
-
-		{
-			&Sandbox{
-				id:    testSandboxID,
-				agent: agent,
-				config: &SandboxConfig{
-					ProxyType: proxyType,
-					ProxyConfig: ProxyConfig{
-						Path: "echo",
-					},
-				},
-			},
+			// no path
 			proxyParams{
+				id:         "foobar",
 				agentURL:   "agentURL",
+				consoleURL: "consoleURL",
+				logger:     testDefaultLogger,
+			},
+			"", true,
+		},
+		{
+			// invalid path
+			proxyParams{
+				id:         "foobar",
+				path:       invalidPath,
+				agentURL:   "agentURL",
+				consoleURL: "consoleURL",
+				logger:     testDefaultLogger,
+			},
+			"", true,
+		},
+		{
+			// good case
+			proxyParams{
+				id:         testSandboxID,
+				path:       "echo",
+				agentURL:   "agentURL",
+				consoleURL: "consoleURL",
+				logger:     testDefaultLogger,
 			},
 			expectedURI, false,
 		},
 	}
 
 	for _, d := range data {
-		pid, uri, err := proxy.start(d.sandbox, d.params)
+		pid, uri, err := proxy.start(d.params)
 		if d.expectError {
 			assert.Error(err)
 			continue
@@ -338,3 +312,43 @@ func testProxyStart(t *testing.T, agent agent, proxy proxy, proxyType ProxyType)
 		assert.Equal(d.expectedURI, uri)
 	}
 }
+
+func TestValidateProxyConfig(t *testing.T) {
+	assert := assert.New(t)
+
+	config := ProxyConfig{}
+	err := validateProxyConfig(config)
+	assert.Error(err)
+
+	config.Path = "foobar"
+	err = validateProxyConfig(config)
+	assert.Nil(err)
+}
+
+func TestValidateProxyParams(t *testing.T) {
+	assert := assert.New(t)
+
+	p := proxyParams{}
+	err := validateProxyParams(p)
+	assert.Error(err)
+
+	p.path = "foobar"
+	err = validateProxyParams(p)
+	assert.Error(err)
+
+	p.id = "foobar1"
+	err = validateProxyParams(p)
+	assert.Error(err)
+
+	p.agentURL = "foobar2"
+	err = validateProxyParams(p)
+	assert.Error(err)
+
+	p.consoleURL = "foobar3"
+	err = validateProxyParams(p)
+	assert.Error(err)
+
+	p.logger = &logrus.Entry{}
+	err = validateProxyParams(p)
+	assert.Nil(err)
+}

virtcontainers/sandbox.go

@@ -1151,6 +1151,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
@@ -1640,9 +1642,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
 			}
@@ -1677,9 +1679,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
 			}

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

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,
@@ -43,6 +44,8 @@ func TestNewVM(t *testing.T) {
 	assert.Nil(err)
 	err = vm.Start()
 	assert.Nil(err)
+	err = vm.Disconnect()
+	assert.Nil(err)
 	err = vm.Save()
 	assert.Nil(err)
 	err = vm.Stop()
@@ -86,3 +89,24 @@ func TestVMConfigValid(t *testing.T) {
 	err = config.Valid()
 	assert.Nil(err)
 }
+
+func TestSetupProxy(t *testing.T) {
+	assert := assert.New(t)
+
+	config := VMConfig{
+		HypervisorType: MockHypervisor,
+		AgentType:      NoopAgentType,
+	}
+
+	hypervisor := &mockHypervisor{}
+	agent := &noopAgent{}
+
+	// wrong proxy type
+	config.ProxyType = "invalidProxyType"
+	_, _, _, err := setupProxy(hypervisor, agent, config, "foobar")
+	assert.NotNil(err)
+
+	config.ProxyType = NoopProxyType
+	_, _, _, err = setupProxy(hypervisor, agent, config, "foobar")
+	assert.Nil(err)
+}