Skip to content

Commit

Permalink
ovnkube: Add retry parameter to the ovs-vsctl command
Browse files Browse the repository at this point in the history
Without this option, if **ovs-vsctl** connects outward to the OVN database server (the default)
then  **ovs-vsctl** will try to connect once and exit with an error if the connection fails.
Regardless of this setting, **--timeout** always limits how long **ovs-vsctl** will wait.

Signed-off-by: Rafael Chicoli <[email protected]>
  • Loading branch information
rchicoli committed Jul 19, 2023
1 parent 300cafe commit c188ec7
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 57 deletions.
88 changes: 44 additions & 44 deletions go-controller/pkg/cni/bandwidth_test.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion go-controller/pkg/cni/ovs.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func ovsExec(args ...string) (string, error) {
return "", fmt.Errorf("OVS exec runner not initialized")
}

args = append([]string{"--timeout=30"}, args...)
args = append([]string{"--timeout=30", "--retry"}, args...)
output, err := runner.Command(vsctlPath, args...).CombinedOutput()
if err != nil {
return "", fmt.Errorf("failed to run 'ovs-vsctl %s': %v\n %q", strings.Join(args, " "), err, string(output))
Expand Down
10 changes: 5 additions & 5 deletions go-controller/pkg/cni/ovs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var _ = Describe("CNI OVS tests", func() {

It("returns non-empty elements from ovsFind", func() {
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-vsctl --timeout=30 --no-heading --format=csv --data=bare --columns=_uuid find Interface external-ids:iface-id=foobar",
Cmd: "ovs-vsctl --timeout=30 --retry --no-heading --format=csv --data=bare --columns=_uuid find Interface external-ids:iface-id=foobar",
Output: `75419b50-ec6e-4989-b769-164488f53375
4609184a-cb69-46ed-880f-807b6a4e99f5
d9af11aa-37c3-4ea9-8ba3-a74843cc0f47
Expand All @@ -37,7 +37,7 @@ d9af11aa-37c3-4ea9-8ba3-a74843cc0f47

It("returns nil if no element is returned from ovsFind", func() {
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-vsctl --timeout=30 --no-heading --format=csv --data=bare --columns=_uuid find Interface external-ids:iface-id=foobar",
Cmd: "ovs-vsctl --timeout=30 --retry --no-heading --format=csv --data=bare --columns=_uuid find Interface external-ids:iface-id=foobar",
Output: "",
})

Expand All @@ -48,7 +48,7 @@ d9af11aa-37c3-4ea9-8ba3-a74843cc0f47

It("returns empty values if the elements themselves are empty from ovsFind", func() {
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-vsctl --timeout=30 --no-heading --format=csv --data=bare --columns=_uuid find Interface external-ids:iface-id=foobar",
Cmd: "ovs-vsctl --timeout=30 --retry --no-heading --format=csv --data=bare --columns=_uuid find Interface external-ids:iface-id=foobar",
Output: `
`,
Expand All @@ -63,7 +63,7 @@ d9af11aa-37c3-4ea9-8ba3-a74843cc0f47

It("returns quoted empty values if the elements themselves are empty from ovsFind", func() {
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-vsctl --timeout=30 --no-heading --format=csv --data=bare --columns=_uuid find Interface external-ids:iface-id=foobar",
Cmd: "ovs-vsctl --timeout=30 --retry --no-heading --format=csv --data=bare --columns=_uuid find Interface external-ids:iface-id=foobar",
Output: `""
""
`,
Expand All @@ -78,7 +78,7 @@ d9af11aa-37c3-4ea9-8ba3-a74843cc0f47

It("returns unquoted value if the elements themselves are quoted from ovsGet", func() {
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-vsctl --timeout=30 --if-exists get Interface blah external-ids:iface-id",
Cmd: "ovs-vsctl --timeout=30 --retry --if-exists get Interface blah external-ids:iface-id",
Output: `"1234"
`,
})
Expand Down
6 changes: 3 additions & 3 deletions go-controller/pkg/cni/ovs_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,21 +232,21 @@ func TestOvsFind(t *testing.T) {
{
desc: "Test codepath when ovsExec returns an error",
expectedErr: fmt.Errorf("failed to run ovsFind"),
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
onRetArgsCmdList: &ovntest.TestifyMockHelper{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, fmt.Errorf("failed to run ovsFind")}},
runnerInstance: mockKexecIface,
},
{
desc: "Test codepath when ovsExec output is nil",
expectedErr: nil,
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
onRetArgsCmdList: &ovntest.TestifyMockHelper{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, nil}},
runnerInstance: mockKexecIface,
},
{
desc: "Positive test codepath for ovsFind; ovsExec output is not nil",
expectedErr: nil,
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
onRetArgsCmdList: &ovntest.TestifyMockHelper{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{[]byte{}, nil}},
runnerInstance: mockKexecIface,
},
Expand Down
2 changes: 1 addition & 1 deletion go-controller/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1474,7 +1474,7 @@ func rawExec(exec kexec.Interface, cmd string, args ...string) (string, error) {

// Can't use pkg/ovs or pkg/util here because those package import this one
func runOVSVsctl(exec kexec.Interface, args ...string) (string, error) {
newArgs := append([]string{"--timeout=15"}, args...)
newArgs := append([]string{"--timeout=15", "--retry"}, args...)
out, err := rawExec(exec, ovsVsctlCommand, newArgs...)
if err != nil {
return "", err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func genOVSAddPortCmd(hostIfaceName, ifaceID, mac, ip, sandboxID, podUID string)
if ip != "" {
ipAddrExtID = fmt.Sprintf("external_ids:ip_addresses=%s ", ip)
}
return fmt.Sprintf("ovs-vsctl --timeout=30 --may-exist add-port br-int %s other_config:transient=true "+
return fmt.Sprintf("ovs-vsctl --timeout=30 --retry --may-exist add-port br-int %s other_config:transient=true "+
"-- set interface %s external_ids:attached_mac=%s external_ids:iface-id=%s external_ids:iface-id-ver=%s "+
"%sexternal_ids:sandbox=%s external_ids:vf-netdev-name=%s "+
"-- --if-exists remove interface %s external_ids k8s.ovn.org/network "+
Expand All @@ -50,7 +50,7 @@ func genOVSGetCmd(table, record, column, key string) string {
if key != "" {
column = column + ":" + key
}
return fmt.Sprintf("ovs-vsctl --timeout=30 --if-exists get %s %s %s", table, record, column)
return fmt.Sprintf("ovs-vsctl --timeout=30 --retry --if-exists get %s %s %s", table, record, column)
}

func genOfctlDumpFlowsCmd(queryStr string) string {
Expand Down
2 changes: 1 addition & 1 deletion go-controller/pkg/util/ovs.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func RunOVSOfctl(args ...string) (string, string, error) {

// RunOVSVsctl runs a command via ovs-vsctl.
func RunOVSVsctl(args ...string) (string, string, error) {
cmdArgs := []string{fmt.Sprintf("--timeout=%d", ovsCommandTimeout)}
cmdArgs := []string{fmt.Sprintf("--timeout=%d", ovsCommandTimeout), "--retry"}
cmdArgs = append(cmdArgs, args...)
stdout, stderr, err := run(runner.vsctlPath, cmdArgs...)
return strings.Trim(strings.TrimSpace(stdout.String()), "\""), stderr.String(), err
Expand Down

0 comments on commit c188ec7

Please sign in to comment.