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

Fix The Server Impl so OVN-K CI can pass #238

Merged
merged 4 commits into from
Oct 8, 2021

Conversation

dave-tucker
Copy link
Collaborator

Signed-off-by: Dave Tucker dave@dtucker.co.uk

This fixes a couple of bugs with the Populate2/Update2 handling

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
There are issues when list_dbs fails due to context deadline exceeded
and for some reason the `defer` doesn't properly clean up the rpcClient.
This isn't the best fix, but it does solve the problem for now.

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
This ensures that the cache is locked when calling Populate/Populate2
and expands the model used to represent the OVS table such that we don't
have issues caused by Monitors attempting to update unmapped fields.
In future, we should probably change the Monitor code to only ask for
ORM mapped fields.

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
@dave-tucker dave-tucker marked this pull request as ready for review October 8, 2021 13:03
@coveralls
Copy link

coveralls commented Oct 8, 2021

Pull Request Test Coverage Report for Build 1320580526

  • 126 of 186 (67.74%) changed or added relevant lines in 6 files are covered.
  • 7 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.5%) to 72.88%

Changes Missing Coverage Covered Lines Changed/Added Lines %
server/server.go 0 8 0.0%
server/transact.go 46 58 79.31%
client/client.go 7 20 35.0%
server/mutate.go 59 72 81.94%
ovsdb/updates2.go 7 21 33.33%
Files with Coverage Reduction New Missed Lines %
cache/cache.go 1 71.41%
client/client.go 1 67.07%
server/monitor.go 2 47.77%
server/transact.go 3 67.1%
Totals Coverage Status
Change from base Build 1317323617: 0.5%
Covered Lines: 3910
Relevant Lines: 5365

💛 - Coveralls

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
}
}
default:
panic("ARGH!")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be something more descriptive? Like "libovsdb: Invalid type detected during merge?

}

func insertToSlice(a, b reflect.Value) reflect.Value {
func insertToSlice(a, b reflect.Value) (reflect.Value, bool) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice on some of these functions to have doc text on them. a is the slice, b is the value to attempt to insert into the slice if it doesn't already exist in a. Returns true if b was added to a.

@@ -58,74 +57,121 @@ func mutate(current interface{}, mutator ovsdb.Mutator, value interface{}) (inte
return current, value
}

func mutateInsert(current, value interface{}) interface{} {
func mutateInsert(current, value interface{}) (interface{}, interface{}) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same some doctext on this function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants