From 89850f8d097519600a17ad64c5a7a21c9a67ebc6 Mon Sep 17 00:00:00 2001 From: Yan Zhu Date: Mon, 18 Sep 2023 17:32:16 +0800 Subject: [PATCH 1/3] Generate Table() func in model.Model interface via modelgen Signed-off-by: Yan Zhu --- modelgen/table.go | 3 +++ modelgen/table_test.go | 28 ++++++++++++++++++++++++++++ ovsdb/serverdb/database.go | 4 ++++ 3 files changed, 35 insertions(+) diff --git a/modelgen/table.go b/modelgen/table.go index 2c0b301e..a7cc1854 100644 --- a/modelgen/table.go +++ b/modelgen/table.go @@ -246,6 +246,9 @@ type {{ index . "StructName" }} struct { {{ end }} {{ template "extraFields" . }} } +func (a *{{ index . "StructName" }}) Table() string { + return {{ index . "StructName" }}Table +} {{ template "postStructDefinitions" . }} {{ template "extraDefinitions" . }} {{ template "extendedGen" . }} diff --git a/modelgen/table_test.go b/modelgen/table_test.go index 6d4e2711..b3e38111 100644 --- a/modelgen/table_test.go +++ b/modelgen/table_test.go @@ -80,6 +80,10 @@ type AtomicTable struct { Protocol *AtomicTableProtocol ` + "`" + `ovsdb:"protocol"` + "`" + ` Str string ` + "`" + `ovsdb:"str"` + "`" + ` } + +func (a *AtomicTable) Table() string { + return AtomicTableTable +} `, }, { @@ -103,6 +107,10 @@ type AtomicTable struct { Protocol *string ` + "`" + `ovsdb:"protocol"` + "`" + ` Str string ` + "`" + `ovsdb:"str"` + "`" + ` } + +func (a *AtomicTable) Table() string { + return AtomicTableTable +} `, }, { @@ -151,6 +159,10 @@ type AtomicTable struct { OtherProtocol *string OtherStr string } + +func (a *AtomicTable) Table() string { + return AtomicTableTable +} `, }, { @@ -189,6 +201,10 @@ type AtomicTable struct { Str string ` + "`" + `ovsdb:"str"` + "`" + ` } +func (a *AtomicTable) Table() string { + return AtomicTableTable +} + func (a *AtomicTable) GetUUID() string { return a.UUID } @@ -354,6 +370,10 @@ type AtomicTable struct { OtherStr string } +func (a *AtomicTable) Table() string { + return AtomicTableTable +} + func copyAtomicTableOtherProtocol(a *AtomicTableProtocol) *AtomicTableProtocol { if a == nil { return nil @@ -483,6 +503,10 @@ type AtomicTable struct { Str string ` + "`" + `ovsdb:"str"` + "`" + ` } +func (a *AtomicTable) Table() string { + return AtomicTableTable +} + func (a *AtomicTable) GetUUID() string { return a.UUID } @@ -606,6 +630,10 @@ type AtomicTable struct { Str string ` + "`" + `ovsdb:"str"` + "`" + ` } +func (a *AtomicTable) Table() string { + return AtomicTableTable +} + func TestFunc() string { return "AtomicTable" } diff --git a/ovsdb/serverdb/database.go b/ovsdb/serverdb/database.go index 274a7164..8d6f4ad7 100644 --- a/ovsdb/serverdb/database.go +++ b/ovsdb/serverdb/database.go @@ -30,6 +30,10 @@ type Database struct { Sid *string `ovsdb:"sid"` } +func (a *Database) Table() string { + return DatabaseTable +} + func (a *Database) GetUUID() string { return a.UUID } From 50f22030ac4f1170939fa9aa5674c3320f1a543f Mon Sep 17 00:00:00 2001 From: Yan Zhu Date: Mon, 18 Sep 2023 18:38:45 +0800 Subject: [PATCH 2/3] refactor WhereCache func to import performance Signed-off-by: Yan Zhu --- cache/cache_test.go | 236 ++++++++++++++++++------------- client/api.go | 64 +++------ client/api_test.go | 98 ++++++------- client/client.go | 4 +- client/client_test.go | 8 ++ client/condition.go | 8 +- client/condition_test.go | 8 +- cmd/stress/stress.go | 8 ++ model/model.go | 21 +-- model/model_test.go | 34 +++-- test/ovs/ovs_integration_test.go | 22 ++- test/test_data.go | 12 ++ 12 files changed, 296 insertions(+), 227 deletions(-) diff --git a/cache/cache_test.go b/cache/cache_test.go index abca11d5..1b56e14d 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -24,6 +24,10 @@ type testModel struct { Datapath *string `ovsdb:"datapath"` } +func (t *testModel) Table() string { + return "Open_vSwitch" +} + const testSchemaFmt string = `{ "name": "Open_vSwitch", "tables": { @@ -369,14 +373,19 @@ func TestRowCacheCreateMultiIndex(t *testing.T) { } } +type testModel1 struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"foo"` + Bar map[string]string `ovsdb:"bar"` +} + +func (t *testModel1) Table() string { + return "Open_vSwitch" +} + func TestRowCacheCreateMultiClientIndex(t *testing.T) { - type testModel struct { - UUID string `ovsdb:"_uuid"` - Foo string `ovsdb:"foo"` - Bar map[string]string `ovsdb:"bar"` - } var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel1{}}) require.Nil(t, err) db.SetIndexes(map[string][]model.ClientIndex{ @@ -419,7 +428,7 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { require.Nil(t, err) testData := Data{ - "Open_vSwitch": map[string]model.Model{"bar": &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}}, + "Open_vSwitch": map[string]model.Model{"bar": &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}}, } dbModel, errs := model.NewDatabaseModel(schema, db) require.Empty(t, errs) @@ -432,22 +441,22 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { tests := []struct { name string uuid string - model *testModel + model *testModel1 wantErr bool expected []expected }{ { name: "inserts a new row", uuid: "foo", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + model: &testModel1{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + index: &testModel1{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("foo"), }, }, @@ -455,17 +464,17 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { { name: "error duplicate uuid", uuid: "bar", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, + model: &testModel1{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, wantErr: true, }, { name: "inserts duplicate index", uuid: "baz", - model: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + model: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "baz"), }, }, @@ -473,15 +482,15 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { { name: "new row with one duplicate value", uuid: "baz", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, + model: &testModel1{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("baz"), }, }, @@ -489,15 +498,15 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { { name: "new row with other duplicate value", uuid: "baz", - model: &testModel{Foo: "bar", Bar: map[string]string{"bar": "foo"}}, + model: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "foo"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "foo"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("baz"), }, }, @@ -505,15 +514,15 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { { name: "new row with nil map index", uuid: "baz", - model: &testModel{Foo: "bar"}, + model: &testModel1{Foo: "bar"}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, { - index: &testModel{Foo: "bar"}, + index: &testModel1{Foo: "bar"}, uuids: newUUIDSet("baz"), }, }, @@ -791,15 +800,20 @@ func TestRowCacheUpdateMultiIndex(t *testing.T) { } } +type testModel2 struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"foo"` + Bar map[string]string `ovsdb:"bar"` + Baz string `ovsdb:"baz"` +} + +func (t *testModel2) Table() string { + return "Open_vSwitch" +} + func TestRowCacheUpdateMultiClientIndex(t *testing.T) { - type testModel struct { - UUID string `ovsdb:"_uuid"` - Foo string `ovsdb:"foo"` - Bar map[string]string `ovsdb:"bar"` - Baz string `ovsdb:"baz"` - } var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel2{}}) require.Nil(t, err) db.SetIndexes(map[string][]model.ClientIndex{ @@ -846,9 +860,9 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { testData := Data{ "Open_vSwitch": map[string]model.Model{ - "foo": &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, - "bar": &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, - "foobar": &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + "foo": &testModel2{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + "bar": &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + "foobar": &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, }, } dbModel, errs := model.NewDatabaseModel(schema, db) @@ -862,27 +876,27 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { tests := []struct { name string uuid string - model *testModel + model *testModel2 wantErr bool expected []expected }{ { name: "error if row does not exist", uuid: "baz", - model: &testModel{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, + model: &testModel2{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, wantErr: true, }, { name: "update non-index", uuid: "foo", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}, Baz: "bar"}, + model: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "foo"}, Baz: "bar"}, expected: []expected{ { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + index: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("foo"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "foobar"), }, }, @@ -890,15 +904,15 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { { name: "update one index column", uuid: "foo", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "baz"}}, + model: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "baz"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "baz"}}, + index: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "baz"}}, uuids: newUUIDSet("foo"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "foobar"), }, }, @@ -906,15 +920,15 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { { name: "update other index column", uuid: "foo", - model: &testModel{Foo: "baz", Bar: map[string]string{"bar": "foo"}}, + model: &testModel2{Foo: "baz", Bar: map[string]string{"bar": "foo"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "baz", Bar: map[string]string{"bar": "foo"}}, + index: &testModel2{Foo: "baz", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("foo"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "foobar"), }, }, @@ -922,15 +936,15 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { { name: "update both index columns", uuid: "foo", - model: &testModel{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, + model: &testModel2{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, + index: &testModel2{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, uuids: newUUIDSet("foo"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "foobar"), }, }, @@ -938,11 +952,11 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { { name: "update unique index to existing index", uuid: "foo", - model: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + model: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("foo", "bar", "foobar"), }, }, @@ -950,15 +964,15 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { { name: "update multi index to different index", uuid: "foobar", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + model: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + index: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("foo", "foobar"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, }, @@ -1038,14 +1052,19 @@ func TestRowCacheDelete(t *testing.T) { } } +type testModel3 struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"foo"` + Bar map[string]string `ovsdb:"bar"` +} + +func (t *testModel3) Table() string { + return "Open_vSwitch" +} + func TestRowCacheDeleteClientIndex(t *testing.T) { - type testModel struct { - UUID string `ovsdb:"_uuid"` - Foo string `ovsdb:"foo"` - Bar map[string]string `ovsdb:"bar"` - } var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel3{}}) require.Nil(t, err) db.SetIndexes(map[string][]model.ClientIndex{ @@ -1089,9 +1108,9 @@ func TestRowCacheDeleteClientIndex(t *testing.T) { testData := Data{ "Open_vSwitch": map[string]model.Model{ - "foo": &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, - "bar": &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, - "foobar": &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + "foo": &testModel3{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + "bar": &testModel3{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + "foobar": &testModel3{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, }, } dbModel, errs := model.NewDatabaseModel(schema, db) @@ -1105,24 +1124,24 @@ func TestRowCacheDeleteClientIndex(t *testing.T) { tests := []struct { name string uuid string - model *testModel + model *testModel3 wantErr bool expected []expected }{ { name: "error if row does not exist", uuid: "baz", - model: &testModel{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, + model: &testModel3{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, wantErr: true, }, { name: "delete a row with unique index", uuid: "foo", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + model: &testModel3{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel3{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "foobar"), }, }, @@ -1130,15 +1149,15 @@ func TestRowCacheDeleteClientIndex(t *testing.T) { { name: "delete a row with duplicated index", uuid: "foobar", - model: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + model: &testModel3{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + index: &testModel3{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("foo"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel3{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, }, @@ -1898,6 +1917,15 @@ func setupRowByModelSingleIndex(t require.TestingT) (*testModel, *TableCache) { return myFoo, tc } +type badModel struct { + UUID string `ovsdb:"_uuid"` + Baz string `ovsdb:"baz"` +} + +func (b *badModel) Table() string { + return "bad" +} + func TestTableCacheRowByModelSingleIndex(t *testing.T) { myFoo, tc := setupRowByModelSingleIndex(t) @@ -1921,10 +1949,6 @@ func TestTableCacheRowByModelSingleIndex(t *testing.T) { }) t.Run("wrong model type", func(t *testing.T) { - type badModel struct { - UUID string `ovsdb:"_uuid"` - Baz string `ovsdb:"baz"` - } _, _, err := tc.Table("Open_vSwitch").RowByModel(&badModel{Baz: "baz"}) assert.Error(t, err) }) @@ -2125,15 +2149,20 @@ func TestTableCacheRowByModelMultiIndex(t *testing.T) { }) } +type testModel4 struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"foo"` + Bar string `ovsdb:"bar"` + Baz map[string]string `ovsdb:"baz"` +} + +func (t *testModel4) Table() string { + return "Open_vSwitch" +} + func TestTableCacheRowsByModels(t *testing.T) { - type testModel struct { - UUID string `ovsdb:"_uuid"` - Foo string `ovsdb:"foo"` - Bar string `ovsdb:"bar"` - Baz map[string]string `ovsdb:"baz"` - } var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel4{}}) require.NoError(t, err) db.SetIndexes(map[string][]model.ClientIndex{ "Open_vSwitch": { @@ -2185,12 +2214,12 @@ func TestTableCacheRowsByModels(t *testing.T) { testData := Data{ "Open_vSwitch": map[string]model.Model{ - "foo": &testModel{Foo: "foo", Bar: "foo", Baz: map[string]string{"baz": "foo", "other": "other"}}, - "bar": &testModel{Foo: "bar", Bar: "bar", Baz: map[string]string{"baz": "bar", "other": "other"}}, - "foobar": &testModel{Foo: "foobar", Bar: "bar", Baz: map[string]string{"baz": "foobar", "other": "other"}}, - "baz": &testModel{Foo: "baz", Bar: "baz", Baz: map[string]string{"baz": "baz", "other": "other"}}, - "quux": &testModel{Foo: "quux", Bar: "quux", Baz: map[string]string{"baz": "quux", "other": "other"}}, - "quuz": &testModel{Foo: "quuz", Bar: "quux", Baz: map[string]string{"baz": "quux", "other": "other"}}, + "foo": &testModel4{Foo: "foo", Bar: "foo", Baz: map[string]string{"baz": "foo", "other": "other"}}, + "bar": &testModel4{Foo: "bar", Bar: "bar", Baz: map[string]string{"baz": "bar", "other": "other"}}, + "foobar": &testModel4{Foo: "foobar", Bar: "bar", Baz: map[string]string{"baz": "foobar", "other": "other"}}, + "baz": &testModel4{Foo: "baz", Bar: "baz", Baz: map[string]string{"baz": "baz", "other": "other"}}, + "quux": &testModel4{Foo: "quux", Bar: "quux", Baz: map[string]string{"baz": "quux", "other": "other"}}, + "quuz": &testModel4{Foo: "quuz", Bar: "quux", Baz: map[string]string{"baz": "quux", "other": "other"}}, }, } dbModel, errs := model.NewDatabaseModel(schema, db) @@ -2204,14 +2233,14 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by non index, no result", models: []model.Model{ - &testModel{Foo: "no", Bar: "no", Baz: map[string]string{"baz": "no"}}, + &testModel4{Foo: "no", Bar: "no", Baz: map[string]string{"baz": "no"}}, }, rows: nil, }, { name: "by single column client index, single result", models: []model.Model{ - &testModel{Bar: "foo"}, + &testModel4{Bar: "foo"}, }, rows: map[string]model.Model{ "foo": testData["Open_vSwitch"]["foo"], @@ -2220,8 +2249,8 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by single column client index, multiple models, multiple results", models: []model.Model{ - &testModel{Bar: "foo"}, - &testModel{Bar: "baz"}, + &testModel4{Bar: "foo"}, + &testModel4{Bar: "baz"}, }, rows: map[string]model.Model{ "foo": testData["Open_vSwitch"]["foo"], @@ -2231,7 +2260,7 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by single column client index, multiple results", models: []model.Model{ - &testModel{Bar: "bar"}, + &testModel4{Bar: "bar"}, }, rows: map[string]model.Model{ "bar": testData["Open_vSwitch"]["bar"], @@ -2241,7 +2270,7 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by multi column client index, single result", models: []model.Model{ - &testModel{Bar: "baz", Baz: map[string]string{"baz": "baz"}}, + &testModel4{Bar: "baz", Baz: map[string]string{"baz": "baz"}}, }, rows: map[string]model.Model{ "baz": testData["Open_vSwitch"]["baz"], @@ -2250,7 +2279,7 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by client index, multiple results", models: []model.Model{ - &testModel{Bar: "quux", Baz: map[string]string{"baz": "quux"}}, + &testModel4{Bar: "quux", Baz: map[string]string{"baz": "quux"}}, }, rows: map[string]model.Model{ "quux": testData["Open_vSwitch"]["quux"], @@ -2260,8 +2289,8 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by client index, multiple models, multiple results", models: []model.Model{ - &testModel{Bar: "quux", Baz: map[string]string{"baz": "quux"}}, - &testModel{Bar: "bar", Baz: map[string]string{"baz": "foobar"}}, + &testModel4{Bar: "quux", Baz: map[string]string{"baz": "quux"}}, + &testModel4{Bar: "bar", Baz: map[string]string{"baz": "foobar"}}, }, rows: map[string]model.Model{ "quux": testData["Open_vSwitch"]["quux"], @@ -2273,7 +2302,7 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by schema index prioritized over client index", models: []model.Model{ - &testModel{Foo: "foo", Bar: "bar", Baz: map[string]string{"baz": "bar"}}, + &testModel4{Foo: "foo", Bar: "bar", Baz: map[string]string{"baz": "bar"}}, }, rows: map[string]model.Model{ "foo": testData["Open_vSwitch"]["foo"], @@ -2302,6 +2331,10 @@ type rowsByConditionTestModel struct { Empty string `ovsdb:"empty"` } +func (r *rowsByConditionTestModel) Table() string { + return "Open_vSwitch" +} + func setupRowsByConditionCache(t require.TestingT) *TableCache { var schema ovsdb.DatabaseSchema db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &rowsByConditionTestModel{}}) @@ -2709,11 +2742,16 @@ func BenchmarkRowsByCondition(b *testing.B) { } } +type testDBModel struct { + UUID string `ovsdb:"_uuid"` + Set []string `ovsdb:"set"` +} + +func (t *testDBModel) Table() string { + return "Open_vSwitch" +} + func BenchmarkPopulate2SingleModify(b *testing.B) { - type testDBModel struct { - UUID string `ovsdb:"_uuid"` - Set []string `ovsdb:"set"` - } aFooSet, _ := ovsdb.NewOvsSet([]string{"foo"}) base := &testDBModel{Set: []string{}} for i := 0; i < 57000; i++ { diff --git a/client/api.go b/client/api.go index 8d5a30ef..1a427c9b 100644 --- a/client/api.go +++ b/client/api.go @@ -21,10 +21,10 @@ type API interface { // If it has a capacity != 0, only 'capacity' elements will be filled in List(ctx context.Context, result interface{}) error - // Create a Conditional API from a Function that is used to filter cached data + // WhereCache creates a Conditional API from a Function that is used to filter cached data // The function must accept a Model implementation and return a boolean. E.g: - // ConditionFromFunc(func(l *LogicalSwitch) bool { return l.Enabled }) - WhereCache(predicate interface{}) ConditionalAPI + // WhereCache(&LogicalSwitchPort{}, func(m model.Model) bool { l := m.(*LogicalSwitchPort); return l.Enabled }) + WhereCache(model.Model, func(model.Model) bool) ConditionalAPI // Create a ConditionalAPI from a Model's index data, where operations // apply to elements that match the values provided in one or more @@ -120,13 +120,20 @@ func (a api) List(ctx context.Context, result interface{}) error { // structs var appendValue func(reflect.Value) var m model.Model + var ok bool if resultVal.Type().Elem().Kind() == reflect.Ptr { - m = reflect.New(resultVal.Type().Elem().Elem()).Interface() + m, ok = reflect.New(resultVal.Type().Elem().Elem()).Interface().(model.Model) + if !ok { + return &ErrWrongType{resultPtr.Type(), "Expected pointer to slice of valid Models"} + } appendValue = func(v reflect.Value) { resultVal.Set(reflect.Append(resultVal, v)) } } else { - m = reflect.New(resultVal.Type().Elem()).Interface() + m, ok = reflect.New(resultVal.Type().Elem()).Interface().(model.Model) + if !ok { + return &ErrWrongType{resultPtr.Type(), "Expected pointer to slice of valid Models"} + } appendValue = func(v reflect.Value) { resultVal.Set(reflect.Append(resultVal, reflect.Indirect(v))) } @@ -194,14 +201,17 @@ func (a api) WhereAll(m model.Model, cond ...model.Condition) ConditionalAPI { } // WhereCache returns a conditionalAPI based a Predicate -func (a api) WhereCache(predicate interface{}) ConditionalAPI { - return newConditionalAPI(a.cache, a.conditionFromFunc(predicate), a.logger) +func (a api) WhereCache(m model.Model, predicate func(model.Model) bool) ConditionalAPI { + return newConditionalAPI(a.cache, a.conditionFromFunc(m, predicate), a.logger) } // Conditional interface implementation // FromFunc returns a Condition from a function -func (a api) conditionFromFunc(predicate interface{}) Conditional { - table, err := a.getTableFromFunc(predicate) +func (a api) conditionFromFunc(m model.Model, predicate func(model.Model) bool) Conditional { + if predicate == nil { + return newErrorConditional(fmt.Errorf("expect predicate as a function, got nil")) + } + table, err := a.getTableFromModel(m) if err != nil { return newErrorConditional(err) } @@ -538,43 +548,15 @@ func (a api) Wait(untilConFun ovsdb.WaitCondition, timeout *int, model model.Mod // getTableFromModel returns the table name from a Model object after performing // type verifications on the model -func (a api) getTableFromModel(m interface{}) (string, error) { - if _, ok := m.(model.Model); !ok { - return "", &ErrWrongType{reflect.TypeOf(m), "Type does not implement Model interface"} - } - table := a.cache.DatabaseModel().FindTable(reflect.TypeOf(m)) - if table == "" { +func (a api) getTableFromModel(m model.Model) (string, error) { + table := m.Table() + _, found := a.cache.DatabaseModel().Types()[table] + if table == "" || !found { return "", &ErrWrongType{reflect.TypeOf(m), "Model not found in Database Model"} } return table, nil } -// getTableFromModel returns the table name from a the predicate after performing -// type verifications -func (a api) getTableFromFunc(predicate interface{}) (string, error) { - predType := reflect.TypeOf(predicate) - if predType == nil || predType.Kind() != reflect.Func { - return "", &ErrWrongType{predType, "Expected function"} - } - if predType.NumIn() != 1 || predType.NumOut() != 1 || predType.Out(0).Kind() != reflect.Bool { - return "", &ErrWrongType{predType, "Expected func(Model) bool"} - } - - modelInterface := reflect.TypeOf((*model.Model)(nil)).Elem() - modelType := predType.In(0) - if !modelType.Implements(modelInterface) { - return "", &ErrWrongType{predType, - fmt.Sprintf("Type %s does not implement Model interface", modelType.String())} - } - - table := a.cache.DatabaseModel().FindTable(modelType) - if table == "" { - return "", &ErrWrongType{predType, - fmt.Sprintf("Model %s not found in Database Model", modelType.String())} - } - return table, nil -} - // newAPI returns a new API to interact with the database func newAPI(cache *cache.TableCache, logger *logr.Logger) API { return api{ diff --git a/client/api_test.go b/client/api_test.go index 7c768109..fea0d14a 100644 --- a/client/api_test.go +++ b/client/api_test.go @@ -217,13 +217,15 @@ func TestAPIListPredicate(t *testing.T) { test := []struct { name string - predicate interface{} + m model.Model + predicate func(model.Model) bool content []model.Model err bool }{ { name: "none", - predicate: func(t *testLogicalSwitch) bool { + m: &testLogicalSwitch{}, + predicate: func(t model.Model) bool { return false }, content: []model.Model{}, @@ -231,7 +233,8 @@ func TestAPIListPredicate(t *testing.T) { }, { name: "all", - predicate: func(t *testLogicalSwitch) bool { + m: &testLogicalSwitch{}, + predicate: func(t model.Model) bool { return true }, content: lscacheList, @@ -239,30 +242,26 @@ func TestAPIListPredicate(t *testing.T) { }, { name: "nil function must fail", + m: &testLogicalSwitch{}, err: true, }, { name: "arbitrary condition", - predicate: func(t *testLogicalSwitch) bool { + m: &testLogicalSwitch{}, + predicate: func(m model.Model) bool { + t := m.(*testLogicalSwitch) return strings.HasPrefix(t.Name, "magic") }, content: []model.Model{lscacheList[1], lscacheList[3]}, err: false, }, - { - name: "error wrong type", - predicate: func(t testLogicalSwitch) string { - return "foo" - }, - err: true, - }, } for _, tt := range test { t.Run(fmt.Sprintf("ApiListPredicate: %s", tt.name), func(t *testing.T) { var result []*testLogicalSwitch api := newAPI(tcache, &discardLogger) - cond := api.WhereCache(tt.predicate) + cond := api.WhereCache(tt.m, tt.predicate) err := cond.List(context.Background(), &result) if tt.err { assert.NotNil(t, err) @@ -536,26 +535,14 @@ func TestAPIListMulti(t *testing.T) { func TestConditionFromFunc(t *testing.T) { test := []struct { name string - arg interface{} + m model.Model + arg func(model.Model) bool err bool }{ - { - name: "wrong function must fail", - arg: func(s string) bool { - return false - }, - err: true, - }, - { - name: "wrong function must fail2 ", - arg: func(t *testLogicalSwitch) string { - return "foo" - }, - err: true, - }, { name: "correct func should succeed", - arg: func(t *testLogicalSwitch) bool { + m: &testLogicalSwitch{}, + arg: func(m model.Model) bool { return true }, err: false, @@ -566,7 +553,7 @@ func TestConditionFromFunc(t *testing.T) { t.Run(fmt.Sprintf("conditionFromFunc: %s", tt.name), func(t *testing.T) { cache := apiTestCache(t, nil) apiIface := newAPI(cache, &discardLogger) - condition := apiIface.(api).conditionFromFunc(tt.arg) + condition := apiIface.(api).conditionFromFunc(tt.m, tt.arg) if tt.err { assert.IsType(t, &errorConditional{}, condition) } else { @@ -584,23 +571,6 @@ func TestConditionFromModel(t *testing.T) { conds []model.Condition err bool }{ - { - name: "wrong model must fail", - models: []model.Model{ - &struct{ a string }{}, - }, - err: true, - }, - { - name: "wrong condition must fail", - models: []model.Model{ - &struct { - a string `ovsdb:"_uuid"` - }{}, - }, - conds: []model.Condition{{Field: "foo"}}, - err: true, - }, { name: "correct model must succeed", models: []model.Model{ @@ -1009,7 +979,8 @@ func TestAPIMutate(t *testing.T) { { name: "select single by predicate name insert element in map", condition: func(a API) ConditionalAPI { - return a.WhereCache(func(lsp *testLogicalSwitchPort) bool { + return a.WhereCache(&testLogicalSwitchPort{}, func(m model.Model) bool { + lsp := m.(*testLogicalSwitchPort) return lsp.Name == "lsp2" }) }, @@ -1033,7 +1004,8 @@ func TestAPIMutate(t *testing.T) { { name: "select many by predicate name insert element in map", condition: func(a API) ConditionalAPI { - return a.WhereCache(func(lsp *testLogicalSwitchPort) bool { + return a.WhereCache(&testLogicalSwitchPort{}, func(m model.Model) bool { + lsp := m.(*testLogicalSwitchPort) return lsp.Type == "someType" }) }, @@ -1063,7 +1035,8 @@ func TestAPIMutate(t *testing.T) { { name: "No mutations should error", condition: func(a API) ConditionalAPI { - return a.WhereCache(func(lsp *testLogicalSwitchPort) bool { + return a.WhereCache(&testLogicalSwitchPort{}, func(m model.Model) bool { + lsp := m.(*testLogicalSwitchPort) return lsp.Type == "someType" }) }, @@ -1411,7 +1384,8 @@ 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 a.WhereCache(&testLogicalSwitchPort{}, func(m model.Model) bool { + t := m.(*testLogicalSwitchPort) return t.Enabled != nil && *t.Enabled == true }) }, @@ -1645,7 +1619,8 @@ func TestAPIDelete(t *testing.T) { { name: "select multiple by predicate", condition: func(a API) ConditionalAPI { - return a.WhereCache(func(t *testLogicalSwitchPort) bool { + return a.WhereCache(&testLogicalSwitchPort{}, func(m model.Model) bool { + t := m.(*testLogicalSwitchPort) return t.Enabled != nil && *t.Enabled == true }) }, @@ -1724,29 +1699,36 @@ func BenchmarkAPIList(b *testing.B) { test := []struct { name string - predicate interface{} + m model.Model + predicate func(model.Model) bool }{ { name: "predicate returns none", - predicate: func(t *testLogicalSwitchPort) bool { + m: &testLogicalSwitchPort{}, + predicate: func(t model.Model) bool { return false }, }, { name: "predicate returns all", - predicate: func(t *testLogicalSwitchPort) bool { + m: &testLogicalSwitchPort{}, + predicate: func(t model.Model) bool { return true }, }, { name: "predicate on an arbitrary condition", - predicate: func(t *testLogicalSwitchPort) bool { + m: &testLogicalSwitchPort{}, + predicate: func(m model.Model) bool { + t := m.(*testLogicalSwitchPort) return strings.HasPrefix(t.Name, "ls1") }, }, { name: "predicate matches name", - predicate: func(t *testLogicalSwitchPort) bool { + m: &testLogicalSwitchPort{}, + predicate: func(m model.Model) bool { + t := m.(*testLogicalSwitchPort) return t.Name == lscacheList[index].Name }, }, @@ -1763,7 +1745,7 @@ func BenchmarkAPIList(b *testing.B) { api := newAPI(tcache, &discardLogger) var cond ConditionalAPI if tt.predicate != nil { - cond = api.WhereCache(tt.predicate) + cond = api.WhereCache(tt.m, tt.predicate) } else { cond = api.Where(lscacheList[index]) } @@ -1979,7 +1961,7 @@ func TestAPIWait(t *testing.T) { { name: "no operation", condition: func(a API) ConditionalAPI { - return a.WhereCache(func(t *testLogicalSwitchPort) bool { return false }) + return a.WhereCache(&testLogicalSwitchPort{}, func(t model.Model) bool { return false }) }, until: "==", prepare: func() (model.Model, []interface{}) { diff --git a/client/client.go b/client/client.go index 10ea757e..7a526c27 100644 --- a/client/client.go +++ b/client/client.go @@ -1475,6 +1475,6 @@ func (o *ovsdbClient) WhereAll(m model.Model, conditions ...model.Condition) Con } // WhereCache implements the API interface's WhereCache function -func (o *ovsdbClient) WhereCache(predicate interface{}) ConditionalAPI { - return o.primaryDB().api.WhereCache(predicate) +func (o *ovsdbClient) WhereCache(m model.Model, predicate func(model.Model) bool) ConditionalAPI { + return o.primaryDB().api.WhereCache(m, predicate) } diff --git a/client/client_test.go b/client/client_test.go index 1f66651a..23ac2da9 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -81,6 +81,10 @@ type Bridge struct { STPEnable bool `ovsdb:"stp_enable"` } +func (b *Bridge) Table() string { + return "Bridge" +} + // OpenvSwitch defines an object in Open_vSwitch table type OpenvSwitch struct { UUID string `ovsdb:"_uuid"` @@ -103,6 +107,10 @@ type OpenvSwitch struct { SystemVersion *string `ovsdb:"system_version"` } +func (o *OpenvSwitch) Table() string { + return "Open_vSwitch" +} + var defDB, _ = model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &OpenvSwitch{}, diff --git a/client/condition.go b/client/condition.go index 1dfabda0..435111c0 100644 --- a/client/condition.go +++ b/client/condition.go @@ -2,7 +2,6 @@ package client import ( "fmt" - "reflect" "github.com/ovn-org/libovsdb/cache" "github.com/ovn-org/libovsdb/mapper" @@ -176,7 +175,7 @@ func newExplicitConditional(table string, cache *cache.TableCache, matchAll bool // to match on models. type predicateConditional struct { tableName string - predicate interface{} + predicate func(model.Model) bool cache *cache.TableCache } @@ -192,8 +191,7 @@ func (c *predicateConditional) Matches() (map[string]model.Model, error) { // run the predicate on a shallow copy of the models for speed and only // clone the matches for u, m := range tableCache.RowsShallow() { - ret := reflect.ValueOf(c.predicate).Call([]reflect.Value{reflect.ValueOf(m)}) - if ret[0].Bool() { + if c.predicate(m) { found[u] = model.Clone(m) } } @@ -215,7 +213,7 @@ func (c *predicateConditional) Generate() ([][]ovsdb.Condition, error) { } // newPredicateConditional creates a new predicateConditional -func newPredicateConditional(table string, cache *cache.TableCache, predicate interface{}) (Conditional, error) { +func newPredicateConditional(table string, cache *cache.TableCache, predicate func(model.Model) bool) (Conditional, error) { return &predicateConditional{ tableName: table, predicate: predicate, diff --git a/client/condition_test.go b/client/condition_test.go index 5bd63b33..00fa8eeb 100644 --- a/client/condition_test.go +++ b/client/condition_test.go @@ -204,14 +204,15 @@ func TestPredicateConditional(t *testing.T) { test := []struct { name string - predicate interface{} + predicate func(model.Model) bool condition [][]ovsdb.Condition matches map[string]model.Model err bool }{ { name: "simple value comparison", - predicate: func(lsp *testLogicalSwitchPort) bool { + predicate: func(m model.Model) bool { + lsp := m.(*testLogicalSwitchPort) return lsp.UUID == aUUID0 }, condition: [][]ovsdb.Condition{ @@ -225,7 +226,8 @@ func TestPredicateConditional(t *testing.T) { }, { name: "by random field", - predicate: func(lsp *testLogicalSwitchPort) bool { + predicate: func(m model.Model) bool { + lsp := m.(*testLogicalSwitchPort) return lsp.Enabled != nil && *lsp.Enabled == false }, condition: [][]ovsdb.Condition{ diff --git a/cmd/stress/stress.go b/cmd/stress/stress.go index 2ebbdcb7..e333be0e 100644 --- a/cmd/stress/stress.go +++ b/cmd/stress/stress.go @@ -29,12 +29,20 @@ type bridgeType struct { Status map[string]string `ovsdb:"status"` } +func (b *bridgeType) Table() string { + return "Bridge" +} + // ORMovs is the simplified ORM model of the Bridge table type ovsType struct { UUID string `ovsdb:"_uuid"` Bridges []string `ovsdb:"bridges"` } +func (o *ovsType) Table() string { + return "Open_vSwitch" +} + var ( cpuprofile = flag.String("cpuprofile", "", "write cpu profile to this file") memprofile = flag.String("memoryprofile", "", "write memory profile to this file") diff --git a/model/model.go b/model/model.go index c8575f5b..57eb9db5 100644 --- a/model/model.go +++ b/model/model.go @@ -16,20 +16,25 @@ import ( // The struct may also have non-tagged fields (which will be ignored by the API calls) // The Model interface must be implemented by the pointer to such type // Example: -//type MyLogicalRouter struct { -// UUID string `ovsdb:"_uuid"` -// Name string `ovsdb:"name"` -// ExternalIDs map[string]string `ovsdb:"external_ids"` -// LoadBalancers []string `ovsdb:"load_balancer"` -//} -type Model interface{} +// +// type MyLogicalRouter struct { +// UUID string `ovsdb:"_uuid"` +// Name string `ovsdb:"name"` +// ExternalIDs map[string]string `ovsdb:"external_ids"` +// LoadBalancers []string `ovsdb:"load_balancer"` +// } +type Model interface { + Table() string +} type CloneableModel interface { + Model CloneModel() Model CloneModelInto(Model) } type ComparableModel interface { + Model EqualsModel(Model) bool } @@ -43,7 +48,7 @@ func Clone(a Model) Model { b := reflect.New(val.Type()).Interface() aBytes, _ := json.Marshal(a) _ = json.Unmarshal(aBytes, b) - return b + return b.(Model) } // CloneInto deep copies a model into another one diff --git a/model/model_test.go b/model/model_test.go index e75a4d45..2e40daf8 100644 --- a/model/model_test.go +++ b/model/model_test.go @@ -14,16 +14,28 @@ type modelA struct { UUID string `ovsdb:"_uuid"` } +func (m *modelA) Table() string { + return "A" +} + type modelB struct { UID string `ovsdb:"_uuid"` Foo string `ovsdb:"bar"` Bar string `ovsdb:"baz"` } +func (m *modelB) Table() string { + return "B" +} + type modelInvalid struct { Foo string } +func (m *modelInvalid) Table() string { + return "Invalid" +} + func TestClientDBModel(t *testing.T) { type Test struct { name string @@ -86,16 +98,22 @@ func TestSetUUID(t *testing.T) { } +type testTable struct { + AUUID string `ovsdb:"_uuid"` + AString string `ovsdb:"aString"` + AInt int `ovsdb:"aInt"` + AFloat float64 `ovsdb:"aFloat"` + ASet []string `ovsdb:"aSet"` + AMap map[string]string `ovsdb:"aMap"` +} + +func (t *testTable) Table() string { + return "TestTable" +} + func TestValidate(t *testing.T) { model, err := NewClientDBModel("TestDB", map[string]Model{ - "TestTable": &struct { - aUUID string `ovsdb:"_uuid"` - aString string `ovsdb:"aString"` - aInt int `ovsdb:"aInt"` - aFloat float64 `ovsdb:"aFloat"` - aSet []string `ovsdb:"aSet"` - aMap map[string]string `ovsdb:"aMap"` - }{}, + "TestTable": &testTable{}, }) assert.Nil(t, err) diff --git a/test/ovs/ovs_integration_test.go b/test/ovs/ovs_integration_test.go index daafc926..a3621a3d 100644 --- a/test/ovs/ovs_integration_test.go +++ b/test/ovs/ovs_integration_test.go @@ -166,6 +166,10 @@ type bridgeType struct { DatapathID *string `ovsdb:"datapath_id"` } +func (b *bridgeType) Table() string { + return "Bridge" +} + // ovsType is the ORM model of the OVS table type ovsType struct { UUID string `ovsdb:"_uuid"` @@ -188,18 +192,30 @@ type ovsType struct { SystemVersion *string `ovsdb:"system_version"` } +func (o *ovsType) Table() string { + return "Open_vSwitch" +} + // ipfixType is a simplified ORM model for the IPFIX table type ipfixType struct { UUID string `ovsdb:"_uuid"` Targets []string `ovsdb:"targets"` } +func (i *ipfixType) Table() string { + return "IPFIX" +} + // queueType is the simplified ORM model of the Queue table type queueType struct { UUID string `ovsdb:"_uuid"` DSCP *int `ovsdb:"dscp"` } +func (q *queueType) Table() string { + return "Queue" +} + var defDB, _ = model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}, @@ -585,7 +601,7 @@ func (suite *OVSIntegrationSuite) TestInsertAndDeleteTransactIntegration() { require.NoError(suite.T(), err) ovsRow := ovsType{} - delMutateOp, err := suite.clientWithoutInactvityCheck.WhereCache(func(*ovsType) bool { return true }). + delMutateOp, err := suite.clientWithoutInactvityCheck.WhereCache(&ovsType{}, func(m model.Model) bool { return true }). Mutate(&ovsRow, model.Mutation{ Field: &ovsRow.Bridges, Mutator: ovsdb.MutateOperationDelete, @@ -828,7 +844,7 @@ func (suite *OVSIntegrationSuite) createBridge(bridgeName string) (string, error // Inserting a Bridge row in Bridge table requires mutating the open_vswitch table. ovsRow := ovsType{} - mutateOp, err := suite.clientWithoutInactvityCheck.WhereCache(func(*ovsType) bool { return true }). + mutateOp, err := suite.clientWithoutInactvityCheck.WhereCache(&ovsType{}, func(m model.Model) bool { return true }). Mutate(&ovsRow, model.Mutation{ Field: &ovsRow.Bridges, Mutator: ovsdb.MutateOperationInsert, @@ -1157,7 +1173,7 @@ func (suite *OVSIntegrationSuite) TestMultipleOpsSameRow() { ops = append(ops, op...) ovs := ovsType{} - op, err = suite.clientWithoutInactvityCheck.WhereCache(func(*ovsType) bool { return true }).Mutate(&ovs, model.Mutation{ + op, err = suite.clientWithoutInactvityCheck.WhereCache(&ovsType{}, func(m model.Model) bool { return true }).Mutate(&ovs, model.Mutation{ Field: &ovs.Bridges, Mutator: ovsdb.MutateOperationInsert, Value: []string{bridgeUUID}, diff --git a/test/test_data.go b/test/test_data.go index 6be6b8b2..55a8b105 100644 --- a/test/test_data.go +++ b/test/test_data.go @@ -144,12 +144,20 @@ type BridgeType struct { Status map[string]string `ovsdb:"status"` } +func (b *BridgeType) Table() string { + return "Bridge" +} + // OvsType is the simplified ORM model of the Bridge table type OvsType struct { UUID string `ovsdb:"_uuid"` Bridges []string `ovsdb:"bridges"` } +func (o *OvsType) Table() string { + return "Open_vSwitch" +} + type FlowSampleCollectorSetType struct { UUID string `ovsdb:"_uuid"` Bridge string `ovsdb:"bridge"` @@ -158,6 +166,10 @@ type FlowSampleCollectorSetType struct { IPFIX *string // `ovsdb:"ipfix"` } +func (f *FlowSampleCollectorSetType) Table() string { + return "FlowSampleCollectorSet" +} + func GetModel() (model.DatabaseModel, error) { client, err := model.NewClientDBModel( "Open_vSwitch", From da639d785db2bd278f827f13f3a779f2c18f7b3f Mon Sep 17 00:00:00 2001 From: Yan Zhu Date: Wed, 27 Sep 2023 19:45:21 +0800 Subject: [PATCH 3/3] rename model.Model interface's Table func name, avoid conflict with sb https://github.com/ovn-org/ovn/blob/686caaf66d5be811c655ea2938b082564d5f5f75/ovn-sb.ovsschema#L383 Signed-off-by: Yan Zhu --- .gitignore | 17 +++++++- Makefile | 22 ++++++++-- cache/cache_test.go | 16 +++---- client/api.go | 2 +- client/api_test_model.go | 10 ++--- client/client_test.go | 4 +- cmd/stress/stress.go | 4 +- example/icnb/gen.go | 3 ++ example/icsb/gen.go | 3 ++ example/nb/gen.go | 3 ++ example/sb/gen.go | 3 ++ example/vtep/gen.go | 3 ++ model/model.go | 2 +- model/model_test.go | 17 ++++++-- modelgen/table.go | 2 +- modelgen/table_test.go | 73 +++++++++++++++++++++++++++++--- ovsdb/serverdb/database.go | 2 +- test/ovs/ovs_integration_test.go | 8 ++-- test/test_data.go | 6 +-- 19 files changed, 157 insertions(+), 43 deletions(-) create mode 100644 example/icnb/gen.go create mode 100644 example/icsb/gen.go create mode 100644 example/nb/gen.go create mode 100644 example/sb/gen.go create mode 100644 example/vtep/gen.go diff --git a/.gitignore b/.gitignore index b64d0470..0c14df28 100644 --- a/.gitignore +++ b/.gitignore @@ -60,4 +60,19 @@ Temporary Items *.cov example/vswitchd/ovs.ovsschema example/vswitchd/*.go -!example/vswitchd/gen.go \ No newline at end of file +!example/vswitchd/gen.go +example/vtep/vtep.ovsschema +example/vtep/*.go +!example/vtep/gen.go +example/nb/ovn-nb.ovsschema +example/nb/*.go +!example/nb/gen.go +example/sb/ovn-sb.ovsschema +example/sb/*.go +!example/sb/gen.go +example/icnb/ovn-ic-nb.ovsschema +example/icnb/*.go +!example/icnb/gen.go +example/icsb/ovn-ic-sb.ovsschema +example/icsb/*.go +!example/icsb/gen.go diff --git a/Makefile b/Makefile index af63e7db..37f31fc4 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,5 @@ -OVS_VERSION ?= v2.16.0 +OVS_VERSION ?= v2.17.7 +OVN_VERSION ?= v22.03.4 .PHONY: all all: lint build test integration-test coverage @@ -9,12 +10,12 @@ modelgen: @go build -v -o ./bin ./cmd/modelgen .PHONY: prebuild -prebuild: modelgen ovsdb/serverdb/_server.ovsschema example/vswitchd/ovs.ovsschema +prebuild: modelgen ovsdb/serverdb/_server.ovsschema example/vswitchd/ovs.ovsschema example/vtep/vtep.ovsschema example/nb/ovn-nb.ovsschema example/sb/ovn-sb.ovsschema example/icnb/ovn-ic-nb.ovsschema example/icsb/ovn-ic-sb.ovsschema @echo "+ $@" @go generate -v ./... .PHONY: build -build: prebuild +build: prebuild @echo "+ $@" @go build -v ./... @@ -56,3 +57,18 @@ ovsdb/serverdb/_server.ovsschema: example/vswitchd/ovs.ovsschema: @curl -sSL https://raw.githubusercontent.com/openvswitch/ovs/${OVS_VERSION}/vswitchd/vswitch.ovsschema -o $@ + +example/vtep/vtep.ovsschema: + @curl -sSL https://raw.githubusercontent.com/openvswitch/ovs/${OVS_VERSION}/vtep/vtep.ovsschema -o $@ + +example/nb/ovn-nb.ovsschema: + @curl -sSL https://raw.githubusercontent.com/ovn-org/ovn/${OVN_VERSION}/ovn-nb.ovsschema -o $@ + +example/sb/ovn-sb.ovsschema: + @curl -sSL https://raw.githubusercontent.com/ovn-org/ovn/${OVN_VERSION}/ovn-sb.ovsschema -o $@ + +example/icnb/ovn-ic-nb.ovsschema: + @curl -sSL https://raw.githubusercontent.com/ovn-org/ovn/${OVN_VERSION}/ovn-ic-nb.ovsschema -o $@ + +example/icsb/ovn-ic-sb.ovsschema: + @curl -sSL https://raw.githubusercontent.com/ovn-org/ovn/${OVN_VERSION}/ovn-ic-sb.ovsschema -o $@ diff --git a/cache/cache_test.go b/cache/cache_test.go index 1b56e14d..d87344fc 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -24,7 +24,7 @@ type testModel struct { Datapath *string `ovsdb:"datapath"` } -func (t *testModel) Table() string { +func (t *testModel) GetTableName() string { return "Open_vSwitch" } @@ -379,7 +379,7 @@ type testModel1 struct { Bar map[string]string `ovsdb:"bar"` } -func (t *testModel1) Table() string { +func (t *testModel1) GetTableName() string { return "Open_vSwitch" } @@ -807,7 +807,7 @@ type testModel2 struct { Baz string `ovsdb:"baz"` } -func (t *testModel2) Table() string { +func (t *testModel2) GetTableName() string { return "Open_vSwitch" } @@ -1058,7 +1058,7 @@ type testModel3 struct { Bar map[string]string `ovsdb:"bar"` } -func (t *testModel3) Table() string { +func (t *testModel3) GetTableName() string { return "Open_vSwitch" } @@ -1922,7 +1922,7 @@ type badModel struct { Baz string `ovsdb:"baz"` } -func (b *badModel) Table() string { +func (b *badModel) GetTableName() string { return "bad" } @@ -2156,7 +2156,7 @@ type testModel4 struct { Baz map[string]string `ovsdb:"baz"` } -func (t *testModel4) Table() string { +func (t *testModel4) GetTableName() string { return "Open_vSwitch" } @@ -2331,7 +2331,7 @@ type rowsByConditionTestModel struct { Empty string `ovsdb:"empty"` } -func (r *rowsByConditionTestModel) Table() string { +func (r *rowsByConditionTestModel) GetTableName() string { return "Open_vSwitch" } @@ -2747,7 +2747,7 @@ type testDBModel struct { Set []string `ovsdb:"set"` } -func (t *testDBModel) Table() string { +func (t *testDBModel) GetTableName() string { return "Open_vSwitch" } diff --git a/client/api.go b/client/api.go index 1a427c9b..b8cfd210 100644 --- a/client/api.go +++ b/client/api.go @@ -549,7 +549,7 @@ func (a api) Wait(untilConFun ovsdb.WaitCondition, timeout *int, model model.Mod // getTableFromModel returns the table name from a Model object after performing // type verifications on the model func (a api) getTableFromModel(m model.Model) (string, error) { - table := m.Table() + table := m.GetTableName() _, found := a.cache.DatabaseModel().Types()[table] if table == "" || !found { return "", &ErrWrongType{reflect.TypeOf(m), "Model not found in Database Model"} diff --git a/client/api_test_model.go b/client/api_test_model.go index 36ea476e..677761e6 100644 --- a/client/api_test_model.go +++ b/client/api_test_model.go @@ -123,12 +123,12 @@ type testLogicalSwitch struct { Acls []string `ovsdb:"acls"` } -// Table returns the table name. It's part of the Model interface -func (*testLogicalSwitch) Table() string { +// GetTableName returns the table name. It's part of the Model interface +func (*testLogicalSwitch) GetTableName() string { return "Logical_Switch" } -//LogicalSwitchPort struct defines an object in Logical_Switch_Port table +// LogicalSwitchPort struct defines an object in Logical_Switch_Port table type testLogicalSwitchPort struct { UUID string `ovsdb:"_uuid"` Up *bool `ovsdb:"up"` @@ -148,8 +148,8 @@ type testLogicalSwitchPort struct { ParentName *string `ovsdb:"parent_name"` } -// Table returns the table name. It's part of the Model interface -func (*testLogicalSwitchPort) Table() string { +// GetTableName returns the table name. It's part of the Model interface +func (*testLogicalSwitchPort) GetTableName() string { return "Logical_Switch_Port" } diff --git a/client/client_test.go b/client/client_test.go index 23ac2da9..9eeec738 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -81,7 +81,7 @@ type Bridge struct { STPEnable bool `ovsdb:"stp_enable"` } -func (b *Bridge) Table() string { +func (b *Bridge) GetTableName() string { return "Bridge" } @@ -107,7 +107,7 @@ type OpenvSwitch struct { SystemVersion *string `ovsdb:"system_version"` } -func (o *OpenvSwitch) Table() string { +func (o *OpenvSwitch) GetTableName() string { return "Open_vSwitch" } diff --git a/cmd/stress/stress.go b/cmd/stress/stress.go index e333be0e..c2948b73 100644 --- a/cmd/stress/stress.go +++ b/cmd/stress/stress.go @@ -29,7 +29,7 @@ type bridgeType struct { Status map[string]string `ovsdb:"status"` } -func (b *bridgeType) Table() string { +func (b *bridgeType) GetTableName() string { return "Bridge" } @@ -39,7 +39,7 @@ type ovsType struct { Bridges []string `ovsdb:"bridges"` } -func (o *ovsType) Table() string { +func (o *ovsType) GetTableName() string { return "Open_vSwitch" } diff --git a/example/icnb/gen.go b/example/icnb/gen.go new file mode 100644 index 00000000..11a89ee8 --- /dev/null +++ b/example/icnb/gen.go @@ -0,0 +1,3 @@ +package icnb + +//go:generate ../../bin/modelgen --extended -p icnb -o . ovn-ic-nb.ovsschema diff --git a/example/icsb/gen.go b/example/icsb/gen.go new file mode 100644 index 00000000..5eea84c8 --- /dev/null +++ b/example/icsb/gen.go @@ -0,0 +1,3 @@ +package icsb + +//go:generate ../../bin/modelgen --extended -p icsb -o . ovn-ic-sb.ovsschema diff --git a/example/nb/gen.go b/example/nb/gen.go new file mode 100644 index 00000000..136ce7ef --- /dev/null +++ b/example/nb/gen.go @@ -0,0 +1,3 @@ +package nb + +//go:generate ../../bin/modelgen --extended -p nb -o . ovn-nb.ovsschema diff --git a/example/sb/gen.go b/example/sb/gen.go new file mode 100644 index 00000000..f5b5798b --- /dev/null +++ b/example/sb/gen.go @@ -0,0 +1,3 @@ +package sb + +//go:generate ../../bin/modelgen --extended -p sb -o . ovn-sb.ovsschema diff --git a/example/vtep/gen.go b/example/vtep/gen.go new file mode 100644 index 00000000..038afbd7 --- /dev/null +++ b/example/vtep/gen.go @@ -0,0 +1,3 @@ +package vtep + +//go:generate ../../bin/modelgen --extended -p vtep -o . vtep.ovsschema diff --git a/model/model.go b/model/model.go index 57eb9db5..cfe5fb05 100644 --- a/model/model.go +++ b/model/model.go @@ -24,7 +24,7 @@ import ( // LoadBalancers []string `ovsdb:"load_balancer"` // } type Model interface { - Table() string + GetTableName() string } type CloneableModel interface { diff --git a/model/model_test.go b/model/model_test.go index 2e40daf8..390d4144 100644 --- a/model/model_test.go +++ b/model/model_test.go @@ -14,7 +14,7 @@ type modelA struct { UUID string `ovsdb:"_uuid"` } -func (m *modelA) Table() string { +func (m *modelA) GetTableName() string { return "A" } @@ -24,7 +24,7 @@ type modelB struct { Bar string `ovsdb:"baz"` } -func (m *modelB) Table() string { +func (m *modelB) GetTableName() string { return "B" } @@ -32,7 +32,7 @@ type modelInvalid struct { Foo string } -func (m *modelInvalid) Table() string { +func (m *modelInvalid) GetTableName() string { return "Invalid" } @@ -107,7 +107,7 @@ type testTable struct { AMap map[string]string `ovsdb:"aMap"` } -func (t *testTable) Table() string { +func (t *testTable) GetTableName() string { return "TestTable" } @@ -454,3 +454,12 @@ func TestEqualViaComparable(t *testing.T) { a.UID = "baz" assert.False(t, Equal(a, b)) } + +func TestGetTableName(t *testing.T) { + a := &testTable{} + func(a interface{}) { + m, ok := a.(Model) + assert.True(t, ok, "is not a model") + assert.Equal(t, m.GetTableName(), "TestTable") + }(a) +} diff --git a/modelgen/table.go b/modelgen/table.go index a7cc1854..3b9c7f94 100644 --- a/modelgen/table.go +++ b/modelgen/table.go @@ -246,7 +246,7 @@ type {{ index . "StructName" }} struct { {{ end }} {{ template "extraFields" . }} } -func (a *{{ index . "StructName" }}) Table() string { +func (a *{{ index . "StructName" }}) GetTableName() string { return {{ index . "StructName" }}Table } {{ template "postStructDefinitions" . }} diff --git a/modelgen/table_test.go b/modelgen/table_test.go index b3e38111..f69194b5 100644 --- a/modelgen/table_test.go +++ b/modelgen/table_test.go @@ -8,7 +8,12 @@ import ( "text/template" "github.com/google/uuid" + "github.com/ovn-org/libovsdb/example/icnb" + "github.com/ovn-org/libovsdb/example/icsb" + "github.com/ovn-org/libovsdb/example/nb" + "github.com/ovn-org/libovsdb/example/sb" "github.com/ovn-org/libovsdb/example/vswitchd" + "github.com/ovn-org/libovsdb/example/vtep" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" "github.com/stretchr/testify/assert" @@ -81,7 +86,7 @@ type AtomicTable struct { Str string ` + "`" + `ovsdb:"str"` + "`" + ` } -func (a *AtomicTable) Table() string { +func (a *AtomicTable) GetTableName() string { return AtomicTableTable } `, @@ -108,7 +113,7 @@ type AtomicTable struct { Str string ` + "`" + `ovsdb:"str"` + "`" + ` } -func (a *AtomicTable) Table() string { +func (a *AtomicTable) GetTableName() string { return AtomicTableTable } `, @@ -160,7 +165,7 @@ type AtomicTable struct { OtherStr string } -func (a *AtomicTable) Table() string { +func (a *AtomicTable) GetTableName() string { return AtomicTableTable } `, @@ -201,7 +206,7 @@ type AtomicTable struct { Str string ` + "`" + `ovsdb:"str"` + "`" + ` } -func (a *AtomicTable) Table() string { +func (a *AtomicTable) GetTableName() string { return AtomicTableTable } @@ -370,7 +375,7 @@ type AtomicTable struct { OtherStr string } -func (a *AtomicTable) Table() string { +func (a *AtomicTable) GetTableName() string { return AtomicTableTable } @@ -503,7 +508,7 @@ type AtomicTable struct { Str string ` + "`" + `ovsdb:"str"` + "`" + ` } -func (a *AtomicTable) Table() string { +func (a *AtomicTable) GetTableName() string { return AtomicTableTable } @@ -630,7 +635,7 @@ type AtomicTable struct { Str string ` + "`" + `ovsdb:"str"` + "`" + ` } -func (a *AtomicTable) Table() string { +func (a *AtomicTable) GetTableName() string { return AtomicTableTable } @@ -1020,3 +1025,57 @@ func BenchmarkDeepEqual(b *testing.B) { }) } } + +func TestVswitchedGetTableName(t *testing.T) { + bridge := &vswitchd.Bridge{} + func(bridge interface{}) { + m, ok := bridge.(model.Model) + assert.True(t, ok, "is not a model") + assert.Equal(t, m.GetTableName(), vswitchd.BridgeTable) + }(bridge) +} + +func TestVtepGetTableName(t *testing.T) { + sw := &vtep.LogicalSwitch{} + func(sw interface{}) { + m, ok := sw.(model.Model) + assert.True(t, ok, "is not a model") + assert.Equal(t, m.GetTableName(), vtep.LogicalSwitchTable) + }(sw) +} + +func TestNBGetTableName(t *testing.T) { + sw := &nb.LogicalSwitch{} + func(sw interface{}) { + m, ok := sw.(model.Model) + assert.True(t, ok, "is not a model") + assert.Equal(t, m.GetTableName(), nb.LogicalSwitchTable) + }(sw) +} + +func TestSBGetTableName(t *testing.T) { + p := &sb.RBACPermission{} + func(p interface{}) { + m, ok := p.(model.Model) + assert.True(t, ok, "is not a model") + assert.Equal(t, m.GetTableName(), sb.RBACPermissionTable) + }(p) +} + +func TestICNBGetTableName(t *testing.T) { + sw := &icnb.TransitSwitch{} + func(sw interface{}) { + m, ok := sw.(model.Model) + assert.True(t, ok, "is not a model") + assert.Equal(t, m.GetTableName(), icnb.TransitSwitchTable) + }(sw) +} + +func TestICSBGetTableName(t *testing.T) { + gw := &icsb.Gateway{} + func(gw interface{}) { + m, ok := gw.(model.Model) + assert.True(t, ok, "is not a model") + assert.Equal(t, m.GetTableName(), icsb.GatewayTable) + }(gw) +} diff --git a/ovsdb/serverdb/database.go b/ovsdb/serverdb/database.go index 8d6f4ad7..52b948c5 100644 --- a/ovsdb/serverdb/database.go +++ b/ovsdb/serverdb/database.go @@ -30,7 +30,7 @@ type Database struct { Sid *string `ovsdb:"sid"` } -func (a *Database) Table() string { +func (a *Database) GetTableName() string { return DatabaseTable } diff --git a/test/ovs/ovs_integration_test.go b/test/ovs/ovs_integration_test.go index a3621a3d..b74869ce 100644 --- a/test/ovs/ovs_integration_test.go +++ b/test/ovs/ovs_integration_test.go @@ -166,7 +166,7 @@ type bridgeType struct { DatapathID *string `ovsdb:"datapath_id"` } -func (b *bridgeType) Table() string { +func (b *bridgeType) GetTableName() string { return "Bridge" } @@ -192,7 +192,7 @@ type ovsType struct { SystemVersion *string `ovsdb:"system_version"` } -func (o *ovsType) Table() string { +func (o *ovsType) GetTableName() string { return "Open_vSwitch" } @@ -202,7 +202,7 @@ type ipfixType struct { Targets []string `ovsdb:"targets"` } -func (i *ipfixType) Table() string { +func (i *ipfixType) GetTableName() string { return "IPFIX" } @@ -212,7 +212,7 @@ type queueType struct { DSCP *int `ovsdb:"dscp"` } -func (q *queueType) Table() string { +func (q *queueType) GetTableName() string { return "Queue" } diff --git a/test/test_data.go b/test/test_data.go index 55a8b105..857d0d5e 100644 --- a/test/test_data.go +++ b/test/test_data.go @@ -144,7 +144,7 @@ type BridgeType struct { Status map[string]string `ovsdb:"status"` } -func (b *BridgeType) Table() string { +func (b *BridgeType) GetTableName() string { return "Bridge" } @@ -154,7 +154,7 @@ type OvsType struct { Bridges []string `ovsdb:"bridges"` } -func (o *OvsType) Table() string { +func (o *OvsType) GetTableName() string { return "Open_vSwitch" } @@ -166,7 +166,7 @@ type FlowSampleCollectorSetType struct { IPFIX *string // `ovsdb:"ipfix"` } -func (f *FlowSampleCollectorSetType) Table() string { +func (f *FlowSampleCollectorSetType) GetTableName() string { return "FlowSampleCollectorSet" }