diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/on-push-pre-commit.yml similarity index 78% rename from .github/workflows/pre-commit.yml rename to .github/workflows/on-push-pre-commit.yml index f17e487..1fe8af0 100644 --- a/.github/workflows/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 new file mode 100644 index 0000000..8bf1dec --- /dev/null +++ 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 index ec30b5c..f43e5d4 100644 --- a/Makefile +++ b/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 diff --git a/mock_src/__init__.py b/mock_src/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/mock_src/rados.py b/mock_src/rados.py new file mode 100644 index 0000000..59bd7b8 --- /dev/null +++ b/mock_src/rados.py @@ -0,0 +1,3 @@ +# -*- coding: utf-8 -*- + +pass diff --git a/pyproject.toml b/pyproject.toml index 16f1b0a..d00c896 100644 --- 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/requirements.txt b/requirements.txt index 1c8606a..3f4fb3f 100644 --- a/requirements.txt +++ b/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 diff --git a/src/rookify/modules/ceph.py b/src/rookify/modules/ceph.py index 60e1e8c..ba16b87 100644 --- 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 index a35df0e..48168b8 100644 --- 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 index 392d6d9..17edc11 100644 --- a/src/rookify/modules/k8s.py +++ b/src/rookify/modules/k8s.py @@ -43,7 +43,7 @@ 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 @@ -51,7 +51,7 @@ 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 @@ -59,7 +59,7 @@ 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 @@ -67,7 +67,7 @@ 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 @@ -75,7 +75,7 @@ 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 index cb8c9d5..b8303c1 100644 --- 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(self, dry_run_mode: bool = False) -> None: diff --git a/src/rookify/modules/migrate_mds/main.py b/src/rookify/modules/migrate_mds/main.py index 05b1c29..bf0200a 100644 --- 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 index f86914e..a14792a 100644 --- 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 index 1040aa7..a44adc2 100644 --- 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 index 0a5d2ca..bf84dbf 100644 --- 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 index ec6ae8b..e5a734f 100644 --- 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,16 +27,16 @@ 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: @@ -45,28 +44,26 @@ def machine(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 diff --git a/tests/mock_ceph.py b/tests/mock_ceph.py index ce359d1..f3c0a6e 100644 --- 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 index 548412b..c7a9fd6 100644 --- 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 new file mode 100644 index 0000000..db34ed0 --- /dev/null +++ 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 new file mode 100644 index 0000000..2df3880 --- /dev/null +++ b/tests/modules/test_k8s_prerequisites_check.py @@ -0,0 +1,69 @@ +import unittest +from unittest.mock import Mock + +from rookify.modules.exception import ModuleException +from typing import Any +from ..mock_k8s_prerequisite_check import MockK8sPrerequisitesCheckHandler + +from kubernetes.client import ( + V1DeploymentList, + V1Namespace, + V1ObjectMeta, + V1NamespaceList, +) + + +# Note: currently this test works with pytest but not with unittest, which is not able to import needed classes +class TestK8sPrerequisitesCheckHandler(unittest.TestCase): + def setUp(self) -> None: + self.config = {"rook": {"cluster": {"namespace": "test-namespace"}}} + self.empty_deployment_list = False + + def _request_callback(self, method: str, *args: Any, **kwargs: Any) -> Any: + if method == "apps_v1_api.list_deployment_for_all_namespaces": + if self.empty_deployment_list is True: + return V1DeploymentList(items=[]) + + return V1DeploymentList(items=["apple", "banana", "cherry"]) + elif method == "core_v1_api.list_namespace": + # Create a list of Kubernetes V1Namespace objects + namespaces = [ + V1Namespace(metadata=V1ObjectMeta(name="default")), + V1Namespace(metadata=V1ObjectMeta(name="kube-system")), + V1Namespace(metadata=V1ObjectMeta(name="test-namespace")), + ] + + return V1NamespaceList(items=namespaces) + + def test_namespaces(self) -> None: + # Instantiate K8sPrerequisitesCheckHandler with the mock ModuleHandler + handler_instance = MockK8sPrerequisitesCheckHandler( + self._request_callback, Mock(), self.config + ) + # Set the k8s attribute to the mock_k8s instance + handler_instance.preflight() + + def test_list_deployment_for_all_namespaces_fails(self) -> None: + # Check the case where the deployment list is empty + self.empty_deployment_list = True + + # Instantiate K8sPrerequisitesCheckHandler with the mock ModuleHandler + handler_instance = MockK8sPrerequisitesCheckHandler( + self._request_callback, Mock(), self.config + ) + + # Call the preflight method to run the test + with self.assertRaises(ModuleException): + handler_instance.preflight() + + def test_namespaces_fails(self) -> None: + # Instantiate K8sPrerequisitesCheckHandler with the mock ModuleHandler + handler_instance = MockK8sPrerequisitesCheckHandler( + self._request_callback, Mock(), self.config + ) + + # Modify the config to have a different namespace than what is expected + handler_instance._config["rook"]["cluster"]["namespace"] = "wrong-namespace" + # Call the preflight method to run the test + with self.assertRaises(ModuleException): + handler_instance.preflight() diff --git a/tests/test_mock_ceph.py b/tests/test_mock_ceph.py index 4cb136b..7dd9b3c 100644 --- a/tests/test_mock_ceph.py +++ b/tests/test_mock_ceph.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -from typing import Any, Dict, Tuple +from typing import Any, Tuple from unittest import TestCase from .mock_ceph import MockCeph @@ -11,7 +11,7 @@ def setUp(self) -> None: self.ceph = MockCeph({}, self._command_callback) def _command_callback( - self, command: str, inbuf: bytes, **kwargs: Dict[Any, Any] + self, command: str, inbuf: bytes, **kwargs: Any ) -> Tuple[int, bytes, str]: if command == "test": return 0, b'["ok"]', "" diff --git a/tests/test_mock_k8s.py b/tests/test_mock_k8s.py index 9273bb3..982f130 100644 --- a/tests/test_mock_k8s.py +++ b/tests/test_mock_k8s.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -from typing import Any, Dict, List +from typing import Any from unittest import TestCase from .mock_k8s import MockK8s @@ -10,9 +10,7 @@ class TestMockK8s(TestCase): def setUp(self) -> None: self.k8s = MockK8s(self._request_callback) - def _request_callback( - self, method: str, *args: List[Any], **kwargs: Dict[Any, Any] - ) -> Any: + def _request_callback(self, method: str, *args: Any, **kwargs: Any) -> Any: if method == "core_v1_api.test": return True return None