From a30c9caee4baa008e8d1688fe6892ad426131d5d Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Tue, 10 Sep 2024 12:56:48 +0000 Subject: [PATCH 01/35] feat: add picklefile read option with tests Signed-off-by: Boekhorst --- src/rookify/__main__.py | 102 ++++++++++++++++-- src/rookify/modules/machine.py | 1 + tests/test_main.py | 192 +++++++++++++++++++++++++++++++++ 3 files changed, 286 insertions(+), 9 deletions(-) create mode 100644 tests/test_main.py diff --git a/src/rookify/__main__.py b/src/rookify/__main__.py index 4dd6383..5b61e27 100644 --- a/src/rookify/__main__.py +++ b/src/rookify/__main__.py @@ -1,20 +1,67 @@ # -*- coding: utf-8 -*- -from argparse import ArgumentParser +import json +from pickle import Unpickler +import sys +import argparse +from argparse import ArgumentParser, Namespace +from typing import Any, Dict, Optional from .modules import load_modules from .modules.machine import Machine from .logger import configure_logging, get_logger from .yaml import load_config -def main() -> None: +def parse_args(args: list[str]) -> Namespace: + # Putting args-parser in seperate function to make this testable arg_parser = ArgumentParser("Rookify") arg_parser.add_argument("--dry-run", action="store_true", dest="dry_run_mode") - args = arg_parser.parse_args() + + # Custom ShowAction to set 'all' if nothing is specified for --show + class ShowAction(argparse.Action): + def __call__( + self, + parser: ArgumentParser, + namespace: Namespace, + values: Optional[Any], + option_string: Optional[str] = None, + ) -> None: + setattr(namespace, self.dest, values if values is not None else "all") + + arg_parser.add_argument( + "--show", + nargs="?", + action=ShowAction, + dest="show_progress", + metavar="
", + help="Show the state of progress, as read from the pickle file. Default argument is 'all', you can also specify a section you want to look at.", + required=False, + ) + return arg_parser.parse_args(args) + + +def load_pickler(pickle_file_name: str) -> Any: + with open(pickle_file_name, "ab+") as pickle_file: + pickle_file.seek(0) + states_data = Unpickler(pickle_file).load() + return states_data + + +def sort_pickle_file(unsorted_states_data: Dict[str, Any]) -> Dict[str, Any]: + # sort the pickle-file alfabetically + iterable_dict = iter(unsorted_states_data) + first_key = next(iterable_dict) + data_values = unsorted_states_data[first_key]["data"] + sorted_data_by_keys = {k: data_values[k] for k in sorted(data_values)} + return sorted_data_by_keys + + +def main() -> None: + args = parse_args(sys.argv[1:]) # Load configuration file try: - config = load_config("config.yaml") + config: Dict[str, Any] = load_config("config.yaml") except FileNotFoundError as err: raise SystemExit(f"Could not load config: {err}") @@ -23,11 +70,48 @@ def main() -> None: configure_logging(config["logging"]) except Exception as e: raise SystemExit(f"Error configuring logging: {e}") - + # Get Logger log = get_logger() - log.debug("Executing Rookify") - machine = Machine(config["general"].get("machine_pickle_file")) - load_modules(machine, config) + # Get Pickle File if configured in config.yaml + pickle_file_name = config["general"].get("machine_pickle_file") + if pickle_file_name is None: + log.info("No pickle file was set in the configuration.") + else: + log.info(f"Pickle file set: {pickle_file_name}") + + # Get Pickle File if configured in config.yaml + pickle_file_name = config["general"].get("machine_pickle_file") + if pickle_file_name is None: + log.info("No pickle file was set in the configuration.") + else: + log.info(f"Pickle file set: {pickle_file_name}") + + # If show_progress is not None and pickle_file_name is not None, show the current progress of the migration + if args.show_progress is not None: + if pickle_file_name is None: + return + states_data = load_pickler(pickle_file_name) + sorted_states_data = sort_pickle_file(states_data) + + # Check if a specific section should be listed + if args.show_progress != "all": + if args.show_progress not in sorted_states_data.keys(): + get_logger().error(f"The section {args.show_progress} does not exist") + return + else: + sorted_states_data = sorted_states_data[args.show_progress] + + get_logger().info( + 'Current state as retrieved from pickle-file: \n "{0}": {1}'.format( + args.show_progress, json.dumps(sorted_states_data, indent=4) + ) + ) + # Else run the rook migration + else: + log.debug("Executing Rookify") + + machine = Machine(config["general"].get("machine_pickle_file")) + load_modules(machine, config) - machine.execute(args.dry_run_mode) + machine.execute(dry_run_mode=args.dry_run_mode) diff --git a/src/rookify/modules/machine.py b/src/rookify/modules/machine.py index cb8c9d5..3a6bcf8 100644 --- a/src/rookify/modules/machine.py +++ b/src/rookify/modules/machine.py @@ -52,6 +52,7 @@ def execute(self, dry_run_mode: bool = False) -> None: def _execute(self, pickle_file: Optional[IO[Any]] = None) -> None: states_data = {} + # Read pickle file if it exists, to continue from the stored state if pickle_file is not None and pickle_file.tell() > 0: pickle_file.seek(0) diff --git a/tests/test_main.py b/tests/test_main.py new file mode 100644 index 0000000..9ef0bfb --- /dev/null +++ b/tests/test_main.py @@ -0,0 +1,192 @@ +import sys +from typing import Any, Callable, Optional +import pytest +from _pytest.monkeypatch import MonkeyPatch +import yaml +from unittest.mock import MagicMock +import pytest_structlog + +from argparse import Namespace + +from rookify.__main__ import parse_args, main, sort_pickle_file +import rookify.yaml + + +# Test the arugment parser +def test_parse_args_dry_run() -> None: + args = parse_args(["--dry-run"]) + expected = Namespace(dry_run_mode=True, show_progress=None) + assert args == expected + + +def test_parse_args_show_progress() -> None: + args = parse_args(["--show"]) + expected = Namespace(dry_run_mode=False, show_progress="all") + assert args == expected + + +def test_parse_args_both_flags() -> None: + args = parse_args(["--dry-run", "--show"]) + expected = Namespace(dry_run_mode=True, show_progress="all") + assert args == expected + + +def test_parse_args_no_flags() -> None: + args = parse_args([]) + expected = Namespace(dry_run_mode=False, show_progress=None) + assert args == expected + + +# Test the --show option + + +@pytest.fixture # type: ignore +def mock_load_config(monkeypatch: MonkeyPatch) -> Callable[[Optional[Any]], MagicMock]: + def _mock_load_config(pickle_file: Optional[Any] = None) -> MagicMock: + # Mock the configuration data + # Load config.example.yaml + with open("config.example.yaml", "r") as file: + config_data = yaml.safe_load(file) + + config_data["general"]["machine_pickle_file"] = pickle_file + + # Mock load_config function + mock = MagicMock(return_value=config_data) + monkeypatch.setattr(rookify.__main__, "load_config", mock) + + return mock + + return _mock_load_config + + +@pytest.fixture # type: ignore +def mock_load_pickler(monkeypatch: MonkeyPatch) -> MagicMock: + # Mock states_data from pickle file + mock_states_data = {"teststuff": {"data": {"mock_key": "mock_value"}}} + + # Mock load_pickler function + mock = MagicMock(return_value=mock_states_data) + monkeypatch.setattr(rookify.__main__, "load_pickler", mock) + + return mock + + +def test_main_show_progress( + mock_load_config: Callable[[Optional[Any]], MagicMock], + mock_load_pickler: MonkeyPatch, + monkeypatch: MonkeyPatch, + log: pytest_structlog.StructuredLogCapture, +) -> None: + # Load example config with mock.pickle as pickle file + mock_load_config("mock.pickle") + + # Mock sys.argv to simulate command-line arguments + monkeypatch.setattr(sys, "argv", ["main_script.py", "--show", "--dry-run"]) + + # Run main() + main() + + # Verify logging messages + expected_log_message = ( + 'Current state as retrieved from pickle-file: \n "all": ' + "{\n" + ' "mock_key": "mock_value"\n' + "}" + ) + assert log.has("Pickle file set: mock.pickle", level="info") + assert log.has(expected_log_message, level="info") + + +def test_main_no_pickle_file( + mock_load_config: Callable[[Optional[str]], MagicMock], + mock_load_pickler: MonkeyPatch, + monkeypatch: MonkeyPatch, + log: pytest_structlog.StructuredLogCapture, +) -> None: + # Load a configuration without pickle: This should load the default data.pickle file if it is available + mock_load_config(None) + + # Mock sys.argv to simulate command-line arguments + monkeypatch.setattr(sys, "argv", ["main_script.py", "--show", "--dry-run"]) + + # Run main() + main() + + # Assertions + assert log.has("No pickle file was set in the configuration.") + + +def test_sort_pickle_file() -> None: + # Prepare unsorted data + unsorted_states_data = getUnsortedData() + + # Expected keys order + expected_order = ["device", "fs", "mon", "node", "osd", "ssh"] + + # Run sort_pickle_file + sorted_states_data = sort_pickle_file(unsorted_states_data) + + # Assert that order is correct + sorted_states_data_keys = list(sorted_states_data.keys()) + + assert expected_order == sorted_states_data_keys + + +def getUnsortedData() -> Any: + unsorted_states_data = { + "preflighttestdata": { + "data": { + "mon": { + "dump": {"epoch": 1}, + "mons": [{"rank": 0}], + "quorum": [0, 1, 2], + }, + "fs": {"dump": {"epoch": 1}}, + "device": {"ls": ["devid", 1]}, + "osd": { + "dump": [{"epoch": 138}], + "osds": [{"osd": 0}], + "osd_xinfo": [{"osd": 0}], + }, + "node": { + "ls": { + "mon": { + "test-node-0": ["test-node-0"], + "test-node-1": ["test-node-1"], + "test-node-2": ["test-node-2"], + }, + "osd": { + "test-node-0": [2, 3], + "test-node-1": [0, 5], + "test-node-2": [1, 4], + }, + "mds": { + "test-node-0": ["test-node-0"], + "test-node-1": ["test-node-1"], + "test-node-2": ["test-node-2"], + }, + "mgr": { + "test-node-0": ["test-node-0"], + "test-node-1": ["test-node-1"], + "test-node-2": ["test-node-2"], + }, + } + }, + "ssh": { + "osd": { + "test-node-0": { + "devices": ["/dev/ceph-bla-1", "/dev/ceph-bla-2"] + }, + "test-node-1": { + "devices": ["/dev/ceph-bla-3", "/dev/ceph-bla-4"] + }, + "test-node-2": { + "devices": ["/dev/ceph-bla-5", "/dev/ceph-bla-6"] + }, + } + }, + } + } + } + + return unsorted_states_data From d27d5ad8aa608e1802015f3d900ee7a195a21614 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Tue, 10 Sep 2024 13:09:41 +0000 Subject: [PATCH 02/35] updating Makefile as in #73 Signed-off-by: Boekhorst --- Makefile | 78 ++++++++++++------- scripts/check_local_rados_lib_installation.sh | 29 +++++++ 2 files changed, 77 insertions(+), 30 deletions(-) create mode 100755 scripts/check_local_rados_lib_installation.sh diff --git a/Makefile b/Makefile index ec30b5c..3f9399a 100644 --- a/Makefile +++ b/Makefile @@ -4,12 +4,8 @@ COLOUR_BLUE=\033[0;34m COLOUR_END=\033[0m .DEFAULT_GOAL:=help -SHELL := /bin/bash +SHELL:=/bin/bash -# Get needed paths and information from locally installed librados -export RADOSLIB_VERSION := 2.0.0 -export GENERAL_LIB_LOCATION := ${shell pip show rados | grep -oP "(?<=Location: ).*"} -export RADOSLIB_INSTALLED_VERSION := ${shell pip show rados | grep Version | awk '{print $$2}'} ## checking if docker, or podman should be used. Podman is preferred. ifeq ($(shell command -v podman 2> /dev/null),) @@ -18,27 +14,41 @@ 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 - @echo -e '${COLOUR_RED}Usage: make ${COLOUR_END}' + @echo -e '\n${COLOUR_BLUE}ROOKIFY MAKEFILE${COLOUR_BLUE}' + @echo -e '\n${COLOUR_RED}Usage: make ${COLOUR_END}' @cat $(MAKEFILE_LIST) | grep '^[a-zA-Z]' | \ awk -F ':.*?## ' 'NF==2 {printf " %-26s%s\n\n", $$1, "${COLOUR_GREEN}"$$2"${COLOUR_END}"}' + @echo -e '${COLOUR_RED}OSISM helperscript usage: make ${COLOUR_END}' + @cat $(MAKEFILE_LIST) | grep '^[a-zA-Z]' | \ + awk -F ':.*?#osism# ' '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 @@ -50,33 +60,33 @@ update-requirements: ## Update the requirements.txt with newer versions of pip p pip freeze -l > requirements.txt .PHONY: check-radoslib +export RADOSLIB_VERSION:=2.0.0 check-radoslib: ## Checks if radoslib is installed and if it contains the right version - @if [ -z "${GENERAL_LIB_LOCATION}" ]; then \ - echo -e "${COLOUR_RED}ERROR: 'rados' library not found. Please make sure it's installed.${COLOUR_END}"; \ - exit 1; \ - else \ - echo -e "GENERAL_LIB_LOCATION: $(GENERAL_LIB_LOCATION)"; \ - fi - @if [ "${RADOSLIB_INSTALLED_VERSION}" != "${RADOSLIB_VERSION}" ]; then \ - echo -e "${COLOUR_RED}ERROR: Incorrect version of 'rados' library found. Expected version $(RADOSLIB_VERSION), found $$RADOSLIB_INSTALLED_VERSION.${COLOUR_END}"; \ - exit 1; \ - else \ - echo -e "RADOSLIB_INSTALLED_VERSION: $(RADOSLIB_INSTALLED_VERSION)"; \ - fi + # Get needed paths and information from locally installed librados + ./scripts/check_local_rados_lib_installation.sh ${RADOSLIB_VERSION} + +.PHONY: build-local-rookify +build-local-rookify: ## This builds rookify into .venv/bin/rookify + source .venv/bin/activate && pip install -e . + +.PHONY: build-container +build-container: ## Build container from Dockerfile, add e.g. ROOKIFY_VERSION=0.0.1 to specify the version. Default value is 0.0.0.dev1 + ${CONTAINERCMD} build --build-arg ROOKIFY_VERSION=$(ROOKIFY_VERSION) --target rookify -t rookify:latest -f Dockerfile . .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 + if [ ! -f ./.venv/bin/rookify ]; then \ + ${MAKE} build-local-rookify; \ + fi; \ + .venv/bin/rookify .PHONY: run-rookify run-rookify: ## Runs rookify in the container docker exec -it rookify-dev /app/rookify/.venv/bin/rookify -.PHONY: build-container -build-container: ## Build container from Dockerfile, add e.g. ROOKIFY_VERSION=0.0.1 to specify the version. Default value is 0.0.0.dev1 - ${CONTAINERCMD} build --build-arg ROOKIFY_VERSION=$(ROOKIFY_VERSION) --target rookify -t rookify:latest -f Dockerfile . +.PHONY: get-testbed-configs-for-rookify-testing +get-testbed-configs-for-rookify-testing: ## Gets the needed config (like .kube, /etc/ceph and so on) from the testbed + bash ./scripts/get_configs_from_testbed.sh .PHONY: run-tests-locally run-tests-locally: ## Runs the tests in the tests directory. NB: check that your local setup is connected through vpn to the testbed! @@ -101,3 +111,11 @@ down: ## Remove the containers as setup by docker-compose.yml .PHONY: up up: ## Sets up the container as specified in docker-compose.yml and opens a bash terminal ${CONTAINERCMD} compose up -d + +## +# Add osism specific scripts below here (so they appear below helper header) +## + +.PHONY: get-config +get-config: #osism# Gets configuration files from the OSISM testbed + ./scripts/osism/get_osism_configs_from_testbed.sh diff --git a/scripts/check_local_rados_lib_installation.sh b/scripts/check_local_rados_lib_installation.sh new file mode 100755 index 0000000..6d88303 --- /dev/null +++ b/scripts/check_local_rados_lib_installation.sh @@ -0,0 +1,29 @@ +#!/bin/env bash + +# Check if a version argument is provided +if [ $# -eq 1 ]; then + RADOSLIB_VERSION="$1" +else + # Default version if no argument is provided + RADOSLIB_VERSION="2.0.0" +fi + +# Get the location of the installed rados library +GENERAL_LIB_LOCATION=$(pip show rados | grep -oP "(?<=Location: ).*") + +# Get the installed version of the rados library +RADOSLIB_INSTALLED_VERSION=$(pip show rados | grep Version | awk '{print $2}') + +# Check if the rados library is installed +if [ -z "$GENERAL_LIB_LOCATION" ]; then + echo -e "\033[0;31mERROR: 'rados' library not found. Please make sure it's installed.\033[0m" + exit 1 +fi + +# Check if the installed version matches the expected version +if [ "$RADOSLIB_INSTALLED_VERSION" != "$RADOSLIB_VERSION" ]; then + echo -e "\033[0;31mERROR: 'rados' library version $RADOSLIB_INSTALLED_VERSION does not match the expected version $RADOSLIB_VERSION.\033[0m" + exit 1 +else + echo -e "\033[0;32m'rados' library version $RADOSLIB_INSTALLED_VERSION is correct.\033[0m" +fi From 02fc7c10190cff84367a87e0e618133459defbaf Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Tue, 10 Sep 2024 13:51:26 +0000 Subject: [PATCH 03/35] fix: addin pytest_structlog Signed-off-by: Boekhorst --- requirements.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/requirements.txt b/requirements.txt index 1c8606a..330c82c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,3 +32,5 @@ urllib3==2.2.1 yamale==5.1.0 websocket-client==1.7.0 wrapt==1.16.0 +pytest==8.0.2 +pytest-structlog==1.1 From 6fe4b39d5a86b2600cb75ba1b64222d7041e1f43 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Tue, 10 Sep 2024 13:58:18 +0000 Subject: [PATCH 04/35] fix: rename show progress to read pickle Signed-off-by: Boekhorst --- src/rookify/__main__.py | 26 +++++++++++++------------- tests/test_main.py | 27 ++++++++++++++++----------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/rookify/__main__.py b/src/rookify/__main__.py index 5b61e27..a7de3f0 100644 --- a/src/rookify/__main__.py +++ b/src/rookify/__main__.py @@ -17,8 +17,8 @@ def parse_args(args: list[str]) -> Namespace: arg_parser = ArgumentParser("Rookify") arg_parser.add_argument("--dry-run", action="store_true", dest="dry_run_mode") - # Custom ShowAction to set 'all' if nothing is specified for --show - class ShowAction(argparse.Action): + # Custom ReadAction to set 'all' if nothing is specified for --read-pickle + class ReadAction(argparse.Action): def __call__( self, parser: ArgumentParser, @@ -29,12 +29,12 @@ def __call__( setattr(namespace, self.dest, values if values is not None else "all") arg_parser.add_argument( - "--show", + "--read-pickle", nargs="?", - action=ShowAction, - dest="show_progress", + action=ReadAction, + dest="read_pickle", metavar="
", - help="Show the state of progress, as read from the pickle file. Default argument is 'all', you can also specify a section you want to look at.", + help="Show the content of the pickle file. Default argument is 'all', you can also specify a section you want to look at.", required=False, ) return arg_parser.parse_args(args) @@ -87,24 +87,24 @@ def main() -> None: else: log.info(f"Pickle file set: {pickle_file_name}") - # If show_progress is not None and pickle_file_name is not None, show the current progress of the migration - if args.show_progress is not None: + # If read_pickle is not None and pickle_file_name is not None, show the current progress of the migration + if args.read_pickle is not None: if pickle_file_name is None: return states_data = load_pickler(pickle_file_name) sorted_states_data = sort_pickle_file(states_data) # Check if a specific section should be listed - if args.show_progress != "all": - if args.show_progress not in sorted_states_data.keys(): - get_logger().error(f"The section {args.show_progress} does not exist") + if args.read_pickle != "all": + if args.read_pickle not in sorted_states_data.keys(): + get_logger().error(f"The section {args.read_pickle} does not exist") return else: - sorted_states_data = sorted_states_data[args.show_progress] + sorted_states_data = sorted_states_data[args.read_pickle] get_logger().info( 'Current state as retrieved from pickle-file: \n "{0}": {1}'.format( - args.show_progress, json.dumps(sorted_states_data, indent=4) + args.read_pickle, json.dumps(sorted_states_data, indent=4) ) ) # Else run the rook migration diff --git a/tests/test_main.py b/tests/test_main.py index 9ef0bfb..f4c7856 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -15,29 +15,34 @@ # Test the arugment parser def test_parse_args_dry_run() -> None: args = parse_args(["--dry-run"]) - expected = Namespace(dry_run_mode=True, show_progress=None) + expected = Namespace(dry_run_mode=True, read_pickle=None) assert args == expected -def test_parse_args_show_progress() -> None: - args = parse_args(["--show"]) - expected = Namespace(dry_run_mode=False, show_progress="all") +def test_parse_args_read_pickle() -> None: + args = parse_args(["--read-pickle"]) + expected = Namespace(dry_run_mode=False, read_pickle="all") assert args == expected def test_parse_args_both_flags() -> None: - args = parse_args(["--dry-run", "--show"]) - expected = Namespace(dry_run_mode=True, show_progress="all") + args = parse_args(["--dry-run", "--read-pickle"]) + expected = Namespace(dry_run_mode=True, read_pickle="all") assert args == expected def test_parse_args_no_flags() -> None: args = parse_args([]) - expected = Namespace(dry_run_mode=False, show_progress=None) + expected = Namespace(dry_run_mode=False, read_pickle=None) assert args == expected -# Test the --show option +# def test_parse_args_show_progress() -> None: +# args = parse_args([]) +# expected = Namespace(dry_run_mode=True, show_progress=None) +# assert args == expected + +# Test the --read-pickle option @pytest.fixture # type: ignore @@ -71,7 +76,7 @@ def mock_load_pickler(monkeypatch: MonkeyPatch) -> MagicMock: return mock -def test_main_show_progress( +def test_main_read_pickle( mock_load_config: Callable[[Optional[Any]], MagicMock], mock_load_pickler: MonkeyPatch, monkeypatch: MonkeyPatch, @@ -81,7 +86,7 @@ def test_main_show_progress( mock_load_config("mock.pickle") # Mock sys.argv to simulate command-line arguments - monkeypatch.setattr(sys, "argv", ["main_script.py", "--show", "--dry-run"]) + monkeypatch.setattr(sys, "argv", ["main_script.py", "--read-pickle", "--dry-run"]) # Run main() main() @@ -107,7 +112,7 @@ def test_main_no_pickle_file( mock_load_config(None) # Mock sys.argv to simulate command-line arguments - monkeypatch.setattr(sys, "argv", ["main_script.py", "--show", "--dry-run"]) + monkeypatch.setattr(sys, "argv", ["main_script.py", "--read-pickle", "--dry-run"]) # Run main() main() From 08d73561800a2a3cc1b530bbe41b9b004b0e62e8 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Tue, 10 Sep 2024 14:19:46 +0000 Subject: [PATCH 05/35] feat: adding cli option to show progress, adding tests as well Signed-off-by: Boekhorst --- src/rookify/__main__.py | 21 +++++++++++++++++++++ tests/test_main.py | 34 ++++++++++++++++++++++++---------- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/rookify/__main__.py b/src/rookify/__main__.py index a7de3f0..c07c4d6 100644 --- a/src/rookify/__main__.py +++ b/src/rookify/__main__.py @@ -28,6 +28,17 @@ def __call__( ) -> None: setattr(namespace, self.dest, values if values is not None else "all") + # Custom ShowProgressAction to set 'all' if nothing is specified for --show-progress + class ShowProgressAction(argparse.Action): + def __call__( + self, + parser: ArgumentParser, + namespace: Namespace, + values: Optional[Any], + option_string: Optional[str] = None, + ) -> None: + setattr(namespace, self.dest, values if values is not None else "all") + arg_parser.add_argument( "--read-pickle", nargs="?", @@ -37,6 +48,16 @@ def __call__( help="Show the content of the pickle file. Default argument is 'all', you can also specify a section you want to look at.", required=False, ) + + arg_parser.add_argument( + "--show-progress", + nargs="?", + action=ShowProgressAction, + dest="show_progress", + metavar="", + help="Show progress of the modules. Default argument is 'all', you can also specify a module you want to get the progress status from.", + required=False, + ) return arg_parser.parse_args(args) diff --git a/tests/test_main.py b/tests/test_main.py index f4c7856..f61edf3 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -15,34 +15,48 @@ # Test the arugment parser def test_parse_args_dry_run() -> None: args = parse_args(["--dry-run"]) - expected = Namespace(dry_run_mode=True, read_pickle=None) + expected = Namespace(dry_run_mode=True, read_pickle=None, show_progress=None) assert args == expected def test_parse_args_read_pickle() -> None: args = parse_args(["--read-pickle"]) - expected = Namespace(dry_run_mode=False, read_pickle="all") + expected = Namespace(dry_run_mode=False, read_pickle="all", show_progress=None) assert args == expected def test_parse_args_both_flags() -> None: args = parse_args(["--dry-run", "--read-pickle"]) - expected = Namespace(dry_run_mode=True, read_pickle="all") + expected = Namespace(dry_run_mode=True, read_pickle="all", show_progress=None) + assert args == expected + + +def test_parse_args_show_progress() -> None: + args = parse_args(["--show-progress"]) + expected = Namespace(dry_run_mode=False, read_pickle=None, show_progress="all") + assert args == expected + + +# check: should it be possible to add all arguments? +def test_parse_args_both_dry_run_show_progress() -> None: + args = parse_args(["--dry-run", "--read-pickle", "--show-progress"]) + expected = Namespace(dry_run_mode=True, read_pickle="all", show_progress="all") + assert args == expected + + +def test_parse_args_all_dry_run_show_progress_read_pickle() -> None: + args = parse_args(["--dry-run", "--show-progress"]) + expected = Namespace(dry_run_mode=True, read_pickle=None, show_progress="all") assert args == expected def test_parse_args_no_flags() -> None: args = parse_args([]) - expected = Namespace(dry_run_mode=False, read_pickle=None) + expected = Namespace(dry_run_mode=False, read_pickle=None, show_progress=None) assert args == expected -# def test_parse_args_show_progress() -> None: -# args = parse_args([]) -# expected = Namespace(dry_run_mode=True, show_progress=None) -# assert args == expected - -# Test the --read-pickle option +# Test the --read-pickle and --show-progress options @pytest.fixture # type: ignore From eca2435ecd29a0581e38e614d1270118919a81f8 Mon Sep 17 00:00:00 2001 From: boekhorstb1 <91957243+boekhorstb1@users.noreply.github.com> Date: Tue, 10 Sep 2024 21:39:45 +0200 Subject: [PATCH 06/35] 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 Co-authored-by: Tobias Wolf --- ...{pre-commit.yml => on-push-pre-commit.yml} | 4 +- .github/workflows/on-push-test.yml | 52 ++++++++++++++ Makefile | 26 ++++--- mock_src/__init__.py | 0 mock_src/rados.py | 3 + pyproject.toml | 2 +- requirements.txt | 1 + src/rookify/modules/ceph.py | 12 ++-- .../modules/create_rook_resources/main.py | 4 +- src/rookify/modules/k8s.py | 10 +-- 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 +- src/rookify/modules/module.py | 33 ++++----- tests/mock_ceph.py | 6 +- tests/mock_k8s.py | 4 +- tests/mock_k8s_prerequisite_check.py | 12 ++++ tests/modules/test_k8s_prerequisites_check.py | 69 +++++++++++++++++++ tests/test_mock_ceph.py | 4 +- tests/test_mock_k8s.py | 6 +- 22 files changed, 200 insertions(+), 64 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 create mode 100644 tests/mock_k8s_prerequisite_check.py create mode 100644 tests/modules/test_k8s_prerequisites_check.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/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 From 24ff45ec4180d3118cdfab36bac8b17eac6f3df9 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 10 Sep 2024 22:05:08 +0200 Subject: [PATCH 07/35] Add support for external modules This commmit adds support to define and use external modules for custom migration steps. Signed-off-by: Tobias Wolf --- Dockerfile | 1 + src/rookify/modules/__init__.py | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index f9f7c80..03e3b1e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -27,6 +27,7 @@ LABEL org.opencontainers.image.source="https://github.com/SovereignCloudStack/ro ARG ROOKIFY_VERSION=0.0.0.dev1 ENV ROOKIFY_VERSION=$ROOKIFY_VERSION +ENV PYTHONPATH="${PYTHONPATH}:/app/rookify/src" WORKDIR /app/rookify diff --git a/src/rookify/modules/__init__.py b/src/rookify/modules/__init__.py index a5c4947..51af7e6 100644 --- a/src/rookify/modules/__init__.py +++ b/src/rookify/modules/__init__.py @@ -30,7 +30,16 @@ def _load_module(machine: Machine, config: Dict[str, Any], module_name: str) -> :return: returns tuple of preflight_modules, modules """ - module = importlib.import_module("rookify.modules.{0}".format(module_name)) + if "." in module_name: + absolute_module_name = module_name + else: + absolute_module_name = "rookify.modules.{0}".format(module_name) + + try: + module = importlib.import_module(absolute_module_name) + except ModuleNotFoundError as e: + raise ModuleLoadException(module_name, str(e)) + additional_modules = [] if not hasattr(module, "ModuleHandler") or not callable( @@ -63,6 +72,11 @@ def load_modules(machine: Machine, config: Dict[str, Any]) -> None: migration_modules.remove(entry.name) _load_module(machine, config, entry.name) + for migration_module in migration_modules.copy(): + if "." in migration_module: + migration_modules.remove(migration_module) + _load_module(machine, config, migration_module) + if len(migration_modules) > 0 or len(config["migration_modules"]) < 1: logger = get_logger() From a261611d8df0f76166a753de2acf2dd1e75ab5f5 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 10 Sep 2024 22:31:07 +0200 Subject: [PATCH 08/35] Bump cryptography from 42.0.4 to 43.0.1 (#75) Bumps [cryptography](https://github.com/pyca/cryptography) from 42.0.4 to 43.0.1. - [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst) - [Commits](https://github.com/pyca/cryptography/compare/42.0.4...43.0.1) --- updated-dependencies: - dependency-name: cryptography dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: Tobias Wolf --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 3f4fb3f..e21e376 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ cachetools==5.3.2 certifi==2024.2.2 cffi==1.16.0 charset-normalizer==3.3.2 -cryptography==42.0.4 +cryptography==43.0.1 dill==0.3.8 decorator==5.1.1 Deprecated==1.2.14 From c1b526ea3304aa5e46f2114b8c5c35fe85b52e80 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 10 Sep 2024 22:32:35 +0200 Subject: [PATCH 09/35] Bump certifi from 2024.2.2 to 2024.7.4 (#80) Bumps [certifi](https://github.com/certifi/python-certifi) from 2024.2.2 to 2024.7.4. - [Commits](https://github.com/certifi/python-certifi/compare/2024.02.02...2024.07.04) --- updated-dependencies: - dependency-name: certifi dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: Tobias Wolf --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index e21e376..ac2b413 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ bcrypt==4.1.2 cachetools==5.3.2 -certifi==2024.2.2 +certifi==2024.7.4 cffi==1.16.0 charset-normalizer==3.3.2 cryptography==43.0.1 From 48b013a4334c72e15df29b675e7d35a662cb671c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 10 Sep 2024 22:33:19 +0200 Subject: [PATCH 10/35] Bump jinja2 from 3.1.3 to 3.1.4 (#81) Bumps [jinja2](https://github.com/pallets/jinja) from 3.1.3 to 3.1.4. - [Release notes](https://github.com/pallets/jinja/releases) - [Changelog](https://github.com/pallets/jinja/blob/main/CHANGES.rst) - [Commits](https://github.com/pallets/jinja/compare/3.1.3...3.1.4) --- updated-dependencies: - dependency-name: jinja2 dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: Tobias Wolf --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index ac2b413..72fadc3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,7 @@ fabric==3.2.2 google-auth==2.28.1 idna==3.6 invoke==2.2.0 -Jinja2==3.1.3 +Jinja2==3.1.4 kubernetes==29.0.0 MarkupSafe==2.1.5 oauthlib==3.2.2 From 47ce0397ff1cbc340bf79a98ac7dff993f64e55a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 10 Sep 2024 22:33:52 +0200 Subject: [PATCH 11/35] Bump idna from 3.6 to 3.7 (#82) Bumps [idna](https://github.com/kjd/idna) from 3.6 to 3.7. - [Release notes](https://github.com/kjd/idna/releases) - [Changelog](https://github.com/kjd/idna/blob/master/HISTORY.rst) - [Commits](https://github.com/kjd/idna/compare/v3.6...v3.7) --- updated-dependencies: - dependency-name: idna dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: Tobias Wolf --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 72fadc3..cff87c3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,7 +9,7 @@ decorator==5.1.1 Deprecated==1.2.14 fabric==3.2.2 google-auth==2.28.1 -idna==3.6 +idna==3.7 invoke==2.2.0 Jinja2==3.1.4 kubernetes==29.0.0 From 0d6328c3ddeaebf28eac3f6867ce8d7968327b5c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 10 Sep 2024 22:34:25 +0200 Subject: [PATCH 12/35] Bump requests from 2.31.0 to 2.32.2 (#83) Bumps [requests](https://github.com/psf/requests) from 2.31.0 to 2.32.2. - [Release notes](https://github.com/psf/requests/releases) - [Changelog](https://github.com/psf/requests/blob/main/HISTORY.md) - [Commits](https://github.com/psf/requests/compare/v2.31.0...v2.32.2) --- updated-dependencies: - dependency-name: requests dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: Tobias Wolf --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index cff87c3..99f27de 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,7 +22,7 @@ pycparser==2.21 PyNaCl==1.5.0 python-dateutil==2.8.2 PyYAML==6.0.1 -requests==2.31.0 +requests==2.32.2 requests-oauthlib==1.3.1 rsa==4.9 six==1.16.0 From 77b7263689906b6c069fb265cab2c11ffe144e02 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 10 Sep 2024 22:34:57 +0200 Subject: [PATCH 13/35] Bump urllib3 from 2.2.1 to 2.2.2 (#84) Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.2.1 to 2.2.2. - [Release notes](https://github.com/urllib3/urllib3/releases) - [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst) - [Commits](https://github.com/urllib3/urllib3/compare/2.2.1...2.2.2) --- updated-dependencies: - dependency-name: urllib3 dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: Tobias Wolf --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 99f27de..b57a7b0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,7 +28,7 @@ rsa==4.9 six==1.16.0 structlog==24.1.0 transitions==0.9.0 -urllib3==2.2.1 +urllib3==2.2.2 yamale==5.1.0 websocket-client==1.7.0 wrapt==1.16.0 From 3e9d54decb01750197ff5d146a9b9387a52abf24 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Wed, 11 Sep 2024 08:44:12 +0000 Subject: [PATCH 14/35] fix: change decision tree for arguments to clearify usage of --show-progress Signed-off-by: Boekhorst --- mock.pickle | Bin 0 -> 66 bytes src/rookify/__main__.py | 76 ++++++++++++++++------- src/rookify/modules/analyze_ceph/main.py | 4 ++ tests/test_main.py | 33 ++++++++++ 4 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 mock.pickle diff --git a/mock.pickle b/mock.pickle new file mode 100644 index 0000000000000000000000000000000000000000..a4d3c945bef508c85556f9882f454b589ae4b0d5 GIT binary patch literal 66 zcmZo*nQG1e0ku Namespace: +def parse_args(args: list[str]) -> argparse.Namespace: # Putting args-parser in seperate function to make this testable arg_parser = ArgumentParser("Rookify") arg_parser.add_argument("--dry-run", action="store_true", dest="dry_run_mode") @@ -22,7 +23,7 @@ class ReadAction(argparse.Action): def __call__( self, parser: ArgumentParser, - namespace: Namespace, + namespace: argparse.Namespace, values: Optional[Any], option_string: Optional[str] = None, ) -> None: @@ -33,7 +34,7 @@ class ShowProgressAction(argparse.Action): def __call__( self, parser: ArgumentParser, - namespace: Namespace, + namespace: argparse.Namespace, values: Optional[Any], option_string: Optional[str] = None, ) -> None: @@ -77,6 +78,26 @@ def sort_pickle_file(unsorted_states_data: Dict[str, Any]) -> Dict[str, Any]: return sorted_data_by_keys +def read_pickle_file( + args: argparse.Namespace, pickle_file_name: str, log: BindableLogger +) -> None: + states_data = load_pickler(pickle_file_name) + sorted_states_data = sort_pickle_file(states_data) + + # Check if a specific section should be listed + if args.read_pickle != "all": + if args.read_pickle not in sorted_states_data.keys(): + log.error(f"The section {args.read_pickle} does not exist") + else: + sorted_states_data = sorted_states_data[args.read_pickle] + + log.info( + 'Current state as retrieved from pickle-file: \n "{0}": {1}'.format( + args.read_pickle, json.dumps(sorted_states_data, indent=4) + ) + ) + + def main() -> None: args = parse_args(sys.argv[1:]) @@ -108,26 +129,35 @@ def main() -> None: else: log.info(f"Pickle file set: {pickle_file_name}") - # If read_pickle is not None and pickle_file_name is not None, show the current progress of the migration - if args.read_pickle is not None: - if pickle_file_name is None: - return - states_data = load_pickler(pickle_file_name) - sorted_states_data = sort_pickle_file(states_data) - - # Check if a specific section should be listed - if args.read_pickle != "all": - if args.read_pickle not in sorted_states_data.keys(): - get_logger().error(f"The section {args.read_pickle} does not exist") - return - else: - sorted_states_data = sorted_states_data[args.read_pickle] - - get_logger().info( - 'Current state as retrieved from pickle-file: \n "{0}": {1}'.format( - args.read_pickle, json.dumps(sorted_states_data, indent=4) - ) + # If read_pickle is run and there is a picklefile, show the picklefiles contents. + # NOTE: preflight mode (--dry-run) has no effect here, because no module actions are required. + if args.read_pickle is not None and pickle_file_name is not None: + read_pickle_file(args, pickle_file_name, log) + return + elif args.read_pickle is not None and pickle_file_name is None: + log.info( + "No pickle file configured to read from. Check if the pickle file exists and is configured in config.yaml" ) + return + + # If show_progress is run and there is a picklefile, show progress status based on picklefile contents + # NOTE: this is always run in preflight-mode (migration shoudl not be executed) + if args.show_progress is not None and pickle_file_name is not None: + # Check if a specific mode should be targeted + if args.show_progress != "all": + print("TEST") + print(args.show_progress) + + # TODO: implement module and method loading to let module check state itself + return + + # TODO: should progress be checkable if no pickle file is present? Should rookify check the status by analyzing the source and traget machines state? + elif args.show_progress is not None and pickle_file_name is None: + log.info( + "Currently rookify can only check the state of progress by analyzing the pickle file states." + ) + return + # Else run the rook migration else: log.debug("Executing Rookify") diff --git a/src/rookify/modules/analyze_ceph/main.py b/src/rookify/modules/analyze_ceph/main.py index 1698196..6bc0d71 100644 --- a/src/rookify/modules/analyze_ceph/main.py +++ b/src/rookify/modules/analyze_ceph/main.py @@ -6,6 +6,10 @@ class AnalyzeCephHandler(ModuleHandler): + def status(self) -> Any: + # state = self.machine.get_preflight_state("AnalyzeCephHandler") + self.logger.info("AnalyzeCephHander has allready been run") + def preflight(self) -> Any: commands = ["mon dump", "osd dump", "device ls", "fs ls", "node ls"] diff --git a/tests/test_main.py b/tests/test_main.py index f61edf3..3a0c5b7 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -11,6 +11,13 @@ from rookify.__main__ import parse_args, main, sort_pickle_file import rookify.yaml +# +# These tests test the src/rookify/__main__.py +# +# Run all tests: .venv/bin/python -m pytest tests/ +# Run one test with output: .venv/bin/python -m pytest tests/ -k name_of_test -s +# + # Test the arugment parser def test_parse_args_dry_run() -> None: @@ -37,6 +44,14 @@ def test_parse_args_show_progress() -> None: assert args == expected +def test_parse_args_show_progress_with_module() -> None: + args = parse_args(["--show-progress", "ceph-analyze"]) + expected = Namespace( + dry_run_mode=False, read_pickle=None, show_progress="ceph-analyze" + ) + assert args == expected + + # check: should it be possible to add all arguments? def test_parse_args_both_dry_run_show_progress() -> None: args = parse_args(["--dry-run", "--read-pickle", "--show-progress"]) @@ -209,3 +224,21 @@ def getUnsortedData() -> Any: } return unsorted_states_data + + +def test_ceph_analyze_progress( + mock_load_config: Callable[[Optional[Any]], MagicMock], + mock_load_pickler: MonkeyPatch, + monkeypatch: MonkeyPatch, + log: pytest_structlog.StructuredLogCapture, +) -> None: + # Load example config with mock.pickle as pickle file + mock_load_config("mock.pickle") + + # Mock sys.argv to simulate command-line arguments + monkeypatch.setattr( + sys, "argv", ["main_script.py", "--dry-run", "--show-progress", "analyze_ceph"] + ) + + # Run main() + main() From e15ce2694fd5e21146dcbf7d41ae979c78fb1f0b Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Wed, 11 Sep 2024 09:40:04 +0000 Subject: [PATCH 15/35] feat: add cli option to list modules. Also add initial code to deal with progressper module Signed-off-by: Boekhorst --- src/rookify/__main__.py | 50 ++++++++++++++++++++++++++++++++++++----- tests/test_main.py | 33 ++++++++++++++++++++------- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/src/rookify/__main__.py b/src/rookify/__main__.py index 981d802..9ddcfbb 100644 --- a/src/rookify/__main__.py +++ b/src/rookify/__main__.py @@ -11,13 +11,21 @@ from .logger import configure_logging, get_logger from .yaml import load_config from structlog.typing import BindableLogger +from pathlib import Path def parse_args(args: list[str]) -> argparse.Namespace: # Putting args-parser in seperate function to make this testable arg_parser = ArgumentParser("Rookify") + + # --dry-run option arg_parser.add_argument("--dry-run", action="store_true", dest="dry_run_mode") + # --list-modules option + arg_parser.add_argument( + "--list-modules", action="store_true", help="List all modules" + ) + # Custom ReadAction to set 'all' if nothing is specified for --read-pickle class ReadAction(argparse.Action): def __call__( @@ -69,6 +77,16 @@ def load_pickler(pickle_file_name: str) -> Any: return states_data +def get_all_modules() -> list[str]: + base_path = Path(__file__).resolve().parent + module_path = Path(base_path) / "modules" + module_names = [] + for item in module_path.iterdir(): + if item.is_dir() and item.name != "__pycache__": + module_names.append(item.name) + return module_names + + def sort_pickle_file(unsorted_states_data: Dict[str, Any]) -> Dict[str, Any]: # sort the pickle-file alfabetically iterable_dict = iter(unsorted_states_data) @@ -98,9 +116,34 @@ def read_pickle_file( ) +def show_progress_from_pickle_file( + args: argparse.Namespace, pickle_file_name: str, log: BindableLogger +) -> None: + # states_data = load_pickler(pickle_file_name) + modules = get_all_modules() + + # Check if a specific module should be targeted + # TODO: allow for multiple modules + if args.show_progress != "all": + module = args.show_progress + if args.show_progress not in modules: + log.error(f"The module {module} does not exist") + log.info( + 'Progress of : \n "{0}": {1}'.format(args.show_progress, "some....progess....") + ) + + def main() -> None: args = parse_args(sys.argv[1:]) + # Handle --list-modules + if args.list_modules: + modules = get_all_modules() + print("Available modules:\n") + for module in modules: + print(f"- {module}") + return + # Load configuration file try: config: Dict[str, Any] = load_config("config.yaml") @@ -143,12 +186,7 @@ def main() -> None: # If show_progress is run and there is a picklefile, show progress status based on picklefile contents # NOTE: this is always run in preflight-mode (migration shoudl not be executed) if args.show_progress is not None and pickle_file_name is not None: - # Check if a specific mode should be targeted - if args.show_progress != "all": - print("TEST") - print(args.show_progress) - - # TODO: implement module and method loading to let module check state itself + show_progress_from_pickle_file(args, pickle_file_name, log) return # TODO: should progress be checkable if no pickle file is present? Should rookify check the status by analyzing the source and traget machines state? diff --git a/tests/test_main.py b/tests/test_main.py index 3a0c5b7..1836325 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -22,32 +22,43 @@ # Test the arugment parser def test_parse_args_dry_run() -> None: args = parse_args(["--dry-run"]) - expected = Namespace(dry_run_mode=True, read_pickle=None, show_progress=None) + expected = Namespace( + dry_run_mode=True, list_modules=False, read_pickle=None, show_progress=None + ) assert args == expected def test_parse_args_read_pickle() -> None: args = parse_args(["--read-pickle"]) - expected = Namespace(dry_run_mode=False, read_pickle="all", show_progress=None) + expected = Namespace( + dry_run_mode=False, list_modules=False, read_pickle="all", show_progress=None + ) assert args == expected def test_parse_args_both_flags() -> None: args = parse_args(["--dry-run", "--read-pickle"]) - expected = Namespace(dry_run_mode=True, read_pickle="all", show_progress=None) + expected = Namespace( + dry_run_mode=True, list_modules=False, read_pickle="all", show_progress=None + ) assert args == expected def test_parse_args_show_progress() -> None: args = parse_args(["--show-progress"]) - expected = Namespace(dry_run_mode=False, read_pickle=None, show_progress="all") + expected = Namespace( + dry_run_mode=False, list_modules=False, read_pickle=None, show_progress="all" + ) assert args == expected def test_parse_args_show_progress_with_module() -> None: args = parse_args(["--show-progress", "ceph-analyze"]) expected = Namespace( - dry_run_mode=False, read_pickle=None, show_progress="ceph-analyze" + dry_run_mode=False, + list_modules=False, + read_pickle=None, + show_progress="ceph-analyze", ) assert args == expected @@ -55,19 +66,25 @@ def test_parse_args_show_progress_with_module() -> None: # check: should it be possible to add all arguments? def test_parse_args_both_dry_run_show_progress() -> None: args = parse_args(["--dry-run", "--read-pickle", "--show-progress"]) - expected = Namespace(dry_run_mode=True, read_pickle="all", show_progress="all") + expected = Namespace( + dry_run_mode=True, list_modules=False, read_pickle="all", show_progress="all" + ) assert args == expected def test_parse_args_all_dry_run_show_progress_read_pickle() -> None: args = parse_args(["--dry-run", "--show-progress"]) - expected = Namespace(dry_run_mode=True, read_pickle=None, show_progress="all") + expected = Namespace( + dry_run_mode=True, list_modules=False, read_pickle=None, show_progress="all" + ) assert args == expected def test_parse_args_no_flags() -> None: args = parse_args([]) - expected = Namespace(dry_run_mode=False, read_pickle=None, show_progress=None) + expected = Namespace( + dry_run_mode=False, list_modules=False, read_pickle=None, show_progress=None + ) assert args == expected From db931ccd1323571d2493f37549952a8a80428af4 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Wed, 11 Sep 2024 10:02:19 +0000 Subject: [PATCH 16/35] fix: simplify argument tests by add fixture Signed-off-by: Boekhorst --- tests/test_main.py | 97 +++++++++++++--------------------------------- 1 file changed, 27 insertions(+), 70 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 1836325..0fb4ed9 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,12 +1,12 @@ import sys -from typing import Any, Callable, Optional +from typing import Any, Callable, Optional, List, Tuple import pytest from _pytest.monkeypatch import MonkeyPatch import yaml from unittest.mock import MagicMock import pytest_structlog -from argparse import Namespace +import argparse from rookify.__main__ import parse_args, main, sort_pickle_file import rookify.yaml @@ -20,75 +20,32 @@ # Test the arugment parser -def test_parse_args_dry_run() -> None: - args = parse_args(["--dry-run"]) - expected = Namespace( - dry_run_mode=True, list_modules=False, read_pickle=None, show_progress=None - ) - assert args == expected - - -def test_parse_args_read_pickle() -> None: - args = parse_args(["--read-pickle"]) - expected = Namespace( - dry_run_mode=False, list_modules=False, read_pickle="all", show_progress=None - ) - assert args == expected - - -def test_parse_args_both_flags() -> None: - args = parse_args(["--dry-run", "--read-pickle"]) - expected = Namespace( - dry_run_mode=True, list_modules=False, read_pickle="all", show_progress=None - ) - assert args == expected - - -def test_parse_args_show_progress() -> None: - args = parse_args(["--show-progress"]) - expected = Namespace( - dry_run_mode=False, list_modules=False, read_pickle=None, show_progress="all" - ) - assert args == expected - - -def test_parse_args_show_progress_with_module() -> None: - args = parse_args(["--show-progress", "ceph-analyze"]) - expected = Namespace( - dry_run_mode=False, - list_modules=False, - read_pickle=None, - show_progress="ceph-analyze", - ) - assert args == expected - - -# check: should it be possible to add all arguments? -def test_parse_args_both_dry_run_show_progress() -> None: - args = parse_args(["--dry-run", "--read-pickle", "--show-progress"]) - expected = Namespace( - dry_run_mode=True, list_modules=False, read_pickle="all", show_progress="all" - ) - assert args == expected - - -def test_parse_args_all_dry_run_show_progress_read_pickle() -> None: - args = parse_args(["--dry-run", "--show-progress"]) - expected = Namespace( - dry_run_mode=True, list_modules=False, read_pickle=None, show_progress="all" - ) - assert args == expected - - -def test_parse_args_no_flags() -> None: - args = parse_args([]) - expected = Namespace( - dry_run_mode=False, list_modules=False, read_pickle=None, show_progress=None - ) - assert args == expected - -# Test the --read-pickle and --show-progress options +TestCase = Tuple[List[str], argparse.Namespace] + +# fmt: off +test_cases: List[TestCase] = [ + (["--dry-run"], argparse.Namespace(dry_run_mode=True, list_modules=False, read_pickle=None, show_progress=None)), + (["--read-pickle"], argparse.Namespace(dry_run_mode=False, list_modules=False, read_pickle="all", show_progress=None)), + (["--show-progress"], argparse.Namespace(dry_run_mode=False, list_modules=False, read_pickle=None, show_progress="all")), + (["--show-progress", "ceph-analyze"], argparse.Namespace(dry_run_mode=False, list_modules=False, read_pickle=None, show_progress="ceph-analyze")), + (["--dry-run", "--read-pickle"], argparse.Namespace(dry_run_mode=True, list_modules=False, read_pickle="all", show_progress=None)), + (["--dry-run", "--show-progress"], argparse.Namespace(dry_run_mode=True, list_modules=False, read_pickle=None, show_progress="all")), + (["--dry-run", "--show-progress", "--read-pickle"], argparse.Namespace(dry_run_mode=True, list_modules=False, read_pickle="all", show_progress="all")), + ([], argparse.Namespace(dry_run_mode=False, list_modules=False, read_pickle=None, show_progress=None)), +] +# fmt: on + + +@pytest.mark.parametrize( + "args_list, expected_namespace", + test_cases, +) # type: ignore +def test_parse_args( + args_list: List[str], expected_namespace: argparse.Namespace +) -> None: + args = parse_args(args_list) + assert args == expected_namespace @pytest.fixture # type: ignore From 55dd5856250858e3ac02b776aefe5523f8a699e1 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Thu, 12 Sep 2024 06:22:13 +0000 Subject: [PATCH 17/35] fix: using more parametrized fixtures to add more tests and simplify code Signed-off-by: Boekhorst --- src/rookify/__main__.py | 7 +++---- tests/test_main.py | 32 +++++++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/rookify/__main__.py b/src/rookify/__main__.py index 9ddcfbb..4f2024f 100644 --- a/src/rookify/__main__.py +++ b/src/rookify/__main__.py @@ -123,14 +123,13 @@ def show_progress_from_pickle_file( modules = get_all_modules() # Check if a specific module should be targeted - # TODO: allow for multiple modules if args.show_progress != "all": module = args.show_progress if args.show_progress not in modules: log.error(f"The module {module} does not exist") - log.info( - 'Progress of : \n "{0}": {1}'.format(args.show_progress, "some....progess....") - ) + log.info("Show progress of the {0} module".format(args.show_progress)) + else: + log.info("Show progress of {0} modules".format(args.show_progress)) def main() -> None: diff --git a/tests/test_main.py b/tests/test_main.py index 0fb4ed9..6ddcec1 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -21,6 +21,7 @@ # Test the arugment parser +# Custom type for argument parser tests TestCase = Tuple[List[str], argparse.Namespace] # fmt: off @@ -200,19 +201,40 @@ def getUnsortedData() -> Any: return unsorted_states_data -def test_ceph_analyze_progress( +# Custom Type for Logger of CLI +TestCaseLogger = Tuple[List[str], str, str] + +# fmt: off +logger_test_cases: List[TestCaseLogger] = [ + (["--show-progress", "analyze_ceph"], "Show progress of the analyze_ceph module", "info"), + (["--dry-run", "--show-progress", "analyze_ceph"], "Show progress of the analyze_ceph module", "info"), + (["--show-progress"], "Show progress of all modules", "info"), + (["--dry-run", "--show-progress"], "Show progress of all modules", "info") +] +# fmt: on + + +@pytest.mark.parametrize( + "args_list, expected_log_message, expected_level", + logger_test_cases, +) # type: ignore +def test_show_progress( mock_load_config: Callable[[Optional[Any]], MagicMock], - mock_load_pickler: MonkeyPatch, monkeypatch: MonkeyPatch, log: pytest_structlog.StructuredLogCapture, + args_list: List[str], + expected_log_message: str, + expected_level: str, ) -> None: # Load example config with mock.pickle as pickle file mock_load_config("mock.pickle") # Mock sys.argv to simulate command-line arguments - monkeypatch.setattr( - sys, "argv", ["main_script.py", "--dry-run", "--show-progress", "analyze_ceph"] - ) + args_list.insert(0, "main_script.py") + monkeypatch.setattr(sys, "argv", args_list) # Run main() main() + + # Assertions + assert log.has(expected_log_message, level=expected_level) From b33cd08c3b29ae7597632c35d7537059bb7546a0 Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Thu, 12 Sep 2024 08:34:54 +0000 Subject: [PATCH 18/35] feat: add status command to ceph-analyze module Signed-off-by: Boekhorst --- src/rookify/modules/analyze_ceph/main.py | 57 +++++++++++++++++------- tests/test_main.py | 5 ++- 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/src/rookify/modules/analyze_ceph/main.py b/src/rookify/modules/analyze_ceph/main.py index 6bc0d71..607c7cf 100644 --- a/src/rookify/modules/analyze_ceph/main.py +++ b/src/rookify/modules/analyze_ceph/main.py @@ -1,35 +1,62 @@ # -*- coding: utf-8 -*- -from typing import Any +from typing import Any, Optional from ..machine import Machine from ..module import ModuleHandler class AnalyzeCephHandler(ModuleHandler): - def status(self) -> Any: - # state = self.machine.get_preflight_state("AnalyzeCephHandler") - self.logger.info("AnalyzeCephHander has allready been run") + def _process_command( + self, state_data: Any, command: str, value: Optional[Any] = None + ) -> bool: + """Helper method to process commands by either setting or checking state data.""" + parts = command.split(" ") + current_level = state_data # the root of the data structure + + # Traverse the dictionary structure based on command parts + for idx, part in enumerate(parts): + if len(parts) == idx + 1: # Last part of the command + if value is not None: + current_level[part] = value + else: + return part in current_level + else: + if part not in current_level: + current_level[part] = {} + current_level = current_level[part] + + return True def preflight(self) -> Any: commands = ["mon dump", "osd dump", "device ls", "fs ls", "node ls"] - state = self.machine.get_preflight_state("AnalyzeCephHandler") state.data = {} + # Execute each command and store the result for command in commands: - parts = command.split(" ") - leaf = state.data - - for idx, part in enumerate(parts): - if len(parts) == idx + 1: - leaf[part] = self.ceph.mon_command(command) - else: - if part not in leaf: - leaf[part] = {} - leaf = leaf[part] + result = self.ceph.mon_command(command) + self._process_command(state.data, command, result) self.logger.info("AnalyzeCephHandler ran successfully.") + def status(self) -> Any: + commands = ["mon dump", "osd dump", "device ls", "fs ls", "node ls"] + state = self.machine.get_preflight_state("AnalyzeCephHandler") + + # Check if all expected commands have been run + all_commands_found = True + for command in commands: + if not self._process_command(state.data, command): + all_commands_found = False + break + + # Log the status + if all_commands_found: + self.logger.info("AnalyzeCephHandler has already been run.") + self.logger.info("Current state data: %s", state.data) + else: + self.logger.info("Progress: Not all commands have been run yet.") + @staticmethod def register_preflight_state( machine: Machine, state_name: str, handler: ModuleHandler, **kwargs: Any diff --git a/tests/test_main.py b/tests/test_main.py index 6ddcec1..18350e3 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -209,7 +209,8 @@ def getUnsortedData() -> Any: (["--show-progress", "analyze_ceph"], "Show progress of the analyze_ceph module", "info"), (["--dry-run", "--show-progress", "analyze_ceph"], "Show progress of the analyze_ceph module", "info"), (["--show-progress"], "Show progress of all modules", "info"), - (["--dry-run", "--show-progress"], "Show progress of all modules", "info") + (["--dry-run", "--show-progress"], "Show progress of all modules", "info"), + (["--show-progress"], "Analyze ceph has been run", "info") ] # fmt: on @@ -218,7 +219,7 @@ def getUnsortedData() -> Any: "args_list, expected_log_message, expected_level", logger_test_cases, ) # type: ignore -def test_show_progress( +def test_show_progress_logs( mock_load_config: Callable[[Optional[Any]], MagicMock], monkeypatch: MonkeyPatch, log: pytest_structlog.StructuredLogCapture, From bff688cbc2f2d0b70de3de5b7b9ced55e399fbce Mon Sep 17 00:00:00 2001 From: Boekhorst Date: Thu, 12 Sep 2024 14:10:35 +0000 Subject: [PATCH 19/35] fix: add status command through load modules, probably should add an own status-mode? Signed-off-by: Boekhorst --- src/rookify/__main__.py | 37 +++++++++++------------- src/rookify/modules/__init__.py | 19 ++++++++---- src/rookify/modules/analyze_ceph/main.py | 4 ++- src/rookify/modules/machine.py | 4 ++- src/rookify/modules/module.py | 29 +++++++++++++++++-- tests/test_main.py | 2 +- 6 files changed, 64 insertions(+), 31 deletions(-) diff --git a/src/rookify/__main__.py b/src/rookify/__main__.py index 4f2024f..7f079d5 100644 --- a/src/rookify/__main__.py +++ b/src/rookify/__main__.py @@ -116,9 +116,9 @@ def read_pickle_file( ) -def show_progress_from_pickle_file( +def show_progress_from_state( args: argparse.Namespace, pickle_file_name: str, log: BindableLogger -) -> None: +) -> bool: # states_data = load_pickler(pickle_file_name) modules = get_all_modules() @@ -127,9 +127,12 @@ def show_progress_from_pickle_file( module = args.show_progress if args.show_progress not in modules: log.error(f"The module {module} does not exist") + return False log.info("Show progress of the {0} module".format(args.show_progress)) + return True else: log.info("Show progress of {0} modules".format(args.show_progress)) + return True def main() -> None: @@ -182,24 +185,18 @@ def main() -> None: ) return - # If show_progress is run and there is a picklefile, show progress status based on picklefile contents - # NOTE: this is always run in preflight-mode (migration shoudl not be executed) - if args.show_progress is not None and pickle_file_name is not None: - show_progress_from_pickle_file(args, pickle_file_name, log) - return - - # TODO: should progress be checkable if no pickle file is present? Should rookify check the status by analyzing the source and traget machines state? - elif args.show_progress is not None and pickle_file_name is None: - log.info( - "Currently rookify can only check the state of progress by analyzing the pickle file states." - ) - return - - # Else run the rook migration else: - log.debug("Executing Rookify") - machine = Machine(config["general"].get("machine_pickle_file")) - load_modules(machine, config) - machine.execute(dry_run_mode=args.dry_run_mode) + if args.show_progress is not None: + if show_progress_from_state(args, pickle_file_name, log) is True: + load_modules(machine, config, show_progress=True) + # NOTE: this is always run in preflight-mode (migration should not be executed) + machine.execute(dry_run_mode=True) + return + else: + return + else: + load_modules(machine, config) + log.debug("Executing Rookify") + machine.execute(dry_run_mode=args.dry_run_mode) diff --git a/src/rookify/modules/__init__.py b/src/rookify/modules/__init__.py index a5c4947..d959cd3 100644 --- a/src/rookify/modules/__init__.py +++ b/src/rookify/modules/__init__.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- import importlib -from typing import Any, Dict +from typing import Any, Dict, Optional from ..logger import get_logger from .machine import Machine @@ -22,7 +22,12 @@ def __init__(self, module_name: str, message: str): self.message = message -def _load_module(machine: Machine, config: Dict[str, Any], module_name: str) -> None: +def _load_module( + machine: Machine, + config: Dict[str, Any], + module_name: str, + show_progress: Optional[bool] = False, +) -> None: """ Dynamically loads a module from the 'rookify.modules' package. @@ -43,12 +48,14 @@ def _load_module(machine: Machine, config: Dict[str, Any], module_name: str) -> additional_modules = module.ModuleHandler.REQUIRES for module_name in additional_modules: - _load_module(machine, config, module_name) + _load_module(machine, config, module_name, show_progress) - module.ModuleHandler.register_states(machine, config) + module.ModuleHandler.register_states(machine, config, show_progress) -def load_modules(machine: Machine, config: Dict[str, Any]) -> None: +def load_modules( + machine: Machine, config: Dict[str, Any], show_progress: Optional[bool] = False +) -> None: """ Dynamically loads modules from the 'modules' package. @@ -61,7 +68,7 @@ def load_modules(machine: Machine, config: Dict[str, Any]) -> None: for entry in importlib.resources.files("rookify.modules").iterdir(): if entry.is_dir() and entry.name in config["migration_modules"]: migration_modules.remove(entry.name) - _load_module(machine, config, entry.name) + _load_module(machine, config, entry.name, show_progress) if len(migration_modules) > 0 or len(config["migration_modules"]) < 1: logger = get_logger() diff --git a/src/rookify/modules/analyze_ceph/main.py b/src/rookify/modules/analyze_ceph/main.py index 607c7cf..293c9e3 100644 --- a/src/rookify/modules/analyze_ceph/main.py +++ b/src/rookify/modules/analyze_ceph/main.py @@ -55,7 +55,9 @@ def status(self) -> Any: self.logger.info("AnalyzeCephHandler has already been run.") self.logger.info("Current state data: %s", state.data) else: - self.logger.info("Progress: Not all commands have been run yet.") + self.logger.info( + "AnalyzeCephHandler Progress: Not all commands have been run yet." + ) @staticmethod def register_preflight_state( diff --git a/src/rookify/modules/machine.py b/src/rookify/modules/machine.py index 3a6bcf8..fbc53fd 100644 --- a/src/rookify/modules/machine.py +++ b/src/rookify/modules/machine.py @@ -26,7 +26,9 @@ def add_execution_state(self, name: str, **kwargs: Dict[str, Any]) -> None: def add_preflight_state(self, name: str, **kwargs: Dict[str, Any]) -> None: self._preflight_states.append(self.__class__.state_cls(name, **kwargs)) - def execute(self, dry_run_mode: bool = False) -> None: + def execute( + self, dry_run_mode: bool = False, show_progress: Optional[bool] = False + ) -> None: states = self._preflight_states if not dry_run_mode: states = states + self._execution_states diff --git a/src/rookify/modules/module.py b/src/rookify/modules/module.py index ec6ae8b..0818ce5 100644 --- a/src/rookify/modules/module.py +++ b/src/rookify/modules/module.py @@ -59,6 +59,13 @@ def ssh(self) -> SSH: 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: """ @@ -87,7 +94,12 @@ def load_template(self, filename: str, **variables: Any) -> Template: return template @classmethod - def register_states(cls, machine: Machine, config: Dict[str, Any]) -> None: + def register_states( + cls, + machine: Machine, + config: Dict[str, Any], + show_progress: Optional[bool] = False, + ) -> None: """ Register states for transitions """ @@ -116,7 +128,10 @@ def register_states(cls, machine: Machine, config: Dict[str, Any]) -> None: ) else: if preflight_state_name is not None: - cls.register_preflight_state(machine, preflight_state_name, handler) + if show_progress is True: + cls.register_status_state(machine, preflight_state_name, handler) + else: + cls.register_preflight_state(machine, preflight_state_name, handler) if execution_state_name is not None: cls.register_execution_state(machine, execution_state_name, handler) @@ -131,6 +146,16 @@ def register_preflight_state( machine.add_preflight_state(state_name, on_enter=handler.preflight, **kwargs) + @staticmethod + def register_status_state( + machine: Machine, state_name: str, handler: Any, **kwargs: Any + ) -> None: + """ + Register state for transitions + """ + + machine.add_preflight_state(state_name, on_enter=handler.status, **kwargs) + @staticmethod def register_execution_state( machine: Machine, state_name: str, handler: Any, **kwargs: Any diff --git a/tests/test_main.py b/tests/test_main.py index 18350e3..b163ae4 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -210,7 +210,7 @@ def getUnsortedData() -> Any: (["--dry-run", "--show-progress", "analyze_ceph"], "Show progress of the analyze_ceph module", "info"), (["--show-progress"], "Show progress of all modules", "info"), (["--dry-run", "--show-progress"], "Show progress of all modules", "info"), - (["--show-progress"], "Analyze ceph has been run", "info") + (["--show-progress"], "AnalyzeCephHandler Progress: Not all commands have been run yet.", "info") ] # fmt: on From 3a7f91de8e6a331cf2ca6667dde53e9a5c2719bc Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Sat, 14 Sep 2024 11:54:33 +0200 Subject: [PATCH 20/35] Integrate presentation of the current state into `ModuleHandler`. This partly revers commit 3e9d54decb01750197ff5d146a9b9387a52abf24. This reverts commit e15ce2694fd5e21146dcbf7d41ae979c78fb1f0b. This reverts commit bff688cbc2f2d0b70de3de5b7b9ced55e399fbce. Signed-off-by: Tobias Wolf --- src/rookify/__main__.py | 189 +++++---------------------- src/rookify/modules/__init__.py | 31 +++-- src/rookify/modules/machine.py | 56 +++++--- src/rookify/modules/module.py | 44 +++++-- tests/test_main.py | 221 ++------------------------------ 5 files changed, 128 insertions(+), 413 deletions(-) diff --git a/src/rookify/__main__.py b/src/rookify/__main__.py index 7f079d5..b79f284 100644 --- a/src/rookify/__main__.py +++ b/src/rookify/__main__.py @@ -1,73 +1,15 @@ # -*- coding: utf-8 -*- -import json from pickle import Unpickler import sys import argparse from argparse import ArgumentParser -from typing import Any, Dict, Optional +from typing import Any, Dict from .modules import load_modules from .modules.machine import Machine +from .modules.module import ModuleHandler from .logger import configure_logging, get_logger from .yaml import load_config -from structlog.typing import BindableLogger -from pathlib import Path - - -def parse_args(args: list[str]) -> argparse.Namespace: - # Putting args-parser in seperate function to make this testable - arg_parser = ArgumentParser("Rookify") - - # --dry-run option - arg_parser.add_argument("--dry-run", action="store_true", dest="dry_run_mode") - - # --list-modules option - arg_parser.add_argument( - "--list-modules", action="store_true", help="List all modules" - ) - - # Custom ReadAction to set 'all' if nothing is specified for --read-pickle - class ReadAction(argparse.Action): - def __call__( - self, - parser: ArgumentParser, - namespace: argparse.Namespace, - values: Optional[Any], - option_string: Optional[str] = None, - ) -> None: - setattr(namespace, self.dest, values if values is not None else "all") - - # Custom ShowProgressAction to set 'all' if nothing is specified for --show-progress - class ShowProgressAction(argparse.Action): - def __call__( - self, - parser: ArgumentParser, - namespace: argparse.Namespace, - values: Optional[Any], - option_string: Optional[str] = None, - ) -> None: - setattr(namespace, self.dest, values if values is not None else "all") - - arg_parser.add_argument( - "--read-pickle", - nargs="?", - action=ReadAction, - dest="read_pickle", - metavar="
", - help="Show the content of the pickle file. Default argument is 'all', you can also specify a section you want to look at.", - required=False, - ) - - arg_parser.add_argument( - "--show-progress", - nargs="?", - action=ShowProgressAction, - dest="show_progress", - metavar="", - help="Show progress of the modules. Default argument is 'all', you can also specify a module you want to get the progress status from.", - required=False, - ) - return arg_parser.parse_args(args) def load_pickler(pickle_file_name: str) -> Any: @@ -77,75 +19,26 @@ def load_pickler(pickle_file_name: str) -> Any: return states_data -def get_all_modules() -> list[str]: - base_path = Path(__file__).resolve().parent - module_path = Path(base_path) / "modules" - module_names = [] - for item in module_path.iterdir(): - if item.is_dir() and item.name != "__pycache__": - module_names.append(item.name) - return module_names - - -def sort_pickle_file(unsorted_states_data: Dict[str, Any]) -> Dict[str, Any]: - # sort the pickle-file alfabetically - iterable_dict = iter(unsorted_states_data) - first_key = next(iterable_dict) - data_values = unsorted_states_data[first_key]["data"] - sorted_data_by_keys = {k: data_values[k] for k in sorted(data_values)} - return sorted_data_by_keys - - -def read_pickle_file( - args: argparse.Namespace, pickle_file_name: str, log: BindableLogger -) -> None: - states_data = load_pickler(pickle_file_name) - sorted_states_data = sort_pickle_file(states_data) +def parse_args(args: list[str]) -> argparse.Namespace: + # Putting args-parser in seperate function to make this testable + arg_parser = ArgumentParser("Rookify") - # Check if a specific section should be listed - if args.read_pickle != "all": - if args.read_pickle not in sorted_states_data.keys(): - log.error(f"The section {args.read_pickle} does not exist") - else: - sorted_states_data = sorted_states_data[args.read_pickle] + # --dry-run option + arg_parser.add_argument("--dry-run", action="store_true", dest="dry_run_mode") - log.info( - 'Current state as retrieved from pickle-file: \n "{0}": {1}'.format( - args.read_pickle, json.dumps(sorted_states_data, indent=4) - ) + arg_parser.add_argument( + "--show-states", + action="store_true", + dest="show_states", + help="Show states of the modules.", ) - -def show_progress_from_state( - args: argparse.Namespace, pickle_file_name: str, log: BindableLogger -) -> bool: - # states_data = load_pickler(pickle_file_name) - modules = get_all_modules() - - # Check if a specific module should be targeted - if args.show_progress != "all": - module = args.show_progress - if args.show_progress not in modules: - log.error(f"The module {module} does not exist") - return False - log.info("Show progress of the {0} module".format(args.show_progress)) - return True - else: - log.info("Show progress of {0} modules".format(args.show_progress)) - return True + return arg_parser.parse_args(args) def main() -> None: args = parse_args(sys.argv[1:]) - # Handle --list-modules - if args.list_modules: - modules = get_all_modules() - print("Available modules:\n") - for module in modules: - print(f"- {module}") - return - # Load configuration file try: config: Dict[str, Any] = load_config("config.yaml") @@ -154,49 +47,29 @@ def main() -> None: # Configure logging try: - configure_logging(config["logging"]) + if args.show_states is True: + configure_logging( + {"level": "ERROR", "format": {"renderer": "console", "time": "iso"}} + ) + else: + configure_logging(config["logging"]) except Exception as e: raise SystemExit(f"Error configuring logging: {e}") + # Get Logger log = get_logger() - # Get Pickle File if configured in config.yaml - pickle_file_name = config["general"].get("machine_pickle_file") - if pickle_file_name is None: - log.info("No pickle file was set in the configuration.") - else: - log.info(f"Pickle file set: {pickle_file_name}") + log.info("Executing Rookify ...") - # Get Pickle File if configured in config.yaml - pickle_file_name = config["general"].get("machine_pickle_file") - if pickle_file_name is None: - log.info("No pickle file was set in the configuration.") - else: - log.info(f"Pickle file set: {pickle_file_name}") - - # If read_pickle is run and there is a picklefile, show the picklefiles contents. - # NOTE: preflight mode (--dry-run) has no effect here, because no module actions are required. - if args.read_pickle is not None and pickle_file_name is not None: - read_pickle_file(args, pickle_file_name, log) - return - elif args.read_pickle is not None and pickle_file_name is None: - log.info( - "No pickle file configured to read from. Check if the pickle file exists and is configured in config.yaml" - ) - return + machine = Machine(config["general"].get("machine_pickle_file")) + + load_modules(machine, config) + if args.show_states is True: + ModuleHandler.show_states(machine, config) else: - machine = Machine(config["general"].get("machine_pickle_file")) - - if args.show_progress is not None: - if show_progress_from_state(args, pickle_file_name, log) is True: - load_modules(machine, config, show_progress=True) - # NOTE: this is always run in preflight-mode (migration should not be executed) - machine.execute(dry_run_mode=True) - return - else: - return - else: - load_modules(machine, config) - log.debug("Executing Rookify") - machine.execute(dry_run_mode=args.dry_run_mode) + machine.execute(dry_run_mode=args.dry_run_mode) + + +if __name__ == "__main__": + main() diff --git a/src/rookify/modules/__init__.py b/src/rookify/modules/__init__.py index d959cd3..ebd1c17 100644 --- a/src/rookify/modules/__init__.py +++ b/src/rookify/modules/__init__.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- import importlib -from typing import Any, Dict, Optional +from typing import Any, Dict, List from ..logger import get_logger from .machine import Machine @@ -22,11 +22,18 @@ def __init__(self, module_name: str, message: str): self.message = message +_modules_loaded: List[Any] = [] + + +def get_modules() -> List[Any]: + global _modules_loaded + return _modules_loaded.copy() + + def _load_module( machine: Machine, config: Dict[str, Any], module_name: str, - show_progress: Optional[bool] = False, ) -> None: """ Dynamically loads a module from the 'rookify.modules' package. @@ -35,8 +42,10 @@ def _load_module( :return: returns tuple of preflight_modules, modules """ + global _modules_loaded + module = importlib.import_module("rookify.modules.{0}".format(module_name)) - additional_modules = [] + additional_module_names = [] if not hasattr(module, "ModuleHandler") or not callable( getattr(module.ModuleHandler, "register_states") @@ -45,17 +54,17 @@ def _load_module( if hasattr(module.ModuleHandler, "REQUIRES"): assert isinstance(module.ModuleHandler.REQUIRES, list) - additional_modules = module.ModuleHandler.REQUIRES + additional_module_names = module.ModuleHandler.REQUIRES - for module_name in additional_modules: - _load_module(machine, config, module_name, show_progress) + for additional_module_name in additional_module_names: + _load_module(machine, config, additional_module_name) - module.ModuleHandler.register_states(machine, config, show_progress) + if module not in _modules_loaded: + _modules_loaded.append(module) + module.ModuleHandler.register_states(machine, config) -def load_modules( - machine: Machine, config: Dict[str, Any], show_progress: Optional[bool] = False -) -> None: +def load_modules(machine: Machine, config: Dict[str, Any]) -> None: """ Dynamically loads modules from the 'modules' package. @@ -68,7 +77,7 @@ def load_modules( for entry in importlib.resources.files("rookify.modules").iterdir(): if entry.is_dir() and entry.name in config["migration_modules"]: migration_modules.remove(entry.name) - _load_module(machine, config, entry.name, show_progress) + _load_module(machine, config, entry.name) if len(migration_modules) > 0 or len(config["migration_modules"]) < 1: logger = get_logger() diff --git a/src/rookify/modules/machine.py b/src/rookify/modules/machine.py index c3877d7..2f42a32 100644 --- a/src/rookify/modules/machine.py +++ b/src/rookify/modules/machine.py @@ -26,22 +26,14 @@ def add_execution_state(self, name: str, **kwargs: 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, show_progress: Optional[bool] = False - ) -> None: + def execute(self, dry_run_mode: bool = False) -> None: states = self._preflight_states if not dry_run_mode: states = states + self._execution_states - logger = get_logger() + self._register_states(states) - for state in states: - if state.name not in self.states: - logger.debug("Registering state '{0}'".format(state.name)) - self.add_state(state) - - self.add_state("migrated") - self.add_ordered_transitions(loop=False) + logger = get_logger() if self._machine_pickle_file is None: logger.info("Execution started without machine pickle file") @@ -54,12 +46,8 @@ def execute( def _execute(self, pickle_file: Optional[IO[Any]] = None) -> None: states_data = {} - # Read pickle file if it exists, to continue from the stored state - if pickle_file is not None and pickle_file.tell() > 0: - pickle_file.seek(0) - - states_data = Unpickler(pickle_file).load() - self._restore_state_data(states_data) + if pickle_file is not None: + self._restore_state_data(pickle_file) try: while True: @@ -110,7 +98,39 @@ def get_preflight_state_data( ) -> Any: return getattr(self.get_preflight_state(name), tag, default_value) - def _restore_state_data(self, data: Dict[str, Any]) -> None: + def register_states(self) -> None: + self._register_states(self._preflight_states + self._execution_states) + + if self._machine_pickle_file is not None: + with open(self._machine_pickle_file, "rb") as file: + file.seek(1) + self._restore_state_data(file) + + def _register_states(self, states: List[State]) -> None: + logger = get_logger() + + for state in states: + if state.name not in self.states: + logger.debug("Registering state '{0}'".format(state.name)) + self.add_state(state) + + self.add_state("migrated") + self.add_ordered_transitions(loop=False) + + """ +Read pickle file if it exists, to continue from the stored state. It is +required that the position of the pointer of the pickle file given is not +at the start. + """ + + def _restore_state_data(self, pickle_file: IO[Any]) -> None: + if pickle_file.tell() == 0: + return None + + pickle_file.seek(0) + + data = Unpickler(pickle_file).load() + for state_name in data: try: state = self.get_state(state_name) diff --git a/src/rookify/modules/module.py b/src/rookify/modules/module.py index ead1e4c..967c0bc 100644 --- a/src/rookify/modules/module.py +++ b/src/rookify/modules/module.py @@ -1,9 +1,12 @@ # -*- coding: utf-8 -*- +import abc +import json import os import structlog from typing import Any, Dict, Optional from ..logger import get_logger +from . import get_modules from .ceph import Ceph from .k8s import K8s from .machine import Machine @@ -58,6 +61,17 @@ def ssh(self) -> SSH: self._ssh = SSH(self._config["ssh"]) return self._ssh + def _get_readable_json_dump(self, data: Any) -> Any: + return json.dumps(data, sort_keys=True, indent="\t") + + def get_readable_key_value_state(self) -> Optional[Dict[str, str]]: + """ + Run the modules status check + """ + + return None + + @abc.abstractmethod def preflight(self) -> None: """ Run the modules preflight check @@ -118,10 +132,7 @@ def register_states( ) else: if preflight_state_name is not None: - if show_progress is True: - cls.register_status_state(machine, preflight_state_name, handler) - else: - cls.register_preflight_state(machine, preflight_state_name, handler) + cls.register_preflight_state(machine, preflight_state_name, handler) if execution_state_name is not None: cls.register_execution_state(machine, execution_state_name, handler) @@ -137,21 +148,28 @@ def register_preflight_state( machine.add_preflight_state(state_name, on_enter=handler.preflight, **kwargs) @staticmethod - def register_status_state( + def register_execution_state( machine: Machine, state_name: str, handler: Any, **kwargs: Any ) -> None: """ Register state for transitions """ - machine.add_preflight_state(state_name, on_enter=handler.status, **kwargs) + machine.add_execution_state(state_name, on_enter=handler.execute, **kwargs) @staticmethod - def register_execution_state( - machine: Machine, state_name: str, handler: Any, **kwargs: Any - ) -> None: - """ - Register state for transitions - """ + def show_states(machine: Machine, config: Dict[str, Any]) -> None: + machine.register_states() + modules = get_modules() - machine.add_execution_state(state_name, on_enter=handler.execute, **kwargs) + for module in modules: + module_handler = module.ModuleHandler(machine, config) + + if hasattr(module_handler, "get_readable_key_value_state"): + state_data = module_handler.get_readable_key_value_state() + + if state_data is None: + continue + + for state_key, state_value in state_data.items(): + print("{0}: {1}".format(state_key, state_value)) diff --git a/tests/test_main.py b/tests/test_main.py index b163ae4..6504e33 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,15 +1,7 @@ -import sys -from typing import Any, Callable, Optional, List, Tuple -import pytest -from _pytest.monkeypatch import MonkeyPatch -import yaml -from unittest.mock import MagicMock -import pytest_structlog - import argparse - -from rookify.__main__ import parse_args, main, sort_pickle_file -import rookify.yaml +import pytest +from rookify.__main__ import parse_args +from typing import List, Tuple # # These tests test the src/rookify/__main__.py @@ -18,22 +10,17 @@ # Run one test with output: .venv/bin/python -m pytest tests/ -k name_of_test -s # - -# Test the arugment parser +# Test the argument parser # Custom type for argument parser tests TestCase = Tuple[List[str], argparse.Namespace] # fmt: off test_cases: List[TestCase] = [ - (["--dry-run"], argparse.Namespace(dry_run_mode=True, list_modules=False, read_pickle=None, show_progress=None)), - (["--read-pickle"], argparse.Namespace(dry_run_mode=False, list_modules=False, read_pickle="all", show_progress=None)), - (["--show-progress"], argparse.Namespace(dry_run_mode=False, list_modules=False, read_pickle=None, show_progress="all")), - (["--show-progress", "ceph-analyze"], argparse.Namespace(dry_run_mode=False, list_modules=False, read_pickle=None, show_progress="ceph-analyze")), - (["--dry-run", "--read-pickle"], argparse.Namespace(dry_run_mode=True, list_modules=False, read_pickle="all", show_progress=None)), - (["--dry-run", "--show-progress"], argparse.Namespace(dry_run_mode=True, list_modules=False, read_pickle=None, show_progress="all")), - (["--dry-run", "--show-progress", "--read-pickle"], argparse.Namespace(dry_run_mode=True, list_modules=False, read_pickle="all", show_progress="all")), - ([], argparse.Namespace(dry_run_mode=False, list_modules=False, read_pickle=None, show_progress=None)), + (["--dry-run"], argparse.Namespace(dry_run_mode=True, show_states=False)), + (["--show-states"], argparse.Namespace(dry_run_mode=False, show_states=True)), + (["--dry-run", "--show-states"], argparse.Namespace(dry_run_mode=True, show_states=True)), + ([], argparse.Namespace(dry_run_mode=False, show_states=False)), ] # fmt: on @@ -47,195 +34,3 @@ def test_parse_args( ) -> None: args = parse_args(args_list) assert args == expected_namespace - - -@pytest.fixture # type: ignore -def mock_load_config(monkeypatch: MonkeyPatch) -> Callable[[Optional[Any]], MagicMock]: - def _mock_load_config(pickle_file: Optional[Any] = None) -> MagicMock: - # Mock the configuration data - # Load config.example.yaml - with open("config.example.yaml", "r") as file: - config_data = yaml.safe_load(file) - - config_data["general"]["machine_pickle_file"] = pickle_file - - # Mock load_config function - mock = MagicMock(return_value=config_data) - monkeypatch.setattr(rookify.__main__, "load_config", mock) - - return mock - - return _mock_load_config - - -@pytest.fixture # type: ignore -def mock_load_pickler(monkeypatch: MonkeyPatch) -> MagicMock: - # Mock states_data from pickle file - mock_states_data = {"teststuff": {"data": {"mock_key": "mock_value"}}} - - # Mock load_pickler function - mock = MagicMock(return_value=mock_states_data) - monkeypatch.setattr(rookify.__main__, "load_pickler", mock) - - return mock - - -def test_main_read_pickle( - mock_load_config: Callable[[Optional[Any]], MagicMock], - mock_load_pickler: MonkeyPatch, - monkeypatch: MonkeyPatch, - log: pytest_structlog.StructuredLogCapture, -) -> None: - # Load example config with mock.pickle as pickle file - mock_load_config("mock.pickle") - - # Mock sys.argv to simulate command-line arguments - monkeypatch.setattr(sys, "argv", ["main_script.py", "--read-pickle", "--dry-run"]) - - # Run main() - main() - - # Verify logging messages - expected_log_message = ( - 'Current state as retrieved from pickle-file: \n "all": ' - "{\n" - ' "mock_key": "mock_value"\n' - "}" - ) - assert log.has("Pickle file set: mock.pickle", level="info") - assert log.has(expected_log_message, level="info") - - -def test_main_no_pickle_file( - mock_load_config: Callable[[Optional[str]], MagicMock], - mock_load_pickler: MonkeyPatch, - monkeypatch: MonkeyPatch, - log: pytest_structlog.StructuredLogCapture, -) -> None: - # Load a configuration without pickle: This should load the default data.pickle file if it is available - mock_load_config(None) - - # Mock sys.argv to simulate command-line arguments - monkeypatch.setattr(sys, "argv", ["main_script.py", "--read-pickle", "--dry-run"]) - - # Run main() - main() - - # Assertions - assert log.has("No pickle file was set in the configuration.") - - -def test_sort_pickle_file() -> None: - # Prepare unsorted data - unsorted_states_data = getUnsortedData() - - # Expected keys order - expected_order = ["device", "fs", "mon", "node", "osd", "ssh"] - - # Run sort_pickle_file - sorted_states_data = sort_pickle_file(unsorted_states_data) - - # Assert that order is correct - sorted_states_data_keys = list(sorted_states_data.keys()) - - assert expected_order == sorted_states_data_keys - - -def getUnsortedData() -> Any: - unsorted_states_data = { - "preflighttestdata": { - "data": { - "mon": { - "dump": {"epoch": 1}, - "mons": [{"rank": 0}], - "quorum": [0, 1, 2], - }, - "fs": {"dump": {"epoch": 1}}, - "device": {"ls": ["devid", 1]}, - "osd": { - "dump": [{"epoch": 138}], - "osds": [{"osd": 0}], - "osd_xinfo": [{"osd": 0}], - }, - "node": { - "ls": { - "mon": { - "test-node-0": ["test-node-0"], - "test-node-1": ["test-node-1"], - "test-node-2": ["test-node-2"], - }, - "osd": { - "test-node-0": [2, 3], - "test-node-1": [0, 5], - "test-node-2": [1, 4], - }, - "mds": { - "test-node-0": ["test-node-0"], - "test-node-1": ["test-node-1"], - "test-node-2": ["test-node-2"], - }, - "mgr": { - "test-node-0": ["test-node-0"], - "test-node-1": ["test-node-1"], - "test-node-2": ["test-node-2"], - }, - } - }, - "ssh": { - "osd": { - "test-node-0": { - "devices": ["/dev/ceph-bla-1", "/dev/ceph-bla-2"] - }, - "test-node-1": { - "devices": ["/dev/ceph-bla-3", "/dev/ceph-bla-4"] - }, - "test-node-2": { - "devices": ["/dev/ceph-bla-5", "/dev/ceph-bla-6"] - }, - } - }, - } - } - } - - return unsorted_states_data - - -# Custom Type for Logger of CLI -TestCaseLogger = Tuple[List[str], str, str] - -# fmt: off -logger_test_cases: List[TestCaseLogger] = [ - (["--show-progress", "analyze_ceph"], "Show progress of the analyze_ceph module", "info"), - (["--dry-run", "--show-progress", "analyze_ceph"], "Show progress of the analyze_ceph module", "info"), - (["--show-progress"], "Show progress of all modules", "info"), - (["--dry-run", "--show-progress"], "Show progress of all modules", "info"), - (["--show-progress"], "AnalyzeCephHandler Progress: Not all commands have been run yet.", "info") -] -# fmt: on - - -@pytest.mark.parametrize( - "args_list, expected_log_message, expected_level", - logger_test_cases, -) # type: ignore -def test_show_progress_logs( - mock_load_config: Callable[[Optional[Any]], MagicMock], - monkeypatch: MonkeyPatch, - log: pytest_structlog.StructuredLogCapture, - args_list: List[str], - expected_log_message: str, - expected_level: str, -) -> None: - # Load example config with mock.pickle as pickle file - mock_load_config("mock.pickle") - - # Mock sys.argv to simulate command-line arguments - args_list.insert(0, "main_script.py") - monkeypatch.setattr(sys, "argv", args_list) - - # Run main() - main() - - # Assertions - assert log.has(expected_log_message, level=expected_level) From d89d12cfd526c0f71ed1f852e03e2b73563b042d Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Sat, 14 Sep 2024 11:59:25 +0200 Subject: [PATCH 21/35] Return representive state data for module `analyze_ceph` This reverts commit b33cd08c3b29ae7597632c35d7537059bb7546a0. Signed-off-by: Tobias Wolf --- src/rookify/modules/analyze_ceph/main.py | 39 +++++++++++++++--------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/rookify/modules/analyze_ceph/main.py b/src/rookify/modules/analyze_ceph/main.py index 293c9e3..25fc206 100644 --- a/src/rookify/modules/analyze_ceph/main.py +++ b/src/rookify/modules/analyze_ceph/main.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- -from typing import Any, Optional +from collections import OrderedDict +from typing import Any, Dict, Optional from ..machine import Machine from ..module import ModuleHandler @@ -39,26 +40,34 @@ def preflight(self) -> Any: self.logger.info("AnalyzeCephHandler ran successfully.") - def status(self) -> Any: - commands = ["mon dump", "osd dump", "device ls", "fs ls", "node ls"] + def get_readable_key_value_state(self) -> Dict[str, str]: state = self.machine.get_preflight_state("AnalyzeCephHandler") - # Check if all expected commands have been run - all_commands_found = True - for command in commands: - if not self._process_command(state.data, command): - all_commands_found = False - break + kv_state_data = OrderedDict() + + if "mon" not in state.data or "dump" not in state.data["mon"]: + kv_state_data["ceph mon dump"] = "Not analyzed yet" + else: + kv_state_data["ceph mon dump"] = self._get_readable_json_dump( + state.data["mon"]["dump"] + ) - # Log the status - if all_commands_found: - self.logger.info("AnalyzeCephHandler has already been run.") - self.logger.info("Current state data: %s", state.data) + if "osd" not in state.data or "dump" not in state.data["osd"]: + kv_state_data["ceph osd dump"] = "Not analyzed yet" else: - self.logger.info( - "AnalyzeCephHandler Progress: Not all commands have been run yet." + kv_state_data["ceph osd dump"] = self._get_readable_json_dump( + state.data["osd"]["dump"] ) + if "device" not in state.data or "ls" not in state.data["device"]: + kv_state_data["OSD devices"] = "Not analyzed yet" + else: + kv_state_data["OSD devices"] = self._get_readable_json_dump( + state.data["device"]["ls"] + ) + + return kv_state_data + @staticmethod def register_preflight_state( machine: Machine, state_name: str, handler: ModuleHandler, **kwargs: Any From 355e023a90d5cecc5b15c1d25c6be2cd2b6546e4 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 17 Sep 2024 20:25:15 +0200 Subject: [PATCH 22/35] Return representive state data for module `cephx_auth_config` Signed-off-by: Tobias Wolf --- src/rookify/modules/cephx_auth_config/main.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/rookify/modules/cephx_auth_config/main.py b/src/rookify/modules/cephx_auth_config/main.py index 98a50a4..fc7f4fe 100644 --- a/src/rookify/modules/cephx_auth_config/main.py +++ b/src/rookify/modules/cephx_auth_config/main.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -from typing import Any +from typing import Any, Dict from ..exception import ModuleException from ..machine import Machine from ..module import ModuleHandler @@ -30,6 +30,12 @@ def preflight(self) -> Any: def is_cephx_set(self, values: str) -> Any: return "cephx" in [value.strip() for value in values.split(",")] + def get_readable_key_value_state(self) -> Dict[str, str]: + is_verified = self.machine.get_preflight_state_data( + "CephXAuthHandler", "verified", default_value=False + ) + return {"cephx auth is verified": str(is_verified)} + @staticmethod def register_preflight_state( machine: Machine, state_name: str, handler: ModuleHandler, **kwargs: Any From f05e8f05bec391a5434a10ddb296534f11f39cdf Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 17 Sep 2024 20:26:13 +0200 Subject: [PATCH 23/35] Return representive state data for module `create_rook_resources` Signed-off-by: Tobias Wolf --- .../modules/create_rook_resources/main.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/rookify/modules/create_rook_resources/main.py b/src/rookify/modules/create_rook_resources/main.py index 48168b8..5ab018b 100644 --- a/src/rookify/modules/create_rook_resources/main.py +++ b/src/rookify/modules/create_rook_resources/main.py @@ -2,6 +2,7 @@ import kubernetes import json +from collections import OrderedDict from typing import Any, Dict from ..exception import ModuleException from ..machine import Machine @@ -135,6 +136,33 @@ def execute(self) -> None: "CreateRookResourcesHandler" ).secret = secret.to_dict() + def get_readable_key_value_state(self) -> Dict[str, str]: + kv_state_data = OrderedDict() + + configmap = self.machine.get_preflight_state_data( + "CreateRookResourcesHandler", "configmap" + ) + + if configmap is None: + kv_state_data["rook-ceph-mon-endpoints"] = "Not created yet" + else: + kv_state_data["rook-ceph-mon-endpoints"] = self._get_readable_json_dump( + configmap + ) + + secret = self.machine.get_execution_state_data( + "CreateRookResourcesHandler", "secret" + ) + + if secret is None: + kv_state_data["rook-ceph-mon-endpoints has been created"] = "False" + kv_state_data["rook-ceph-mon"] = "Not created yet" + else: + kv_state_data["rook-ceph-mon-endpoints has been created"] = "True" + kv_state_data["rook-ceph-mon"] = self._get_readable_json_dump(secret) + + return kv_state_data + @staticmethod def register_execution_state( machine: Machine, state_name: str, handler: ModuleHandler, **kwargs: Any From cc35e40af9865e2a06d4f08d063d0ee55110e5e8 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 17 Sep 2024 20:26:35 +0200 Subject: [PATCH 24/35] Return representive state data for module `create_rook_cluster` Signed-off-by: Tobias Wolf --- .../modules/create_rook_cluster/main.py | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/rookify/modules/create_rook_cluster/main.py b/src/rookify/modules/create_rook_cluster/main.py index 28ce87a..8d932cd 100644 --- a/src/rookify/modules/create_rook_cluster/main.py +++ b/src/rookify/modules/create_rook_cluster/main.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- -from typing import Any +from collections import OrderedDict +from typing import Any, Dict from ..exception import ModuleException from ..machine import Machine from ..module import ModuleHandler @@ -157,6 +158,28 @@ def _watch_cluster_phase_callback(self, event_object: Any) -> Any: return None + def get_readable_key_value_state(self) -> Dict[str, str]: + kv_state_data = OrderedDict() + + cluster_definition = self.machine.get_preflight_state_data( + "CreateRookClusterHandler", "cluster_definition" + ) + cluster_name = self._config["rook"]["cluster"]["name"] + + if cluster_definition is None: + kv_state_data[cluster_name] = "Not created yet" + else: + kv_state_data[cluster_name] = self._get_readable_json_dump( + cluster_definition + ) + + is_generated = self.machine.get_execution_state_data( + "CreateRookClusterHandler", "generated", default_value=False + ) + kv_state_data["{0} is generated".format(cluster_name)] = str(is_generated) + + return kv_state_data + @staticmethod def register_execution_state( machine: Machine, state_name: str, handler: ModuleHandler, **kwargs: Any From ac40e55d2fcec0f0feda7879990ab72112ef4952 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 17 Sep 2024 20:26:58 +0200 Subject: [PATCH 25/35] Return representive state data for module `migrate_mons` Signed-off-by: Tobias Wolf --- src/rookify/modules/migrate_mons/main.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/rookify/modules/migrate_mons/main.py b/src/rookify/modules/migrate_mons/main.py index 596ddb8..8d8df4e 100644 --- a/src/rookify/modules/migrate_mons/main.py +++ b/src/rookify/modules/migrate_mons/main.py @@ -32,6 +32,15 @@ def execute(self) -> None: for mon in state_data["mon"]["dump"]["mons"]: self._migrate_mon(mon) + def get_readable_key_value_state(self) -> Dict[str, str]: + state_data = self.machine.get_preflight_state("AnalyzeCephHandler").data + + return { + "ceph mon daemons": self._get_readable_json_dump( + state_data["mon"]["dump"]["mons"] + ) + } + def _migrate_mon(self, mon: Dict[str, Any]) -> None: migrated_mons = self.machine.get_execution_state_data( "MigrateMonsHandler", "migrated_mons", default_value=[] From ae5399c6efdb760a25d59bc6bda6859d71b6efa5 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 17 Sep 2024 20:27:16 +0200 Subject: [PATCH 26/35] Return representive state data for module `migrate_mgrs` Signed-off-by: Tobias Wolf --- src/rookify/modules/migrate_mgrs/main.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/rookify/modules/migrate_mgrs/main.py b/src/rookify/modules/migrate_mgrs/main.py index 29be0f7..381b77b 100644 --- a/src/rookify/modules/migrate_mgrs/main.py +++ b/src/rookify/modules/migrate_mgrs/main.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from time import sleep -from typing import Any +from typing import Any, Dict from ..exception import ModuleException from ..machine import Machine from ..module import ModuleHandler @@ -22,9 +22,18 @@ def preflight(self) -> None: def execute(self) -> None: state_data = self.machine.get_preflight_state("AnalyzeCephHandler").data - for node, _ in state_data["node"]["ls"]["mgr"].items(): + for node in state_data["node"]["ls"]["mgr"].keys(): self._migrate_mgr(node) + def get_readable_key_value_state(self) -> Dict[str, str]: + state_data = self.machine.get_preflight_state("AnalyzeCephHandler").data + + return { + "ceph mgr daemons": self._get_readable_json_dump( + list(state_data["node"]["ls"]["mgr"].keys()) + ) + } + def _migrate_mgr(self, mgr_host: str) -> None: migrated_mgrs = self.machine.get_execution_state_data( "MigrateMgrsHandler", "migrated_mgrs", default_value=[] From 074ac03f190f3b911353cf14944b53e30d1d09d9 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 17 Sep 2024 20:27:31 +0200 Subject: [PATCH 27/35] Return representive state data for module `migrate_osds` Signed-off-by: Tobias Wolf --- src/rookify/modules/migrate_osds/main.py | 31 +++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/rookify/modules/migrate_osds/main.py b/src/rookify/modules/migrate_osds/main.py index a14792a..acebdf5 100644 --- a/src/rookify/modules/migrate_osds/main.py +++ b/src/rookify/modules/migrate_osds/main.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +from collections import OrderedDict from time import sleep from typing import Any, Dict, List from ..exception import ModuleException @@ -84,7 +85,7 @@ def preflight(self) -> None: def execute(self) -> None: osd_host_devices = self.machine.get_preflight_state_data( - "MigrateOSDsHandler", "osd_host_devices", default_value=[] + "MigrateOSDsHandler", "osd_host_devices", default_value={} ) state_data = self.machine.get_preflight_state("AnalyzeCephHandler").data @@ -92,6 +93,34 @@ def execute(self) -> None: for host in osd_host_devices.keys(): self.migrate_osds(host, state_data["node"]["ls"]["osd"][host]) + def get_readable_key_value_state(self) -> Dict[str, str]: + state_data = self.machine.get_preflight_state("AnalyzeCephHandler").data + + osd_host_devices = self.machine.get_preflight_state_data( + "MigrateOSDsHandler", "osd_host_devices", default_value={} + ) + + kv_state_data = OrderedDict() + + for host in state_data["node"]["ls"]["osd"].keys(): + if host in osd_host_devices: + osd_device_list = [] + + for osd_id, device_path in osd_host_devices[host].items(): + osd_device_list.append( + {"OSD ID": osd_id, "Device path": device_path} + ) + + kv_state_data["ceph OSD node {0} devices".format(host)] = ( + self._get_readable_json_dump(osd_device_list) + ) + else: + kv_state_data["ceph OSD node {0} devices".format(host)] = ( + "Not analyzed yet" + ) + + return kv_state_data + def migrate_osds(self, host: str, osd_ids: List[int]) -> None: migrated_osd_ids = self.machine.get_execution_state_data( "MigrateOSDsHandler", "migrated_osd_ids", default_value=[] From a8177f1ea56e5c3d6c54990fff8358fd7ca99dd3 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 17 Sep 2024 20:35:25 +0200 Subject: [PATCH 28/35] Revert "feat: add picklefile read option with tests" This reverts commit a30c9caee4baa008e8d1688fe6892ad426131d5d. Signed-off-by: Tobias Wolf --- mock.pickle | Bin 66 -> 0 bytes src/rookify/__main__.py | 8 -------- 2 files changed, 8 deletions(-) delete mode 100644 mock.pickle diff --git a/mock.pickle b/mock.pickle deleted file mode 100644 index a4d3c945bef508c85556f9882f454b589ae4b0d5..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 66 zcmZo*nQG1e0ku Any: - with open(pickle_file_name, "ab+") as pickle_file: - pickle_file.seek(0) - states_data = Unpickler(pickle_file).load() - return states_data - - def parse_args(args: list[str]) -> argparse.Namespace: # Putting args-parser in seperate function to make this testable arg_parser = ArgumentParser("Rookify") From 6604ec2e06d047f567bcc3eb3058c56cd2d7c409 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 17 Sep 2024 20:39:37 +0200 Subject: [PATCH 29/35] Fix import error in `rookify.yaml` for Python < 3.11 Signed-off-by: Tobias Wolf --- src/rookify/yaml.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/rookify/yaml.py b/src/rookify/yaml.py index b0c178b..24dab07 100644 --- a/src/rookify/yaml.py +++ b/src/rookify/yaml.py @@ -1,15 +1,12 @@ # -*- coding: utf-8 -*- import importlib.resources -import importlib.resources.abc import yamale from pathlib import Path from typing import Any, Dict - -_config_schema_file: Path | importlib.resources.abc.Traversable = Path( - "rookify", "config.schema.yaml" -) +# Use Any instead importlib.resources.abc. Traversable for Python < 3.11 +_config_schema_file: Any = Path("rookify", "config.schema.yaml") for entry in importlib.resources.files("rookify").iterdir(): if entry.name == "config.schema.yaml": _config_schema_file = entry From 9971137bb44efff319eb2be8072baf7faff94a84 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Thu, 19 Sep 2024 21:22:00 +0200 Subject: [PATCH 30/35] Return representive state data for module `migrate_mds` Signed-off-by: Tobias Wolf --- src/rookify/modules/migrate_mds/main.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/rookify/modules/migrate_mds/main.py b/src/rookify/modules/migrate_mds/main.py index bf0200a..163bc56 100644 --- a/src/rookify/modules/migrate_mds/main.py +++ b/src/rookify/modules/migrate_mds/main.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from time import sleep -from typing import Any +from typing import Any, Dict from ..exception import ModuleException from ..machine import Machine from ..module import ModuleHandler @@ -139,6 +139,15 @@ def _enable_rook_based_mds(self, mds_host: str) -> None: "Rook based ceph-mds daemon node '{0}' available".format(mds_host) ) + def get_readable_key_value_state(self) -> Dict[str, str]: + state_data = self.machine.get_preflight_state("AnalyzeCephHandler").data + + return { + "ceph MDS daemons": self._get_readable_json_dump( + list(state_data["node"]["ls"]["mds"].keys()) + ) + } + @staticmethod def register_execution_state( machine: Machine, state_name: str, handler: ModuleHandler, **kwargs: Any From bc9fa4a3e9e79c2d729cde591bc6d417ad0367c4 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Thu, 19 Sep 2024 21:22:26 +0200 Subject: [PATCH 31/35] Return representive state data for module `migrate_mds_pools` Signed-off-by: Tobias Wolf --- src/rookify/modules/migrate_mds_pools/main.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/rookify/modules/migrate_mds_pools/main.py b/src/rookify/modules/migrate_mds_pools/main.py index 8a796dd..df67695 100644 --- a/src/rookify/modules/migrate_mds_pools/main.py +++ b/src/rookify/modules/migrate_mds_pools/main.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +from collections import OrderedDict from typing import Any, Dict from ..machine import Machine from ..module import ModuleHandler @@ -71,6 +72,24 @@ def execute(self) -> None: for pool in pools.values(): self._migrate_pool(pool) + def get_readable_key_value_state(self) -> Dict[str, str]: + migrated_mds_pools = self.machine.get_execution_state_data( + "MigrateMdsPoolsHandler", "migrated_mds_pools", default_value=[] + ) + + pools = self.machine.get_preflight_state("MigrateMdsPoolsHandler").pools + + kv_state_data = OrderedDict() + + for pool in pools: + key_name = "ceph MDS pool {0}".format(pool["name"]) + kv_state_data[key_name] = self._get_readable_json_dump(pool) + + key_name = "ceph MDS pool {0} is created".format(pool["name"]) + kv_state_data[key_name] = pool["name"] in migrated_mds_pools + + return kv_state_data + def _migrate_pool(self, pool: Dict[str, Any]) -> None: migrated_mds_pools = self.machine.get_execution_state_data( "MigrateMdsPoolsHandler", "migrated_mds_pools", default_value=[] From 4845e24eee7af7e3a0b5203b4f319a8624895756 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Thu, 19 Sep 2024 21:22:41 +0200 Subject: [PATCH 32/35] Return representive state data for module `migrate_osd_pools` Signed-off-by: Tobias Wolf --- src/rookify/modules/migrate_osd_pools/main.py | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/rookify/modules/migrate_osd_pools/main.py b/src/rookify/modules/migrate_osd_pools/main.py index eafe828..9da3253 100644 --- a/src/rookify/modules/migrate_osd_pools/main.py +++ b/src/rookify/modules/migrate_osd_pools/main.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- -from typing import Any, Dict +from collections import OrderedDict +from typing import Any, Dict, List from ..machine import Machine from ..module import ModuleHandler @@ -14,8 +15,12 @@ class MigrateOSDPoolsHandler(ModuleHandler): ] def execute(self) -> None: - state_data = self.machine.get_preflight_state("AnalyzeCephHandler").data + pools = self._get_filtered_osd_pools_list() + + for pool in pools: + self._migrate_pool(pool) + def _get_filtered_osd_pools_list(self) -> List[Dict[str, Any]]: migrated_mds_pools = self.machine.get_execution_state_data( name="MigrateMdsPoolsHandler", tag="migrated_pools", default_value=[] ) @@ -25,6 +30,8 @@ def execute(self) -> None: migrated_pools = migrated_mds_pools + migrated_rgw_pools + state_data = self.machine.get_preflight_state("AnalyzeCephHandler").data + osd_pool_configurations = self.ceph.get_osd_pool_configurations_from_osd_dump( state_data["osd"]["dump"] ) @@ -38,8 +45,25 @@ def execute(self) -> None: ): pools.append(pool) + return pools + + def get_readable_key_value_state(self) -> Dict[str, str]: + migrated_pools = self.machine.get_execution_state_data( + "MigrateOSDPoolsHandler", "migrated_pools", default_value=[] + ) + + pools = self._get_filtered_osd_pools_list() + + kv_state_data = OrderedDict() + for pool in pools: - self._migrate_pool(pool) + key_name = "ceph OSD pool {0}".format(pool["pool_name"]) + kv_state_data[key_name] = self._get_readable_json_dump(pool) + + key_name = "ceph OSD pool {0} is created".format(pool["pool_name"]) + kv_state_data[key_name] = pool["pool_name"] in migrated_pools + + return kv_state_data def _migrate_pool(self, pool: Dict[str, Any]) -> None: migrated_pools = self.machine.get_execution_state_data( From 88cbc3472c3d1cd192fd7ed0ffacb4e9e33e7a98 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Thu, 19 Sep 2024 21:22:58 +0200 Subject: [PATCH 33/35] Return representive state data for module `migrate_rgw_pools` Signed-off-by: Tobias Wolf --- src/rookify/modules/migrate_rgw_pools/main.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/rookify/modules/migrate_rgw_pools/main.py b/src/rookify/modules/migrate_rgw_pools/main.py index a44adc2..dddd3c5 100644 --- a/src/rookify/modules/migrate_rgw_pools/main.py +++ b/src/rookify/modules/migrate_rgw_pools/main.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +from collections import OrderedDict from typing import Any, Dict from ..exception import ModuleException from ..machine import Machine @@ -70,6 +71,25 @@ def execute(self) -> None: for zone_name, zone_data in zones.items(): self._migrate_zone(zone_name, zone_data) + def get_readable_key_value_state(self) -> Dict[str, str]: + migrated_pools = self.machine.get_execution_state_data( + "MigrateRgwPoolsHandler", "migrated_pools", default_value=[] + ) + + zones = self.machine.get_preflight_state("MigrateRgwPoolsHandler").zones + + kv_state_data = OrderedDict() + + for zone_data in zones.values(): + for osd_pool in zone_data["osd_pools"].values(): + key_name = "ceph RGW pool {0}".format(osd_pool["pool_name"]) + kv_state_data[key_name] = self._get_readable_json_dump(osd_pool) + + key_name = "ceph RGW pool {0} is created".format(osd_pool["pool_name"]) + kv_state_data[key_name] = osd_pool["pool_name"] in migrated_pools + + return kv_state_data + def _migrate_zone(self, zone_name: str, zone_data: Dict[str, Any]) -> None: migrated_zones = self.machine.get_execution_state_data( "MigrateRgwPoolsHandler", "migrated_zones", default_value=[] From 94a029c481640416e7d31bafdc4ddadf0c771008 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Thu, 19 Sep 2024 21:23:49 +0200 Subject: [PATCH 34/35] Return representive state data for module `migrate_rgws` Signed-off-by: Tobias Wolf --- src/rookify/modules/migrate_rgws/main.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/rookify/modules/migrate_rgws/main.py b/src/rookify/modules/migrate_rgws/main.py index bf84dbf..9dea10c 100644 --- a/src/rookify/modules/migrate_rgws/main.py +++ b/src/rookify/modules/migrate_rgws/main.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from time import sleep -from typing import Any, List +from typing import Any, Dict, List from ..exception import ModuleException from ..machine import Machine from ..module import ModuleHandler @@ -133,6 +133,11 @@ def _migrate_rgw(self, rgw_host: str) -> None: "Rook based RGW daemon for node '{0}' available".format(rgw_host) ) + def get_readable_key_value_state(self) -> Dict[str, str]: + return { + "ceph RGW hosts": self._get_readable_json_dump(self._get_rgw_daemon_hosts()) + } + @staticmethod def register_execution_state( machine: Machine, state_name: str, handler: ModuleHandler, **kwargs: Any From ad6f0dae05e17d170a958d45184797a2f1a669c2 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Sat, 21 Sep 2024 09:28:58 +0200 Subject: [PATCH 35/35] Fix serialization issue for JSON dumps in "show states" mode This implements a generic fix to use `repr()` for data to be encoded to JSON values that would normally cause errors like: `TypeError: Object of type datetime is not JSON serializable` Signed-off-by: Tobias Wolf --- src/rookify/modules/module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rookify/modules/module.py b/src/rookify/modules/module.py index 967c0bc..76c23f1 100644 --- a/src/rookify/modules/module.py +++ b/src/rookify/modules/module.py @@ -62,7 +62,7 @@ def ssh(self) -> SSH: return self._ssh def _get_readable_json_dump(self, data: Any) -> Any: - return json.dumps(data, sort_keys=True, indent="\t") + return json.dumps(data, default=repr, sort_keys=True, indent="\t") def get_readable_key_value_state(self) -> Optional[Dict[str, str]]: """