Skip to content

Commit

Permalink
Remove some nb/sb ctl helper functions
Browse files Browse the repository at this point in the history 
Remove all of the unused nb/sb ctl helper functions
from the util package and add notes on when
the rest can be removed

Signed-off-by: astoycos <[email protected]>
  • Loading branch information
astoycos committed Dec 13, 2021
1 parent 3951b18 commit 52d245b
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 178 deletions.
55 changes: 6 additions & 49 deletions go-controller/pkg/util/ovs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package util

import (
"bytes"
"encoding/csv"
"encoding/json"
"fmt"
"os"
Expand All @@ -13,7 +12,6 @@ import (
"strings"
"sync/atomic"
"time"
"unicode"

"github.com/pkg/errors"

Expand Down Expand Up @@ -409,6 +407,7 @@ func RunOVNAppctlWithTimeout(timeout int, args ...string) (string, string, error

// Run the ovn-ctl command and retry if "Connection refused"
// poll waitng for service to become available
// FIXME: Remove when https://github.com/ovn-org/libovsdb/issues/235 is fixed
func runOVNretry(cmdPath string, envVars []string, args ...string) (*bytes.Buffer, *bytes.Buffer, error) {

retriesLeft := ovnCmdRetryCount
Expand Down Expand Up @@ -513,23 +512,15 @@ func getNbOVSDBArgs(command string, args ...string) []string {
return cmdArgs
}

// RunOVNNbctlUnix runs command via ovn-nbctl, with ovn-nbctl using the unix
// domain sockets to connect to the ovsdb-server backing the OVN NB database.
func RunOVNNbctlUnix(args ...string) (string, string, error) {
cmdArgs := []string{fmt.Sprintf("--timeout=%d", ovsCommandTimeout)}
cmdArgs = append(cmdArgs, args...)
stdout, stderr, err := runOVNretry(runner.nbctlPath, nil, cmdArgs...)
return strings.Trim(strings.TrimFunc(stdout.String(), unicode.IsSpace), "\""),
stderr.String(), err
}

// RunOVNNbctlWithTimeout runs command via ovn-nbctl with a specific timeout
// FIXME: Remove when https://github.com/ovn-org/libovsdb/issues/235 is fixed
func RunOVNNbctlWithTimeout(timeout int, args ...string) (string, string, error) {
stdout, stderr, err := RunOVNNbctlRawOutput(timeout, args...)
return strings.Trim(strings.TrimSpace(stdout), "\""), stderr, err
}

// RunOVNNbctlRawOutput returns the output with no trimming or other string manipulation
// FIXME: Remove when https://github.com/ovn-org/libovsdb/issues/235 is fixed
func RunOVNNbctlRawOutput(timeout int, args ...string) (string, string, error) {
cmdArgs, envVars := getNbctlArgsAndEnv(timeout, args...)
start := time.Now()
Expand All @@ -540,43 +531,14 @@ func RunOVNNbctlRawOutput(timeout int, args ...string) (string, string, error) {
return stdout.String(), stderr.String(), err
}

// RunOVNNbctlCSV runs an nbctl command that results in CSV output, parses the rows returned,
// and returns the records
func RunOVNNbctlCSV(args []string) ([][]string, error) {
args = append([]string{"--no-heading", "--format=csv"}, args...)

stdout, _, err := RunOVNNbctlRawOutput(15, args...)
if err != nil {
return nil, err
}
if len(stdout) == 0 {
return nil, nil
}

r := csv.NewReader(strings.NewReader(stdout))
records, err := r.ReadAll()
if err != nil {
return nil, fmt.Errorf("failed to parse nbctl CSV response: %w", err)
}
return records, nil
}

// RunOVNNbctl runs a command via ovn-nbctl.
// FIXME: Remove when https://github.com/ovn-org/libovsdb/issues/235 is fixed
func RunOVNNbctl(args ...string) (string, string, error) {
return RunOVNNbctlWithTimeout(ovsCommandTimeout, args...)
}

// RunOVNSbctlUnix runs command via ovn-sbctl, with ovn-sbctl using the unix
// domain sockets to connect to the ovsdb-server backing the OVN SB database.
func RunOVNSbctlUnix(args ...string) (string, string, error) {
cmdArgs := []string{fmt.Sprintf("--timeout=%d", ovsCommandTimeout)}
cmdArgs = append(cmdArgs, args...)
stdout, stderr, err := runOVNretry(runner.sbctlPath, nil, cmdArgs...)
return strings.Trim(strings.TrimFunc(stdout.String(), unicode.IsSpace), "\""),
stderr.String(), err
}

// RunOVNSbctlWithTimeout runs command via ovn-sbctl with a specific timeout
// FIXME: Remove when https://github.com/ovn-org/libovsdb/issues/235 is fixed
func RunOVNSbctlWithTimeout(timeout int, args ...string) (string, string,
error) {
var cmdArgs []string
Expand Down Expand Up @@ -623,16 +585,11 @@ func RunOVSDBClientOVNNB(command string, args ...string) (string, string, error)
}

// RunOVNSbctl runs a command via ovn-sbctl.
// FIXME: Remove when https://github.com/ovn-org/libovsdb/issues/235 is fixed
func RunOVNSbctl(args ...string) (string, string, error) {
return RunOVNSbctlWithTimeout(ovsCommandTimeout, args...)
}

// RunOVNCtl runs an ovn-ctl command.
func RunOVNCtl(args ...string) (string, string, error) {
stdout, stderr, err := runOVNretry(runner.ovnctlPath, nil, args...)
return strings.Trim(strings.TrimSpace(stdout.String()), "\""), stderr.String(), err
}

// RunOVNNBAppCtlWithTimeout runs an ovn-appctl command with a timeout to nbdb
func RunOVNNBAppCtlWithTimeout(timeout int, args ...string) (string, string, error) {
cmdArgs := []string{fmt.Sprintf("--timeout=%d", timeout)}
Expand Down
129 changes: 0 additions & 129 deletions go-controller/pkg/util/ovs_unit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1302,49 +1302,6 @@ func TestRunOVNAppctlWithTimeout(t *testing.T) {
}
}

func TestRunOVNNbctlUnix(t *testing.T) {
mockKexecIface := new(mock_k8s_io_utils_exec.Interface)
mockExecRunner := new(mocks.ExecRunner)
mockCmd := new(mock_k8s_io_utils_exec.Cmd)
// below is defined in ovs.go
runCmdExecRunner = mockExecRunner
// note runner is defined in ovs.go file
runner = &execHelper{exec: mockKexecIface}
tests := []struct {
desc string
expectedErr error
onRetArgsExecUtilsIface *ovntest.TestifyMockHelper
onRetArgsKexecIface *ovntest.TestifyMockHelper
}{
{
desc: "negative: run `ovn-nbctl` command with no env vars generated",
expectedErr: fmt.Errorf("failed to execute ovn-nbctl command"),
onRetArgsExecUtilsIface: &ovntest.TestifyMockHelper{OnCallMethodName: "RunCmd", OnCallMethodArgType: []string{"*mocks.Cmd", "string", "[]string", "string"}, RetArgList: []interface{}{nil, nil, fmt.Errorf("failed to execute ovn-nbctl command")}},
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string"}, RetArgList: []interface{}{mockCmd}},
},
{
desc: "positive: run `ovn-nbctl` command with no env vars generated",
expectedErr: nil,
onRetArgsExecUtilsIface: &ovntest.TestifyMockHelper{OnCallMethodName: "RunCmd", OnCallMethodArgType: []string{"*mocks.Cmd", "string", "[]string", "string"}, RetArgList: []interface{}{bytes.NewBuffer([]byte("testblah")), bytes.NewBuffer([]byte("")), nil}},
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string"}, RetArgList: []interface{}{mockCmd}},
},
}
for i, tc := range tests {
t.Run(fmt.Sprintf("%d:%s", i, tc.desc), func(t *testing.T) {
ovntest.ProcessMockFn(&mockExecRunner.Mock, *tc.onRetArgsExecUtilsIface)
ovntest.ProcessMockFn(&mockKexecIface.Mock, *tc.onRetArgsKexecIface)

_, _, e := RunOVNNbctlUnix()

if tc.expectedErr != nil {
assert.Error(t, e)
}
mockExecRunner.AssertExpectations(t)
mockKexecIface.AssertExpectations(t)
})
}
}

func TestRunOVNNbctlWithTimeout(t *testing.T) {
mockKexecIface := new(mock_k8s_io_utils_exec.Interface)
mockExecRunner := new(mocks.ExecRunner)
Expand Down Expand Up @@ -1434,49 +1391,6 @@ func TestRunOVNNbctl(t *testing.T) {
}
}

func TestRunOVNSbctlUnix(t *testing.T) {
mockKexecIface := new(mock_k8s_io_utils_exec.Interface)
mockExecRunner := new(mocks.ExecRunner)
mockCmd := new(mock_k8s_io_utils_exec.Cmd)
// below is defined in ovs.go
runCmdExecRunner = mockExecRunner
// note runner is defined in ovs.go file
runner = &execHelper{exec: mockKexecIface}
tests := []struct {
desc string
expectedErr error
onRetArgsExecUtilsIface *ovntest.TestifyMockHelper
onRetArgsKexecIface *ovntest.TestifyMockHelper
}{
{
desc: "negative: run `ovn-sbctl` command with no env vars generated",
expectedErr: fmt.Errorf("failed to execute ovn-sbctl command"),
onRetArgsExecUtilsIface: &ovntest.TestifyMockHelper{OnCallMethodName: "RunCmd", OnCallMethodArgType: []string{"*mocks.Cmd", "string", "[]string", "string"}, RetArgList: []interface{}{nil, nil, fmt.Errorf("failed to execute ovn-sbctl command")}},
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string"}, RetArgList: []interface{}{mockCmd}},
},
{
desc: "positive: run `ovn-sbctl` command with no env vars generated",
expectedErr: nil,
onRetArgsExecUtilsIface: &ovntest.TestifyMockHelper{OnCallMethodName: "RunCmd", OnCallMethodArgType: []string{"*mocks.Cmd", "string", "[]string", "string"}, RetArgList: []interface{}{bytes.NewBuffer([]byte("testblah")), bytes.NewBuffer([]byte("")), nil}},
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string"}, RetArgList: []interface{}{mockCmd}},
},
}
for i, tc := range tests {
t.Run(fmt.Sprintf("%d:%s", i, tc.desc), func(t *testing.T) {
ovntest.ProcessMockFn(&mockExecRunner.Mock, *tc.onRetArgsExecUtilsIface)
ovntest.ProcessMockFn(&mockKexecIface.Mock, *tc.onRetArgsKexecIface)

_, _, e := RunOVNSbctlUnix()

if tc.expectedErr != nil {
assert.Error(t, e)
}
mockExecRunner.AssertExpectations(t)
mockKexecIface.AssertExpectations(t)
})
}
}

func TestRunOVNSbctlWithTimeout(t *testing.T) {
mockKexecIface := new(mock_k8s_io_utils_exec.Interface)
mockExecRunner := new(mocks.ExecRunner)
Expand Down Expand Up @@ -1695,49 +1609,6 @@ func TestRunOVSDBClientOVNNB(t *testing.T) {
}
}

func TestRunOVNCtl(t *testing.T) {
mockKexecIface := new(mock_k8s_io_utils_exec.Interface)
mockExecRunner := new(mocks.ExecRunner)
mockCmd := new(mock_k8s_io_utils_exec.Cmd)
// below is defined in ovs.go
runCmdExecRunner = mockExecRunner
// note runner is defined in ovs.go file
runner = &execHelper{exec: mockKexecIface}
tests := []struct {
desc string
expectedErr error
onRetArgsExecUtilsIface *ovntest.TestifyMockHelper
onRetArgsKexecIface *ovntest.TestifyMockHelper
}{
{
desc: "negative: run `ovn-ctl` command",
expectedErr: fmt.Errorf("failed to execute ovn-ctl command"),
onRetArgsExecUtilsIface: &ovntest.TestifyMockHelper{OnCallMethodName: "RunCmd", OnCallMethodArgType: []string{"*mocks.Cmd", "string", "[]string", "string", "string"}, RetArgList: []interface{}{nil, nil, fmt.Errorf("failed to execute ovn-ctl command")}},
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
},
{
desc: "positive: run `ovn-ctl` command",
expectedErr: nil,
onRetArgsExecUtilsIface: &ovntest.TestifyMockHelper{OnCallMethodName: "RunCmd", OnCallMethodArgType: []string{"*mocks.Cmd", "string", "[]string", "string", "string"}, RetArgList: []interface{}{bytes.NewBuffer([]byte("testblah")), bytes.NewBuffer([]byte("")), nil}},
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
},
}
for i, tc := range tests {
t.Run(fmt.Sprintf("%d:%s", i, tc.desc), func(t *testing.T) {
ovntest.ProcessMockFn(&mockExecRunner.Mock, *tc.onRetArgsExecUtilsIface)
ovntest.ProcessMockFn(&mockKexecIface.Mock, *tc.onRetArgsKexecIface)

_, _, e := RunOVNCtl()

if tc.expectedErr != nil {
assert.Error(t, e)
}
mockExecRunner.AssertExpectations(t)
mockKexecIface.AssertExpectations(t)
})
}
}

func TestRunOVNNBAppCtl(t *testing.T) {
mockKexecIface := new(mock_k8s_io_utils_exec.Interface)
mockExecRunner := new(mocks.ExecRunner)
Expand Down

0 comments on commit 52d245b

Please sign in to comment.