[juniper] Removing last trunk member errors out

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,18 @@ 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")
 
+    def _validate(self, 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 +502,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)

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,11 @@ 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")
+
+    def _validate(self, 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 super(JuniperQfxCopperNetconfDatastore, self)._validate(configuration)

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

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)

setup.cfg

@@ -24,6 +24,7 @@ packages =
 tests = tests
 no-path-adjustment = 1
 logging-level=INFO
+verbosity = 2
 
 [wheel]
 universal = 1

tests/juniper/juniper_base_protocol_test.py

@@ -340,6 +340,38 @@ 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()
+
+        self.cleanup(vlan("VLAN2995"), vlan("VLAN2996"), vlan("VLAN2997"),
+                     interface("ge-0/0/3", [self.PORT_MODE_TAG]))
+        result = self.nc.get_config(source="running", filter=dict_2_etree({"filter": {
+            "configuration": {"vlans": {}}}
+        }))
+        assert_that(result.xpath("data/configuration/vlans/vlan"), has_length(0))
+
     def test_trunk_mode(self):
         self.edit({
             "vlans": [
@@ -404,6 +436,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 +935,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": {