ut: add ovs and ovn nb and cidr check (#4671)

* ut: add ovn nb --------- Signed-off-by: bobz965 <[email protected]>
kubeovn · Nov 25, 2024 · 0fabca7 · 0fabca7
1 parent dca755a
commit 0fabca7
Show file tree

Hide file tree

Showing 13 changed files with 211 additions and 8 deletions.
diff --git a/pkg/ovs/ovn-nb-acl.go b/pkg/ovs/ovn-nb-acl.go
@@ -1154,6 +1154,10 @@ func (c *OVNNbClient) CreateAclsOps(parentName, parentType string, acls ...*ovnn
 // delete to-lport and from-lport direction acl when direction is empty, otherwise one-way
 // parentType is 'ls' or 'pg'
 func (c *OVNNbClient) DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string) ([]ovsdb.Operation, error) {
+	if parentName == "" {
+		return nil, errors.New("the port group name or logical switch name is required")
+	}
+
 	if externalIDs == nil {
 		externalIDs = make(map[string]string)
 	}

diff --git a/pkg/ovs/ovn-nb-logical_switch_port_test.go b/pkg/ovs/ovn-nb-logical_switch_port_test.go
@@ -1549,6 +1549,7 @@ func (suite *OvnClientTestSuite) testListLogicalSwitchPorts() {
 	t.Parallel()
 
 	nbClient := suite.ovnNBClient
+	failedNbClient := suite.faiedOvnNBClient
 
 	lsName := "test-list-lsp-ls"
 	err := nbClient.CreateBareLogicalSwitch(lsName)
@@ -1570,7 +1571,19 @@ func (suite *OvnClientTestSuite) testListLogicalSwitchPorts() {
 		require.Equal(t, lspName, out[0].Name)
 	})
 
-	t.Run("patch lsp", func(t *testing.T) {
+	t.Run("failed client create patch lsp", func(t *testing.T) {
+		t.Parallel()
+
+		// patch lsp
+		lrName := "test-list-patch-lsp-lr"
+		lspName := "test-list-patch-lsp-lsp"
+		lrpName := "test-list-patch-lsp-lrp"
+
+		err = failedNbClient.CreateLogicalPatchPort(lsName, lrName, lspName, lrpName, "10.19.100.1/24", "")
+		require.Error(t, err)
+	})
+
+	t.Run("failed client create patch lsp", func(t *testing.T) {
 		t.Parallel()
 
 		// patch lsp

diff --git a/pkg/ovs/ovn-nb-nat_test.go b/pkg/ovs/ovn-nb-nat_test.go
@@ -569,7 +569,7 @@ func (suite *OvnClientTestSuite) testGetNat() {
 		})
 
 		t.Run("logical ip is different", func(t *testing.T) {
-			_, err := nbClient.GetNat(lrName, natType, externalIP, "10.250.0.10", false)
+			_, err := nbClient.GetNat(lrName, natType, externalIP, "10.250.0.171", false)
 			require.ErrorContains(t, err, "not found")
 		})
 

diff --git a/pkg/ovs/ovn-nb-port_group.go b/pkg/ovs/ovn-nb-port_group.go
@@ -188,6 +188,9 @@ func (c *OVNNbClient) DeletePortGroup(pgName ...string) error {
 
 // GetPortGroup get port group by name
 func (c *OVNNbClient) GetPortGroup(pgName string, ignoreNotFound bool) (*ovnnb.PortGroup, error) {
+	if pgName == "" {
+		return nil, errors.New("port group name is empty")
+	}
 	ctx, cancel := context.WithTimeout(context.Background(), c.Timeout)
 	defer cancel()
 

diff --git a/pkg/ovs/ovn-nb-suite_test.go b/pkg/ovs/ovn-nb-suite_test.go
@@ -1056,6 +1056,10 @@ func (suite *OvnClientTestSuite) Test_OvsFind() {
 	suite.testOvsFind()
 }
 
+func (suite *OvnClientTestSuite) Test_ParseOvsFindOutput() {
+	suite.testParseOvsFindOutput()
+}
+
 func (suite *OvnClientTestSuite) Test_OvsClear() {
 	suite.testOvsClear()
 }

diff --git a/pkg/ovs/ovn-nb_global.go b/pkg/ovs/ovn-nb_global.go
@@ -13,6 +13,12 @@ import (
 )
 
 func (c *OVNNbClient) CreateNbGlobal(nbGlobal *ovnnb.NBGlobal) error {
+	if nbGlobal == nil {
+		err := errors.New("nb global is nil")
+		klog.Error(err)
+		return err
+	}
+
 	op, err := c.ovsDbClient.Create(nbGlobal)
 	if err != nil {
 		klog.Error(err)
@@ -61,6 +67,12 @@ func (c *OVNNbClient) GetNbGlobal() (*ovnnb.NBGlobal, error) {
 }
 
 func (c *OVNNbClient) UpdateNbGlobal(nbGlobal *ovnnb.NBGlobal, fields ...interface{}) error {
+	if nbGlobal == nil {
+		err := errors.New("nb global is nil")
+		klog.Error(err)
+		return err
+	}
+
 	op, err := c.Where(nbGlobal).Update(nbGlobal, fields...)
 	if err != nil {
 		klog.Error(err)

diff --git a/pkg/ovs/ovn-nb_global_test.go b/pkg/ovs/ovn-nb_global_test.go
@@ -22,9 +22,13 @@ func (suite *OvnClientTestSuite) testGetNbGlobal() {
 	t := suite.T()
 
 	nbClient := suite.ovnNBClient
+	failedNbClient := suite.faiedOvnNBClient
 
 	t.Cleanup(func() {
-		err := nbClient.DeleteNbGlobal()
+		err := failedNbClient.DeleteNbGlobal()
+		require.Error(t, err)
+
+		err = nbClient.DeleteNbGlobal()
 		require.NoError(t, err)
 
 		_, err = nbClient.GetNbGlobal()
@@ -45,12 +49,18 @@ func (suite *OvnClientTestSuite) testGetNbGlobal() {
 		require.NoError(t, err)
 		require.NotEmpty(t, out.UUID)
 	})
+
+	t.Run("failed client create nb_global", func(t *testing.T) {
+		err := failedNbClient.CreateNbGlobal(nil)
+		require.Error(t, err)
+	})
 }
 
 func (suite *OvnClientTestSuite) testUpdateNbGlobal() {
 	t := suite.T()
 
 	nbClient := suite.ovnNBClient
+	failedNbClient := suite.faiedOvnNBClient
 
 	t.Cleanup(func() {
 		err := nbClient.DeleteNbGlobal()
@@ -79,6 +89,11 @@ func (suite *OvnClientTestSuite) testUpdateNbGlobal() {
 		require.Equal(t, "16711680", out.Options["max_tunid"])
 	})
 
+	t.Run("failed client update", func(t *testing.T) {
+		err = failedNbClient.UpdateNbGlobal(nil)
+		require.Error(t, err)
+	})
+
 	t.Run("create options", func(t *testing.T) {
 		nbGlobal := &ovnnb.NBGlobal{
 			UUID: nbGlobal.UUID,

diff --git a/pkg/ovs/ovn-nb_test.go b/pkg/ovs/ovn-nb_test.go
@@ -15,6 +15,7 @@ func (suite *OvnClientTestSuite) testCreateGatewayLogicalSwitch() {
 	t.Parallel()
 
 	nbClient := suite.ovnNBClient
+	failedNbClient := suite.faiedOvnNBClient
 	lsName := "test-create-gw-ls"
 	lrName := "test-create-gw-lr"
 	lspName := fmt.Sprintf("%s-%s", lsName, lrName)
@@ -25,6 +26,11 @@ func (suite *OvnClientTestSuite) testCreateGatewayLogicalSwitch() {
 	err := nbClient.CreateLogicalRouter(lrName)
 	require.NoError(t, err)
 
+	// create with failed client
+	err = failedNbClient.CreateGatewayLogicalSwitch(lsName, lrName, "test-external", "192.168.230.1/24,fc00::0af4:01/112", util.GenerateMac(), 210, chassises...)
+	require.Error(t, err)
+
+	// create with normal client
 	err = nbClient.CreateGatewayLogicalSwitch(lsName, lrName, "test-external", "192.168.230.1/24,fc00::0af4:01/112", util.GenerateMac(), 210, chassises...)
 	require.NoError(t, err)
 
@@ -52,6 +58,8 @@ func (suite *OvnClientTestSuite) testCreateLogicalPatchPort() {
 	t.Parallel()
 
 	nbClient := suite.ovnNBClient
+	failedNbClient := suite.faiedOvnNBClient
+
 	lsName := "test-create-router-ls"
 	lrName := "test-create-router-lr"
 	lspName := fmt.Sprintf("%s-%s", lsName, lrName)
@@ -101,13 +109,42 @@ func (suite *OvnClientTestSuite) testCreateLogicalPatchPort() {
 		require.NoError(t, err)
 		require.Empty(t, lrp.GatewayChassis)
 	})
+
+	t.Run("failed client to create router port with chassises", func(t *testing.T) {
+		t.Parallel()
+		// failed to create with failed client
+		err := failedNbClient.CreateLogicalPatchPort(lsName, lrName, lspName, lrpName, "192.168.230.1/24,fc00::0af4:01/112", util.GenerateMac(), chassises...)
+		require.Error(t, err)
+
+		// failed to create with invalid cidr
+		err = failedNbClient.CreateLogicalPatchPort(lsName, lrName, lspName, lrpName, "192.168.230.1/33,fc00::0af4:01/129", util.GenerateMac(), chassises...)
+		require.Error(t, err)
+
+		err = failedNbClient.CreateLogicalPatchPort(lsName, lrName, lspName, lrpName, "0.0.0.0/0,fc00::0af4:01/112", util.GenerateMac(), chassises...)
+		require.Error(t, err)
+
+		err = failedNbClient.CreateLogicalPatchPort(lsName, lrName, lspName, lrpName, "192.168.230.1/24,00::00:00/0", util.GenerateMac(), chassises...)
+		require.Error(t, err)
+
+		err = failedNbClient.CreateLogicalPatchPort(lsName, lrName, lspName, lrpName, "192.168.230.1/31,fc00::0af4:01/127", util.GenerateMac(), chassises...)
+		require.Error(t, err)
+
+		err = failedNbClient.CreateLogicalPatchPort(lsName, lrName, lspName, lrpName, "192.168.230.1/32fc00::0af4:01/128", util.GenerateMac(), chassises...)
+		require.Error(t, err)
+
+		lrp, err := nbClient.GetLogicalRouterPort(lrpName, false)
+		require.Error(t, err)
+		require.Nil(t, lrp)
+	})
 }
 
 func (suite *OvnClientTestSuite) testRemoveRouterPort() {
 	t := suite.T()
 	t.Parallel()
 
 	nbClient := suite.ovnNBClient
+	failedNbClient := suite.faiedOvnNBClient
+
 	lsName := "test-remove-router-type-ls"
 	lrName := "test-remove-router-type-lr"
 	lspName := fmt.Sprintf("%s-%s", lsName, lrName)
@@ -131,13 +168,24 @@ func (suite *OvnClientTestSuite) testRemoveRouterPort() {
 		err = nbClient.RemoveLogicalPatchPort(lspName, lrpName)
 		require.NoError(t, err)
 	})
+
+	t.Run("failed client del router type port", func(t *testing.T) {
+		err = failedNbClient.RemoveLogicalPatchPort(lspName, lrpName)
+		require.Nil(t, err)
+		err = failedNbClient.RemoveLogicalPatchPort("", lrpName)
+		require.Error(t, err)
+		err = failedNbClient.RemoveLogicalPatchPort(lspName, "")
+		require.Nil(t, err)
+	})
 }
 
 func (suite *OvnClientTestSuite) testDeleteLogicalGatewaySwitch() {
 	t := suite.T()
 	t.Parallel()
 
 	nbClient := suite.ovnNBClient
+	failedNbClient := suite.faiedOvnNBClient
+
 	lsName := "test-del-gw-ls"
 	lrName := "test-del-gw-lr"
 
@@ -147,6 +195,15 @@ func (suite *OvnClientTestSuite) testDeleteLogicalGatewaySwitch() {
 	err = nbClient.CreateGatewayLogicalSwitch(lsName, lrName, "test-external", "192.168.230.1/24,fc00::0af4:01/112", util.GenerateMac(), 210)
 	require.NoError(t, err)
 
+	err = failedNbClient.DeleteLogicalGatewaySwitch("", lrName)
+	require.Error(t, err)
+
+	err = failedNbClient.DeleteLogicalGatewaySwitch(lsName, "")
+	require.Nil(t, err)
+
+	err = failedNbClient.DeleteLogicalGatewaySwitch(lsName, lrName)
+	require.Nil(t, err)
+
 	// localnet port and lsp will be deleted when delete logical switch in real ovsdb,
 	// it's different from the mock memory ovsdb,
 	// so no need to check localnet port and lsp existence
@@ -162,14 +219,27 @@ func (suite *OvnClientTestSuite) testDeleteSecurityGroup() {
 	t.Parallel()
 
 	nbClient := suite.ovnNBClient
+	failedNbClient := suite.faiedOvnNBClient
 	sgName := "test_del_sg"
 	asName := "test_del_sg_as"
 	pgName := GetSgPortGroupName(sgName)
 	priority := "5111"
 	match := "outport == @ovn.sg.test_del_sg && ip"
 
-	/* prepate test */
-	err := nbClient.CreatePortGroup(pgName, map[string]string{
+	// create with empty pg
+	err := nbClient.CreatePortGroup("", map[string]string{
+		"type": "security_group",
+		sgKey:  sgName,
+	})
+	require.Error(t, err)
+	// create with failed client
+	err = failedNbClient.CreatePortGroup(pgName, map[string]string{
+		"type": "security_group",
+		sgKey:  sgName,
+	})
+	require.Error(t, err)
+	//  create with normal pg
+	err = nbClient.CreatePortGroup(pgName, map[string]string{
 		"type": "security_group",
 		sgKey:  sgName,
 	})
@@ -186,7 +256,14 @@ func (suite *OvnClientTestSuite) testDeleteSecurityGroup() {
 	})
 	require.NoError(t, err)
 
-	/* run test */
+	// failed client delete sg
+	err = failedNbClient.DeleteSecurityGroup("")
+	require.Error(t, err)
+
+	err = failedNbClient.DeleteSecurityGroup(sgName)
+	require.Nil(t, err)
+
+	// normal delete sg
 	err = nbClient.DeleteSecurityGroup(sgName)
 	require.NoError(t, err)
 

diff --git a/pkg/ovs/ovn-sb-chassis_test.go b/pkg/ovs/ovn-sb-chassis_test.go
@@ -491,7 +491,14 @@ func (suite *OvnClientTestSuite) testGetKubeOvnChassisses() {
 		require.NoError(t, err)
 		chassisList, err := sbClient.GetKubeOvnChassisses()
 		require.NoError(t, err)
-		require.Empty(t, *chassisList)
+		names := make(map[string]bool)
+		for _, chassis := range *chassisList {
+			names[chassis.Name] = true
+		}
+		require.False(t, names["kube-ovn-chassis-1"])
+		require.False(t, names["kube-ovn-chassis-2"])
+		require.False(t, names["non-kube-ovn-chassis"])
+		require.False(t, names["mixed-chassis"])
 	})
 
 	kubeOvnChassis1 := newChassis(0, "host-1", "kube-ovn-chassis-1", nil, nil, nil, map[string]string{"vendor": util.CniTypeName}, nil)

diff --git a/pkg/ovs/ovs-vsctl.go b/pkg/ovs/ovs-vsctl.go
@@ -112,6 +112,11 @@ func ovsFind(table, column string, conditions ...string) ([]string, error) {
 		klog.Error(err)
 		return nil, err
 	}
+	ret := parseOvsFindOutput(output)
+	return ret, nil
+}
+
+func parseOvsFindOutput(output string) []string {
 	values := strings.Split(output, "\n\n")
 	// We want "bare" values for strings, but we can't pass --bare to ovs-vsctl because
 	// it breaks more complicated types. So try passing each value through Unquote();
@@ -127,7 +132,7 @@ func ovsFind(table, column string, conditions ...string) ([]string, error) {
 			ret = append(ret, strings.Trim(strings.TrimSpace(val), "\""))
 		}
 	}
-	return ret, nil
+	return ret
 }
 
 func ovsClear(table, record string, columns ...string) error {

diff --git a/pkg/ovs/ovs-vsctl_test.go b/pkg/ovs/ovs-vsctl_test.go
@@ -75,6 +75,17 @@ func (suite *OvnClientTestSuite) testOvsFind() {
 	require.Empty(t, ret)
 }
 
+func (suite *OvnClientTestSuite) testParseOvsFindOutput() {
+	t := suite.T()
+	t.Parallel()
+	input := `br-int
+
+br-businessnet
+`
+	ret := parseOvsFindOutput(input)
+	require.Len(t, ret, 2)
+}
+
 func (suite *OvnClientTestSuite) testOvsClear() {
 	t := suite.T()
 	t.Parallel()

diff --git a/pkg/ovs/util.go b/pkg/ovs/util.go
@@ -43,6 +43,9 @@ func LogicalSwitchPortName(lr, ls string) string {
 }
 
 func GetSgPortGroupName(sgName string) string {
+	if sgName == "" {
+		return ""
+	}
 	return strings.ReplaceAll(fmt.Sprintf("ovn.sg.%s", sgName), "-", ".")
 }