Commit b9c0b131 authored by Waleed Akbar's avatar Waleed Akbar
Browse files

Enhance gRPC error handling and normalize driver results in service handlers

- Introduced truncate_grpc_details function to cap gRPC error messages.
- Updated safe_and_metered_rpc_method to use truncated error details.
- Normalized driver results in configure_rules and deconfigure_rules functions.
- Added checks for devices not managed by any controller in service handlers.
- Created a new YANG model for Huawei NCE application-flow API.
parent 951c2cee
Loading
Loading
Loading
Loading
+21 −4
Changes for src/common/method_wrappers/Decorator.py: 21 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -209,6 +209,23 @@ def metered_subclass_method(metrics_pool : MetricsPool):
        return inner_wrapper
    return outer_wrapper

MAX_GRPC_DETAILS_LENGTH = 4096

def truncate_grpc_details(details : str) -> str:
    """Cap an error string before it is sent as a gRPC status.

    gRPC carries error details in trailing metadata, which is size-limited (~8KB by
    default). Oversized details do not simply get trimmed: the call fails at the client
    with RESOURCE_EXHAUSTED("received initial metadata size exceeds limit") and the real
    error is lost. Callers log the full text before aborting, so truncating here costs
    nothing and keeps the cause visible to the caller.
    """
    details = str(details)
    if len(details) <= MAX_GRPC_DETAILS_LENGTH: return details
    MSG = '... [truncated {:d} of {:d} characters; see server logs for the full error]'
    return details[:MAX_GRPC_DETAILS_LENGTH] + MSG.format(
        len(details) - MAX_GRPC_DETAILS_LENGTH, len(details))

def safe_and_metered_rpc_method(metrics_pool : MetricsPool, logger : logging.Logger):
    def outer_wrapper(func):
        method_name = func.__name__
@@ -231,11 +248,11 @@ def safe_and_metered_rpc_method(metrics_pool : MetricsPool, logger : logging.Log
                    counter_failed.inc()
                else:
                    counter_completed.inc()
                grpc_context.abort(e.code, e.details)
                grpc_context.abort(e.code, truncate_grpc_details(e.details))
            except Exception as e:          # pragma: no cover, pylint: disable=broad-except
                logger.exception('{:s} exception'.format(method_name))
                counter_failed.inc()
                grpc_context.abort(grpc.StatusCode.INTERNAL, str(e))
                grpc_context.abort(grpc.StatusCode.INTERNAL, truncate_grpc_details(str(e)))
        return inner_wrapper
    return outer_wrapper

@@ -260,11 +277,11 @@ def safe_and_metered_rpc_method_async(metrics_pool: MetricsPool, logger: logging
                    counter_failed.inc()
                else:
                    counter_completed.inc()
                await grpc_context.abort(e.code, e.details)
                await grpc_context.abort(e.code, truncate_grpc_details(e.details))
            except Exception as e:  # pragma: no cover, pylint: disable=broad-except
                logger.exception('{:s} exception'.format(method_name))
                counter_failed.inc()
                await grpc_context.abort(grpc.StatusCode.INTERNAL, str(e))
                await grpc_context.abort(grpc.StatusCode.INTERNAL, truncate_grpc_details(str(e)))

        return inner_wrapper

+59 −11
Changes for src/device/service/Tools.py: 59 added lines, 11 removed lines.
Original line number Diff line number Diff line
@@ -475,29 +475,77 @@ def compute_rules_to_add_delete(

    return resources_to_set, resources_to_delete

def configure_rules(device : Device, driver : _Driver, resources_to_set : List[Tuple[str, Any]]) -> List[str]:
    if len(resources_to_set) == 0: return []
def _normalize_driver_results(
    resources : List[Tuple[str, Any]], driver_results : List[Any], device_uuid : str, operation : str
) -> List[Tuple[str, Union[Any, Exception, None]]]:
    """Normalize SetConfig/DeleteConfig results into (resource_key, value) pairs.

    Drivers report outcomes using one of two conventions:

    * bare values aligned positionally with `resources` (e.g. EmulatedDriver), or
    * (resource_key, value) tuples, which may omit resources the driver skipped
      (e.g. IetfSliceDriver, NCEDriver, IetfL3VpnDriver, IetfL2VpnDriver,
      OpticalTfsDriver).

    Both are accepted, so a driver using either convention has its failures reported
    correctly instead of silently discarded. Note that GetConfig legitimately uses the
    tuple convention and does not go through here.

    Returns one pair per requested resource: an Exception on failure, the requested
    resource_value on success, or None when the driver did not report on that resource
    (which _raw_config_rules_to_grpc ignores).
    """
    is_keyed = len(driver_results) > 0 and all(
        isinstance(result, tuple) and len(result) == 2 and isinstance(result[0], str)
        for result in driver_results
    )

    results_setconfig = driver.SetConfig(resources_to_set)
    results_setconfig = [
    if is_keyed:
        # Match by resource key; the same key may legitimately appear more than once.
        results_by_key : Dict[str, List[Any]] = dict()
        for resource_key, result in driver_results:
            results_by_key.setdefault(resource_key, list()).append(result)

        normalized : List[Tuple[str, Any]] = list()
        for resource_key, resource_value in resources:
            pending = results_by_key.get(resource_key)
            if not pending:
                # Driver did not report on this resource; do not invent a success.
                normalized.append((resource_key, None))
                continue
            result = pending.pop(0)
            normalized.append(
                (resource_key, result if isinstance(result, Exception) else resource_value))
        return normalized

    if len(driver_results) != len(resources):
        MSG = '[{:s}] Device({:s}): driver reported {:d} result(s) for {:d} resource(s); ' \
              'results cannot be matched reliably'
        LOGGER.warning(MSG.format(
            str(operation), str(device_uuid), len(driver_results), len(resources)))

    return [
        (resource_key, result if isinstance(result, Exception) else resource_value)
        for (resource_key, resource_value), result in zip(resources_to_set, results_setconfig)
        for (resource_key, resource_value), result in zip(resources, driver_results)
    ]

def configure_rules(device : Device, driver : _Driver, resources_to_set : List[Tuple[str, Any]]) -> List[str]:
    if len(resources_to_set) == 0: return []

    device_uuid = device.device_id.device_uuid.uuid
    results_setconfig = _normalize_driver_results(
        resources_to_set, driver.SetConfig(resources_to_set), device_uuid, 'SetConfig')

    return _raw_config_rules_to_grpc(
        device_uuid, device.device_config, ERROR_SET, ConfigActionEnum.CONFIGACTION_SET, results_setconfig)

def deconfigure_rules(device : Device, driver : _Driver, resources_to_delete : List[Tuple[str, Any]]) -> List[str]:
    if len(resources_to_delete) == 0: return []

    results_deleteconfig = driver.DeleteConfig(resources_to_delete)
    results_deleteconfig = [
        (resource_key, result if isinstance(result, Exception) else resource_value)
        for (resource_key, resource_value), result in zip(resources_to_delete, results_deleteconfig)
    ]

    device_uuid = device.device_id.device_uuid.uuid
    results_deleteconfig = _normalize_driver_results(
        resources_to_delete, driver.DeleteConfig(resources_to_delete), device_uuid, 'DeleteConfig')

    return _raw_config_rules_to_grpc(
        device_uuid, device.device_config, ERROR_DELETE, ConfigActionEnum.CONFIGACTION_DELETE, results_deleteconfig)

+19 −0
Changes for src/device/service/drivers/ietf_l3vpn/TfsApiClient.py: 19 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ GET_CONTEXT_IDS_URL = '/tfs-api/context_ids'
GET_DEVICES_URL     = '/tfs-api/devices'
GET_LINKS_URL       = '/tfs-api/links'
TOPOLOGY_URL        = '/tfs-api/context/{context_uuid:s}/topology_details/{topology_uuid:s}'
GET_DEVICE_URL      = '/tfs-api/device/{device_uuid:s}'


IETF_L3VPN_ALL_URL  = '/restconf/data/ietf-l3vpn-svc:l3vpn-svc/vpn-services'
@@ -144,6 +145,24 @@ class TfsApiClient(RestApiClient):
                .get('device_config', dict())
                .get('config_rules', list())
            )

            if len(config_rule_list) == 0:
                # The topology_details endpoint does not return device_config, so the
                # endpoint settings (IP address, prefix, VLAN tag, ...) of the remote
                # devices would be lost on import. Retrieve them per-device instead;
                # this is skipped when the topology response already carries them.
                try:
                    json_device_details = self.get(
                        GET_DEVICE_URL.format(device_uuid=device_uuid))
                    config_rule_list = (
                        json_device_details
                        .get('device_config', dict())
                        .get('config_rules', list())
                    )
                except Exception:  # pylint: disable=broad-except
                    MSG = '[get_devices_endpoints] unable to retrieve config rules of Device({:s})'
                    LOGGER.warning(MSG.format(str(device_uuid)))

            config_rule_dict : Dict[str, Dict] = dict()
            for cr in config_rule_list:
                if cr['action'] != 'CONFIGACTION_SET': continue
+13 −17
Changes for src/service/service/service_handlers/l3nm_ietfl3vpn/L3NM_IETFL3VPN_ServiceHandler.py: 13 added lines, 17 removed lines.
Original line number Diff line number Diff line
@@ -223,6 +223,10 @@ class L3NM_IETFL3VPN_ServiceHandler(_ServiceHandler):
                DeviceId(**json_device_id(device_uuid))
            )
            device_controller = self.__task_executor.get_device_controller(device_obj)
            # get_device_controller() returns None for devices not managed by any
            # controller (e.g. local packet-pop nodes). Those are never IP transport
            # edges, so skip them instead of dereferencing None.
            if device_controller is None: continue
            if device_controller.device_type in PACKET_SDN_CONTROLLERS:
                src_device_uuid, src_endpoint_uuid = device_uuid, endpoint_uuid
                src_device_controller = device_controller
@@ -237,6 +241,10 @@ class L3NM_IETFL3VPN_ServiceHandler(_ServiceHandler):
                DeviceId(**json_device_id(device_uuid))
            )
            device_controller = self.__task_executor.get_device_controller(device_obj)
            # get_device_controller() returns None for devices not managed by any
            # controller (e.g. local packet-pop nodes). Those are never IP transport
            # edges, so skip them instead of dereferencing None.
            if device_controller is None: continue
            if device_controller.device_type in PACKET_SDN_CONTROLLERS:
                dst_device_uuid, dst_endpoint_uuid = device_uuid, endpoint_uuid
                dst_device_controller = device_controller
@@ -462,23 +470,11 @@ class L3NM_IETFL3VPN_ServiceHandler(_ServiceHandler):
        ][0]["id"]
        results = []
        try:
            src_device_uuid, _ = get_device_endpoint_uuids(endpoints[0])
            src_device = self.__task_executor.get_device(
                DeviceId(**json_device_id(src_device_uuid))
            )
            src_controller = self.__task_executor.get_device_controller(src_device)

            dst_device_uuid, _ = get_device_endpoint_uuids(endpoints[1])
            dst_device = self.__task_executor.get_device(
                DeviceId(**json_device_id(dst_device_uuid))
            )
            dst_controller = self.__task_executor.get_device_controller(dst_device)
            if (
                src_controller.device_id.device_uuid.uuid
                != dst_controller.device_id.device_uuid.uuid
            ):
                raise Exception("Different Src-Dst devices not supported by now")
            controller = src_controller
            # Mirror SetEndpoint: locate the IP transport edges by scanning for
            # controller-managed devices rather than assuming endpoints[0]/[1] are the
            # edges. This also tolerates path hops with no controller, and already
            # enforces that both edges share the same controller.
            (_, _, _, _, controller) = self.__find_IP_transport_edge_endpoints(endpoints)
            json_config_rules = teardown_config_rules(service_id)
            del controller.device_config.config_rules[:]
            for jcr in json_config_rules:
+6 −0
Changes for src/service/service/service_handlers/l3nm_ietfslice/L3NM_IETFSlice_ServiceHandler.py: 6 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -71,6 +71,9 @@ class L3NM_IETFSlice_ServiceHandler(_ServiceHandler):


            # 2. Identify controller to be used
            if src_controller is None or dst_controller is None:
                MSG = 'Device({:s}) and/or Device({:s}) are not managed by any controller'
                raise Exception(MSG.format(str(src_device_name), str(dst_device_name)))
            if src_controller.device_id.device_uuid.uuid != dst_controller.device_id.device_uuid.uuid:
                raise Exception('Different Src-Dst devices not supported by now')
            controller = src_controller  # same device controller
@@ -187,6 +190,9 @@ class L3NM_IETFSlice_ServiceHandler(_ServiceHandler):
                DeviceId(**json_device_id(src_device_uuid))
            )
            controller = self.__task_executor.get_device_controller(src_device_obj)
            if controller is None:
                MSG = 'Device({:s}) is not managed by any controller'
                raise Exception(MSG.format(str(src_device_uuid)))

            datastore_delta = DataStoreDelta(self.__service)
            running_slice = datastore_delta.running_data
Loading