commit fe1e0a48f43f8c5a4b82238c84c733f9943c55c6 Author: Wiktor Sleczka Date: Mon Aug 31 09:31:31 2020 +0200 Update to error handling Added FabricError Exception type and change how few elements handle errors, eg missing fabric elements not result in error Change-Id: Ibf2c8949ef971f748f71f0d10322f53490f493dc (cherry picked from commit dfdfc93979781083e6f496b53dc00a81cb6bedd6) diff --git a/networking_opencontrail/exceptions.py b/networking_opencontrail/exceptions.py new file mode 100644 index 0000000..04b3cec --- /dev/null +++ b/networking_opencontrail/exceptions.py @@ -0,0 +1,34 @@ +# Copyright (c) 2016 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + + +class FabricError(Exception): + pass + + +class ResourceNotFound(FabricError): + @classmethod + def create(cls, type_name, resource_name): + """Alternative constructor creating exception with standard message""" + msg = ( + "Object of {type} '{resource}' was not present " + "in Contrail database" + .format(type=type_name, resource=resource_name) + ) + return cls(msg) + + +class InvalidResource(FabricError): + pass diff --git a/networking_opencontrail/ml2/mech_driver.py b/networking_opencontrail/ml2/mech_driver.py index eac79a4..b4b2afe 100644 --- a/networking_opencontrail/ml2/mech_driver.py +++ b/networking_opencontrail/ml2/mech_driver.py @@ -18,6 +18,7 @@ from oslo_log import log as logging from neutron_lib.plugins.ml2 import api from networking_opencontrail.constants import NTF_SYNC_LOCK_NAME +from networking_opencontrail.exceptions import InvalidResource from networking_opencontrail import options from networking_opencontrail import repository from networking_opencontrail.sync import worker @@ -93,13 +94,11 @@ class OpenContrailMechDriver(api.MechanismDriver): q_port = context.current q_network = context.network.current - vpg = repository.vpg.create(q_port, q_network) - if vpg is None: - LOG.error("VPG for port {} could not be created.".format( - q_port["id"] - )) - return - repository.vmi.create(q_port, q_network) + try: + repository.vpg.create(q_port, q_network) + repository.vmi.create(q_port, q_network) + except InvalidResource as e: + LOG.info("Port ommited by plugin, cause: {}".format(str(e))) @lockutils.synchronized(NTF_SYNC_LOCK_NAME, external=True) def update_port_postcommit(self, context): @@ -108,16 +107,15 @@ class OpenContrailMechDriver(api.MechanismDriver): old_q_port = context.original q_network = context.network.current - repository.vmi.delete(old_q_port, q_network, context._plugin_context) - repository.vpg.delete(old_q_port, q_network) + try: + repository.vmi.delete( + old_q_port, q_network, context._plugin_context) + repository.vpg.delete(old_q_port, q_network) - vpg = repository.vpg.create(q_port, q_network) - if vpg is None: - LOG.error("VPG for port {} could not be created.".format( - q_port["id"] - )) - return - repository.vmi.create(q_port, q_network) + repository.vpg.create(q_port, q_network) + repository.vmi.create(q_port, q_network) + except InvalidResource as e: + LOG.info("Port ommited by plugin, cause: {}".format(str(e))) @lockutils.synchronized(NTF_SYNC_LOCK_NAME, external=True) def delete_port_postcommit(self, context): @@ -125,8 +123,11 @@ class OpenContrailMechDriver(api.MechanismDriver): q_port = context.current q_network = context.network.current - repository.vmi.delete(q_port, q_network, context._plugin_context) - repository.vpg.delete(q_port, q_network) + try: + repository.vmi.delete(q_port, q_network, context._plugin_context) + repository.vpg.delete(q_port, q_network) + except InvalidResource as e: + LOG.info("Port ommited by plugin, cause: {}".format(str(e))) def get_workers(self): return [ diff --git a/networking_opencontrail/repository/utils/utils.py b/networking_opencontrail/repository/utils/utils.py index 81be85c..d2b5feb 100644 --- a/networking_opencontrail/repository/utils/utils.py +++ b/networking_opencontrail/repository/utils/utils.py @@ -17,6 +17,7 @@ import uuid from oslo_log import log as logging +from networking_opencontrail.exceptions import ResourceNotFound from networking_opencontrail.repository.utils.client import tf_client from networking_opencontrail.repository.utils.tagger import is_data_port from networking_opencontrail.repository.utils.tagger import is_management_port @@ -36,6 +37,9 @@ def request_node(name): name ] node = tf_client.read_node(fq_name=node_fq_name) + if node is None: + raise ResourceNotFound.create('node', name) + return node @@ -107,14 +111,20 @@ def request_fabric_from_physical_interface(physical_interface): def request_fabric_from_node(node): ports = request_ports_from_node(node) if not ports: - return None + raise ResourceNotFound( + "No ports attached to node {}".format(node.name)) port = ports[0] physical_interfaces = request_physical_interfaces_from_port(port) if not physical_interfaces: - return None + raise ResourceNotFound( + "No physical interfaces attached to port {}".format(node.name)) physical_interface = physical_interfaces[0] fabric = request_fabric_from_physical_interface(physical_interface) + if fabric is None: + raise ResourceNotFound( + "No fabric could be determined for node {}".format(node.name)) + return fabric diff --git a/networking_opencontrail/repository/vmi.py b/networking_opencontrail/repository/vmi.py index 364a1e3..c8eecb6 100644 --- a/networking_opencontrail/repository/vmi.py +++ b/networking_opencontrail/repository/vmi.py @@ -14,6 +14,7 @@ # import logging +from networking_opencontrail.exceptions import InvalidResource from networking_opencontrail.neutron import neutron_client from networking_opencontrail.repository.utils.client import tf_client from networking_opencontrail.repository.utils.initialize import reconnect @@ -36,12 +37,7 @@ REQUIRED_PORT_FIELDS = [ @reconnect def create(q_port, q_network): - try: - resources.vmi.validate(q_port, q_network) - except ValueError as e: - LOG.debug("Did not create a VMI for port %s", q_port["id"]) - LOG.debug(e) - return + resources.vmi.validate(q_port, q_network) project = request_project(q_network) node_name = q_port['binding:host_id'] @@ -66,12 +62,7 @@ def create(q_port, q_network): @reconnect def delete(q_port, q_network, context): - try: - resources.vmi.validate(q_port, q_network) - except ValueError as e: - LOG.debug("Did not delete a VMI for port %s", q_port["id"]) - LOG.debug(e) - return + resources.vmi.validate(q_port, q_network) network_uuid = q_network['id'] node_name = q_port['binding:host_id'] @@ -132,7 +123,7 @@ def detach_from_vpg(vmi): def _is_managed_by_tf(q_port, q_network): try: resources.vmi.validate(q_port, q_network) - except ValueError: + except InvalidResource: return False return True diff --git a/networking_opencontrail/repository/vpg.py b/networking_opencontrail/repository/vpg.py index e2ccc91..82ff28b 100644 --- a/networking_opencontrail/repository/vpg.py +++ b/networking_opencontrail/repository/vpg.py @@ -15,6 +15,7 @@ from oslo_log import log as logging +from networking_opencontrail.exceptions import ResourceNotFound from networking_opencontrail.repository.utils.client import tf_client from networking_opencontrail.repository.utils.initialize import reconnect from networking_opencontrail.repository.utils import tagger @@ -30,15 +31,9 @@ PHYSICAL_NETWORK = 'provider:physical_network' @reconnect def create(q_port, q_network): - try: - resources.vmi.validate(q_port, q_network) - except ValueError as e: - LOG.debug("Did not create a VPG for port %s", q_port["id"]) - LOG.debug(e) - return + resources.vmi.validate(q_port, q_network) node = utils.request_node(q_port['binding:host_id']) - if resources.utils.is_sriov_node(node): physical_network = q_network[PHYSICAL_NETWORK] vpg = create_for_physical_network(node, physical_network) @@ -52,12 +47,7 @@ def create(q_port, q_network): @reconnect def delete(q_port, q_network): - try: - resources.vmi.validate(q_port, q_network) - except ValueError as e: - LOG.debug("Did not delete a VPG for port %s", q_port["id"]) - LOG.debug(e) - return + resources.vmi.validate(q_port, q_network) node = utils.request_node(q_port['binding:host_id']) if resources.utils.is_sriov_node(node): @@ -111,7 +101,7 @@ def create_for_node(node): fabric = utils.request_fabric_from_node(node) if not fabric: - raise Exception("Couldn't find fabric for VPG") + raise ResourceNotFound("Couldn't find fabric for VPG") vpg = resources.vpg.create(node, fabric) tf_client.create_vpg(vpg) @@ -133,7 +123,7 @@ def create_for_physical_network(node, network_name): fabric = utils.request_fabric_from_node(node) if not fabric: - raise Exception("Couldn't find fabric for VPG") + raise ResourceNotFound("Couldn't find fabric for VPG") vpg = resources.vpg.create(node, fabric, network_name) tf_client.create_vpg(vpg) diff --git a/networking_opencontrail/resources/vmi.py b/networking_opencontrail/resources/vmi.py index 00f658b..e62815c 100644 --- a/networking_opencontrail/resources/vmi.py +++ b/networking_opencontrail/resources/vmi.py @@ -15,6 +15,7 @@ from neutron_lib import constants from vnc_api import vnc_api +from networking_opencontrail.exceptions import InvalidResource from networking_opencontrail.resources.utils import destandardize_name from networking_opencontrail.resources.utils import first from networking_opencontrail.resources.utils import make_uuid @@ -48,15 +49,15 @@ def create(project, network, node_name, vlan_id): def validate(q_port, q_network): for field in REQUIRED_PORT_FIELDS: - if field not in q_port: - raise ValueError("No {} field in port".format(field)) + if not q_port.get(field): + raise InvalidResource("No {} field in port".format(field)) if not q_port.get("device_owner", "").startswith( constants.DEVICE_OWNER_COMPUTE_PREFIX): - raise ValueError("Invalid device_owner field value") + raise InvalidResource("Invalid device_owner field value") if not q_network.get("provider:segmentation_id"): - raise ValueError( + raise InvalidResource( "No VLAN ID set for network {}".format(q_network["name"])) @@ -113,7 +114,7 @@ def make_names_from_q_data(q_ports, q_networks): if q_network: try: validate(q_port, q_network) - except ValueError: + except InvalidResource: continue q_port_network_uuid = q_network['id'] diff --git a/networking_opencontrail/resources/vpg.py b/networking_opencontrail/resources/vpg.py index c9caeb6..9d4b18b 100644 --- a/networking_opencontrail/resources/vpg.py +++ b/networking_opencontrail/resources/vpg.py @@ -16,6 +16,7 @@ from oslo_log import log as logging from vnc_api import vnc_api +from networking_opencontrail.exceptions import InvalidResource from networking_opencontrail.resources.utils import destandardize_name from networking_opencontrail.resources.utils import first from networking_opencontrail.resources.utils import make_uuid @@ -104,7 +105,7 @@ def make_names_from_q_data(q_ports, q_networks): if q_network: try: validate(q_port, q_network) - except ValueError: + except InvalidResource: continue node_name = q_port['binding:host_id'] diff --git a/networking_opencontrail/sync/synchronizers.py b/networking_opencontrail/sync/synchronizers.py index 17eff30..975b8e0 100644 --- a/networking_opencontrail/sync/synchronizers.py +++ b/networking_opencontrail/sync/synchronizers.py @@ -96,7 +96,7 @@ class VPGSynchronizer(base.ResourceSynchronizer): def _delete_resources(self, to_delete): for vpg_name in to_delete: - vpg_uuid = utils.make_uuid(vpg_name) + vpg_uuid = resources.utils.make_uuid(vpg_name) repository.tf_client.delete_vpg(uuid=vpg_uuid) @staticmethod @@ -159,7 +159,7 @@ class VMISynchronizer(base.ResourceSynchronizer): def _delete_resources(self, to_delete): for vmi_name in to_delete: - vmi_uuid = utils.make_uuid(vmi_name) + vmi_uuid = resources.utils.make_uuid(vmi_name) vmi = repository.tf_client.read_vmi(uuid=vmi_uuid) if vmi is None: diff --git a/networking_opencontrail/tests/unit/resources/test_make_vpg_and_vmi_names_from_q_data.py b/networking_opencontrail/tests/unit/resources/test_make_vpg_and_vmi_names_from_q_data.py index d60f69a..ae11d20 100644 --- a/networking_opencontrail/tests/unit/resources/test_make_vpg_and_vmi_names_from_q_data.py +++ b/networking_opencontrail/tests/unit/resources/test_make_vpg_and_vmi_names_from_q_data.py @@ -267,6 +267,7 @@ class MakeVPGAndVMINamesFromQDataTestCase(base.TestCase): PORT_NETWORK_1_NODE_2['binding:host_id'] ), ] + self.assertItemsEqual(vpgs, expected_vpgs) self.assertItemsEqual(vmis, expected_vmis) @@ -283,6 +284,7 @@ class MakeVPGAndVMINamesFromQDataTestCase(base.TestCase): expected_vpgs = [] expected_vmis = [] + self.assertItemsEqual(vpgs, expected_vpgs) self.assertItemsEqual(vmis, expected_vmis) @@ -299,6 +301,7 @@ class MakeVPGAndVMINamesFromQDataTestCase(base.TestCase): expected_vpgs = [] expected_vmis = [] + self.assertItemsEqual(vpgs, expected_vpgs) self.assertItemsEqual(vmis, expected_vmis) diff --git a/networking_opencontrail/tests/unit/resources/test_vmi.py b/networking_opencontrail/tests/unit/resources/test_vmi.py index 5ac751e..8b0213e 100644 --- a/networking_opencontrail/tests/unit/resources/test_vmi.py +++ b/networking_opencontrail/tests/unit/resources/test_vmi.py @@ -14,13 +14,14 @@ # import ddt -from networking_opencontrail.tests import base from vnc_api import vnc_api +from networking_opencontrail.exceptions import InvalidResource from networking_opencontrail.resources.vmi import create from networking_opencontrail.resources.vmi import make_name from networking_opencontrail.resources.vmi import unzip_name from networking_opencontrail.resources.vmi import validate +from networking_opencontrail.tests import base PORT_VALID = { @@ -91,7 +92,7 @@ class VMIResourceTestCase(base.TestCase): ) @ddt.unpack def test_validate_error(self, port, network): - self.assertRaises(ValueError, validate, port, network) + self.assertRaises(InvalidResource, validate, port, network) def test_validate(self): self.assertIsNone(validate(PORT_VALID, NETWORK_VALID))