From 7cd33c7bc68cace718ed509bcbf6be98db22b4ab Mon Sep 17 00:00:00 2001 From: chenhb-zte Date: Sat, 13 Aug 2016 17:38:46 +0800 Subject: [PATCH] Use attachment_id instead of volume_id when detaching volume Maybe this change is trivial, however, i think it is accordant with the API. Change-Id: I22896e2d3a4e6aa064f5829e0524645b81c6fbf7 --- .../openstack/scenarios/cinder/volumes.py | 28 +++++++-------- .../openstack/scenarios/nova/servers.py | 8 ++--- .../plugins/openstack/scenarios/nova/utils.py | 15 ++++---- .../scenarios/cinder/test_volumes.py | 34 ++++++++++++------- .../openstack/scenarios/nova/test_servers.py | 15 +++++--- .../openstack/scenarios/nova/test_utils.py | 24 +++++++++++-- 6 files changed, 79 insertions(+), 45 deletions(-) mode change 100644 => 100755 rally/plugins/openstack/scenarios/cinder/volumes.py diff --git a/rally/plugins/openstack/scenarios/cinder/volumes.py b/rally/plugins/openstack/scenarios/cinder/volumes.py old mode 100644 new mode 100755 index 1de023ed..9bb14a4f --- a/rally/plugins/openstack/scenarios/cinder/volumes.py +++ b/rally/plugins/openstack/scenarios/cinder/volumes.py @@ -311,8 +311,8 @@ class CinderVolumes(cinder_utils.CinderScenario, server = self._boot_server(image, flavor, **create_vm_params) volume = self._create_volume(size, **create_volume_params) - self._attach_volume(server, volume) - self._detach_volume(server, volume) + attachment = self._attach_volume(server, volume) + self._detach_volume(server, volume, attachment) self._delete_volume(volume) self._delete_server(server) @@ -354,8 +354,8 @@ class CinderVolumes(cinder_utils.CinderScenario, server = self.get_random_server() - self._attach_volume(server, volume) - self._detach_volume(server, volume) + attachment = self._attach_volume(server, volume) + self._detach_volume(server, volume, attachment) self._delete_snapshot(snapshot) self._delete_volume(volume) @@ -400,29 +400,27 @@ class CinderVolumes(cinder_utils.CinderScenario, size = random.randint(size["min"], size["max"]) create_volume_kwargs = create_volume_kwargs or {} - create_snapshot_kwargs = create_snapshot_kwargs or kwargs or {} + server = self.get_random_server() source_vol = self._create_volume(size, **create_volume_kwargs) - nes_objs = [(self.get_random_server(), source_vol, - self._create_snapshot(source_vol.id, False, - **create_snapshot_kwargs))] - - self._attach_volume(nes_objs[0][0], nes_objs[0][1]) - snapshot = nes_objs[0][2] + snapshot = self._create_snapshot(source_vol.id, False, + **create_snapshot_kwargs) + attachment = self._attach_volume(server, source_vol) + nes_objs = [(server, source_vol, snapshot, attachment)] for i in range(nested_level - 1): volume = self._create_volume(size, snapshot_id=snapshot.id) snapshot = self._create_snapshot(volume.id, False, **create_snapshot_kwargs) server = self.get_random_server() - self._attach_volume(server, volume) + attachment = self._attach_volume(server, volume) - nes_objs.append((server, volume, snapshot)) + nes_objs.append((server, volume, snapshot, attachment)) nes_objs.reverse() - for server, volume, snapshot in nes_objs: - self._detach_volume(server, volume) + for server, volume, snapshot, attachment in nes_objs: + self._detach_volume(server, volume, attachment) self._delete_snapshot(snapshot) self._delete_volume(volume) diff --git a/rally/plugins/openstack/scenarios/nova/servers.py b/rally/plugins/openstack/scenarios/nova/servers.py index 8578d0e1..35b9c5d9 100755 --- a/rally/plugins/openstack/scenarios/nova/servers.py +++ b/rally/plugins/openstack/scenarios/nova/servers.py @@ -480,7 +480,7 @@ class NovaServers(utils.NovaScenario, server = self._boot_server(image, flavor, **boot_server_kwargs) volume = self._create_volume(volume_size, **create_volume_kwargs) - self._attach_volume(server, volume) + attachment = self._attach_volume(server, volume) self.sleep_between(min_sleep, max_sleep) self._resize(server, to_flavor) @@ -490,7 +490,7 @@ class NovaServers(utils.NovaScenario, self._resize_revert(server) if do_delete: - self._detach_volume(server, volume) + self._detach_volume(server, volume, attachment) self._delete_volume(volume) self._delete_server(server, force=force_delete) @@ -747,7 +747,7 @@ class NovaServers(utils.NovaScenario, server = self._boot_server(image, flavor, **boot_server_kwargs) volume = self._create_volume(size, **create_volume_kwargs) - self._attach_volume(server, volume) + attachment = self._attach_volume(server, volume) self.sleep_between(min_sleep, max_sleep) @@ -755,7 +755,7 @@ class NovaServers(utils.NovaScenario, self._live_migrate(server, new_host, block_migration, disk_over_commit) - self._detach_volume(server, volume) + self._detach_volume(server, volume, attachment) self._delete_volume(volume) self._delete_server(server) diff --git a/rally/plugins/openstack/scenarios/nova/utils.py b/rally/plugins/openstack/scenarios/nova/utils.py index b6a4438f..69c32006 100755 --- a/rally/plugins/openstack/scenarios/nova/utils.py +++ b/rally/plugins/openstack/scenarios/nova/utils.py @@ -675,9 +675,8 @@ class NovaScenario(scenario.OpenStackScenario): def _attach_volume(self, server, volume, device=None): server_id = server.id volume_id = volume.id - self.clients("nova").volumes.create_server_volume(server_id, - volume_id, - device) + attachment = self.clients("nova").volumes.create_server_volume( + server_id, volume_id, device) utils.wait_for( volume, ready_statuses=["in-use"], @@ -686,13 +685,17 @@ class NovaScenario(scenario.OpenStackScenario): check_interval=( CONF.benchmark.nova_server_resize_revert_poll_interval) ) + return attachment @atomic.action_timer("nova.detach_volume") - def _detach_volume(self, server, volume): + def _detach_volume(self, server, volume, attachment=None): server_id = server.id - volume_id = volume.id + # NOTE(chenhb): Recommend the use of attachment.The use of + # volume.id is retained mainly for backwoard compatible. + attachment_id = attachment.id if attachment else volume.id + self.clients("nova").volumes.delete_server_volume(server_id, - volume_id) + attachment_id) utils.wait_for( volume, ready_statuses=["available"], diff --git a/tests/unit/plugins/openstack/scenarios/cinder/test_volumes.py b/tests/unit/plugins/openstack/scenarios/cinder/test_volumes.py index 76264614..8d38367d 100755 --- a/tests/unit/plugins/openstack/scenarios/cinder/test_volumes.py +++ b/tests/unit/plugins/openstack/scenarios/cinder/test_volumes.py @@ -188,9 +188,10 @@ class CinderServersTestCase(test.ScenarioTestCase): def test_create_and_attach_volume(self): fake_volume = mock.MagicMock() fake_server = mock.MagicMock() + fake_attach = mock.MagicMock() scenario = volumes.CinderVolumes(self.context) - scenario._attach_volume = mock.MagicMock() + scenario._attach_volume = mock.MagicMock(return_value=fake_attach) scenario._detach_volume = mock.MagicMock() scenario._boot_server = mock.MagicMock(return_value=fake_server) @@ -207,7 +208,8 @@ 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_volume, + fake_attach) scenario._delete_volume.assert_called_once_with(fake_volume) scenario._delete_server.assert_called_once_with(fake_server) @@ -254,9 +256,10 @@ class CinderServersTestCase(test.ScenarioTestCase): fake_volume = mock.MagicMock() fake_snapshot = mock.MagicMock() fake_server = mock.MagicMock() + fake_attach = mock.MagicMock() scenario = volumes.CinderVolumes(self._get_context()) - scenario._attach_volume = mock.MagicMock() + scenario._attach_volume = mock.MagicMock(return_value=fake_attach) scenario._detach_volume = mock.MagicMock() scenario._boot_server = mock.MagicMock(return_value=fake_server) scenario._delete_server = mock.MagicMock() @@ -277,17 +280,19 @@ 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_volume, + fake_attach) scenario._delete_volume.assert_called_once_with(fake_volume) def test_create_snapshot_and_attach_volume_use_volume_type(self): fake_volume = mock.MagicMock() fake_snapshot = mock.MagicMock() fake_server = mock.MagicMock() + fake_attach = mock.MagicMock() scenario = volumes.CinderVolumes(self._get_context()) - scenario._attach_volume = mock.MagicMock() + scenario._attach_volume = mock.MagicMock(return_value=fake_attach) scenario._detach_volume = mock.MagicMock() scenario._boot_server = mock.MagicMock(return_value=fake_server) scenario._delete_server = mock.MagicMock() @@ -315,16 +320,18 @@ 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_volume, + fake_attach) scenario._delete_volume.assert_called_once_with(fake_volume) def test_create_nested_snapshots_and_attach_volume(self): fake_volume = mock.MagicMock() fake_snapshot = mock.MagicMock() + fake_attach = mock.MagicMock() scenario = volumes.CinderVolumes(context=self._get_context()) - scenario._attach_volume = mock.MagicMock() + scenario._attach_volume = mock.MagicMock(return_value=fake_attach) scenario._detach_volume = mock.MagicMock() scenario._delete_server = mock.MagicMock() scenario._create_volume = mock.MagicMock(return_value=fake_volume) @@ -345,10 +352,11 @@ class CinderServersTestCase(test.ScenarioTestCase): def test_create_nested_snapshots_and_attach_volume_kwargs(self): fake_volume = mock.MagicMock() fake_snapshot = mock.MagicMock() + fake_attach = mock.MagicMock() scenario = volumes.CinderVolumes(context=self._get_context()) - scenario._attach_volume = mock.MagicMock() + scenario._attach_volume = mock.MagicMock(return_value=fake_attach) scenario._detach_volume = mock.MagicMock() scenario._delete_server = mock.MagicMock() scenario._create_volume = mock.MagicMock(return_value=fake_volume) @@ -368,10 +376,11 @@ class CinderServersTestCase(test.ScenarioTestCase): fake_volume = mock.MagicMock() fake_volume.id = "FAKE_ID" fake_snapshot = mock.MagicMock() + fake_attach = mock.MagicMock() scenario = volumes.CinderVolumes(context=self._get_context()) - scenario._attach_volume = mock.MagicMock() + scenario._attach_volume = mock.MagicMock(return_value=fake_attach) scenario._detach_volume = mock.MagicMock() scenario._delete_server = mock.MagicMock() scenario._create_volume = mock.MagicMock(return_value=fake_volume) @@ -396,10 +405,11 @@ class CinderServersTestCase(test.ScenarioTestCase): fake_volume = mock.MagicMock() fake_volume.id = "FAKE_ID" fake_snapshot = mock.MagicMock() + fake_attach = mock.MagicMock() scenario = volumes.CinderVolumes(context=self._get_context()) - scenario._attach_volume = mock.MagicMock() + scenario._attach_volume = mock.MagicMock(return_value=fake_attach) scenario._detach_volume = mock.MagicMock() scenario._delete_server = mock.MagicMock() scenario._create_volume = mock.MagicMock(return_value=fake_volume) @@ -428,7 +438,7 @@ class CinderServersTestCase(test.ScenarioTestCase): scenario = volumes.CinderVolumes(self._get_context()) - scenario._attach_volume = mock.MagicMock() + scenario._attach_volume = mock.MagicMock(return_value=mock.MagicMock()) scenario._detach_volume = mock.MagicMock() scenario._delete_server = mock.MagicMock() scenario._create_volume = mock.MagicMock( @@ -458,7 +468,7 @@ class CinderServersTestCase(test.ScenarioTestCase): scenario = volumes.CinderVolumes(self._get_context()) scenario.get_random_server = mock.MagicMock(return_value=fake_server) - scenario._attach_volume = mock.MagicMock() + scenario._attach_volume = mock.MagicMock(return_value=mock.MagicMock()) scenario._detach_volume = mock.MagicMock() scenario._delete_server = mock.MagicMock() scenario._create_volume = mock.MagicMock(return_value=fake_volume) diff --git a/tests/unit/plugins/openstack/scenarios/nova/test_servers.py b/tests/unit/plugins/openstack/scenarios/nova/test_servers.py index 76237359..16a48102 100755 --- a/tests/unit/plugins/openstack/scenarios/nova/test_servers.py +++ b/tests/unit/plugins/openstack/scenarios/nova/test_servers.py @@ -461,12 +461,13 @@ class NovaServersTestCase(test.ScenarioTestCase): fake_server = mock.MagicMock() flavor = mock.MagicMock() to_flavor = mock.MagicMock() + fake_attachment = mock.MagicMock() scenario = servers.NovaServers(self.context) scenario.generate_random_name = mock.MagicMock(return_value="name") scenario._boot_server = mock.MagicMock(return_value=fake_server) scenario._create_volume = mock.MagicMock(return_value=fake_volume) - scenario._attach_volume = mock.MagicMock() + scenario._attach_volume = mock.MagicMock(return_value=fake_attachment) scenario._resize_confirm = mock.MagicMock() scenario._resize_revert = mock.MagicMock() scenario._resize = mock.MagicMock() @@ -485,7 +486,8 @@ 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_volume, + fake_attachment) scenario.sleep_between.assert_called_once_with(10, 20) scenario._resize.assert_called_once_with(fake_server, to_flavor) @@ -496,7 +498,8 @@ class NovaServersTestCase(test.ScenarioTestCase): if do_delete: scenario._detach_volume.assert_called_once_with(fake_server, - fake_volume) + fake_volume, + fake_attachment) scenario._delete_volume.assert_called_once_with(fake_volume) scenario._delete_server.assert_called_once_with(fake_server, force=False) @@ -613,10 +616,11 @@ class NovaServersTestCase(test.ScenarioTestCase): def test_boot_server_attach_created_volume_and_live_migrate(self): fake_volume = mock.MagicMock() fake_server = mock.MagicMock() + fake_attachment = mock.MagicMock() scenario = servers.NovaServers(self.context) - scenario._attach_volume = mock.MagicMock() + scenario._attach_volume = mock.MagicMock(return_value=fake_attachment) scenario._detach_volume = mock.MagicMock() scenario.sleep_between = mock.MagicMock() @@ -643,7 +647,8 @@ 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_volume, + fake_attachment) scenario.sleep_between.assert_called_once_with(10, 20) scenario._live_migrate.assert_called_once_with(fake_server, "host_name", diff --git a/tests/unit/plugins/openstack/scenarios/nova/test_utils.py b/tests/unit/plugins/openstack/scenarios/nova/test_utils.py index a972da53..7e9178a1 100755 --- a/tests/unit/plugins/openstack/scenarios/nova/test_utils.py +++ b/tests/unit/plugins/openstack/scenarios/nova/test_utils.py @@ -595,16 +595,34 @@ class NovaScenarioTestCase(test.ScenarioTestCase): "nova.resize_revert") def test__attach_volume(self): - self.clients("nova").volumes.create_server_volume.return_value = None + expect_attach = mock.MagicMock() + device = None + (self.clients("nova").volumes.create_server_volume + .return_value) = expect_attach nova_scenario = utils.NovaScenario(context=self.context) - nova_scenario._attach_volume(self.server, self.volume) + attach = nova_scenario._attach_volume(self.server, self.volume, device) + (self.clients("nova").volumes.create_server_volume + .assert_called_once_with(self.server.id, self.volume.id, device)) + self.assertEqual(expect_attach, attach) self._test_atomic_action_timer(nova_scenario.atomic_actions(), "nova.attach_volume") def test__detach_volume(self): + attach = mock.MagicMock(id="attach_id") self.clients("nova").volumes.delete_server_volume.return_value = None nova_scenario = utils.NovaScenario(context=self.context) - nova_scenario._detach_volume(self.server, self.volume) + 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)) + self._test_atomic_action_timer(nova_scenario.atomic_actions(), + "nova.detach_volume") + + def test__detach_volume_no_attach(self): + self.clients("nova").volumes.delete_server_volume.return_value = None + nova_scenario = utils.NovaScenario(context=self.context) + nova_scenario._detach_volume(self.server, self.volume, None) + (self.clients("nova").volumes.delete_server_volume + .assert_called_once_with(self.server.id, self.volume.id)) self._test_atomic_action_timer(nova_scenario.atomic_actions(), "nova.detach_volume")