Bug 2209690

Summary: RFE - bridge mapping API for OVS
Product: Red Hat Enterprise Linux 9 Reporter: Miguel Duarte Barroso <mduarted>
Component: nmstateAssignee: Gris Ge <fge>
Status: CLOSED ERRATA QA Contact: Mingyu Shi <mshi>
Severity: high Docs Contact:
Priority: unspecified    
Version: 9.2CC: ferferna, fge, jiji, jishi, maypatil, network-qe, phoracek, sfaye, till
Target Milestone: rcKeywords: FutureFeature, Triaged
Target Release: ---Flags: pm-rhel: mirror+
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: nmstate-2.2.13-1.el9 Doc Type: Enhancement
Doc Text:
Feature: Bridge mapping API for OVS Reason: Maintaining physical network name to an OVS bridge mapping through OVS external IDs is complex and prone to errors, OpenShift team is expecting simpler API in nmstate. Result: Nmstate 2.2.13 introduced the `ovn` section for this use case. For example: ```yml ovn: bridge-mappings: - localnet: blue bridge: ovsbr1 - localnet: red bridge: ovsbr2 ``` The `state: absent` could be used for deleting a mapping. For example: ```yml ovn: bridge-mappings: - localnet: red state: absent ```
Story Points: ---
Clone Of: Environment:
Last Closed: 2023-11-07 08:24:03 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Miguel Duarte Barroso 2023-05-24 14:01:56 UTC
Description of problem:
OVN has an API (ovn-bridge-mappings) - [0] - where the network administrator can associate a physical network name to an OVS bridge.

The OVN controller reacts to this by creating patch ports between the OVN integration bridge (configured and owned by OVN Kubernetes) and the OVS bridge (configured using nmstate by cluster admin) we point via the mappings.

This is an example of an OVN bridge mapping from the openvswitch table (ovs-vsctl list open .):
```
external_ids        : {..., ovn-bridge-mappings="physnet:br-ex,tenantblue_br-localnet:brsec", ...}
```

We (CNV) are using this feature to configure the underlay for *secondary* OVN-Kubernetes networks; despite that, the first listed mapping belongs to the cluster's default network - i.e. `physnet:br-ex`. That mapping is created by OVN-Kubernetes, and should be hidden from the user - since any changes to it will break the openshift cluster default network.

To make it harder for the network admin to stumble on this error we would like to have an higher "bridge-mapping" API (for OVS) where they could define the mappings.

For instance, I envision something like:
```
ovs-db:
  ovn-bridge-mappings:
    - physnet-name: tenantblue
      bridge-name: ovsbr1
      state: up | present # (?)
    - physnet-name: tenantred
      bridge-name: ovsbr1
      state: up | present # (?)
    - physnet-name: tenantpurple
      bridge-name: ovsbr2
      state: up | present # (?)
    - physnet-name: evilnetwork
      state: absent
```

AFAIU this would fit naturally with nmstate's API - any key not specified would simply be ignored.

[0] - https://access.redhat.com/documentation/en-us/red_hat_openstack_platform/16.0/html/networking_guide/bridge-mappings

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:


Additional info:

Comment 2 Petr Horáček 2023-05-24 14:58:55 UTC
+1, I like the proposed API. I would just ask for ovn-bridge-mappings instead of bridge-mappings, but that's a nit, the rest looks good. Thanks!

Comment 3 Miguel Duarte Barroso 2023-05-24 15:11:49 UTC
(In reply to Petr Horáček from comment #2)
> +1, I like the proposed API. I would just ask for ovn-bridge-mappings
> instead of bridge-mappings, but that's a nit, the rest looks good. Thanks!

Makes sense.

Comment 4 Gris Ge 2023-06-13 13:53:31 UTC
I am OK to include syntax sugar for vital use case like this. The schema looks good to me too.

Comment 5 Gris Ge 2023-06-27 12:48:09 UTC
My assumption on the schema is:

```rust
ovs-db:
  external_ids:
    hostname: 2414d3d0e2cc
    rundir: /var/run/openvswitch
    system-id: 08c1ef91-1406-4233-99a4-cd7508e17cbc
  ovn-bridge:
    mappings:
    - physnet: provider
      state: present
      bridge: br-provider
```

When showing, the `external_ids` section will _not_ hold any `ovn-bridge-mappings` data.
When applying, user cannot change ovn-bridge-mappings in external_ids section.
The reason to have `ovn-bridge: {}` instead of `ovn-bridge-mappings: []` is to allow future changes to ovs-bridge part.

When applying with `external_ids: []`, the `ovn-bridge-mappings` data should not be touched, user can only modify `ovn-bridge-mappings` through `ovn-bridge` section.

Internally, the wrapping code should be placed at top level of nmstate, so backend code in `nm` and `nispor` is not required to handle this syntax sugar.

Comment 6 Petr Horáček 2023-06-27 13:52:21 UTC
Updated schema from an offline discussion:

ovs-db:
  external_ids:
    hostname: 2414d3d0e2cc
    rundir: /var/run/openvswitch
    system-id: 08c1ef91-1406-4233-99a4-cd7508e17cbc
ovn:
  bridge-mappings:
  - localnet: provider
    bridge: br-provider
    state: present

Comment 7 Gris Ge 2023-06-29 09:01:29 UTC
In my team, the `POST` state means patch merged in upstream. Currently, we still working on patches, hence move back to assign state.

Comment 9 Miguel Duarte Barroso 2023-07-05 16:12:25 UTC
(In reply to Gris Ge from comment #7)
> In my team, the `POST` state means patch merged in upstream. Currently, we
> still working on patches, hence move back to assign state.

Oh, sorry about that. Thanks for setting it straight - and educating me while at it :) 

I think the PR is ready for a decent round of reviews.

Thanks for all the help so far @fge

Comment 10 Miguel Duarte Barroso 2023-07-07 12:55:36 UTC
Change merged upstream.

Comment 16 Mingyu Shi 2023-08-03 09:55:35 UTC
Tested `ovn` key
Checking `ovs-db` editing

Comment 19 errata-xmlrpc 2023-11-07 08:24:03 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (nmstate bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2023:6323