From e310a40eab90bb5ecd8471dbbccc1d02daf2dea3 Mon Sep 17 00:00:00 2001 From: Vratko Polak Date: Thu, 5 Sep 2019 16:15:19 +0200 Subject: [PATCH] Read environment variables in Constants.py Instead of using EnsureGlobalVariable, which is clunky to use from Python. As a consequence, all caps variables from Constants.py are used directly and tests/__init__.robot and robot_enhancements.robot are deleted. + Rename the CRC global kill switch based on ci-man review. Change-Id: I10723792475bc83352bf8c3b7f7946ecf885a194 Signed-off-by: Vratko Polak --- resources/libraries/python/Constants.py | 120 ++++++++++++++++++++- resources/libraries/python/PapiExecutor.py | 15 +-- resources/libraries/python/VppApiCrc.py | 4 +- .../robot/performance/performance_utils.robot | 5 +- resources/libraries/robot/robot_enhancements.robot | 55 ---------- resources/libraries/robot/shared/container.robot | 7 +- resources/libraries/robot/shared/memif.robot | 5 +- .../libraries/robot/shared/test_teardown.robot | 3 +- tests/__init__.robot | 40 ------- 9 files changed, 133 insertions(+), 121 deletions(-) delete mode 100644 resources/libraries/robot/robot_enhancements.robot delete mode 100644 tests/__init__.robot diff --git a/resources/libraries/python/Constants.py b/resources/libraries/python/Constants.py index e8209544e8..9606a10fad 100644 --- a/resources/libraries/python/Constants.py +++ b/resources/libraries/python/Constants.py @@ -11,7 +11,112 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Constants used in CSIT.""" +"""Constants used in CSIT. + +Here, "constant" means a value that keeps its value since initialization. +However, the value does not need to be hardcoded here, +some values are affected by environment variables. + +TODO: Review env and constant names, make them matching if possible. +""" + + +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 convensions. + Here "several" means there are hardcoded 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 True if env_str in ("true", "yes", "y", "1") else False + + +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 False if env_str in ("false", "no", "n", "0") else True class Constants(object): @@ -112,8 +217,19 @@ class Constants(object): # Default path to VPP API socket. SOCKSVR_PATH = "/run/vpp/api.sock" + # Number of trials to execute in MRR test. + PERF_TRIAL_MULTIPLICITY = get_int_from_env("PERF_TRIAL_MULTIPLICITY", 10) + + # Duration of one trial in MRR test. + PERF_TRIAL_DURATION = get_float_from_env("PERF_TRIAL_DURATION", 1.0) + + # UUID string of DUT1 /tmp volume created outside of the + # DUT1 docker in case of vpp-device test. ${EMPTY} value means that + # /tmp directory is inside the DUT1 docker. + DUT1_UUID = get_str_from_env("DUT1_UUID", "") + # Global "kill switch" for CRC checking during runtime. - CRC_MISMATCH_FAILS_TEST = True + FAIL_ON_CRC_MISMATCH = get_optimistic_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. diff --git a/resources/libraries/python/PapiExecutor.py b/resources/libraries/python/PapiExecutor.py index 0a714c182c..74ff7a0f9b 100644 --- a/resources/libraries/python/PapiExecutor.py +++ b/resources/libraries/python/PapiExecutor.py @@ -166,18 +166,6 @@ class PapiSocketExecutor(object): if self.vpp_instance: return cls = self.__class__ # Shorthand for setting class fields. - fail_on_mismatch = Constants.CRC_MISMATCH_FAILS_TEST - try: - from robot.libraries.BuiltIn import BuiltIn - from_robot = BuiltIn().get_variable_value( - "\${crc_mismatch_fails}", None) - if from_robot is not None: - # Robot interprets env vars as strings. - fail_on_mismatch = not from_robot.lower() in ("false", "n", "0") - except (ImportError, AttributeError): - # If robot is not installed or not running, or value is not string, - # the Constants value applies. - pass package_path = None tmp_dir = tempfile.mkdtemp(dir="/tmp") try: @@ -199,8 +187,7 @@ class PapiSocketExecutor(object): api_json_directory = tmp_dir + "/usr/share/vpp/api" # Perform initial checks before .api.json files are gone, # by creating the checker instance. - cls.crc_checker = VppApiCrcChecker( - api_json_directory, fail_on_mismatch=fail_on_mismatch) + cls.crc_checker = VppApiCrcChecker(api_json_directory) # When present locally, we finally can find the installation path. package_path = glob.glob(tmp_dir + installed_papi_glob)[0] # Package path has to be one level above the vpp_papi directory. diff --git a/resources/libraries/python/VppApiCrc.py b/resources/libraries/python/VppApiCrc.py index d66d4d7568..cb6f5b0c60 100644 --- a/resources/libraries/python/VppApiCrc.py +++ b/resources/libraries/python/VppApiCrc.py @@ -43,8 +43,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, - fail_on_mismatch=Constants.CRC_MISMATCH_FAILS_TEST): + 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 diff --git a/resources/libraries/robot/performance/performance_utils.robot b/resources/libraries/robot/performance/performance_utils.robot index d809376c0f..bffd35d05e 100644 --- a/resources/libraries/robot/performance/performance_utils.robot +++ b/resources/libraries/robot/performance/performance_utils.robot @@ -23,6 +23,7 @@ | Library | resources.libraries.python.TrafficGenerator.OptimizedSearch | Library | resources.libraries.python.TrafficGenerator.TGDropRateSearchImpl | Library | resources.libraries.python.Trace +| Variables | resources/libraries/python/Constants.py | ... | Documentation | ... | Performance suite keywords - utilities to find and verify NDR and PDR. @@ -396,8 +397,8 @@ | | ... | \| Traffic should pass with maximum rate \| ${1} \| ${10.0} \ | | ... | \| ${False} \| ${False} \| ${0} \| ${1} \| | | ... -| | [Arguments] | ${trial_duration}=${perf_trial_duration} -| | ... | ${fail_no_traffic}=${True} | ${subsamples}=${perf_trial_multiplicity} +| | [Arguments] | ${trial_duration}=${PERF_TRIAL_DURATION} +| | ... | ${fail_no_traffic}=${True} | ${subsamples}=${PERF_TRIAL_MULTIPLICITY} | | ... | ${unidirection}=${False} | ${tx_port}=${0} | ${rx_port}=${1} | | ... | | ${results} = | Send traffic at specified rate | ${trial_duration} diff --git a/resources/libraries/robot/robot_enhancements.robot b/resources/libraries/robot/robot_enhancements.robot deleted file mode 100644 index 7b337eafeb..0000000000 --- a/resources/libraries/robot/robot_enhancements.robot +++ /dev/null @@ -1,55 +0,0 @@ -# Copyright (c) 2018 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: -# -# 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. - -*** Settings *** -| Documentation -| ... | Place to store general keywords, similar to ones in BuiltIn library. -| Library | OperatingSystem -| Library | String - -*** Keywords *** -| Ensure Global Variable -| | [Documentation] | Give default value (from environment or code) to variable. -| | ... -| | ... | Ideally we would just use Variables table to set global variables. -| | ... | Unfortunately, __init__ files are special (among other things) -| | ... | by the fact that variables set in the table are not available -| | ... | for (sub)directory suites. -| | ... | Therefore we are using BuiltIn.Set_Global_Variable explicitly. -| | ... | While we are running code here, we allow environment variables -| | ... | to override the default values. -| | ... | If environment variable name is not specified (or empty), -| | ... | the upper case of the variable name is used, prefixed by "CSIT_". -| | ... | The --variable parameter to pybot takes precedence to environment. -| | ... -| | ... | *Arguments:* -| | ... | - variable_name - Name of global variable to set. Type: string -| | ... | - default_value - Value to set if not set otherwise. Type: string -| | ... | - env_var_name - Name of environment variable to read. Type: string -| | ... | - application - Prefix for default environment variable. Type: string -| | ... -| | ... | *Example:* -| | ... | \| Ensure Global Variable \| perf_trial_duration \| 10.0 \| TRIAL_DUR -| | ... -| | [Arguments] | ${variable_name} | ${default_value} | ${env_var_name}=${EMPTY} -| | ... | ${application}=CSIT -| | ... -| | ${env_var_length} = | Get Length | ${env_var_name} -| | ${default_env_var} = | Convert To Uppercase | ${variable_name} -| | ${env_var} = | Set Variable If | ${env_var_length} -| | ... | ${env_var_name} | ${application}_${default_env_var} -| | ${updated_default} = | Get Environment Variable -| | ... | ${env_var} | ${default_value} -| | ${final_value} = | Get Variable Value -| | ... | \${${variable_name}} | ${updated_default} -| | Set Global Variable | \${${variable_name}} | ${final_value} diff --git a/resources/libraries/robot/shared/container.robot b/resources/libraries/robot/shared/container.robot index 15bcad7999..cb3bc29490 100644 --- a/resources/libraries/robot/shared/container.robot +++ b/resources/libraries/robot/shared/container.robot @@ -16,6 +16,7 @@ | ... | Library | resources.libraries.python.CpuUtils | Library | resources.libraries.python.topology.Topology +| Variables | resources/libraries/python/Constants.py *** Keywords *** | Construct container on all DUTs @@ -52,9 +53,9 @@ | | :FOR | ${dut} | IN | @{duts} | | | ${nf_id}= | Evaluate | (${nf_chain} - ${1}) * ${nf_nodes} + ${nf_node} | | | ${env}= | Create List | DEBIAN_FRONTEND=noninteractive -| | | ${dut1_uuid_length} = | Get Length | ${dut1_uuid} +| | | ${dut1_uuid_length} = | Get Length | ${DUT1_UUID} | | | ${root}= | Run Keyword If | ${dut1_uuid_length} -| | | ... | Get Docker Mergeddir | ${nodes['DUT1']} | ${dut1_uuid} +| | | ... | Get Docker Mergeddir | ${nodes['DUT1']} | ${DUT1_UUID} | | | ... | ELSE | Set Variable | ${EMPTY} | | | ${node_arch}= | Get Node Arch | ${nodes['${dut}']} | | | ${mnt}= | Create List @@ -71,7 +72,7 @@ | | | ... | nf_chain=${nf_chain} | nf_node=${nf_node} | | | ... | vs_dtc=${cpu_count_int} | nf_dtc=${nf_dtc} | nf_dtcr=${nf_dtcr} | | | &{cont_args}= | Create Dictionary -| | | ... | name=${dut}_${container_group}${nf_id}${dut1_uuid} +| | | ... | name=${dut}_${container_group}${nf_id}${DUT1_UUID} | | | ... | node=${nodes['${dut}']} | mnt=${mnt} | env=${env} | | | Run Keyword If | ${pinning} | | | ... | Set To Dictionary | ${cont_args} | cpuset_cpus=${nf_cpus} diff --git a/resources/libraries/robot/shared/memif.robot b/resources/libraries/robot/shared/memif.robot index 0fda454764..a0fae656f9 100644 --- a/resources/libraries/robot/shared/memif.robot +++ b/resources/libraries/robot/shared/memif.robot @@ -15,6 +15,7 @@ | Documentation | Memif interface keyword library. | ... | Library | resources.libraries.python.Memif +| Variables | resources/libraries/python/Constants.py *** Keywords *** | Set up memif interfaces on DUT node @@ -56,10 +57,10 @@ | | ${sid_1}= | Evaluate | (${mid}*2)-1 | | ${sid_2}= | Evaluate | (${mid}*2) | | ${memif_1}= | Create memif interface | ${dut_node} -| | ... | ${filename1}${mid}${dut1_uuid}-${sid_1} | ${mid} | ${sid_1} +| | ... | ${filename1}${mid}${DUT1_UUID}-${sid_1} | ${mid} | ${sid_1} | | ... | rxq=${rxq} | txq=${txq} | role=${role} | | ${memif_2}= | Create memif interface | ${dut_node} -| | ... | ${filename2}${mid}${dut1_uuid}-${sid_2} | ${mid} | ${sid_2} +| | ... | ${filename2}${mid}${DUT1_UUID}-${sid_2} | ${mid} | ${sid_2} | | ... | rxq=${rxq} | txq=${txq} | role=${role} | | Set Interface State | ${dut_node} | ${memif_1} | up | | Set Interface State | ${dut_node} | ${memif_2} | up diff --git a/resources/libraries/robot/shared/test_teardown.robot b/resources/libraries/robot/shared/test_teardown.robot index e130038fbc..5d2c2fa165 100644 --- a/resources/libraries/robot/shared/test_teardown.robot +++ b/resources/libraries/robot/shared/test_teardown.robot @@ -17,6 +17,7 @@ | Resource | resources/libraries/robot/shared/container.robot | Library | resources.libraries.python.PapiHistory | Library | resources.libraries.python.topology.Topology +| Variables | resources/libraries/python/Constants.py | ... | Documentation | Test teardown keywords. @@ -44,7 +45,7 @@ | | ... | Additional teardown for tests which uses performance measurement. | | ... | | Run Keyword If Test Failed -| | ... | Send traffic at specified rate | ${perf_trial_duration} | 10000pps +| | ... | Send traffic at specified rate | ${PERF_TRIAL_DURATION} | 10000pps | | ... | ${frame_size} | ${traffic_profile} | pkt_trace=${True} | Additional Test Tear Down Action For packet_trace diff --git a/tests/__init__.robot b/tests/__init__.robot deleted file mode 100644 index e40911413c..0000000000 --- a/tests/__init__.robot +++ /dev/null @@ -1,40 +0,0 @@ -# Copyright (c) 2019 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: -# -# 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. - -*** Settings *** -| Documentation | Set global variables common to all tests. -| # The two paths below assume working directory when pybot is started, -| # but we cannot use ${CURDIR} as this file may be copied under generated/. -| Resource | resources/libraries/robot/robot_enhancements.robot -| Variables | resources/libraries/python/Constants.py -| Suite Setup | Set Common Variables - -*** Keywords *** -| Set Common Variables -| | [Documentation] | Set the following global variables. -| | ... -| | ... | While currently only MRR tests are using the values, -| | ... | any new test might decide to use them, -| | ... | so variable names are generic for "perf" scope. -| | ... -| | ... | perf_trial_multiplicity - Number of trials to execute in MRR test. -| | ... | perf_trial_duration - Duration of one trial in MRR test. -| | ... | dut1_uuid - UUID string of DUT1 /tmp volume created outside of the -| | ... | DUT1 docker in case of vpp-device test. ${EMPTY} value means that -| | ... | /tmp directory is inside the DUT1 docker -| | ... -| | Ensure Global Variable | perf_trial_multiplicity | 10 -| | Ensure Global Variable | perf_trial_duration | 1 -| | Ensure Global Variable | dut1_uuid | ${EMPTY} -| | # Avoiding dangers of case insensitive Robot variable names. -| | Ensure Global Variable | crc_mismatch_fails | ${CRC_MISMATCH_FAILS_TEST} -- 2.16.6