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

YAML consolidation prelude: OVS handling (FR-702) #249

Merged

Conversation

schopin-pro
Copy link
Contributor

Description

A couple of patches related to OpenVSwitch handling. The consistency patch is needed to be able to ensure idempotency of the user input (within reason). The type name definition will be needed when we'll rewrite sriov.py as we'll need to identify those definitions.

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

Copy link
Collaborator

@slyon slyon left a comment

Choose a reason for hiding this comment

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

LGTM! It makes sense to handle the definitions in an ordered way. If you don't mind I'd like the ovs-ports name, though, as this is kind of an edge case and should be visible as such, IMO.

Thanks!

src/names.c Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2022

Codecov Report

Merging #249 (901c928) into main (e0a07bc) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #249   +/-   ##
=======================================
  Coverage   99.10%   99.10%           
=======================================
  Files          58       58           
  Lines       10052    10052           
=======================================
  Hits         9962     9962           
  Misses         90       90           
Impacted Files Coverage Δ
src/netplan.c 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0a07bc...901c928. Read the comment docs.

schopin-pro and others added 3 commits January 10, 2022 10:03
The previous implementation relied on the order of traversal of a
hashtable, which isn't exactly reliable. It wasn't problematic because
the semantics of the port pair are symetrical, but since we're planing
on using the YAML netplan generator backend as a passthrough for
`netplan set`, we want to preserve the user's input as pristinely as
possible.

The new implementation uses the ordered list of netdefs to determine the
order in which netdefs are processed, which is both deterministic and
sensible, since it respects the order in which the user fed us the info
(modulo the grouping by devtype).
Later in the patchset we expose this data to Python, and are using the
names rather than the integers to represent the netdef types. As a
result, we need a proper name for this type to distinguish it from the
default value.
Contrary to all other def types, this one is "virtual", as it doesn't have a YAML key matching it. Prefixing it with an underscore should make it obvious that *something* is different there.

Co-authored-by: Lukas Märdan <[email protected]>

Co-authored-by: Lukas Märdian <[email protected]>
@schopin-pro schopin-pro merged commit 3e3c371 into canonical:main Jan 10, 2022
@schopin-pro schopin-pro deleted the yaml-consolidation-ovs-prelude branch January 13, 2022 15:10
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.

3 participants