Implement a new API called ExecuteR() (#128)

* Implement a new API called ExecuteR(). ExecuteR() works exactly the same as the existing Execute() function, except that it returns a list of UUIDs for each object created in the call. If no objects are created in the ExecuteR call, or there is an error, nil is returned. Signed-off-by: Andre Fredette <[email protected]> Co-authored-by: Vasiliy Tolstov <[email protected]>
eBay · Mar 17, 2021 · bd08b71 · bd08b71
1 parent f6ee8f3
commit bd08b71
Show file tree

Hide file tree

Showing 5 changed files with 149 additions and 35 deletions.
diff --git a/acl_test.go b/acl_test.go
@@ -200,7 +200,6 @@ func TestLogicalSwitchACLs(t *testing.T) {
 	}
 	assert.Equal(t, true, len(acls) == 4, "test[%s]", "add second acl")
 
-
 	cmd, err = ovndbapi.ACLDel(LSW, "to-lport", MATCH_SECOND, 1002, nil)
 	if err != nil {
 		t.Fatal(err)
@@ -349,10 +348,10 @@ func compareMeterSlices(s1, s2 []string) bool {
 func containsACL(aclList []*ACL, acl *ACL) bool{
 	for _, a := range aclList {
 		// Compare everything except UUID
-		if a.Action == acl.Action && 
-			a.Direction == acl.Direction && 
-			a.Match == acl.Match && 
-			a.Priority == acl.Priority && 
+		if a.Action == acl.Action &&
+			a.Direction == acl.Direction &&
+			a.Match == acl.Match &&
+			a.Priority == acl.Priority &&
 			a.Log == acl.Log &&
 			compareMeterSlices(a.Meter, acl.Meter) &&
 			a.Severity == acl.Severity &&
@@ -408,14 +407,12 @@ func TestPortGroupACLs(t *testing.T) {
 		cmds = append(cmds, cmd)
 		assert.Nil(t, err)
 
-		err = ovndbapi.Execute(cmds...)
+		result, err := ovndbapi.ExecuteR(cmds...)
 		assert.Nil(t, err)
+		assert.Equal(t, 3, len(result))
 
-		// LSP commands require that ports be described by a UUID, so get the UUIDs
-		lsp1UUID, err := lspNameToUUID(PG_TEST_LSP1, ovndbapi)
-		assert.Nil(t, err)
-		lsp2UUID, err := lspNameToUUID(PG_TEST_LSP2, ovndbapi)
-		assert.Nil(t, err)
+		lsp1UUID := result[1]
+		lsp2UUID := result[2]
 
 		// Create port group
 		cmd, err = ovndbapi.PortGroupAdd(PG_TEST_PG1, []string{lsp1UUID, lsp2UUID}, nil)

diff --git a/client.go b/client.go
@@ -193,6 +193,8 @@ type Client interface {
 	MeterBandsList() ([]*MeterBand, error)
 	// Exec command, support mul-commands in one transaction.
 	Execute(cmds ...*OvnCommand) error
+	// Same as Execute, but returns a UUID for each object created.
+	ExecuteR(cmds ...*OvnCommand) ([]string, error)
 
 	// Add chassis with given name
 	ChassisAdd(name string, hostname string, etype []string, ip string, external_ids map[string]string,
@@ -656,6 +658,10 @@ func (c *ovndb) Execute(cmds ...*OvnCommand) error {
 	return c.execute(cmds...)
 }
 
+func (c *ovndb) ExecuteR(cmds ...*OvnCommand) ([]string, error) {
+	return c.executeR(cmds...)
+}
+
 func (c *ovndb) LSGet(ls string) ([]*LogicalSwitch, error) {
 	return c.lsGetImp(ls)
 }

diff --git a/ovnimp.go b/ovnimp.go
@@ -178,8 +178,13 @@ func (odbi *ovndb) transact(db string, ops ...libovsdb.Operation) ([]libovsdb.Op
 }
 
 func (odbi *ovndb) execute(cmds ...*OvnCommand) error {
+	_, err := odbi.ExecuteR(cmds...)
+	return err
+}
+
+func (odbi *ovndb) executeR(cmds ...*OvnCommand) ([]string, error) {
 	if cmds == nil {
-		return nil
+		return nil, nil
 	}
 	var ops []libovsdb.Operation
 	for _, cmd := range cmds {
@@ -188,11 +193,24 @@ func (odbi *ovndb) execute(cmds ...*OvnCommand) error {
 		}
 	}
 
-	_, err := odbi.transact(odbi.db, ops...)
+	results, err := odbi.transact(odbi.db, ops...)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	return nil
+
+	// The total number of UUIDs will be <= number of results returned.
+	UUIDs := make([]string, 0, len(results))
+	for _, r := range results {
+		if len(r.UUID.GoUUID) > 0 {
+			UUIDs = append(UUIDs, r.UUID.GoUUID)
+		}
+	}
+
+	if len(UUIDs) > 0 {
+		return UUIDs, nil
+	}
+
+	return nil, nil
 }
 
 func (odbi *ovndb) float64_to_int(row libovsdb.Row) {

diff --git a/ovnimp_test.go b/ovnimp_test.go
@@ -102,3 +102,108 @@ func TestConvertGoSetToStringArray(t *testing.T) {
 	}
 	t.Logf("Deleted the logical switch " + LSW)
 }
+
+func lsNameToUUID(lsName string, c Client) (string, error) {
+	lsList, err := c.LSList()
+	if err == nil {
+		for _, ls := range lsList {
+			if ls.Name == lsName {
+				return ls.UUID, nil
+			}
+			return "", ErrorNotFound
+		}
+	}
+	return "", err
+}
+
+func lspNameToUUID(lspName string, c Client) (string, error) {
+	lsp, err := c.LSPGet(lspName)
+	if err == nil {
+		return lsp.UUID, nil
+	} else {
+		return "", ErrorNotFound
+	}
+}
+
+func TestExecuteR(t *testing.T) {
+	ovndbapi := getOVNClient(DBNB)
+
+	t.Run("execute one command in an ExecuteR call", func(t *testing.T) {
+		// Create Switch
+		cmd, err := ovndbapi.LSAdd(PG_TEST_LS1)
+		assert.Nil(t, err)
+		result, err := ovndbapi.ExecuteR(cmd)
+		assert.Nil(t, err)
+		assert.Equal(t, 1, len(result))
+		//Check UUID returned
+		lsUUID, err := lsNameToUUID(PG_TEST_LS1, ovndbapi)
+		assert.Nil(t, err)
+		assert.Greater(t, len(lsUUID), 0)
+		assert.Equal(t, lsUUID, result[0])
+
+		// Delete Switch (LSPs will get deleted by OVSDB)
+		cmd, err = ovndbapi.LSDel(PG_TEST_LS1)
+		assert.Nil(t, err)
+		result, err = ovndbapi.ExecuteR(cmd)
+		assert.Nil(t, err)
+		// LSDel should not return any UUIDs
+		assert.Nil(t, result)
+	})
+
+	t.Run("execute multiple commands in one ExecuteR call", func(t *testing.T) {
+		var cmds []*OvnCommand
+
+		// Create switch and ports
+		cmd, err := ovndbapi.LSAdd(PG_TEST_LS1)
+		assert.Nil(t, err)
+		cmds = append(cmds, cmd)
+		// Add ports
+		cmd, err = ovndbapi.LSPAdd(PG_TEST_LS1, PG_TEST_LSP1)
+		assert.Nil(t, err)
+		cmds = append(cmds, cmd)
+		cmd, err = ovndbapi.LSPSetAddress(PG_TEST_LSP1, ADDR)
+		assert.Nil(t, err)
+		cmds = append(cmds, cmd)
+		cmd, err = ovndbapi.LSPSetPortSecurity(PG_TEST_LSP1, ADDR)
+		assert.Nil(t, err)
+		cmds = append(cmds, cmd)
+		cmd, err = ovndbapi.LSPAdd(PG_TEST_LS1, PG_TEST_LSP2)
+		assert.Nil(t, err)
+		cmds = append(cmds, cmd)
+		cmd, err = ovndbapi.LSPSetAddress(PG_TEST_LSP2, ADDR2)
+		assert.Nil(t, err)
+		cmds = append(cmds, cmd)
+		cmd, err = ovndbapi.LSPSetPortSecurity(PG_TEST_LSP2, ADDR2)
+		cmds = append(cmds, cmd)
+		assert.Nil(t, err)
+
+		result, err := ovndbapi.ExecuteR(cmds...)
+		assert.Nil(t, err)
+		// Only the 3 "Add" commands above should return a UUID
+		assert.Equal(t, 3, len(result))
+
+		//Check UUIDs returned
+		lsUUID, err := lsNameToUUID(PG_TEST_LS1, ovndbapi)
+		assert.Nil(t, err)
+		assert.Greater(t, len(lsUUID), 0)
+		assert.Equal(t, lsUUID, result[0])
+
+		lsp1UUID, err := lspNameToUUID(PG_TEST_LSP1, ovndbapi)
+		assert.Nil(t, err)
+		assert.Greater(t, len(lsp1UUID), 0)
+		assert.Equal(t, lsp1UUID, result[1])
+
+		lsp2UUID, err := lspNameToUUID(PG_TEST_LSP2, ovndbapi)
+		assert.Nil(t, err)
+		assert.Greater(t, len(lsp2UUID), 0)
+		assert.Equal(t, lsp2UUID, result[2])
+
+		// Delete Switch (LSPs will get deleted by OVSDB)
+		cmd, err = ovndbapi.LSDel(PG_TEST_LS1)
+		assert.Nil(t, err)
+		result, err = ovndbapi.ExecuteR(cmd)
+		assert.Nil(t, err)
+		// LSDel should not return any UUIDs
+		assert.Nil(t, result)
+	})
+}
diff --git a/port_group_test.go b/port_group_test.go
@@ -37,15 +37,6 @@ startConfig pgConfig
 testConfig  pgConfig
 }
 
-func lspNameToUUID(lspName string, c Client) (string, error) {
-	lsp1, err := c.LSPGet(lspName)
-	if err == nil {
-		return lsp1.UUID, nil
-	} else {
-		return "", ErrorNotFound
-	}
-}
-
 func compareExternalIds(want map[string]string, got map[interface{}]interface{}) bool {
 	if len(want) != len(got) {
 		return false
@@ -63,6 +54,7 @@ func TestPortGroupAPI(t *testing.T) {
 	assert := assert.New(t)
 	var cmd *OvnCommand
 	var err error
+	var lsp1UUID, lsp2UUID, lsp3UUID, lsp4UUID string
 
 	// create Switch with four ports
 	createSwitch := func(t *testing.T) {
@@ -86,8 +78,14 @@ func TestPortGroupAPI(t *testing.T) {
 		assert.Nil(err)
 		cmds = append(cmds, cmd)
 
-		err = ovndbapi.Execute(cmds...)
+		result, err := ovndbapi.ExecuteR(cmds...)
 		assert.Nil(err)
+
+		assert.Equal(5, len(result))
+		lsp1UUID = result[1]
+		lsp2UUID = result[2]
+		lsp3UUID = result[3]
+		lsp4UUID = result[4]
 	}
 
 	// Delete Switch
@@ -104,16 +102,6 @@ func TestPortGroupAPI(t *testing.T) {
 	// Create a switch w/four ports to be used for logical port tests
 	createSwitch(t)
 
-	// LSP commands require that ports be described by a UUID, so get the UUIDs
-	lsp1UUID, err := lspNameToUUID(PG_TEST_LSP1, ovndbapi)
-	assert.Nil(err)
-	lsp2UUID, err := lspNameToUUID(PG_TEST_LSP2, ovndbapi)
-	assert.Nil(err)
-	lsp3UUID, err := lspNameToUUID(PG_TEST_LSP3, ovndbapi)
-	assert.Nil(err)
-	lsp4UUID, err := lspNameToUUID(PG_TEST_LSP4, ovndbapi)
-	assert.Nil(err)
-
 	portGroupAddTests := []pgTest {
 		{
 			name: "add an empty port group",