From b8b75e860168fdab30a050b5badcd3b080d61261 Mon Sep 17 00:00:00 2001 From: Stephan Pampel <7194553+SPFZ@users.noreply.github.com> Date: Thu, 20 Jan 2022 10:42:06 +0100 Subject: [PATCH] check_sriov_numvfs.py: parse arguments via regex; extend function docstrings --- .../network/files/check_sriov_numvfs.py | 53 +++++++++++++------ .../network/files/test_check_sriov_numvfs.py | 5 +- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/charmhelpers/contrib/network/files/check_sriov_numvfs.py b/charmhelpers/contrib/network/files/check_sriov_numvfs.py index 7b22c19ce..62bd0c23e 100755 --- a/charmhelpers/contrib/network/files/check_sriov_numvfs.py +++ b/charmhelpers/contrib/network/files/check_sriov_numvfs.py @@ -36,11 +36,13 @@ import argparse import os.path +import re import traceback import sys DEVICE_TEMPLATE = "/sys/class/net/{0}" +DEVICE_VF_PATTERN = r'(?P[0-9a-z]{3,}):(?P\d+)' SRIOV_NUMVFS_TEMPLATE = "/sys/class/net/{0}/device/sriov_numvfs" SRIOV_TOTALVFS_TEMPLATE = "/sys/class/net/{0}/device/sriov_totalvfs" @@ -52,14 +54,28 @@ class ArgsFormatError(Exception): def get_interface_setting(file): - """Return the value content of a setting file as int""" + """Return the value content of a settings file as int. + + :param file: Full path of the settings file. + :type file: str + :returns: The interface setting as int. + :rtype: int + """ with open(file) as f: value = f.read() return int(value) def check_interface_numvfs(iface, numvfs): - """Check SR-IOV numvfs config""" + """Verify SR-IOV interface configuration for number of virtual functions. + + :param iface: Name of the interface. + :type iface: str + :param numvfs: Number of virtual functions that should to be configured. + :type numvfs: int + :returns: List of check violations found for the device. + :rtype: [str] + """ sriov_numvfs_path = SRIOV_NUMVFS_TEMPLATE.format(iface) sriov_totalvfs_path = SRIOV_TOTALVFS_TEMPLATE.format(iface) msg = [] @@ -72,15 +88,18 @@ def check_interface_numvfs(iface, numvfs): if not os.path.exists(sriov_totalvfs_path) or not os.path.exists(sriov_numvfs_path): return ["{}: VFs are disabled or not-available".format(iface)] - # Verify number of virtual functions sriov_numvfs = get_interface_setting(sriov_numvfs_path) sriov_totalvfs = get_interface_setting(sriov_totalvfs_path) + + # Verify that number of virtual functions is set to the expected number if numvfs != sriov_numvfs: msg.append( "{}: Number of VFs on interface ({}) does not match expected ({})".format( iface, sriov_numvfs, numvfs ) ) + + # Verify that the device supports the amount of VFs that we expect to be configured if numvfs > sriov_totalvfs: msg.append( "{}: Maximum number of VFs available on interface ({}) is lower than the expected ({})".format( @@ -91,25 +110,29 @@ def check_interface_numvfs(iface, numvfs): def parse_sriov_numvfs(device_numvfs): - """Parse parameters and check format""" + """Parse sriov_numvfs string, verify format and return parsed values. + + :param device_numvfs: The devicename and number of Virtual Functions, format ':'. + :type device_numvfs: str + :returns: The interface name and number of VFs. + :rtype: (str, int) + :raises: ArgsFormatError if the format of the parameter is not correct. + """ msg = "Parameter format must be ':', e.g. ens3f0:32, given: '{}'".format( device_numvfs ) - parts = device_numvfs.split(":") - if ( - len(parts) != 2 or # exactly 2 parts - len(parts[0]) < 3 or # interface name should be at least 3 chars - len(parts[1]) < 1 or # numvfs should be at least 1 char - int(parts[1]) < 0 # numvfs should be a valid int > 0 - ): + match = re.match(DEVICE_VF_PATTERN, device_numvfs) + if match is None: raise ArgsFormatError(msg) - iface = str(device_numvfs.split(":")[0]) - numvfs = int(device_numvfs.split(":")[1]) - return (iface, numvfs) + return match.group('iface'), int(match.group('numvfs')) def parse_args(): - """Parse command-line options.""" + """Parse command-line options. + + :returns: Argparsed cli parameters as argparse.Namespace + :rtype: argparse.Namespace + """ parser = argparse.ArgumentParser( description="Check SR-IOV number of virtual functions configuration" ) diff --git a/tests/contrib/network/files/test_check_sriov_numvfs.py b/tests/contrib/network/files/test_check_sriov_numvfs.py index 3c8e328b2..84dbff0f9 100644 --- a/tests/contrib/network/files/test_check_sriov_numvfs.py +++ b/tests/contrib/network/files/test_check_sriov_numvfs.py @@ -35,12 +35,9 @@ def test_parameter_parsing(self): self.assertEqual(iface, 'ens3f0') self.assertEqual(numvfs, 32) - for param in ['', ':', ':test', ':32', 'ens3f2', 'a:1:b', 'ens3f2:']: + for param in ['', ':', ':test', ':32', 'ens3f2', 'a:1:b', 'ens3f2:', 'ens3f2:test']: self.assertRaises(check_sriov_numvfs.ArgsFormatError, check_sriov_numvfs.parse_sriov_numvfs, param) - for param in ['ens3f2:test']: - self.assertRaises(ValueError, check_sriov_numvfs.parse_sriov_numvfs, param) - def test_check_interface_numvfs_no_interface(self): """Check should ignore the interface if it does not exists""" self.assertListEqual(