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/hyperstart_agent.go b/virtcontainers/hyperstart_agent.go index c7ee02a3b9..35d7964200 100644 --- a/virtcontainers/hyperstart_agent.go +++ b/virtcontainers/hyperstart_agent.go @@ -389,20 +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(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 } @@ -465,7 +479,18 @@ func (h *hyper) stopSandbox(sandbox *Sandbox) error { return err } - return h.proxy.stop(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 @@ -936,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 088ee4b155..f26a084962 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -471,6 +471,15 @@ 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 { @@ -500,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 { + 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 } @@ -521,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() @@ -611,7 +647,19 @@ func (k *kataAgent) stopSandbox(sandbox *Sandbox) error { return err } - return k.proxy.stop(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 58b7c6edd4..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 { @@ -53,7 +55,7 @@ func (p *kataBuiltInProxy) start(params proxyParams) (int, string, error) { params.logger.Info("Starting builtin kata proxy") p.sandboxID = params.id - err := p.watchConsole(consoleProtoUnix, params.consoleURL, params.logger) + err := p.watchConsole(buildinProxyConsoleProto, params.consoleURL, params.logger) if err != nil { p.sandboxID = "" return -1, "", err 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/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) +}