diff --git a/virtcontainers/agent.go b/virtcontainers/agent.go index 29d9862bd8..f5ada6ada6 100644 --- a/virtcontainers/agent.go +++ b/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 diff --git a/virtcontainers/cc_proxy.go b/virtcontainers/cc_proxy.go index ef6cce5eb0..fbfa733f28 100644 --- a/virtcontainers/cc_proxy.go +++ b/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 } diff --git a/virtcontainers/cc_proxy_test.go b/virtcontainers/cc_proxy_test.go index ed19855271..faa32e891d 100644 --- a/virtcontainers/cc_proxy_test.go +++ b/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()) } 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..1b1f7888b4 100644 --- a/virtcontainers/factory/factory_test.go +++ b/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 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..a5021f6107 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,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 = f.GetBaseVM(ctx, vmConfig) + assert.Nil(err) + + err = tt.createTemplateVM(ctx) + assert.Nil(err) + + _, err = tt.GetBaseVM(ctx, vmConfig) assert.Nil(err) - _, err = tt.GetBaseVM(ctx) + _, err = f.GetBaseVM(ctx, vmConfig) assert.Nil(err) // CloseFactory diff --git a/virtcontainers/hyperstart_agent.go b/virtcontainers/hyperstart_agent.go index 4c135b8bc4..35d7964200 100644 --- a/virtcontainers/hyperstart_agent.go +++ b/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 +} diff --git a/virtcontainers/hyperstart_agent_test.go b/virtcontainers/hyperstart_agent_test.go index 41b5faf9e1..c17f997c25 100644 --- a/virtcontainers/hyperstart_agent_test.go +++ b/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) +} diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 41e0d1541a..f26a084962 100644 --- a/virtcontainers/kata_agent.go +++ b/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{ - agentURL: agentURL, - logger: k.Logger().WithField("sandbox", sandbox.id), + 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 { diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index 305bfabf81..043f49d79f 100644 --- a/virtcontainers/kata_agent_test.go +++ b/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) + +} diff --git a/virtcontainers/kata_builtin_proxy.go b/virtcontainers/kata_builtin_proxy.go index ebd3cef0c3..ef18d0765b 100644 --- a/virtcontainers/kata_builtin_proxy.go +++ b/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) +func (p *kataBuiltInProxy) start(params proxyParams) (int, string, error) { + if err := p.validateParams(params); err != nil { + return -1, "", err } - p.sandboxID = sandbox.id - console, err := sandbox.hypervisor.getSandboxConsole(sandbox.id) - if err != nil { - return -1, "", err + if p.consoleWatched() { + return -1, "", fmt.Errorf("kata builtin proxy running for sandbox %s", params.id) } - err = p.watchConsole(consoleProtoUnix, console, params.logger) + 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 diff --git a/virtcontainers/kata_builtin_proxy_test.go b/virtcontainers/kata_builtin_proxy_test.go new file mode 100644 index 0000000000..32e9b95b1d --- /dev/null +++ b/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()) +} diff --git a/virtcontainers/kata_proxy.go b/virtcontainers/kata_proxy.go index fe4b01e9c7..1ff9d01f14 100644 --- a/virtcontainers/kata_proxy.go +++ b/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) } diff --git a/virtcontainers/kata_proxy_test.go b/virtcontainers/kata_proxy_test.go index 0e202d7a57..d14fc7bcb5 100644 --- a/virtcontainers/kata_proxy_test.go +++ b/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) } diff --git a/virtcontainers/no_proxy.go b/virtcontainers/no_proxy.go index 33664a56bf..ef97e4a30d 100644 --- a/virtcontainers/no_proxy.go +++ b/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 } diff --git a/virtcontainers/no_proxy_test.go b/virtcontainers/no_proxy_test.go index 4b1bd1b63a..52375eeaf0 100644 --- a/virtcontainers/no_proxy_test.go +++ b/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) - } - - if vmURL != agentURL { - t.Fatalf("Got URL %q, expecting %q", vmURL, agentURL) - } - - 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.start(proxyParams{ + agentURL: agentURL, + }) + assert.NotNil(err) + + pid, vmURL, err := p.start(proxyParams{ + agentURL: agentURL, + logger: testDefaultLogger, + }) + assert.Nil(err) + assert.Equal(vmURL, agentURL) + assert.Equal(pid, 0) + + err = p.stop(0) + assert.Nil(err) + + assert.False(p.consoleWatched()) } diff --git a/virtcontainers/noop_agent.go b/virtcontainers/noop_agent.go index 27eaea301e..e98bcd74fe 100644 --- a/virtcontainers/noop_agent.go +++ b/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 +} diff --git a/virtcontainers/noop_agent_test.go b/virtcontainers/noop_agent_test.go index 144c8dff6c..163e443226 100644 --- a/virtcontainers/noop_agent_test.go +++ b/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) +} diff --git a/virtcontainers/noop_proxy.go b/virtcontainers/noop_proxy.go index 473d8a63ae..5a80f7b60d 100644 --- a/virtcontainers/noop_proxy.go +++ b/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 } 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/proxy.go b/virtcontainers/proxy.go index 4af852254f..050980c3ea 100644 --- a/virtcontainers/proxy.go +++ b/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 { - agentURL string - logger *logrus.Entry + 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 nil +} + +func validateProxyConfig(proxyConfig ProxyConfig) error { + if len(proxyConfig.Path) == 0 { + return fmt.Errorf("Proxy path cannot be empty") } - return config, nil + return nil } -func defaultProxyURL(sandbox *Sandbox, socketType string) (string, error) { +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 diff --git a/virtcontainers/proxy_test.go b/virtcontainers/proxy_test.go index b796d30780..4a396e9e9c 100644 --- a/virtcontainers/proxy_test.go +++ b/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}, - { - &Sandbox{ - config: &SandboxConfig{ - ProxyType: "invalid", - }, - }, - proxyParams{}, - "", true, - }, + {proxyParams{}, "", true}, { - &Sandbox{ - config: &SandboxConfig{ - ProxyType: proxyType, - // invalid - no path - ProxyConfig: ProxyConfig{}, - }, + // no path + proxyParams{ + id: "foobar", + agentURL: "agentURL", + consoleURL: "consoleURL", + logger: testDefaultLogger, }, - proxyParams{}, "", true, }, { - &Sandbox{ - config: &SandboxConfig{ - ProxyType: proxyType, - ProxyConfig: ProxyConfig{ - Path: invalidPath, - }, - }, + // invalid path + proxyParams{ + id: "foobar", + path: invalidPath, + agentURL: "agentURL", + consoleURL: "consoleURL", + logger: testDefaultLogger, }, - proxyParams{}, "", true, }, - { - &Sandbox{ - id: testSandboxID, - agent: agent, - config: &SandboxConfig{ - ProxyType: proxyType, - ProxyConfig: ProxyConfig{ - Path: "echo", - }, - }, - }, + // good case proxyParams{ - agentURL: "agentURL", + 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) +} diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 7d249d975c..a3d8d783db 100644 --- a/virtcontainers/sandbox.go +++ b/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 } 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..40a9729d6b 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, @@ -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) +}