CA-388295: Fix bugs from py2->py3 conversion in perfmon and hfx_filename

pyproject.toml

@@ -70,8 +70,6 @@ discard_messages_matching = [
     "No Node.TEXT_NODE in module xml.dom.minidom, referenced from 'xml.dom.expatbuilder'"
 ]
 expected_to_fail = [
-    "scripts/hfx_filename",
-    "scripts/perfmon",
     # Need 2to3 -w <file> and maybe a few other minor updates:
     "scripts/hatests",
     "scripts/backup-sr-metadata.py",

scripts/hfx_filename

@@ -28,18 +28,14 @@ def rpc(session_id, request):
         headers = [
           "POST %s?session_id=%s HTTP/1.0" % (db_url, session_id),
           "Connection:close",
-          "content-length:%d" % (len(request)),
+          "content-length:%d" % (len(request.encode('utf-8'))),
           ""
         ]
-        #print "Sending HTTP request:"
         for h in headers:
-          s.send("%s\r\n" % h)
-          #print "%s\r\n" % h,
-        s.send(request)
+          s.send((h + "\r\n").encode('utf-8'))
+        s.send(request.encode('utf-8'))
 
-        result = s.recv(1024)
-        #print "Received HTTP response:"
-        #print result
+        result = s.recv(1024).decode('utf-8')
         if "200 OK" not in result:
           print("Expected an HTTP 200, got %s" % result, file=sys.stderr)
           return
@@ -55,13 +51,14 @@ def rpc(session_id, request):
         s.close()
 
 def parse_string(txt):
+    assert txt is not None
     prefix = "<value><array><data><value>success</value><value>"
     if not txt.startswith(prefix):
-        raise "Unable to parse string response"
+        raise Exception("Unable to parse string response")
     txt = txt[len(prefix):]
     suffix = "</value></data></array></value>"
     if not txt.endswith(suffix):
-        raise "Unable to parse string response"
+        raise Exception("Unable to parse string response")
     txt = txt[:len(txt)-len(suffix)]
     return txt
 

scripts/import_file.py

@@ -0,0 +1,27 @@
+"""
+This module is used for importing a non-".py" file as a module
+"""
+import sys
+import os
+if sys.version_info.major > 2:
+    from importlib import machinery, util
+
+def import_from_file(module_name, file_path):
+    """Import a file as a module"""
+    # Only for python3, but CI has python2 pytest check, so add this line
+    if sys.version_info.major == 2:
+        return None
+    loader = machinery.SourceFileLoader(module_name, file_path)
+    spec = util.spec_from_loader(module_name, loader)
+    assert spec
+    assert spec.loader
+    module = util.module_from_spec(spec)
+    # Probably a good idea to add manually imported module stored in sys.modules
+    sys.modules[module_name] = module
+    spec.loader.exec_module(module)
+    return module
+
+def get_module(module_name, file_path):
+    """get the module from a file"""
+    testdir = os.path.dirname(__file__)
+    return import_from_file(module_name, testdir + file_path)

scripts/perfmon

@@ -36,8 +36,10 @@ import getopt
 import traceback
 import XenAPI
 import urllib.request
-from xml import sax            # used to parse rrd_updates because this may be large and sax is more efficient
-from xml.dom import minidom    # used to parse other-config:perfmon.  Efficiency is less important than reliability here
+# used to parse rrd_updates because this may be large and sax is more efficient
+from xml import sax
+# used to parse other-config:perfmon.  Efficiency is less important than reliability here
+from xml.dom import minidom # pytype: disable=pyi-error
 from xml.parsers.expat import ExpatError
 import time
 import re
@@ -313,7 +315,7 @@ class RRDUpdates:
         print_debug("Calling http://localhost/rrd_updates?%s" % paramstr)
 
         sock = urllib.request.urlopen("http://localhost/rrd_updates?%s" % paramstr)
-        xmlsource = sock.read()
+        xmlsource = sock.read().decode('utf-8')
         sock.close()
 
         # Use sax rather than minidom and save Vvvast amounts of time and memory.
@@ -379,8 +381,15 @@ def get_percent_mem_usage(ignored):
         memfd = open('/proc/meminfo', 'r')
         memlist = memfd.readlines()
         memfd.close()
-        memdict = [ m.split(':', 1) for m in memlist ]
-        memdict = dict([(k.strip(), float(re.search('\d+', v.strip()).group(0))) for (k,v) in memdict])
+        memorylist = [ m.split(':', 1) for m in memlist ]
+        memdict = {}
+        for item in memorylist:
+            memtype = item[0].strip()
+            size = item[1].strip()
+            match = re.search(r'\d+', size.strip())
+            assert match
+            memdict[memtype] = float(match.group(0))
+
         # We consider the sum of res memory and swap in use as the hard demand
         # of mem usage, it is bad if this number is beyond the physical mem, as
         # in such case swapping is obligatory rather than voluntary, hence
@@ -476,21 +485,15 @@ def variable_configs_differ(vc1, vc2):
     "Say whether configuration of one variable differs from that of another"
     return vc1.xmldoc.toxml() != vc2.xmldoc.toxml()
 
-class VariableState:
-    """ Object storing the state of a Variable
-    """
-    def __init__(self):
-        self.value = None
-        self.timeof_last_alarm = time.time() - self.alarm_auto_inhibit_period
-        self.trigger_down_counter = self.alarm_trigger_period
-
-class Variable(VariableConfig, VariableState):
+class Variable(VariableConfig):
     """ Variable() is used by ObjectMonitor to create one Variable object for each
     variable specified in it's config string
     """
     def __init__(self, *args):
         VariableConfig.__init__(self, *args)
-        VariableState.__init__(self)
+        self.value = None
+        self.timeof_last_alarm = time.time() - self.alarm_auto_inhibit_period
+        self.trigger_down_counter = self.alarm_trigger_period
         self.active = True
         print_debug("Created Variable %s" % self.name)
 
@@ -500,7 +503,10 @@ class Variable(VariableConfig, VariableState):
             return # nothing to do
         self.active = active
         if active:
-            VariableState.__init__(self) # reset when reactivating
+            # reset when reactivating
+            self.value = None
+            self.timeof_last_alarm = time.time() - self.alarm_auto_inhibit_period
+            self.trigger_down_counter = self.alarm_trigger_period
 
     def __generate_alarm(self, session):
         """ Generate an alarm using callback provided by creator
@@ -1067,7 +1073,8 @@ def main():
             vm_uuid_list = rrd_updates.get_uuid_list_by_objtype('vm')
 
             # Remove any monitors for VMs no longer listed in rrd_updates page
-            for uuid in vm_mon_lookup:
+            # We use .pop() inside the loop, use list(dict_var.keys()):
+            for uuid in list(vm_mon_lookup.keys()):
                 if uuid not in vm_uuid_list:
                     vm_mon_lookup.pop(uuid)
 
@@ -1103,7 +1110,8 @@ def main():
             print_debug("sr_uuid_list = %s" % sr_uuid_list)
 
             # Remove monitors for SRs no longer listed in the rrd_updates page
-            for uuid in sr_mon_lookup:
+            # We use .pop() inside the loop, use list(dict_var.keys()):
+            for uuid in list(sr_mon_lookup.keys()):
                 if uuid not in sr_uuid_list:
                     sr_mon_lookup.pop(uuid)
             # Create monitors for SRs that have just appeared in rrd_updates page
@@ -1124,29 +1132,22 @@ def main():
             # And for the sr_mons
             for sr_mon in sr_mon_lookup.values():
                 sr_mon.process_rrd_updates(rrd_updates, session)
-
-        except socket.error as e:
-            if e.args[0] == 111:
-                # "Connection refused" - this happens when we try to restart session and *that* fails
-                time.sleep(2)
-                pass
-
-            log_err("caught socket.error: (%s) - restarting XAPI session" % " ".join([str(x) for x in e.args]))
+
+        except ConnectionRefusedError as e:
+            time.sleep(2)
+            log_err("caught connection refused error: (%s) - restarting XAPI session" % str(e))
             restart_session = True
-
-        except IOError as e:
-            if e.args[0] == 'http error' and e.args[1] in (401, 500):
+        except urllib.error.HTTPError as e:
+            if e.code in (401, 500):
                 # Error getting rrd_updates: 401=Unauthorised, 500=Internal - start new session
-                pass 
-            elif e.args[0] == 'socket error':
-                # This happens if we send messages or read other-config:perfmon after xapi is restarted
-                pass
+                log_err("caught http.error: (%s) - restarting XAPI session" % str(e))
+                restart_session = True
             else:
-                # Don't know why we got this error - crash, die and look at logs later
                 raise
-
-            log_err("caught IOError: (%s) - restarting XAPI session" % " ".join([str(x) for x in e.args]))
-            restart_session = True
+        except (ConnectionError, urllib.error.URLError) as e:
+            # This happens if we send messages or read other-config:perfmon after xapi is restarted
+            log_err("caught socket.error: (%s) - restarting XAPI session" % str(e))
+            restart_session = True   
 
         runs += 1
         if maxruns is not None and runs >= maxruns:
@@ -1246,7 +1247,7 @@ if __name__ == "__main__":
             log_err(errmsg)
         except Exception as ignored:
             try:
-                errmsg = "\n".join([ str(x) for x in e.details ])
+                errmsg = "\n".join([ str(x) for x in e.details ]) # pytype: disable=attribute-error
                 # print the exception args nicely
                 log_err(errmsg)
             except Exception as ignored:

scripts/test_hfx_filename.py

@@ -0,0 +1,127 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+"""
+This module provides unittest for hfx_filename
+"""
+
+import sys
+import unittest
+from mock import MagicMock, patch, call
+from import_file import get_module
+
+# mock modules to avoid dependencies
+sys.modules["XenAPI"] = MagicMock()
+
+hfx_filename = get_module("hfx_filename", "/hfx_filename")
+
+@unittest.skipIf(sys.version_info < (3, 0), reason="requires python3")
+@patch("socket.socket")
+class TestRpc(unittest.TestCase):
+    """
+    This class tests blow functions:
+    rpc()
+    db_get_uuid()
+    read_field()
+    """
+    def test_rpc(self, mock_socket):
+        """
+        Tests rpc
+        """
+        mock_connected_socket = MagicMock()
+        mock_socket.return_value = mock_connected_socket
+
+        recv_data = b"HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\nHelloWorld"
+        mock_connected_socket.recv.return_value = recv_data
+
+        session_id = 0
+        request = "socket request"
+        body = hfx_filename.rpc(session_id, request)
+
+        # Assert that the socket methods were called as expected
+        expected_data = [
+            b"POST /remote_db_access?session_id=0 HTTP/1.0\r\n",
+            b"Connection:close\r\n",
+            b"content-length:14\r\n",
+            b"\r\n",
+            b"socket request"
+        ]
+        mock_connected_socket.send.assert_has_calls([call(data) for data in expected_data])
+
+        expected_return = "HelloWorld"
+        self.assertEqual(expected_return, body)
+
+    def test_rpc_international_character(self, mock_socket):
+        """
+        Tests rpc using non-ascii characters
+        """
+        mock_connected_socket = MagicMock()
+        mock_socket.return_value = mock_connected_socket
+
+        recv_data = b"HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\nHelloWorld"
+        mock_connected_socket.recv.return_value = recv_data
+
+        session_id = 0
+        # Use international character"socket 请求" as request
+        request = "socket 请求"
+        body = hfx_filename.rpc(session_id, request)
+
+        # Assert that the socket methods were called as expected
+        expected_data = [
+            b"POST /remote_db_access?session_id=0 HTTP/1.0\r\n",
+            b"Connection:close\r\n",
+            b"content-length:13\r\n",
+            b"\r\n",
+            request.encode('utf-8')
+        ]
+        mock_connected_socket.send.assert_has_calls([call(data) for data in expected_data])
+
+        expected_return = "HelloWorld"
+        self.assertEqual(expected_return, body)
+
+    def test_db_get_uuid(self, mock_socket):
+        """
+        Tests db_get_uuid 
+        """
+        mock_connected_socket = MagicMock()
+        mock_socket.return_value = mock_connected_socket
+
+        header = "HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\n"
+        body = ("<value><array><data><value>success</value><value>HelloWorld"
+                "</value></data></array></value>")
+        recv_data = (header + body).encode('utf-8')
+        mock_connected_socket.recv.return_value = recv_data
+
+        expected_response = "HelloWorld"
+        response = hfx_filename.db_get_by_uuid(0, "pool_patch", "22345")
+        self.assertEqual(expected_response, response)
+
+    def test_read_field(self, mock_socket):
+        """
+        Tests read_field 
+        """
+        mock_connected_socket = MagicMock()
+        mock_socket.return_value = mock_connected_socket
+
+        header = "HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\n"
+        body = ("<value><array><data><value>success</value><value>file_name"
+                "</value></data></array></value>")
+        recv_data = (header + body).encode('utf-8')
+        mock_connected_socket.recv.return_value = recv_data
+
+        expected_filename = "file_name"
+        filename = hfx_filename.read_field(0, "pool_patch", "filename", "rf")
+        self.assertEqual(expected_filename, filename)
+@unittest.skipIf(sys.version_info < (3, 0), reason="requires python3")
+class TestParse(unittest.TestCase):
+    """
+    This class tests function parse_string()
+    """
+    def test_parse_string(self):
+        """
+        Tests parse_string
+        """
+        txt = ("<value><array><data><value>success</value><value>abcde"
+               "</value></data></array></value>")
+        expected_txt = "abcde"
+        return_txt = hfx_filename.parse_string(txt)
+        self.assertEqual(expected_txt, return_txt)

scripts/test_perfmon.py

@@ -5,35 +5,13 @@
 import unittest
 from mock import MagicMock, patch
 import sys
-import os
-import subprocess
 import math
+from import_file import get_module
 
 # mock modules to avoid dependencies
 sys.modules["XenAPI"] = MagicMock()
 
-def import_from_file(module_name, file_path):
-    """Import a file as a module"""
-    if sys.version_info.major == 2:
-        return None
-    else:
-        from importlib import machinery, util
-        loader = machinery.SourceFileLoader(module_name, file_path)
-        spec = util.spec_from_loader(module_name, loader)
-        assert spec
-        assert spec.loader
-        module = util.module_from_spec(spec)
-        # Probably a good idea to add manually imported module stored in sys.modules
-        sys.modules[module_name] = module
-        spec.loader.exec_module(module)
-        return module
-
-def get_module():
-    """Import the perfmon script as a module for executing unit tests on functions"""
-    testdir = os.path.dirname(__file__)
-    return import_from_file("perfmon", testdir + "/perfmon")
-
-perfmon = get_module()
+perfmon = get_module("perfmon", "/perfmon")
 @unittest.skipIf(sys.version_info < (3, 0), reason="requires python3")
 @patch("subprocess.getoutput")
 class TestGetPercentage(unittest.TestCase):