proxy: Add proxyParams to proxy interface

This allows to simplify the agent interface, with no need to define
vmURL() which was more a workaround when it was introduced. Indeed,
we want to be explicit about parameters that we provide to the proxy
rather than implicitely call into the agent interface.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>

agent.go

@@ -18,7 +18,6 @@ package virtcontainers
 
 import (
 	"fmt"
-	"path/filepath"
 	"syscall"
 
 	"github.com/mitchellh/mapstructure"
@@ -130,19 +129,6 @@ func newAgentConfig(config PodConfig) interface{} {
 	}
 }
 
-func defaultAgentURL(pod *Pod, socketType string) (string, error) {
-	switch socketType {
-	case SocketTypeUNIX:
-		socketPath := filepath.Join(runStoragePath, pod.id, "proxy.sock")
-		return fmt.Sprintf("unix://%s", socketPath), nil
-	case SocketTypeVSOCK:
-		// TODO Build the VSOCK default URL
-		return "", nil
-	default:
-		return "", fmt.Errorf("Unknown socket type: %s", socketType)
-	}
-}
-
 // agent is the virtcontainers agent interface.
 // Agents are running in the guest VM and handling
 // communications between the host and guest.
@@ -154,15 +140,6 @@ type agent interface {
 	// to handle all other Agent interface methods.
 	init(pod *Pod, config interface{}) error
 
-	// vmURL returns the agent URL exposed by the hypervisor. This URL has
-	// been previously built from the agent implementation and provided to
-	// the hypervisor through a specific hypervisor interface function. It
-	// represents the entry point to connect to the agent through the VM.
-	// This function is particularly useful in case there is no proxy
-	// needed, meaning the proxy implementation will have to provide this
-	// URL instead of a real proxy URL.
-	vmURL() (string, error)
-
 	// capabilities should return a structure that specifies the capabilities
 	// supported by the agent.
 	capabilities() capabilities

agent_test.go

@@ -17,8 +17,6 @@
 package virtcontainers
 
 import (
-	"fmt"
-	"path/filepath"
 	"reflect"
 	"testing"
 )
@@ -156,46 +154,3 @@ func TestNewAgentConfigFromUnknownAgentType(t *testing.T) {
 
 	testNewAgentConfig(t, PodConfig{}, agentConfig)
 }
-
-const podID = "123456789"
-
-func testDefaultAgentURL(expectedURL string, socketType string, podID string) error {
-	pod := &Pod{
-		id: podID,
-	}
-
-	url, err := defaultAgentURL(pod, socketType)
-	if err != nil {
-		return err
-	}
-
-	if url != expectedURL {
-		return fmt.Errorf("Mismatched URL: %s vs %s", url, expectedURL)
-	}
-
-	return nil
-}
-
-func TestDefaultAgentURLUnix(t *testing.T) {
-	path := filepath.Join(runStoragePath, podID, "proxy.sock")
-	socketPath := fmt.Sprintf("unix://%s", path)
-
-	if err := testDefaultAgentURL(socketPath, SocketTypeUNIX, podID); err != nil {
-		t.Fatal(err)
-	}
-}
-
-func TestDefaultAgentURLVSock(t *testing.T) {
-	if err := testDefaultAgentURL("", SocketTypeVSOCK, podID); err != nil {
-		t.Fatal(err)
-	}
-}
-
-func TestDefaultAgentURLUnknown(t *testing.T) {
-	path := filepath.Join(runStoragePath, podID, "proxy.sock")
-	socketPath := fmt.Sprintf("unix://%s", path)
-
-	if err := testDefaultAgentURL(socketPath, "foobar", podID); err == nil {
-		t.Fatal()
-	}
-}

cc_proxy.go

@@ -17,26 +17,26 @@
 package virtcontainers
 
 import (
-	"fmt"
 	"os/exec"
-	"path/filepath"
 )
 
 type ccProxy struct {
 }
 
 // start is the proxy start implementation for ccProxy.
-func (p *ccProxy) start(pod Pod) (int, string, error) {
+func (p *ccProxy) start(pod Pod, params proxyParams) (int, string, error) {
 	config, err := newProxyConfig(pod.config)
 	if err != nil {
 		return -1, "", err
 	}
 
 	// construct the socket path the proxy instance will use
-	socketPath := filepath.Join(runStoragePath, pod.id, "proxy.sock")
-	uri := fmt.Sprintf("unix://%s", socketPath)
+	proxyURL, err := defaultProxyURL(pod, SocketTypeUNIX)
+	if err != nil {
+		return -1, "", err
+	}
 
-	args := []string{config.Path, "-uri", uri}
+	args := []string{config.Path, "-uri", proxyURL}
 	if config.Debug {
 		args = append(args, "-log", "debug")
 	}
@@ -46,7 +46,7 @@ func (p *ccProxy) start(pod Pod) (int, string, error) {
 		return -1, "", err
 	}
 
-	return cmd.Process.Pid, uri, nil
+	return cmd.Process.Pid, proxyURL, nil
 }
 
 func (p *ccProxy) stop(pod Pod) error {

cc_proxy_test.go

@@ -87,7 +87,7 @@ func TestCCProxyStart(t *testing.T) {
 	}
 
 	for _, d := range data {
-		pid, uri, err := proxy.start(d.pod)
+		pid, uri, err := proxy.start(d.pod, proxyParams{})
 		if d.expectError {
 			assert.Error(err)
 			continue

hyperstart_agent.go

@@ -260,11 +260,6 @@ func (h *hyper) init(pod *Pod, config interface{}) (err error) {
 	return nil
 }
 
-// vmURL returns VM URL from hyperstart agent implementation.
-func (h *hyper) vmURL() (string, error) {
-	return "", nil
-}
-
 func (h *hyper) createPod(pod *Pod) (err error) {
 	for _, volume := range h.config.Volumes {
 		err := pod.hypervisor.addDevice(volume, fsDev)
@@ -341,7 +336,7 @@ func (h *hyper) exec(pod *Pod, c Container, cmd Cmd) (*Process, error) {
 // startPod is the agent Pod starting implementation for hyperstart.
 func (h *hyper) startPod(pod Pod) error {
 	// Start the proxy here
-	pid, uri, err := h.pod.proxy.start(pod)
+	pid, uri, err := h.pod.proxy.start(pod, proxyParams{})
 	if err != nil {
 		return err
 	}

kata_agent.go

@@ -145,7 +145,7 @@ func (k *kataAgent) init(pod *Pod, config interface{}) error {
 	return nil
 }
 
-func (k *kataAgent) vmURL() (string, error) {
+func (k *kataAgent) agentURL() (string, error) {
 	switch s := k.vmSocket.(type) {
 	case Socket:
 		return s.HostPath, nil
@@ -297,8 +297,18 @@ func (k *kataAgent) startPod(pod Pod) error {
 		return errorMissingProxy
 	}
 
+	// Get agent socket path to provide it to the proxy.
+	agentURL, err := k.agentURL()
+	if err != nil {
+		return err
+	}
+
+	proxyParams := proxyParams{
+		agentURL: agentURL,
+	}
+
 	// Start the proxy here
-	pid, uri, err := k.pod.proxy.start(pod)
+	pid, uri, err := k.pod.proxy.start(pod, proxyParams)
 	if err != nil {
 		return err
 	}

kata_proxy.go

@@ -26,34 +26,30 @@ import (
 // This is pretty simple since it provides the same interface to both
 // runtime and shim as if they were talking directly to the agent.
 type kataProxy struct {
-	proxyURL string
 }
 
 // start is kataProxy start implementation for proxy interface.
-func (p *kataProxy) start(pod Pod) (int, string, error) {
+func (p *kataProxy) start(pod Pod, params proxyParams) (int, string, error) {
 	if pod.agent == nil {
 		return -1, "", fmt.Errorf("No agent")
 	}
 
-	config, err := newProxyConfig(pod.config)
-	if err != nil {
-		return -1, "", err
+	if params.agentURL == "" {
+		return -1, "", fmt.Errorf("AgentURL cannot be empty")
 	}
 
-	// construct the socket path the proxy instance will use
-	proxyURL, err := defaultAgentURL(&pod, SocketTypeUNIX)
+	config, err := newProxyConfig(pod.config)
 	if err != nil {
 		return -1, "", err
 	}
 
-	vmURL, err := pod.agent.vmURL()
+	// construct the socket path the proxy instance will use
+	proxyURL, err := defaultProxyURL(pod, SocketTypeUNIX)
 	if err != nil {
 		return -1, "", err
 	}
 
-	p.proxyURL = proxyURL
-
-	args := []string{config.Path, "-listen-socket", proxyURL, "-mux-socket", vmURL}
+	args := []string{config.Path, "-listen-socket", proxyURL, "-mux-socket", params.agentURL}
 	if config.Debug {
 		args = append(args, "-log", "debug")
 		args = append(args, "-agent-logs-socket", pod.hypervisor.getPodConsole(pod.id))
@@ -64,7 +60,7 @@ func (p *kataProxy) start(pod Pod) (int, string, error) {
 		return -1, "", err
 	}
 
-	return cmd.Process.Pid, p.proxyURL, nil
+	return cmd.Process.Pid, proxyURL, nil
 }
 
 // stop is kataProxy stop implementation for proxy interface.

no_proxy.go

@@ -16,6 +16,10 @@
 
 package virtcontainers
 
+import (
+	"fmt"
+)
+
 // This is the no proxy implementation of the proxy interface. This
 // is a generic implementation for any case (basically any agent),
 // where no actual proxy is needed. This happens when the combination
@@ -27,19 +31,15 @@ package virtcontainers
 // is to provide both shim and runtime the correct URL to connect
 // directly to the VM.
 type noProxy struct {
-	vmURL string
 }
 
 // start is noProxy start implementation for proxy interface.
-func (p *noProxy) start(pod Pod) (int, string, error) {
-	url, err := pod.agent.vmURL()
-	if err != nil {
-		return -1, "", err
+func (p *noProxy) start(pod Pod, params proxyParams) (int, string, error) {
+	if params.agentURL == "" {
+		return -1, "", fmt.Errorf("AgentURL cannot be empty")
 	}
 
-	p.vmURL = url
-
-	return 0, p.vmURL, nil
+	return 0, params.agentURL, nil
 }
 
 // stop is noProxy stop implementation for proxy interface.

no_proxy_test.go

@@ -29,13 +29,14 @@ func TestNoProxyStart(t *testing.T) {
 
 	p := &noProxy{}
 
-	pid, vmURL, err := p.start(pod)
+	agentURL := "agentURL"
+	pid, vmURL, err := p.start(pod, proxyParams{agentURL: agentURL})
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	if vmURL != "" {
-		t.Fatalf("Got URL %q, expecting empty URL", vmURL)
+	if vmURL != agentURL {
+		t.Fatalf("Got URL %q, expecting %q", vmURL, agentURL)
 	}
 
 	if pid != 0 {

noop_agent.go

@@ -30,11 +30,6 @@ func (n *noopAgent) init(pod *Pod, config interface{}) error {
 	return nil
 }
 
-// vmURL returns the VM URL from the Noop agent. It does nothing.
-func (n *noopAgent) vmURL() (string, error) {
-	return "", nil
-}
-
 // createPod is the Noop agent pod creation implementation. It does nothing.
 func (n *noopAgent) createPod(pod *Pod) error {
 	return nil

noop_agent_test.go

@@ -49,19 +49,6 @@ func TestNoopAgentInit(t *testing.T) {
 	}
 }
 
-func TestNoopAgentVmURL(t *testing.T) {
-	n := &noopAgent{}
-
-	url, err := n.vmURL()
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if url != "" {
-		t.Fatalf("URL should be empty: %s", url)
-	}
-}
-
 func TestNoopAgentExec(t *testing.T) {
 	n := &noopAgent{}
 	cmd := Cmd{}

noop_proxy.go

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

proxy.go

@@ -18,6 +18,7 @@ package virtcontainers
 
 import (
 	"fmt"
+	"path/filepath"
 
 	"github.com/mitchellh/mapstructure"
 	"github.com/sirupsen/logrus"
@@ -30,6 +31,12 @@ type ProxyConfig struct {
 	Debug bool
 }
 
+// proxyParams is the structure providing specific parameters needed
+// for the execution of the proxy binary.
+type proxyParams struct {
+	agentURL string
+}
+
 // ProxyType describes a proxy type.
 type ProxyType string
 
@@ -133,11 +140,24 @@ func newProxyConfig(podConfig *PodConfig) (ProxyConfig, error) {
 	return config, nil
 }
 
+func defaultProxyURL(pod Pod, socketType string) (string, error) {
+	switch socketType {
+	case SocketTypeUNIX:
+		socketPath := filepath.Join(runStoragePath, pod.id, "proxy.sock")
+		return fmt.Sprintf("unix://%s", socketPath), nil
+	case SocketTypeVSOCK:
+		// TODO Build the VSOCK default URL
+		return "", nil
+	default:
+		return "", fmt.Errorf("Unknown socket type: %s", socketType)
+	}
+}
+
 // proxy is the virtcontainers proxy interface.
 type proxy interface {
 	// start launches a proxy instance for the specified pod, returning
 	// the PID of the process and the URL used to connect to it.
-	start(pod Pod) (int, string, error)
+	start(pod Pod, params proxyParams) (int, string, error)
 
 	// stop terminates a proxy instance after all communications with the
 	// agent inside the VM have been properly stopped.