Skip to content

Commit

Permalink
ovs api: drop port's "type" attribute
Browse files Browse the repository at this point in the history
OVS port "type" attribute is redundant. It can be currently set to "system" or
"internal", but these can be easily deduced from desired and current state. With
this patch we drop the attribute from the API, so we don't keep redundant
information and make schema more streamlined.

No other nmstate interface type requires users to specify slave type (both bond
and linux-bridge require the name only).

OVS bridge schema would change from:

```yaml
interfaces:
- name: ovs-br0
  type: ovs-bridge
  state: up
  bridge:
    port:
    - name: eth3
      type: system
```

To:

```yaml
interfaces:
- name: ovs-br0
  type: ovs-bridge
  state: up
  bridge:
    port:
    - name: eth3
```

https://nmstate.atlassian.net/browse/NMSTATE-248

Note that no changes were needed to implement this, apart from dropping the
"type" from reporting, schema and tests.

This API change is backwardly compatible for the `apply` action. If a user
specifies port's "type", it will be ignored. The "type" won't be reported
anymore in `show` action.

Signed-off-by: Petr Horacek <phoracek@redhat.com>
  • Loading branch information
phoracek authored and EdDev committed Oct 29, 2019
1 parent 0744dc1 commit 5a9f3dd
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 72 deletions.
2 changes: 0 additions & 2 deletions examples/ovsbridge_create.yml
Expand Up @@ -22,6 +22,4 @@ interfaces:
stp: true
port:
- name: eth1
type: system
- name: ovs0
type: internal
9 changes: 0 additions & 9 deletions libnmstate/nm/ovs.py
Expand Up @@ -21,7 +21,6 @@

from libnmstate.error import NmstateValueError
from libnmstate.schema import OVSBridge as OB
from libnmstate.schema import OVSBridgePortType as OBPortType

from . import connection
from . import device
Expand Down Expand Up @@ -184,15 +183,7 @@ def _get_bridge_port_info(port_profile, devices_info):

if port_slave_names:
iface_slave_name = port_slave_names[0]
iface_device = device.get_device_by_name(iface_slave_name)

if is_ovs_interface_type_id(iface_device.props.device_type):
port_type = OBPortType.INTERNAL
else:
port_type = OBPortType.SYSTEM

port_info['name'] = iface_slave_name
port_info['type'] = port_type
if vlan_mode:
port_info['vlan-mode'] = vlan_mode
port_info['access-tag'] = port_setting.props.tag
Expand Down
6 changes: 0 additions & 6 deletions libnmstate/schema.py
Expand Up @@ -214,11 +214,5 @@ class OVSBridge(object):

PORT_SUBTREE = 'port'
PORT_NAME = 'name'
PORT_TYPE = 'type'
PORT_VLAN_MODE = 'vlan-mode'
PORT_ACCESS_TAG = 'access-tag'


class OVSBridgePortType(object):
SYSTEM = 'system'
INTERNAL = 'internal'
2 changes: 0 additions & 2 deletions libnmstate/schemas/operational-state.yaml
Expand Up @@ -283,8 +283,6 @@ definitions:
properties:
name:
type: string
type:
type: string
vlan-mode:
type: string
access-tag:
Expand Down
19 changes: 12 additions & 7 deletions tests/integration/nm/ovs_test.py
Expand Up @@ -23,7 +23,6 @@

from libnmstate import nm
from libnmstate.schema import OVSBridge as OB
from libnmstate.schema import OVSBridgePortType as OBPortType

from .testlib import mainloop
from .testlib import MainloopTestError
Expand Down Expand Up @@ -77,7 +76,6 @@ def test_bridge_with_system_port(eth1_up, bridge_default_config):

eth1_port = {
OB.PORT_NAME: 'eth1',
OB.PORT_TYPE: OBPortType.SYSTEM,
# OVS vlan/s are not yet supported.
# OB.PORT_VLAN_MODE: None,
# OB.PORT_ACCESS_TAG: 0,
Expand All @@ -95,10 +93,10 @@ def test_bridge_with_system_port(eth1_up, bridge_default_config):
@pytest.mark.xfail(
raises=MainloopTestError, reason='https://bugzilla.redhat.com/1724901'
)
def test_bridge_with_internal_interface(eth1_up, bridge_default_config):
def test_bridge_with_internal_interface(bridge_default_config):
bridge_desired_state = bridge_default_config

ovs_port = {OB.PORT_NAME: 'ovs0', OB.PORT_TYPE: OBPortType.INTERNAL}
ovs_port = {OB.PORT_NAME: 'ovs0'}

bridge_desired_state[OB.CONFIG_SUBTREE][OB.PORT_SUBTREE].append(ovs_port)

Expand Down Expand Up @@ -149,11 +147,18 @@ def _attach_port_to_bridge(port_state):
port_profile_name = nm.ovs.PORT_PROFILE_PREFIX + port_state[OB.PORT_NAME]

_create_proxy_port(port_profile_name, port_state)
if port_state[OB.PORT_TYPE] == OBPortType.SYSTEM:
_connect_interface(port_profile_name, port_state)
elif port_state[OB.PORT_TYPE] == OBPortType.INTERNAL:
if _is_internal_interface(port_state[OB.PORT_NAME]):
iface_name = port_state[OB.PORT_NAME]
_create_internal_interface(iface_name, master_name=port_profile_name)
else:
_connect_interface(port_profile_name, port_state)


def _is_internal_interface(iface_name):
dev = nm.device.get_device_by_name(iface_name)
if not dev:
return True
return dev.get_device_type() == nm.nmclient.NM.DeviceType.OVS_INTERFACE


def _create_internal_interface(iface_name, master_name):
Expand Down
11 changes: 5 additions & 6 deletions tests/integration/testlib/ovslib.py
Expand Up @@ -25,10 +25,9 @@
from libnmstate.schema import InterfaceState
from libnmstate.schema import InterfaceType
from libnmstate.schema import OVSBridge
from libnmstate.schema import OVSBridgePortType


class Bridge:
class Bridge(object):
def __init__(self, name):
self._name = name
self._ifaces = [
Expand All @@ -46,10 +45,10 @@ def set_options(self, options):
] = options

def add_system_port(self, name):
self._add_port(name, OVSBridgePortType.SYSTEM)
self._add_port(name)

def add_internal_port(self, name, ipv4_state):
self._add_port(name, OVSBridgePortType.INTERNAL)
self._add_port(name)
self._ifaces.append(
{
Interface.NAME: name,
Expand All @@ -58,10 +57,10 @@ def add_internal_port(self, name, ipv4_state):
}
)

def _add_port(self, name, _type):
def _add_port(self, name):
self._bridge_iface[OVSBridge.CONFIG_SUBTREE].setdefault(
OVSBridge.PORT_SUBTREE, []
).append({OVSBridge.PORT_NAME: name, OVSBridge.PORT_TYPE: _type})
).append({OVSBridge.PORT_NAME: name})

@contextmanager
def create(self):
Expand Down
48 changes: 9 additions & 39 deletions tests/lib/metadata_test.py
Expand Up @@ -30,7 +30,6 @@
from libnmstate.schema import InterfaceIPv6
from libnmstate.schema import InterfaceType
from libnmstate.schema import InterfaceState
from libnmstate.schema import OVSBridgePortType as OBPortType
from libnmstate.schema import Route


Expand Down Expand Up @@ -339,10 +338,7 @@ def test_ovs_creation_with_new_ports(self):
'type': TYPE_OVS_BR,
'state': 'up',
'bridge': {
'port': [
{'name': 'eth0', 'type': OBPortType.SYSTEM},
{'name': 'eth1', 'type': OBPortType.SYSTEM},
]
'port': [{'name': 'eth0'}, {'name': 'eth1'}]
},
},
{'name': 'eth0', 'type': 'unknown'},
Expand Down Expand Up @@ -378,10 +374,7 @@ def test_ovs_creation_with_existing_ports(self):
'type': TYPE_OVS_BR,
'state': 'up',
'bridge': {
'port': [
{'name': 'eth0', 'type': OBPortType.SYSTEM},
{'name': 'eth1', 'type': OBPortType.SYSTEM},
]
'port': [{'name': 'eth0'}, {'name': 'eth1'}]
},
}
]
Expand Down Expand Up @@ -431,10 +424,7 @@ def test_ovs_editing_option(self):
'type': TYPE_OVS_BR,
'state': 'up',
'bridge': {
'port': [
{'name': 'eth0', 'type': OBPortType.SYSTEM},
{'name': 'eth1', 'type': OBPortType.SYSTEM},
]
'port': [{'name': 'eth0'}, {'name': 'eth1'}]
},
},
{'name': 'eth0', 'type': 'unknown'},
Expand All @@ -459,10 +449,7 @@ def test_ovs_adding_slaves(self):
'type': TYPE_OVS_BR,
'state': 'up',
'bridge': {
'port': [
{'name': 'eth0', 'type': OBPortType.SYSTEM},
{'name': 'eth1', 'type': OBPortType.SYSTEM},
]
'port': [{'name': 'eth0'}, {'name': 'eth1'}]
},
},
{'name': 'eth1', 'state': 'up', 'type': 'unknown'},
Expand Down Expand Up @@ -503,11 +490,7 @@ def test_ovs_removing_slaves(self):
'name': OVS_NAME,
'type': TYPE_OVS_BR,
'state': 'up',
'bridge': {
'port': [
{'name': 'eth0', 'type': OBPortType.SYSTEM}
]
},
'bridge': {'port': [{'name': 'eth0'}]},
}
]
}
Expand All @@ -520,10 +503,7 @@ def test_ovs_removing_slaves(self):
'type': TYPE_OVS_BR,
'state': 'up',
'bridge': {
'port': [
{'name': 'eth0', 'type': OBPortType.SYSTEM},
{'name': 'eth1', 'type': OBPortType.SYSTEM},
]
'port': [{'name': 'eth0'}, {'name': 'eth1'}]
},
},
{'name': 'eth0', 'state': 'up', 'type': 'unknown'},
Expand Down Expand Up @@ -562,10 +542,7 @@ def test_ovs_edit_slave(self):
'type': TYPE_OVS_BR,
'state': 'up',
'bridge': {
'port': [
{'name': 'eth0', 'type': OBPortType.SYSTEM},
{'name': 'eth1', 'type': OBPortType.SYSTEM},
]
'port': [{'name': 'eth0'}, {'name': 'eth1'}]
},
},
{'name': 'eth0', 'type': 'unknown'},
Expand Down Expand Up @@ -595,11 +572,7 @@ def test_ovs_reusing_slave_used_by_existing_bridge(self):
'name': OVS2_NAME,
'type': TYPE_OVS_BR,
'state': 'up',
'bridge': {
'port': [
{'name': 'eth0', 'type': OBPortType.SYSTEM}
]
},
'bridge': {'port': [{'name': 'eth0'}]},
}
]
}
Expand All @@ -612,10 +585,7 @@ def test_ovs_reusing_slave_used_by_existing_bridge(self):
'type': TYPE_OVS_BR,
'state': 'up',
'bridge': {
'port': [
{'name': 'eth0', 'type': OBPortType.SYSTEM},
{'name': 'eth1', 'type': OBPortType.SYSTEM},
]
'port': [{'name': 'eth0'}, {'name': 'eth1'}]
},
},
{'name': 'eth0', 'state': 'up', 'type': 'unknown'},
Expand Down
1 change: 0 additions & 1 deletion tests/lib/nm/ovs_test.py
Expand Up @@ -122,7 +122,6 @@ def test_get_ovs_info_with_ports_with_interfaces(

assert len(info['port']) == 1
assert 'name' in info['port'][0]
assert 'type' in info['port'][0]
assert 'vlan-mode' in info['port'][0]
assert 'access-tag' in info['port'][0]

Expand Down

0 comments on commit 5a9f3dd

Please sign in to comment.