From 0d69561125df5a9d969459d6730efb8cb3f14463 Mon Sep 17 00:00:00 2001 From: Rafael te Boekhorst Date: Tue, 11 Jun 2024 16:47:09 +0200 Subject: [PATCH 01/14] feat: adding test for k8s prerequisites fix: adding 'object' to module path in order to use correct class definition, allows to remove ignore comments for mypy Signed-off-by: Rafael te Boekhorst --- src/rookify/modules/module.py | 2 +- tests/mock_k8s_prerequisite_check.py | 15 +++++ tests/modules/test_k8s_prerequisites_check.py | 60 +++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 tests/mock_k8s_prerequisite_check.py create mode 100644 tests/modules/test_k8s_prerequisites_check.py diff --git a/src/rookify/modules/module.py b/src/rookify/modules/module.py index ec6ae8b..2098a54 100644 --- a/src/rookify/modules/module.py +++ b/src/rookify/modules/module.py @@ -12,7 +12,7 @@ from .template import Template -class ModuleHandler: +class ModuleHandler(object): """ ModuleHandler is an abstract class that modules have to extend. """ diff --git a/tests/mock_k8s_prerequisite_check.py b/tests/mock_k8s_prerequisite_check.py new file mode 100644 index 0000000..a18152a --- /dev/null +++ b/tests/mock_k8s_prerequisite_check.py @@ -0,0 +1,15 @@ +# -*- 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) + + def execute(self) -> None: + K8sPrerequisitesCheckHandler.execute(self) diff --git a/tests/modules/test_k8s_prerequisites_check.py b/tests/modules/test_k8s_prerequisites_check.py new file mode 100644 index 0000000..e91daa8 --- /dev/null +++ b/tests/modules/test_k8s_prerequisites_check.py @@ -0,0 +1,60 @@ +import unittest +from unittest.mock import Mock + +from rookify.modules.exception import ModuleException +from typing import Any, Dict, List +from ..mock_k8s_prerequisite_check import MockK8sPrerequisitesCheckHandler + + +# 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: + # Mock configuration + self.config = {"rook": {"cluster": {"namespace": "test-namespace"}}} + + def _request_callback( + self, method: str, *args: List[Any], **kwargs: Dict[Any, Any] + ) -> Any: + if method == "core_v1_api.list_namespace": + return self._mock_list_namespace() + + def _mock_list_namespace(self) -> Any: + class Metadata: + def __init__(self, name: str): + self.name = name + + class Namespace: + def __init__(self, name: str): + self.metadata = Metadata(name) + + class NamespaceList: + def __init__(self, items: List[Namespace]): + self.items = items + + return NamespaceList( + [ + Namespace("default"), + Namespace("kube-system"), + Namespace("test-namespace"), + ] + ) + + 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_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() From 85503ccb82f640c77ef158584856f5da2d4170a9 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Mon, 9 Sep 2024 12:03:22 +0200 Subject: [PATCH 02/14] Fix: change __variable to _variable in modules.py and adapt tests accordingly Signed-off-by: Boekhorst --- src/rookify/modules/module.py | 28 +++++++++---------- tests/mock_k8s_prerequisite_check.py | 2 +- tests/modules/test_k8s_prerequisites_check.py | 8 ++++++ 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/rookify/modules/module.py b/src/rookify/modules/module.py index 2098a54..f787bf7 100644 --- a/src/rookify/modules/module.py +++ b/src/rookify/modules/module.py @@ -28,16 +28,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,19 +45,19 @@ 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: diff --git a/tests/mock_k8s_prerequisite_check.py b/tests/mock_k8s_prerequisite_check.py index a18152a..6be782d 100644 --- a/tests/mock_k8s_prerequisite_check.py +++ b/tests/mock_k8s_prerequisite_check.py @@ -9,7 +9,7 @@ class MockK8sPrerequisitesCheckHandler(K8sPrerequisitesCheckHandler): def __init__(self, request_callback: Any, *args: Any, **kwargs: Any) -> None: K8sPrerequisitesCheckHandler.__init__(self, *args, **kwargs) - self._k8s = MockK8s(request_callback) + self._k8s = MockK8s(request_callback) # type: ignore def execute(self) -> None: K8sPrerequisitesCheckHandler.execute(self) diff --git a/tests/modules/test_k8s_prerequisites_check.py b/tests/modules/test_k8s_prerequisites_check.py index e91daa8..689aafe 100644 --- a/tests/modules/test_k8s_prerequisites_check.py +++ b/tests/modules/test_k8s_prerequisites_check.py @@ -15,6 +15,8 @@ def setUp(self) -> None: def _request_callback( self, method: str, *args: List[Any], **kwargs: Dict[Any, Any] ) -> Any: + if method == "apps_v1_api.list_deployment_for_all_namespaces": + return MockResponse(["apple", "banana", "cherry"]) if method == "core_v1_api.list_namespace": return self._mock_list_namespace() @@ -58,3 +60,9 @@ def test_namespaces_fails(self) -> None: # Call the preflight method to run the test with self.assertRaises(ModuleException): handler_instance.preflight() + + +# Create a Mock respons object +class MockResponse: + def __init__(self, items: List[str]) -> None: + self.items = items From 28f5dfdcbbc3acacbd29b41690de52648fe72b1d Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Mon, 9 Sep 2024 12:33:53 +0200 Subject: [PATCH 03/14] feat: add unit-test to check for empty response in list_deployment_for_all_namespaces() Signed-off-by: Boekhorst --- tests/mock_k8s_prerequisite_check.py | 4 ++-- tests/modules/test_k8s_prerequisites_check.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/mock_k8s_prerequisite_check.py b/tests/mock_k8s_prerequisite_check.py index 6be782d..470fca0 100644 --- a/tests/mock_k8s_prerequisite_check.py +++ b/tests/mock_k8s_prerequisite_check.py @@ -6,10 +6,10 @@ # Note: currently this test works with pytest but not with unittest, which is not able to import needed classes -class MockK8sPrerequisitesCheckHandler(K8sPrerequisitesCheckHandler): +class MockK8sPrerequisitesCheckHandler(K8sPrerequisitesCheckHandler): # type: ignore def __init__(self, request_callback: Any, *args: Any, **kwargs: Any) -> None: K8sPrerequisitesCheckHandler.__init__(self, *args, **kwargs) - self._k8s = MockK8s(request_callback) # type: ignore + self._k8s = MockK8s(request_callback) def execute(self) -> None: K8sPrerequisitesCheckHandler.execute(self) diff --git a/tests/modules/test_k8s_prerequisites_check.py b/tests/modules/test_k8s_prerequisites_check.py index 689aafe..6e39707 100644 --- a/tests/modules/test_k8s_prerequisites_check.py +++ b/tests/modules/test_k8s_prerequisites_check.py @@ -11,11 +11,15 @@ class TestK8sPrerequisitesCheckHandler(unittest.TestCase): def setUp(self) -> None: # Mock configuration self.config = {"rook": {"cluster": {"namespace": "test-namespace"}}} + # No response option + self.empty_response = False def _request_callback( self, method: str, *args: List[Any], **kwargs: Dict[Any, Any] ) -> Any: if method == "apps_v1_api.list_deployment_for_all_namespaces": + if self.empty_response is True: + return MockResponse([]) return MockResponse(["apple", "banana", "cherry"]) if method == "core_v1_api.list_namespace": return self._mock_list_namespace() @@ -49,6 +53,19 @@ def test_namespaces(self) -> None: # Set the k8s attribute to the mock_k8s instance handler_instance.preflight() + def test_list_deployment_for_all_namespaces_fails(self) -> None: + # Set no response + self.empty_response = 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( From 4b7a6e99fb5f419caf1f298d51f0532f59885a95 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Mon, 9 Sep 2024 15:04:54 +0200 Subject: [PATCH 04/14] fix: add python3 for ubuntu distros Signed-off-by: Boekhorst --- Makefile | 11 +++++------ tests/mock_k8s_prerequisite_check.py | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index ec30b5c..29f5cdf 100644 --- a/Makefile +++ b/Makefile @@ -28,17 +28,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/python3.12 -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 + python3 -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 @@ -68,7 +67,7 @@ check-radoslib: ## Checks if radoslib is installed and if it contains the right 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 + cd src && python3 -m rookify .PHONY: run-rookify run-rookify: ## Runs rookify in the container diff --git a/tests/mock_k8s_prerequisite_check.py b/tests/mock_k8s_prerequisite_check.py index 470fca0..6be782d 100644 --- a/tests/mock_k8s_prerequisite_check.py +++ b/tests/mock_k8s_prerequisite_check.py @@ -6,10 +6,10 @@ # Note: currently this test works with pytest but not with unittest, which is not able to import needed classes -class MockK8sPrerequisitesCheckHandler(K8sPrerequisitesCheckHandler): # type: ignore +class MockK8sPrerequisitesCheckHandler(K8sPrerequisitesCheckHandler): def __init__(self, request_callback: Any, *args: Any, **kwargs: Any) -> None: K8sPrerequisitesCheckHandler.__init__(self, *args, **kwargs) - self._k8s = MockK8s(request_callback) + self._k8s = MockK8s(request_callback) # type: ignore def execute(self) -> None: K8sPrerequisitesCheckHandler.execute(self) From b83ea40898c2de69c60e8e08ee3207fac68c9373 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Tue, 10 Sep 2024 08:57:28 +0000 Subject: [PATCH 05/14] fix: not specifying specific python version but finding it dynamically Signed-off-by: Boekhorst --- Makefile | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 29f5cdf..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 @@ -32,11 +43,11 @@ setup: check-radoslib setup-venv setup-pre-commit ## Setup the pre-commit enviro .PHONY: setup-pre-commit setup-pre-commit: - ./.venv/bin/pip install --user pre-commit && ./.venv/bin/python3.12 -m pre_commit install + ./.venv/bin/pip install --user pre-commit && ./.venv/bin/python -m pre_commit install .PHONY: setup-venv setup-venv: - python3 -m venv --system-site-packages ./.venv && \ + ${PYTHON} -m venv --system-site-packages ./.venv && \ ./.venv/bin/pip install -r requirements.txt .PHONY: run-precommit @@ -65,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 && python3 -m rookify + source ./.venv/bin/activate && pip install -e . && rookify .PHONY: run-rookify run-rookify: ## Runs rookify in the container From 8fe348d9d3aa218bee1111a61a81153df282c58d Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Tue, 10 Sep 2024 09:17:56 +0000 Subject: [PATCH 06/14] add pytest to requirements Signed-off-by: Boekhorst --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 1c8606a..3013c86 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.3.2 From 69140d5397b7b1924d539192c7719d0a842302e6 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Tue, 10 Sep 2024 09:19:35 +0000 Subject: [PATCH 07/14] fix: changing name of response mock to V1DeploymentList Signed-off-by: Boekhorst --- tests/modules/test_k8s_prerequisites_check.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/modules/test_k8s_prerequisites_check.py b/tests/modules/test_k8s_prerequisites_check.py index 6e39707..714f05f 100644 --- a/tests/modules/test_k8s_prerequisites_check.py +++ b/tests/modules/test_k8s_prerequisites_check.py @@ -19,8 +19,8 @@ def _request_callback( ) -> Any: if method == "apps_v1_api.list_deployment_for_all_namespaces": if self.empty_response is True: - return MockResponse([]) - return MockResponse(["apple", "banana", "cherry"]) + return V1DeploymentList([]) + return V1DeploymentList(["apple", "banana", "cherry"]) if method == "core_v1_api.list_namespace": return self._mock_list_namespace() @@ -79,7 +79,7 @@ def test_namespaces_fails(self) -> None: handler_instance.preflight() -# Create a Mock respons object -class MockResponse: +# Create a Mock respons for the V1DeploymentList object +class V1DeploymentList: def __init__(self, items: List[str]) -> None: self.items = items From eb5ae4233b6edeab8e4c0df2f0cc243b02f66f84 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Tue, 10 Sep 2024 09:24:18 +0000 Subject: [PATCH 08/14] fix: setting pytest to same version as ci Signed-off-by: Boekhorst --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 3013c86..3f4fb3f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,4 +32,4 @@ urllib3==2.2.1 yamale==5.1.0 websocket-client==1.7.0 wrapt==1.16.0 -pytest==8.3.2 +pytest==8.0.2 From 750499762484ed3a36e2cb9ae07d4d999b0c4e3a Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Tue, 10 Sep 2024 09:32:05 +0000 Subject: [PATCH 09/14] fix: use kubernetes python cli itself to generate mock v1namespace list Signed-off-by: Boekhorst --- tests/modules/test_k8s_prerequisites_check.py | 33 +++++++------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/tests/modules/test_k8s_prerequisites_check.py b/tests/modules/test_k8s_prerequisites_check.py index 714f05f..b8a431c 100644 --- a/tests/modules/test_k8s_prerequisites_check.py +++ b/tests/modules/test_k8s_prerequisites_check.py @@ -2,6 +2,7 @@ from unittest.mock import Mock from rookify.modules.exception import ModuleException +from kubernetes.client import V1Namespace, V1ObjectMeta, V1NamespaceList from typing import Any, Dict, List from ..mock_k8s_prerequisite_check import MockK8sPrerequisitesCheckHandler @@ -22,28 +23,16 @@ def _request_callback( return V1DeploymentList([]) return V1DeploymentList(["apple", "banana", "cherry"]) if method == "core_v1_api.list_namespace": - return self._mock_list_namespace() - - def _mock_list_namespace(self) -> Any: - class Metadata: - def __init__(self, name: str): - self.name = name - - class Namespace: - def __init__(self, name: str): - self.metadata = Metadata(name) - - class NamespaceList: - def __init__(self, items: List[Namespace]): - self.items = items - - return NamespaceList( - [ - Namespace("default"), - Namespace("kube-system"), - Namespace("test-namespace"), - ] - ) + return self._mock_v1namespacelist() + + def _mock_v1namespacelist(self) -> Any: + # 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 From b44abca661a0a6f2da2e6efa3b99942d76e9e672 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Tue, 10 Sep 2024 09:39:53 +0000 Subject: [PATCH 10/14] fix: better naming for flag for empty deployment list Signed-off-by: Boekhorst --- tests/modules/test_k8s_prerequisites_check.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/modules/test_k8s_prerequisites_check.py b/tests/modules/test_k8s_prerequisites_check.py index b8a431c..db20994 100644 --- a/tests/modules/test_k8s_prerequisites_check.py +++ b/tests/modules/test_k8s_prerequisites_check.py @@ -10,16 +10,14 @@ # 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: - # Mock configuration self.config = {"rook": {"cluster": {"namespace": "test-namespace"}}} - # No response option - self.empty_response = False + self.empty_deployment_list = False def _request_callback( self, method: str, *args: List[Any], **kwargs: Dict[Any, Any] ) -> Any: if method == "apps_v1_api.list_deployment_for_all_namespaces": - if self.empty_response is True: + if self.empty_deployment_list is True: return V1DeploymentList([]) return V1DeploymentList(["apple", "banana", "cherry"]) if method == "core_v1_api.list_namespace": @@ -43,8 +41,8 @@ def test_namespaces(self) -> None: handler_instance.preflight() def test_list_deployment_for_all_namespaces_fails(self) -> None: - # Set no response - self.empty_response = True + # Check the case where the deployment list is empty + self.empty_deployment_list = True # Instantiate K8sPrerequisitesCheckHandler with the mock ModuleHandler handler_instance = MockK8sPrerequisitesCheckHandler( From c407d2c8a479d770fa7d04ac714eb146af79648e Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 10 Sep 2024 20:36:04 +0200 Subject: [PATCH 11/14] Code cleanup Signed-off-by: Tobias Wolf --- src/rookify/modules/ceph.py | 12 +++--- .../modules/create_rook_resources/main.py | 4 +- src/rookify/modules/machine.py | 4 +- src/rookify/modules/migrate_mds/main.py | 2 +- src/rookify/modules/migrate_osds/main.py | 6 +-- src/rookify/modules/migrate_rgw_pools/main.py | 2 +- src/rookify/modules/migrate_rgws/main.py | 2 +- tests/mock_ceph.py | 6 +-- tests/mock_k8s.py | 4 +- tests/mock_k8s_prerequisite_check.py | 3 -- tests/modules/test_k8s_prerequisites_check.py | 43 +++++++++---------- tests/test_mock_ceph.py | 4 +- tests/test_mock_k8s.py | 6 +-- 13 files changed, 43 insertions(+), 55 deletions(-) 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/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/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 index 6be782d..db34ed0 100644 --- a/tests/mock_k8s_prerequisite_check.py +++ b/tests/mock_k8s_prerequisite_check.py @@ -10,6 +10,3 @@ 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 - - def execute(self) -> None: - K8sPrerequisitesCheckHandler.execute(self) diff --git a/tests/modules/test_k8s_prerequisites_check.py b/tests/modules/test_k8s_prerequisites_check.py index db20994..2df3880 100644 --- a/tests/modules/test_k8s_prerequisites_check.py +++ b/tests/modules/test_k8s_prerequisites_check.py @@ -2,10 +2,16 @@ from unittest.mock import Mock from rookify.modules.exception import ModuleException -from kubernetes.client import V1Namespace, V1ObjectMeta, V1NamespaceList -from typing import Any, Dict, List +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): @@ -13,24 +19,21 @@ def setUp(self) -> None: self.config = {"rook": {"cluster": {"namespace": "test-namespace"}}} self.empty_deployment_list = False - 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 == "apps_v1_api.list_deployment_for_all_namespaces": if self.empty_deployment_list is True: - return V1DeploymentList([]) - return V1DeploymentList(["apple", "banana", "cherry"]) - if method == "core_v1_api.list_namespace": - return self._mock_v1namespacelist() + 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")), + ] - def _mock_v1namespacelist(self) -> Any: - # 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) + return V1NamespaceList(items=namespaces) def test_namespaces(self) -> None: # Instantiate K8sPrerequisitesCheckHandler with the mock ModuleHandler @@ -64,9 +67,3 @@ def test_namespaces_fails(self) -> None: # Call the preflight method to run the test with self.assertRaises(ModuleException): handler_instance.preflight() - - -# Create a Mock respons for the V1DeploymentList object -class V1DeploymentList: - def __init__(self, items: List[str]) -> None: - self.items = items 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 From 93231b7f05bc9151e1932427c68425c2b2779750 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 10 Sep 2024 20:36:19 +0200 Subject: [PATCH 12/14] Remove `abstractmethod` decorator Modules may only implement either `preflight()` or `execute()` as well. Remove the `@abstractmethod` decorator therefore. Signed-off-by: Tobias Wolf --- src/rookify/modules/module.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/rookify/modules/module.py b/src/rookify/modules/module.py index f787bf7..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 @@ -59,14 +58,12 @@ def ssh(self) -> SSH: 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 From 6e95bfa7e7cbe01470c386501977c53c95b6e15e Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 10 Sep 2024 20:44:43 +0200 Subject: [PATCH 13/14] Add GitHub pytest workflow Signed-off-by: Tobias Wolf --- ...{pre-commit.yml => on-push-pre-commit.yml} | 4 +- .github/workflows/on-push-test.yml | 52 +++++++++++++++++++ mock_src/__init__.py | 0 mock_src/rados.py | 3 ++ pyproject.toml | 2 +- 5 files changed, 58 insertions(+), 3 deletions(-) rename .github/workflows/{pre-commit.yml => on-push-pre-commit.yml} (78%) create mode 100644 .github/workflows/on-push-test.yml create mode 100644 mock_src/__init__.py create mode 100644 mock_src/rados.py 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/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" ] From dde5d60ab6c997df989382ab178df69c9ecc139f Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 10 Sep 2024 21:09:57 +0200 Subject: [PATCH 14/14] Fix unsupported string formatting code in `K8s` Signed-off-by: Tobias Wolf --- src/rookify/modules/k8s.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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: