From 0ce533e0e194741b6ade7eed12c107628d0e6d3d Mon Sep 17 00:00:00 2001 From: Craig Vyvial Date: Thu, 24 May 2012 13:16:08 -0500 Subject: fixing issue with db.volume_update not returning the volume_ref fixes bug #1003664 - changed the sqlalchemy db api code to return the volume_ref from the volume_update method. This was causing the volume notifications to have the incorrect information in the payload. - Fixed up the unit tests because they started failing badly. - fixed the volume-usage-audit reading the default config values - fix hacking issue with volume-usage-audit Change-Id: Iba5634b0c351a6cc0c48b697217a6f85533de93e --- bin/volume-usage-audit | 7 +++---- nova/db/sqlalchemy/api.py | 6 +++++- nova/tests/api/ec2/test_cloud.py | 8 +++----- nova/tests/test_volume.py | 4 +++- nova/volume/manager.py | 4 ++-- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/bin/volume-usage-audit b/bin/volume-usage-audit index d8591557c..0329d184e 100755 --- a/bin/volume-usage-audit +++ b/bin/volume-usage-audit @@ -57,15 +57,14 @@ from nova import flags from nova import log as logging from nova import rpc from nova import utils -import nova.volume.utils - +from nova.volume import utils as volume_utils FLAGS = flags.FLAGS if __name__ == '__main__': rpc.register_opts(FLAGS) admin_context = context.get_admin_context() - utils.default_flagfile() + utils.default_cfgfile() flags.FLAGS(sys.argv) logging.setup() begin, end = utils.last_completed_audit_period() @@ -77,7 +76,7 @@ if __name__ == '__main__': print "Found %d volumes" % len(volumes) for volume_ref in volumes: try: - nova.volume.utils.notify_usage_exists( + volume_utils.notify_usage_exists( admin_context, volume_ref) except Exception, e: print traceback.format_exc(e) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 39b26a759..e00e1ef81 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -2848,6 +2848,7 @@ def volume_data_get_for_project(context, project_id, session=None): def volume_destroy(context, volume_id): session = get_session() with session.begin(): + volume_ref = volume_get(context, volume_id, session=session) session.query(models.Volume).\ filter_by(id=volume_id).\ update({'deleted': True, @@ -2861,6 +2862,7 @@ def volume_destroy(context, volume_id): update({'deleted': True, 'deleted_at': utils.utcnow(), 'updated_at': literal_column('updated_at')}) + return volume_ref @require_admin_context @@ -2952,6 +2954,7 @@ def volume_get_iscsi_target_num(context, volume_id): @require_context def volume_update(context, volume_id, values): session = get_session() + volume_ref = volume_get(context, volume_id, session=session) metadata = values.get('metadata') if metadata is not None: volume_metadata_update(context, @@ -2959,10 +2962,11 @@ def volume_update(context, volume_id, values): values.pop('metadata'), delete=True) with session.begin(): - volume_ref = volume_get(context, volume_id, session=session) volume_ref.update(values) volume_ref.save(session=session) + return volume_ref + @require_context def ec2_volume_create(context, volume_uuid, id=None): diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py index 3135364ec..c9080c0e3 100644 --- a/nova/tests/api/ec2/test_cloud.py +++ b/nova/tests/api/ec2/test_cloud.py @@ -623,7 +623,7 @@ class CloudTestCase(test.TestCase): self.assertEqual(result['volumeSet'][0]['availabilityZone'], availabilityZone) - db.volume_destroy(self.context, ec2utils.ec2_id_to_id(volume_id)) + db.volume_destroy(self.context, ec2utils.ec2_vol_id_to_uuid(volume_id)) def test_create_volume_from_snapshot(self): """Makes sure create_volume works when we specify a snapshot.""" @@ -640,7 +640,7 @@ class CloudTestCase(test.TestCase): self.assertEqual(len(result['volumeSet']), 2) self.assertEqual(result['volumeSet'][1]['volumeId'], volume_id) - db.volume_destroy(self.context, ec2utils.ec2_id_to_id(volume_id)) + db.volume_destroy(self.context, ec2utils.ec2_vol_id_to_uuid(volume_id)) db.snapshot_destroy(self.context, snap['id']) db.volume_destroy(self.context, vol['id']) @@ -2083,7 +2083,7 @@ class CloudTestCase(test.TestCase): 'snapshot_id': snapshot2_id, 'delete_on_termination': True}]} ec2_instance_id = self._run_instance(**kwargs) - instance_id = ec2utils.ec2_id_to_id(ec2_instance_id) + instance_id = ec2utils.ec2_vol_id_to_uuid(ec2_instance_id) instance_uuid = ec2utils.ec2_instance_id_to_uuid(self.context, ec2_instance_id) @@ -2122,8 +2122,6 @@ class CloudTestCase(test.TestCase): for snapshot_id in (ec2_snapshot1_id, ec2_snapshot2_id): self.cloud.delete_snapshot(self.context, snapshot_id) - db.volume_destroy(self.context, vol['id']) - def test_create_image(self): """Make sure that CreateImage works""" # enforce periodic tasks run in short time to avoid wait for 60s. diff --git a/nova/tests/test_volume.py b/nova/tests/test_volume.py index f9b54badc..2909b51f7 100644 --- a/nova/tests/test_volume.py +++ b/nova/tests/test_volume.py @@ -395,6 +395,8 @@ class VolumeTestCase(test.TestCase): self.assertEquals(len(test_notifier.NOTIFICATIONS), 2) msg = test_notifier.NOTIFICATIONS[0] self.assertEquals(msg['event_type'], 'volume.create.start') + payload = msg['payload'] + self.assertEquals(payload['status'], 'creating') msg = test_notifier.NOTIFICATIONS[1] self.assertEquals(msg['priority'], 'INFO') self.assertEquals(msg['event_type'], 'volume.create.end') @@ -402,7 +404,7 @@ class VolumeTestCase(test.TestCase): self.assertEquals(payload['tenant_id'], volume['project_id']) self.assertEquals(payload['user_id'], volume['user_id']) self.assertEquals(payload['volume_id'], volume['id']) - self.assertEquals(payload['status'], 'creating') + self.assertEquals(payload['status'], 'available') self.assertEquals(payload['size'], volume['size']) self.assertTrue('display_name' in payload) self.assertTrue('snapshot_id' in payload) diff --git a/nova/volume/manager.py b/nova/volume/manager.py index 5e57a42fb..6ca923bd0 100644 --- a/nova/volume/manager.py +++ b/nova/volume/manager.py @@ -152,7 +152,7 @@ class VolumeManager(manager.SchedulerDependentManager): volume_ref['id'], {'status': 'error'}) now = utils.utcnow() - self.db.volume_update(context, + volume_ref = self.db.volume_update(context, volume_ref['id'], {'status': 'available', 'launched_at': now}) LOG.debug(_("volume %s: created successfully"), volume_ref['name']) @@ -197,7 +197,7 @@ class VolumeManager(manager.SchedulerDependentManager): reservations = None LOG.exception(_("Failed to update usages deleting volume")) - self.db.volume_destroy(context, volume_id) + volume_ref = self.db.volume_destroy(context, volume_id) LOG.debug(_("volume %s: deleted successfully"), volume_ref['name']) self._notify_about_volume_usage(context, volume_ref, "delete.end") -- cgit