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

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

Merged
merged 1 commit into from
May 16, 2023
Merged

Conversation

jcaamano
Copy link
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 [email protected]
Co-authored-by: Dan Williams [email protected]

@coveralls
Copy link

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
Copy link

blacob commented Feb 15, 2023

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

@dcbw
Copy link
Contributor

dcbw commented May 11, 2023

@jcaamano want to grab 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 <[email protected]>
Co-authored-by: Dan Williams [email protected]
@dcbw
Copy link
Contributor

dcbw commented May 16, 2023

LGTM, I'll follow up some validation and cleanups #333

@dcbw dcbw merged commit de1704e into ovn-org: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