agent: add setProxy/getAgentURL interface

Callers can use setProxy to ask agent to use an existing proxy.
agent is modified to rely on its state.URL to tell if an
its proxy is a valid one. And startProxy will skip a valid
proxy since it is already started.

Signed-off-by: Peng Tao <bergwolf@gmail.com>

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/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
+	if err = h.setProxy(sandbox, h.proxy, pid, uri); err != nil {
-	h.state.URL = uri
-	if err := sandbox.storage.storeAgentState(sandbox.id, h.state); 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
+}

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,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
+	if err = k.setProxy(sandbox, k.proxy, pid, uri); err != nil {
-	k.state.URL = uri
-	if err = sandbox.storage.storeAgentState(sandbox.id, k.state); 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 {

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

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