Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Commit

Permalink
proxy: remove newProxyConfig
Browse files Browse the repository at this point in the history
The proxy config does not depend on proxy type. Let's not misture them.

Signed-off-by: Peng Tao <[email protected]>
  • Loading branch information
bergwolf committed Sep 14, 2018
1 parent c41c9de commit f39fa5d
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 43 deletions.
9 changes: 7 additions & 2 deletions virtcontainers/cc_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package virtcontainers

import (
"fmt"
"os/exec"
)

Expand All @@ -14,8 +15,12 @@ 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 {
if sandbox.config == nil {
return -1, "", fmt.Errorf("Sandbox config cannot be nil")
}

config := sandbox.config.ProxyConfig
if err := validateProxyConfig(config); err != nil {
return -1, "", err
}

Expand Down
8 changes: 6 additions & 2 deletions virtcontainers/kata_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ 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")
}
Expand All @@ -33,8 +37,8 @@ func (p *kataProxy) start(sandbox *Sandbox, params proxyParams) (int, string, er
return -1, "", fmt.Errorf("AgentURL cannot be empty")
}

config, err := newProxyConfig(sandbox.config)
if err != nil {
config := sandbox.config.ProxyConfig
if err := validateProxyConfig(config); err != nil {
return -1, "", err
}

Expand Down
24 changes: 4 additions & 20 deletions virtcontainers/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,28 +119,12 @@ 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 validateProxyConfig(proxyConfig ProxyConfig) error {
if len(proxyConfig.Path) == 0 {
return fmt.Errorf("Proxy path cannot be empty")
}

var config ProxyConfig
switch sandboxConfig.ProxyType {
case KataProxyType:
fallthrough
case CCProxyType:
config = sandboxConfig.ProxyConfig
default:
return ProxyConfig{}, fmt.Errorf("unknown proxy type %v", sandboxConfig.ProxyType)
}

if config.Path == "" {
return ProxyConfig{}, fmt.Errorf("Proxy path cannot be empty")
}

return config, nil
return nil
}

func defaultProxyURL(sandbox *Sandbox, socketType string) (string, error) {
Expand Down
27 changes: 8 additions & 19 deletions virtcontainers/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,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"
Expand All @@ -177,7 +177,7 @@ func TestNewProxyConfigFromCCProxySandboxConfig(t *testing.T) {
ProxyConfig: proxyConfig,
}

testNewProxyConfigFromSandboxConfig(t, sandboxConfig, proxyConfig)
testNewProxyFromSandboxConfig(t, sandboxConfig)
}

func TestNewProxyConfigFromKataProxySandboxConfig(t *testing.T) {
Expand All @@ -190,22 +190,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")
}
}
Expand Down

0 comments on commit f39fa5d

Please sign in to comment.