Skip to content

Fix the block and file storage detail. #1232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
FernandoOjeda wants to merge 2 commits into from
Closed

Fix the block and file storage detail. #1232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4007d75
Fix the block and file storage detail.
Feb 20, 2020
b07e0ec
Add docstring for block and file new method.
Feb 20, 2020
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
21 changes: 20 additions & 1 deletion SoftLayer/CLI/block/detail.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,35 @@
import SoftLayer
from SoftLayer.CLI import environment
from SoftLayer.CLI import formatting
from SoftLayer.CLI import helpers
from SoftLayer import utils


def get_block_volume_id(volume_id, block_manager):
Copy link
Member

@allmightyspiff allmightyspiff Feb 21, 2020

Choose a reason for hiding this comment

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

lets move this to the block and file managers as _get_ids_from_username(username)

Then add it as a resolver to the manager itself.

class BlockStorageManager(utils.IdentifierMixin, object):
    def __init__(self, client):
        self.configuration = {}
        self.client = client
        self.resolvers = [self._get_ids_from_username]

"""Returns the volume id.
:param volume_id: ID of volume.
:param block_manager: Block Storage Manager.
:return: Returns the volume id.
"""
storage_list = block_manager.list_block_volumes()
Copy link
Member

@allmightyspiff allmightyspiff Feb 21, 2020

Choose a reason for hiding this comment

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

list_block_volumes() takes in a username parameter to filter by that, which will let you only get the volume back that matches the username. Also pass in a mask of just id help reduce API call returned data.

for storage in storage_list:
if volume_id == storage['username']:
volume_id = storage['id']
break

return volume_id


@click.command()
@click.argument('volume_id')
@environment.pass_env
def cli(env, volume_id):
"""Display details for a specified volume."""
block_manager = SoftLayer.BlockStorageManager(env.client)
block_volume = block_manager.get_block_volume_details(volume_id)
volume_id = get_block_volume_id(volume_id, block_manager)
Copy link
Member

@allmightyspiff allmightyspiff Feb 21, 2020

Choose a reason for hiding this comment

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

since were moving get_block_volume_id to the block manager's resolvers feature, we can just remove this line and the rest should work.

block_volume_id = helpers.resolve_id(block_manager.resolve_ids, volume_id, 'Block Volume')
block_volume = block_manager.get_block_volume_details(block_volume_id)
block_volume = utils.NestedDict(block_volume)

table = formatting.KeyValueTable(['Name', 'Value'])
Expand Down
21 changes: 20 additions & 1 deletion SoftLayer/CLI/file/detail.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,35 @@
import SoftLayer
from SoftLayer.CLI import environment
from SoftLayer.CLI import formatting
from SoftLayer.CLI import helpers
from SoftLayer import utils


def get_file_volume_id(volume_id, file_manager):
"""Returns the volume id.

:param volume_id: ID of volume.
:param block_manager: Block Storage Manager.
:return: Returns the volume id.
"""
storage_list = file_manager.list_file_volumes()
for storage in storage_list:
if volume_id == storage['username']:
volume_id = storage['id']
break

return volume_id


@click.command()
@click.argument('volume_id')
@environment.pass_env
def cli(env, volume_id):
"""Display details for a specified volume."""
file_manager = SoftLayer.FileStorageManager(env.client)
file_volume = file_manager.get_file_volume_details(volume_id)
volume_id = get_file_volume_id(volume_id, file_manager)
file_volume_id = helpers.resolve_id(file_manager.resolve_ids, volume_id, 'File Storage')
file_volume = file_manager.get_file_volume_details(file_volume_id)
file_volume = utils.NestedDict(file_volume)

table = formatting.KeyValueTable(['Name', 'Value'])
Expand Down
43 changes: 43 additions & 0 deletions tests/CLI/modules/block_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,49 @@ def test_volume_detail(self):
]
}, json.loads(result.output))

def test_volume_detail_name_identifier(self):
result = self.run_command(['block', 'volume-detail', 'username'])

self.assert_no_fail(result)
isinstance(json.loads(result.output)['IOPs'], float)
self.assertEqual({
'Username': 'username',
'LUN Id': '2',
'Endurance Tier': 'READHEAVY_TIER',
'IOPs': 1000,
'Snapshot Capacity (GB)': '10',
'Snapshot Used (Bytes)': 1024,
'Capacity (GB)': '20GB',
'Target IP': '10.1.2.3',
'Data Center': 'dal05',
'Type': 'ENDURANCE',
'ID': 100,
'# of Active Transactions': '1',
'Ongoing Transaction': 'This is a buffer time in which the customer may cancel the server',
'Replicant Count': '1',
'Replication Status': 'Replicant Volume Provisioning '
'has completed.',
'Replicant Volumes': [[
{'Replicant ID': 'Volume Name', '1784': 'TEST_REP_1'},
{'Replicant ID': 'Target IP', '1784': '10.3.174.79'},
{'Replicant ID': 'Data Center', '1784': 'wdc01'},
{'Replicant ID': 'Schedule', '1784': 'REPLICATION_HOURLY'},
], [
{'Replicant ID': 'Volume Name', '1785': 'TEST_REP_2'},
{'Replicant ID': 'Target IP', '1785': '10.3.177.84'},
{'Replicant ID': 'Data Center', '1785': 'dal01'},
{'Replicant ID': 'Schedule', '1785': 'REPLICATION_DAILY'},
]],
'Original Volume Properties': [
{'Property': 'Original Volume Size',
'Value': '20'},
{'Property': 'Original Volume Name',
'Value': 'test-original-volume-name'},
{'Property': 'Original Snapshot Name',
'Value': 'test-original-snapshot-name'}
]
}, json.loads(result.output))

def test_volume_list(self):
result = self.run_command(['block', 'volume-list'])

Expand Down
43 changes: 43 additions & 0 deletions tests/CLI/modules/file_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,49 @@ def test_volume_detail(self):
]
}, json.loads(result.output))

def test_volume_detail_name_identifier(self):
result = self.run_command(['file', 'volume-detail', 'user'])

self.assert_no_fail(result)
self.assertEqual({
Copy link
Member

@allmightyspiff allmightyspiff Feb 21, 2020

Choose a reason for hiding this comment

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

We don't really need to test the WHOLE output is equal, that will make this test break anytime we change the fixture.

Instead you can do something like this to test a few fields of the output

    self.assert_called_with('SoftLayer_Network_Storage', 'getObject')
    self.assertIn('username', result.output)
    self.assertIn('READHEAVY_TIER', result.output)

or whatever other strings you want to look for to make sure it called the correct data.

'Username': 'username',
'Used Space': '0B',
'Endurance Tier': 'READHEAVY_TIER',
'IOPs': 1000,
'Mount Address': '127.0.0.1:/TEST',
'Snapshot Capacity (GB)': '10',
'Snapshot Used (Bytes)': 1024,
'Capacity (GB)': '20GB',
'Target IP': '10.1.2.3',
'Data Center': 'dal05',
'Type': 'ENDURANCE',
'ID': 100,
'# of Active Transactions': '1',
'Ongoing Transaction': 'This is a buffer time in which the customer may cancel the server',
'Replicant Count': '1',
'Replication Status': 'Replicant Volume Provisioning '
'has completed.',
'Replicant Volumes': [[
{'Replicant ID': 'Volume Name', '1784': 'TEST_REP_1'},
{'Replicant ID': 'Target IP', '1784': '10.3.174.79'},
{'Replicant ID': 'Data Center', '1784': 'wdc01'},
{'Replicant ID': 'Schedule', '1784': 'REPLICATION_HOURLY'},
], [
{'Replicant ID': 'Volume Name', '1785': 'TEST_REP_2'},
{'Replicant ID': 'Target IP', '1785': '10.3.177.84'},
{'Replicant ID': 'Data Center', '1785': 'dal01'},
{'Replicant ID': 'Schedule', '1785': 'REPLICATION_DAILY'},
]],
'Original Volume Properties': [
{'Property': 'Original Volume Size',
'Value': '20'},
{'Property': 'Original Volume Name',
'Value': 'test-original-volume-name'},
{'Property': 'Original Snapshot Name',
'Value': 'test-original-snapshot-name'}
]
}, json.loads(result.output))

def test_volume_order_performance_iops_not_given(self):
result = self.run_command(['file', 'volume-order',
'--storage-type=performance', '--size=20',
Expand Down