diff --git a/src/tetra_rp/cli/commands/resource.py b/src/tetra_rp/cli/commands/resource.py index 97b53f8..c39cd65 100644 --- a/src/tetra_rp/cli/commands/resource.py +++ b/src/tetra_rp/cli/commands/resource.py @@ -1,13 +1,11 @@ """Resource management commands.""" +import time import typer from rich.console import Console from rich.table import Table from rich.panel import Panel from rich.live import Live -from rich.progress import Progress, SpinnerColumn, TextColumn -import questionary -import time from ...core.resources.resource_manager import ResourceManager @@ -42,58 +40,6 @@ def report_command( console.print(table) -def clean_command( - force: bool = typer.Option(False, "--force", "-f", help="Skip confirmation"), -): - """Remove all tracked resources after confirmation.""" - - resource_manager = ResourceManager() - resources = resource_manager._resources - - if not resources: - console.print("🧹 No resources to clean") - return - - # Show cleanup preview - console.print(generate_cleanup_preview(resources)) - - # Confirmation unless forced - if not force: - try: - confirmed = questionary.confirm( - "Are you sure you want to clean all resources?" - ).ask() - - if not confirmed: - console.print("Cleanup cancelled") - return - except KeyboardInterrupt: - console.print("\nCleanup cancelled") - return - - # Clean resources with progress - with Progress( - SpinnerColumn(), - TextColumn("[progress.description]{task.description}"), - console=console, - ) as progress: - task = progress.add_task("Cleaning resources...", total=len(resources)) - - for uid in list(resources.keys()): - resource = resources[uid] - progress.update( - task, description=f"Removing {resource.__class__.__name__}..." - ) - - # Remove resource (this will also clean up remotely if needed) - resource_manager.remove_resource(uid) - - progress.advance(task) - time.sleep(0.1) # Small delay for visual feedback - - console.print("All resources cleaned successfully") - - def generate_resource_table(resource_manager: ResourceManager) -> Panel: """Generate a formatted table of resources.""" @@ -160,32 +106,3 @@ def generate_resource_table(resource_manager: ResourceManager) -> Panel: summary += ")" return Panel(table, subtitle=summary, expand=False) - - -def generate_cleanup_preview(resources: dict) -> Panel: - """Generate a preview of resources to be cleaned.""" - - content = "The following resources will be removed:\n\n" - - for uid, resource in resources.items(): - resource_type = resource.__class__.__name__ - - try: - status = "Active" if resource.is_deployed() else "Inactive" - except Exception: - status = "Unknown" - - try: - url = ( - f" - {resource.url}" - if hasattr(resource, "url") and resource.url != "N/A" - else "" - ) - except Exception: - url = "" - - content += f" • {resource_type} ({status}){url}\n" - - content += "\n⚠️ This action cannot be undone!" - - return Panel(content, title="🧹 Cleanup Preview", expand=False) diff --git a/src/tetra_rp/cli/commands/undeploy.py b/src/tetra_rp/cli/commands/undeploy.py index 5208350..7ceacc8 100644 --- a/src/tetra_rp/cli/commands/undeploy.py +++ b/src/tetra_rp/cli/commands/undeploy.py @@ -11,7 +11,6 @@ from ...core.resources.base import DeployableResource from ...core.resources.resource_manager import ResourceManager -from ...core.api.runpod import RunpodGraphQLClient console = Console() @@ -130,47 +129,6 @@ def list_command(): ) -async def _delete_endpoint(endpoint_id: str, resource_id: str, name: str) -> dict: - """Delete an endpoint via RunPod API and remove from tracking. - - Args: - endpoint_id: RunPod endpoint ID - resource_id: Local resource ID for tracking - name: Human-readable name - - Returns: - Dict with success status and message - """ - try: - async with RunpodGraphQLClient() as client: - result = await client.delete_endpoint(endpoint_id) - - if result.get("success"): - # Remove from tracking - manager = ResourceManager() - manager.remove_resource(resource_id) - return { - "success": True, - "name": name, - "endpoint_id": endpoint_id, - "message": f"Successfully deleted endpoint '{name}' ({endpoint_id})", - } - else: - return { - "success": False, - "name": name, - "endpoint_id": endpoint_id, - "message": f"Failed to delete endpoint '{name}' ({endpoint_id})", - } - except Exception as e: - return { - "success": False, - "name": name, - "endpoint_id": endpoint_id, - "message": f"Error deleting endpoint '{name}': {str(e)}", - } - - def _cleanup_stale_endpoints( resources: Dict[str, DeployableResource], manager: ResourceManager ) -> None: @@ -219,7 +177,7 @@ def _cleanup_stale_endpoints( removed_count = 0 for resource_id, resource in inactive: try: - manager.remove_resource(resource_id) + manager._remove_resource(resource_id) removed_count += 1 console.print( f"[green]✓[/green] Removed [cyan]{resource.name}[/cyan] from tracking" @@ -359,24 +317,11 @@ def _undeploy_by_name(name: str, resources: dict): raise typer.Exit(0) # Delete endpoints + manager = ResourceManager() with console.status("Deleting endpoint(s)..."): results = [] for resource_id, resource in matches: - endpoint_id = getattr(resource, "id", None) - if not endpoint_id: - results.append( - { - "success": False, - "name": resource.name, - "endpoint_id": "N/A", - "message": f"Skipped '{resource.name}': No endpoint ID found", - } - ) - continue - - result = asyncio.run( - _delete_endpoint(endpoint_id, resource_id, resource.name) - ) + result = asyncio.run(manager.undeploy_resource(resource_id, resource.name)) results.append(result) # Show results @@ -437,24 +382,12 @@ def _undeploy_all(resources: dict): raise typer.Exit(0) # Delete all endpoints + manager = ResourceManager() with console.status(f"Deleting {len(resources)} endpoint(s)..."): results = [] for resource_id, resource in resources.items(): - endpoint_id = getattr(resource, "id", None) name = getattr(resource, "name", "N/A") - - if not endpoint_id: - results.append( - { - "success": False, - "name": name, - "endpoint_id": "N/A", - "message": f"Skipped '{name}': No endpoint ID found", - } - ) - continue - - result = asyncio.run(_delete_endpoint(endpoint_id, resource_id, name)) + result = asyncio.run(manager.undeploy_resource(resource_id, name)) results.append(result) # Show results @@ -534,24 +467,12 @@ def _interactive_undeploy(resources: dict): raise typer.Exit(0) # Delete selected endpoints + manager = ResourceManager() with console.status(f"Deleting {len(selected_resources)} endpoint(s)..."): results = [] for resource_id, resource in selected_resources: - endpoint_id = getattr(resource, "id", None) name = getattr(resource, "name", "N/A") - - if not endpoint_id: - results.append( - { - "success": False, - "name": name, - "endpoint_id": "N/A", - "message": f"Skipped '{name}': No endpoint ID found", - } - ) - continue - - result = asyncio.run(_delete_endpoint(endpoint_id, resource_id, name)) + result = asyncio.run(manager.undeploy_resource(resource_id, name)) results.append(result) # Show results diff --git a/src/tetra_rp/cli/main.py b/src/tetra_rp/cli/main.py index bf230f8..06cc76b 100644 --- a/src/tetra_rp/cli/main.py +++ b/src/tetra_rp/cli/main.py @@ -38,7 +38,6 @@ def get_version() -> str: app.command("run")(run.run_command) app.command("build")(build.build_command) # app.command("report")(resource.report_command) -# app.command("clean")(resource.clean_command) # command: flash deploy diff --git a/src/tetra_rp/core/resources/base.py b/src/tetra_rp/core/resources/base.py index 2e2e28c..256f3e2 100644 --- a/src/tetra_rp/core/resources/base.py +++ b/src/tetra_rp/core/resources/base.py @@ -45,3 +45,12 @@ def is_deployed(self) -> bool: async def deploy(self) -> "DeployableResource": """Deploy the resource.""" raise NotImplementedError("Subclasses should implement this method.") + + @abstractmethod + async def undeploy(self) -> bool: + """Undeploy/delete the resource. + + Returns: + True if successfully undeployed, False otherwise + """ + raise NotImplementedError("Subclasses should implement this method.") diff --git a/src/tetra_rp/core/resources/network_volume.py b/src/tetra_rp/core/resources/network_volume.py index 240fa58..ea5f73b 100644 --- a/src/tetra_rp/core/resources/network_volume.py +++ b/src/tetra_rp/core/resources/network_volume.py @@ -146,3 +146,18 @@ async def deploy(self) -> "DeployableResource": except Exception as e: log.error(f"{self} failed to deploy: {e}") raise + + async def undeploy(self) -> bool: + """ + Undeploy network volume. + + Returns: + True if successfully undeployed, False otherwise + + Raises: + NotImplementedError: NetworkVolume undeploy is not yet supported + """ + raise NotImplementedError( + f"{self.__class__.__name__} undeploy is not yet supported. " + "Network volumes must be manually deleted via RunPod UI or API." + ) diff --git a/src/tetra_rp/core/resources/resource_manager.py b/src/tetra_rp/core/resources/resource_manager.py index 2973817..cba33c6 100644 --- a/src/tetra_rp/core/resources/resource_manager.py +++ b/src/tetra_rp/core/resources/resource_manager.py @@ -1,7 +1,7 @@ import asyncio import cloudpickle import logging -from typing import Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Tuple from pathlib import Path from ..exceptions import RunpodAPIKeyError @@ -64,14 +64,13 @@ def _save_resources(self) -> None: log.error(f"Failed to save resources to {RESOURCE_STATE_FILE}: {e}") raise - def add_resource(self, uid: str, resource: DeployableResource): - """Add a resource to the manager.""" + def _add_resource(self, uid: str, resource: DeployableResource): + """Add a resource to the manager (protected method for internal use).""" self._resources[uid] = resource self._save_resources() - # function to check if resource still exists remotely, else remove it - def remove_resource(self, uid: str): - """Remove a resource from the manager.""" + def _remove_resource(self, uid: str): + """Remove a resource from the manager (protected method for internal use).""" if uid not in self._resources: log.warning(f"Resource {uid} not found for removal") return @@ -126,11 +125,11 @@ async def get_or_deploy_resource( if existing := self._resources.get(uid): if not existing.is_deployed(): log.warning(f"{existing} is no longer valid, redeploying.") - self.remove_resource(uid) + self._remove_resource(uid) # Don't recursive call - deploy directly within the lock deployed_resource = await self._deploy_with_error_context(config) log.info(f"URL: {deployed_resource.url}") - self.add_resource(uid, deployed_resource) + self._add_resource(uid, deployed_resource) return deployed_resource log.debug(f"{existing} exists, reusing.") @@ -141,7 +140,7 @@ async def get_or_deploy_resource( log.debug(f"Deploying new resource: {uid}") deployed_resource = await self._deploy_with_error_context(config) log.info(f"URL: {deployed_resource.url}") - self.add_resource(uid, deployed_resource) + self._add_resource(uid, deployed_resource) return deployed_resource def list_all_resources(self) -> Dict[str, DeployableResource]: @@ -166,3 +165,74 @@ def find_resources_by_name(self, name: str) -> List[Tuple[str, DeployableResourc if hasattr(resource, "name") and resource.name == name: matches.append((uid, resource)) return matches + + async def undeploy_resource( + self, resource_id: str, resource_name: Optional[str] = None + ) -> Dict[str, Any]: + """Undeploy a resource and remove from tracking. + + This is the public interface for removing resources. It calls the resource's + undeploy() method (polymorphic) and removes from tracking on success. + + Args: + resource_id: The resource ID to undeploy + resource_name: Optional human-readable name for error messages + + Returns: + Dict with keys: + - success: bool indicating if undeploy succeeded + - name: resource name (if available) + - endpoint_id: resource endpoint ID (if available) + - message: status message + """ + resource = self._resources.get(resource_id) + + if not resource: + return { + "success": False, + "name": resource_name or "Unknown", + "endpoint_id": "N/A", + "message": f"Resource {resource_id} not found in tracking", + } + + # Get resource metadata for response + name = resource_name or getattr(resource, "name", "Unknown") + endpoint_id = getattr(resource, "id", "N/A") + + try: + # Call polymorphic undeploy method + success = await resource.undeploy() + + if success: + # Remove from tracking on successful undeploy + self._remove_resource(resource_id) + return { + "success": True, + "name": name, + "endpoint_id": endpoint_id, + "message": f"Successfully undeployed '{name}' ({endpoint_id})", + } + else: + return { + "success": False, + "name": name, + "endpoint_id": endpoint_id, + "message": f"Failed to undeploy '{name}' ({endpoint_id})", + } + + except NotImplementedError as e: + # Resource type doesn't support undeploy yet + return { + "success": False, + "name": name, + "endpoint_id": endpoint_id, + "message": f"Cannot undeploy '{name}': {str(e)}", + } + except Exception as e: + # Unexpected error during undeploy + return { + "success": False, + "name": name, + "endpoint_id": endpoint_id, + "message": f"Error undeploying '{name}': {str(e)}", + } diff --git a/src/tetra_rp/core/resources/serverless.py b/src/tetra_rp/core/resources/serverless.py index 8ec7052..d42cfa5 100644 --- a/src/tetra_rp/core/resources/serverless.py +++ b/src/tetra_rp/core/resources/serverless.py @@ -268,6 +268,33 @@ async def deploy(self) -> "DeployableResource": log.error(f"{self} failed to deploy: {e}") raise + async def undeploy(self) -> bool: + """ + Undeploys (deletes) the serverless endpoint. + + Returns: + True if successfully undeployed, False otherwise + """ + if not self.id: + log.warning(f"{self} has no endpoint ID, cannot undeploy") + return False + + try: + async with RunpodGraphQLClient() as client: + result = await client.delete_endpoint(self.id) + success = result.get("success", False) + + if success: + log.info(f"{self} successfully undeployed") + else: + log.error(f"{self} failed to undeploy") + + return success + + except Exception as e: + log.error(f"{self} failed to undeploy: {e}") + return False + async def run_sync(self, payload: Dict[str, Any]) -> "JobOutput": """ Executes a serverless endpoint request with the payload. diff --git a/tests/unit/cli/test_undeploy.py b/tests/unit/cli/test_undeploy.py index 5789631..5132f69 100644 --- a/tests/unit/cli/test_undeploy.py +++ b/tests/unit/cli/test_undeploy.py @@ -1,7 +1,7 @@ """Unit tests for undeploy CLI command.""" import pytest -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import MagicMock, patch from typer.testing import CliRunner from tetra_rp.cli.main import app @@ -56,6 +56,22 @@ def sample_resources(): } +@pytest.fixture +def mock_asyncio_run_coro(): + """Create a mock asyncio.run that executes coroutines.""" + + def run_coro(coro): + import asyncio + + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + + return run_coro + + class TestUndeployList: """Test undeploy list command.""" @@ -152,7 +168,9 @@ def test_undeploy_by_name_cancelled(self, runner, sample_resources): assert "cancelled" in result.stdout.lower() @patch("tetra_rp.cli.commands.undeploy.asyncio.run") - def test_undeploy_by_name_success(self, mock_asyncio_run, runner, sample_resources): + def test_undeploy_by_name_success( + self, mock_asyncio_run, runner, sample_resources, mock_asyncio_run_coro + ): """Test successful undeploy by name.""" with ( patch("tetra_rp.cli.commands.undeploy.ResourceManager") as MockRM, @@ -160,6 +178,17 @@ def test_undeploy_by_name_success(self, mock_asyncio_run, runner, sample_resourc ): mock_manager = MagicMock() mock_manager.list_all_resources.return_value = sample_resources + + # Mock undeploy_resource as async coroutine that returns success + async def mock_undeploy(resource_id, name): + return { + "success": True, + "name": name, + "endpoint_id": "endpoint-id-1", + "message": f"Successfully undeployed '{name}'", + } + + mock_manager.undeploy_resource = mock_undeploy MockRM.return_value = mock_manager # User confirms @@ -167,21 +196,17 @@ def test_undeploy_by_name_success(self, mock_asyncio_run, runner, sample_resourc mock_confirm.ask.return_value = True mock_questionary.confirm.return_value = mock_confirm - # Mock successful deletion - mock_asyncio_run.return_value = { - "success": True, - "name": "test-api-1", - "endpoint_id": "endpoint-id-1", - "message": "Successfully deleted", - } + mock_asyncio_run.side_effect = mock_asyncio_run_coro result = runner.invoke(app, ["undeploy", "test-api-1"]) assert result.exit_code == 0 - assert "Successfully deleted" in result.stdout + assert "Successfully" in result.stdout @patch("tetra_rp.cli.commands.undeploy.asyncio.run") - def test_undeploy_all_flag(self, mock_asyncio_run, runner, sample_resources): + def test_undeploy_all_flag( + self, mock_asyncio_run, runner, sample_resources, mock_asyncio_run_coro + ): """Test undeploy --all flag.""" with ( patch("tetra_rp.cli.commands.undeploy.ResourceManager") as MockRM, @@ -189,6 +214,17 @@ def test_undeploy_all_flag(self, mock_asyncio_run, runner, sample_resources): ): mock_manager = MagicMock() mock_manager.list_all_resources.return_value = sample_resources + + # Mock undeploy_resource as async coroutine that returns success + async def mock_undeploy(resource_id, name): + return { + "success": True, + "name": name, + "endpoint_id": "endpoint-id", + "message": f"Successfully undeployed '{name}'", + } + + mock_manager.undeploy_resource = mock_undeploy MockRM.return_value = mock_manager # User confirms both prompts @@ -200,18 +236,12 @@ def test_undeploy_all_flag(self, mock_asyncio_run, runner, sample_resources): mock_questionary.confirm.return_value = mock_confirm mock_questionary.text.return_value = mock_text - # Mock successful deletions - mock_asyncio_run.return_value = { - "success": True, - "name": "test-api", - "endpoint_id": "endpoint-id", - "message": "Successfully deleted", - } + mock_asyncio_run.side_effect = mock_asyncio_run_coro result = runner.invoke(app, ["undeploy", "--all"]) assert result.exit_code == 0 - assert "Successfully deleted" in result.stdout + assert "Successfully" in result.stdout def test_undeploy_all_wrong_confirmation(self, runner, sample_resources): """Test undeploy --all with wrong confirmation text.""" @@ -238,79 +268,6 @@ def test_undeploy_all_wrong_confirmation(self, runner, sample_resources): assert "Confirmation failed" in result.stdout -class TestDeleteEndpoint: - """Test _delete_endpoint helper function.""" - - @pytest.mark.asyncio - async def test_delete_endpoint_success(self, sample_resources): - """Test successful endpoint deletion.""" - from tetra_rp.cli.commands.undeploy import _delete_endpoint - - resource = list(sample_resources.values())[0] - endpoint_id = resource.id - resource_id = resource.resource_id - name = resource.name - - with ( - patch("tetra_rp.cli.commands.undeploy.RunpodGraphQLClient") as MockClient, - patch("tetra_rp.cli.commands.undeploy.ResourceManager") as MockRM, - ): - # Mock successful API deletion - mock_client = AsyncMock() - mock_client.delete_endpoint.return_value = {"success": True} - mock_client.__aenter__ = AsyncMock(return_value=mock_client) - mock_client.__aexit__ = AsyncMock(return_value=None) - MockClient.return_value = mock_client - - # Mock manager - mock_manager = MagicMock() - MockRM.return_value = mock_manager - - result = await _delete_endpoint(endpoint_id, resource_id, name) - - assert result["success"] is True - assert result["name"] == name - assert result["endpoint_id"] == endpoint_id - mock_manager.remove_resource.assert_called_once_with(resource_id) - - @pytest.mark.asyncio - async def test_delete_endpoint_api_failure(self): - """Test endpoint deletion with API failure (malformed response).""" - from tetra_rp.cli.commands.undeploy import _delete_endpoint - - with patch("tetra_rp.cli.commands.undeploy.RunpodGraphQLClient") as MockClient: - # Mock failed API deletion - returns empty dict (missing deleteEndpoint key) - mock_client = AsyncMock() - mock_client.delete_endpoint.return_value = {"success": False} - mock_client.__aenter__ = AsyncMock(return_value=mock_client) - mock_client.__aexit__ = AsyncMock(return_value=None) - MockClient.return_value = mock_client - - result = await _delete_endpoint("endpoint-id", "resource-id", "test-name") - - assert result["success"] is False - assert "Failed to delete" in result["message"] - - @pytest.mark.asyncio - async def test_delete_endpoint_exception(self): - """Test endpoint deletion with exception.""" - from tetra_rp.cli.commands.undeploy import _delete_endpoint - - with patch("tetra_rp.cli.commands.undeploy.RunpodGraphQLClient") as MockClient: - # Mock exception during deletion - mock_client = AsyncMock() - mock_client.delete_endpoint.side_effect = Exception("API Error") - mock_client.__aenter__ = AsyncMock(return_value=mock_client) - mock_client.__aexit__ = AsyncMock(return_value=None) - MockClient.return_value = mock_client - - result = await _delete_endpoint("endpoint-id", "resource-id", "test-name") - - assert result["success"] is False - assert "Error deleting" in result["message"] - assert "API Error" in result["message"] - - class TestResourceStatusHelpers: """Test helper functions for resource status.""" diff --git a/tests/unit/resources/test_resource_manager.py b/tests/unit/resources/test_resource_manager.py index 49fab4a..0f0b17e 100644 --- a/tests/unit/resources/test_resource_manager.py +++ b/tests/unit/resources/test_resource_manager.py @@ -188,7 +188,7 @@ def test_add_and_find_resource(self, mock_resource_file): resource.id = "test-endpoint-id" with patch.object(manager, "_save_resources"): - manager.add_resource(resource.resource_id, resource) + manager._add_resource(resource.resource_id, resource) # Find by name matches = manager.find_resources_by_name("my-endpoint") @@ -210,7 +210,7 @@ def test_remove_resource_updates_find_results( # Remove one uid_to_remove = matches[0][0] with patch.object(manager, "_save_resources"): - manager.remove_resource(uid_to_remove) + manager._remove_resource(uid_to_remove) # Now should be 1 match matches_after = manager.find_resources_by_name("test-api-1") @@ -235,14 +235,14 @@ def test_list_all_resources_integration_with_add_remove(self, mock_resource_file ) with patch.object(manager, "_save_resources"): - manager.add_resource(resource.resource_id, resource) + manager._add_resource(resource.resource_id, resource) # Should have 1 resource assert len(manager.list_all_resources()) == 1 # Remove resource with patch.object(manager, "_save_resources"): - manager.remove_resource(resource.resource_id) + manager._remove_resource(resource.resource_id) # Should be empty again assert len(manager.list_all_resources()) == 0 diff --git a/tests/unit/test_concurrency_issues.py b/tests/unit/test_concurrency_issues.py index 63adf0d..9d5b28d 100644 --- a/tests/unit/test_concurrency_issues.py +++ b/tests/unit/test_concurrency_issues.py @@ -68,6 +68,11 @@ async def deploy(self) -> "DeployableResource": deployed._deploy_count = self._deploy_count return deployed + async def undeploy(self) -> bool: + """Mock undeploy method.""" + self._deployed = False + return True + class TestSingleton: """Test thread safety of SingletonMixin.""" @@ -237,7 +242,7 @@ def test_file_state_race_condition(self): def save_resource_1(): try: - manager1.add_resource("resource1", resource1) + manager1._add_resource("resource1", resource1) # Add delay to increase race condition likelihood time.sleep(0.01) except Exception as e: @@ -245,7 +250,7 @@ def save_resource_1(): def save_resource_2(): try: - manager2.add_resource("resource2", resource2) + manager2._add_resource("resource2", resource2) time.sleep(0.01) except Exception as e: exceptions.append(e)