Backport CRC checking from master

Change-Id: Iac99a7928a721e2fb7c608b92d5c1cf2ba047c51
Signed-off-by: Vratko Polak <vrpolak@cisco.com>

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())