From 4ed90ac4a7baa231972e297af7f2acac3c06378c Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 11:50:06 +0100 Subject: [PATCH 01/26] Implement initialize --- .github/workflows/tests.yaml | 47 ++++++++++ requirements-fmt.txt | 6 +- requirements-lint.txt | 14 +-- requirements-unit.in | 1 + requirements-unit.txt | 133 +++++++++++++++++++++++++-- requirements.in | 4 + requirements.txt | 82 +++++++++++++++++ setup.cfg | 6 ++ src/dss/initialize.py | 104 +++++++++++++++++++++ src/dss/logger.py | 33 +++++++ src/dss/main.py | 50 +++++++++++ src/dss/manifests.yaml | 61 +++++++++++++ tests/integration/test_dss.py | 42 +++++++++ tests/unit/prepare_host_env.py | 1 - tests/unit/test_initialize.py | 160 +++++++++++++++++++++++++++++++++ tox.ini | 21 +++-- 16 files changed, 744 insertions(+), 21 deletions(-) create mode 100644 requirements.in create mode 100644 requirements.txt create mode 100644 src/dss/initialize.py create mode 100644 src/dss/logger.py create mode 100644 src/dss/main.py create mode 100644 src/dss/manifests.yaml create mode 100644 tests/integration/test_dss.py delete mode 100644 tests/unit/prepare_host_env.py create mode 100644 tests/unit/test_initialize.py diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 89b4a87..f03b9d4 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -2,6 +2,17 @@ name: Tests on: workflow_call: + microk8s-channel: + description: "Microk8s channel e.g. 1.28/stable" + required: true + default: "1.28/stable" + type: string + python-version: + description: "Version of python to install. If empty, the workflow will not install and will use machine's default" + default: "3.10" + required: false + type: string + jobs: lint: @@ -14,6 +25,7 @@ jobs: run: python3 -m pip install tox - name: Run linters run: tox -e lint + unit-test: name: Unit tests runs-on: ubuntu-22.04 @@ -24,3 +36,38 @@ jobs: run: python -m pip install tox - name: Run tests run: tox -e unit + + integration: + name: Integration tests + runs-on: ubuntu-22.04 + needs: [lint, unit-test] # Depends on lint and unit-test jobs + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + # Remove once https://github.com/canonical/bundle-kubeflow/issues/761 + # is resolved and applied to all ROCKs repositories + - name: Install python version from input + if: ${{ inputs.python-version }} + run: | + sudo add-apt-repository ppa:deadsnakes/ppa -y + sudo apt update -y + VERSION=${{ inputs.python-version }} + sudo apt install python${{ inputs.python-version }} python${{ inputs.python-version }}-distutils python${{ inputs.python-version }}-venv -y + wget https://bootstrap.pypa.io/get-pip.py + python${{ inputs.python-version }} get-pip.py + python${{ inputs.python-version }} -m pip install tox + + - name: Setup operator environment + uses: charmed-kubernetes/actions-operator@main + with: + provider: microk8s + channel: ${{ inputs.microk8s-channel }} + microk8s-addons: "hostpath-storage" + + - name: Install library + run: tox -e integration -- -vv -s + + - name: Setup tmate session + uses: mxschmitt/action-tmate@v3 + timeout-minutes: 15 # This will allow to open a sesion for max 15 minutes diff --git a/requirements-fmt.txt b/requirements-fmt.txt index 5eaa044..7bd6d98 100644 --- a/requirements-fmt.txt +++ b/requirements-fmt.txt @@ -2,14 +2,14 @@ # This file is autogenerated by pip-compile with Python 3.10 # by the following command: # -# pip-compile ./requirements-fmt.in +# pip-compile requirements-fmt.in # black==23.12.1 - # via -r ./requirements-fmt.in + # via -r requirements-fmt.in click==8.1.7 # via black isort==5.13.2 - # via -r ./requirements-fmt.in + # via -r requirements-fmt.in mypy-extensions==1.0.0 # via black packaging==23.2 diff --git a/requirements-lint.txt b/requirements-lint.txt index 39a5fbe..2b5ecb5 100644 --- a/requirements-lint.txt +++ b/requirements-lint.txt @@ -2,30 +2,30 @@ # This file is autogenerated by pip-compile with Python 3.10 # by the following command: # -# pip-compile ./requirements-lint.in +# pip-compile requirements-lint.in # codespell==2.2.6 - # via -r ./requirements-lint.in + # via -r requirements-lint.in flake8==6.1.0 # via - # -r ./requirements-lint.in + # -r requirements-lint.in # flake8-builtins # pep8-naming # pyproject-flake8 flake8-builtins==2.2.0 - # via -r ./requirements-lint.in + # via -r requirements-lint.in flake8-copyright==0.2.4 - # via -r ./requirements-lint.in + # via -r requirements-lint.in mccabe==0.7.0 # via flake8 pep8-naming==0.13.3 - # via -r ./requirements-lint.in + # via -r requirements-lint.in pycodestyle==2.11.1 # via flake8 pyflakes==3.1.0 # via flake8 pyproject-flake8==6.1.0 - # via -r ./requirements-lint.in + # via -r requirements-lint.in tomli==2.0.1 # via pyproject-flake8 diff --git a/requirements-unit.in b/requirements-unit.in index 89b68f8..1d2112e 100644 --- a/requirements-unit.in +++ b/requirements-unit.in @@ -1,3 +1,4 @@ pytest pytest-mock coverage[toml] +-r requirements.txt diff --git a/requirements-unit.txt b/requirements-unit.txt index c08bec2..b403cda 100644 --- a/requirements-unit.txt +++ b/requirements-unit.txt @@ -2,25 +2,148 @@ # This file is autogenerated by pip-compile with Python 3.10 # by the following command: # -# pip-compile ./requirements-unit.in +# pip-compile requirements-unit.in # +anyio==4.2.0 + # via + # -r requirements.txt + # httpx +attrs==23.2.0 + # via + # -r requirements.txt + # jsonschema +certifi==2024.2.2 + # via + # -r requirements.txt + # httpcore + # httpx + # requests +charmed-kubeflow-chisme==0.2.1 + # via -r requirements.txt +charset-normalizer==3.3.2 + # via + # -r requirements.txt + # requests +click==8.1.7 + # via -r requirements.txt coverage[toml]==7.4.0 - # via -r ./requirements-unit.in + # via -r requirements-unit.in +deepdiff==6.2.1 + # via + # -r requirements.txt + # charmed-kubeflow-chisme exceptiongroup==1.2.0 - # via pytest + # via + # -r requirements.txt + # anyio + # pytest +h11==0.14.0 + # via + # -r requirements.txt + # httpcore +httpcore==1.0.2 + # via + # -r requirements.txt + # httpx +httpx==0.26.0 + # via + # -r requirements.txt + # lightkube +idna==3.6 + # via + # -r requirements.txt + # anyio + # httpx + # requests iniconfig==2.0.0 # via pytest +jinja2==3.1.3 + # via + # -r requirements.txt + # charmed-kubeflow-chisme +jsonschema==4.17.3 + # via + # -r requirements.txt + # serialized-data-interface +lightkube==0.15.1 + # via + # -r requirements.txt + # charmed-kubeflow-chisme +lightkube-models==1.29.0.7 + # via + # -r requirements.txt + # lightkube +markupsafe==2.1.5 + # via + # -r requirements.txt + # jinja2 +ops==2.10.0 + # via + # -r requirements.txt + # charmed-kubeflow-chisme + # serialized-data-interface +ordered-set==4.1.0 + # via + # -r requirements.txt + # deepdiff packaging==23.2 # via pytest pluggy==1.3.0 # via pytest +pyrsistent==0.20.0 + # via + # -r requirements.txt + # jsonschema pytest==7.4.4 # via - # -r ./requirements-unit.in + # -r requirements-unit.in # pytest-mock pytest-mock==3.12.0 - # via -r ./requirements-unit.in + # via -r requirements-unit.in +pyyaml==6.0.1 + # via + # -r requirements.txt + # lightkube + # ops + # serialized-data-interface +requests==2.31.0 + # via + # -r requirements.txt + # serialized-data-interface +ruamel-yaml==0.18.6 + # via + # -r requirements.txt + # charmed-kubeflow-chisme +ruamel-yaml-clib==0.2.8 + # via + # -r requirements.txt + # ruamel-yaml +serialized-data-interface==0.7.0 + # via + # -r requirements.txt + # charmed-kubeflow-chisme +sniffio==1.3.0 + # via + # -r requirements.txt + # anyio + # httpx +tenacity==8.2.3 + # via + # -r requirements.txt + # charmed-kubeflow-chisme tomli==2.0.1 # via # coverage # pytest +typing-extensions==4.9.0 + # via + # -r requirements.txt + # anyio +urllib3==2.2.0 + # via + # -r requirements.txt + # requests +websocket-client==1.7.0 + # via + # -r requirements.txt + # ops diff --git a/requirements.in b/requirements.in new file mode 100644 index 0000000..346a360 --- /dev/null +++ b/requirements.in @@ -0,0 +1,4 @@ +Click +charmed_kubeflow_chisme +lightkube +pyyaml diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 0000000..c5da55d --- /dev/null +++ b/requirements.txt @@ -0,0 +1,82 @@ +# +# This file is autogenerated by pip-compile with Python 3.10 +# by the following command: +# +# pip-compile requirements.in +# +anyio==4.2.0 + # via httpx +attrs==23.2.0 + # via jsonschema +certifi==2024.2.2 + # via + # httpcore + # httpx + # requests +charmed-kubeflow-chisme==0.2.1 + # via -r requirements.in +charset-normalizer==3.3.2 + # via requests +click==8.1.7 + # via -r requirements.in +deepdiff==6.2.1 + # via charmed-kubeflow-chisme +exceptiongroup==1.2.0 + # via anyio +h11==0.14.0 + # via httpcore +httpcore==1.0.2 + # via httpx +httpx==0.26.0 + # via lightkube +idna==3.6 + # via + # anyio + # httpx + # requests +jinja2==3.1.3 + # via charmed-kubeflow-chisme +jsonschema==4.17.3 + # via serialized-data-interface +lightkube==0.15.1 + # via + # -r requirements.in + # charmed-kubeflow-chisme +lightkube-models==1.29.0.7 + # via lightkube +markupsafe==2.1.5 + # via jinja2 +ops==2.10.0 + # via + # charmed-kubeflow-chisme + # serialized-data-interface +ordered-set==4.1.0 + # via deepdiff +pyrsistent==0.20.0 + # via jsonschema +pyyaml==6.0.1 + # via + # -r requirements.in + # lightkube + # ops + # serialized-data-interface +requests==2.31.0 + # via serialized-data-interface +ruamel-yaml==0.18.6 + # via charmed-kubeflow-chisme +ruamel-yaml-clib==0.2.8 + # via ruamel-yaml +serialized-data-interface==0.7.0 + # via charmed-kubeflow-chisme +sniffio==1.3.0 + # via + # anyio + # httpx +tenacity==8.2.3 + # via charmed-kubeflow-chisme +typing-extensions==4.9.0 + # via anyio +urllib3==2.2.0 + # via requests +websocket-client==1.7.0 + # via ops diff --git a/setup.cfg b/setup.cfg index ed409f5..282dcd4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -7,7 +7,13 @@ package_dir = = src packages = find: install_requires = + charmed-kubeflow-chisme Click + lightkube +include_package_data = True + +[options.package_data] +dss = manifests.yaml [options.packages.find] where = src diff --git a/src/dss/initialize.py b/src/dss/initialize.py new file mode 100644 index 0000000..ddd3843 --- /dev/null +++ b/src/dss/initialize.py @@ -0,0 +1,104 @@ +import os +import time + +import yaml +from charmed_kubeflow_chisme.kubernetes import KubernetesResourceHandler +from lightkube import Client, KubeConfig +from lightkube.generic_resource import load_in_cluster_generic_resources +from lightkube.resources.apps_v1 import Deployment +from lightkube.resources.core_v1 import Namespace, PersistentVolumeClaim, Service + +from dss.logger import setup_logger + +# Set up logger +logger = setup_logger("logs/dss.log") + + +def wait_for_deployment_ready( + client: Client, + namespace: str, + deployment_name: str, + timeout_seconds: int = 60, + interval_seconds: int = 10, +) -> None: + """ + Waits for a Kubernetes deployment to be ready. + + Args: + client (Client): The Kubernetes client. + namespace (str): The namespace of the deployment. + deployment_name (str): The name of the deployment. + timeout_seconds (int): Timeout in seconds. Defaults to 600. + interval_seconds (int): Interval between checks in seconds. Defaults to 10. + + Returns: + None + """ + logger.info( + f"Waiting for deployment {deployment_name} in namespace {namespace} to be ready..." + ) + start_time = time.time() + while True: + deployment: Deployment = client.get(Deployment, namespace=namespace, name=deployment_name) + if deployment.status.availableReplicas == deployment.spec.replicas: + logger.info(f"Deployment {deployment_name} in namespace {namespace} is ready") + break + elif time.time() - start_time >= timeout_seconds: + raise TimeoutError( + f"Timeout waiting for deployment {deployment_name} in namespace {namespace} to be ready" # noqa E501 + ) + else: + time.sleep(interval_seconds) + logger.info( + f"Waiting for deployment {deployment_name} in namespace {namespace} to be ready..." + ) + + +def initialize() -> None: + """ + Initializes the Kubernetes cluster by applying manifests from a YAML file. + + Returns: + None + """ + # Path to the manifests YAML file + manifests_file = os.path.join(os.path.dirname(__file__), "manifests.yaml") + + # Read kubeconfig content from environment variable + kubeconfig_content = os.environ.get("DSS_KUBECONFIG", "") + if not kubeconfig_content: + raise ValueError("Kubeconfig content not found in environment variable DSS_KUBECONFIG") + + # Initialize Kubernetes client with kubeconfig content + kubeconfig = KubeConfig.from_dict(yaml.safe_load(kubeconfig_content)) + client = Client(kubeconfig) + + # Initialize KubernetesResourceHandler + k8s_resource_handler = KubernetesResourceHandler( + field_manager="dss", + labels={"app": "dss", "dss-component": "mlflow"}, + template_files=[manifests_file], + context={}, + resource_types={Deployment, Service, PersistentVolumeClaim, Namespace}, + ) + + # Load Kubernetes generic resources + load_in_cluster_generic_resources(client) + + try: + # Apply resources using KubernetesResourceHandler + k8s_resource_handler.apply() + + # Wait for mlflow deployment to be ready + wait_for_deployment_ready(client, namespace="dss", deployment_name="mlflow-deployment") + + logger.info( + "DSS initialized. To create your first notebook run the command:\n\ndss create-notebook" # noqa E501 + ) + + except TimeoutError: + logger.error( + "Timeout waiting for deployment 'mlflow-deployment' in namespace 'dss' to be ready. " # noqa E501 + "Deleting resources..." + ) + k8s_resource_handler.delete() diff --git a/src/dss/logger.py b/src/dss/logger.py new file mode 100644 index 0000000..7b2f755 --- /dev/null +++ b/src/dss/logger.py @@ -0,0 +1,33 @@ +import logging +import os +from logging.handlers import RotatingFileHandler + + +def setup_logger(log_file_path, log_level=logging.DEBUG): + logger = logging.getLogger(__name__) + + # Check if the logger already has handlers to avoid duplication + if not logger.handlers: + logger.setLevel(log_level) + + # Create log formatter + formatter = logging.Formatter( + "%(asctime)s [%(levelname)s] [%(module)s] [%(funcName)s]: %(message)s", + datefmt="%Y-%m-%d %H:%M:%S", + ) + + if not os.path.exists(os.path.dirname(log_file_path)): + os.makedirs(os.path.dirname(log_file_path)) + + file_handler = RotatingFileHandler(log_file_path, maxBytes=5 * 1024 * 1024, backupCount=5) + file_handler.setLevel(log_level) + file_handler.setFormatter(formatter) + + console_handler = logging.StreamHandler() + console_handler.setLevel(log_level) + console_handler.setFormatter(formatter) + + logger.addHandler(file_handler) + logger.addHandler(console_handler) + + return logger diff --git a/src/dss/main.py b/src/dss/main.py new file mode 100644 index 0000000..1dd2a2e --- /dev/null +++ b/src/dss/main.py @@ -0,0 +1,50 @@ +import os + +import click + +from dss.initialize import initialize +from dss.logger import setup_logger + +# Set up logger +logger = setup_logger("logs/dss.log") + +# Name for the environment variable storing kubeconfig +KUBECONFIG_ENV_VAR = "DSS_KUBECONFIG" + + +@click.group() +def main(): + """Command line interface for managing the DSS application.""" + + +@main.command(name="initialize") +@click.option( + "--kubeconfig", + help="Specify the kubeconfig content. Defaults to the value of file at path on the KUBECONFIG environment variable.", # noqa E501 +) +def initialize_command(kubeconfig: str) -> None: + """ + Initialize the Kubernetes cluster and deploy charms. + """ + logger.info("Executing initialize command") + + # If kubeconfig is provided, set it as an environment variable + if kubeconfig: + os.environ[KUBECONFIG_ENV_VAR] = kubeconfig + + # If kubeconfig is not provided, check environment variable + elif KUBECONFIG_ENV_VAR not in os.environ: + kubeconfig_path = os.environ.get("KUBECONFIG", "") + if not kubeconfig_path: + logger.error( + "Missing required argument: --kubeconfig or environment variable KUBECONFIG" + ) + exit(1) + with open(kubeconfig_path, "r") as file: + os.environ[KUBECONFIG_ENV_VAR] = file.read() + + initialize() + + +if __name__ == "__main__": + main() diff --git a/src/dss/manifests.yaml b/src/dss/manifests.yaml new file mode 100644 index 0000000..59cb52e --- /dev/null +++ b/src/dss/manifests.yaml @@ -0,0 +1,61 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: dss + +--- +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: mlflow-pvc + namespace: dss +spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: mlflow-deployment + namespace: dss +spec: + replicas: 1 + selector: + matchLabels: + app: mlflow + template: + metadata: + labels: + app: mlflow + spec: + containers: + - name: mlflow-container + image: ubuntu/mlflow:2.1.1_1.0-22.04 + args: ["mlflow", "server", "--host", "0.0.0.0", "--port", "5000"] + ports: + - containerPort: 5000 + volumeMounts: + - name: mlflow-pvc + mountPath: /mlruns + volumes: + - name: mlflow-pvc + persistentVolumeClaim: + claimName: mlflow-pvc + +--- +apiVersion: v1 +kind: Service +metadata: + name: mlflow-svc + namespace: dss +spec: + selector: + app: mlflow + ports: + - protocol: TCP + port: 5000 diff --git a/tests/integration/test_dss.py b/tests/integration/test_dss.py new file mode 100644 index 0000000..584f694 --- /dev/null +++ b/tests/integration/test_dss.py @@ -0,0 +1,42 @@ +import subprocess + + +def test_initialize_creates_dss_namespace() -> None: + """ + Integration test to verify if the initialize command creates the 'dss' namespace and + the 'mlflow-deployment' deployment is active in the 'dss' namespace. + """ + # Run the initialize command with the provided kubeconfig + microk8s_config = subprocess.run(["microk8s", "config"], capture_output=True, text=True) + kubeconfig_content: str = microk8s_config.stdout.strip() + + result = subprocess.run( + ["dss", "initialize", "--kubeconfig", kubeconfig_content], + capture_output=True, + text=True, + ) + + # Check if the command executed successfully + print(result.stdout) + assert result.returncode == 0 + + # Check if the dss namespace exists using kubectl + kubectl_result = subprocess.run( + ["kubectl", "get", "namespace", "dss"], capture_output=True, text=True + ) + + # Check if the namespace exists + assert "dss" in kubectl_result.stdout + + # Check if the mlflow-deployment deployment is active in the dss namespace + kubectl_result = subprocess.run( + ["kubectl", "get", "deployment", "mlflow-deployment", "-n", "dss"], + capture_output=True, + text=True, + ) + + # Check if the deployment exists and is active + assert "mlflow-deployment" in kubectl_result.stdout + assert ( + "1/1" in kubectl_result.stdout + ) # Assuming it should have 1 replica and all are available diff --git a/tests/unit/prepare_host_env.py b/tests/unit/prepare_host_env.py deleted file mode 100644 index 0d4b760..0000000 --- a/tests/unit/prepare_host_env.py +++ /dev/null @@ -1 +0,0 @@ -# Test file \ No newline at end of file diff --git a/tests/unit/test_initialize.py b/tests/unit/test_initialize.py new file mode 100644 index 0000000..128b072 --- /dev/null +++ b/tests/unit/test_initialize.py @@ -0,0 +1,160 @@ +from unittest.mock import MagicMock, patch + +import pytest +from lightkube.resources.apps_v1 import Deployment + +from dss.initialize import initialize, wait_for_deployment_ready + + +@pytest.fixture +def mock_environ_get() -> MagicMock: + """ + Fixture to mock the os.environ.get function. + """ + with patch("dss.initialize.os.environ.get") as mock_env_get: + yield mock_env_get + + +@pytest.fixture +def mock_kubeconfig_from_dict() -> MagicMock: + """ + Fixture to mock the KubeConfig.from_dict function. + """ + with patch("dss.initialize.KubeConfig.from_dict") as mock_kubeconfig: + yield mock_kubeconfig + + +@pytest.fixture +def mock_client() -> MagicMock: + """ + Fixture to mock the Client class. + """ + with patch("dss.initialize.Client") as mock_client: + yield mock_client + + +@pytest.fixture +def mock_resource_handler() -> MagicMock: + """ + Fixture to mock the KubernetesResourceHandler class. + """ + with patch("dss.initialize.KubernetesResourceHandler") as mock_handler: + yield mock_handler + + +@pytest.fixture +def mock_load_generic_resources() -> MagicMock: + """ + Fixture to mock the load_in_cluster_generic_resources function. + """ + with patch("dss.initialize.load_in_cluster_generic_resources") as mock_load: + yield mock_load + + +@pytest.fixture +def mock_logger() -> MagicMock: + """ + Fixture to mock the logger object. + """ + with patch("dss.initialize.logger") as mock_logger: + yield mock_logger + + +def test_initialize_success( + mock_environ_get: MagicMock, + mock_kubeconfig_from_dict: MagicMock, + mock_client: MagicMock, + mock_resource_handler: MagicMock, + mock_load_generic_resources: MagicMock, + mock_logger: MagicMock, +) -> None: + """ + Test case to verify successful initialization. + """ + # Mock the return value of os.environ.get + mock_environ_get.return_value = "dummy_kubeconfig_content" + + # Mock the behavior of KubeConfig.from_dict + mock_kubeconfig = MagicMock() + mock_kubeconfig_from_dict.return_value = mock_kubeconfig + + # Mock the behavior of Client + mock_client_instance = MagicMock() + mock_client.return_value = mock_client_instance + + # Mock the behavior of KubernetesResourceHandler + mock_resource_handler_instance = MagicMock() + mock_resource_handler.return_value = mock_resource_handler_instance + + # Mock wait_for_deployment_ready + with patch("dss.initialize.wait_for_deployment_ready") as mock_wait_for_deployment_ready: + # Call the function to test + initialize() + + # Assertions + mock_environ_get.assert_called_once_with("DSS_KUBECONFIG", "") + mock_kubeconfig_from_dict.assert_called_once_with("dummy_kubeconfig_content") + mock_client.assert_called_once_with(mock_kubeconfig) + mock_load_generic_resources.assert_called_once_with(mock_client_instance) + mock_resource_handler_instance.apply.assert_called_once() + mock_wait_for_deployment_ready.assert_called_once_with( + mock_client_instance, namespace="dss", deployment_name="mlflow-deployment" + ) + mock_logger.info.assert_called_with( + "DSS initialized. To create your first notebook run the command:\n\ndss create-notebook" + ) + + +def test_initialize_missing_kubeconfig_env_var( + mock_environ_get: MagicMock, + mock_kubeconfig_from_dict: MagicMock, + mock_client: MagicMock, + mock_load_generic_resources: MagicMock, +) -> None: + """ + Test case to verify missing kubeconfig environment variable. + """ + # Mock the absence of the DSS_KUBECONFIG environment variable + mock_environ_get.return_value = "" + + # Call the function to test + with pytest.raises(ValueError) as exc_info: + initialize() + + # Assertions + assert ( + str(exc_info.value) + == "Kubeconfig content not found in environment variable DSS_KUBECONFIG" + ) + mock_environ_get.assert_called_once_with("DSS_KUBECONFIG", "") + mock_kubeconfig_from_dict.assert_not_called() + mock_client.assert_not_called() + mock_load_generic_resources.assert_not_called() + + +def test_wait_for_deployment_ready_timeout(mock_client: MagicMock, mock_logger: MagicMock) -> None: + """ + Test case to verify timeout while waiting for deployment to be ready. + """ + # Mock the behavior of the client.get method to return a deployment with available replicas = 0 + mock_client_instance = MagicMock() + mock_client_instance.get.return_value = MagicMock( + spec=Deployment, status=MagicMock(availableReplicas=0), spec_replicas=1 + ) + + # Call the function to test + with pytest.raises(TimeoutError) as exc_info: + wait_for_deployment_ready( + mock_client_instance, + namespace="test-namespace", + deployment_name="test-deployment", + timeout_seconds=5, + interval_seconds=1, + ) + + # Assertions + assert ( + str(exc_info.value) + == "Timeout waiting for deployment test-deployment in namespace test-namespace to be ready" + ) + assert mock_client_instance.get.call_count == 6 # 5 attempts, 1 final attempt after timeout diff --git a/tox.ini b/tox.ini index 3a7bb37..2b297be 100644 --- a/tox.ini +++ b/tox.ini @@ -7,7 +7,7 @@ max-line-length = 100 [tox] skipsdist=True skip_missing_interpreters = True -envlist = fmt, lint, unit, update-requirements +envlist = fmt, integration, lint, unit, update-requirements [vars] src_path = {toxinidir}/src/ @@ -28,9 +28,12 @@ allowlist_externals = find pip-compile xargs -commands = -; uses 'bash -c' because piping didn't work in regular tox commands - bash -c 'find . -type f -name "requirements*.in" | xargs --replace=\{\} pip-compile --resolver=backtracking \{\}' +commands = + ; we must preserve the order of compilation, since each *.in file depends on some *.txt file. + ; For example, requirements-unit.in depends on requirements.txt and we must compile first + ; requirements.txt to ensure that requirements-unit.txt get the same dependency as the requirements.txt + bash -c 'for pattern in "requirements.in" "requirements-fmt.in" "requirements*.in"; do find . -type f -name "$pattern" -exec bash -c "cd \$(dirname "{}") && pip-compile --resolver=backtracking \$(basename "{}")" \;; done' + deps = pip-tools description = Update requirements files by executing pip-compile on all requirements*.in files, including those in subdirs. @@ -64,8 +67,16 @@ description = Check code against coding style standards [testenv:unit] commands = coverage run --source={[vars]src_path} \ - -m pytest {[vars]tst_path}/unit -v --tb native -s {posargs} + -m pytest --ignore={[vars]tst_path}integration -v --tb native -s {posargs} coverage report deps = -r requirements-unit.txt description = Run unit tests + +[testenv:integration] +commands = + pip install . + pytest {[vars]tst_path}/integration -v --tb native -s {posargs} +deps = + -r requirements-unit.txt +description = Run integration tests From 16062373d8f63906ba1be97060a9941aff62b799 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 13:57:02 +0100 Subject: [PATCH 02/26] Refactor --- .github/workflows/tests.yaml | 2 +- src/dss/logger.py | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index f03b9d4..dde56a7 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -4,7 +4,7 @@ on: workflow_call: microk8s-channel: description: "Microk8s channel e.g. 1.28/stable" - required: true + required: false default: "1.28/stable" type: string python-version: diff --git a/src/dss/logger.py b/src/dss/logger.py index 7b2f755..e019a46 100644 --- a/src/dss/logger.py +++ b/src/dss/logger.py @@ -1,9 +1,20 @@ import logging import os from logging.handlers import RotatingFileHandler +from typing import Optional -def setup_logger(log_file_path, log_level=logging.DEBUG): +def setup_logger(log_file_path: str, log_level: int = logging.DEBUG) -> logging.Logger: + """ + Set up a logger with a file handler and console handler. + + Args: + log_file_path (str): Path to the log file. + log_level (int, optional): Logging level. Defaults to logging.DEBUG. + + Returns: + logging.Logger: Configured logger object. + """ logger = logging.getLogger(__name__) # Check if the logger already has handlers to avoid duplication @@ -16,17 +27,21 @@ def setup_logger(log_file_path, log_level=logging.DEBUG): datefmt="%Y-%m-%d %H:%M:%S", ) + # Create the log directory if it doesn't exist if not os.path.exists(os.path.dirname(log_file_path)): os.makedirs(os.path.dirname(log_file_path)) + # Create file handler file_handler = RotatingFileHandler(log_file_path, maxBytes=5 * 1024 * 1024, backupCount=5) file_handler.setLevel(log_level) file_handler.setFormatter(formatter) + # Create console handler console_handler = logging.StreamHandler() console_handler.setLevel(log_level) console_handler.setFormatter(formatter) + # Add handlers to the logger logger.addHandler(file_handler) logger.addHandler(console_handler) From 3445d198ef76cadea527adf913cf52142768002e Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 13:58:36 +0100 Subject: [PATCH 03/26] Refactor --- .github/workflows/tests.yaml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index dde56a7..9caee7f 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -2,17 +2,17 @@ name: Tests on: workflow_call: - microk8s-channel: - description: "Microk8s channel e.g. 1.28/stable" - required: false - default: "1.28/stable" - type: string - python-version: - description: "Version of python to install. If empty, the workflow will not install and will use machine's default" - default: "3.10" - required: false - type: string - + microk8s-channel: + description: "Microk8s channel e.g. 1.28/stable" + required: false + default: "1.28/stable" + type: string + python-version: + description: "Version of python to install. If empty, the workflow will not install and will use machine's default" + default: "3.10" + required: false + type: string + jobs: lint: From fe6f6bb82c72a1520ccde8fc3b7776dbbfcc8103 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 14:05:02 +0100 Subject: [PATCH 04/26] Refactor --- .github/workflows/tests.yaml | 89 +++++++++++++++--------------------- 1 file changed, 38 insertions(+), 51 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 9caee7f..170e84a 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -1,73 +1,60 @@ name: Tests - on: workflow_call: - microk8s-channel: - description: "Microk8s channel e.g. 1.28/stable" - required: false - default: "1.28/stable" - type: string - python-version: - description: "Version of python to install. If empty, the workflow will not install and will use machine's default" - default: "3.10" - required: false - type: string - jobs: lint: name: Lint runs-on: ubuntu-22.04 steps: - - name: Checkout - uses: actions/checkout@v2 - - name: Install dependencies - run: python3 -m pip install tox - - name: Run linters - run: tox -e lint + - name: Checkout + uses: actions/checkout@v2 + - name: Install dependencies + run: python3 -m pip install tox + - name: Run linters + run: tox -e lint unit-test: name: Unit tests runs-on: ubuntu-22.04 steps: - - name: Checkout - uses: actions/checkout@v2 - - name: Install dependencies - run: python -m pip install tox - - name: Run tests - run: tox -e unit + - name: Checkout + uses: actions/checkout@v2 + - name: Install dependencies + run: python -m pip install tox + - name: Run tests + run: tox -e unit integration: name: Integration tests runs-on: ubuntu-22.04 needs: [lint, unit-test] # Depends on lint and unit-test jobs steps: - - name: Checkout repository - uses: actions/checkout@v4 + - name: Checkout repository + uses: actions/checkout@v4 - # Remove once https://github.com/canonical/bundle-kubeflow/issues/761 - # is resolved and applied to all ROCKs repositories - - name: Install python version from input - if: ${{ inputs.python-version }} - run: | - sudo add-apt-repository ppa:deadsnakes/ppa -y - sudo apt update -y - VERSION=${{ inputs.python-version }} - sudo apt install python${{ inputs.python-version }} python${{ inputs.python-version }}-distutils python${{ inputs.python-version }}-venv -y - wget https://bootstrap.pypa.io/get-pip.py - python${{ inputs.python-version }} get-pip.py - python${{ inputs.python-version }} -m pip install tox - - - name: Setup operator environment - uses: charmed-kubernetes/actions-operator@main - with: - provider: microk8s - channel: ${{ inputs.microk8s-channel }} - microk8s-addons: "hostpath-storage" + # Remove once https://github.com/canonical/bundle-kubeflow/issues/761 + # is resolved and applied to all ROCKs repositories + - name: Install python version from input + run: | + sudo add-apt-repository ppa:deadsnakes/ppa -y + sudo apt update -y + VERSION=3.10 + sudo apt install python3.10 python3.10-distutils python$3.10-venv -y + wget https://bootstrap.pypa.io/get-pip.py + python3.10 get-pip.py + python3.10 -m pip install tox + + - name: Setup operator environment + uses: charmed-kubernetes/actions-operator@main + with: + provider: microk8s + channel: 1.28/stable + microk8s-addons: "hostpath-storage" - - name: Install library - run: tox -e integration -- -vv -s - - - name: Setup tmate session - uses: mxschmitt/action-tmate@v3 - timeout-minutes: 15 # This will allow to open a sesion for max 15 minutes + - name: Install library + run: tox -e integration -- -vv -s + + - name: Setup tmate session + uses: mxschmitt/action-tmate@v3 + timeout-minutes: 15 # This will allow to open a sesion for max 15 minutes From ccd549c789a0e1403dd4b262a1814999348a6a86 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 14:08:28 +0100 Subject: [PATCH 05/26] Refactor --- src/dss/logger.py | 1 - tests/integration/test_dss.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/dss/logger.py b/src/dss/logger.py index e019a46..dc5382a 100644 --- a/src/dss/logger.py +++ b/src/dss/logger.py @@ -1,7 +1,6 @@ import logging import os from logging.handlers import RotatingFileHandler -from typing import Optional def setup_logger(log_file_path: str, log_level: int = logging.DEBUG) -> logging.Logger: diff --git a/tests/integration/test_dss.py b/tests/integration/test_dss.py index 584f694..5f2e84f 100644 --- a/tests/integration/test_dss.py +++ b/tests/integration/test_dss.py @@ -1,7 +1,7 @@ import subprocess -def test_initialize_creates_dss_namespace() -> None: +def test_initialize_creates_dss() -> None: """ Integration test to verify if the initialize command creates the 'dss' namespace and the 'mlflow-deployment' deployment is active in the 'dss' namespace. From 45ad8c2cbb620da3af5b44e59e246ed32c2449b5 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 14:12:19 +0100 Subject: [PATCH 06/26] Refactor --- .github/workflows/tests.yaml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 170e84a..c689dda 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -7,10 +7,10 @@ jobs: name: Lint runs-on: ubuntu-22.04 steps: - - name: Checkout - uses: actions/checkout@v2 + - name: Checkout repository + uses: actions/checkout@v4 - name: Install dependencies - run: python3 -m pip install tox + run: sudo apt-get install python3-pip tox - name: Run linters run: tox -e lint @@ -18,17 +18,16 @@ jobs: name: Unit tests runs-on: ubuntu-22.04 steps: - - name: Checkout - uses: actions/checkout@v2 + - name: Checkout repository + uses: actions/checkout@v4 - name: Install dependencies - run: python -m pip install tox + run: sudo apt-get install python3-pip tox - name: Run tests run: tox -e unit integration: name: Integration tests runs-on: ubuntu-22.04 - needs: [lint, unit-test] # Depends on lint and unit-test jobs steps: - name: Checkout repository uses: actions/checkout@v4 From 585b8a462ffff804ee1b25d8a43fd92466290c27 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 14:14:11 +0100 Subject: [PATCH 07/26] Refactor --- .github/workflows/tests.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index c689dda..4cc9283 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -10,7 +10,7 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 - name: Install dependencies - run: sudo apt-get install python3-pip tox + run: python3 -m pip install tox - name: Run linters run: tox -e lint @@ -21,7 +21,7 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 - name: Install dependencies - run: sudo apt-get install python3-pip tox + run: python -m pip install tox - name: Run tests run: tox -e unit @@ -39,7 +39,7 @@ jobs: sudo add-apt-repository ppa:deadsnakes/ppa -y sudo apt update -y VERSION=3.10 - sudo apt install python3.10 python3.10-distutils python$3.10-venv -y + sudo apt install python3.10 python3.10-distutils python3.10-venv -y wget https://bootstrap.pypa.io/get-pip.py python3.10 get-pip.py python3.10 -m pip install tox From 686098070f142d0d8b85b6c2601e989febdfb6f7 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 14:24:15 +0100 Subject: [PATCH 08/26] Refactor --- .github/workflows/tests.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 4cc9283..14d1bf3 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -48,7 +48,7 @@ jobs: uses: charmed-kubernetes/actions-operator@main with: provider: microk8s - channel: 1.28/stable + channel: 1.28-strict/stable microk8s-addons: "hostpath-storage" - name: Install library @@ -56,4 +56,5 @@ jobs: - name: Setup tmate session uses: mxschmitt/action-tmate@v3 + if: always() timeout-minutes: 15 # This will allow to open a sesion for max 15 minutes From c67c279fedadfc5837f11a672ceeb04178a25074 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 14:39:29 +0100 Subject: [PATCH 09/26] Refactor --- .github/workflows/tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 14d1bf3..c6920d7 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -52,7 +52,7 @@ jobs: microk8s-addons: "hostpath-storage" - name: Install library - run: tox -e integration -- -vv -s + run: sg snap_microk8s -c "tox -vve integration" - name: Setup tmate session uses: mxschmitt/action-tmate@v3 From 003d48818010bdff2e9d3fb678ff398c5e5f1816 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 14:56:56 +0100 Subject: [PATCH 10/26] Refactor --- .github/workflows/tests.yaml | 10 ++++------ tox.ini | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index c6920d7..a4d1112 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -45,14 +45,12 @@ jobs: python3.10 -m pip install tox - name: Setup operator environment - uses: charmed-kubernetes/actions-operator@main - with: - provider: microk8s - channel: 1.28-strict/stable - microk8s-addons: "hostpath-storage" + run: | + sudo snap install microk8s --classic --channel=1.28/stable + sudo microk8s enable hostpath-storage - name: Install library - run: sg snap_microk8s -c "tox -vve integration" + run: sg microk8s -c "tox -vve integration" - name: Setup tmate session uses: mxschmitt/action-tmate@v3 diff --git a/tox.ini b/tox.ini index 2b297be..6880b37 100644 --- a/tox.ini +++ b/tox.ini @@ -58,7 +58,7 @@ commands = --skip {toxinidir}/./icon.svg --skip *.json.tmpl # pflake8 wrapper supports config from pyproject.toml pflake8 {[vars]all_path} - isort --check-only --diff {[vars]all_path} + # isort --check-only --diff {[vars]all_path} black --check --diff {[vars]all_path} deps = -r requirements-lint.txt From 7f5755acc3b168c50939cc597e10b254080a5e46 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 15:05:22 +0100 Subject: [PATCH 11/26] Refactor --- .github/workflows/tests.yaml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index a4d1112..a241f37 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -48,9 +48,15 @@ jobs: run: | sudo snap install microk8s --classic --channel=1.28/stable sudo microk8s enable hostpath-storage + mkdir -p $USER ~/.kube + sudo usermod -a -G microk8s $USER + sudo chown -f -R $USER ~/.kube + microk8s stop + microk8s start + microk8s config > ~/.kube/config - name: Install library - run: sg microk8s -c "tox -vve integration" + run: su ubuntu -c "tox -vve integration" - name: Setup tmate session uses: mxschmitt/action-tmate@v3 From ee7363e4d3c6eaf12937af3421cf6ecf03fcc9da Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 15:10:48 +0100 Subject: [PATCH 12/26] Refactor --- .github/workflows/tests.yaml | 6 +++--- tox.ini | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index a241f37..0524e79 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -43,6 +43,7 @@ jobs: wget https://bootstrap.pypa.io/get-pip.py python3.10 get-pip.py python3.10 -m pip install tox + rm get-pip.py - name: Setup operator environment run: | @@ -51,12 +52,11 @@ jobs: mkdir -p $USER ~/.kube sudo usermod -a -G microk8s $USER sudo chown -f -R $USER ~/.kube - microk8s stop - microk8s start + newgrp microk8s microk8s config > ~/.kube/config - name: Install library - run: su ubuntu -c "tox -vve integration" + run: tox -vve integration - name: Setup tmate session uses: mxschmitt/action-tmate@v3 diff --git a/tox.ini b/tox.ini index 6880b37..2b297be 100644 --- a/tox.ini +++ b/tox.ini @@ -58,7 +58,7 @@ commands = --skip {toxinidir}/./icon.svg --skip *.json.tmpl # pflake8 wrapper supports config from pyproject.toml pflake8 {[vars]all_path} - # isort --check-only --diff {[vars]all_path} + isort --check-only --diff {[vars]all_path} black --check --diff {[vars]all_path} deps = -r requirements-lint.txt From 8f5595a5945f21e3527efde2581cec1299479105 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 15:15:44 +0100 Subject: [PATCH 13/26] Refactor --- .github/workflows/tests.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 0524e79..d8ddb0d 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -52,8 +52,6 @@ jobs: mkdir -p $USER ~/.kube sudo usermod -a -G microk8s $USER sudo chown -f -R $USER ~/.kube - newgrp microk8s - microk8s config > ~/.kube/config - name: Install library run: tox -vve integration From 31391deb60d3fa84155470dbc736fa10b37aad01 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 15:18:22 +0100 Subject: [PATCH 14/26] Refactor --- tests/integration/test_dss.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_dss.py b/tests/integration/test_dss.py index 5f2e84f..1d7a949 100644 --- a/tests/integration/test_dss.py +++ b/tests/integration/test_dss.py @@ -7,7 +7,7 @@ def test_initialize_creates_dss() -> None: the 'mlflow-deployment' deployment is active in the 'dss' namespace. """ # Run the initialize command with the provided kubeconfig - microk8s_config = subprocess.run(["microk8s", "config"], capture_output=True, text=True) + microk8s_config = subprocess.run(["sudo", "microk8s", "config"], capture_output=True, text=True) kubeconfig_content: str = microk8s_config.stdout.strip() result = subprocess.run( From 576820801fa354c742ac5992c3a3b774dd8f04f3 Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 15:27:50 +0100 Subject: [PATCH 15/26] Refactor --- .github/workflows/tests.yaml | 13 +++++-------- requirements-lint.in | 1 + requirements-lint.txt | 33 ++++++++++++++++++++++++++++++++- tests/integration/test_dss.py | 2 +- 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index d8ddb0d..546641a 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -45,16 +45,13 @@ jobs: python3.10 -m pip install tox rm get-pip.py - - name: Setup operator environment - run: | - sudo snap install microk8s --classic --channel=1.28/stable - sudo microk8s enable hostpath-storage - mkdir -p $USER ~/.kube - sudo usermod -a -G microk8s $USER - sudo chown -f -R $USER ~/.kube + - uses: balchua/microk8s-actions@v0.3.2 + with: + channel: '1.28/stable' + addons: '["hostpath-storage"]' - name: Install library - run: tox -vve integration + run: sg microk8s -c "tox -vve integration" - name: Setup tmate session uses: mxschmitt/action-tmate@v3 diff --git a/requirements-lint.in b/requirements-lint.in index c453874..e3f71ef 100644 --- a/requirements-lint.in +++ b/requirements-lint.in @@ -4,3 +4,4 @@ flake8-builtins flake8-copyright pep8-naming pyproject-flake8 +-r requirements-fmt.txt \ No newline at end of file diff --git a/requirements-lint.txt b/requirements-lint.txt index 2b5ecb5..9b84d64 100644 --- a/requirements-lint.txt +++ b/requirements-lint.txt @@ -4,6 +4,12 @@ # # pip-compile requirements-lint.in # +black==23.12.1 + # via -r requirements-fmt.txt +click==8.1.7 + # via + # -r requirements-fmt.txt + # black codespell==2.2.6 # via -r requirements-lint.in flake8==6.1.0 @@ -16,10 +22,28 @@ flake8-builtins==2.2.0 # via -r requirements-lint.in flake8-copyright==0.2.4 # via -r requirements-lint.in +isort==5.13.2 + # via -r requirements-fmt.txt mccabe==0.7.0 # via flake8 +mypy-extensions==1.0.0 + # via + # -r requirements-fmt.txt + # black +packaging==23.2 + # via + # -r requirements-fmt.txt + # black +pathspec==0.12.1 + # via + # -r requirements-fmt.txt + # black pep8-naming==0.13.3 # via -r requirements-lint.in +platformdirs==4.1.0 + # via + # -r requirements-fmt.txt + # black pycodestyle==2.11.1 # via flake8 pyflakes==3.1.0 @@ -27,7 +51,14 @@ pyflakes==3.1.0 pyproject-flake8==6.1.0 # via -r requirements-lint.in tomli==2.0.1 - # via pyproject-flake8 + # via + # -r requirements-fmt.txt + # black + # pyproject-flake8 +typing-extensions==4.9.0 + # via + # -r requirements-fmt.txt + # black # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/tests/integration/test_dss.py b/tests/integration/test_dss.py index 1d7a949..5f2e84f 100644 --- a/tests/integration/test_dss.py +++ b/tests/integration/test_dss.py @@ -7,7 +7,7 @@ def test_initialize_creates_dss() -> None: the 'mlflow-deployment' deployment is active in the 'dss' namespace. """ # Run the initialize command with the provided kubeconfig - microk8s_config = subprocess.run(["sudo", "microk8s", "config"], capture_output=True, text=True) + microk8s_config = subprocess.run(["microk8s", "config"], capture_output=True, text=True) kubeconfig_content: str = microk8s_config.stdout.strip() result = subprocess.run( From 85eefd1be3f297276d9c4e006d676220e6710c5d Mon Sep 17 00:00:00 2001 From: misohu Date: Thu, 15 Feb 2024 15:35:08 +0100 Subject: [PATCH 16/26] Refactor --- .github/workflows/tests.yaml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 546641a..79a3f43 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -52,8 +52,4 @@ jobs: - name: Install library run: sg microk8s -c "tox -vve integration" - - - name: Setup tmate session - uses: mxschmitt/action-tmate@v3 - if: always() - timeout-minutes: 15 # This will allow to open a sesion for max 15 minutes + From 46948a5f5295512fe87aedb10fe1f6999534cdd5 Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Tue, 20 Feb 2024 14:18:17 -0500 Subject: [PATCH 17/26] nit changes in tox.ini, requirements-lint.in --- requirements-lint.in | 2 +- tox.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements-lint.in b/requirements-lint.in index e3f71ef..87f005e 100644 --- a/requirements-lint.in +++ b/requirements-lint.in @@ -4,4 +4,4 @@ flake8-builtins flake8-copyright pep8-naming pyproject-flake8 --r requirements-fmt.txt \ No newline at end of file +-r requirements-fmt.txt diff --git a/tox.ini b/tox.ini index 2b297be..5fc571b 100644 --- a/tox.ini +++ b/tox.ini @@ -67,7 +67,7 @@ description = Check code against coding style standards [testenv:unit] commands = coverage run --source={[vars]src_path} \ - -m pytest --ignore={[vars]tst_path}integration -v --tb native -s {posargs} + -m pytest {[vars]tst_path}/unit -v --tb native -s {posargs} coverage report deps = -r requirements-unit.txt From ce866221ede7efcdb1be217f3c30b64f82ba797d Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Thu, 22 Feb 2024 16:01:14 -0500 Subject: [PATCH 18/26] remove load_in_cluster_generic_resources --- src/dss/initialize.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/dss/initialize.py b/src/dss/initialize.py index ddd3843..96269f1 100644 --- a/src/dss/initialize.py +++ b/src/dss/initialize.py @@ -4,7 +4,6 @@ import yaml from charmed_kubeflow_chisme.kubernetes import KubernetesResourceHandler from lightkube import Client, KubeConfig -from lightkube.generic_resource import load_in_cluster_generic_resources from lightkube.resources.apps_v1 import Deployment from lightkube.resources.core_v1 import Namespace, PersistentVolumeClaim, Service @@ -82,9 +81,6 @@ def initialize() -> None: resource_types={Deployment, Service, PersistentVolumeClaim, Namespace}, ) - # Load Kubernetes generic resources - load_in_cluster_generic_resources(client) - try: # Apply resources using KubernetesResourceHandler k8s_resource_handler.apply() From 8a47e17c07833a038f85fa6004a8d483208a4fca Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Thu, 22 Feb 2024 16:01:41 -0500 Subject: [PATCH 19/26] update labels for the dss-managed k8s resources --- src/dss/initialize.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dss/initialize.py b/src/dss/initialize.py index 96269f1..f15ec47 100644 --- a/src/dss/initialize.py +++ b/src/dss/initialize.py @@ -13,6 +13,9 @@ logger = setup_logger("logs/dss.log") +DSS_CLI_MANAGER_LABELS = {"app.kubernetes.io/managed-by": "dss-cli"} + + def wait_for_deployment_ready( client: Client, namespace: str, @@ -75,7 +78,7 @@ def initialize() -> None: # Initialize KubernetesResourceHandler k8s_resource_handler = KubernetesResourceHandler( field_manager="dss", - labels={"app": "dss", "dss-component": "mlflow"}, + labels=DSS_CLI_MANAGER_LABELS, template_files=[manifests_file], context={}, resource_types={Deployment, Service, PersistentVolumeClaim, Namespace}, From f05b4e4c974a3e842559dc3b575500347df2eb43 Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Thu, 22 Feb 2024 16:02:54 -0500 Subject: [PATCH 20/26] remove object-type suffixes from k8s object names --- src/dss/initialize.py | 2 +- src/dss/manifests.yaml | 14 +++++++------- tests/integration/test_dss.py | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/dss/initialize.py b/src/dss/initialize.py index f15ec47..4c4346b 100644 --- a/src/dss/initialize.py +++ b/src/dss/initialize.py @@ -89,7 +89,7 @@ def initialize() -> None: k8s_resource_handler.apply() # Wait for mlflow deployment to be ready - wait_for_deployment_ready(client, namespace="dss", deployment_name="mlflow-deployment") + wait_for_deployment_ready(client, namespace="dss", deployment_name="mlflow") logger.info( "DSS initialized. To create your first notebook run the command:\n\ndss create-notebook" # noqa E501 diff --git a/src/dss/manifests.yaml b/src/dss/manifests.yaml index 59cb52e..66e1e69 100644 --- a/src/dss/manifests.yaml +++ b/src/dss/manifests.yaml @@ -8,7 +8,7 @@ metadata: apiVersion: v1 kind: PersistentVolumeClaim metadata: - name: mlflow-pvc + name: mlflow namespace: dss spec: accessModes: @@ -21,7 +21,7 @@ spec: apiVersion: apps/v1 kind: Deployment metadata: - name: mlflow-deployment + name: mlflow namespace: dss spec: replicas: 1 @@ -34,24 +34,24 @@ spec: app: mlflow spec: containers: - - name: mlflow-container + - name: mlflow image: ubuntu/mlflow:2.1.1_1.0-22.04 args: ["mlflow", "server", "--host", "0.0.0.0", "--port", "5000"] ports: - containerPort: 5000 volumeMounts: - - name: mlflow-pvc + - name: mlflow mountPath: /mlruns volumes: - - name: mlflow-pvc + - name: mlflow persistentVolumeClaim: - claimName: mlflow-pvc + claimName: mlflow --- apiVersion: v1 kind: Service metadata: - name: mlflow-svc + name: mlflow namespace: dss spec: selector: diff --git a/tests/integration/test_dss.py b/tests/integration/test_dss.py index 5f2e84f..a0c2f0a 100644 --- a/tests/integration/test_dss.py +++ b/tests/integration/test_dss.py @@ -4,7 +4,7 @@ def test_initialize_creates_dss() -> None: """ Integration test to verify if the initialize command creates the 'dss' namespace and - the 'mlflow-deployment' deployment is active in the 'dss' namespace. + the 'mlflow' deployment is active in the 'dss' namespace. """ # Run the initialize command with the provided kubeconfig microk8s_config = subprocess.run(["microk8s", "config"], capture_output=True, text=True) @@ -30,13 +30,13 @@ def test_initialize_creates_dss() -> None: # Check if the mlflow-deployment deployment is active in the dss namespace kubectl_result = subprocess.run( - ["kubectl", "get", "deployment", "mlflow-deployment", "-n", "dss"], + ["kubectl", "get", "deployment", "mlflow", "-n", "dss"], capture_output=True, text=True, ) # Check if the deployment exists and is active - assert "mlflow-deployment" in kubectl_result.stdout + assert "mlflow" in kubectl_result.stdout assert ( "1/1" in kubectl_result.stdout ) # Assuming it should have 1 replica and all are available From 0b85f414754057f12b7612a11acdfc1147f2c7f4 Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Thu, 22 Feb 2024 16:04:01 -0500 Subject: [PATCH 21/26] add automated environment cleanup for integration tests --- tests/integration/test_dss.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_dss.py b/tests/integration/test_dss.py index a0c2f0a..782e0c8 100644 --- a/tests/integration/test_dss.py +++ b/tests/integration/test_dss.py @@ -1,7 +1,14 @@ import subprocess +import pytest +from charmed_kubeflow_chisme.kubernetes import KubernetesResourceHandler +from lightkube.resources.apps_v1 import Deployment +from lightkube.resources.core_v1 import Namespace, PersistentVolumeClaim, Service -def test_initialize_creates_dss() -> None: +from dss.initialize import DSS_CLI_MANAGER_LABELS + + +def test_initialize_creates_dss(cleanup_after_initialize) -> None: """ Integration test to verify if the initialize command creates the 'dss' namespace and the 'mlflow' deployment is active in the 'dss' namespace. @@ -40,3 +47,27 @@ def test_initialize_creates_dss() -> None: assert ( "1/1" in kubectl_result.stdout ) # Assuming it should have 1 replica and all are available + + +@pytest.fixture() +def cleanup_after_initialize(): + """Cleans up resources that might have been deployed by dss initialize. + + Note that this is a white-box implemention - it depends on knowing what could be deployed and explicitly removing + those objects, rather than truly restoring the cluster to a previous state. This could be leaky, depending on how + `dss initialize` is changed in future. + """ + yield + + k8s_resource_handler = KubernetesResourceHandler( + field_manager="dss", + labels=DSS_CLI_MANAGER_LABELS, + template_files=[], + context={}, + resource_types={Deployment, Service, PersistentVolumeClaim, Namespace}, + ) + + # Attempt to clean up anything that initialize might create + # Note that .delete() does not wait on the objects to be successfully deleted, so repeating the tests + # quickly can still cause an issue + k8s_resource_handler.delete() From 3e511bafe738698f2725d6aca4ca00f8235822fd Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Thu, 22 Feb 2024 16:04:51 -0500 Subject: [PATCH 22/26] update k8s object labels to use app.kubernetes.io/name and /part-of --- src/dss/manifests.yaml | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/dss/manifests.yaml b/src/dss/manifests.yaml index 66e1e69..892af61 100644 --- a/src/dss/manifests.yaml +++ b/src/dss/manifests.yaml @@ -3,6 +3,8 @@ apiVersion: v1 kind: Namespace metadata: name: dss + labels: + app.kubernetes.io/part-of: dss --- apiVersion: v1 @@ -10,6 +12,9 @@ kind: PersistentVolumeClaim metadata: name: mlflow namespace: dss + labels: + app.kubernetes.io/name: mlflow + app.kubernetes.io/part-of: dss spec: accessModes: - ReadWriteOnce @@ -23,15 +28,20 @@ kind: Deployment metadata: name: mlflow namespace: dss + labels: + app.kubernetes.io/name: mlflow + app.kubernetes.io/part-of: dss spec: replicas: 1 selector: matchLabels: - app: mlflow + app.kubernetes.io/name: mlflow + app.kubernetes.io/part-of: dss template: metadata: labels: - app: mlflow + app.kubernetes.io/name: mlflow + app.kubernetes.io/part-of: dss spec: containers: - name: mlflow @@ -53,9 +63,13 @@ kind: Service metadata: name: mlflow namespace: dss + labels: + app.kubernetes.io/name: mlflow + app.kubernetes.io/part-of: dss spec: selector: - app: mlflow + app.kubernetes.io/name: mlflow + app.kubernetes.io/part-of: dss ports: - protocol: TCP port: 5000 From 4ff4535520f796347f9ddcf5625bffd075900fa0 Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Thu, 22 Feb 2024 16:25:42 -0500 Subject: [PATCH 23/26] fix linting/formatting --- tests/integration/test_dss.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_dss.py b/tests/integration/test_dss.py index 782e0c8..5a41f3b 100644 --- a/tests/integration/test_dss.py +++ b/tests/integration/test_dss.py @@ -53,9 +53,9 @@ def test_initialize_creates_dss(cleanup_after_initialize) -> None: def cleanup_after_initialize(): """Cleans up resources that might have been deployed by dss initialize. - Note that this is a white-box implemention - it depends on knowing what could be deployed and explicitly removing - those objects, rather than truly restoring the cluster to a previous state. This could be leaky, depending on how - `dss initialize` is changed in future. + Note that this is a white-box implementation - it depends on knowing what could be deployed and + explicitly removing those objects, rather than truly restoring the cluster to a previous state. + This could be leaky, depending on how `dss initialize` is changed in future. """ yield @@ -68,6 +68,6 @@ def cleanup_after_initialize(): ) # Attempt to clean up anything that initialize might create - # Note that .delete() does not wait on the objects to be successfully deleted, so repeating the tests - # quickly can still cause an issue + # Note that .delete() does not wait on the objects to be successfully deleted, so repeating + # the tests quickly can still cause an issue k8s_resource_handler.delete() From b9bbb2e84e97337674737f40d1aff38bd8a2a27f Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Thu, 22 Feb 2024 16:28:08 -0500 Subject: [PATCH 24/26] fix unit tests to match previous code refactors --- tests/unit/test_initialize.py | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/tests/unit/test_initialize.py b/tests/unit/test_initialize.py index 128b072..5743486 100644 --- a/tests/unit/test_initialize.py +++ b/tests/unit/test_initialize.py @@ -42,15 +42,6 @@ def mock_resource_handler() -> MagicMock: yield mock_handler -@pytest.fixture -def mock_load_generic_resources() -> MagicMock: - """ - Fixture to mock the load_in_cluster_generic_resources function. - """ - with patch("dss.initialize.load_in_cluster_generic_resources") as mock_load: - yield mock_load - - @pytest.fixture def mock_logger() -> MagicMock: """ @@ -65,7 +56,6 @@ def test_initialize_success( mock_kubeconfig_from_dict: MagicMock, mock_client: MagicMock, mock_resource_handler: MagicMock, - mock_load_generic_resources: MagicMock, mock_logger: MagicMock, ) -> None: """ @@ -95,10 +85,9 @@ def test_initialize_success( mock_environ_get.assert_called_once_with("DSS_KUBECONFIG", "") mock_kubeconfig_from_dict.assert_called_once_with("dummy_kubeconfig_content") mock_client.assert_called_once_with(mock_kubeconfig) - mock_load_generic_resources.assert_called_once_with(mock_client_instance) mock_resource_handler_instance.apply.assert_called_once() mock_wait_for_deployment_ready.assert_called_once_with( - mock_client_instance, namespace="dss", deployment_name="mlflow-deployment" + mock_client_instance, namespace="dss", deployment_name="mlflow" ) mock_logger.info.assert_called_with( "DSS initialized. To create your first notebook run the command:\n\ndss create-notebook" @@ -109,7 +98,6 @@ def test_initialize_missing_kubeconfig_env_var( mock_environ_get: MagicMock, mock_kubeconfig_from_dict: MagicMock, mock_client: MagicMock, - mock_load_generic_resources: MagicMock, ) -> None: """ Test case to verify missing kubeconfig environment variable. @@ -129,7 +117,6 @@ def test_initialize_missing_kubeconfig_env_var( mock_environ_get.assert_called_once_with("DSS_KUBECONFIG", "") mock_kubeconfig_from_dict.assert_not_called() mock_client.assert_not_called() - mock_load_generic_resources.assert_not_called() def test_wait_for_deployment_ready_timeout(mock_client: MagicMock, mock_logger: MagicMock) -> None: From fe06db3fda8948eb732b349f60ca269c2ce5efb0 Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Mon, 26 Feb 2024 09:14:14 -0500 Subject: [PATCH 25/26] refactor main.py and initialize.py's kubeconfig input Previously, kubeconfig was passed as the content of a kubeconfig file and stored in an environment variable. This refactor changes inputs to accept a file path kubeconfig to a kubernetes config file, with defaults for when that file is omitted. --- src/dss/initialize.py | 20 ++++----- src/dss/main.py | 30 ++++---------- src/dss/utils.py | 36 ++++++++++++++++ tests/integration/test_dss.py | 11 ++--- tests/unit/test_initialize.py | 37 +---------------- tests/unit/test_utils.py | 78 +++++++++++++++++++++++++++++++++++ 6 files changed, 132 insertions(+), 80 deletions(-) create mode 100644 src/dss/utils.py create mode 100644 tests/unit/test_utils.py diff --git a/src/dss/initialize.py b/src/dss/initialize.py index 4c4346b..4fb626e 100644 --- a/src/dss/initialize.py +++ b/src/dss/initialize.py @@ -1,9 +1,8 @@ import os import time -import yaml from charmed_kubeflow_chisme.kubernetes import KubernetesResourceHandler -from lightkube import Client, KubeConfig +from lightkube import Client from lightkube.resources.apps_v1 import Deployment from lightkube.resources.core_v1 import Namespace, PersistentVolumeClaim, Service @@ -56,25 +55,19 @@ def wait_for_deployment_ready( ) -def initialize() -> None: +def initialize(lightkube_client: Client) -> None: """ Initializes the Kubernetes cluster by applying manifests from a YAML file. + Args: + lightkube_client (Client): The Kubernetes client. + Returns: None """ # Path to the manifests YAML file manifests_file = os.path.join(os.path.dirname(__file__), "manifests.yaml") - # Read kubeconfig content from environment variable - kubeconfig_content = os.environ.get("DSS_KUBECONFIG", "") - if not kubeconfig_content: - raise ValueError("Kubeconfig content not found in environment variable DSS_KUBECONFIG") - - # Initialize Kubernetes client with kubeconfig content - kubeconfig = KubeConfig.from_dict(yaml.safe_load(kubeconfig_content)) - client = Client(kubeconfig) - # Initialize KubernetesResourceHandler k8s_resource_handler = KubernetesResourceHandler( field_manager="dss", @@ -82,6 +75,7 @@ def initialize() -> None: template_files=[manifests_file], context={}, resource_types={Deployment, Service, PersistentVolumeClaim, Namespace}, + lightkube_client=lightkube_client, ) try: @@ -89,7 +83,7 @@ def initialize() -> None: k8s_resource_handler.apply() # Wait for mlflow deployment to be ready - wait_for_deployment_ready(client, namespace="dss", deployment_name="mlflow") + wait_for_deployment_ready(lightkube_client, namespace="dss", deployment_name="mlflow") logger.info( "DSS initialized. To create your first notebook run the command:\n\ndss create-notebook" # noqa E501 diff --git a/src/dss/main.py b/src/dss/main.py index 1dd2a2e..73d5766 100644 --- a/src/dss/main.py +++ b/src/dss/main.py @@ -1,16 +1,12 @@ -import os - import click from dss.initialize import initialize from dss.logger import setup_logger +from dss.utils import KUBECONFIG_DEFAULT, get_default_kubeconfig, get_lightkube_client # Set up logger logger = setup_logger("logs/dss.log") -# Name for the environment variable storing kubeconfig -KUBECONFIG_ENV_VAR = "DSS_KUBECONFIG" - @click.group() def main(): @@ -20,30 +16,18 @@ def main(): @main.command(name="initialize") @click.option( "--kubeconfig", - help="Specify the kubeconfig content. Defaults to the value of file at path on the KUBECONFIG environment variable.", # noqa E501 + help=f"Path to a Kubernetes config file. Defaults to the value of the KUBECONFIG environment variable, else to '{KUBECONFIG_DEFAULT}'.", # noqa E501 ) def initialize_command(kubeconfig: str) -> None: """ - Initialize the Kubernetes cluster and deploy charms. + Initialize DSS on the given Kubernetes cluster. """ logger.info("Executing initialize command") - # If kubeconfig is provided, set it as an environment variable - if kubeconfig: - os.environ[KUBECONFIG_ENV_VAR] = kubeconfig - - # If kubeconfig is not provided, check environment variable - elif KUBECONFIG_ENV_VAR not in os.environ: - kubeconfig_path = os.environ.get("KUBECONFIG", "") - if not kubeconfig_path: - logger.error( - "Missing required argument: --kubeconfig or environment variable KUBECONFIG" - ) - exit(1) - with open(kubeconfig_path, "r") as file: - os.environ[KUBECONFIG_ENV_VAR] = file.read() - - initialize() + kubeconfig = get_default_kubeconfig(kubeconfig) + lightkube_client = get_lightkube_client(kubeconfig) + + initialize(lightkube_client=lightkube_client) if __name__ == "__main__": diff --git a/src/dss/utils.py b/src/dss/utils.py new file mode 100644 index 0000000..1fea09d --- /dev/null +++ b/src/dss/utils.py @@ -0,0 +1,36 @@ +import os +from typing import Optional + +from lightkube import Client, KubeConfig + +# Name for the environment variable storing kubeconfig +KUBECONFIG_ENV_VAR = "DSS_KUBECONFIG" +KUBECONFIG_DEFAULT = "./kubeconfig" + + +def get_default_kubeconfig(kubeconfig: Optional[str] = None) -> str: + """ + Get the kubeconfig file path, from input, environment variable, or default. + + Args: + kubeconfig (str): Path to a kubeconfig file. Defaults to None. + + Returns: + str: the value of kubeconfig if it is not None, else: + the value of the DSS_KUBECONFIG environment variable if it is set, else: + the default value of "./kubeconfig" + + """ + if kubeconfig: + return kubeconfig + elif os.environ.get(KUBECONFIG_ENV_VAR, ""): + return os.environ.get(KUBECONFIG_ENV_VAR, "") + else: + return KUBECONFIG_DEFAULT + + +def get_lightkube_client(kubeconfig: Optional[str] = None): + # Compute the default kubeconfig, if not specified + kubeconfig = KubeConfig.from_file(kubeconfig) + lightkube_client = Client(config=kubeconfig) + return lightkube_client diff --git a/tests/integration/test_dss.py b/tests/integration/test_dss.py index 5a41f3b..168d76d 100644 --- a/tests/integration/test_dss.py +++ b/tests/integration/test_dss.py @@ -13,12 +13,11 @@ def test_initialize_creates_dss(cleanup_after_initialize) -> None: Integration test to verify if the initialize command creates the 'dss' namespace and the 'mlflow' deployment is active in the 'dss' namespace. """ - # Run the initialize command with the provided kubeconfig - microk8s_config = subprocess.run(["microk8s", "config"], capture_output=True, text=True) - kubeconfig_content: str = microk8s_config.stdout.strip() + # TODO: is there a better way to initialize this? Maybe an optional argument to the test? + kubeconfig = "~/.kube/config" result = subprocess.run( - ["dss", "initialize", "--kubeconfig", kubeconfig_content], + ["dss", "initialize", "--kubeconfig", kubeconfig], capture_output=True, text=True, ) @@ -31,8 +30,6 @@ def test_initialize_creates_dss(cleanup_after_initialize) -> None: kubectl_result = subprocess.run( ["kubectl", "get", "namespace", "dss"], capture_output=True, text=True ) - - # Check if the namespace exists assert "dss" in kubectl_result.stdout # Check if the mlflow-deployment deployment is active in the dss namespace @@ -41,8 +38,6 @@ def test_initialize_creates_dss(cleanup_after_initialize) -> None: capture_output=True, text=True, ) - - # Check if the deployment exists and is active assert "mlflow" in kubectl_result.stdout assert ( "1/1" in kubectl_result.stdout diff --git a/tests/unit/test_initialize.py b/tests/unit/test_initialize.py index 5743486..3e69914 100644 --- a/tests/unit/test_initialize.py +++ b/tests/unit/test_initialize.py @@ -61,13 +61,6 @@ def test_initialize_success( """ Test case to verify successful initialization. """ - # Mock the return value of os.environ.get - mock_environ_get.return_value = "dummy_kubeconfig_content" - - # Mock the behavior of KubeConfig.from_dict - mock_kubeconfig = MagicMock() - mock_kubeconfig_from_dict.return_value = mock_kubeconfig - # Mock the behavior of Client mock_client_instance = MagicMock() mock_client.return_value = mock_client_instance @@ -79,12 +72,9 @@ def test_initialize_success( # Mock wait_for_deployment_ready with patch("dss.initialize.wait_for_deployment_ready") as mock_wait_for_deployment_ready: # Call the function to test - initialize() + initialize(lightkube_client=mock_client_instance) # Assertions - mock_environ_get.assert_called_once_with("DSS_KUBECONFIG", "") - mock_kubeconfig_from_dict.assert_called_once_with("dummy_kubeconfig_content") - mock_client.assert_called_once_with(mock_kubeconfig) mock_resource_handler_instance.apply.assert_called_once() mock_wait_for_deployment_ready.assert_called_once_with( mock_client_instance, namespace="dss", deployment_name="mlflow" @@ -94,31 +84,6 @@ def test_initialize_success( ) -def test_initialize_missing_kubeconfig_env_var( - mock_environ_get: MagicMock, - mock_kubeconfig_from_dict: MagicMock, - mock_client: MagicMock, -) -> None: - """ - Test case to verify missing kubeconfig environment variable. - """ - # Mock the absence of the DSS_KUBECONFIG environment variable - mock_environ_get.return_value = "" - - # Call the function to test - with pytest.raises(ValueError) as exc_info: - initialize() - - # Assertions - assert ( - str(exc_info.value) - == "Kubeconfig content not found in environment variable DSS_KUBECONFIG" - ) - mock_environ_get.assert_called_once_with("DSS_KUBECONFIG", "") - mock_kubeconfig_from_dict.assert_not_called() - mock_client.assert_not_called() - - def test_wait_for_deployment_ready_timeout(mock_client: MagicMock, mock_logger: MagicMock) -> None: """ Test case to verify timeout while waiting for deployment to be ready. diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py new file mode 100644 index 0000000..057e842 --- /dev/null +++ b/tests/unit/test_utils.py @@ -0,0 +1,78 @@ +from unittest.mock import MagicMock, patch + +import pytest + +from dss.utils import KUBECONFIG_DEFAULT, get_default_kubeconfig, get_lightkube_client + + +@pytest.fixture +def mock_environ_get() -> MagicMock: + """ + Fixture to mock the os.environ.get function. + """ + with patch("dss.utils.os.environ.get") as mock_env_get: + yield mock_env_get + + +@pytest.fixture +def mock_kubeconfig() -> MagicMock: + """ + Fixture to mock the KubeConfig.from_dict function. + """ + with patch("dss.utils.KubeConfig") as mock_kubeconfig: + yield mock_kubeconfig + + +@pytest.fixture +def mock_client() -> MagicMock: + """ + Fixture to mock the Client class. + """ + with patch("dss.utils.Client") as mock_client: + yield mock_client + + +@pytest.mark.parametrize( + "kubeconfig, kubeconfig_env_var, expected", + [ + ("some_file", "", "some_file"), + (None, "some_file", "some_file"), + (None, "", KUBECONFIG_DEFAULT), + ], +) +def test_get_default_kubeconfig_successful( + kubeconfig: str, + kubeconfig_env_var: str, + expected: str, + mock_environ_get: MagicMock, +) -> None: + """ + Test case to verify missing kubeconfig environment variable. + + Args: + kubeconfig: path to a kubeconfig file, passed to get_lightkube_client by arg + kubeconfig_env_var: environment variable for kubeconfig + expected: expected returned value for kubeconfig + """ + mock_environ_get.return_value = kubeconfig_env_var + + returned = get_default_kubeconfig(kubeconfig) + assert returned == expected + + +def test_get_lightkube_client_successful( + mock_kubeconfig: MagicMock, + mock_client: MagicMock, +) -> None: + """ + Tests that we successfully try to create a lightkube client, given a kubeconfig. + """ + kubeconfig = "some_file" + mock_kubeconfig_instance = "kubeconfig-returned" + mock_kubeconfig.from_file.return_value = mock_kubeconfig_instance + + returned_client = get_lightkube_client(kubeconfig) + + mock_kubeconfig.from_file.assert_called_with(kubeconfig) + mock_client.assert_called_with(config=mock_kubeconfig_instance) + assert returned_client is not None From f100a7ce8599e090e3584827a16f7ad857078643 Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Mon, 26 Feb 2024 09:29:19 -0500 Subject: [PATCH 26/26] refactor to move wait_for_deployment to a common utils.py --- src/dss/initialize.py | 42 +-------------------------- src/dss/utils.py | 47 +++++++++++++++++++++++++++++++ tests/unit/test_initialize.py | 41 +-------------------------- tests/unit/test_utils.py | 53 +++++++++++++++++++++++++++++++---- 4 files changed, 97 insertions(+), 86 deletions(-) diff --git a/src/dss/initialize.py b/src/dss/initialize.py index 4fb626e..4bb2b3c 100644 --- a/src/dss/initialize.py +++ b/src/dss/initialize.py @@ -1,5 +1,4 @@ import os -import time from charmed_kubeflow_chisme.kubernetes import KubernetesResourceHandler from lightkube import Client @@ -7,6 +6,7 @@ from lightkube.resources.core_v1 import Namespace, PersistentVolumeClaim, Service from dss.logger import setup_logger +from dss.utils import wait_for_deployment_ready # Set up logger logger = setup_logger("logs/dss.log") @@ -15,46 +15,6 @@ DSS_CLI_MANAGER_LABELS = {"app.kubernetes.io/managed-by": "dss-cli"} -def wait_for_deployment_ready( - client: Client, - namespace: str, - deployment_name: str, - timeout_seconds: int = 60, - interval_seconds: int = 10, -) -> None: - """ - Waits for a Kubernetes deployment to be ready. - - Args: - client (Client): The Kubernetes client. - namespace (str): The namespace of the deployment. - deployment_name (str): The name of the deployment. - timeout_seconds (int): Timeout in seconds. Defaults to 600. - interval_seconds (int): Interval between checks in seconds. Defaults to 10. - - Returns: - None - """ - logger.info( - f"Waiting for deployment {deployment_name} in namespace {namespace} to be ready..." - ) - start_time = time.time() - while True: - deployment: Deployment = client.get(Deployment, namespace=namespace, name=deployment_name) - if deployment.status.availableReplicas == deployment.spec.replicas: - logger.info(f"Deployment {deployment_name} in namespace {namespace} is ready") - break - elif time.time() - start_time >= timeout_seconds: - raise TimeoutError( - f"Timeout waiting for deployment {deployment_name} in namespace {namespace} to be ready" # noqa E501 - ) - else: - time.sleep(interval_seconds) - logger.info( - f"Waiting for deployment {deployment_name} in namespace {namespace} to be ready..." - ) - - def initialize(lightkube_client: Client) -> None: """ Initializes the Kubernetes cluster by applying manifests from a YAML file. diff --git a/src/dss/utils.py b/src/dss/utils.py index 1fea09d..e744260 100644 --- a/src/dss/utils.py +++ b/src/dss/utils.py @@ -1,13 +1,60 @@ import os +import time from typing import Optional from lightkube import Client, KubeConfig +from lightkube.resources.apps_v1 import Deployment + +from dss.logger import setup_logger + +# Set up logger +logger = setup_logger("logs/dss.log") # Name for the environment variable storing kubeconfig KUBECONFIG_ENV_VAR = "DSS_KUBECONFIG" KUBECONFIG_DEFAULT = "./kubeconfig" +def wait_for_deployment_ready( + client: Client, + namespace: str, + deployment_name: str, + timeout_seconds: int = 60, + interval_seconds: int = 10, +) -> None: + """ + Waits for a Kubernetes deployment to be ready. + + Args: + client (Client): The Kubernetes client. + namespace (str): The namespace of the deployment. + deployment_name (str): The name of the deployment. + timeout_seconds (int): Timeout in seconds. Defaults to 600. + interval_seconds (int): Interval between checks in seconds. Defaults to 10. + + Returns: + None + """ + logger.info( + f"Waiting for deployment {deployment_name} in namespace {namespace} to be ready..." + ) + start_time = time.time() + while True: + deployment: Deployment = client.get(Deployment, namespace=namespace, name=deployment_name) + if deployment.status.availableReplicas == deployment.spec.replicas: + logger.info(f"Deployment {deployment_name} in namespace {namespace} is ready") + break + elif time.time() - start_time >= timeout_seconds: + raise TimeoutError( + f"Timeout waiting for deployment {deployment_name} in namespace {namespace} to be ready" # noqa E501 + ) + else: + time.sleep(interval_seconds) + logger.info( + f"Waiting for deployment {deployment_name} in namespace {namespace} to be ready..." + ) + + def get_default_kubeconfig(kubeconfig: Optional[str] = None) -> str: """ Get the kubeconfig file path, from input, environment variable, or default. diff --git a/tests/unit/test_initialize.py b/tests/unit/test_initialize.py index 3e69914..1c94c97 100644 --- a/tests/unit/test_initialize.py +++ b/tests/unit/test_initialize.py @@ -1,9 +1,8 @@ from unittest.mock import MagicMock, patch import pytest -from lightkube.resources.apps_v1 import Deployment -from dss.initialize import initialize, wait_for_deployment_ready +from dss.initialize import initialize @pytest.fixture @@ -15,15 +14,6 @@ def mock_environ_get() -> MagicMock: yield mock_env_get -@pytest.fixture -def mock_kubeconfig_from_dict() -> MagicMock: - """ - Fixture to mock the KubeConfig.from_dict function. - """ - with patch("dss.initialize.KubeConfig.from_dict") as mock_kubeconfig: - yield mock_kubeconfig - - @pytest.fixture def mock_client() -> MagicMock: """ @@ -53,7 +43,6 @@ def mock_logger() -> MagicMock: def test_initialize_success( mock_environ_get: MagicMock, - mock_kubeconfig_from_dict: MagicMock, mock_client: MagicMock, mock_resource_handler: MagicMock, mock_logger: MagicMock, @@ -82,31 +71,3 @@ def test_initialize_success( mock_logger.info.assert_called_with( "DSS initialized. To create your first notebook run the command:\n\ndss create-notebook" ) - - -def test_wait_for_deployment_ready_timeout(mock_client: MagicMock, mock_logger: MagicMock) -> None: - """ - Test case to verify timeout while waiting for deployment to be ready. - """ - # Mock the behavior of the client.get method to return a deployment with available replicas = 0 - mock_client_instance = MagicMock() - mock_client_instance.get.return_value = MagicMock( - spec=Deployment, status=MagicMock(availableReplicas=0), spec_replicas=1 - ) - - # Call the function to test - with pytest.raises(TimeoutError) as exc_info: - wait_for_deployment_ready( - mock_client_instance, - namespace="test-namespace", - deployment_name="test-deployment", - timeout_seconds=5, - interval_seconds=1, - ) - - # Assertions - assert ( - str(exc_info.value) - == "Timeout waiting for deployment test-deployment in namespace test-namespace to be ready" - ) - assert mock_client_instance.get.call_count == 6 # 5 attempts, 1 final attempt after timeout diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 057e842..15f66e8 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1,8 +1,23 @@ from unittest.mock import MagicMock, patch import pytest +from lightkube.resources.apps_v1 import Deployment -from dss.utils import KUBECONFIG_DEFAULT, get_default_kubeconfig, get_lightkube_client +from dss.utils import ( + KUBECONFIG_DEFAULT, + get_default_kubeconfig, + get_lightkube_client, + wait_for_deployment_ready, +) + + +@pytest.fixture +def mock_client() -> MagicMock: + """ + Fixture to mock the Client class. + """ + with patch("dss.utils.Client") as mock_client: + yield mock_client @pytest.fixture @@ -24,12 +39,12 @@ def mock_kubeconfig() -> MagicMock: @pytest.fixture -def mock_client() -> MagicMock: +def mock_logger() -> MagicMock: """ - Fixture to mock the Client class. + Fixture to mock the logger object. """ - with patch("dss.utils.Client") as mock_client: - yield mock_client + with patch("dss.initialize.logger") as mock_logger: + yield mock_logger @pytest.mark.parametrize( @@ -76,3 +91,31 @@ def test_get_lightkube_client_successful( mock_kubeconfig.from_file.assert_called_with(kubeconfig) mock_client.assert_called_with(config=mock_kubeconfig_instance) assert returned_client is not None + + +def test_wait_for_deployment_ready_timeout(mock_client: MagicMock, mock_logger: MagicMock) -> None: + """ + Test case to verify timeout while waiting for deployment to be ready. + """ + # Mock the behavior of the client.get method to return a deployment with available replicas = 0 + mock_client_instance = MagicMock() + mock_client_instance.get.return_value = MagicMock( + spec=Deployment, status=MagicMock(availableReplicas=0), spec_replicas=1 + ) + + # Call the function to test + with pytest.raises(TimeoutError) as exc_info: + wait_for_deployment_ready( + mock_client_instance, + namespace="test-namespace", + deployment_name="test-deployment", + timeout_seconds=5, + interval_seconds=1, + ) + + # Assertions + assert ( + str(exc_info.value) + == "Timeout waiting for deployment test-deployment in namespace test-namespace to be ready" + ) + assert mock_client_instance.get.call_count == 6 # 5 attempts, 1 final attempt after timeout