Merge pull request #129 from stfc/CI_CD_Monitoring_Tools

ENH: Added CI/CD for this folder

.github/workflows/MonitoringTools.yaml

@@ -0,0 +1,45 @@
+name: Pylint-Tests-Codecov
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    paths:
+      - "MonitoringTools/**"
+      - ".github/workflows/MonitoringTools.yaml"
+
+jobs:
+  Pylint-Tests-Codecov:
+    runs-on: ubuntu-20.04
+    strategy:
+      matrix:
+        python-version: ["3.x"]
+    steps:
+      - uses: actions/checkout@v3
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v3
+        with:
+          python-version: ${{ matrix.python-version }}
+          cache: "pip"
+
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          cd MonitoringTools
+          pip install -r requirements.txt
+
+      - name: Analyse with pylint
+        run: cd MonitoringTools && pylint $(git ls-files '*.py')
+
+      - name: Run tests and collect coverage
+        run: cd MonitoringTools && python3 -m pytest
+
+      - name: Run tests and collect coverage
+        run: cd MonitoringTools && python3 -m pytest . --cov-report xml:coverage.xml --cov
+
+      - name: Upload coverage to Codecov
+        uses: codecov/codecov-action@v3
+        with:
+          token: ${{secrets.CODECOV_TOKEN}}
+          files: ./MonitoringTools/coverage.xml

MonitoringTools/.coveragerc

@@ -0,0 +1,2 @@
+[run]
+omit = /usr/lib/*

MonitoringTools/.pylintrc

@@ -0,0 +1,11 @@
+[FORMAT]
+# Black will enforce 88 chars on Python code
+# this will enforce 120 chars on docs / comments
+max-line-length=118
+
+# Disable various warnings:
+# C0114: Missing module string - we don't need module strings for the small repo
+# W1401: Influxdb required backslashes
+# R0801: Duplicate code due to test case
+
+disable=C0114, W1401, R0801

MonitoringTools/pytest.ini

@@ -0,0 +1,6 @@
+[pytest]
+pythonpath = ./usr/local/bin
+testpaths = tests
+python_files = *.py
+python_functions = test_*
+addopts = --ignore=setup.py

MonitoringTools/requirements.txt

@@ -0,0 +1,4 @@
+openstacksdk
+pytest
+pylint
+pytest-cov

MonitoringTools/tests/test_limits_to_influx.py

@@ -84,24 +84,24 @@ def test_extract_limits_valid():
     """
     mock_project_limits_dict = {
         "server_meta": NonCallableMock(),
-        "personality":  NonCallableMock(),
-        "server_groups_used":  NonCallableMock(),
-        "image_meta":  NonCallableMock(),
-        "personality_size":  NonCallableMock(),
-        "keypairs":  NonCallableMock(),
-        "security_group_rules":  NonCallableMock(),
-        "server_groups":  NonCallableMock(),
-        "total_cores_used":  NonCallableMock(),
-        "total_ram_used":  NonCallableMock(),
-        "instances_used":  NonCallableMock(),
-        "security_groups":  NonCallableMock(),
-        "floating_ips_used":  NonCallableMock(),
-        "total_cores":  NonCallableMock(),
-        "server_group_members":  NonCallableMock(),
-        "floating_ips":  NonCallableMock(),
-        "security_groups_used":  NonCallableMock(),
-        "instances":  NonCallableMock(),
-        "total_ram":  NonCallableMock(),
+        "personality": NonCallableMock(),
+        "server_groups_used": NonCallableMock(),
+        "image_meta": NonCallableMock(),
+        "personality_size": NonCallableMock(),
+        "keypairs": NonCallableMock(),
+        "security_group_rules": NonCallableMock(),
+        "server_groups": NonCallableMock(),
+        "total_cores_used": NonCallableMock(),
+        "total_ram_used": NonCallableMock(),
+        "instances_used": NonCallableMock(),
+        "security_groups": NonCallableMock(),
+        "floating_ips_used": NonCallableMock(),
+        "total_cores": NonCallableMock(),
+        "server_group_members": NonCallableMock(),
+        "floating_ips": NonCallableMock(),
+        "security_groups_used": NonCallableMock(),
+        "instances": NonCallableMock(),
+        "total_ram": NonCallableMock(),
     }
     assert extract_limits(mock_project_limits_dict) == {
         "maxServerMeta": mock_project_limits_dict["server_meta"],
@@ -143,7 +143,9 @@ def test_get_limits_for_project(mock_openstack, mock_extract_limits):
     mock_openstack.connect.assert_called_once_with(mock_instance)
     mock_conn.get_compute_limits.assert_called_once_with(mock_project_id)
     mock_conn.get_volume_limits.assert_called_once_with(mock_project_id)
-    mock_extract_limits.assert_called_once_with(mock_conn.get_compute_limits.return_value)
+    mock_extract_limits.assert_called_once_with(
+        mock_conn.get_compute_limits.return_value
+    )
     assert res == {"lim1": "val1", "lim2": "val2"}
 
 

MonitoringTools/tests/test_send_metric_utils.py

@@ -60,6 +60,7 @@ def test_post_to_influxdb_valid(mock_requests):
         "http://localhost:8086/write?db=cloud&precision=s",
         data=mock_data_string,
         auth=(mock_user, mock_pass),
+        timeout=60,
     )
     mock_response = mock_requests.post.return_value
     mock_response.raise_for_status.assert_called_once()
@@ -81,7 +82,7 @@ def test_parse_args_valid_args(mock_read_config_file):
     """
     tests parse_args function with a valid filepath
     """
-    res = parse_args(["../usr/local/bin/influxdb.conf"])
+    res = parse_args(["./usr/local/bin/influxdb.conf"])
     assert res == mock_read_config_file.return_value
 
 
@@ -108,11 +109,9 @@ def test_parse_args_filepath_read_config_fails(mock_read_config_file):
     """
     mock_read_config_file.side_effect = configparser.Error
     with pytest.raises(RuntimeError):
-        parse_args(["../usr/local/bin/influxdb.conf"])
+        parse_args(["./usr/local/bin/influxdb.conf"])
 
-    mock_read_config_file.assert_called_once_with(
-        Path("../usr/local/bin/influxdb.conf")
-    )
+    mock_read_config_file.assert_called_once_with(Path("./usr/local/bin/influxdb.conf"))
 
 
 @patch("send_metric_utils.post_to_influxdb")

MonitoringTools/tests/test_service_status_to_influx.py

@@ -1,5 +1,4 @@
 from unittest.mock import patch, call, NonCallableMock, MagicMock
-import pytest
 
 from service_status_to_influx import (
     get_hypervisor_properties,

MonitoringTools/tests/test_slottifier.py

@@ -18,7 +18,7 @@
 
 @pytest.fixture(name="mock_hypervisors")
 def mock_hypervisors_fixture():
-    """ fixture for setting up various mock hvs"""
+    """fixture for setting up various mock hvs"""
     return {
         "hv1": {
             "name": "hv1",
@@ -36,15 +36,15 @@ def mock_hypervisors_fixture():
             "memory_size": 2048,
             "memory_used": 4096,
         },
-        "hv3": {
-            "name": "hv3",
-            "status": "disabled"
-        },
+        "hv3": {"name": "hv3", "status": "disabled"},
     }
 
 
 @pytest.fixture(name="mock_compute_services")
 def mock_service_fixture():
+    """
+    Returns a mock set of services to use as test data
+    """
     return {
         "svc1": {"host": "hv1", "name": "svc1"},
         "svc2": {"host": "hv2", "name": "svc2"},
@@ -54,7 +54,8 @@ def mock_service_fixture():
 
 @pytest.fixture(name="mock_aggregate")
 def mock_aggregate_fixture():
-    """ fixture for setting up a mock aggregate"""
+    """fixture for setting up a mock aggregate"""
+
     def _mock_aggregate(hosttype=None, gpu_num=None):
         """
         helper function for setting up mock aggregate
@@ -73,7 +74,7 @@ def _mock_aggregate(hosttype=None, gpu_num=None):
 
 @pytest.fixture(name="mock_flavors_list")
 def mock_flavors_fixture():
-    """ fixture for setting up various mock flavors """
+    """fixture for setting up various mock flavors"""
     return [
         {"id": 1, "extra_specs": {"aggregate_instance_extra_specs:hosttype": "A"}},
         {"id": 2, "extra_specs": {"aggregate_instance_extra_specs:hosttype": "B"}},
@@ -83,10 +84,8 @@ def mock_flavors_fixture():
     ]
 
 
-def test_get_hv_info_exists_and_enabled(
-    mock_hypervisors, mock_aggregate
-):
-    """ tests get_hv_info when hv exists and enabled - should parse results properly """
+def test_get_hv_info_exists_and_enabled(mock_hypervisors, mock_aggregate):
+    """tests get_hv_info when hv exists and enabled - should parse results properly"""
 
     assert get_hv_info(
         mock_hypervisors["hv1"], mock_aggregate(gpu_num="1"), {"status": "enabled"}
@@ -100,9 +99,7 @@ def test_get_hv_info_exists_and_enabled(
     }
 
 
-def test_get_hv_info_negative_results_floored(
-    mock_hypervisors, mock_aggregate
-):
+def test_get_hv_info_negative_results_floored(mock_hypervisors, mock_aggregate):
     """
     tests get_hv_info when results for available mem/cores are negative
         - should set it to 0 instead
@@ -120,9 +117,7 @@ def test_get_hv_info_negative_results_floored(
     }
 
 
-def test_get_hv_info_exists_but_disabled(
-    mock_hypervisors, mock_aggregate
-):
+def test_get_hv_info_exists_but_disabled(mock_hypervisors, mock_aggregate):
     """
     tests get_hv_info when hv is disabled - should return default results
     """
@@ -191,7 +186,7 @@ def test_get_valid_flavors_with_empty_flavors_list(mock_aggregate):
     """
     test get_valid_flavors_for_aggregate should return empty list if no flavors given
     """
-    assert get_valid_flavors_for_aggregate([], mock_aggregate("A")) == []
+    assert not get_valid_flavors_for_aggregate([], mock_aggregate("A"))
 
 
 def test_get_valid_flavors_with_non_matching_hosttype(
@@ -201,14 +196,14 @@ def test_get_valid_flavors_with_non_matching_hosttype(
     test get_valid_flavors_for_aggregate should return empty list if no flavors found with
     matching aggregate hosttype
     """
-    assert get_valid_flavors_for_aggregate(mock_flavors_list, mock_aggregate("D")) == []
+    assert not get_valid_flavors_for_aggregate(mock_flavors_list, mock_aggregate("D"))
 
 
 def test_convert_to_data_string_no_items():
     """
     Tests convert_to_data_string returns empty string when given empty dict as slots_dict
     """
-    assert convert_to_data_string(NonCallableMock(), {}) == ""
+    assert not convert_to_data_string(NonCallableMock(), {})
 
 
 def test_convert_to_data_string_one_item():
@@ -261,7 +256,7 @@ def test_calculate_slots_on_hv_non_gpu_disabled():
     """
     tests calculate_slots_on_hv calculates slots properly for non-gpu flavor
         - should return 0s since hv is disabled
-     """
+    """
     res = calculate_slots_on_hv(
         "flavor1",
         {"cores_required": 10, "mem_required": 10},
@@ -293,7 +288,7 @@ def test_calculate_slots_on_hv_gpu_no_gpunum():
                 # can fit 10 slots, but should be 0 since compute service disabled
                 "cores_available": 100,
                 "mem_available": 100,
-            }
+            },
         )
 
 
@@ -464,9 +459,13 @@ def test_get_all_hv_info_for_aggregate_with_valid_data(
     mock_get_hv_info.assert_has_calls(
         [
             # svc1 holds host: hv1
-            call(mock_hypervisors["hv1"], mock_aggregate, mock_compute_services["svc1"]),
+            call(
+                mock_hypervisors["hv1"], mock_aggregate, mock_compute_services["svc1"]
+            ),
             # svc2 holds host: hv2
-            call(mock_hypervisors["hv2"], mock_aggregate, mock_compute_services["svc2"]),
+            call(
+                mock_hypervisors["hv2"], mock_aggregate, mock_compute_services["svc2"]
+            ),
         ]
     )
     assert res == [mock_get_hv_info.return_value, mock_get_hv_info.return_value]
@@ -487,12 +486,10 @@ def test_get_all_hv_info_for_aggregate_with_invalid_data(
             "hv5",
         ]
     }
-    assert (
+    assert not (
         get_all_hv_info_for_aggregate(
-            mock_aggregate,
-            mock_compute_services.values(),
-            mock_hypervisors.values()
-        ) == []
+            mock_aggregate, mock_compute_services.values(), mock_hypervisors.values()
+        )
     )
 
 
@@ -504,12 +501,10 @@ def test_get_all_hv_info_for_aggregate_with_empty_aggregate(
     should do nothing and return empty list
     """
     mock_aggregate = {"hosts": []}
-    assert (
+    assert not (
         get_all_hv_info_for_aggregate(
-            mock_aggregate,
-            mock_hypervisors.values(),
-            mock_compute_services.values()
-        ) == []
+            mock_aggregate, mock_hypervisors.values(), mock_compute_services.values()
+        )
     )
 
 

MonitoringTools/tests/test_slottifier_entry.py

@@ -9,19 +9,19 @@ def test_add():
         slots_available=1,
         estimated_gpu_slots_used=1,
         max_gpu_slots_capacity=1,
-        max_gpu_slots_capacity_enabled=1
+        max_gpu_slots_capacity_enabled=1,
     )
 
     b = SlottifierEntry(
         slots_available=2,
         estimated_gpu_slots_used=3,
         max_gpu_slots_capacity=4,
-        max_gpu_slots_capacity_enabled=5
+        max_gpu_slots_capacity_enabled=5,
     )
 
     assert a + b == SlottifierEntry(
         slots_available=3,
         estimated_gpu_slots_used=4,
         max_gpu_slots_capacity=5,
-        max_gpu_slots_capacity_enabled=6
+        max_gpu_slots_capacity_enabled=6,
     )

MonitoringTools/usr/local/bin/limits_to_influx.py

@@ -1,4 +1,3 @@
-#!/usr/bin/python
 import sys
 from typing import Dict, List
 import openstack
@@ -86,7 +85,7 @@ def get_limits_for_project(instance: str, project_id) -> Dict:
     conn = openstack.connect(instance)
     project_details = {
         **extract_limits(conn.get_compute_limits(project_id)),
-        **conn.get_volume_limits(project_id)["absolute"]
+        **conn.get_volume_limits(project_id)["absolute"],
     }
     return project_details
 
@@ -111,7 +110,9 @@ def get_all_limits(instance: str) -> str:
     limit_details = {}
     for project in conn.list_projects():
         if is_valid_project(project):
-            limit_details[project["name"]] = get_limits_for_project(instance, project["id"])
+            limit_details[project["name"]] = get_limits_for_project(
+                instance, project["id"]
+            )
     return convert_to_data_string(instance, limit_details)