Skip to content

Commit 2b9b7f8

Browse files
dcbwdcbw
committed
ovsdb: don't create sized arrays for OVS Sets
OVS Sets must be unique, but modelgen created sized arrays to represent OVS Sets, which when marshalled are filled with duplicate default values. For a column schema like: "cvlans": { "type": {"key": {"type": "integer", "minInteger": 0, "maxInteger": 4095}, "min": 0, "max": 4096}}, modelgen would create: CVLANs [4096]int which when marshalled and sent to ovsdb-server becomes: cvlans:{GoSet:[0 0 0 0 0 0 0 <repeat many times>]} and is rejected by ovsdb-server with: {Count:0 Error:ovsdb error Details:set contains duplicate UUID:{GoUUID:} Rows:[]}] and errors [ovsdb error: set contains duplicate]: 1 ovsdb operations failed Instead, generate these fields as slices instead of sized arrays and enforce the size when marshalling the set. Signed-off-by: Dan Williams <[email protected]>
1 parent e4d2d86 commit 2b9b7f8

21 files changed

+307
-190
lines changed

‎cache/cache_test.go‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1951,6 +1951,24 @@ func BenchmarkIndexExists(b *testing.B) {
19511951
}
19521952
}
19531953

1954+
func testOvsSet(t assert.TestingT, keyType string, set interface{}) ovsdb.OvsSet {
1955+
colSchema := fmt.Sprintf(`{
1956+
"type": {
1957+
"key": {"type": "%s"},
1958+
"min": 0,
1959+
"max": "unlimited"
1960+
}
1961+
}`, keyType)
1962+
1963+
var column ovsdb.ColumnSchema
1964+
err := json.Unmarshal([]byte(colSchema), &column)
1965+
assert.NoError(t, err)
1966+
1967+
oSet, err := ovsdb.NewOvsSet(&column, set)
1968+
assert.NoError(t, err)
1969+
return oSet
1970+
}
1971+
19541972
func BenchmarkPopulate2UpdateArray(b *testing.B) {
19551973
const numRows int = 500
19561974

@@ -1977,7 +1995,8 @@ func BenchmarkPopulate2UpdateArray(b *testing.B) {
19771995
b.ResetTimer()
19781996
for n := 0; n < b.N; n++ {
19791997
for i := 0; i < numRows; i++ {
1980-
updatedRow := ovsdb.Row(map[string]interface{}{"array": ovsdb.OvsSet{GoSet: updateSet}})
1998+
ovsSet := testOvsSet(b, ovsdb.TypeString, updateSet)
1999+
updatedRow := ovsdb.Row(map[string]interface{}{"array": ovsSet})
19812000
err := tc.Populate2(ovsdb.TableUpdates2{
19822001
"Open_vSwitch": {
19832002
"foo": &ovsdb.RowUpdate2{
@@ -1995,7 +2014,7 @@ func BenchmarkTableCacheApplyModificationsSet(b *testing.B) {
19952014
UUID string `ovsdb:"_uuid"`
19962015
Set []string `ovsdb:"set"`
19972016
}
1998-
aFooSet, _ := ovsdb.NewOvsSet([]string{"foo"})
2017+
aFooSet := testOvsSet(b, ovsdb.TypeString, []string{"foo"})
19992018
base := &testDBModel{Set: []string{}}
20002019
for i := 0; i < 57000; i++ {
20012020
base.Set = append(base.Set, fmt.Sprintf("foo%d", i))
@@ -2119,20 +2138,20 @@ func TestTableCacheApplyModifications(t *testing.T) {
21192138
Ptr *string `ovsdb:"ptr"`
21202139
Ptr2 *string `ovsdb:"ptr2"`
21212140
}
2122-
aEmptySet, _ := ovsdb.NewOvsSet([]string{})
2123-
aFooSet, _ := ovsdb.NewOvsSet([]string{"foo"})
2124-
aFooBarSet, _ := ovsdb.NewOvsSet([]string{"foo", "bar"})
2125-
aFooBarBazQuxSet, _ := ovsdb.NewOvsSet([]string{"foo", "bar", "baz", "qux"})
2141+
aEmptySet := testOvsSet(t, ovsdb.TypeString, []string{})
2142+
aFooSet := testOvsSet(t, ovsdb.TypeString, []string{"foo"})
2143+
aFooBarSet := testOvsSet(t, ovsdb.TypeString, []string{"foo", "bar"})
2144+
aFooBarBazQuxSet := testOvsSet(t, ovsdb.TypeString, []string{"foo", "bar", "baz", "qux"})
21262145
aFooMap, _ := ovsdb.NewOvsMap(map[string]string{"foo": "bar"})
21272146
aBarMap, _ := ovsdb.NewOvsMap(map[string]string{"bar": "baz"})
21282147
aBarBazMap, _ := ovsdb.NewOvsMap(map[string]string{
21292148
"bar": "baz",
21302149
"baz": "quux",
21312150
})
21322151
wallace := "wallace"
2133-
aWallaceSet, _ := ovsdb.NewOvsSet([]string{wallace})
2152+
aWallaceSet := testOvsSet(t, ovsdb.TypeString, []string{wallace})
21342153
gromit := "gromit"
2135-
aWallaceGromitSet, _ := ovsdb.NewOvsSet([]string{wallace, gromit})
2154+
aWallaceGromitSet := testOvsSet(t, ovsdb.TypeString, []string{wallace, gromit})
21362155
tests := []struct {
21372156
name string
21382157
update ovsdb.Row

‎client/api_test.go‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -914,7 +914,7 @@ func TestAPIMutate(t *testing.T) {
914914
{
915915
Op: ovsdb.OperationMutate,
916916
Table: "Logical_Switch_Port",
917-
Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []string{"1.1.1.1"})}},
917+
Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"1.1.1.1"})}},
918918
Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID0}}},
919919
},
920920
},
@@ -940,19 +940,19 @@ func TestAPIMutate(t *testing.T) {
940940
{
941941
Op: ovsdb.OperationMutate,
942942
Table: "Logical_Switch_Port",
943-
Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []string{"2.2.2.2"})}},
943+
Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"2.2.2.2"})}},
944944
Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID0}}},
945945
},
946946
{
947947
Op: ovsdb.OperationMutate,
948948
Table: "Logical_Switch_Port",
949-
Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []string{"2.2.2.2"})}},
949+
Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"2.2.2.2"})}},
950950
Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID1}}},
951951
},
952952
{
953953
Op: ovsdb.OperationMutate,
954954
Table: "Logical_Switch_Port",
955-
Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []string{"2.2.2.2"})}},
955+
Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"2.2.2.2"})}},
956956
Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID2}}},
957957
},
958958
},
@@ -976,7 +976,7 @@ func TestAPIMutate(t *testing.T) {
976976
{
977977
Op: ovsdb.OperationMutate,
978978
Table: "Logical_Switch_Port",
979-
Mutations: []ovsdb.Mutation{{Column: "external_ids", Mutator: ovsdb.MutateOperationDelete, Value: testOvsSet(t, []string{"foo"})}},
979+
Mutations: []ovsdb.Mutation{{Column: "external_ids", Mutator: ovsdb.MutateOperationDelete, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"foo"})}},
980980
Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID2}}},
981981
},
982982
},
@@ -1000,7 +1000,7 @@ func TestAPIMutate(t *testing.T) {
10001000
{
10011001
Op: ovsdb.OperationMutate,
10021002
Table: "Logical_Switch_Port",
1003-
Mutations: []ovsdb.Mutation{{Column: "external_ids", Mutator: ovsdb.MutateOperationDelete, Value: testOvsSet(t, []string{"foo"})}},
1003+
Mutations: []ovsdb.Mutation{{Column: "external_ids", Mutator: ovsdb.MutateOperationDelete, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"foo"})}},
10041004
Where: []ovsdb.Condition{{Column: "name", Function: ovsdb.ConditionEqual, Value: "foo"}},
10051005
},
10061006
},
@@ -1136,10 +1136,10 @@ func TestAPIUpdate(t *testing.T) {
11361136
tcache := apiTestCache(t, testData)
11371137

11381138
testObj := testLogicalSwitchPort{}
1139-
testRow := ovsdb.Row(map[string]interface{}{"type": "somethingElse", "tag": testOvsSet(t, []int{6})})
1140-
tagRow := ovsdb.Row(map[string]interface{}{"tag": testOvsSet(t, []int{6})})
1139+
testRow := ovsdb.Row(map[string]interface{}{"type": "somethingElse", "tag": testOvsSet(t, ovsdb.TypeInteger, 1, []int{6})})
1140+
tagRow := ovsdb.Row(map[string]interface{}{"tag": testOvsSet(t, ovsdb.TypeInteger, 1, []int{6})})
11411141
var nilInt *int
1142-
testNilRow := ovsdb.Row(map[string]interface{}{"type": "somethingElse", "tag": testOvsSet(t, nilInt)})
1142+
testNilRow := ovsdb.Row(map[string]interface{}{"type": "somethingElse", "tag": testOvsSet(t, ovsdb.TypeInteger, 1, nilInt)})
11431143
typeRow := ovsdb.Row(map[string]interface{}{"type": "somethingElse"})
11441144
fields := []interface{}{&testObj.Tag, &testObj.Type}
11451145

@@ -1350,7 +1350,7 @@ func TestAPIUpdate(t *testing.T) {
13501350
Row: tagRow,
13511351
Where: []ovsdb.Condition{
13521352
{Column: "type", Function: ovsdb.ConditionEqual, Value: "sometype"},
1353-
{Column: "enabled", Function: ovsdb.ConditionIncludes, Value: testOvsSet(t, &trueVal)},
1353+
{Column: "enabled", Function: ovsdb.ConditionIncludes, Value: testOvsSet(t, ovsdb.TypeBoolean, 1, &trueVal)},
13541354
},
13551355
},
13561356
},
@@ -1403,7 +1403,7 @@ func TestAPIUpdate(t *testing.T) {
14031403
Op: ovsdb.OperationUpdate,
14041404
Table: "Logical_Switch_Port",
14051405
Row: tagRow,
1406-
Where: []ovsdb.Condition{{Column: "tag", Function: ovsdb.ConditionNotEqual, Value: testOvsSet(t, &one)}},
1406+
Where: []ovsdb.Condition{{Column: "tag", Function: ovsdb.ConditionNotEqual, Value: testOvsSet(t, ovsdb.TypeInteger, 1, &one)}},
14071407
},
14081408
},
14091409
err: false,
@@ -1843,6 +1843,7 @@ func TestAPIWait(t *testing.T) {
18431843
tcache := apiTestCache(t, cache.Data{})
18441844
timeout0 := 0
18451845

1846+
trueSet := testOvsSet(t, ovsdb.TypeBoolean, 1, []interface{}{true})
18461847
test := []struct {
18471848
name string
18481849
condition func(API) ConditionalAPI
@@ -1944,7 +1945,7 @@ func TestAPIWait(t *testing.T) {
19441945
{
19451946
Column: "up",
19461947
Function: ovsdb.ConditionNotEqual,
1947-
Value: ovsdb.OvsSet{GoSet: []interface{}{true}},
1948+
Value: trueSet,
19481949
},
19491950
},
19501951
Until: string(ovsdb.WaitConditionNotEqual),

‎client/client_test.go‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ type Bridge struct {
5757
DatapathVersion string `ovsdb:"datapath_version"`
5858
ExternalIDs map[string]string `ovsdb:"external_ids"`
5959
FailMode *BridgeFailMode `ovsdb:"fail_mode"`
60-
FloodVLANs [4096]int `ovsdb:"flood_vlans"`
60+
FloodVLANs []int `ovsdb:"flood_vlans"`
6161
FlowTables map[int]string `ovsdb:"flow_tables"`
6262
IPFIX *string `ovsdb:"ipfix"`
6363
McastSnoopingEnable bool `ovsdb:"mcast_snooping_enable"`
@@ -486,8 +486,24 @@ var schema = `{
486486
}
487487
}`
488488

489-
func testOvsSet(t *testing.T, set interface{}) ovsdb.OvsSet {
490-
oSet, err := ovsdb.NewOvsSet(set)
489+
func testOvsSet(t *testing.T, keyType string, maxSize int, set interface{}) ovsdb.OvsSet {
490+
maxStr := `"unlimited"`
491+
if maxSize > 0 {
492+
maxStr = fmt.Sprintf("%d", maxSize)
493+
}
494+
colSchema := fmt.Sprintf(`{
495+
"type": {
496+
"key": {"type": "%s"},
497+
"min": 0,
498+
"max": %s
499+
}
500+
}`, keyType, maxStr)
501+
502+
var column ovsdb.ColumnSchema
503+
err := json.Unmarshal([]byte(colSchema), &column)
504+
assert.Nil(t, err)
505+
506+
oSet, err := ovsdb.NewOvsSet(&column, set)
491507
assert.Nil(t, err)
492508
return oSet
493509
}

‎client/condition_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ func TestExplicitConditionalWithNoCache(t *testing.T) {
347347
{
348348
Column: "enabled",
349349
Function: ovsdb.ConditionEqual,
350-
Value: testOvsSet(t, &trueVal),
350+
Value: testOvsSet(t, ovsdb.TypeBoolean, 1, &trueVal),
351351
}}},
352352
},
353353
{
@@ -369,7 +369,7 @@ func TestExplicitConditionalWithNoCache(t *testing.T) {
369369
{
370370
Column: "enabled",
371371
Function: ovsdb.ConditionEqual,
372-
Value: testOvsSet(t, &trueVal),
372+
Value: testOvsSet(t, ovsdb.TypeBoolean, 1, &trueVal),
373373
}},
374374
{
375375
{
@@ -396,7 +396,7 @@ func TestExplicitConditionalWithNoCache(t *testing.T) {
396396
{
397397
Column: "enabled",
398398
Function: ovsdb.ConditionEqual,
399-
Value: testOvsSet(t, &trueVal),
399+
Value: testOvsSet(t, ovsdb.TypeBoolean, 1, &trueVal),
400400
},
401401
{
402402
Column: "name",

‎mapper/mapper.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ func (m Mapper) NewMutation(data *Info, column string, mutator ovsdb.Mutator, va
247247
// keys (rfc7047 5.1). Handle this special case here.
248248
if mutator == "delete" && columnSchema.Type == ovsdb.TypeMap && reflect.TypeOf(value).Kind() != reflect.Map {
249249
// It's OK to cast the value to a list of elements because validation has passed
250-
ovsSet, err := ovsdb.NewOvsSet(value)
250+
ovsSet, err := ovsdb.NewOvsSet(columnSchema, value)
251251
if err != nil {
252252
return nil, err
253253
}

0 commit comments

Comments
 (0)