Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use pointers for optional datatypes. Use arrays for fixed size sets #204

Merged
merged 2 commits into from
Aug 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
91 changes: 49 additions & 42 deletions client/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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{
Expand All @@ -648,7 +656,6 @@ func TestAPIMutate(t *testing.T) {
tcache := apiTestCache(t, testData)

testObj := testLogicalSwitchPort{}

test := []struct {
name string
condition func(API) ConditionalAPI
Expand All @@ -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}}},
},
},
Expand Down Expand Up @@ -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")
}
})
Expand All @@ -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{
Expand All @@ -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{
{
Expand All @@ -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{
{
Expand All @@ -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,
Expand All @@ -883,7 +890,7 @@ func TestAPIUpdate(t *testing.T) {
})
},
prepare: func(t *testLogicalSwitchPort) {
t.Tag = []int{6}
t.Tag = &six
},
result: []ovsdb.Operation{
{
Expand All @@ -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{
{
Expand All @@ -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,
Expand All @@ -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{
{
Expand All @@ -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)},
},
},
},
Expand All @@ -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,
Expand All @@ -976,7 +983,7 @@ func TestAPIUpdate(t *testing.T) {
})
},
prepare: func(t *testLogicalSwitchPort) {
t.Tag = []int{6}
t.Tag = &six
},
result: []ovsdb.Operation{
{
Expand All @@ -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{
{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
18 changes: 9 additions & 9 deletions client/api_test_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading