Merge pull request #359 from dcbw/named-uuid-cleanup

Named UUID cleanup and enhancements
ovn-org · Jun 30, 2023 · 2fcc81c · 2fcc81c
2 parents 745dea0 + f5b801a
commit 2fcc81c
Show file tree

Hide file tree

Showing 13 changed files with 825 additions and 204 deletions.
diff --git a/client/api.go b/client/api.go
@@ -283,7 +283,7 @@ func (a api) Create(models ...model.Model) ([]ovsdb.Operation, error) {
 	var operations []ovsdb.Operation
 
 	for _, model := range models {
-		var namedUUID string
+		var realUUID, namedUUID string
 		var err error
 
 		tableName, err := a.getTableFromModel(model)
@@ -297,7 +297,12 @@ func (a api) Create(models ...model.Model) ([]ovsdb.Operation, error) {
 			return nil, err
 		}
 		if uuid, err := info.FieldByColumn("_uuid"); err == nil {
-			namedUUID = uuid.(string)
+			tmpUUID := uuid.(string)
+			if ovsdb.IsNamedUUID(tmpUUID) {
+				namedUUID = tmpUUID
+			} else if ovsdb.IsValidUUID(tmpUUID) {
+				realUUID = tmpUUID
+			}
 		} else {
 			return nil, err
 		}
@@ -306,11 +311,14 @@ func (a api) Create(models ...model.Model) ([]ovsdb.Operation, error) {
 		if err != nil {
 			return nil, err
 		}
+		// UUID is given in the operation, not the object
+		delete(row, "_uuid")
 
 		operations = append(operations, ovsdb.Operation{
 			Op:       ovsdb.OperationInsert,
 			Table:    tableName,
 			Row:      row,
+			UUID:     realUUID,
 			UUIDName: namedUUID,
 		})
 	}

diff --git a/database/transaction.go b/database/transaction.go
@@ -45,6 +45,22 @@ func (t *Transaction) Transact(operations []ovsdb.Operation) ([]*ovsdb.Operation
 	results := []*ovsdb.OperationResult{}
 	update := updates.ModelUpdates{}
 
+	// Every Insert operation must have a UUID
+	for i := range operations {
+		op := &operations[i]
+		if op.Op == ovsdb.OperationInsert && op.UUID == "" {
+			op.UUID = uuid.NewString()
+		}
+	}
+
+	// Ensure Named UUIDs are expanded in all operations
+	var err error
+	operations, err = ovsdb.ExpandNamedUUIDs(operations, &t.Model.Schema)
+	if err != nil {
+		r := ovsdb.ResultFromError(err)
+		return []*ovsdb.OperationResult{&r}, nil
+	}
+
 	var r ovsdb.OperationResult
 	for _, op := range operations {
 		// if we had a previous error, just append a nil result for every op
@@ -200,19 +216,18 @@ func (t *Transaction) checkIndexes() error {
 }
 
 func (t *Transaction) Insert(op *ovsdb.Operation) (ovsdb.OperationResult, *updates.ModelUpdates) {
-	rowUUID := op.UUIDName
-	if rowUUID == "" {
-		rowUUID = uuid.NewString()
+	if err := ovsdb.ValidateUUID(op.UUID); err != nil {
+		return ovsdb.ResultFromError(err), nil
 	}
 
 	update := updates.ModelUpdates{}
-	err := update.AddOperation(t.Model, op.Table, rowUUID, nil, op)
+	err := update.AddOperation(t.Model, op.Table, op.UUID, nil, op)
 	if err != nil {
 		return ovsdb.ResultFromError(err), nil
 	}
 
 	result := ovsdb.OperationResult{
-		UUID: ovsdb.UUID{GoUUID: rowUUID},
+		UUID: ovsdb.UUID{GoUUID: op.UUID},
 	}
 
 	return result, &update

diff --git a/database/transaction_test.go b/database/transaction_test.go
@@ -48,20 +48,20 @@ func TestWaitOpEquals(t *testing.T) {
 	transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil)
 
 	operation := ovsdb.Operation{
-		Op:       ovsdb.OperationInsert,
-		Table:    "Open_vSwitch",
-		UUIDName: ovsUUID,
-		Row:      ovsRow,
+		Op:    ovsdb.OperationInsert,
+		Table: "Open_vSwitch",
+		UUID:  ovsUUID,
+		Row:   ovsRow,
 	}
 	res, updates := transaction.Insert(&operation)
 	_, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}})
 	require.Nil(t, err)
 
 	operation = ovsdb.Operation{
-		Op:       ovsdb.OperationInsert,
-		Table:    "Bridge",
-		UUIDName: bridgeUUID,
-		Row:      bridgeRow,
+		Op:    ovsdb.OperationInsert,
+		Table: "Bridge",
+		UUID:  bridgeUUID,
+		Row:   bridgeRow,
 	}
 	res, update2 := transaction.Insert(&operation)
 	_, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}})
@@ -175,20 +175,20 @@ func TestWaitOpNotEquals(t *testing.T) {
 	transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil)
 
 	operation := ovsdb.Operation{
-		Op:       ovsdb.OperationInsert,
-		Table:    "Open_vSwitch",
-		UUIDName: ovsUUID,
-		Row:      ovsRow,
+		Op:    ovsdb.OperationInsert,
+		Table: "Open_vSwitch",
+		UUID:  ovsUUID,
+		Row:   ovsRow,
 	}
 	res, updates := transaction.Insert(&operation)
 	_, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}})
 	require.Nil(t, err)
 
 	operation = ovsdb.Operation{
-		Op:       ovsdb.OperationInsert,
-		Table:    "Bridge",
-		UUIDName: bridgeUUID,
-		Row:      bridgeRow,
+		Op:    ovsdb.OperationInsert,
+		Table: "Bridge",
+		UUID:  bridgeUUID,
+		Row:   bridgeRow,
 	}
 	res, update2 := transaction.Insert(&operation)
 	_, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}})
@@ -295,20 +295,20 @@ func TestMutateOp(t *testing.T) {
 	transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil)
 
 	operation := ovsdb.Operation{
-		Op:       ovsdb.OperationInsert,
-		Table:    "Open_vSwitch",
-		UUIDName: ovsUUID,
-		Row:      ovsRow,
+		Op:    ovsdb.OperationInsert,
+		Table: "Open_vSwitch",
+		UUID:  ovsUUID,
+		Row:   ovsRow,
 	}
 	res, updates := transaction.Insert(&operation)
 	_, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}})
 	require.Nil(t, err)
 
 	operation = ovsdb.Operation{
-		Op:       ovsdb.OperationInsert,
-		Table:    "Bridge",
-		UUIDName: bridgeUUID,
-		Row:      bridgeRow,
+		Op:    ovsdb.OperationInsert,
+		Table: "Bridge",
+		UUID:  bridgeUUID,
+		Row:   bridgeRow,
 	}
 	res, update2 := transaction.Insert(&operation)
 	_, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}})
@@ -411,10 +411,10 @@ func TestOvsdbServerInsert(t *testing.T) {
 	transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil)
 
 	operation := ovsdb.Operation{
-		Op:       ovsdb.OperationInsert,
-		Table:    "Bridge",
-		UUIDName: bridgeUUID,
-		Row:      bridgeRow,
+		Op:    ovsdb.OperationInsert,
+		Table: "Bridge",
+		UUID:  bridgeUUID,
+		Row:   bridgeRow,
 	}
 	res, updates := transaction.Insert(&operation)
 	_, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}})
@@ -464,10 +464,10 @@ func TestOvsdbServerUpdate(t *testing.T) {
 	transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil)
 
 	operation := ovsdb.Operation{
-		Op:       ovsdb.OperationInsert,
-		Table:    "Bridge",
-		UUIDName: bridgeUUID,
-		Row:      bridgeRow,
+		Op:    ovsdb.OperationInsert,
+		Table: "Bridge",
+		UUID:  bridgeUUID,
+		Row:   bridgeRow,
 	}
 	res, updates := transaction.Insert(&operation)
 	_, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}})
@@ -556,9 +556,9 @@ func TestMultipleOps(t *testing.T) {
 
 	bridgeUUID := uuid.NewString()
 	op = ovsdb.Operation{
-		Op:       ovsdb.OperationInsert,
-		Table:    "Bridge",
-		UUIDName: bridgeUUID,
+		Op:    ovsdb.OperationInsert,
+		Table: "Bridge",
+		UUID:  bridgeUUID,
 		Row: ovsdb.Row{
 			"name": "a_bridge_to_nowhere",
 		},
@@ -667,9 +667,9 @@ func TestOvsdbServerDbDoesNotExist(t *testing.T) {
 
 	ops := []ovsdb.Operation{
 		{
-			Op:       ovsdb.OperationInsert,
-			Table:    "Bridge",
-			UUIDName: uuid.NewString(),
+			Op:    ovsdb.OperationInsert,
+			Table: "Bridge",
+			UUID:  uuid.NewString(),
 			Row: ovsdb.Row{
 				"name": "bridge",
 			},
@@ -706,19 +706,21 @@ func TestCheckIndexes(t *testing.T) {
 
 	bridgeUUID := uuid.NewString()
 	fscsUUID := uuid.NewString()
+	fscsUUID2 := uuid.NewString()
+	fscsUUID3 := uuid.NewString()
 	ops := []ovsdb.Operation{
 		{
-			Table:    "Bridge",
-			Op:       ovsdb.OperationInsert,
-			UUIDName: bridgeUUID,
+			Table: "Bridge",
+			Op:    ovsdb.OperationInsert,
+			UUID:  bridgeUUID,
 			Row: ovsdb.Row{
 				"name": "a_bridge_to_nowhere",
 			},
 		},
 		{
-			Table:    "Flow_Sample_Collector_Set",
-			Op:       ovsdb.OperationInsert,
-			UUIDName: fscsUUID,
+			Table: "Flow_Sample_Collector_Set",
+			Op:    ovsdb.OperationInsert,
+			UUID:  fscsUUID,
 			Row: ovsdb.Row{
 				"id":     1,
 				"bridge": ovsdb.UUID{GoUUID: bridgeUUID},
@@ -727,6 +729,7 @@ func TestCheckIndexes(t *testing.T) {
 		{
 			Table: "Flow_Sample_Collector_Set",
 			Op:    ovsdb.OperationInsert,
+			UUID:  fscsUUID2,
 			Row: ovsdb.Row{
 				"id":     2,
 				"bridge": ovsdb.UUID{GoUUID: bridgeUUID},
@@ -755,6 +758,7 @@ func TestCheckIndexes(t *testing.T) {
 					{
 						Table: "Flow_Sample_Collector_Set",
 						Op:    ovsdb.OperationInsert,
+						UUID:  fscsUUID3,
 						Row: ovsdb.Row{
 							"id":     1,
 							"bridge": ovsdb.UUID{GoUUID: bridgeUUID},
@@ -790,6 +794,7 @@ func TestCheckIndexes(t *testing.T) {
 					{
 						Table: "Flow_Sample_Collector_Set",
 						Op:    ovsdb.OperationInsert,
+						UUID:  fscsUUID3,
 						Row: ovsdb.Row{
 							"id":     3,
 							"bridge": ovsdb.UUID{GoUUID: bridgeUUID},
@@ -817,6 +822,7 @@ func TestCheckIndexes(t *testing.T) {
 					{
 						Table: "Flow_Sample_Collector_Set",
 						Op:    ovsdb.OperationInsert,
+						UUID:  fscsUUID3,
 						Row: ovsdb.Row{
 							"id":     1,
 							"bridge": ovsdb.UUID{GoUUID: bridgeUUID},
@@ -844,6 +850,7 @@ func TestCheckIndexes(t *testing.T) {
 					{
 						Table: "Flow_Sample_Collector_Set",
 						Op:    ovsdb.OperationInsert,
+						UUID:  fscsUUID3,
 						Row: ovsdb.Row{
 							"id":     1,
 							"bridge": ovsdb.UUID{GoUUID: bridgeUUID},

diff --git a/ovsdb/bindings.go b/ovsdb/bindings.go
@@ -409,7 +409,7 @@ func isDefaultBaseValue(elem interface{}, etype ExtendedType) bool {
 	}
 	switch etype {
 	case TypeUUID:
-		return elem.(string) == "00000000-0000-0000-0000-000000000000" || elem.(string) == "" || isNamed(elem.(string))
+		return elem.(string) == "00000000-0000-0000-0000-000000000000" || elem.(string) == ""
 	case TypeMap, TypeSet:
 		if value.Kind() == reflect.Array {
 			return value.Len() == 0