Merge pull request #122 from liulinC/private/linl/yangtze

Backport `Bugtool should contain up-to-date RRDs fix` to Yangtze

.github/workflows/main.yml

@@ -85,4 +85,10 @@ jobs:
           flags: python2.7-xs8-yangtze
           env_vars: OS,PYTHON
           name: Python 2.7 Coverage for XS8/Yangtze
+          # Whether the job should fail if Codecov runs into an error during upload.
+          # Not failing the job in this case is ok because the pre-commit checks
+          # also contain a diff-cover job which would fail on missing changed lines:
+          fail_ci_if_error: false
+          flags: >
+            ${{ format('python{0}', steps.python.outputs.python-version ) }}
           verbose: true

.vscode/ltex.dictionary.en-US.txt

@@ -98,6 +98,7 @@ init
 initialisation
 initialised
 initiatorname
+inotify
 iomem
 ioports
 iscsi

requirements-dev.txt

@@ -1,3 +1,5 @@
 lxml
+mock
 pytest-coverage
 pytest-mock
+types-mock

tests/integration/dom0-template/etc/xensource/bugtool/mock/stuff.xml

@@ -3,6 +3,34 @@
 <files>/etc/passwd</files>
 <files>/etc/group</files>
 <files>/proc/self/status</files>
+
+<!--
+Test that multiple patterns work together and both are collected.
+
+Two patterns for the same directory, and each pattern matches one file:
+- the 1st pattern is negated and matches the file max_queued_events
+- the 2nd pattern matches the file max_user_instances:
+-->
+<directory pattern=".*user_.*" negate="yes">/proc/sys/fs/inotify</directory>
+<directory pattern=".*max_user_instances.*">/proc/sys/fs/inotify</directory>
+<!--
+Expected result, asserted by tests/unit/test_output.py:
+- 1st entry: /proc/sys/fs/inotify/max_queued_events
+- 2nd entry: /proc/sys/fs/inotify/max_user_instances
+-->
+
+<!--
+Test that the 2nd directory pattern does not affect the first directory pattern,
+even when it matches no file:
+- the 1st pattern matches the file /proc/sys/fs/epoll/max_user_watches
+- the 2nd pattern matches no file
+-->
 <directory pattern=".*ax_user_watches">/proc/sys/fs/epoll</directory>
+<directory pattern="no" negate="false">/proc/sys/fs/epoll</directory>
+<!--
+Expected result, asserted by tests/unit/test_output.py:
+- /proc/sys/fs/epoll/max_user_watches
+-->
+
 <command label="proc_version">cat /proc/version</command>
 </collect>

tests/unit/test_dump_xapi_rrds.py

@@ -0,0 +1,160 @@
+"""Test: test_dump_xapi_rrds"""
+
+import sys
+from email.message import Message
+
+import pytest
+from mock import MagicMock
+
+if sys.version_info.major == 2:  # pragma: no cover
+    from urllib2 import HTTPError  # type:ignore[import-not-found,attr-defined]
+else:
+    from urllib.request import HTTPError
+
+
+@pytest.fixture
+def mock_session1():
+    """Return a mock XenAPI session.
+
+    xapi_local_session is mocked to return this session.
+
+    The pool has 3 VMs, one of which is not resident and one template.
+    The test runs on the pool master.
+
+    Because the session runs on the pool master, dump_xapi_rrds() will
+    fetch the RRDs of resident VMs on any host and of the pool master.
+    """
+    session = MagicMock()
+    session._session = "id"
+    session.xenapi.login_with_password.return_value = "session_id"
+
+    # Simulates that the pool has 3 VMs, one of them is a template:
+    session.xenapi.VM.get_all_records.return_value = {
+        "invalid VM: mock_urlopen will return 404 on opening the 1st URL": {
+            "uuid": "0",
+            "is_a_template": False,
+            "power_state": "Suspended",
+            "resident_on": "host0",
+        },
+        "vm1": {
+            "uuid": "1",
+            "is_a_template": False,
+            "resident_on": "host1",
+            "power_state": "Running",
+        },
+        "vm2": {
+            "uuid": "2",
+            "is_a_template": False,
+            "resident_on": "host2",
+            "power_state": "Suspended",
+        },
+        "vm4": {
+            "uuid": "4",
+            "is_a_template": False,
+            "resident_on": "OpaqueRef:NULL",
+        },
+        "template": {"is_a_template": True},
+    }
+    # Simulate that the test runs on the pool master:
+    session.xenapi.pool.get_all_records.return_value = {
+        "pool1": {"master": "host1"},
+    }
+    session.xenapi.session.get_this_host.return_value = "host1"
+    return session
+
+
+@pytest.fixture
+def mock_urlopen():
+    """Return a mock urlopen() that returns different mock RRD data on each call"""
+
+    mock_response = MagicMock()
+    mock_response.read.side_effect = [b"mock_rrd1", b"mock_rrd2", b"mock_rrd3"]
+
+    mock_urlopen = MagicMock(return_value=mock_response)
+
+    # Mock the urlopen() to return the mock_response with different data on each call
+    side_effect = [HTTPError("url", 404, "Not Found", Message(), None)]
+
+    side_effect += [mock_response] * 3
+    mock_urlopen.side_effect = side_effect
+
+    # Mock the fileno() method to return 0 for use by select(), except for Python3,
+    # urlopen returns http.client.HTTPResponse, which doesn't have a fileno() method:
+    if sys.version_info.major == 2:  # pragma: no cover
+        mock_urlopen.return_value.fileno.return_value = 0
+
+    return mock_urlopen
+
+
+def assert_mock_session1(bugtool, mock_urlopen):
+    """Assert the results expected from the mock_session1() fixture.
+
+    The pool has 3 VMs, one of which is not resident and one template.
+
+    Because the session was run on the pool master, dump_xapi_rrds() is expected
+    fetch the RRDs of resident VMs on any host and of the pool master.
+    """
+    # Expect to see 3 calls to urlopen, one for each VM and one for the host
+    assert mock_urlopen.return_value.read.call_count == 3
+
+    # Assert that the RRDs of both VM are fetched
+    mock_urlopen.assert_any_call(
+        "http://localhost/vm_rrd?session_id=id&uuid=1",
+        timeout=5,
+    )
+    mock_urlopen.assert_any_call(
+        "http://localhost/vm_rrd?session_id=id&uuid=2",
+        timeout=5,
+    )
+
+    # Check the keys of the data dictionary
+    files = sorted(bugtool.data.keys())
+    assert files == ["xapi_rrd-1.out", "xapi_rrd-2.out", "xapi_rrd-host"]
+
+    # Check the cap values of the data dictionary
+    expected_caps = ["persistent-stats"] * 3
+    assert [key["cap"] for key in bugtool.data.values()] == expected_caps
+
+    # Call the func_output() functions and check the return values
+    values = [key["func"]("") for key in bugtool.data.values() if "func" in key]
+    assert sorted(values) == [b"mock_rrd1", b"mock_rrd2", b"mock_rrd3"]
+
+    with open(bugtool.XEN_BUGTOOL_LOG, "r") as f:
+        log = f.read()
+        assert log == "Failed to fetch RRD for VM 0: HTTP Error 404: Not Found\n"
+
+    # Clear the log file, this indicates to the isolated_bugtool fixture
+    # that we checked the log file contents to be what we expected.
+    with open(bugtool.XEN_BUGTOOL_LOG, "w") as f:
+        f.write("")
+
+
+def run_dump_xapi_rrds(mocker, bugtool, mock_session, mock_urlopen):
+    """Run the bugtool function dump_xapi_rrds(entries) with the given mocks."""
+    # Patch the urlopen, xapi_local_session and entries
+    mocker.patch("xen-bugtool.urlopen", side_effect=mock_urlopen)
+    mocker.patch("xen-bugtool.xapi_local_session", return_value=mock_session)
+    mocker.patch("xen-bugtool.entries", [bugtool.CAP_PERSISTENT_STATS])
+
+    # Run the function
+    bugtool.dump_xapi_rrds(bugtool.entries)
+
+    # Check if host RRD is fetched
+    mock_urlopen.assert_any_call("http://localhost/host_rrd?session_id=id")
+
+    # Check the calls to xapi_local_session
+    assert mock_session.xenapi.VM.get_all_records.call_count == 1
+    assert mock_session.xenapi.pool.get_all_records.call_count == 1
+    assert mock_session.xenapi.session.get_this_host.call_count == 1
+    assert mock_session.xenapi.session.logout.call_count == 1
+
+
+def test_dump_xapi_rrds_master(mocker, isolated_bugtool, mock_session1, mock_urlopen):
+    """Test dump_xapi_rrds() on a pool master with 2 VMs in the pool.
+
+    Test the bugtool function dump_xapi_rrds(entries) to perform as expected
+    with mock_session1 which simulates a pool with 2 VMs and a pool master.
+    """
+
+    run_dump_xapi_rrds(mocker, isolated_bugtool, mock_session1, mock_urlopen)
+    assert_mock_session1(isolated_bugtool, mock_urlopen)

tests/unit/test_load_plugins.py

@@ -44,13 +44,31 @@ def test_load_plugins(bugtool, dom0_template):
             "cap": "mock",
             "filename": "/proc/self/status",
         },
-        "/proc/sys/fs/epoll/max_user_watches": {
-            "cap": "mock",
-            "filename": "/proc/sys/fs/epoll/max_user_watches",
-        },
         "proc_version": {
             "cap": "mock",
             "cmd_args": "cat /proc/version",
             "filter": None,
         },
     }
+
+    # Assert the tree_output entries for /proc/sys/fs/inotify:
+    entry_one, entry_two = bugtool.directory_specifications["/proc/sys/fs/inotify"]
+    cap, regex, negate = entry_one
+    assert cap == "mock"
+    assert regex.pattern == ".*user_.*"
+    assert negate
+    cap, regex, negate = entry_two
+    assert cap == "mock"
+    assert regex.pattern == ".*max_user_instances.*"
+    assert not negate
+
+    # Assert the tree_output entry for /proc/sys/fs/epoll:
+    entry_one, entry_two = bugtool.directory_specifications["/proc/sys/fs/epoll"]
+    cap, regex, negate = entry_one
+    assert cap == "mock"
+    assert regex.pattern == ".*ax_user_watches"
+    assert not negate
+    cap, regex, negate = entry_two
+    assert cap == "mock"
+    assert regex.pattern == "no"
+    assert not negate

tests/unit/test_output.py

@@ -24,6 +24,8 @@ def assert_mock_bugtool_plugin_output(temporary_directory, subdir, names):
         subdir + "/ls-l-%etc.out",
         subdir + "/proc/self/status",
         subdir + "/proc/sys/fs/epoll/max_user_watches",
+        subdir + "/proc/sys/fs/inotify/max_queued_events",
+        subdir + "/proc/sys/fs/inotify/max_user_instances",
         subdir + "/proc_version.out",
     ]
     assert sorted(names) == expected_names