Skip to content

ovsdb: don't create sized arrays for OVS Sets#347

Merged
dcbw merged 1 commit into
ovn-kubernetes:mainfrom
jcaamano:main
May 16, 2023
Merged

ovsdb: don't create sized arrays for OVS Sets#347
dcbw merged 1 commit into
ovn-kubernetes:mainfrom
jcaamano:main

Conversation

@jcaamano

Copy link
Copy Markdown
Collaborator

OVS Sets must be unique, but modelgen created sized arrays to represent OVS Sets, which when marshalled are filled with duplicate default values.

For a column schema like:

"cvlans": {
"type": {"key": {"type": "integer",
"minInteger": 0,
"maxInteger": 4095},
"min": 0, "max": 4096}},

modelgen would create:

CVLANs [4096]int

which when marshalled and sent to ovsdb-server becomes:

cvlans:{GoSet:[0 0 0 0 0 0 0 ]}

and is rejected by ovsdb-server with:

{Count:0 Error:ovsdb error Details:set contains duplicate UUID:{GoUUID:} Rows:[]}] and errors [ovsdb error: set contains duplicate]: 1 ovsdb operations failed

Instead, generate these fields as slices instead of sized arrays and enforce the size when marshalling the set.

Signed-off-by: Jaime Caamaño Ruiz jcaamano@redhat.com
Co-authored-by: Dan Williams dcbw@redhat.com

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 3930363015

  • 2 of 6 (33.33%) changed or added relevant lines in 3 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.2%) to 75.216%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ovsdb/bindings.go 0 1 0.0%
cache/cache.go 1 4 25.0%
Files with Coverage Reduction New Missed Lines %
ovsdb/bindings.go 1 79.82%
client/client.go 2 71.55%
Totals Coverage Status
Change from base Build 3912893139: 0.2%
Covered Lines: 5044
Relevant Lines: 6706

💛 - Coveralls

@blacob

blacob commented Feb 15, 2023

Copy link
Copy Markdown

This looks good to me Jaime. We've run into this issue as well. Would be great to get it merged!

@dcbw

dcbw commented May 11, 2023

Copy link
Copy Markdown
Contributor

@jcaamano want to grab ovn-org@a4ac416 here? It's rebased on current main.

OVS Sets must be unique, but modelgen created sized arrays
to represent OVS Sets, which when marshalled are filled with
duplicate default values.

For a column schema like:

   "cvlans": {
     "type": {"key": {"type": "integer",
                      "minInteger": 0,
                      "maxInteger": 4095},
              "min": 0, "max": 4096}},

modelgen would create:

   CVLANs [4096]int

which when marshalled and sent to ovsdb-server becomes:

cvlans:{GoSet:[0 0 0 0 0 0 0 <repeat many times>]}

and is rejected by ovsdb-server with:

{Count:0 Error:ovsdb error Details:set contains duplicate UUID:{GoUUID:} Rows:[]}] and errors [ovsdb error: set contains duplicate]: 1 ovsdb operations failed

Instead, generate these fields as slices instead of sized
arrays.

Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Co-authored-by: Dan Williams dcbw@redhat.com
@dcbw

dcbw commented May 16, 2023

Copy link
Copy Markdown
Contributor

LGTM, I'll follow up some validation and cleanups https://github.com/ovn-org/libovsdb/pull/333

@dcbw
dcbw merged commit de1704e into ovn-kubernetes:main May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants