Skip to content

Commit fc42f12

Browse files
stephenfinstephenfin
committed
volume: Migrate 'snapshot delete' to SDK
Change-Id: Iba89d521ec17a642c5905b0cff908b5a4a9dafd0 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
1 parent e1ff450 commit fc42f12

File tree

4 files changed

+124
-98
lines changed

4 files changed

+124
-98
lines changed

‎openstackclient/tests/unit/volume/v2/test_volume_snapshot.py‎

Lines changed: 51 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@
1313

1414
from unittest import mock
1515

16+
from openstack.block_storage.v2 import snapshot as _snapshot
17+
from openstack import exceptions as sdk_exceptions
18+
from openstack.test import fakes as sdk_fakes
1619
from osc_lib.cli import format_columns
1720
from osc_lib import exceptions
18-
from osc_lib import utils
1921

2022
from openstackclient.tests.unit.identity.v3 import fakes as project_fakes
2123
from openstackclient.tests.unit import utils as test_utils
@@ -184,16 +186,16 @@ def test_snapshot_create_with_remote_source(self):
184186
self.assertEqual(self.data, data)
185187

186188

187-
class TestVolumeSnapshotDelete(TestVolumeSnapshot):
188-
snapshots = volume_fakes.create_snapshots(count=2)
189-
189+
class TestVolumeSnapshotDelete(volume_fakes.TestVolume):
190190
def setUp(self):
191191
super().setUp()
192192

193-
self.snapshots_mock.get = volume_fakes.get_snapshots(self.snapshots)
194-
self.snapshots_mock.delete.return_value = None
193+
self.snapshots = list(
194+
sdk_fakes.generate_fake_resources(_snapshot.Snapshot)
195+
)
196+
self.volume_sdk_client.find_snapshot.side_effect = self.snapshots
197+
self.volume_sdk_client.delete_snapshot.return_value = None
195198

196-
# Get the command object to mock
197199
self.cmd = volume_snapshot.DeleteVolumeSnapshot(self.app, None)
198200

199201
def test_snapshot_delete(self):
@@ -202,23 +204,29 @@ def test_snapshot_delete(self):
202204
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
203205

204206
result = self.cmd.take_action(parsed_args)
207+
self.assertIsNone(result)
205208

206-
self.snapshots_mock.delete.assert_called_with(
207-
self.snapshots[0].id, False
209+
self.volume_sdk_client.find_snapshot.assert_called_once_with(
210+
self.snapshots[0].id, ignore_missing=False
211+
)
212+
self.volume_sdk_client.delete_snapshot.assert_called_once_with(
213+
self.snapshots[0].id, force=False
208214
)
209-
self.assertIsNone(result)
210215

211216
def test_snapshot_delete_with_force(self):
212217
arglist = ['--force', self.snapshots[0].id]
213218
verifylist = [('force', True), ("snapshots", [self.snapshots[0].id])]
214219
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
215220

216221
result = self.cmd.take_action(parsed_args)
222+
self.assertIsNone(result)
217223

218-
self.snapshots_mock.delete.assert_called_with(
219-
self.snapshots[0].id, True
224+
self.volume_sdk_client.find_snapshot.assert_called_once_with(
225+
self.snapshots[0].id, ignore_missing=False
226+
)
227+
self.volume_sdk_client.delete_snapshot.assert_called_once_with(
228+
self.snapshots[0].id, force=True
220229
)
221-
self.assertIsNone(result)
222230

223231
def test_delete_multiple_snapshots(self):
224232
arglist = []
@@ -227,17 +235,24 @@ def test_delete_multiple_snapshots(self):
227235
verifylist = [
228236
('snapshots', arglist),
229237
]
230-
231238
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
232-
result = self.cmd.take_action(parsed_args)
233239

234-
calls = []
235-
for s in self.snapshots:
236-
calls.append(mock.call(s.id, False))
237-
self.snapshots_mock.delete.assert_has_calls(calls)
240+
result = self.cmd.take_action(parsed_args)
238241
self.assertIsNone(result)
239242

243+
self.volume_sdk_client.find_snapshot.assert_has_calls(
244+
[mock.call(x.id, ignore_missing=False) for x in self.snapshots]
245+
)
246+
self.volume_sdk_client.delete_snapshot.assert_has_calls(
247+
[mock.call(x.id, force=False) for x in self.snapshots]
248+
)
249+
240250
def test_delete_multiple_snapshots_with_exception(self):
251+
self.volume_sdk_client.find_snapshot.side_effect = [
252+
self.snapshots[0],
253+
sdk_exceptions.NotFoundException(),
254+
]
255+
241256
arglist = [
242257
self.snapshots[0].id,
243258
'unexist_snapshot',
@@ -248,25 +263,24 @@ def test_delete_multiple_snapshots_with_exception(self):
248263

249264
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
250265

251-
find_mock_result = [self.snapshots[0], exceptions.CommandError]
252-
with mock.patch.object(
253-
utils, 'find_resource', side_effect=find_mock_result
254-
) as find_mock:
255-
try:
256-
self.cmd.take_action(parsed_args)
257-
self.fail('CommandError should be raised.')
258-
except exceptions.CommandError as e:
259-
self.assertEqual('1 of 2 snapshots failed to delete.', str(e))
260-
261-
find_mock.assert_any_call(
262-
self.snapshots_mock, self.snapshots[0].id
263-
)
264-
find_mock.assert_any_call(self.snapshots_mock, 'unexist_snapshot')
266+
exc = self.assertRaises(
267+
exceptions.CommandError,
268+
self.cmd.take_action,
269+
parsed_args,
270+
)
271+
self.assertEqual('1 of 2 snapshots failed to delete.', str(exc))
265272

266-
self.assertEqual(2, find_mock.call_count)
267-
self.snapshots_mock.delete.assert_called_once_with(
268-
self.snapshots[0].id, False
269-
)
273+
self.volume_sdk_client.find_snapshot.assert_has_calls(
274+
[
275+
mock.call(self.snapshots[0].id, ignore_missing=False),
276+
mock.call('unexist_snapshot', ignore_missing=False),
277+
]
278+
)
279+
self.volume_sdk_client.delete_snapshot.assert_has_calls(
280+
[
281+
mock.call(self.snapshots[0].id, force=False),
282+
]
283+
)
270284

271285

272286
class TestVolumeSnapshotList(TestVolumeSnapshot):

‎openstackclient/tests/unit/volume/v3/test_volume_snapshot.py‎

Lines changed: 58 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@
1313

1414
from unittest import mock
1515

16+
from openstack.block_storage.v3 import snapshot as _snapshot
17+
from openstack import exceptions as sdk_exceptions
18+
from openstack.test import fakes as sdk_fakes
1619
from osc_lib.cli import format_columns
1720
from osc_lib import exceptions
18-
from osc_lib import utils
1921

2022
from openstackclient.tests.unit.identity.v3 import fakes as project_fakes
2123
from openstackclient.tests.unit import utils as test_utils
@@ -187,16 +189,17 @@ def test_snapshot_create_with_remote_source(self):
187189
self.assertEqual(self.data, data)
188190

189191

190-
class TestVolumeSnapshotDelete(TestVolumeSnapshot):
191-
snapshots = volume_fakes.create_snapshots(count=2)
192-
192+
class TestVolumeSnapshotDelete(volume_fakes_v3.TestVolume):
193193
def setUp(self):
194194
super().setUp()
195195

196-
self.snapshots_mock.get = volume_fakes.get_snapshots(self.snapshots)
197-
self.snapshots_mock.delete.return_value = None
196+
self.snapshots = list(
197+
sdk_fakes.generate_fake_resources(_snapshot.Snapshot)
198+
)
199+
self.volume_sdk_client.find_snapshot.side_effect = self.snapshots
200+
self.volume_sdk_client.delete_snapshot.return_value = None
201+
self.volume_sdk_client.unmanage_snapshot.return_value = None
198202

199-
# Get the command object to mock
200203
self.cmd = volume_snapshot.DeleteVolumeSnapshot(self.app, None)
201204

202205
def test_snapshot_delete(self):
@@ -205,23 +208,29 @@ def test_snapshot_delete(self):
205208
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
206209

207210
result = self.cmd.take_action(parsed_args)
211+
self.assertIsNone(result)
208212

209-
self.snapshots_mock.delete.assert_called_with(
210-
self.snapshots[0].id, False
213+
self.volume_sdk_client.find_snapshot.assert_called_once_with(
214+
self.snapshots[0].id, ignore_missing=False
215+
)
216+
self.volume_sdk_client.delete_snapshot.assert_called_once_with(
217+
self.snapshots[0].id, force=False
211218
)
212-
self.assertIsNone(result)
213219

214220
def test_snapshot_delete_with_force(self):
215221
arglist = ['--force', self.snapshots[0].id]
216222
verifylist = [('force', True), ("snapshots", [self.snapshots[0].id])]
217223
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
218224

219225
result = self.cmd.take_action(parsed_args)
226+
self.assertIsNone(result)
220227

221-
self.snapshots_mock.delete.assert_called_with(
222-
self.snapshots[0].id, True
228+
self.volume_sdk_client.find_snapshot.assert_called_once_with(
229+
self.snapshots[0].id, ignore_missing=False
230+
)
231+
self.volume_sdk_client.delete_snapshot.assert_called_once_with(
232+
self.snapshots[0].id, force=True
223233
)
224-
self.assertIsNone(result)
225234

226235
def test_delete_multiple_snapshots(self):
227236
arglist = []
@@ -230,17 +239,24 @@ def test_delete_multiple_snapshots(self):
230239
verifylist = [
231240
('snapshots', arglist),
232241
]
233-
234242
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
235-
result = self.cmd.take_action(parsed_args)
236243

237-
calls = []
238-
for s in self.snapshots:
239-
calls.append(mock.call(s.id, False))
240-
self.snapshots_mock.delete.assert_has_calls(calls)
244+
result = self.cmd.take_action(parsed_args)
241245
self.assertIsNone(result)
242246

247+
self.volume_sdk_client.find_snapshot.assert_has_calls(
248+
[mock.call(x.id, ignore_missing=False) for x in self.snapshots]
249+
)
250+
self.volume_sdk_client.delete_snapshot.assert_has_calls(
251+
[mock.call(x.id, force=False) for x in self.snapshots]
252+
)
253+
243254
def test_delete_multiple_snapshots_with_exception(self):
255+
self.volume_sdk_client.find_snapshot.side_effect = [
256+
self.snapshots[0],
257+
sdk_exceptions.NotFoundException(),
258+
]
259+
244260
arglist = [
245261
self.snapshots[0].id,
246262
'unexist_snapshot',
@@ -251,37 +267,36 @@ def test_delete_multiple_snapshots_with_exception(self):
251267

252268
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
253269

254-
find_mock_result = [self.snapshots[0], exceptions.CommandError]
255-
with mock.patch.object(
256-
utils, 'find_resource', side_effect=find_mock_result
257-
) as find_mock:
258-
try:
259-
self.cmd.take_action(parsed_args)
260-
self.fail('CommandError should be raised.')
261-
except exceptions.CommandError as e:
262-
self.assertEqual('1 of 2 snapshots failed to delete.', str(e))
263-
264-
find_mock.assert_any_call(
265-
self.snapshots_mock, self.snapshots[0].id
266-
)
267-
find_mock.assert_any_call(self.snapshots_mock, 'unexist_snapshot')
270+
exc = self.assertRaises(
271+
exceptions.CommandError,
272+
self.cmd.take_action,
273+
parsed_args,
274+
)
275+
self.assertEqual('1 of 2 snapshots failed to delete.', str(exc))
268276

269-
self.assertEqual(2, find_mock.call_count)
270-
self.snapshots_mock.delete.assert_called_once_with(
271-
self.snapshots[0].id, False
272-
)
277+
self.volume_sdk_client.find_snapshot.assert_has_calls(
278+
[
279+
mock.call(self.snapshots[0].id, ignore_missing=False),
280+
mock.call('unexist_snapshot', ignore_missing=False),
281+
]
282+
)
283+
self.volume_sdk_client.delete_snapshot.assert_has_calls(
284+
[
285+
mock.call(self.snapshots[0].id, force=False),
286+
]
287+
)
273288

274289
def test_snapshot_delete_remote(self):
275290
arglist = ['--remote', self.snapshots[0].id]
276291
verifylist = [('remote', True), ("snapshots", [self.snapshots[0].id])]
277292
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
278293

279294
result = self.cmd.take_action(parsed_args)
295+
self.assertIsNone(result)
280296

281297
self.volume_sdk_client.unmanage_snapshot.assert_called_with(
282298
self.snapshots[0].id
283299
)
284-
self.assertIsNone(result)
285300

286301
def test_snapshot_delete_with_remote_force(self):
287302
arglist = ['--remote', '--force', self.snapshots[0].id]
@@ -305,16 +320,15 @@ def test_delete_multiple_snapshots_remote(self):
305320
for s in self.snapshots:
306321
arglist.append(s.id)
307322
verifylist = [('remote', True), ('snapshots', arglist[1:])]
308-
309323
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
310-
result = self.cmd.take_action(parsed_args)
311324

312-
calls = []
313-
for s in self.snapshots:
314-
calls.append(mock.call(s.id))
315-
self.volume_sdk_client.unmanage_snapshot.assert_has_calls(calls)
325+
result = self.cmd.take_action(parsed_args)
316326
self.assertIsNone(result)
317327

328+
self.volume_sdk_client.unmanage_snapshot.assert_has_calls(
329+
[mock.call(s.id) for s in self.snapshots]
330+
)
331+
318332

319333
class TestVolumeSnapshotList(TestVolumeSnapshot):
320334
volume = volume_fakes.create_one_volume()

‎openstackclient/volume/v2/volume_snapshot.py‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -175,16 +175,16 @@ def get_parser(self, prog_name):
175175
return parser
176176

177177
def take_action(self, parsed_args):
178-
volume_client = self.app.client_manager.volume
178+
volume_client = self.app.client_manager.sdk_connection.volume
179179
result = 0
180180

181-
for i in parsed_args.snapshots:
181+
for snapshot in parsed_args.snapshots:
182182
try:
183-
snapshot_id = utils.find_resource(
184-
volume_client.volume_snapshots, i
183+
snapshot_id = volume_client.find_snapshot(
184+
snapshot, ignore_missing=False
185185
).id
186-
volume_client.volume_snapshots.delete(
187-
snapshot_id, parsed_args.force
186+
volume_client.delete_snapshot(
187+
snapshot_id, force=parsed_args.force
188188
)
189189
except Exception as e:
190190
result += 1
@@ -193,7 +193,7 @@ def take_action(self, parsed_args):
193193
"Failed to delete snapshot with "
194194
"name or ID '%(snapshot)s': %(e)s"
195195
)
196-
% {'snapshot': i, 'e': e}
196+
% {'snapshot': snapshot, 'e': e}
197197
)
198198

199199
if result > 0:

0 commit comments

Comments
 (0)