From 68124be918b0399def8d5736ebae0f22a6a76ca1 Mon Sep 17 00:00:00 2001 From: Seungju Baek Date: Wed, 13 Aug 2025 11:48:28 +0900 Subject: [PATCH 1/2] chore : ruff failure fix --- src/openstack_mcp_server/tools/__init__.py | 2 +- .../tools/block_storage_tools.py | 24 +++-- .../tools/identity_tools.py | 23 +++-- tests/tools/test_block_storage_tools.py | 89 +++++++++++++------ tests/tools/test_compute_tools.py | 4 +- tests/tools/test_identity_tools.py | 70 ++++++++++----- 6 files changed, 146 insertions(+), 66 deletions(-) diff --git a/src/openstack_mcp_server/tools/__init__.py b/src/openstack_mcp_server/tools/__init__.py index 48b7784..e42a705 100644 --- a/src/openstack_mcp_server/tools/__init__.py +++ b/src/openstack_mcp_server/tools/__init__.py @@ -6,11 +6,11 @@ def register_tool(mcp: FastMCP): Register Openstack MCP tools. """ + from .block_storage_tools import BlockStorageTools from .compute_tools import ComputeTools from .identity_tools import IdentityTools from .image_tools import ImageTools from .network_tools import NetworkTools - from .block_storage_tools import BlockStorageTools ComputeTools().register_tools(mcp) ImageTools().register_tools(mcp) diff --git a/src/openstack_mcp_server/tools/block_storage_tools.py b/src/openstack_mcp_server/tools/block_storage_tools.py index f5f9cf3..d682ebd 100644 --- a/src/openstack_mcp_server/tools/block_storage_tools.py +++ b/src/openstack_mcp_server/tools/block_storage_tools.py @@ -1,9 +1,10 @@ +from fastmcp import FastMCP + +from .base import get_openstack_conn from .response.block_storage import ( Volume, VolumeAttachment, ) -from .base import get_openstack_conn -from fastmcp import FastMCP class BlockStorageTools: @@ -39,7 +40,7 @@ def get_volumes(self) -> list[Volume]: server_id=attachment.get("server_id"), device=attachment.get("device"), attachment_id=attachment.get("id"), - ) + ), ) volume_list.append( @@ -57,7 +58,7 @@ def get_volumes(self) -> list[Volume]: is_encrypted=volume.is_encrypted, description=volume.description, attachments=attachments, - ) + ), ) return volume_list @@ -80,7 +81,7 @@ def get_volume_details(self, volume_id: str) -> Volume: server_id=attachment.get("server_id"), device=attachment.get("device"), attachment_id=attachment.get("id"), - ) + ), ) return Volume( @@ -133,7 +134,10 @@ def create_volume( volume_kwargs["availability_zone"] = availability_zone volume = conn.block_storage.create_volume( - size=size, image=image, bootable=bootable, **volume_kwargs + size=size, + image=image, + bootable=bootable, + **volume_kwargs, ) volume_obj = Volume( @@ -162,7 +166,11 @@ def delete_volume(self, volume_id: str, force: bool = False) -> None: """ conn = get_openstack_conn() - conn.block_storage.delete_volume(volume_id, force=force, ignore_missing=False) + conn.block_storage.delete_volume( + volume_id, + force=force, + ignore_missing=False, + ) def extend_volume(self, volume_id: str, new_size: int) -> None: """ @@ -174,4 +182,4 @@ def extend_volume(self, volume_id: str, new_size: int) -> None: """ conn = get_openstack_conn() - conn.block_storage.extend_volume(volume_id, new_size) \ No newline at end of file + conn.block_storage.extend_volume(volume_id, new_size) diff --git a/src/openstack_mcp_server/tools/identity_tools.py b/src/openstack_mcp_server/tools/identity_tools.py index 977f17b..ec53375 100644 --- a/src/openstack_mcp_server/tools/identity_tools.py +++ b/src/openstack_mcp_server/tools/identity_tools.py @@ -1,7 +1,7 @@ from fastmcp import FastMCP from .base import get_openstack_conn -from .response.identity import Region, Domain +from .response.identity import Domain, Region class IdentityTools: @@ -19,7 +19,7 @@ def register_tools(self, mcp: FastMCP): mcp.tool()(self.create_region) mcp.tool()(self.delete_region) mcp.tool()(self.update_region) - + mcp.tool()(self.get_domains) mcp.tool()(self.get_domain) @@ -103,7 +103,7 @@ def update_region(self, id: str, description: str = "") -> Region: id=updated_region.id, description=updated_region.description, ) - + def get_domains(self) -> list[Domain]: """ Get the list of Identity domains. @@ -115,7 +115,12 @@ def get_domains(self) -> list[Domain]: domain_list = [] for domain in conn.identity.domains(): domain_list.append( - Domain(id=domain.id, name=domain.name, description=domain.description, is_enabled=domain.is_enabled), + Domain( + id=domain.id, + name=domain.name, + description=domain.description, + is_enabled=domain.is_enabled, + ), ) return domain_list @@ -128,8 +133,12 @@ def get_domain(self, id: str) -> Domain: :return: The Domain object. """ conn = get_openstack_conn() - + domain = conn.identity.get_domain(domain=id) - return Domain(id=domain.id, name=domain.name, description=domain.description, is_enabled=domain.is_enabled) - + return Domain( + id=domain.id, + name=domain.name, + description=domain.description, + is_enabled=domain.is_enabled, + ) diff --git a/tests/tools/test_block_storage_tools.py b/tests/tools/test_block_storage_tools.py index eafc43e..305f66e 100644 --- a/tests/tools/test_block_storage_tools.py +++ b/tests/tools/test_block_storage_tools.py @@ -1,11 +1,13 @@ +from unittest.mock import Mock + import pytest + from openstack_mcp_server.tools.block_storage_tools import BlockStorageTools from openstack_mcp_server.tools.response.block_storage import ( Volume, VolumeAttachment, ) -from unittest.mock import Mock class TestBlockStorageTools: """Test cases for BlockStorageTools class.""" @@ -74,7 +76,8 @@ def test_get_volumes_success(self, mock_get_openstack_conn_block_storage): mock_conn.block_storage.volumes.assert_called_once() def test_get_volumes_empty_list( - self, mock_get_openstack_conn_block_storage + self, + mock_get_openstack_conn_block_storage, ): """Test getting volumes when no volumes exist.""" mock_conn = mock_get_openstack_conn_block_storage @@ -92,7 +95,8 @@ def test_get_volumes_empty_list( mock_conn.block_storage.volumes.assert_called_once() def test_get_volumes_single_volume( - self, mock_get_openstack_conn_block_storage + self, + mock_get_openstack_conn_block_storage, ): """Test getting volumes with a single volume.""" mock_conn = mock_get_openstack_conn_block_storage @@ -125,7 +129,8 @@ def test_get_volumes_single_volume( mock_conn.block_storage.volumes.assert_called_once() def test_get_volumes_multiple_statuses( - self, mock_get_openstack_conn_block_storage + self, + mock_get_openstack_conn_block_storage, ): """Test volumes with various statuses.""" mock_conn = mock_get_openstack_conn_block_storage @@ -175,7 +180,8 @@ def test_get_volumes_multiple_statuses( mock_conn.block_storage.volumes.assert_called_once() def test_get_volumes_with_special_characters( - self, mock_get_openstack_conn_block_storage + self, + mock_get_openstack_conn_block_storage, ): """Test volumes with special characters in names.""" mock_conn = mock_get_openstack_conn_block_storage @@ -229,7 +235,10 @@ def test_get_volumes_with_special_characters( mock_conn.block_storage.volumes.assert_called_once() - def test_get_volume_details_success(self, mock_get_openstack_conn_block_storage): + def test_get_volume_details_success( + self, + mock_get_openstack_conn_block_storage, + ): """Test getting volume details successfully.""" mock_conn = mock_get_openstack_conn_block_storage @@ -268,7 +277,8 @@ def test_get_volume_details_success(self, mock_get_openstack_conn_block_storage) mock_conn.block_storage.get_volume.assert_called_once_with("vol-123") def test_get_volume_details_with_attachments( - self, mock_get_openstack_conn_block_storage + self, + mock_get_openstack_conn_block_storage, ): """Test getting volume details with attachments.""" mock_conn = mock_get_openstack_conn_block_storage @@ -323,11 +333,14 @@ def test_get_volume_details_with_attachments( assert attach2.device == "/dev/vdc" assert attach2.attachment_id == "attach-2" - def test_get_volume_details_error(self, mock_get_openstack_conn_block_storage): + def test_get_volume_details_error( + self, + mock_get_openstack_conn_block_storage, + ): """Test getting volume details with error.""" mock_conn = mock_get_openstack_conn_block_storage mock_conn.block_storage.get_volume.side_effect = Exception( - "Volume not found" + "Volume not found", ) block_storage_tools = BlockStorageTools() @@ -336,7 +349,10 @@ def test_get_volume_details_error(self, mock_get_openstack_conn_block_storage): with pytest.raises(Exception, match="Volume not found"): block_storage_tools.get_volume_details("nonexistent-vol") - def test_create_volume_success(self, mock_get_openstack_conn_block_storage): + def test_create_volume_success( + self, + mock_get_openstack_conn_block_storage, + ): """Test creating volume successfully.""" mock_conn = mock_get_openstack_conn_block_storage @@ -358,7 +374,11 @@ def test_create_volume_success(self, mock_get_openstack_conn_block_storage): block_storage_tools = BlockStorageTools() result = block_storage_tools.create_volume( - "new-volume", 10, "Test volume", "ssd", "nova" + "new-volume", + 10, + "Test volume", + "ssd", + "nova", ) # Verify result is a Volume object @@ -381,7 +401,8 @@ def test_create_volume_success(self, mock_get_openstack_conn_block_storage): ) def test_create_volume_minimal_params( - self, mock_get_openstack_conn_block_storage + self, + mock_get_openstack_conn_block_storage, ): """Test creating volume with minimal parameters.""" mock_conn = mock_get_openstack_conn_block_storage @@ -410,11 +431,15 @@ def test_create_volume_minimal_params( assert result.size == 5 mock_conn.block_storage.create_volume.assert_called_once_with( - size=5, image=None, bootable=None, name="minimal-volume" + size=5, + image=None, + bootable=None, + name="minimal-volume", ) def test_create_volume_with_image_and_bootable( - self, mock_get_openstack_conn_block_storage + self, + mock_get_openstack_conn_block_storage, ): """Test creating volume with image and bootable parameters.""" mock_conn = mock_get_openstack_conn_block_storage @@ -465,7 +490,7 @@ def test_create_volume_error(self, mock_get_openstack_conn_block_storage): """Test creating volume with error.""" mock_conn = mock_get_openstack_conn_block_storage mock_conn.block_storage.create_volume.side_effect = Exception( - "Quota exceeded" + "Quota exceeded", ) block_storage_tools = BlockStorageTools() @@ -473,7 +498,10 @@ def test_create_volume_error(self, mock_get_openstack_conn_block_storage): with pytest.raises(Exception, match="Quota exceeded"): block_storage_tools.create_volume("fail-volume", 100) - def test_delete_volume_success(self, mock_get_openstack_conn_block_storage): + def test_delete_volume_success( + self, + mock_get_openstack_conn_block_storage, + ): """Test deleting volume successfully.""" mock_conn = mock_get_openstack_conn_block_storage @@ -490,7 +518,9 @@ def test_delete_volume_success(self, mock_get_openstack_conn_block_storage): # Verify result is None assert result is None mock_conn.block_storage.delete_volume.assert_called_once_with( - "vol-delete", force=False, ignore_missing=False + "vol-delete", + force=False, + ignore_missing=False, ) def test_delete_volume_force(self, mock_get_openstack_conn_block_storage): @@ -510,14 +540,16 @@ def test_delete_volume_force(self, mock_get_openstack_conn_block_storage): assert result is None mock_conn.block_storage.delete_volume.assert_called_once_with( - "vol-force-delete", force=True, ignore_missing=False + "vol-force-delete", + force=True, + ignore_missing=False, ) def test_delete_volume_error(self, mock_get_openstack_conn_block_storage): """Test deleting volume with error.""" mock_conn = mock_get_openstack_conn_block_storage mock_conn.block_storage.delete_volume.side_effect = Exception( - "Volume not found" + "Volume not found", ) block_storage_tools = BlockStorageTools() @@ -526,7 +558,10 @@ def test_delete_volume_error(self, mock_get_openstack_conn_block_storage): with pytest.raises(Exception, match="Volume not found"): block_storage_tools.delete_volume("nonexistent-vol") - def test_extend_volume_success(self, mock_get_openstack_conn_block_storage): + def test_extend_volume_success( + self, + mock_get_openstack_conn_block_storage, + ): """Test extending volume successfully.""" mock_conn = mock_get_openstack_conn_block_storage @@ -537,14 +572,18 @@ def test_extend_volume_success(self, mock_get_openstack_conn_block_storage): assert result is None mock_conn.block_storage.extend_volume.assert_called_once_with( - "vol-extend", 20 + "vol-extend", + 20, ) - def test_extend_volume_invalid_size(self, mock_get_openstack_conn_block_storage): + def test_extend_volume_invalid_size( + self, + mock_get_openstack_conn_block_storage, + ): """Test extending volume with invalid size.""" mock_conn = mock_get_openstack_conn_block_storage mock_conn.block_storage.extend_volume.side_effect = Exception( - "Invalid size" + "Invalid size", ) block_storage_tools = BlockStorageTools() @@ -556,7 +595,7 @@ def test_extend_volume_error(self, mock_get_openstack_conn_block_storage): """Test extending volume with error.""" mock_conn = mock_get_openstack_conn_block_storage mock_conn.block_storage.extend_volume.side_effect = Exception( - "Volume busy" + "Volume busy", ) block_storage_tools = BlockStorageTools() @@ -643,4 +682,4 @@ def test_all_block_storage_methods_have_docstrings(self): ) assert len(docstring.strip()) > 0, ( f"{method_name} docstring should not be empty" - ) \ No newline at end of file + ) diff --git a/tests/tools/test_compute_tools.py b/tests/tools/test_compute_tools.py index b36ac96..f21f17f 100644 --- a/tests/tools/test_compute_tools.py +++ b/tests/tools/test_compute_tools.py @@ -142,7 +142,7 @@ def test_get_server_success(self, mock_get_openstack_conn): compute_tools = ComputeTools() result = compute_tools.get_server( - "fe4b6b9b-090c-4dee-ab27-5155476e8e7d" + "fe4b6b9b-090c-4dee-ab27-5155476e8e7d", ) expected_output = Server( @@ -152,7 +152,7 @@ def test_get_server_success(self, mock_get_openstack_conn): ) assert result == expected_output mock_conn.compute.get_server.assert_called_once_with( - "fe4b6b9b-090c-4dee-ab27-5155476e8e7d" + "fe4b6b9b-090c-4dee-ab27-5155476e8e7d", ) def test_create_server_success(self, mock_get_openstack_conn): diff --git a/tests/tools/test_identity_tools.py b/tests/tools/test_identity_tools.py index 3b81e1e..68971ea 100644 --- a/tests/tools/test_identity_tools.py +++ b/tests/tools/test_identity_tools.py @@ -5,7 +5,7 @@ from openstack import exceptions from openstack_mcp_server.tools.identity_tools import IdentityTools -from openstack_mcp_server.tools.response.identity import Region, Domain +from openstack_mcp_server.tools.response.identity import Domain, Region class TestIdentityTools: @@ -335,7 +335,7 @@ def test_get_region_not_found(self, mock_get_openstack_conn_identity): mock_conn.identity.get_region.assert_called_once_with( region="RegionOne", ) - + def test_get_domains_success(self, mock_get_openstack_conn_identity): """Test getting identity domains successfully.""" mock_conn = mock_get_openstack_conn_identity @@ -352,18 +352,28 @@ def test_get_domains_success(self, mock_get_openstack_conn_identity): mock_domain2.name = "DomainTwo" mock_domain2.description = "Domain Two description" mock_domain2.is_enabled = False - + # Configure mock domain.domains() mock_conn.identity.domains.return_value = [mock_domain1, mock_domain2] # Test get_domains() identity_tools = self.get_identity_tools() result = identity_tools.get_domains() - + # Verify results assert result == [ - Domain(id="domainone", name="DomainOne", description="Domain One description", is_enabled=True), - Domain(id="domaintwo", name="DomainTwo", description="Domain Two description", is_enabled=False), + Domain( + id="domainone", + name="DomainOne", + description="Domain One description", + is_enabled=True, + ), + Domain( + id="domaintwo", + name="DomainTwo", + description="Domain Two description", + is_enabled=False, + ), ] # Verify mock calls @@ -372,17 +382,17 @@ def test_get_domains_success(self, mock_get_openstack_conn_identity): def test_get_domains_empty_list(self, mock_get_openstack_conn_identity): """Test getting identity domains when there are no domains.""" mock_conn = mock_get_openstack_conn_identity - + # Empty domain list mock_conn.identity.domains.return_value = [] # Test get_domains() identity_tools = self.get_identity_tools() result = identity_tools.get_domains() - + # Verify results assert result == [] - + # Verify mock calls mock_conn.identity.domains.assert_called_once() @@ -396,35 +406,49 @@ def test_get_domain_success(self, mock_get_openstack_conn_identity): mock_domain.name = "DomainOne" mock_domain.description = "Domain One description" mock_domain.is_enabled = True - + # Configure mock domain.get_domain() mock_conn.identity.get_domain.return_value = mock_domain - + # Test get_domain() identity_tools = self.get_identity_tools() result = identity_tools.get_domain(id="domainone") - + # Verify results - assert result == Domain(id="domainone", name="DomainOne", description="Domain One description", is_enabled=True) - + assert result == Domain( + id="domainone", + name="DomainOne", + description="Domain One description", + is_enabled=True, + ) + # Verify mock calls - mock_conn.identity.get_domain.assert_called_once_with(domain="domainone") - + mock_conn.identity.get_domain.assert_called_once_with( + domain="domainone", + ) + def test_get_domain_not_found(self, mock_get_openstack_conn_identity): """Test getting a identity domain that does not exist.""" mock_conn = mock_get_openstack_conn_identity # Configure mock to raise NotFoundException - mock_conn.identity.get_domain.side_effect = exceptions.NotFoundException( - "Domain 'domainone' not found", + mock_conn.identity.get_domain.side_effect = ( + exceptions.NotFoundException( + "Domain 'domainone' not found", + ) ) - + # Test get_domain() identity_tools = self.get_identity_tools() - + # Verify exception is raised - with pytest.raises(exceptions.NotFoundException, match="Domain 'domainone' not found"): + with pytest.raises( + exceptions.NotFoundException, + match="Domain 'domainone' not found", + ): identity_tools.get_domain(id="domainone") - + # Verify mock calls - mock_conn.identity.get_domain.assert_called_once_with(domain="domainone") + mock_conn.identity.get_domain.assert_called_once_with( + domain="domainone", + ) From 949a79c0a4b0570914537bb8bcea9ca559acad04 Mon Sep 17 00:00:00 2001 From: Seungju Baek Date: Wed, 13 Aug 2025 11:51:18 +0900 Subject: [PATCH 2/2] chore : ruff failure fix --- src/openstack_mcp_server/tools/response/block_storage.py | 2 +- src/openstack_mcp_server/tools/response/identity.py | 1 + tests/conftest.py | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/openstack_mcp_server/tools/response/block_storage.py b/src/openstack_mcp_server/tools/response/block_storage.py index ccd9f24..d4f8be3 100644 --- a/src/openstack_mcp_server/tools/response/block_storage.py +++ b/src/openstack_mcp_server/tools/response/block_storage.py @@ -18,4 +18,4 @@ class Volume(BaseModel): is_bootable: bool | None = None is_encrypted: bool | None = None description: str | None = None - attachments: list[VolumeAttachment] = [] \ No newline at end of file + attachments: list[VolumeAttachment] = [] diff --git a/src/openstack_mcp_server/tools/response/identity.py b/src/openstack_mcp_server/tools/response/identity.py index 44620ae..2e6bd08 100644 --- a/src/openstack_mcp_server/tools/response/identity.py +++ b/src/openstack_mcp_server/tools/response/identity.py @@ -7,6 +7,7 @@ class Region(BaseModel): id: str description: str = "" + class Domain(BaseModel): id: str name: str diff --git a/tests/conftest.py b/tests/conftest.py index 7e89a6d..0979358 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -62,6 +62,7 @@ def mock_openstack_connect_network(): ): yield mock_conn + @pytest.fixture def mock_get_openstack_conn_block_storage(): """Mock get_openstack_conn function for block_storage_tools."""