From a6f6f8f60425a9f4dffb3d2429efca01efd9836c Mon Sep 17 00:00:00 2001 From: "Mitch Harding (the weird one)" Date: Thu, 5 Dec 2024 14:13:04 -0500 Subject: [PATCH 1/2] CASMPET-7290: Add pylint to csm-testing build pipeline --- Jenkinsfile.github | 16 +++++++++ Makefile | 14 +++++++- common.sh | 6 ++++ create-python-script-symlinks.sh | 6 ---- pylint-requirements.txt | 3 ++ remove-python-packages.sh | 5 +-- requirements.txt | 1 + update-pylint-requirements.sh | 60 ++++++++++++++++++++++++++++++++ 8 files changed, 100 insertions(+), 11 deletions(-) create mode 100644 pylint-requirements.txt create mode 100755 update-pylint-requirements.sh diff --git a/Jenkinsfile.github b/Jenkinsfile.github index 9342bee7..85959c76 100644 --- a/Jenkinsfile.github +++ b/Jenkinsfile.github @@ -82,6 +82,22 @@ pipeline { sh "make pymod" } } + stage('Validate Python module') { + agent { + docker { + label "${PRIMARY_NODE}" + reuseNode true + args "-v /home/jenkins/.ssh:/home/jenkins/.ssh -v /home/jenkins/.netrc:/home/jenkins/.netrc" + image "${pyImage}:${PY_VERSION}" + } + } + environment { + PY_BIN_PATH = getPythonBinaryPath(PY_VERSION) + } + steps { + sh "make pylint" + } + } stage('Build: RPMs') { agent { docker { diff --git a/Makefile b/Makefile index 7c080c5b..19b1ef0a 100644 --- a/Makefile +++ b/Makefile @@ -50,6 +50,8 @@ SOURCE_NAME := ${NAME}-${VERSION} BUILD_DIR ?= $(PWD)/dist/rpmbuild SOURCE_PATH := ${BUILD_DIR}/SOURCES/${SOURCE_NAME}.tar.bz2 +PYLINT_VENV_DIR := pylint-venv +PYLINT_VENV_PYBIN := $(PYLINT_VENV_DIR)/bin/python3 rpm: rpm_package_source rpm_build_source rpm_build @@ -61,6 +63,16 @@ pymod: $(PY_BIN_PATH) -m build --wheel cp ./dist/csm_testing*.whl . +pylint: + $(PY_BIN_PATH) --version + $(PY_BIN_PATH) -m venv $(PYLINT_VENV_DIR) + $(PYLINT_VENV_PYBIN) -m pip install --upgrade --no-cache pip + ./update-pylint-requirements.sh + $(PYLINT_VENV_PYBIN) -m pip install --disable-pip-version-check --no-cache -r pylint-requirements.txt pylint csm_testing*.whl + $(PYLINT_VENV_PYBIN) -m pip list --format freeze + $(PYLINT_VENV_PYBIN) -m pylint --errors-only csm_testing + $(PYLINT_VENV_PYBIN) -m pylint --fail-under 9 csm_testing || true + prepare: @echo $(NAME) rm -rf $(BUILD_DIR) @@ -68,7 +80,7 @@ prepare: cp $(SPEC_FILE) $(BUILD_DIR)/SPECS/ rpm_package_source: - tar --transform 'flags=r;s,^,/$(SOURCE_NAME)/,' --exclude ./.nox --exclude .git --exclude ./build --exclude ./dist --exclude ./${SOURCE_NAME}.tar.bz2 -cvjf $(SOURCE_PATH) . + tar --transform 'flags=r;s,^,/$(SOURCE_NAME)/,' --exclude ./.nox --exclude .git --exclude ./build --exclude ./'$(PYLINT_VENV_DIR)' --exclude ./dist --exclude ./${SOURCE_NAME}.tar.bz2 -cvjf $(SOURCE_PATH) . rpm_build_source: rpmbuild -vv -bs $(BUILD_DIR)/SPECS/$(SPEC_FILE) --target ${ARCH} --define "_topdir $(BUILD_DIR)" diff --git a/common.sh b/common.sh index ef8cb370..5b726764 100644 --- a/common.sh +++ b/common.sh @@ -36,3 +36,9 @@ SYMLINK_PREFIX="#python-script-symlink" # (e.g. do not use '|') #shellcheck disable=SC2034 SYMLINK_FS="," + +function err_exit +{ + echo "$0: ERROR: $*" >&2 + exit 1 +} diff --git a/create-python-script-symlinks.sh b/create-python-script-symlinks.sh index e535e680..206d9bd7 100755 --- a/create-python-script-symlinks.sh +++ b/create-python-script-symlinks.sh @@ -33,12 +33,6 @@ source ./common.sh # # [] ... -function err_exit -{ - echo "$0: ERROR: $*" >&2 - exit 1 -} - [[ $# -ge 4 ]] || err_exit "Too few arguments ($#): $*" buildroot="$1" diff --git a/pylint-requirements.txt b/pylint-requirements.txt new file mode 100644 index 00000000..b1a808ab --- /dev/null +++ b/pylint-requirements.txt @@ -0,0 +1,3 @@ +-r requirements.txt + +pylint diff --git a/remove-python-packages.sh b/remove-python-packages.sh index 6c6cdf9b..43c744c6 100755 --- a/remove-python-packages.sh +++ b/remove-python-packages.sh @@ -36,10 +36,7 @@ set -eu # command line, in which case those too will be removed from the virtual # environment, if they are present -function err_exit { - echo "ERROR: $0: $*" >&2 - exit 1 -} +source ./common.sh common_pip_flags='--no-cache-dir --disable-pip-version-check' diff --git a/requirements.txt b/requirements.txt index c4bfdfd3..75f1da39 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,6 +11,7 @@ kubernetes PyYAML requests requests-retry-session +urllib3 # There are some additional Python packages that test scripts can rely on that are # not listed above. This is because they are listed as requirements in the csm-testing diff --git a/update-pylint-requirements.sh b/update-pylint-requirements.sh new file mode 100755 index 00000000..ec910bb4 --- /dev/null +++ b/update-pylint-requirements.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# +# MIT License +# +# (C) Copyright 2024 Hewlett Packard Enterprise Development LP +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR +# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, +# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +# OTHER DEALINGS IN THE SOFTWARE. +# + +set -exuo pipefail + +# This updates 'pylint-requirements.txt', adding in some of the system Python requirements +# from requirements.txt. Specifically, we include all of them except rados, since we +# are not going to be able to install it for our pylint run + +source ./common.sh + +SRCFILE=requirements.txt +TRGFILE=pylint-requirements.txt + +[[ -e ${SRCFILE} ]] || err_exit "${SRCFILE} does not exist" +[[ -f ${SRCFILE} ]] || err_exit "${SRCFILE} exists but is not a regular file" +[[ -s ${SRCFILE} ]] || err_exit "${SRCFILE} exists but is zero size" + +SYS_PY_REGEX='^#system_python:' +SYS_RADOS_REGEX='^#system_python:rados[[:space:]]*$' + +if ! grep -Eq "${SYS_PY_REGEX}" "${SRCFILE}" ; then + echo "No system_python lines found in '${SRCFILE}' -> no updates necessary to '${TRGFILE}'" + exit 0 +fi + +if ! grep -E "${SYS_PY_REGEX}" "${SRCFILE}" | grep -Eqv "${SYS_RADOS_REGEX}" ; then + echo "No system_python lines other than rados found in '${SRCFILE}' -> no updates necessary to '${TRGFILE}'" + exit 0 +fi + +tmpfile=$(mktemp) || err_exit "Failed to create temporary file" + +grep -E "${SYS_PY_REGEX}" "${SRCFILE}" | grep -Ev "${SYS_RADOS_REGEX}" | sed "s/${SYS_PY_REGEX}//" > "${tmpfile}" || err_exit "Error creating '${tmpfile}'" + +echo "Appending following lines to '${TRGFILE}'" +cat "${tmpfile}" | tee -a "${TRGFILE}" +rm -v "${tmpfile}" From ebb564431d01bec2898d358abfed6a28545fe08a Mon Sep 17 00:00:00 2001 From: "Mitch Harding (the weird one)" Date: Thu, 5 Dec 2024 14:13:12 -0500 Subject: [PATCH 2/2] CASMPET-7290: Fix errors identified by pylint --- src/csm_testing/lib/data_json_parser.py | 5 +++-- src/csm_testing/tests/check_for_unused_drives/__main__.py | 3 ++- src/csm_testing/tests/check_ncn_uan_ip_dns/__main__.py | 2 +- src/csm_testing/tests/console_verify_services/__main__.py | 2 +- src/csm_testing/tests/ip_not_in_ip_pools/__main__.py | 5 +++-- src/csm_testing/tests/rgw_endpoint_check/__main__.py | 4 ++-- src/csm_testing/tools/print_goss_json_results/__main__.py | 4 ++-- 7 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/csm_testing/lib/data_json_parser.py b/src/csm_testing/lib/data_json_parser.py index 1e8859b0..d75c5a9e 100644 --- a/src/csm_testing/lib/data_json_parser.py +++ b/src/csm_testing/lib/data_json_parser.py @@ -21,7 +21,6 @@ # ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR # OTHER DEALINGS IN THE SOFTWARE. # -from __future__ import print_function """ dataJson is a convenience class to work with the data.json file @@ -48,8 +47,10 @@ """ +from __future__ import print_function import json import re +import sys class dataJson: def __init__(self, data_json_path='/mnt/configs/data.json'): @@ -61,7 +62,7 @@ def __init__(self, data_json_path='/mnt/configs/data.json'): try: self.payload = json.load(obj) except: - print("Unable to open " + objFile + ". Possibly malformed json?") + print("Unable to open " + self.objFile + ". Possibly malformed json?") sys.exit() self.keys = self.payload.keys() diff --git a/src/csm_testing/tests/check_for_unused_drives/__main__.py b/src/csm_testing/tests/check_for_unused_drives/__main__.py index 3b914ae8..aad7508a 100644 --- a/src/csm_testing/tests/check_for_unused_drives/__main__.py +++ b/src/csm_testing/tests/check_for_unused_drives/__main__.py @@ -43,7 +43,8 @@ import argparse import json import logging -import rados +# The rados package is not available when we run pylint +import rados # pylint: disable=import-error import sys CEPH_CONFIG_FILE = "/etc/ceph/ceph.conf" diff --git a/src/csm_testing/tests/check_ncn_uan_ip_dns/__main__.py b/src/csm_testing/tests/check_ncn_uan_ip_dns/__main__.py index 43ccfb09..6337e045 100644 --- a/src/csm_testing/tests/check_ncn_uan_ip_dns/__main__.py +++ b/src/csm_testing/tests/check_ncn_uan_ip_dns/__main__.py @@ -28,7 +28,7 @@ import logging import requests from requests.adapters import HTTPAdapter -from requests.packages.urllib3.util.retry import Retry +from urllib3.util.retry import Retry from urllib.parse import urljoin from kubernetes import client, config diff --git a/src/csm_testing/tests/console_verify_services/__main__.py b/src/csm_testing/tests/console_verify_services/__main__.py index 7561c33b..0d47b5d8 100644 --- a/src/csm_testing/tests/console_verify_services/__main__.py +++ b/src/csm_testing/tests/console_verify_services/__main__.py @@ -114,7 +114,7 @@ def main() -> int: logger.info("Beginning verification that all console services are running") # Find that all services are present - pods = check_services_running() + check_services_running() logger.info("Verification of console services succeeded") return 0 diff --git a/src/csm_testing/tests/ip_not_in_ip_pools/__main__.py b/src/csm_testing/tests/ip_not_in_ip_pools/__main__.py index c33f8598..5f932209 100644 --- a/src/csm_testing/tests/ip_not_in_ip_pools/__main__.py +++ b/src/csm_testing/tests/ip_not_in_ip_pools/__main__.py @@ -69,8 +69,9 @@ def is_ip_between(ip, start_ip, end_ip, file): if ips >= start_ip and ips <= end_ip: print("Failed: This IP is in the pool range.") - print("This IP = ", ips, "Pool start IP = ", start_ip, "Pool end IP = ", end_ip) - logging.error("This IP = ", ips, "Pool start IP = ", start_ip, "Pool end IP = ", end_ip) + msg = f"This IP = {ips} Pool start IP = {start_ip} Pool end IP = {end_ip}" + print(msg) + logging.error(msg) return "FAIL" else: return "PASS" diff --git a/src/csm_testing/tests/rgw_endpoint_check/__main__.py b/src/csm_testing/tests/rgw_endpoint_check/__main__.py index 51a39adb..88ca97d4 100644 --- a/src/csm_testing/tests/rgw_endpoint_check/__main__.py +++ b/src/csm_testing/tests/rgw_endpoint_check/__main__.py @@ -146,7 +146,7 @@ def get_url_and_upload(bucket_name, key_name, file_name): try: s3client.delete_object(Bucket=bucket_name, Key=key_name) except Exception as delete_err: - echo("Unsuccessful upload. Unable to delete object: Error: %s" % delete_err) + print("Unsuccessful upload. Unable to delete object: Error: %s" % delete_err) sys.exit(str(err)) try: @@ -167,7 +167,7 @@ def get_url_and_upload(bucket_name, key_name, file_name): try: s3client.delete_object(Bucket=bucket_name, Key=key_name) except Exception as delete_err: - echo("Unsuccessful upload. Unable to delete object: Error: %s" % delete_err) + print("Unsuccessful upload. Unable to delete object: Error: %s" % delete_err) sys.exit(str(err)) def list_objects(bucket_name): diff --git a/src/csm_testing/tools/print_goss_json_results/__main__.py b/src/csm_testing/tools/print_goss_json_results/__main__.py index 76321338..b00f186b 100644 --- a/src/csm_testing/tools/print_goss_json_results/__main__.py +++ b/src/csm_testing/tools/print_goss_json_results/__main__.py @@ -324,7 +324,7 @@ def to_json(self): class ResultsEntry: - def __init__(self, result_entry_raw: dict): + def __init__(self, result_entry_raw: Dict): self.result_raw = result_entry_raw["result"] self.title = result_entry_raw["title"] self.summary = result_entry_raw["summary-line"] @@ -375,7 +375,7 @@ def dict(self, source: str, node_name: str) -> dict: "Node": node_name } -def extract_results_data(json_results: dict) -> Tuple[List[ResultsEntry], int, DurationSeconds]: +def extract_results_data(json_results: Dict) -> Tuple[List[ResultsEntry], int, DurationSeconds]: try: results = json_results["results"] # Make list of results with a numeric result