CASMPET-7290: Add pylint to build pipeline; correct errors identified by pylint

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 {

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,14 +63,24 @@ 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)
 	mkdir -p $(BUILD_DIR)/SPECS $(BUILD_DIR)/SOURCES
 	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)"

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
+}

create-python-script-symlinks.sh

@@ -33,12 +33,6 @@ source ./common.sh
 #                                         <target subdir1 in buildroot for symlinks>
 #                                        [<target subdir2 in buildroot for symlinks>] ...
 
-function err_exit
-{
-    echo "$0: ERROR: $*" >&2
-    exit 1
-}
-
 [[ $# -ge 4 ]] || err_exit "Too few arguments ($#): $*"
 
 buildroot="$1"

pylint-requirements.txt

@@ -0,0 +1,3 @@
+-r requirements.txt
+
+pylint

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'
 

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

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

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"

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
 

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

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"

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

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

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}"