Extend unit tests and add GitHub test workflow

* feat: adding test for k8s prerequisites
* Fix: change __variable to _variable in modules.py and adapt tests accordingly
* feat: add unit-test to check for empty response in list_deployment_for_all_namespaces()
* fix: add python3 for ubuntu distros
* fix: not specifying specific python version but finding it dynamically
* add pytest to requirements
* fix: changing name of response mock to V1DeploymentList
* fix: setting pytest to same version as ci
* fix: use kubernetes python cli itself to generate mock v1namespace list
* fix: better naming for flag for empty deployment list
* Code cleanup
* Remove `abstractmethod` decorator
* Add GitHub pytest workflow
* Fix unsupported string formatting code in `K8s`

Signed-off-by: Rafael te Boekhorst <[email protected]>
Co-authored-by: Tobias Wolf <[email protected]>

.github/workflows/pre-commit.yml → .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/[email protected]

.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

Makefile

@@ -18,8 +18,19 @@ else
 	CONTAINERCMD=podman
 endif
 
+# Checking if python exists
+ifneq (, $(shell command -v python))
+    $(info Python is installed as 'python')
+    PYTHON := $(shell command -v python)
+else ifneq (, $(shell command -v python3))
+    $(info Python3 is installed as 'python3')
+    PYTHON := $(shell command -v python3)
+else
+    $(error Neither python nor python3 is installed)
+endif
+
 ## Export default rookify version
-export ROOKIFY_VERSION ?= "0.0.0.dev1"
+export ROOKIFY_VERSION?=0.0.0.dev1
 
 .PHONY: help
 help: ## Display this help message
@@ -28,17 +39,16 @@ help: ## Display this help message
 	    awk -F ':.*?## ' 'NF==2 {printf "  %-26s%s\n\n", $$1, "${COLOUR_GREEN}"$$2"${COLOUR_END}"}'
 
 .PHONY: setup
-setup: setup-pre-commit check-radoslib setup-venv ## Setup the pre-commit environment and then the venv environment
+setup: check-radoslib setup-venv setup-pre-commit ## Setup the pre-commit environment and then the venv environment
 
 .PHONY: setup-pre-commit
 setup-pre-commit:
-	pip install --user pre-commit && pre-commit install
+	./.venv/bin/pip install --user pre-commit && ./.venv/bin/python -m pre_commit install
 
 .PHONY: setup-venv
 setup-venv:
-	python -m venv --system-site-packages ./.venv && \
-	source ./.venv/bin/activate && \
-	pip install -r requirements.txt
+	${PYTHON} -m venv --system-site-packages ./.venv && \
+	./.venv/bin/pip install -r requirements.txt
 
 .PHONY: run-precommit
 run-precommit: ## Run pre-commit to check if all files running through
@@ -66,9 +76,7 @@ check-radoslib: ## Checks if radoslib is installed and if it contains the right
 
 .PHONY: run-local-rookify
 run-local-rookify: ## Runs rookify in the local development environment (requires setup-venv)
-	$(eval PYTHONPATH="${PYTHONPATH}:$(pwd)/src")
-	source ./.venv/bin/activate && \
-	cd src && python -m rookify
+	source ./.venv/bin/activate && pip install -e . && rookify
 
 .PHONY: run-rookify
 run-rookify: ## Runs rookify in the container

mock_src/rados.py

@@ -0,0 +1,3 @@
+# -*- coding: utf-8 -*-
+
+pass

pyproject.toml

@@ -26,4 +26,4 @@ classifiers = [
 Homepage = "https://scs.community"
 
 [tool.pytest.ini_options]
-pythonpath = [ "src" ]
+pythonpath = [ "src", "mock_src" ]

requirements.txt

@@ -32,3 +32,4 @@ urllib3==2.2.1
 yamale==5.1.0
 websocket-client==1.7.0
 wrapt==1.16.0
+pytest==8.0.2

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"")

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")
 

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:

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(self, dry_run_mode: bool = False) -> None:

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)

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)

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):

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"]

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,45 +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 preflight(self) -> None:
         """
         Run the modules preflight check
         """
         pass
 
-    @abc.abstractmethod
     def execute(self) -> None:
         """
         Executes the modules tasks

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))