proxy: decouple from sandbox

A proxy is mostly associated with an agent. Decouple it from sandbox
so that we can start it before linking vm with an actual sandbox.

Signed-off-by: Peng Tao <[email protected]>

virtcontainers/cc_proxy.go

@@ -5,33 +5,27 @@
 
 package virtcontainers
 
-import (
-	"fmt"
-	"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) {
-	if sandbox.config == nil {
-		return -1, "", fmt.Errorf("Sandbox config cannot be nil")
-	}
-
-	config := sandbox.config.ProxyConfig
-	if err := validateProxyConfig(config); 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")
 	}
 
@@ -43,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

@@ -12,5 +12,5 @@ import (
 func TestCCProxyStart(t *testing.T) {
 	proxy := &ccProxy{}
 
-	testProxyStart(t, nil, proxy, CCProxyType)
+	testProxyStart(t, nil, proxy)
 }

virtcontainers/hyperstart_agent.go

@@ -390,7 +390,11 @@ func (h *hyper) startProxy(sandbox *Sandbox) error {
 	}
 
 	// 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,
+		logger: h.Logger(),
+	})
 	if err != nil {
 		return err
 	}
@@ -461,7 +465,7 @@ func (h *hyper) stopSandbox(sandbox *Sandbox) error {
 		return err
 	}
 
-	return h.proxy.stop(sandbox, h.state.ProxyPid)
+	return h.proxy.stop(h.state.ProxyPid)
 }
 
 // handleBlockVolumes handles volumes that are block device files, by

virtcontainers/kata_agent.go

@@ -477,13 +477,22 @@ func (k *kataAgent) startProxy(sandbox *Sandbox) error {
 		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
 	}
@@ -492,7 +501,7 @@ func (k *kataAgent) startProxy(sandbox *Sandbox) error {
 	// then rollback to kill kata-proxy process
 	defer func() {
 		if err != nil && pid > 0 {
-			k.proxy.stop(sandbox, pid)
+			k.proxy.stop(pid)
 		}
 	}()
 
@@ -602,7 +611,7 @@ func (k *kataAgent) stopSandbox(sandbox *Sandbox) error {
 		return err
 	}
 
-	return k.proxy.stop(sandbox, k.state.ProxyPid)
+	return k.proxy.stop(k.state.ProxyPid)
 }
 
 func (k *kataAgent) cleanupSandbox(sandbox *Sandbox) error {

virtcontainers/kata_builtin_proxy.go

@@ -26,30 +26,44 @@ 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(consoleProtoUnix, params.consoleURL, params.logger)
 	if err != nil {
+		p.sandboxID = ""
 		return -1, "", err
 	}
 
 	return -1, params.agentURL, nil
 }
 
 // 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_proxy.go

@@ -6,7 +6,6 @@
 package virtcontainers
 
 import (
-	"fmt"
 	"os/exec"
 	"syscall"
 )
@@ -23,46 +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.config == nil {
-		return -1, "", fmt.Errorf("Sandbox config cannot be nil")
-	}
-
-	if sandbox.agent == nil {
-		return -1, "", fmt.Errorf("No agent")
-	}
-
-	if params.agentURL == "" {
-		return -1, "", fmt.Errorf("AgentURL cannot be empty")
-	}
-
-	config := sandbox.config.ProxyConfig
-	if err := validateProxyConfig(config); 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:]...)
@@ -74,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

@@ -10,14 +10,13 @@ import (
 )
 
 func TestNoProxyStart(t *testing.T) {
-	sandbox := &Sandbox{
-		agent: newAgent(NoopAgentType),
-	}
-
 	p := &noProxy{}
 
 	agentURL := "agentURL"
-	pid, vmURL, err := p.start(sandbox, proxyParams{agentURL: agentURL})
+	pid, vmURL, err := p.start(proxyParams{
+		agentURL: agentURL,
+		logger:   testDefaultLogger,
+	})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -34,7 +33,7 @@ func TestNoProxyStart(t *testing.T) {
 func TestNoProxyStop(t *testing.T) {
 	p := &noProxy{}
 
-	if err := p.stop(&Sandbox{}, 0); err != nil {
+	if err := p.stop(0); err != nil {
 		t.Fatal(err)
 	}
 }

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

@@ -22,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.
@@ -119,6 +123,18 @@ func newProxy(pType ProxyType) (proxy, error) {
 	}
 }
 
+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)
+	}
+
+	if p.logger == nil {
+		return fmt.Errorf("Invalid proxy parameter: proxy logger is not set")
+	}
+
+	return nil
+}
+
 func validateProxyConfig(proxyConfig ProxyConfig) error {
 	if len(proxyConfig.Path) == 0 {
 		return fmt.Errorf("Proxy path cannot be empty")
@@ -127,10 +143,10 @@ func validateProxyConfig(proxyConfig ProxyConfig) error {
 	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
@@ -146,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