Merge branch 'main' into prs/adding-pickle-output-to-cli

NotTheEvilOne · NotTheEvilOne · commit 634d4d3c81ed · 2024-09-13T16:16:25.000+02:00
diff --git a/.github/workflows/on-push-pre-commit.yml b/.github/workflows/on-push-pre-commit.yml
@@ -1,4 +1,4 @@
-name: pre-commit
+name: py:pre-commit
 
 on:
   pull_request:
@@ -11,5 +11,5 @@ jobs:
     steps:
     - uses: actions/checkout@v3
     - uses: actions/setup-python@v3
-    - run: python -m pip install .[tests]
+    - run: pip install .[tests]
     - uses: pre-commit/action@v3.0.1
diff --git a/.github/workflows/on-push-test.yml b/.github/workflows/on-push-test.yml
@@ -0,0 +1,52 @@
+name: py:test
+
+on:
+  pull_request:
+  push:
+    branches: [main]
+
+jobs:
+  codeql-analysis:
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v3
+    - name: Set up Python
+      id: setup-python
+      uses: actions/setup-python@v3
+    - name: Get the Python path
+      id: get-python-path
+      run: echo "python-path=`which python`"
+    - name: Install dependencies
+      run: |-
+        pip install -r ./requirements.txt
+        pip install pylint --upgrade
+    - name: Initialize CodeQL
+      uses: github/codeql-action/init@v3
+      with:
+        languages: python
+        queries: security-extended
+    - name: Perform CodeQL analysis
+      env:
+        CODEQL_PYTHON: ${{ steps.get-python-path.outputs.python-path }}
+      uses: github/codeql-action/analyze@v3
+
+  test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: [ "3.9", "3.10", "3.11", "3.12" ]
+
+    steps:
+    - name: Checkout commit
+      uses: actions/checkout@v3
+    - name: Set up Python ${{ matrix.python-version }}
+      uses: actions/setup-python@v3
+      with:
+        python-version: ${{ matrix.python-version }}
+    - name: Install dependencies
+      run: |-
+        pip install -r ./requirements.txt
+    - name: Execute tests
+      run: pytest
diff --git a/Makefile b/Makefile
@@ -75,10 +75,7 @@ build-container: ## Build container from Dockerfile, add e.g. ROOKIFY_VERSION=0.
 
 .PHONY: run-local-rookify
 run-local-rookify: ## Runs rookify in the local development environment (requires setup-venv)
-	if [ ! -f ./.venv/bin/rookify ]; then \
-		${MAKE} build-local-rookify; \
-	fi; \
-	.venv/bin/rookify
+	source ./.venv/bin/activate && pip install -e . && rookify
 
 .PHONY: run-rookify
 run-rookify: ## Runs rookify in the container
diff --git a/mock_src/__init__.py b/mock_src/__init__.py
diff --git a/mock_src/rados.py b/mock_src/rados.py
@@ -0,0 +1,3 @@
+# -*- coding: utf-8 -*-
+
+pass
diff --git a/pyproject.toml b/pyproject.toml
@@ -26,4 +26,4 @@ classifiers = [
 Homepage = "https://scs.community"
 
 [tool.pytest.ini_options]
-pythonpath = [ "src" ]
+pythonpath = [ "src", "mock_src" ]
diff --git a/src/rookify/modules/ceph.py b/src/rookify/modules/ceph.py
@@ -2,7 +2,7 @@
 
 import json
 import rados
-from typing import Any, Dict, List
+from typing import Any, Dict
 from .exception import ModuleException
 
 
@@ -19,7 +19,7 @@ def __init__(self, config: Dict[str, Any]):
     def __getattr__(self, name: str) -> Any:
         return getattr(self.__ceph, name)
 
-    def _json_command(self, handler: Any, *args: Any) -> Dict[str, Any] | List[Any]:
+    def _json_command(self, handler: Any, *args: Any) -> Any:
         result = handler(*args)
         if result[0] != 0:
             raise ModuleException(f"Ceph did return an error: {result}")
@@ -53,19 +53,17 @@ def get_osd_pool_configurations_from_osd_dump(
 
         return osd_pools
 
-    def mon_command(self, command: str, **kwargs: Any) -> Dict[str, Any] | List[Any]:
+    def mon_command(self, command: str, **kwargs: Any) -> Any:
         cmd = {"prefix": command, "format": "json"}
         cmd.update(**kwargs)
         return self._json_command(self.__ceph.mon_command, json.dumps(cmd), b"")
 
-    def mgr_command(self, command: str, **kwargs: Any) -> Dict[str, Any] | List[Any]:
+    def mgr_command(self, command: str, **kwargs: Any) -> Any:
         cmd = {"prefix": command, "format": "json"}
         cmd.update(**kwargs)
         return self._json_command(self.__ceph.mgr_command, json.dumps(cmd), b"")
 
-    def osd_command(
-        self, osd_id: int, command: str, **kwargs: Any
-    ) -> Dict[str, Any] | List[Any]:
+    def osd_command(self, osd_id: int, command: str, **kwargs: Any) -> Any:
         cmd = {"prefix": command, "format": "json"}
         cmd.update(**kwargs)
         return self._json_command(self.__ceph.osd_command, osd_id, json.dumps(cmd), b"")
diff --git a/src/rookify/modules/create_rook_resources/main.py b/src/rookify/modules/create_rook_resources/main.py
@@ -106,11 +106,11 @@ def execute(self) -> None:
             mon="allow *",
             mgr="allow *",
             mds="allow *",
-        )  # type: ignore
+        )
 
         mon_auth: Dict[str, Any] = self.ceph.mon_command(
             "auth get-or-create-key", entity="mon.", mon="allow *"
-        )  # type: ignore
+        )
 
         metadata = kubernetes.client.V1ObjectMeta(name="rook-ceph-mon")
 
diff --git a/src/rookify/modules/k8s.py b/src/rookify/modules/k8s.py
@@ -43,39 +43,39 @@ def mds_placement_label(self) -> str:
         return (
             str(self._rook_config["cluster"]["mds_placement_label"])
             if "mds_placement_label" in self._rook_config["cluster"]
-            else f"placement-{self._rook_config["cluster"]["name"]}-mds"
+            else "placement-{0}-mds".format(self._rook_config["cluster"]["name"])
         )
 
     @property
     def mon_placement_label(self) -> str:
         return (
             str(self._rook_config["cluster"]["mon_placement_label"])
             if "mon_placement_label" in self._rook_config["cluster"]
-            else f"placement-{self._rook_config["cluster"]["name"]}-mon"
+            else "placement-{0}-mon".format(self._rook_config["cluster"]["name"])
         )
 
     @property
     def mgr_placement_label(self) -> str:
         return (
             str(self._rook_config["cluster"]["mgr_placement_label"])
             if "mgr_placement_label" in self._rook_config["cluster"]
-            else f"placement-{self._rook_config["cluster"]["name"]}-mgr"
+            else "placement-{0}-mgr".format(self._rook_config["cluster"]["name"])
         )
 
     @property
     def osd_placement_label(self) -> str:
         return (
             str(self._rook_config["cluster"]["osd_placement_label"])
             if "osd_placement_label" in self._rook_config["cluster"]
-            else f"placement-{self._rook_config["cluster"]["name"]}-osd"
+            else "placement-{0}-osd".format(self._rook_config["cluster"]["name"])
         )
 
     @property
     def rgw_placement_label(self) -> str:
         return (
             str(self._rook_config["cluster"]["rgw_placement_label"])
             if "rgw_placement_label" in self._rook_config["cluster"]
-            else f"placement-{self._rook_config["cluster"]["name"]}-rgw"
+            else "placement-{0}-rgw".format(self._rook_config["cluster"]["name"])
         )
 
     def check_nodes_for_initial_label_state(self, label: str) -> None:
diff --git a/src/rookify/modules/machine.py b/src/rookify/modules/machine.py
@@ -20,10 +20,10 @@ def __init__(self, machine_pickle_file: Optional[str] = None) -> None:
 
         _Machine.__init__(self, states=["uninitialized"], initial="uninitialized")
 
-    def add_execution_state(self, name: str, **kwargs: Dict[str, Any]) -> None:
+    def add_execution_state(self, name: str, **kwargs: Any) -> None:
         self._execution_states.append(self.__class__.state_cls(name, **kwargs))
 
-    def add_preflight_state(self, name: str, **kwargs: Dict[str, Any]) -> None:
+    def add_preflight_state(self, name: str, **kwargs: Any) -> None:
         self._preflight_states.append(self.__class__.state_cls(name, **kwargs))
 
     def execute(
diff --git a/src/rookify/modules/migrate_mds/main.py b/src/rookify/modules/migrate_mds/main.py
@@ -130,7 +130,7 @@ def _enable_rook_based_mds(self, mds_host: str) -> None:
         while True:
             result = self.ceph.mon_command("node ls")
 
-            if mds_host in result["mds"]:  # type: ignore
+            if mds_host in result["mds"]:
                 break
 
             sleep(2)
diff --git a/src/rookify/modules/migrate_osds/main.py b/src/rookify/modules/migrate_osds/main.py
@@ -26,7 +26,7 @@ def _get_devices_of_hosts(self) -> Dict[str, Dict[str, str]]:
 
                 result = self.ssh.command(
                     osd_host,
-                    "sudo pvdisplay -c /dev/{0}".format(osd_data["devices"]),  # type:ignore
+                    "sudo pvdisplay -c /dev/{0}".format(osd_data["devices"]),
                 )
 
                 if result.failed:
@@ -138,7 +138,7 @@ def migrate_osds(self, host: str, osd_ids: List[int]) -> None:
             while True:
                 osd_status = self.ceph.mon_command("osd info", id=osd_id)
 
-                if osd_status["up"] == 0:  # type: ignore
+                if osd_status["up"] == 0:
                     break
 
                 sleep(2)
@@ -182,7 +182,7 @@ def migrate_osds(self, host: str, osd_ids: List[int]) -> None:
             while True:
                 osd_status = self.ceph.mon_command("osd info", id=osd_id)
 
-                if osd_status["up"] != 0:  # type: ignore
+                if osd_status["up"] != 0:
                     break
 
                 sleep(2)
diff --git a/src/rookify/modules/migrate_rgw_pools/main.py b/src/rookify/modules/migrate_rgw_pools/main.py
@@ -21,7 +21,7 @@ def preflight(self) -> None:
 
         service_data = self.ceph.mon_command("service dump")
 
-        rgw_daemons = service_data["services"].get("rgw", {}).get("daemons", {})  # type: ignore
+        rgw_daemons = service_data["services"].get("rgw", {}).get("daemons", {})
 
         for rgw_daemon in rgw_daemons.values():
             if not isinstance(rgw_daemon, dict):
diff --git a/src/rookify/modules/migrate_rgws/main.py b/src/rookify/modules/migrate_rgws/main.py
@@ -13,7 +13,7 @@ class MigrateRgwsHandler(ModuleHandler):
     def _get_rgw_daemon_hosts(self) -> List[str]:
         ceph_status = self.ceph.mon_command("status")
 
-        rgw_daemons = ceph_status["servicemap"]["services"]["rgw"]["daemons"]  # type: ignore
+        rgw_daemons = ceph_status["servicemap"]["services"]["rgw"]["daemons"]
         rgw_daemon_hosts = []
         if "summary" in rgw_daemons:
             del rgw_daemons["summary"]
diff --git a/src/rookify/modules/module.py b/src/rookify/modules/module.py
@@ -1,7 +1,6 @@
 # -*- coding: utf-8 -*-
 
 import os
-import abc
 import structlog
 from typing import Any, Dict, Optional
 from ..logger import get_logger
@@ -12,7 +11,7 @@
 from .template import Template
 
 
-class ModuleHandler:
+class ModuleHandler(object):
     """
     ModuleHandler is an abstract class that modules have to extend.
     """
@@ -28,52 +27,43 @@ def __init__(self, machine: Machine, config: Dict[str, Any]):
         self._config = config
         self._machine = machine
 
-        self.__ceph: Optional[Ceph] = None
-        self.__k8s: Optional[K8s] = None
-        self.__ssh: Optional[SSH] = None
-        self.__logger = get_logger()
+        self._ceph: Optional[Ceph] = None
+        self._k8s: Optional[K8s] = None
+        self._ssh: Optional[SSH] = None
+        self._logger = get_logger()
 
     @property
     def ceph(self) -> Ceph:
-        if self.__ceph is None:
-            self.__ceph = Ceph(self._config["ceph"])
-        return self.__ceph
+        if self._ceph is None:
+            self._ceph = Ceph(self._config["ceph"])
+        return self._ceph
 
     @property
     def machine(self) -> Machine:
         return self._machine
 
     @property
     def k8s(self) -> K8s:
-        if self.__k8s is None:
-            self.__k8s = K8s(self._config)
-        return self.__k8s
+        if self._k8s is None:
+            self._k8s = K8s(self._config)
+        return self._k8s
 
     @property
     def logger(self) -> structlog.getLogger:
-        return self.__logger
+        return self._logger
 
     @property
     def ssh(self) -> SSH:
-        if self.__ssh is None:
-            self.__ssh = SSH(self._config["ssh"])
-        return self.__ssh
+        if self._ssh is None:
+            self._ssh = SSH(self._config["ssh"])
+        return self._ssh
 
-    @abc.abstractmethod
-    def status(self) -> None:
-        """
-        Run the modules status check
-        """
-        pass
-
-    @abc.abstractmethod
     def preflight(self) -> None:
         """
         Run the modules preflight check
         """
         pass
 
-    @abc.abstractmethod
     def execute(self) -> None:
         """
         Executes the modules tasks
diff --git a/tests/mock_ceph.py b/tests/mock_ceph.py
@@ -3,7 +3,7 @@
 import json
 from collections.abc import Callable
 from rookify.modules.exception import ModuleException
-from typing import Any, Dict, List, Tuple
+from typing import Any, Dict, Tuple
 
 
 class MockCeph(object):
@@ -17,9 +17,7 @@ def __init__(
 
         self._callback_handler = _callable
 
-    def mon_command(
-        self, command: str, inbuf: bytes, **kwargs: Any
-    ) -> Dict[str, Any] | List[Any]:
+    def mon_command(self, command: str, inbuf: bytes, **kwargs: Any) -> Any:
         ret, outbuf, outstr = self._callback_handler(command, inbuf, **kwargs)
         if ret != 0:
             raise ModuleException("Ceph did return an error: {0!r}".format(outbuf))
diff --git a/tests/mock_k8s.py b/tests/mock_k8s.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 
 from collections.abc import Callable
-from typing import Any, Dict, List
+from typing import Any
 
 
 class MockK8s(object):
@@ -12,7 +12,7 @@ def __init__(self, _callable: Callable[[str], Any], name: str = "") -> None:
         self._callback_handler = _callable
         self._attr_name = name
 
-    def __call__(self, *args: List[Any], **kwargs: Dict[Any, Any]) -> Any:
+    def __call__(self, *args: Any, **kwargs: Any) -> Any:
         return self._callback_handler(self._attr_name, *args, **kwargs)
 
     def __getattr__(self, name: str) -> Any:
diff --git a/tests/mock_k8s_prerequisite_check.py b/tests/mock_k8s_prerequisite_check.py
@@ -0,0 +1,12 @@
+# -*- coding: utf-8 -*-
+
+from rookify.modules.k8s_prerequisites_check.main import K8sPrerequisitesCheckHandler
+from typing import Any
+from .mock_k8s import MockK8s
+
+
+# Note: currently this test works with pytest but not with unittest, which is not able to import needed classes
+class MockK8sPrerequisitesCheckHandler(K8sPrerequisitesCheckHandler):
+    def __init__(self, request_callback: Any, *args: Any, **kwargs: Any) -> None:
+        K8sPrerequisitesCheckHandler.__init__(self, *args, **kwargs)
+        self._k8s = MockK8s(request_callback)  # type: ignore
diff --git a/tests/modules/test_k8s_prerequisites_check.py b/tests/modules/test_k8s_prerequisites_check.py
diff --git a/tests/test_mock_ceph.py b/tests/test_mock_ceph.py
diff --git a/tests/test_mock_k8s.py b/tests/test_mock_k8s.py

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+# -- coding: utf-8 --`
	`2`	`+`
	`3`	`+pass`