From 8f86fcb4935c7800cfec4ba97329a46fc64ebcb8 Mon Sep 17 00:00:00 2001 From: Dave Tucker Date: Wed, 14 Jul 2021 18:35:08 +0100 Subject: [PATCH] bindings: Update to support new constraints This handles mapping from OvsToNative - min 0, max > 1 && !unlimted to an array - min 0, max 1 to a pointer - min 0, max unlimited to a slice And in NativeToOvs, we convert all cases back in to an OvsSet Signed-off-by: Dave Tucker --- README.md | 10 +++ client/api_test.go | 91 ++++++++++++++------------- client/api_test_model.go | 18 +++--- client/client_test.go | 26 ++++---- client/condition_test.go | 42 ++++++------- mapper/mapper_test.go | 20 +++--- ovsdb/bindings.go | 103 +++++++++++++++++++++++++------ ovsdb/bindings_test.go | 44 +++++++++++-- ovsdb/notation_test.go | 5 +- ovsdb/set.go | 17 +++-- test/ovs/ovs_integration_test.go | 26 +++++--- 11 files changed, 268 insertions(+), 134 deletions(-) diff --git a/README.md b/README.md index fd31b0a3..220f66f6 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,16 @@ The API to interact with OVSDB is based on tagged golang structs. We call it a M Config map[string]string `ovsdb:"other_config"` } +libovsdb is able to translate a Model in to OVSDB format. +To make the API use go idioms, the following mappings occur: + +1. OVSDB Set with min 0 and max unlimited = Slice +1. OVSDB Set with min 0 and max 1 = Pointer to scalar type +1. OVSDB Set with min 0 and max N = Array of N +1. OVSDB Enum = Type-aliased Enum Type +1. OVSDB Map = Map +1. OVSDB Scalar Type = Equivalent scalar Go type + A Open vSwitch Database is modeled using a DBModel which is a created by assigning table names to pointers to these structs: dbModel, _ := model.NewDBModel("OVN_Northbound", map[string]model.Model{ diff --git a/client/api_test.go b/client/api_test.go index bb6724b8..db4c30a7 100644 --- a/client/api_test.go +++ b/client/api_test.go @@ -9,6 +9,14 @@ import ( "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var ( + trueVal = true + falseVal = false + one = 1 + six = 6 ) func TestAPIListSimple(t *testing.T) { @@ -222,25 +230,25 @@ func TestAPIListFields(t *testing.T) { UUID: aUUID0, Name: "lsp0", ExternalIds: map[string]string{"foo": "bar"}, - Enabled: []bool{true}, + Enabled: &trueVal, }, &testLogicalSwitchPort{ UUID: aUUID1, Name: "magiclsp1", ExternalIds: map[string]string{"foo": "baz"}, - Enabled: []bool{false}, + Enabled: &falseVal, }, &testLogicalSwitchPort{ UUID: aUUID2, Name: "lsp2", ExternalIds: map[string]string{"unique": "id"}, - Enabled: []bool{false}, + Enabled: &falseVal, }, &testLogicalSwitchPort{ UUID: aUUID3, Name: "magiclsp2", ExternalIds: map[string]string{"foo": "baz"}, - Enabled: []bool{true}, + Enabled: &trueVal, }, } lspcache := map[string]model.Model{} @@ -624,22 +632,22 @@ func TestAPIMutate(t *testing.T) { Name: "lsp0", Type: "someType", ExternalIds: map[string]string{"foo": "bar"}, - Enabled: []bool{true}, - Tag: []int{1}, + Enabled: &trueVal, + Tag: &one, }, aUUID1: &testLogicalSwitchPort{ UUID: aUUID1, Name: "lsp1", Type: "someType", ExternalIds: map[string]string{"foo": "baz"}, - Tag: []int{1}, + Tag: &one, }, aUUID2: &testLogicalSwitchPort{ UUID: aUUID2, Name: "lsp2", Type: "someOtherType", ExternalIds: map[string]string{"foo": "baz"}, - Tag: []int{1}, + Tag: &one, }, } testData := cache.Data{ @@ -648,7 +656,6 @@ func TestAPIMutate(t *testing.T) { tcache := apiTestCache(t, testData) testObj := testLogicalSwitchPort{} - test := []struct { name string condition func(API) ConditionalAPI @@ -667,16 +674,16 @@ func TestAPIMutate(t *testing.T) { }, mutations: []model.Mutation{ { - Field: &testObj.Tag, + Field: &testObj.Addresses, Mutator: ovsdb.MutateOperationInsert, - Value: []int{5}, + Value: []string{"1.1.1.1"}, }, }, result: []ovsdb.Operation{ { Op: ovsdb.OperationMutate, Table: "Logical_Switch_Port", - Mutations: []ovsdb.Mutation{{Column: "tag", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []int{5})}}, + Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []string{"1.1.1.1"})}}, Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID0}}}, }, }, @@ -777,9 +784,9 @@ func TestAPIMutate(t *testing.T) { cond := tt.condition(api) ops, err := cond.Mutate(&testObj, tt.mutations...) if tt.err { - assert.NotNil(t, err) + require.Error(t, err) } else { - assert.Nil(t, err) + require.Nil(t, err) assert.ElementsMatchf(t, tt.result, ops, "ovsdb.Operations should match") } }) @@ -793,23 +800,23 @@ func TestAPIUpdate(t *testing.T) { Name: "lsp0", Type: "someType", ExternalIds: map[string]string{"foo": "bar"}, - Enabled: []bool{true}, - Tag: []int{1}, + Enabled: &trueVal, + Tag: &one, }, aUUID1: &testLogicalSwitchPort{ UUID: aUUID1, Name: "lsp1", Type: "someType", ExternalIds: map[string]string{"foo": "baz"}, - Tag: []int{1}, - Enabled: []bool{true}, + Tag: &one, + Enabled: &trueVal, }, aUUID2: &testLogicalSwitchPort{ UUID: aUUID2, Name: "lsp2", Type: "someOtherType", ExternalIds: map[string]string{"foo": "baz"}, - Tag: []int{1}, + Tag: &one, }, } testData := cache.Data{ @@ -836,7 +843,7 @@ func TestAPIUpdate(t *testing.T) { }, prepare: func(t *testLogicalSwitchPort) { t.Type = "somethingElse" - t.Tag = []int{6} + t.Tag = &six }, result: []ovsdb.Operation{ { @@ -857,7 +864,7 @@ func TestAPIUpdate(t *testing.T) { }, prepare: func(t *testLogicalSwitchPort) { t.Type = "somethingElse" - t.Tag = []int{6} + t.Tag = &six }, result: []ovsdb.Operation{ { @@ -874,7 +881,7 @@ func TestAPIUpdate(t *testing.T) { condition: func(a API) ConditionalAPI { t := testLogicalSwitchPort{ Type: "sometype", - Enabled: []bool{true}, + Enabled: &trueVal, } return a.Where(&t, model.Condition{ Field: &t.Type, @@ -883,7 +890,7 @@ func TestAPIUpdate(t *testing.T) { }) }, prepare: func(t *testLogicalSwitchPort) { - t.Tag = []int{6} + t.Tag = &six }, result: []ovsdb.Operation{ { @@ -908,11 +915,11 @@ func TestAPIUpdate(t *testing.T) { model.Condition{ Field: &t.Enabled, Function: ovsdb.ConditionIncludes, - Value: []bool{true}, + Value: &trueVal, }) }, prepare: func(t *testLogicalSwitchPort) { - t.Tag = []int{6} + t.Tag = &six }, result: []ovsdb.Operation{ { @@ -925,7 +932,7 @@ func TestAPIUpdate(t *testing.T) { Op: ovsdb.OperationUpdate, Table: "Logical_Switch_Port", Row: tagRow, - Where: []ovsdb.Condition{{Column: "enabled", Function: ovsdb.ConditionIncludes, Value: testOvsSet(t, []bool{true})}}, + Where: []ovsdb.Condition{{Column: "enabled", Function: ovsdb.ConditionIncludes, Value: testOvsSet(t, &trueVal)}}, }, }, err: false, @@ -943,11 +950,11 @@ func TestAPIUpdate(t *testing.T) { model.Condition{ Field: &t.Enabled, Function: ovsdb.ConditionIncludes, - Value: []bool{true}, + Value: &trueVal, }) }, prepare: func(t *testLogicalSwitchPort) { - t.Tag = []int{6} + t.Tag = &six }, result: []ovsdb.Operation{ { @@ -956,7 +963,7 @@ func TestAPIUpdate(t *testing.T) { Row: tagRow, Where: []ovsdb.Condition{ {Column: "type", Function: ovsdb.ConditionEqual, Value: "sometype"}, - {Column: "enabled", Function: ovsdb.ConditionIncludes, Value: testOvsSet(t, []bool{true})}, + {Column: "enabled", Function: ovsdb.ConditionIncludes, Value: testOvsSet(t, &trueVal)}, }, }, }, @@ -967,7 +974,7 @@ func TestAPIUpdate(t *testing.T) { condition: func(a API) ConditionalAPI { t := testLogicalSwitchPort{ Type: "sometype", - Enabled: []bool{true}, + Enabled: &trueVal, } return a.Where(&t, model.Condition{ Field: &t.Type, @@ -976,7 +983,7 @@ func TestAPIUpdate(t *testing.T) { }) }, prepare: func(t *testLogicalSwitchPort) { - t.Tag = []int{6} + t.Tag = &six }, result: []ovsdb.Operation{ { @@ -992,12 +999,12 @@ func TestAPIUpdate(t *testing.T) { name: "select multiple by predicate change multiple field", condition: func(a API) ConditionalAPI { return a.WhereCache(func(t *testLogicalSwitchPort) bool { - return t.Enabled != nil && t.Enabled[0] == true + return t.Enabled != nil && *t.Enabled == true }) }, prepare: func(t *testLogicalSwitchPort) { t.Type = "somethingElse" - t.Tag = []int{6} + t.Tag = &six }, result: []ovsdb.Operation{ { @@ -1041,23 +1048,23 @@ func TestAPIDelete(t *testing.T) { Name: "lsp0", Type: "someType", ExternalIds: map[string]string{"foo": "bar"}, - Enabled: []bool{true}, - Tag: []int{1}, + Enabled: &trueVal, + Tag: &one, }, aUUID1: &testLogicalSwitchPort{ UUID: aUUID1, Name: "lsp1", Type: "someType", ExternalIds: map[string]string{"foo": "baz"}, - Tag: []int{1}, - Enabled: []bool{true}, + Tag: &one, + Enabled: &trueVal, }, aUUID2: &testLogicalSwitchPort{ UUID: aUUID2, Name: "lsp2", Type: "someOtherType", ExternalIds: map[string]string{"foo": "baz"}, - Tag: []int{1}, + Tag: &one, }, } testData := cache.Data{ @@ -1107,7 +1114,7 @@ func TestAPIDelete(t *testing.T) { name: "select by field equality", condition: func(a API) ConditionalAPI { t := testLogicalSwitchPort{ - Enabled: []bool{true}, + Enabled: &trueVal, } return a.Where(&t, model.Condition{ Field: &t.Type, @@ -1128,7 +1135,7 @@ func TestAPIDelete(t *testing.T) { name: "select any by field ", condition: func(a API) ConditionalAPI { t := testLogicalSwitchPort{ - Enabled: []bool{true}, + Enabled: &trueVal, } return a.Where(&t, model.Condition{ @@ -1159,7 +1166,7 @@ func TestAPIDelete(t *testing.T) { name: "select all by field ", condition: func(a API) ConditionalAPI { t := testLogicalSwitchPort{ - Enabled: []bool{true}, + Enabled: &trueVal, } return a.WhereAll(&t, model.Condition{ @@ -1188,7 +1195,7 @@ func TestAPIDelete(t *testing.T) { name: "select multiple by predicate", condition: func(a API) ConditionalAPI { return a.WhereCache(func(t *testLogicalSwitchPort) bool { - return t.Enabled != nil && t.Enabled[0] == true + return t.Enabled != nil && *t.Enabled == true }) }, result: []ovsdb.Operation{ diff --git a/client/api_test_model.go b/client/api_test_model.go index a28a6035..b90541ac 100644 --- a/client/api_test_model.go +++ b/client/api_test_model.go @@ -131,21 +131,21 @@ func (*testLogicalSwitch) Table() string { //LogicalSwitchPort struct defines an object in Logical_Switch_Port table type testLogicalSwitchPort struct { UUID string `ovsdb:"_uuid"` - Up []bool `ovsdb:"up"` - Dhcpv4Options []string `ovsdb:"dhcpv4_options"` + Up *bool `ovsdb:"up"` + Dhcpv4Options *string `ovsdb:"dhcpv4_options"` Name string `ovsdb:"name"` - DynamicAddresses []string `ovsdb:"dynamic_addresses"` - HaChassisGroup []string `ovsdb:"ha_chassis_group"` + DynamicAddresses *string `ovsdb:"dynamic_addresses"` + HaChassisGroup *string `ovsdb:"ha_chassis_group"` Options map[string]string `ovsdb:"options"` - Enabled []bool `ovsdb:"enabled"` + Enabled *bool `ovsdb:"enabled"` Addresses []string `ovsdb:"addresses"` - Dhcpv6Options []string `ovsdb:"dhcpv6_options"` - TagRequest []int `ovsdb:"tag_request"` - Tag []int `ovsdb:"tag"` + Dhcpv6Options *string `ovsdb:"dhcpv6_options"` + TagRequest *int `ovsdb:"tag_request"` + Tag *int `ovsdb:"tag"` PortSecurity []string `ovsdb:"port_security"` ExternalIds map[string]string `ovsdb:"external_ids"` Type string `ovsdb:"type"` - ParentName []string `ovsdb:"parent_name"` + ParentName *string `ovsdb:"parent_name"` } // Table returns the table name. It's part of the Model interface diff --git a/client/client_test.go b/client/client_test.go index fde431aa..f5624169 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -41,26 +41,26 @@ const ( // Bridge defines an object in Bridge table type Bridge struct { UUID string `ovsdb:"_uuid"` - AutoAttach []string `ovsdb:"auto_attach"` + AutoAttach *string `ovsdb:"auto_attach"` Controller []string `ovsdb:"controller"` - DatapathID []string `ovsdb:"datapath_id"` + DatapathID *string `ovsdb:"datapath_id"` DatapathType string `ovsdb:"datapath_type"` DatapathVersion string `ovsdb:"datapath_version"` ExternalIDs map[string]string `ovsdb:"external_ids"` - FailMode []BridgeFailMode `ovsdb:"fail_mode"` - FloodVLANs []int `ovsdb:"flood_vlans"` + FailMode *BridgeFailMode `ovsdb:"fail_mode"` + FloodVLANs [4096]int `ovsdb:"flood_vlans"` FlowTables map[int]string `ovsdb:"flow_tables"` - IPFIX []string `ovsdb:"ipfix"` + IPFIX *string `ovsdb:"ipfix"` McastSnoopingEnable bool `ovsdb:"mcast_snooping_enable"` Mirrors []string `ovsdb:"mirrors"` Name string `ovsdb:"name"` - Netflow []string `ovsdb:"netflow"` + Netflow *string `ovsdb:"netflow"` OtherConfig map[string]string `ovsdb:"other_config"` Ports []string `ovsdb:"ports"` Protocols []BridgeProtocols `ovsdb:"protocols"` RSTPEnable bool `ovsdb:"rstp_enable"` RSTPStatus map[string]string `ovsdb:"rstp_status"` - Sflow []string `ovsdb:"sflow"` + Sflow *string `ovsdb:"sflow"` Status map[string]string `ovsdb:"status"` STPEnable bool `ovsdb:"stp_enable"` } @@ -72,19 +72,19 @@ type OpenvSwitch struct { CurCfg int `ovsdb:"cur_cfg"` DatapathTypes []string `ovsdb:"datapath_types"` Datapaths map[string]string `ovsdb:"datapaths"` - DbVersion []string `ovsdb:"db_version"` + DbVersion *string `ovsdb:"db_version"` DpdkInitialized bool `ovsdb:"dpdk_initialized"` - DpdkVersion []string `ovsdb:"dpdk_version"` + DpdkVersion *string `ovsdb:"dpdk_version"` ExternalIDs map[string]string `ovsdb:"external_ids"` IfaceTypes []string `ovsdb:"iface_types"` ManagerOptions []string `ovsdb:"manager_options"` NextCfg int `ovsdb:"next_cfg"` OtherConfig map[string]string `ovsdb:"other_config"` - OVSVersion []string `ovsdb:"ovs_version"` - SSL []string `ovsdb:"ssl"` + OVSVersion *string `ovsdb:"ovs_version"` + SSL *string `ovsdb:"ssl"` Statistics map[string]string `ovsdb:"statistics"` - SystemType []string `ovsdb:"system_type"` - SystemVersion []string `ovsdb:"system_version"` + SystemType *string `ovsdb:"system_type"` + SystemVersion *string `ovsdb:"system_version"` } var defDB, _ = model.NewDBModel("Open_vSwitch", diff --git a/client/condition_test.go b/client/condition_test.go index 7c44efb2..7ce98730 100644 --- a/client/condition_test.go +++ b/client/condition_test.go @@ -16,25 +16,25 @@ func TestEqualityConditional(t *testing.T) { UUID: aUUID0, Name: "lsp0", ExternalIds: map[string]string{"foo": "bar"}, - Enabled: []bool{true}, + Enabled: &trueVal, }, &testLogicalSwitchPort{ UUID: aUUID1, Name: "lsp1", ExternalIds: map[string]string{"foo": "baz"}, - Enabled: []bool{false}, + Enabled: &falseVal, }, &testLogicalSwitchPort{ UUID: aUUID2, Name: "lsp2", ExternalIds: map[string]string{"unique": "id"}, - Enabled: []bool{false}, + Enabled: &falseVal, }, &testLogicalSwitchPort{ UUID: aUUID3, Name: "lsp3", ExternalIds: map[string]string{"foo": "baz"}, - Enabled: []bool{true}, + Enabled: &trueVal, }, } lspcache := map[string]model.Model{} @@ -156,25 +156,25 @@ func TestPredicateConditional(t *testing.T) { UUID: aUUID0, Name: "lsp0", ExternalIds: map[string]string{"foo": "bar"}, - Enabled: []bool{true}, + Enabled: &trueVal, }, &testLogicalSwitchPort{ UUID: aUUID1, Name: "lsp1", ExternalIds: map[string]string{"foo": "baz"}, - Enabled: []bool{false}, + Enabled: &falseVal, }, &testLogicalSwitchPort{ UUID: aUUID2, Name: "lsp2", ExternalIds: map[string]string{"unique": "id"}, - Enabled: []bool{false}, + Enabled: &falseVal, }, &testLogicalSwitchPort{ UUID: aUUID3, Name: "lsp3", ExternalIds: map[string]string{"foo": "baz"}, - Enabled: []bool{true}, + Enabled: &trueVal, }, } lspcache := map[string]model.Model{} @@ -214,7 +214,7 @@ func TestPredicateConditional(t *testing.T) { { name: "by random field", predicate: func(lsp *testLogicalSwitchPort) bool { - return lsp.Enabled[0] == false + return lsp.Enabled != nil && *lsp.Enabled == false }, condition: [][]ovsdb.Condition{ { @@ -230,8 +230,8 @@ func TestPredicateConditional(t *testing.T) { Value: ovsdb.UUID{GoUUID: aUUID2}, }}}, matches: map[model.Model]bool{ - &testLogicalSwitchPort{UUID: aUUID1, Enabled: []bool{true}}: false, - &testLogicalSwitchPort{UUID: aUUID1, Enabled: []bool{false}}: true, + &testLogicalSwitchPort{UUID: aUUID1, Enabled: &trueVal}: false, + &testLogicalSwitchPort{UUID: aUUID1, Enabled: &falseVal}: true, }, }, } @@ -265,25 +265,25 @@ func TestExplicitConditional(t *testing.T) { UUID: aUUID0, Name: "lsp0", ExternalIds: map[string]string{"foo": "bar"}, - Enabled: []bool{true}, + Enabled: &trueVal, }, &testLogicalSwitchPort{ UUID: aUUID1, Name: "lsp1", ExternalIds: map[string]string{"foo": "baz"}, - Enabled: []bool{false}, + Enabled: &falseVal, }, &testLogicalSwitchPort{ UUID: aUUID2, Name: "lsp2", ExternalIds: map[string]string{"unique": "id"}, - Enabled: []bool{false}, + Enabled: &falseVal, }, &testLogicalSwitchPort{ UUID: aUUID3, Name: "lsp3", ExternalIds: map[string]string{"foo": "baz"}, - Enabled: []bool{true}, + Enabled: &trueVal, }, } lspcache := map[string]model.Model{} @@ -362,7 +362,7 @@ func TestExplicitConditional(t *testing.T) { { Field: &testObj.Enabled, Function: ovsdb.ConditionEqual, - Value: []bool{true}, + Value: &trueVal, }, }, result: [][]ovsdb.Condition{ @@ -370,7 +370,7 @@ func TestExplicitConditional(t *testing.T) { { Column: "enabled", Function: ovsdb.ConditionEqual, - Value: testOvsSet(t, []bool{true}), + Value: testOvsSet(t, &trueVal), }}}, }, { @@ -379,7 +379,7 @@ func TestExplicitConditional(t *testing.T) { { Field: &testObj.Enabled, Function: ovsdb.ConditionEqual, - Value: []bool{true}, + Value: &trueVal, }, { Field: &testObj.Name, @@ -392,7 +392,7 @@ func TestExplicitConditional(t *testing.T) { { Column: "enabled", Function: ovsdb.ConditionEqual, - Value: testOvsSet(t, []bool{true}), + Value: testOvsSet(t, &trueVal), }}, { { @@ -407,7 +407,7 @@ func TestExplicitConditional(t *testing.T) { { Field: &testObj.Enabled, Function: ovsdb.ConditionEqual, - Value: []bool{true}, + Value: &trueVal, }, { Field: &testObj.Name, @@ -419,7 +419,7 @@ func TestExplicitConditional(t *testing.T) { { Column: "enabled", Function: ovsdb.ConditionEqual, - Value: testOvsSet(t, []bool{true}), + Value: testOvsSet(t, &trueVal), }, { Column: "name", diff --git a/mapper/mapper_test.go b/mapper/mapper_test.go index 6bfafde6..98a67073 100644 --- a/mapper/mapper_test.go +++ b/mapper/mapper_test.go @@ -33,7 +33,7 @@ var ( } aFloat = 42.00 - aFloatSet = []float64{ + aFloatSet = [10]float64{ 3.0, 2.0, 42.0, @@ -77,7 +77,8 @@ var testSchema = []byte(`{ "refType": "weak", "type": "uuid" }, - "min": 0 + "min": 0, + "max": "unlimited" } }, "aUUID": { @@ -187,12 +188,12 @@ func TestMapperGetData(t *testing.T) { type ormTestType struct { AString string `ovsdb:"aString"` ASet []string `ovsdb:"aSet"` - ASingleSet []string `ovsdb:"aSingleSet"` + ASingleSet *string `ovsdb:"aSingleSet"` AUUIDSet []string `ovsdb:"aUUIDSet"` AUUID string `ovsdb:"aUUID"` AIntSet []int `ovsdb:"aIntSet"` AFloat float64 `ovsdb:"aFloat"` - AFloatSet []float64 `ovsdb:"aFloatSet"` + AFloatSet [10]float64 `ovsdb:"aFloatSet"` YetAnotherStringSet []string `ovsdb:"aEmptySet"` AEnum string `ovsdb:"aEnum"` AMap map[string]string `ovsdb:"aMap"` @@ -202,7 +203,7 @@ func TestMapperGetData(t *testing.T) { var expected = ormTestType{ AString: aString, ASet: aSet, - ASingleSet: []string{aString}, + ASingleSet: &aString, AUUIDSet: aUUIDSet, AUUID: aUUID0, AIntSet: aIntSet, @@ -304,7 +305,7 @@ func TestMapperNewRow(t *testing.T) { }, { name: "aFloatSet", objInput: &struct { - MyFloatSet []float64 `ovsdb:"aFloatSet"` + MyFloatSet [10]float64 `ovsdb:"aFloatSet"` }{ MyFloatSet: aFloatSet, }, @@ -880,7 +881,8 @@ func TestMapperMutation(t *testing.T) { "set": { "type": { "key": "string", - "min": 0 + "min": 0, + "max": "unlimited" } }, "map": { @@ -890,11 +892,11 @@ func TestMapperMutation(t *testing.T) { } }, "unmutable": { - "mutable": false, + "mutable": false, "type": { "key": "integer" } - }, + }, "int": { "type": { "key": "integer" diff --git a/ovsdb/bindings.go b/ovsdb/bindings.go index e0a714c0..c9957301 100644 --- a/ovsdb/bindings.go +++ b/ovsdb/bindings.go @@ -20,7 +20,7 @@ type ErrWrongType struct { } func (e *ErrWrongType) Error() string { - return fmt.Sprintf("Wrong Type (%s): expected %s but got %s (%s)", + return fmt.Sprintf("Wrong Type (%s): expected %s but got %+v (%s)", e.from, e.expected, e.got, reflect.TypeOf(e.got)) } @@ -54,7 +54,7 @@ func NativeTypeFromAtomic(basicType string) reflect.Type { //NativeType returns the reflect.Type that can hold the value of a column //OVS Type to Native Type convertions: -// OVS sets -> go slices +// OVS sets -> go slices, arrays or a go native type depending on the key // OVS uuid -> go strings // OVS map -> go map // OVS enum -> go native type depending on the type of the enum key @@ -70,6 +70,18 @@ func NativeType(column *ColumnSchema) reflect.Type { return reflect.MapOf(keyType, valueType) case TypeSet: keyType := NativeTypeFromAtomic(column.TypeObj.Key.Type) + // optional type + if column.TypeObj.Min() == 0 && column.TypeObj.Max() == 1 { + return reflect.PtrTo(keyType) + } + // non-optional type with max 1 + if column.TypeObj.Min() == 1 && column.TypeObj.Max() == 1 { + return keyType + } + // max is > 1, use an array + if column.TypeObj.Max() > 1 { + return reflect.ArrayOf(column.TypeObj.Max(), keyType) + } return reflect.SliceOf(keyType) default: panic(fmt.Errorf("unknown extended type %s", column.Type)) @@ -114,31 +126,77 @@ func OvsToNative(column *ColumnSchema, ovsElem interface{}) (interface{}, error) naType := NativeType(column) // The inner slice is []interface{} // We need to convert it to the real type os slice - var nativeSet reflect.Value + switch naType.Kind() { + case reflect.Ptr: + switch ovsSet := ovsElem.(type) { + case OvsSet: + if len(ovsSet.GoSet) > 1 { + return nil, fmt.Errorf("expected a slice of len =< 1, but got a slice with %d elements", len(ovsSet.GoSet)) + } + if len(ovsSet.GoSet) == 0 { + return reflect.Zero(naType).Interface(), nil + } + native, err := OvsToNativeAtomic(column.TypeObj.Key.Type, ovsSet.GoSet[0]) + if err != nil { + return nil, err + } + pv := reflect.New(naType.Elem()) + pv.Elem().Set(reflect.ValueOf(native)) + return pv.Interface(), nil + default: + native, err := OvsToNativeAtomic(column.TypeObj.Key.Type, ovsElem) + if err != nil { + return nil, err + } + pv := reflect.New(naType.Elem()) + pv.Elem().Set(reflect.ValueOf(native)) + return pv.Interface(), nil + } + case reflect.Array: + array := reflect.New(reflect.ArrayOf(column.TypeObj.Max(), naType.Elem())).Elem() + switch ovsSet := ovsElem.(type) { + case OvsSet: + for i, v := range ovsSet.GoSet { + nv, err := OvsToNativeAtomic(column.TypeObj.Key.Type, v) + if err != nil { + return nil, err + } + array.Index(i).Set(reflect.ValueOf(nv)) + } + default: + nv, err := OvsToNativeAtomic(column.TypeObj.Key.Type, ovsElem) + if err != nil { + return nil, err + } + array.Index(0).Set(reflect.ValueOf(nv)) + } + return array.Interface(), nil + case reflect.Slice: + var nativeSet reflect.Value + switch ovsSet := ovsElem.(type) { + case OvsSet: + nativeSet = reflect.MakeSlice(naType, 0, len(ovsSet.GoSet)) + for _, v := range ovsSet.GoSet { + nv, err := OvsToNativeAtomic(column.TypeObj.Key.Type, v) + if err != nil { + return nil, err + } + nativeSet = reflect.Append(nativeSet, reflect.ValueOf(nv)) + } - // RFC says that for a set of exactly one, an atomic type an be sent - switch ovsSet := ovsElem.(type) { - case OvsSet: - nativeSet = reflect.MakeSlice(naType, 0, len(ovsSet.GoSet)) - for _, v := range ovsSet.GoSet { - nv, err := OvsToNativeAtomic(column.TypeObj.Key.Type, v) + default: + nativeSet = reflect.MakeSlice(naType, 0, 1) + nv, err := OvsToNativeAtomic(column.TypeObj.Key.Type, ovsElem) if err != nil { return nil, err } + nativeSet = reflect.Append(nativeSet, reflect.ValueOf(nv)) } - + return nativeSet.Interface(), nil default: - nativeSet = reflect.MakeSlice(naType, 0, 1) - nv, err := OvsToNativeAtomic(column.TypeObj.Key.Type, ovsElem) - if err != nil { - return nil, err - } - - nativeSet = reflect.Append(nativeSet, reflect.ValueOf(nv)) + return nil, fmt.Errorf("native type was not slice, array or pointer. got %d", naType.Kind()) } - return nativeSet.Interface(), nil - case TypeMap: naType := NativeType(column) ovsMap, ok := ovsElem.(OvsMap) @@ -354,11 +412,16 @@ func isDefaultBaseValue(elem interface{}, etype ExtendedType) bool { if !value.IsValid() { return true } - + if reflect.TypeOf(elem).Kind() == reflect.Ptr { + return reflect.ValueOf(elem).IsZero() + } switch etype { case TypeUUID: return elem.(string) == "00000000-0000-0000-0000-000000000000" || elem.(string) == "" || isNamed(elem.(string)) case TypeMap, TypeSet: + if value.Kind() == reflect.Array { + return value.Len() == 0 + } return value.IsNil() || value.Len() == 0 case TypeString: return elem.(string) == "" diff --git a/ovsdb/bindings_test.go b/ovsdb/bindings_test.go index 50c90f7a..8d135f1f 100644 --- a/ovsdb/bindings_test.go +++ b/ovsdb/bindings_test.go @@ -87,6 +87,8 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) { "key3": {GoUUID: aUUID2}, }) + singleStringSet, _ := NewOvsSet([]string{"foo"}) + tests := []struct { name string schema []byte @@ -164,7 +166,8 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) { "refType": "weak", "type": "uuid" }, - "min": 0 + "min": 0, + "max": "unlimited" } }`), input: uss, @@ -180,7 +183,8 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) { "refType": "weak", "type": "uuid" }, - "min": 0 + "min": 0, + "max": "unlimited" } }`), input: UUID{GoUUID: aUUID0}, @@ -356,6 +360,36 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) { native: aUUIDMap, ovs: um, }, + { + name: "String set with min 0 max 1", + schema: []byte(`{ + "type":{ + "key": { + "type": "string" + }, + "min": 0, + "max": 1 + } + }`), + input: singleStringSet, + native: &aString, + ovs: singleStringSet, + }, + { + name: "A string with min 0 max 1", + schema: []byte(`{ + "type":{ + "key": { + "type": "string" + }, + "min": 0, + "max": 1 + } + }`), + input: aString, + native: &aString, + ovs: singleStringSet, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -364,9 +398,9 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) { require.NoError(t, err) native, err := OvsToNative(&column, tt.input) - require.NoErrorf(t, err, "failed to convert %+v: %s", tt, err) + require.NoError(t, err) - require.Equalf(t, native, tt.native, + require.Equalf(t, tt.native, native, "fail to convert ovs2native. input: %v(%s). expected %v(%s). got %v (%s)", tt.input, reflect.TypeOf(tt.input), tt.native, reflect.TypeOf(tt.native), @@ -376,7 +410,7 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) { ovs, err := NativeToOvs(&column, native) require.NoErrorf(t, err, "failed to convert %s: %s", tt, err) - assert.Equalf(t, ovs, tt.ovs, + assert.Equalf(t, tt.ovs, ovs, "fail to convert native2ovs. native: %v(%s). expected %v(%s). got %v (%s)", native, reflect.TypeOf(native), tt.ovs, reflect.TypeOf(tt.ovs), diff --git a/ovsdb/notation_test.go b/ovsdb/notation_test.go index 9d8aebcf..ff183f4a 100644 --- a/ovsdb/notation_test.go +++ b/ovsdb/notation_test.go @@ -86,11 +86,10 @@ func TestValidateOvsSet(t *testing.T) { t.Error("Expected: ", expected, "Got", string(data)) } // Negative condition test - integer := 5 - _, err = NewOvsSet(&integer) + oSet, err = NewOvsSet(struct{ foo string }{}) if err == nil { t.Error("OvsSet must fail for anything other than Slices and atomic types") - t.Error("Expected: ", expected, "Got", string(data)) + t.Error("Got", oSet) } } diff --git a/ovsdb/set.go b/ovsdb/set.go index 3f42e578..60628a23 100644 --- a/ovsdb/set.go +++ b/ovsdb/set.go @@ -19,7 +19,12 @@ type OvsSet struct { // NewOvsSet creates a new OVSDB style set from a Go interface (object) func NewOvsSet(obj interface{}) (OvsSet, error) { - v := reflect.ValueOf(obj) + var v reflect.Value + if reflect.TypeOf(obj).Kind() == reflect.Ptr { + v = reflect.ValueOf(obj).Elem() + } else { + v = reflect.ValueOf(obj) + } ovsSet := make([]interface{}, 0) switch v.Kind() { case reflect.Slice, reflect.Array: @@ -31,10 +36,14 @@ func NewOvsSet(obj interface{}) (OvsSet, error) { reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Float32, reflect.Float64, reflect.Bool: ovsSet = append(ovsSet, v.Interface()) - case reflect.ValueOf(UUID{}).Kind(): - ovsSet = append(ovsSet, v.Interface()) + case reflect.Struct: + if v.Type() == reflect.TypeOf(UUID{}) { + ovsSet = append(ovsSet, v.Interface()) + } else { + return OvsSet{}, fmt.Errorf("ovsset supports only go slice/string/numbers/uuid or pointers to those types") + } default: - return OvsSet{}, fmt.Errorf("ovsset supports only go slice/string/numbers/uuid types") + return OvsSet{}, fmt.Errorf("ovsset supports only go slice/string/numbers/uuid or pointers to those types") } return OvsSet{ovsSet}, nil } diff --git a/test/ovs/ovs_integration_test.go b/test/ovs/ovs_integration_test.go index 06179ad5..6d5aa2fd 100644 --- a/test/ovs/ovs_integration_test.go +++ b/test/ovs/ovs_integration_test.go @@ -56,8 +56,8 @@ func (suite *OVSIntegrationSuite) SetupSuite() { suite.resource, err = suite.pool.RunWithOptions(options, hostConfig) require.NoError(suite.T(), err) - // set expiry to 30 seconds so containers are cleaned up on test panic - err = suite.resource.Expire(30) + // set expiry to 60 seconds so containers are cleaned up on test panic + err = suite.resource.Expire(60) require.NoError(suite.T(), err) // let the container start before we attempt connection @@ -76,6 +76,7 @@ func (suite *OVSIntegrationSuite) SetupSuite() { } err = ovs.Connect(ctx) if err != nil { + suite.T().Log(err) return err } suite.client = ovs @@ -105,14 +106,22 @@ func TestOVSIntegrationTestSuite(t *testing.T) { suite.Run(t, new(OVSIntegrationSuite)) } +type BridgeFailMode = string + +var ( + BridgeFailModeStandalone BridgeFailMode = "standalone" + BridgeFailModeSecure BridgeFailMode = "secure" +) + // bridgeType is the simplified ORM model of the Bridge table type bridgeType struct { - UUID string `ovsdb:"_uuid"` - Name string `ovsdb:"name"` - OtherConfig map[string]string `ovsdb:"other_config"` - ExternalIds map[string]string `ovsdb:"external_ids"` - Ports []string `ovsdb:"ports"` - Status map[string]string `ovsdb:"status"` + UUID string `ovsdb:"_uuid"` + Name string `ovsdb:"name"` + OtherConfig map[string]string `ovsdb:"other_config"` + ExternalIds map[string]string `ovsdb:"external_ids"` + Ports []string `ovsdb:"ports"` + Status map[string]string `ovsdb:"status"` + BridgeFailMode *BridgeFailMode `ovsdb:"fail_mode"` } // ovsType is the simplified ORM model of the Bridge table @@ -552,6 +561,7 @@ func (suite *OVSIntegrationSuite) createBridge(bridgeName string) (string, error "go": "awesome", "docker": "made-for-each-other", }, + BridgeFailMode: &BridgeFailModeSecure, } insertOp, err := suite.client.Create(&br)