Code optimization for detach volume

There's no need to check attachment_id, the meth delete_server_volume
is only support  volume.id, and the delete_server_volume will do
compatible processing.

Implementation reference
https://review.openstack.org/#/c/480826/

Change-Id: Id1cd37a4c69f57f2827411b3440aa45c8a187ae5
This commit is contained in:
zhangdaolong 2017-07-14 14:12:39 +08:00 committed by Boris Pavlovic
parent 78cbfb2e43
commit a650b3d822
6 changed files with 30 additions and 30 deletions

View File

@ -442,8 +442,8 @@ class CreateAndAttachVolume(cinder_utils.CinderBasic,
server = self._boot_server(image, flavor, **create_vm_params)
volume = self.cinder.create_volume(size, **create_volume_params)
attachment = self._attach_volume(server, volume)
self._detach_volume(server, volume, attachment)
self._attach_volume(server, volume)
self._detach_volume(server, volume)
self.cinder.delete_volume(volume)
self._delete_server(server)
@ -480,8 +480,8 @@ class CreateSnapshotAndAttachVolume(cinder_utils.CinderBasic,
server = self.get_random_server()
attachment = self._attach_volume(server, volume)
self._detach_volume(server, volume, attachment)
self._attach_volume(server, volume)
self._detach_volume(server, volume)
self.cinder.delete_snapshot(snapshot)
self.cinder.delete_volume(volume)
@ -553,7 +553,7 @@ class CreateNestedSnapshotsAndAttachVolume(cinder_utils.CinderBasic,
nes_objs.reverse()
for server, volume, snapshot, attachment in nes_objs:
self._detach_volume(server, volume, attachment)
self._detach_volume(server, volume)
self.cinder.delete_snapshot(snapshot)
self.cinder.delete_volume(volume)

View File

@ -477,7 +477,7 @@ class BootServerAttachCreatedVolumeAndResize(utils.NovaScenario,
server = self._boot_server(image, flavor, **boot_server_kwargs)
volume = self.cinder.create_volume(volume_size, **create_volume_kwargs)
attachment = self._attach_volume(server, volume)
self._attach_volume(server, volume)
self.sleep_between(min_sleep, max_sleep)
self._resize(server, to_flavor)
@ -487,7 +487,7 @@ class BootServerAttachCreatedVolumeAndResize(utils.NovaScenario,
self._resize_revert(server)
if do_delete:
self._detach_volume(server, volume, attachment)
self._detach_volume(server, volume)
self.cinder.delete_volume(volume)
self._delete_server(server, force=force_delete)
@ -828,7 +828,7 @@ class BootServerAttachCreatedVolumeAndLiveMigrate(utils.NovaScenario,
server = self._boot_server(image, flavor, **boot_server_kwargs)
volume = self.cinder.create_volume(size, **create_volume_kwargs)
attachment = self._attach_volume(server, volume)
self._attach_volume(server, volume)
self.sleep_between(min_sleep, max_sleep)
@ -836,7 +836,7 @@ class BootServerAttachCreatedVolumeAndLiveMigrate(utils.NovaScenario,
self._live_migrate(server, new_host,
block_migration, disk_over_commit)
self._detach_volume(server, volume, attachment)
self._detach_volume(server, volume)
self.cinder.delete_volume(volume)
self._delete_server(server)

View File

@ -714,13 +714,21 @@ class NovaScenario(scenario.OpenStackScenario):
@atomic.action_timer("nova.detach_volume")
def _detach_volume(self, server, volume, attachment=None):
"""Detach volume from the server.
:param server: A server object to detach volume from.
:param volume: A volume object to detach from the server.
:param attachment: DEPRECATED
"""
if attachment:
LOG.warning("An argument `attachment` of `_detach_volume` is "
"deprecated in favor of `volume` argument since "
"Rally 0.10.0")
server_id = server.id
# NOTE(chenhb): Recommend the use of attachment.The use of
# volume.id is retained mainly for backward compatibility.
attachment_id = attachment.id if attachment else volume.id
self.clients("nova").volumes.delete_server_volume(server_id,
attachment_id)
volume.id)
utils.wait_for_status(
volume,
ready_statuses=["available"],

View File

@ -229,8 +229,7 @@ class CinderServersTestCase(test.ScenarioTestCase):
scenario._attach_volume.assert_called_once_with(
fake_server, mock_service.create_volume.return_value)
scenario._detach_volume.assert_called_once_with(
fake_server, mock_service.create_volume.return_value,
scenario._attach_volume.return_value)
fake_server, mock_service.create_volume.return_value)
mock_service.delete_volume.assert_called_once_with(
mock_service.create_volume.return_value)
@ -283,8 +282,7 @@ class CinderServersTestCase(test.ScenarioTestCase):
scenario._attach_volume.assert_called_once_with(
scenario.get_random_server.return_value, volume)
scenario._detach_volume.assert_called_once_with(
scenario.get_random_server.return_value, volume,
scenario._attach_volume.return_value)
scenario.get_random_server.return_value, volume)
mock_service.delete_volume.assert_called_once_with(volume)
@mock.patch("random.choice")
@ -299,7 +297,6 @@ class CinderServersTestCase(test.ScenarioTestCase):
scenario.run(volume_type="type")
fake_volume = mock_service.create_volume.return_value
fake_attach = scenario._attach_volume.return_value
fake_server = scenario.get_random_server.return_value
fake_snapshot = mock_service.create_snapshot.return_value
@ -311,8 +308,7 @@ class CinderServersTestCase(test.ScenarioTestCase):
scenario._attach_volume.assert_called_once_with(fake_server,
fake_volume)
scenario._detach_volume.assert_called_once_with(fake_server,
fake_volume,
fake_attach)
fake_volume)
mock_service.delete_volume.assert_called_once_with(fake_volume)
@mock.patch("random.randint")
@ -343,8 +339,7 @@ class CinderServersTestCase(test.ScenarioTestCase):
mock_service.create_snapshot.return_value)
scenario._detach_volume.assert_called_once_with(
scenario.get_random_server.return_value,
mock_service.create_volume.return_value,
scenario._attach_volume.return_value)
mock_service.create_volume.return_value)
@mock.patch("random.randint")
def test_create_nested_snapshots_and_attach_volume_2(self, mock_randint):
@ -393,7 +388,7 @@ class CinderServersTestCase(test.ScenarioTestCase):
[mock.call(snapshot) for snapshot in fake_snapshots])
scenario._detach_volume.assert_has_calls(
[mock.call(scenario.get_random_server.return_value,
fake_volumes[i], fake_attachs[i])
fake_volumes[i])
for i in range(len(fake_volumes))])
def test_create_volume_backup(self):

View File

@ -550,8 +550,7 @@ class NovaServersTestCase(test.ScenarioTestCase):
scenario._attach_volume.assert_called_once_with(fake_server,
fake_volume)
scenario._detach_volume.assert_called_once_with(fake_server,
fake_volume,
fake_attachment)
fake_volume)
scenario.sleep_between.assert_called_once_with(10, 20)
scenario._resize.assert_called_once_with(fake_server, to_flavor)
@ -562,8 +561,7 @@ class NovaServersTestCase(test.ScenarioTestCase):
if do_delete:
scenario._detach_volume.assert_called_once_with(fake_server,
fake_volume,
fake_attachment)
fake_volume)
cinder.delete_volume.assert_called_once_with(fake_volume)
scenario._delete_server.assert_called_once_with(fake_server,
force=False)
@ -805,8 +803,7 @@ class NovaServersTestCase(test.ScenarioTestCase):
scenario._attach_volume.assert_called_once_with(fake_server,
fake_volume)
scenario._detach_volume.assert_called_once_with(fake_server,
fake_volume,
fake_attachment)
fake_volume)
scenario.sleep_between.assert_called_once_with(10, 20)
scenario._live_migrate.assert_called_once_with(fake_server,
"host_name",

View File

@ -635,7 +635,7 @@ class NovaScenarioTestCase(test.ScenarioTestCase):
nova_scenario = utils.NovaScenario(context=self.context)
nova_scenario._detach_volume(self.server, self.volume, attach)
(self.clients("nova").volumes.delete_server_volume
.assert_called_once_with(self.server.id, attach.id))
.assert_called_once_with(self.server.id, self.volume.id))
self._test_atomic_action_timer(nova_scenario.atomic_actions(),
"nova.detach_volume")