From 1e97442d61625218c26fcf7f924e09d5677fbd86 Mon Sep 17 00:00:00 2001 From: John Griffith Date: Wed, 15 Nov 2017 17:56:39 +0000 Subject: [PATCH] Fix cinder-manage volume delete The cinder-manage volume delete cmd has been broken for a while. It looks like the code in manage was written against an original rpc version that is no longer valid and never updated. This updates it to use the current rpc method. Change-Id: Ia7ef04c777ccf6214fec53f500bfba4dde341c73 Close-Bug: #1732223 --- cinder/cmd/manage.py | 21 ++++-------------- cinder/tests/unit/test_cmd.py | 40 ++++++++++++----------------------- 2 files changed, 17 insertions(+), 44 deletions(-) diff --git a/cinder/cmd/manage.py b/cinder/cmd/manage.py index 3063121077e..c6880ab8909 100644 --- a/cinder/cmd/manage.py +++ b/cinder/cmd/manage.py @@ -65,12 +65,10 @@ from oslo_config import cfg from oslo_db import exception as db_exc from oslo_db.sqlalchemy import migration from oslo_log import log as logging -import oslo_messaging as messaging from oslo_utils import timeutils # Need to register global_opts from cinder.common import config # noqa -from cinder.common import constants from cinder import context from cinder import db from cinder.db import migration as db_migration @@ -80,6 +78,7 @@ from cinder.i18n import _ from cinder import objects from cinder import rpc from cinder import version +from cinder.volume import rpcapi as volume_rpcapi from cinder.volume import utils as vutils @@ -335,19 +334,6 @@ class VersionCommands(object): class VolumeCommands(object): """Methods for dealing with a cloud in an odd state.""" - def __init__(self): - self._client = None - - def _rpc_client(self): - if self._client is None: - if not rpc.initialized(): - rpc.init(CONF) - target = messaging.Target(topic=constants.VOLUME_TOPIC) - serializer = objects.base.CinderObjectSerializer() - self._client = rpc.get_client(target, serializer=serializer) - - return self._client - @args('volume_id', help='Volume ID to be deleted') def delete(self, volume_id): @@ -367,8 +353,9 @@ class VolumeCommands(object): print(_("Detach volume from instance and then try again.")) return - cctxt = self._rpc_client().prepare(server=host) - cctxt.cast(ctxt, "delete_volume", volume_id=volume.id, volume=volume) + rpc.init(CONF) + rpcapi = volume_rpcapi.VolumeAPI() + rpcapi.delete_volume(ctxt, volume) @args('--currenthost', required=True, help='Existing volume host name') @args('--newhost', required=True, help='New volume host name') diff --git a/cinder/tests/unit/test_cmd.py b/cinder/tests/unit/test_cmd.py index becadeca80b..4276bada0ea 100644 --- a/cinder/tests/unit/test_cmd.py +++ b/cinder/tests/unit/test_cmd.py @@ -37,7 +37,6 @@ from cinder.cmd import rtstool as cinder_rtstool from cinder.cmd import scheduler as cinder_scheduler from cinder.cmd import volume as cinder_volume from cinder.cmd import volume_usage_audit -from cinder.common import constants from cinder import context from cinder import exception from cinder.objects import fields @@ -78,8 +77,9 @@ class TestCinderApiCmd(test.TestCase): rpc_init.assert_called_once_with(CONF) process_launcher.assert_called_once_with() wsgi_service.assert_called_once_with('osapi_volume') - launcher.launch_service.assert_called_once_with(server, - workers=server.workers) + launcher.launch_service.assert_called_once_with( + server, + workers=server.workers) launcher.wait.assert_called_once_with() @@ -379,26 +379,6 @@ class TestCinderManageCmd(test.TestCase): service_get_all.assert_called_once_with(mock.sentinel.ctxt) self.assertEqual(expected_out, fake_out.getvalue()) - @mock.patch('cinder.objects.base.CinderObjectSerializer') - @mock.patch('cinder.rpc.get_client') - @mock.patch('cinder.rpc.init') - @mock.patch('cinder.rpc.initialized', return_value=False) - @mock.patch('oslo_messaging.Target') - def test_volume_commands_init(self, messaging_target, rpc_initialized, - rpc_init, get_client, object_serializer): - mock_target = messaging_target.return_value - mock_rpc_client = get_client.return_value - - volume_cmds = cinder_manage.VolumeCommands() - rpc_client = volume_cmds._rpc_client() - - rpc_initialized.assert_called_once_with() - rpc_init.assert_called_once_with(CONF) - messaging_target.assert_called_once_with(topic=constants.VOLUME_TOPIC) - get_client.assert_called_once_with(mock_target, - serializer=object_serializer()) - self.assertEqual(mock_rpc_client, rpc_client) - @mock.patch('cinder.db.sqlalchemy.api.volume_get') @mock.patch('cinder.context.get_admin_context') @mock.patch('cinder.rpc.get_client') @@ -423,10 +403,16 @@ class TestCinderManageCmd(test.TestCase): volume_cmds.delete(volume_id) volume_get.assert_called_once_with(ctxt, volume_id) - mock_client.prepare.assert_called_once_with(server=host) - cctxt.cast.assert_called_once_with(ctxt, 'delete_volume', - volume_id=volume['id'], - volume=volume_obj) + mock_client.prepare.assert_called_once_with( + server="fake", + topic="cinder-volume.fake@host", + version="3.0") + + cctxt.cast.assert_called_once_with( + ctxt, 'delete_volume', + cascade=False, + unmanage_only=False, + volume=volume_obj) @mock.patch('cinder.db.volume_destroy') @mock.patch('cinder.db.sqlalchemy.api.volume_get')