From d7aec8ee052468f55e2e9bd27de1dedd918c8a37 Mon Sep 17 00:00:00 2001 From: Vratko Polak Date: Tue, 3 Mar 2020 13:59:34 +0100 Subject: Backport CRC checking from master Change-Id: Iac99a7928a721e2fb7c608b92d5c1cf2ba047c51 Signed-off-by: Vratko Polak --- resources/libraries/python/Constants.py | 103 +++++++++++++++++++++++++++- resources/libraries/python/PapiExecutor.py | 7 +- resources/libraries/python/VppApiCrc.py | 106 ++++++++++++++++++++++------- resources/tools/integrated/check_crc.py | 98 +++++++++++++++----------- 4 files changed, 248 insertions(+), 66 deletions(-) diff --git a/resources/libraries/python/Constants.py b/resources/libraries/python/Constants.py index e01536b5e6..86cbefeb9e 100644 --- a/resources/libraries/python/Constants.py +++ b/resources/libraries/python/Constants.py @@ -1,4 +1,4 @@ -# Copyright (c) 2019 Cisco and/or its affiliates. +# Copyright (c) 2020 Cisco and/or its affiliates. # 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: @@ -14,6 +14,104 @@ """Constants used in CSIT.""" +import os + + +def get_str_from_env(env_var_names, default_value): + """Attempt to read string from environment variable, return that or default. + + If environment variable exists, but is empty (and default is not), + empty string is returned. + + Several environment variable names are examined, as CSIT currently supports + a mix of naming conventions. + Here "several" means there are hard coded prefixes to try, + and env_var_names itself can be single name, or a list or a tuple of names. + + :param env_var_names: Base names of environment variable to attempt to read. + :param default_value: Value to return if the env var does not exist. + :type env_var_names: str, or list of str, or tuple of str + :type default_value: str + :returns: The value read, or default value. + :rtype: str + """ + prefixes = ("FDIO_CSIT_", "CSIT_", "") + if not isinstance(env_var_names, (list, tuple)): + env_var_names = [env_var_names] + for name in env_var_names: + for prefix in prefixes: + value = os.environ.get(prefix + name, None) + if value is not None: + return value + return default_value + + +def get_int_from_env(env_var_names, default_value): + """Attempt to read int from environment variable, return that or default. + + String value is read, default is returned also if conversion fails. + + :param env_var_names: Base names of environment variable to attempt to read. + :param default_value: Value to return if read or conversion fails. + :type env_var_names: str, or list of str, or tuple of str + :type default_value: int + :returns: The value read, or default value. + :rtype: int + """ + env_str = get_str_from_env(env_var_names, "") + try: + return int(env_str) + except ValueError: + return default_value + + +def get_float_from_env(env_var_names, default_value): + """Attempt to read float from environment variable, return that or default. + + String value is read, default is returned also if conversion fails. + + :param env_var_names: Base names of environment variable to attempt to read. + :param default_value: Value to return if read or conversion fails. + :type env_var_names: str, or list of str, or tuple of str + :type default_value: float + :returns: The value read, or default value. + :rtype: float + """ + env_str = get_str_from_env(env_var_names, "") + try: + return float(env_str) + except ValueError: + return default_value + + +def get_pessimistic_bool_from_env(env_var_names): + """Attempt to read bool from environment variable, assume False by default. + + Conversion is lenient and pessimistic, only few strings are considered true. + + :param env_var_names: Base names of environment variable to attempt to read. + :type env_var_names: str, or list of str, or tuple of str + :returns: The value read, or False. + :rtype: bool + """ + env_str = get_str_from_env(env_var_names, "").lower() + return bool(env_str in ("true", "yes", "y", "1")) + + +def get_optimistic_bool_from_env(env_var_names): + """Attempt to read bool from environment variable, assume True by default. + + Conversion is lenient and optimistic, only few strings are considered false. + + :param env_var_names: Base names of environment variable to attempt to read. + :type env_var_names: str, or list of str, or tuple of str + :returns: The value read, or True. + :rtype: bool + """ + env_str = get_str_from_env(env_var_names, "").lower() + return bool(env_str not in ("false", "no", "n", "0")) + + class Constants(object): """Constants used in CSIT. @@ -109,6 +207,9 @@ class Constants(object): # Equivalent to ~0 used in vpp code BITWISE_NON_ZERO = 0xffffffff + # Global "kill switch" for CRC checking during runtime. + FAIL_ON_CRC_MISMATCH = get_pessimistic_bool_from_env("FAIL_ON_CRC_MISMATCH") + # Mapping from NIC name to its bps limit. # TODO: Implement logic to lower limits to TG NIC or software. Or PCI. NIC_NAME_TO_LIMIT = { diff --git a/resources/libraries/python/PapiExecutor.py b/resources/libraries/python/PapiExecutor.py index adafa88fa0..7ff7e8cee5 100644 --- a/resources/libraries/python/PapiExecutor.py +++ b/resources/libraries/python/PapiExecutor.py @@ -1,4 +1,4 @@ -# Copyright (c) 2019 Cisco and/or its affiliates. +# Copyright (c) 2020 Cisco and/or its affiliates. # 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: @@ -15,6 +15,7 @@ """ import binascii +import copy import glob import json import shutil @@ -354,11 +355,13 @@ class PapiSocketExecutor(object): :rtype: PapiSocketExecutor :raises RuntimeError: If unverified or conflicting CRC is encountered. """ + self.crc_checker_instance.report_initial_conflicts() if history: PapiHistory.add_to_papi_history( self._node, csit_papi_command, **kwargs) + self.crc_checker_instance.check_api_name(csit_papi_command) self._api_command_list.append( - dict(api_name=csit_papi_command, api_args=kwargs)) + dict(api_name=csit_papi_command, api_args=copy.deepcopy(kwargs))) return self def get_replies(self, err_msg="Failed to get replies."): diff --git a/resources/libraries/python/VppApiCrc.py b/resources/libraries/python/VppApiCrc.py index 0d4e8f4f0c..90131a6960 100644 --- a/resources/libraries/python/VppApiCrc.py +++ b/resources/libraries/python/VppApiCrc.py @@ -1,4 +1,4 @@ -# Copyright (c) 2019 Cisco and/or its affiliates. +# Copyright (c) 2020 Cisco and/or its affiliates. # 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: @@ -19,6 +19,8 @@ import yaml from robot.api import logger +from resources.libraries.python.Constants import Constants + def _str(text): """Convert from possible unicode without interpreting as number. @@ -42,7 +44,8 @@ class VppApiCrcChecker(object): so make sure the calling libraries have appropriate robot library scope. For usual testing, it means "GLOBAL" scope.""" - def __init__(self, directory): + def __init__( + self, directory, fail_on_mismatch=Constants.FAIL_ON_CRC_MISMATCH): """Initialize empty state, then register known collections. This also scans directory for .api.json files @@ -52,22 +55,27 @@ class VppApiCrcChecker(object): :type directory: str """ + self.fail_on_mismatch = fail_on_mismatch + """If True, mismatch leads to test failure, by raising exception. + If False, the mismatch is logged, but the test is allowed to continue. + """ + self._expected = dict() """Mapping from collection name to mapping from API name to CRC string. - Colection name should be something useful for logging. + Collection name should be something useful for logging. - Order of addition reflects the order colections should be queried. + Order of addition reflects the order collections should be queried. If an incompatible CRC is found, affected collections are removed. A CRC that would remove all does not, added to _reported instead, - while causing a failure in single test.""" + while causing a failure in single test (if fail_on_mismatch).""" self._missing = dict() """Mapping from collection name to mapping from API name to CRC string. Starts the same as _expected, but each time an encountered api,crc pair - fits the expectation, the pair is removed from this mapping. - Ideally, the active mappings will become empty. + fits the expectation, the pair is removed from all collections + within this mapping. Ideally, the active mappings will become empty. If not, it is an error, VPP removed or renamed a message CSIT needs.""" self._found = dict() @@ -89,6 +97,17 @@ class VppApiCrcChecker(object): self._register_all() self._check_dir(directory) + def log_and_raise(self, exc_msg): + """Log to console, on fail_on_mismatch also raise runtime exception. + + :param exc_msg: The message to include in log or exception. + :type exc_msg: str + :raises RuntimeError: With the message, if fail_on_mismatch. + """ + logger.console("RuntimeError:\n{m}".format(m=exc_msg)) + if self.fail_on_mismatch: + raise RuntimeError(exc_msg) + def _register_collection(self, collection_name, name_to_crc_mapping): """Add a named (copy of) collection of CRCs. @@ -96,11 +115,13 @@ class VppApiCrcChecker(object): :param name_to_crc_mapping: Mapping from API names to CRCs. :type collection_name: str or unicode :type name_to_crc_mapping: dict from str/unicode to str/unicode + :raises RuntimeError: If the name of a collection is registered already. """ collection_name = _str(collection_name) if collection_name in self._expected: raise RuntimeError("Collection {cl!r} already registered.".format( - cl=collection_name)) + cl=collection_name) + ) mapping = {_str(k): _str(v) for k, v in name_to_crc_mapping.items()} self._expected[collection_name] = mapping self._missing[collection_name] = mapping.copy() @@ -131,7 +152,8 @@ class VppApiCrcChecker(object): continue return _str(item) raise RuntimeError("No name found for message: {obj!r}".format( - obj=msg_obj)) + obj=msg_obj) + ) @staticmethod def _get_crc(msg_obj): @@ -150,11 +172,14 @@ class VppApiCrcChecker(object): if crc: return _str(crc) raise RuntimeError("No CRC found for message: {obj!r}".format( - obj=msg_obj)) + obj=msg_obj) + ) def _process_crc(self, api_name, crc): """Compare API to verified collections, update class state. + Here, API stands for (message name, CRC) pair. + Conflict is NOT when a collection does not recognize the API. Such APIs are merely added to _found for later reporting. Conflict is when a collection recognizes the API under a different CRC. @@ -199,7 +224,7 @@ class VppApiCrcChecker(object): self._expected = new_expected self._missing = {name: self._missing[name] for name in new_expected} return - # No new_expected means some colections knew the api_name, + # No new_expected means some collections knew the api_name, # but CRC does not match any. This has to be reported. self._reported[api_name] = crc @@ -207,7 +232,7 @@ class VppApiCrcChecker(object): """Parse every .api.json found under directory, remember conflicts. As several collections are supported, each conflict invalidates - one of them, failure happens only when no collections would be left. + some of them, failure happens only when no collections would be left. In that case, set of collections just before the failure is preserved, the _reported mapping is filled with conflicting APIs. The _found mapping is filled with discovered api names and crcs. @@ -229,7 +254,8 @@ class VppApiCrcChecker(object): msg_crc = self._get_crc(msg_obj) self._process_crc(msg_name, msg_crc) logger.debug("Surviving collections: {col!r}".format( - col=self._expected.keys())) + col=self._expected.keys()) + ) def report_initial_conflicts(self, report_missing=False): """Report issues discovered by _check_dir, if not done that already. @@ -241,31 +267,49 @@ class VppApiCrcChecker(object): Missing reporting is disabled by default, because some messages come from plugins that might not be enabled at runtime. + After the report, clear _reported, so that test cases report them again, + thus tracking which message is actually used (by which test). + :param report_missing: Whether to raise on missing messages. :type report_missing: bool - :raises RuntimeError: If CRC mismatch or missing messages are detected. + :raises RuntimeError: If CRC mismatch or missing messages are detected, + and fail_on_mismatch is True. """ if self._initial_conflicts_reported: return self._initial_conflicts_reported = True if self._reported: - raise RuntimeError("Dir check found incompatible API CRCs: {rep!r}"\ - .format(rep=self._reported)) + + reported_indented = json.dumps( + self._reported, indent=1, sort_keys=True, + separators=[",", ":"] + ) + self._reported = dict() + self.log_and_raise( + "Incompatible API CRCs found in .api.json files:\n" + "{r_i}".format(r_i=reported_indented) + ) if not report_missing: return missing = {name: mapp for name, mapp in self._missing.items() if mapp} if missing: - raise RuntimeError("Dir check found missing API CRCs: {mis!r}"\ - .format(mis=missing)) + missing_indented = json.dumps( + missing, indent=1, sort_keys=True, separators=[",", ":"]) + self.log_and_raise( + "API CRCs missing from .api.json:\n" + "{m_i}".format(m_i=missing_indented) + ) def check_api_name(self, api_name): - """Fail if the api_name has no known CRC associated. + """Fail if the api_name has no, or different from known CRC associated. Do not fail if this particular failure has been already reported. - Intended use: Call everytime an API call is queued or response received. + Intended use: Call during test (not in initialization), + every time an API call is queued or response received. + - :param api_name: VPP API messagee name to check. + :param api_name: VPP API message name to check. :type api_name: str or unicode :raises RuntimeError: If no verified CRC for the api_name is found. """ @@ -282,9 +326,21 @@ class VppApiCrcChecker(object): if new_expected: # Some collections recognized the message name. self._expected = new_expected - return crc = self._found.get(api_name, None) + matching = False + if crc is not None: + # Regardless of how many collections are remaining, + # verify the known CRC is on one of them. + for col, name_to_crc_mapping in self._expected.items(): + if api_name not in name_to_crc_mapping: + continue + if name_to_crc_mapping[api_name] == crc: + matching = True + break + if matching: + return self._reported[api_name] = crc - # Disabled temporarily during CRC mismatch. - #raise RuntimeError("No active collection has API {api!r}" - # " CRC found {crc!r}".format(api=api_name, crc=crc)) + self.log_and_raise( + "No active collection contains API {a_n!r} with CRC {crc!r}".format( + a_n=api_name, crc=crc) + ) diff --git a/resources/tools/integrated/check_crc.py b/resources/tools/integrated/check_crc.py index 3d5c30a6d6..7930a534ca 100644 --- a/resources/tools/integrated/check_crc.py +++ b/resources/tools/integrated/check_crc.py @@ -1,4 +1,4 @@ -# Copyright (c) 2019 Cisco and/or its affiliates. +# Copyright (c) 2020 Cisco and/or its affiliates. # 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: @@ -19,46 +19,68 @@ No executable flag, nor shebang, as most users do not have .api.json files downloaded to the correct place. """ +from __future__ import print_function import os.path as op import sys from resources.libraries.python.VppApiCrc import VppApiCrcChecker -# TODO: Read FDIO_VPP_DIR environment variable, or some other input, -# instead of using hardcoded relative path? -API_DIR = op.normpath(op.join( - op.dirname(op.abspath(__file__)), "..", "..", "..", "..", - "build-root", "install-vpp-native", "vpp", "share", "vpp", "api")) -CHECKER = VppApiCrcChecker(API_DIR) -try: - CHECKER.report_initial_conflicts(report_missing=True) -except RuntimeError as err: - sys.stderr.write("{err!r}\n".format(err=err)) - sys.stderr.write( - "\n" - "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@\n" - "\n" - "VPP CSIT API CHECK FAIL!\n" - "\n" - "This means the patch under test has missing messages,\n" - "or messages with unexpected CRCs compared to what CSIT needs.\n" - "Either this Change and/or its ancestors were editing .api files,\n" - "or your chain is not rebased upon the recent enough VPP codebase.\n" - "\n" - "Please rebase the patch to see if that fixes the problem.\n" - "If that fails email csit-dev@lists.fd.io for a new\n" - "operational branch supporting the api changes.\n" - "\n" - "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@\n" - ) - sys.exit(1) -else: - sys.stderr.write( - "\n" - "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@\n" - "\n" - "VPP CSIT API CHECK PASS!\n" - "\n" - "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@\n" - ) +def main(): + """Execute the logic, return the return code. + + From current location, construct path to .api file subtree, + initialize and run the CRC checker, print result consequences + to stderr, return the return code to return from the script. + + :returns: Return code to return. 0 if OK, 1 if CRC mismatch. + :rtype: int + """ + + # TODO: Read FDIO_VPP_DIR environment variable, or some other input, + # instead of using hardcoded relative path? + + api_dir = op.normpath(op.join( + op.dirname(op.abspath(__file__)), "..", "..", "..", "..", + "build-root", "install-vpp-native", "vpp", "share", "vpp", + "api" + )) + checker = VppApiCrcChecker(api_dir) + try: + checker.report_initial_conflicts(report_missing=True) + except RuntimeError as err: + stderr_lines = [ + "{err!r}".format(err=err), + "", + "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@", + "", + "VPP CSIT API CHECK FAIL!", + "", + "This means the patch under test has missing messages,", + "or messages with unexpected CRCs compared to what CSIT needs.", + "Either this Change and/or its ancestors were editing .api files,", + "or your chain is not rebased upon a recent enough VPP codebase.", + "", + "Please rebase the patch to see if that fixes the problem.", + "If that fails email csit-dev@lists.fd.io for a new", + "operational branch supporting the api changes.", + "", + "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@", + ] + ret_code = 1 + else: + stderr_lines = [ + "", + "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@", + "", + "VPP CSIT API CHECK PASS!", + "", + "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@", + ] + ret_code = 0 + for stderr_line in stderr_lines: + print(stderr_line, file=sys.stderr) + return ret_code + +if __name__ == "__main__": + sys.exit(main()) -- cgit 1.2.3-korg