Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions src/openstack_mcp_server/tools/block_storage_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def register_tools(self, mcp: FastMCP):
mcp.tool()(self.extend_volume)

mcp.tool()(self.get_attachment_details)
mcp.tool()(self.get_attachments)

def get_volumes(self) -> list[Volume]:
"""
Expand Down Expand Up @@ -225,3 +226,41 @@ def get_attachment_details(self, attachment_id: str) -> Attachment:
}

return Attachment(**params)

def get_attachments(
self,
volume_id: str | None = None,
instance: str | None = None,
) -> list[Attachment]:
"""
Get the list of attachments.

:param volume_id: The ID of the volume.
:param instance: The ID of the instance.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 보기엔 거의 훌륭하게 작성해주셨는데요,

@platanus-kr 님 comment를 보고 생각해본 바로는
docstring에 instance에 대해 instance id 대신 공식문서의 표현을 차용하는것은어떨까요?("volume attachments like nova, glance, ironic etc." 라는 구절이 있으니.. 이러한 것들의 id다 정도로요.)

Copy link
Collaborator Author

@S0okJu S0okJu Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일부 백엔드의 경우에는 instance id가 필요해요.
말씀해주신대로 다양한 백엔드를 지원하려면 백엔드 종류에 해당되는 인자를 추가하는 것이 좋을 것 같아요.
현재로써는 다양한 환경에서 테스트를 못하다보니 해당 기능은 보류하게 되었어요.

관련해서 이슈를 작성하겠습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음..인자를 추가하신다는게 제가 잘 이해가 안가는데요!
instance 인자가 nova뿐만아니라 다양한 block volume의 소비 주체를 포괄하고있다고 저는 이해했는데,
그 말인 즉슨 다양한 백엔드를 커버하기 위해서는 다른 인자를 추가하지 않아도 현재 코드가 이미 완성형이고, 나중에 추가되는 툴마다 테스트만 추가하면 되지않나 하는 의견이었습니다!
@S0okJu 님 의견은 get_attachments가 지금 구현하지 않은 백엔드툴들을 포괄하는 기능 구현이므로 테스트를 충분히 할 수 없으니 보류하겠다는 말씀이신거죠?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jja6312 말씀이 맞습니다.
해당 응답값은 완성형이며, 응답값만으로도 다양한 백엔드를 지원할 수 있습니다.

말씀해주신 것처럼 instacne_id 를 다른 변수 이름으로 바꾼다면 사용자 측면에서 좋을 것 같습니다.
다만, 백엔드에 따라 어떤 형태로 반환되는지 직접 확인하고 변경하고 싶습니다.

@S0okJu 님 의견은 get_attachments가 지금 구현하지 않은 백엔드툴들을 포괄하는 기능 구현이므로 테스트를 충분히 할 수 없으니 보류하겠다는 말씀이신거죠?!

네, 직접 확인한 후에 이 사안에 대해서 반영하겠습니다.

:return: A list of Attachment objects.
"""
conn = get_openstack_conn()

filter = {}
if volume_id:
filter["volume_id"] = volume_id
if instance:
filter["instance"] = instance

attachments = []
for attachment in conn.block_storage.attachments(**filter):
attachments.append(
Attachment(
id=attachment.id,
instance=attachment.instance,
volume_id=attachment.volume_id,
status=attachment.status,
connection_info=attachment.connection_info,
attach_mode=attachment.attach_mode,
connector=attachment.connector,
attached_at=attachment.attached_at,
detached_at=attachment.detached_at,
)
)

return attachments
42 changes: 42 additions & 0 deletions tests/tools/test_block_storage_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -741,3 +741,45 @@ def test_get_attachment_details(
mock_conn.block_storage.get_attachment.assert_called_once_with(
"attach-123"
)

def test_get_attachments(self, mock_get_openstack_conn_block_storage):
"""Test getting attachments."""
mock_conn = mock_get_openstack_conn_block_storage

# Create mock attachment object
mock_attachment = Mock()
mock_attachment.id = "attach-123"
mock_attachment.instance = "server-123"
mock_attachment.volume_id = "vol-123"
mock_attachment.status = "attached"
mock_attachment.connection_info = None
mock_attachment.connector = None
mock_attachment.attach_mode = None
mock_attachment.attached_at = None
mock_attachment.detached_at = None

mock_conn.block_storage.attachments.return_value = [mock_attachment]

# Test attachments
block_storage_tools = BlockStorageTools()

filter = {
"volume_id": "vol-123",
"instance": "server-123",
}
result = block_storage_tools.get_attachments(**filter)

# Verify the result
assert isinstance(result, list)
assert len(result) == 1
assert result[0].id == "attach-123"
assert result[0].instance == "server-123"
assert result[0].volume_id == "vol-123"
assert result[0].attached_at is None
assert result[0].detached_at is None
assert result[0].attach_mode is None
assert result[0].connection_info is None
assert result[0].connector is None

# Verify the mock calls
mock_conn.block_storage.attachments.assert_called_once_with(**filter)