From 5a9f3dd7cd10f67cffcea45a18ebc6dc193668be Mon Sep 17 00:00:00 2001 From: Petr Horacek Date: Sat, 12 Oct 2019 11:48:44 +0200 Subject: [PATCH] ovs api: drop port's "type" attribute 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 --- examples/ovsbridge_create.yml | 2 - libnmstate/nm/ovs.py | 9 ----- libnmstate/schema.py | 6 --- libnmstate/schemas/operational-state.yaml | 2 - tests/integration/nm/ovs_test.py | 19 +++++---- tests/integration/testlib/ovslib.py | 11 +++--- tests/lib/metadata_test.py | 48 +++++------------------ tests/lib/nm/ovs_test.py | 1 - 8 files changed, 26 insertions(+), 72 deletions(-) diff --git a/examples/ovsbridge_create.yml b/examples/ovsbridge_create.yml index f145c4d161..b69804c119 100644 --- a/examples/ovsbridge_create.yml +++ b/examples/ovsbridge_create.yml @@ -22,6 +22,4 @@ interfaces: stp: true port: - name: eth1 - type: system - name: ovs0 - type: internal diff --git a/libnmstate/nm/ovs.py b/libnmstate/nm/ovs.py index f4eb726f6b..53f6f57a69 100644 --- a/libnmstate/nm/ovs.py +++ b/libnmstate/nm/ovs.py @@ -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 @@ -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 diff --git a/libnmstate/schema.py b/libnmstate/schema.py index 84c8f6f3a6..a1da2f3831 100644 --- a/libnmstate/schema.py +++ b/libnmstate/schema.py @@ -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' diff --git a/libnmstate/schemas/operational-state.yaml b/libnmstate/schemas/operational-state.yaml index 3cd4e85762..bd95269f2d 100644 --- a/libnmstate/schemas/operational-state.yaml +++ b/libnmstate/schemas/operational-state.yaml @@ -283,8 +283,6 @@ definitions: properties: name: type: string - type: - type: string vlan-mode: type: string access-tag: diff --git a/tests/integration/nm/ovs_test.py b/tests/integration/nm/ovs_test.py index 7b65d77ddd..d7bddf3ca0 100644 --- a/tests/integration/nm/ovs_test.py +++ b/tests/integration/nm/ovs_test.py @@ -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 @@ -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, @@ -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) @@ -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): diff --git a/tests/integration/testlib/ovslib.py b/tests/integration/testlib/ovslib.py index 9924da9be9..09c42001da 100644 --- a/tests/integration/testlib/ovslib.py +++ b/tests/integration/testlib/ovslib.py @@ -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 = [ @@ -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, @@ -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): diff --git a/tests/lib/metadata_test.py b/tests/lib/metadata_test.py index 5a8881f0f0..e7e08bd782 100644 --- a/tests/lib/metadata_test.py +++ b/tests/lib/metadata_test.py @@ -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 @@ -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'}, @@ -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'}] }, } ] @@ -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'}, @@ -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'}, @@ -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'}]}, } ] } @@ -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'}, @@ -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'}, @@ -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'}]}, } ] } @@ -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'}, diff --git a/tests/lib/nm/ovs_test.py b/tests/lib/nm/ovs_test.py index 83f8a30f56..df2328bb4f 100644 --- a/tests/lib/nm/ovs_test.py +++ b/tests/lib/nm/ovs_test.py @@ -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]