ovnkube: Add retry parameter to the ovs-vsctl command

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]>
ovn-org · Jul 19, 2023 · 656929e · 656929e
1 parent 300cafe
commit 656929e
Show file tree

Hide file tree

Showing 19 changed files with 387 additions and 387 deletions.
diff --git a/go-controller/hybrid-overlay/pkg/controller/node_linux_test.go b/go-controller/hybrid-overlay/pkg/controller/node_linux_test.go
@@ -111,18 +111,18 @@ func addNodeSetupCmds(fexec *ovntest.FakeExec, nodeName string) {
 		Output: thisNodeSubnet,
 	})
 	fexec.AddFakeCmdsNoOutputNoError([]string{
-		"ovs-vsctl --timeout=15 --may-exist add-br br-ext -- set Bridge br-ext fail_mode=secure -- set Interface br-ext mtu_request=1400",
+		"ovs-vsctl --timeout=15 --retry --may-exist add-br br-ext -- set Bridge br-ext fail_mode=secure -- set Interface br-ext mtu_request=1400",
 	})
 	fexec.AddFakeCmd(&ovntest.ExpectedCmd{
-		Cmd:    "ovs-vsctl --timeout=15 --if-exists get interface br-ext mac_in_use",
+		Cmd:    "ovs-vsctl --timeout=15 --retry --if-exists get interface br-ext mac_in_use",
 		Output: "10:11:12:13:14:15",
 	})
 	fexec.AddFakeCmdsNoOutputNoError([]string{
-		"ovs-vsctl --timeout=15 set bridge br-ext other-config:hwaddr=10:11:12:13:14:15",
-		"ovs-vsctl --timeout=15 --may-exist add-port br-int int -- --may-exist add-port br-ext ext -- set Interface int type=patch options:peer=ext external-ids:iface-id=int-" + nodeName + " -- set Interface ext type=patch options:peer=int",
+		"ovs-vsctl --timeout=15 --retry set bridge br-ext other-config:hwaddr=10:11:12:13:14:15",
+		"ovs-vsctl --timeout=15 --retry --may-exist add-port br-int int -- --may-exist add-port br-ext ext -- set Interface int type=patch options:peer=ext external-ids:iface-id=int-" + nodeName + " -- set Interface ext type=patch options:peer=int",
 	})
 	fexec.AddFakeCmdsNoOutputNoError([]string{
-		`ovs-vsctl --timeout=15 --may-exist add-port br-ext ext-vxlan -- set interface ext-vxlan type=vxlan options:remote_ip="flow" options:key="flow" options:dst_port=4789`,
+		`ovs-vsctl --timeout=15 --retry --may-exist add-port br-ext ext-vxlan -- set interface ext-vxlan type=vxlan options:remote_ip="flow" options:key="flow" options:dst_port=4789`,
 	})
 }
 

diff --git a/go-controller/pkg/cni/bandwidth_test.go b/go-controller/pkg/cni/bandwidth_test.go
diff --git a/go-controller/pkg/cni/helper_linux_test.go b/go-controller/pkg/cni/helper_linux_test.go
@@ -483,7 +483,7 @@ func TestSetupSriovInterface(t *testing.T) {
 			inpPCIAddrs:         "0000:03:00.1",
 			errExp:              true,
 			sriovOpsMockHelper:  []ovntest.TestifyMockHelper{},
-			onRetArgsKexecIface: []ovntest.TestifyMockHelper{{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string"}, RetArgList: []interface{}{mockCmd}}},
+			onRetArgsKexecIface: []ovntest.TestifyMockHelper{{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string"}, RetArgList: []interface{}{mockCmd}}},
 			onRetArgsCmdList:    []ovntest.TestifyMockHelper{{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, fmt.Errorf("failed to run 'ovs-vsctl")}}},
 			netLinkOpsMockHelper: []ovntest.TestifyMockHelper{
 				{OnCallMethodName: "LinkByName", OnCallMethodArgType: []string{"string"}, RetArgList: []interface{}{nil, fmt.Errorf("mock error")}},
@@ -503,7 +503,7 @@ func TestSetupSriovInterface(t *testing.T) {
 			inpPCIAddrs:         "0000:03:00.1",
 			errExp:              true,
 			sriovOpsMockHelper:  []ovntest.TestifyMockHelper{},
-			onRetArgsKexecIface: []ovntest.TestifyMockHelper{{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string"}, RetArgList: []interface{}{mockCmd}}},
+			onRetArgsKexecIface: []ovntest.TestifyMockHelper{{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string"}, RetArgList: []interface{}{mockCmd}}},
 			onRetArgsCmdList:    []ovntest.TestifyMockHelper{{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, nil}}},
 			netLinkOpsMockHelper: []ovntest.TestifyMockHelper{
 				{OnCallMethodName: "LinkByName", OnCallMethodArgType: []string{"string"}, RetArgList: []interface{}{mockLink, nil}},
@@ -529,7 +529,7 @@ func TestSetupSriovInterface(t *testing.T) {
 			},
 			inpPCIAddrs:         "0000:03:00.1",
 			errExp:              true,
-			onRetArgsKexecIface: []ovntest.TestifyMockHelper{{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string"}, RetArgList: []interface{}{mockCmd}}},
+			onRetArgsKexecIface: []ovntest.TestifyMockHelper{{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string"}, RetArgList: []interface{}{mockCmd}}},
 			onRetArgsCmdList:    []ovntest.TestifyMockHelper{{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, fmt.Errorf("failed to run 'ovs-vsctl")}}},
 			runnerInstance:      mockKexecIface,
 			sriovOpsMockHelper: []ovntest.TestifyMockHelper{
@@ -591,7 +591,7 @@ func TestSetupSriovInterface(t *testing.T) {
 			inpPCIAddrs: "0000:03:00.1",
 			errExp:      true,
 			onRetArgsKexecIface: []ovntest.TestifyMockHelper{
-				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string"}, RetArgList: []interface{}{mockCmd}},
+				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
 			},
 			onRetArgsCmdList: []ovntest.TestifyMockHelper{{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, nil}}},
 			runnerInstance:   mockKexecIface,
@@ -624,8 +624,8 @@ func TestSetupSriovInterface(t *testing.T) {
 			inpPCIAddrs: "0000:03:00.1",
 			errMatch:    fmt.Errorf("failed to rename"),
 			onRetArgsKexecIface: []ovntest.TestifyMockHelper{
-				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
-				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
+				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
+				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
 			},
 			onRetArgsCmdList: []ovntest.TestifyMockHelper{
 				{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, nil}},
@@ -664,8 +664,8 @@ func TestSetupSriovInterface(t *testing.T) {
 			inpPCIAddrs: "0000:03:00.1",
 			errExp:      true,
 			onRetArgsKexecIface: []ovntest.TestifyMockHelper{
-				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
-				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
+				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
+				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
 			},
 			onRetArgsCmdList: []ovntest.TestifyMockHelper{
 				{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, nil}},
@@ -709,8 +709,8 @@ func TestSetupSriovInterface(t *testing.T) {
 			inpPCIAddrs: "0000:03:00.1",
 			errMatch:    fmt.Errorf("failed to set MTU on"),
 			onRetArgsKexecIface: []ovntest.TestifyMockHelper{
-				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
-				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
+				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
+				{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
 			},
 			onRetArgsCmdList: []ovntest.TestifyMockHelper{
 				{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, nil}},

diff --git a/go-controller/pkg/cni/ovs.go b/go-controller/pkg/cni/ovs.go
@@ -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))

diff --git a/go-controller/pkg/cni/ovs_test.go b/go-controller/pkg/cni/ovs_test.go
@@ -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
@@ -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: "",
 		})
 
@@ -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: `
 
 `,
@@ -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: `""
 ""
 `,
@@ -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"
 `,
 		})

diff --git a/go-controller/pkg/cni/ovs_unit_test.go b/go-controller/pkg/cni/ovs_unit_test.go
@@ -61,14 +61,14 @@ func TestOvsExec(t *testing.T) {
 		{
 			desc:                "Test codepath when runner is non nil and returns error",
 			expectedErr:         fmt.Errorf("failed to run ovs-vsctl"),
-			onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string"}, RetArgList: []interface{}{mockCmd}},
+			onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
 			onRetArgsCmdList:    &ovntest.TestifyMockHelper{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, fmt.Errorf("failed to run 'ovs-vsctl")}},
 			runnerInstance:      mockKexecIface,
 		},
 		{
 			desc:                "Test codepath when runner is not nil and does not return an error",
 			expectedErr:         nil,
-			onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string"}, RetArgList: []interface{}{mockCmd}},
+			onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
 			onRetArgsCmdList:    &ovntest.TestifyMockHelper{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, nil}},
 			runnerInstance:      mockKexecIface,
 		},
@@ -112,7 +112,7 @@ func TestOvsCreate(t *testing.T) {
 		{
 			desc:                "Positive test codepath for ovsCreate",
 			expectedErr:         nil,
-			onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
+			onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
 			onRetArgsCmdList:    &ovntest.TestifyMockHelper{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, nil}},
 			runnerInstance:      mockKexecIface,
 		},
@@ -152,7 +152,7 @@ func TestOvsDestroy(t *testing.T) {
 		{
 			desc:                "Positive test codepath for ovsDestroy",
 			expectedErr:         nil,
-			onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
+			onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
 			onRetArgsCmdList:    &ovntest.TestifyMockHelper{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, nil}},
 			runnerInstance:      mockKexecIface,
 		},
@@ -192,7 +192,7 @@ func TestOvsSet(t *testing.T) {
 		{
 			desc:                "Positive test codepath for ovsSet",
 			expectedErr:         nil,
-			onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
+			onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []string{"string", "string", "string", "string", "string", "string", "string"}, RetArgList: []interface{}{mockCmd}},
 			onRetArgsCmdList:    &ovntest.TestifyMockHelper{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, nil}},
 			runnerInstance:      mockKexecIface,
 		},
@@ -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", "string"}, RetArgList: []interface{}{mockCmd}},
 			onRetArgsCmdList:    &ovntest.TestifyMockHelper{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{[]byte{}, nil}},
 			runnerInstance:      mockKexecIface,
 		},
@@ -286,7 +286,7 @@ func TestOvsClear(t *testing.T) {
 		{
 			desc:                "Positive test codepath for ovsClear",
 			expectedErr:         nil,
-			onRetArgsKexecIface: &ovntest.TestifyMockHelper{OnCallMethodName: "Command", OnCallMethodArgType: []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"}, RetArgList: []interface{}{mockCmd}},
 			onRetArgsCmdList:    &ovntest.TestifyMockHelper{OnCallMethodName: "CombinedOutput", OnCallMethodArgType: []string{}, RetArgList: []interface{}{nil, nil}},
 			runnerInstance:      mockKexecIface,
 		},

diff --git a/go-controller/pkg/config/config.go b/go-controller/pkg/config/config.go
@@ -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