Backport CRC checking from master 54/25554/16
authorVratko Polak <vrpolak@cisco.com>
Tue, 3 Mar 2020 12:59:34 +0000 (13:59 +0100)
committerVratko Polak <vrpolak@cisco.com>
Tue, 3 Mar 2020 15:04:02 +0000 (16:04 +0100)
Change-Id: Iac99a7928a721e2fb7c608b92d5c1cf2ba047c51
Signed-off-by: Vratko Polak <vrpolak@cisco.com>
resources/libraries/python/Constants.py
resources/libraries/python/PapiExecutor.py
resources/libraries/python/VppApiCrc.py
resources/tools/integrated/check_crc.py

index e01536b..86cbefe 100644 (file)
@@ -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:
 # 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:
 """Constants used in CSIT."""
 
 
 """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.
 
 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
 
     # 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 = {
     # 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 = {
index adafa88..7ff7e8c 100644 (file)
@@ -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:
 # 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 binascii
+import copy
 import glob
 import json
 import shutil
 import glob
 import json
 import shutil
@@ -354,11 +355,13 @@ class PapiSocketExecutor(object):
         :rtype: PapiSocketExecutor
         :raises RuntimeError: If unverified or conflicting CRC is encountered.
         """
         :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)
         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(
         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."):
         return self
 
     def get_replies(self, err_msg="Failed to get replies."):
index 0d4e8f4..90131a6 100644 (file)
@@ -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:
 # 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 robot.api import logger
 
+from resources.libraries.python.Constants import Constants
+
 
 def _str(text):
     """Convert from possible unicode without interpreting as number.
 
 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."""
 
     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
         """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
         """
 
         :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.
 
         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,
         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
 
         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()
         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)
 
         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.
 
     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
         :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(
         """
         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()
         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(
                 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):
 
     @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(
             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.
 
 
     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.
         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
             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
 
         # 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
         """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.
         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(
                     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.
 
     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.
 
         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
         :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:
         """
         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:
         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):
 
     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.
 
 
         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.
         """
         :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
         if new_expected:
             # Some collections recognized the message name.
             self._expected = new_expected
-            return
         crc = self._found.get(api_name, None)
         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
         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)
+        )
index 3d5c30a..7930a53 100644 (file)
@@ -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:
 # 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.
 """
 
 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
 
 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())