From c0aaab12c99eea892e32493b987bcda4c5617c3c Mon Sep 17 00:00:00 2001 From: Mathieu Mitchell Date: Thu, 25 Feb 2016 11:33:09 -0500 Subject: [PATCH] [juniper-qfx] Trunk interfaces require members --- .../juniper/juniper_netconf_datastore.py | 36 ++++---- .../juniper_qfx_copper_netconf_datastore.py | 12 ++- fake_switches/netconf/__init__.py | 28 +++++- fake_switches/netconf/netconf_protocol.py | 19 ++++- setup.cfg | 1 + tests/juniper/juniper_base_protocol_test.py | 40 ++++++++- .../juniper_qfx_copper_protocol_test.py | 85 ++++++++++++++----- tests/netconf/netconf_protocol_test.py | 15 +++- 8 files changed, 190 insertions(+), 46 deletions(-) diff --git a/fake_switches/juniper/juniper_netconf_datastore.py b/fake_switches/juniper/juniper_netconf_datastore.py index 2c1e4882..0861e1ff 100644 --- a/fake_switches/juniper/juniper_netconf_datastore.py +++ b/fake_switches/juniper/juniper_netconf_datastore.py @@ -73,7 +73,7 @@ def edit(self, target, etree_conf): raise_for_unused_nodes(etree_conf, handled_elements) def commit_candidate(self): - validate(self.configurations[CANDIDATE]) + self.validate(self.configurations[CANDIDATE]) for updated_vlan in self.configurations[CANDIDATE].vlans: actual_vlan = self.configurations[RUNNING].get_vlan_by_name(updated_vlan.name) if not actual_vlan: @@ -265,14 +265,14 @@ def apply_interface_data(self, interface_node, port): else: for member in port_attributes.xpath("vlan/members"): if resolve_operation(member) == "delete": - if port.mode is None or port.mode == "access": + if port_is_in_access_mode(port): port.access_vlan = None else: port.trunk_vlans.remove(int(member.text)) if len(port.trunk_vlans) == 0: port.trunk_vlans = None else: - if port.mode is None or port.mode == "access": + if port_is_in_access_mode(port): port.access_vlan = parse_range(member.text)[0] else: if port.trunk_vlans is None: @@ -335,19 +335,19 @@ def apply_trunk_native_vlan(self, interface_data, port): def get_trunk_native_vlan_node(self, interface_node): return interface_node.xpath("unit/family/ethernet-switching/native-vlan-id") + @staticmethod + def validate(configuration): + vlan_list = [vlan.number for vlan in configuration.vlans] -def validate(configuration): - vlan_list = [vlan.number for vlan in configuration.vlans] - - for port in configuration.ports: - if port.access_vlan is not None and port.access_vlan not in vlan_list: - raise UnknownVlan(port.access_vlan, port.name, 0) - if port.trunk_native_vlan is not None and port.trunk_native_vlan not in vlan_list: - raise UnknownVlan(port.trunk_native_vlan, port.name, 0) - if port.trunk_vlans is not None: - for trunk_vlan in port.trunk_vlans: - if trunk_vlan not in vlan_list: - raise UnknownVlan(trunk_vlan, port.name, 0) + for port in configuration.ports: + if port.access_vlan is not None and port.access_vlan not in vlan_list: + raise UnknownVlan(port.access_vlan, port.name, 0) + if port.trunk_native_vlan is not None and port.trunk_native_vlan not in vlan_list: + raise UnknownVlan(port.trunk_native_vlan, port.name, 0) + if port.trunk_vlans is not None: + for trunk_vlan in port.trunk_vlans: + if trunk_vlan not in vlan_list: + raise UnknownVlan(trunk_vlan, port.name, 0) def vlan_to_etree(vlan): @@ -503,3 +503,9 @@ def get_or_create_interface(if_list, port): return existing + +def port_is_in_access_mode(port): + return port.mode is None or port.mode == "access" + +def port_is_in_trunk_mode(port): + return not port_is_in_access_mode(port) diff --git a/fake_switches/juniper_qfx_copper/juniper_qfx_copper_netconf_datastore.py b/fake_switches/juniper_qfx_copper/juniper_qfx_copper_netconf_datastore.py index 2a0b00d0..53790706 100644 --- a/fake_switches/juniper_qfx_copper/juniper_qfx_copper_netconf_datastore.py +++ b/fake_switches/juniper_qfx_copper/juniper_qfx_copper_netconf_datastore.py @@ -12,7 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -from fake_switches.juniper.juniper_netconf_datastore import JuniperNetconfDatastore, resolve_new_value +from fake_switches.juniper.juniper_netconf_datastore import JuniperNetconfDatastore, resolve_new_value, port_is_in_trunk_mode +from fake_switches.netconf import FailingCommitResults, TrunkShouldHaveVlanMembers, ConfigurationCheckOutFailed class JuniperQfxCopperNetconfDatastore(JuniperNetconfDatastore): @@ -34,3 +35,12 @@ def parse_trunk_native_vlan(self, interface_node, port): def get_trunk_native_vlan_node(self, interface_node): return interface_node.xpath("native-vlan-id") + + @staticmethod + def validate(configuration): + for port in configuration.ports: + if port_is_in_trunk_mode(port) and \ + (port.trunk_vlans is None or len(port.trunk_vlans) == 0): + raise FailingCommitResults([TrunkShouldHaveVlanMembers(interface=port.name), + ConfigurationCheckOutFailed()]) + return JuniperNetconfDatastore.validate(configuration) diff --git a/fake_switches/netconf/__init__.py b/fake_switches/netconf/__init__.py index 3962b735..5b173c1c 100644 --- a/fake_switches/netconf/__init__.py +++ b/fake_switches/netconf/__init__.py @@ -1,4 +1,4 @@ -# Copyright 2015 Internap. +# Copyright 2015-2016 Internap. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -101,12 +101,13 @@ def unqualify(lxml_element): class NetconfError(Exception): - def __init__(self, msg, severity="error", err_type=None, tag=None, info=None): + def __init__(self, msg, severity="error", err_type=None, tag=None, info=None, path=None): super(NetconfError, self).__init__(msg) self.severity = severity self.type = err_type self.tag = tag self.info = info + self.path = path class AlreadyLocked(NetconfError): @@ -134,6 +135,29 @@ def __init__(self, name): ) +class TrunkShouldHaveVlanMembers(NetconfError): + def __init__(self, interface): + super(TrunkShouldHaveVlanMembers, self).__init__(msg='\nFor trunk interface, please ensure either vlan members is configured or inner-vlan-id-list is configured\n', + severity='error', + err_type='protocol', + tag='operation-failed', + info={'bad-element': 'ethernet-switching'}, + path='\n[edit interfaces {} unit 0 family]\n'.format(interface)) + +class ConfigurationCheckOutFailed(NetconfError): + def __init__(self): + super(ConfigurationCheckOutFailed, self).__init__(msg='\nconfiguration check-out failed\n', + severity='error', + err_type='protocol', + tag='operation-failed', + info=None) + + +class FailingCommitResults(Exception): + def __init__(self, netconf_errors): + self.netconf_errors = netconf_errors + + def xml_equals(actual_node, node): if unqualify(node) != unqualify(actual_node): return False if len(node) != len(actual_node): return False diff --git a/fake_switches/netconf/netconf_protocol.py b/fake_switches/netconf/netconf_protocol.py index 3a51757d..18a970bf 100644 --- a/fake_switches/netconf/netconf_protocol.py +++ b/fake_switches/netconf/netconf_protocol.py @@ -1,4 +1,4 @@ -# Copyright 2015 Internap. +# Copyright 2015-2016 Internap. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -18,7 +18,7 @@ from twisted.internet.protocol import Protocol from lxml import etree from fake_switches.netconf import dict_2_etree, NS_BASE_1_0, normalize_operation_name, SimpleDatastore, \ - Response, OperationNotSupported, NetconfError + Response, OperationNotSupported, NetconfError, FailingCommitResults from fake_switches.netconf.capabilities import Base1_0 @@ -77,6 +77,8 @@ def process(self, data): self.reply(message_id, getattr(capability, operation_name)(operation)) except NetconfError as e: self.reply(message_id, error_to_response(e)) + except FailingCommitResults as e: + self.reply(message_id, commit_results_error_to_response(e)) handled = True if not handled: @@ -98,17 +100,26 @@ def say(self, etree_root): self.transport.write(etree.tostring(etree_root, pretty_print=True) + "]]>]]>\n") -def error_to_response(error): +def error_to_rpcerror_dict(error): error_specs = { "error-message": error.message } + if error.path: error_specs["error-path"] = error.path if error.type: error_specs["error-type"] = error.type if error.tag: error_specs["error-tag"] = error.tag if error.severity: error_specs["error-severity"] = error.severity if error.info: error_specs["error-info"] = error.info + return {"rpc-error": error_specs} + + +def error_to_response(error): + return Response(dict_2_etree(error_to_rpcerror_dict(error))) + + +def commit_results_error_to_response(commit_results_error): + return Response(dict_2_etree({'commit-results': [error_to_rpcerror_dict(e) for e in commit_results_error.netconf_errors]})) - return Response(dict_2_etree({"rpc-error": error_specs})) def remove_namespaces(xml_root): xml_root.tag = unqualify(xml_root.tag) diff --git a/setup.cfg b/setup.cfg index 7a1b0f48..b12d06b1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -24,6 +24,7 @@ packages = tests = tests no-path-adjustment = 1 logging-level=INFO +verbosity = 2 [wheel] universal = 1 diff --git a/tests/juniper/juniper_base_protocol_test.py b/tests/juniper/juniper_base_protocol_test.py index 86cc9dec..200cdc4d 100644 --- a/tests/juniper/juniper_base_protocol_test.py +++ b/tests/juniper/juniper_base_protocol_test.py @@ -340,6 +340,31 @@ def test_assigning_unknown_native_vlan_raises(self): with self.assertRaises(RPCError): self.nc.commit() + def test_trunk_mode_allows_no_vlan_members(self): + self.edit({ + "vlans": [ + {"vlan": [ + {"name": "VLAN2995"}, + {"vlan-id": "2995"}]}, + {"vlan": [ + {"name": "VLAN2996"}, + {"vlan-id": "2996"}]}, + {"vlan": [ + {"name": "VLAN2997"}, + {"vlan-id": "2997"}]}, + ], + "interfaces": { + "interface": [ + {"name": "ge-0/0/3"}, + {"native-vlan-id": "2996"}, + {"unit": [ + {"name": "0"}, + {"family": { + "ethernet-switching": { + self.PORT_MODE_TAG: "trunk" + }}}]}]}}) + self.nc.commit() + def test_trunk_mode(self): self.edit({ "vlans": [ @@ -404,6 +429,19 @@ def test_trunk_mode(self): assert_that(int003.xpath("unit/family/ethernet-switching/vlan/members"), has_length(1)) assert_that(int003.xpath("unit/family/ethernet-switching/vlan/members")[0].text, equal_to("2997")) + self.edit({ + "interfaces": { + "interface": [ + {"name": "ge-0/0/3"}, + {"unit": [ + {"name": "0"}, + {"family": { + "ethernet-switching": { + "vlan": [ + {"members": {XML_TEXT: "2997", XML_ATTRIBUTES: {"operation": "delete"}}}, + ]}}}]}]}}) + self.nc.commit() + self.cleanup(vlan("VLAN2995"), vlan("VLAN2996"), vlan("VLAN2997"), interface("ge-0/0/3", [self.PORT_MODE_TAG, "native-vlan-id", "vlan"])) result = self.nc.get_config(source="running", filter=dict_2_etree({"filter": { @@ -890,7 +928,7 @@ def test_set_interface_disabling(self): assert_that(int002.xpath("disable"), has_length(1)) self.edit({"interfaces": { - "interface": [{"name": "ge-0/0/2"}, {"disable": {XML_ATTRIBUTES: {"operation": "delete"}}}]}}) + "interface": [{"name": "ge-0/0/2"}, {"disable": {XML_ATTRIBUTES: {"operation": "delete"}}}]}}) self.nc.commit() result = self.nc.get_config(source="running", filter=dict_2_etree({"filter": { diff --git a/tests/juniper_qfx_copper/juniper_qfx_copper_protocol_test.py b/tests/juniper_qfx_copper/juniper_qfx_copper_protocol_test.py index daf4f640..c3dbeb7e 100644 --- a/tests/juniper_qfx_copper/juniper_qfx_copper_protocol_test.py +++ b/tests/juniper_qfx_copper/juniper_qfx_copper_protocol_test.py @@ -13,12 +13,14 @@ # limitations under the License. import unittest +from lxml import etree from fake_switches.netconf import dict_2_etree, XML_TEXT, XML_ATTRIBUTES from hamcrest import assert_that, has_length, equal_to, has_items, is_, is_not from ncclient import manager from ncclient.operations import RPCError from tests import contains_regex +from tests.netconf.netconf_protocol_test import xml_equals_to from tests.util.global_reactor import juniper_qfx_copper_switch_ip, \ juniper_qfx_copper_switch_netconf_port @@ -328,6 +330,59 @@ def test_assigning_unknown_vlan_raises(self): with self.assertRaises(RPCError): self.nc.commit() + def test_trunk_mode_does_not_allow_no_vlan_members(self): + self.edit({ + "vlans": [ + {"vlan": [ + {"name": "VLAN2995"}, + {"vlan-id": "2995"}]}, + {"vlan": [ + {"name": "VLAN2996"}, + {"vlan-id": "2996"}]}, + {"vlan": [ + {"name": "VLAN2997"}, + {"vlan-id": "2997"}]}, + ], + "interfaces": { + "interface": [ + {"name": "ge-0/0/3"}, + {"native-vlan-id": "2996"}, + {"unit": [ + {"name": "0"}, + {"family": { + "ethernet-switching": { + self.PORT_MODE_TAG: "trunk" + }}}]}]}}) + with self.assertRaises(RPCError) as context: + self.nc.commit() + + assert_that(etree.tostring(context.exception._raw.xpath('/*/*')[0]), xml_equals_to( + """ + + operation-failed + +For trunk interface, please ensure either vlan members is configured or inner-vlan-id-list is configured + + error + +[edit interfaces ge-0/0/3 unit 0 family] + + protocol + + ethernet-switching + + + + error + operation-failed + protocol + +configuration check-out failed + + +""")) + + def test_trunk_mode(self): self.edit({ "vlans": [ @@ -470,7 +525,10 @@ def test_interface_set_trunk_native_vlan_then_set_members_after(self): {"name": "0"}, {"family": { "ethernet-switching": { - self.PORT_MODE_TAG: "trunk" + self.PORT_MODE_TAG: "trunk", + "vlan": [ + {"members": "2996"} + ] }}}]}]}}) self.nc.commit() @@ -503,6 +561,9 @@ def test_interface_set_trunk_native_vlan_then_set_members_after(self): int003 = result.xpath("data/configuration/interfaces/interface")[0] assert_that(int003.xpath("native-vlan-id")[0].text, equal_to("2995")) + assert_that(int003.xpath("unit/family/ethernet-switching/vlan/members"), has_length(2)) + assert_that(int003.xpath("unit/family/ethernet-switching/vlan/members")[0].text, equal_to("2996")) + assert_that(int003.xpath("unit/family/ethernet-switching/vlan/members")[1].text, equal_to("2997")) self.cleanup(vlan("VLAN2995"), vlan("VLAN2996"), vlan("VLAN2997"), interface("ge-0/0/3", [self.PORT_MODE_TAG, "vlan"])) @@ -534,30 +595,12 @@ def test_passing_from_trunk_mode_to_access_gets_rid_of_stuff_in_trunk_mode(self) "interfaces": { "interface": [ {"name": "ge-0/0/3"}, + {"native-vlan-id": "1200"}, {"unit": [ {"name": "0"}, {"family": { "ethernet-switching": { - self.PORT_MODE_TAG: "trunk" - }}}]}]}}) - self.nc.commit() - - self.edit({ - "interfaces": { - "interface": [ - {"name": "ge-0/0/3"}, - {"native-vlan-id": "1200"} - ]}}) - self.nc.commit() - - self.edit({ - "interfaces": { - "interface": [ - {"name": "ge-0/0/3"}, - {"unit": [ - {"name": "0"}, - {"family": { - "ethernet-switching": { + self.PORT_MODE_TAG: "trunk", "vlan": [ {"members": "1100"}, {"members": "1300"}, diff --git a/tests/netconf/netconf_protocol_test.py b/tests/netconf/netconf_protocol_test.py index e91909ce..7b624023 100644 --- a/tests/netconf/netconf_protocol_test.py +++ b/tests/netconf/netconf_protocol_test.py @@ -265,10 +265,21 @@ def compare_nodes(self, actual_node, node): self.compare_children(node, actual_node) def compare_children(self, expected, actual): - for i, node in enumerate(expected): - actual_node = actual[i] + assert_that(actual, has_length(len(expected))) + tested_nodes = [] + for node in expected: + actual_node = get_children_by_unqualified_tag(unqualify(node.tag), actual, excluding=tested_nodes) self.compare_nodes(actual_node, node) + tested_nodes.append(actual_node) def unqualify(tag): return re.sub("\{[^\}]*\}", "", tag) + + +def get_children_by_unqualified_tag(tag, node, excluding): + for child in node: + if child not in excluding and unqualify(child.tag) == tag: + return child + + raise AssertionError("Missing element {} in {}".format(tag, to_xml(node, pretty_print=True)))