Fix _poll_bandwidth_usage if no network on vif

Bugfix for lp901819

Move DB lookups out of the virt layer for bw usage.
Migrate to storing the mac address in the table.
Only lookup network label when sending usage events.
Attempt to use info_cache for label lookup before
    punting to rpc call.

Change-Id: Iffb4bc220804be7d5fe26d6416b90cd747b72753

13 files changed, 224 insertions, 44 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index bd911ae0d..e5600af32 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -2088,10 +2088,14 @@ class ComputeManager(manager.SchedulerDependentManager):
                 return
 
             for usage in bw_usage:
-                vif = usage['virtual_interface']
+                mac = usage['mac_address']
+                vif = self.network_api.get_vif_by_mac_address(context, mac)
+                if not vif:
+                    continue
+
                 self.db.bw_usage_update(context,
-                                        vif.instance_id,
-                                        vif.network.label,
+                                        vif['instance_id'],
+                                        mac,
                                         start_time,
                                         usage['bw_in'], usage['bw_out'])
 
diff --git a/nova/compute/utils.py b/nova/compute/utils.py
index b8b34fa81..1358160e8 100644
--- a/nova/compute/utils.py
+++ b/nova/compute/utils.py
@@ -22,6 +22,8 @@ from nova import context
 from nova import db
 from nova import exception
 from nova import flags
+from nova import network
+from nova.network import model as network_model
 from nova.notifier import api as notifier_api
 from nova import utils
 
@@ -44,10 +46,26 @@ def notify_usage_exists(instance_ref, current_period=False):
     else:
         audit_start = begin
         audit_end = end
+
+    if (instance_ref.get('info_cache') and
+        instance_ref['info_cache'].get('network_info')):
+
+        cached_info = instance_ref['info_cache']['network_info']
+        nw_info = network_model.NetworkInfo.hydrate(cached_info)
+    else:
+        nw_info = network.API().get_instance_nw_info(admin_context,
+                                                         instance_ref)
+
     for b in db.bw_usage_get_by_instance(admin_context,
                                          instance_ref['id'],
                                          audit_start):
-        bw[b.network_label] = dict(bw_in=b.bw_in, bw_out=b.bw_out)
+        label = 'net-name-not-found-%s' % b['mac']
+        for vif in nw_info:
+            if vif['address'] == b['mac']:
+                label = vif['network']['label']
+                break
+
+        bw[label] = dict(bw_in=b.bw_in, bw_out=b.bw_out)
     usage_info = utils.usage_from_instance(instance_ref,
                           audit_period_beginning=str(audit_start),
                           audit_period_ending=str(audit_end),
diff --git a/nova/db/api.py b/nova/db/api.py
index a8869458c..c73844a8c 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -1549,14 +1549,14 @@ def bw_usage_get_all_by_filters(context, filters):
 
 def bw_usage_update(context,
                     instance_id,
-                    network_label,
+                    mac,
                     start_period,
                     bw_in, bw_out):
     """Update cached bw usage for an instance and network
        Creates new record if needed."""
     return IMPL.bw_usage_update(context,
                                 instance_id,
-                                network_label,
+                                mac,
                                 start_period,
                                 bw_in, bw_out)
 
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 7a0f9b200..b27ac9689 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -3710,7 +3710,7 @@ def bw_usage_get_all_by_filters(context, filters):
 @require_context
 def bw_usage_update(context,
                     instance_id,
-                    network_label,
+                    mac,
                     start_period,
                     bw_in, bw_out,
                     session=None):
@@ -3722,14 +3722,14 @@ def bw_usage_update(context,
                               read_deleted="yes").\
                       filter_by(instance_id=instance_id).\
                       filter_by(start_period=start_period).\
-                      filter_by(network_label=network_label).\
+                      filter_by(mac=mac).\
                       first()
 
         if not bwusage:
             bwusage = models.BandwidthUsage()
             bwusage.instance_id = instance_id
             bwusage.start_period = start_period
-            bwusage.network_label = network_label
+            bwusage.mac = mac
 
         bwusage.last_refreshed = utils.utcnow()
         bwusage.bw_in = bw_in
diff --git a/nova/db/sqlalchemy/migrate_repo/versions/075_convert_bw_usage_to_store_network_id.py b/nova/db/sqlalchemy/migrate_repo/versions/075_convert_bw_usage_to_store_network_id.py
new file mode 100644
index 000000000..e75d6a6d9
--- /dev/null
+++ b/nova/db/sqlalchemy/migrate_repo/versions/075_convert_bw_usage_to_store_network_id.py
@@ -0,0 +1,90 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from sqlalchemy import *
+from migrate import *
+
+from nova import utils
+
+
+meta = MetaData()
+
+
+def upgrade(migrate_engine):
+    meta.bind = migrate_engine
+    bw_usage_cache = Table('bw_usage_cache', meta,
+                           Column('id', Integer, primary_key=True),
+                           Column('network_label', String(255)),
+                           Column('instance_id', Integer, nullable=False),
+                           Column('start_period', DateTime, nullable=False),
+                           Column('last_refreshed', DateTime),
+                           Column('bw_in', BigInteger),
+                           Column('bw_out', BigInteger),
+                           Column('created_at', DateTime(timezone=False),
+                                  default=utils.utcnow()),
+                           Column('updated_at', DateTime(timezone=False),
+                                  onupdate=utils.utcnow()),
+                           Column('deleted_at', DateTime(timezone=False)),
+                           Column('deleted', Boolean(create_constraint=True,
+                                                     name=None)))
+
+    vifs = Table('virtual_interfaces', meta, autoload=True)
+    networks = Table('networks', meta, autoload=True)
+    mac_column = Column('mac', String(255))
+
+    bw_usage_cache.create_column(mac_column)
+
+    bw_usage_cache.update()\
+        .values(mac=select([vifs.c.address])\
+            .where(and_(networks.c.label == bw_usage_cache.c.network_label,
+                        networks.c.id == vifs.c.network_id))\
+            .as_scalar()).execute()
+
+    bw_usage_cache.c.network_label.drop()
+
+
+def downgrade(migrate_engine):
+    meta.bind = migrate_engine
+    bw_usage_cache = Table('bw_usage_cache', meta,
+                           Column('id', Integer, primary_key=True),
+                           Column('network_uuid', String(36)),
+                           Column('instance_id', Integer, nullable=False),
+                           Column('start_period', DateTime, nullable=False),
+                           Column('last_refreshed', DateTime),
+                           Column('bw_in', BigInteger),
+                           Column('bw_out', BigInteger),
+                           Column('created_at', DateTime(timezone=False),
+                                  default=utils.utcnow()),
+                           Column('updated_at', DateTime(timezone=False),
+                                  onupdate=utils.utcnow()),
+                           Column('deleted_at', DateTime(timezone=False)),
+                           Column('deleted', Boolean(create_constraint=True,
+                                                     name=None)))
+
+    vifs = Table('virtual_interfaces', meta, autoload=True)
+    network = Table('networks', meta, autoload=True)
+    network_label_column = Column('network_label', String(255))
+
+    bw_usage_cache.create_column(network_label_column)
+
+    bw_usage_cache.update()\
+        .values(network_label=select([network.c.label])\
+            .where(and_(network.c.id == vifs.c.network_id,
+                        vifs.c.address == bw_usage_cache.c.mac))\
+            .as_scalar()).execute()
+
+    bw_usage_cache.c.network_uuid.drop()
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index f7b9a555c..df7f42c8d 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -927,7 +927,7 @@ class BandwidthUsage(BASE, NovaBase):
     __tablename__ = 'bw_usage_cache'
     id = Column(Integer, primary_key=True, nullable=False)
     instance_id = Column(Integer, nullable=False)
-    network_label = Column(String(255))
+    mac = Column(String(255), nullable=False)
     start_period = Column(DateTime, nullable=False)
     last_refreshed = Column(DateTime)
     bw_in = Column(BigInteger)
diff --git a/nova/network/api.py b/nova/network/api.py
index 257c642a9..27e07b869 100644
--- a/nova/network/api.py
+++ b/nova/network/api.py
@@ -99,6 +99,12 @@ class API(base.Base):
                         {'method': 'get_vifs_by_instance',
                          'args': {'instance_id': instance['id']}})
 
+    def get_vif_by_mac_address(self, context, mac_address):
+        return rpc.call(context,
+                        FLAGS.network_topic,
+                        {'method': 'get_vif_by_mac_address',
+                         'args': {'mac_address': mac_address}})
+
     def allocate_floating_ip(self, context, pool=None):
         """Adds a floating ip to a project from a pool. (allocates)"""
         # NOTE(vish): We don't know which network host should get the ip
diff --git a/nova/network/manager.py b/nova/network/manager.py
index bce00f0fd..053305992 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -1476,6 +1476,11 @@ class NetworkManager(manager.SchedulerDependentManager):
         fixed = self.db.fixed_ip_get(context, id)
         return dict(fixed.iteritems())
 
+    def get_vif_by_mac_address(self, context, mac_address):
+        """Returns the vifs record for the mac_address"""
+        return self.db.virtual_interface_get_by_address(context,
+                                                        mac_address)
+
 
 class FlatManager(NetworkManager):
     """Basic network where no vlans are used.
diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py
index 400e6949f..d99aed310 100644
--- a/nova/tests/test_compute.py
+++ b/nova/tests/test_compute.py
@@ -190,6 +190,15 @@ class BaseTestCase(test.TestCase):
 
 
 class ComputeTestCase(BaseTestCase):
+    def setUp(self):
+        def fake_get_nw_info(cls, ctxt, instance):
+            self.assertTrue(ctxt.is_admin)
+            return fake_network.fake_get_instance_nw_info(self.stubs, 1, 1,
+                                                          spectacular=True)
+
+        super(ComputeTestCase, self).setUp()
+        self.stubs.Set(nova.network.API, 'get_instance_nw_info',
+                       fake_get_nw_info)
 
     def test_wrap_instance_fault(self):
         inst_uuid = "fake_uuid"
@@ -966,38 +975,66 @@ class ComputeTestCase(BaseTestCase):
     def test_finish_resize(self):
         """Contrived test to ensure finish_resize doesn't raise anything"""
 
+        nw_info = fake_network.fake_get_instance_nw_info(self.stubs,
+                                                         spectacular=True)
+
         def fake(*args, **kwargs):
             pass
 
+        def fake_nw_info(*args, **kwargs):
+            return nw_info
+
+        # NOTE(jkoelker) There is a bit of a stubbing issue here.
+        #                fake_network stubs out a bunch of stuff which
+        #                this functional test expects to be acting on
+        #                the db or the stubs it sets.
+        self.stubs.UnsetAll()
+        self.stubs.SmartUnsetAll()
+        self.setUp()
+
         self.stubs.Set(self.compute.driver, 'finish_migration', fake)
-        self.stubs.Set(self.compute.network_api, 'get_instance_nw_info', fake)
+        self.stubs.Set(self.compute.network_api, 'get_instance_nw_info',
+                       fake_nw_info)
+        fake_network.stub_out_nw_api_get_instance_nw_info(self.stubs,
+                                                          func=fake_nw_info)
         context = self.context.elevated()
+
         instance = self._create_fake_instance()
         self.compute.prep_resize(context, instance['uuid'], 1,
                                  filter_properties={})
         migration_ref = db.migration_get_by_instance_and_status(context,
                 instance['uuid'], 'pre-migrating')
-        try:
-            self.compute.finish_resize(context, instance['uuid'],
-                    int(migration_ref['id']), {})
-        except KeyError, e:
-            # Only catch key errors. We want other reasons for the test to
-            # fail to actually error out so we don't obscure anything
-            self.fail()
-
+        self.compute.finish_resize(context, instance['uuid'],
+                                   int(migration_ref['id']), {})
         self.compute.terminate_instance(self.context, instance['uuid'])
 
     def test_finish_resize_handles_error(self):
         """Make sure we don't leave the instance in RESIZE on error"""
 
+        nw_info = fake_network.fake_get_instance_nw_info(self.stubs,
+                                                         spectacular=True)
+
         def throw_up(*args, **kwargs):
             raise Exception()
 
         def fake(*args, **kwargs):
             pass
 
+        def fake_nw_info(*args, **kwargs):
+            return nw_info
+
+        # NOTE(jkoelker) There is a bit of a stubbing issue here.
+        #                fake_network stubs out a bunch of stuff which
+        #                this functional test expects to be acting on
+        #                the db or the stubs it sets.
+        self.stubs.UnsetAll()
+        self.stubs.SmartUnsetAll()
+        self.setUp()
+
         self.stubs.Set(self.compute.driver, 'finish_migration', throw_up)
         self.stubs.Set(self.compute.network_api, 'get_instance_nw_info', fake)
+        fake_network.stub_out_nw_api_get_instance_nw_info(self.stubs,
+                                                          func=fake_nw_info)
         context = self.context.elevated()
         instance = self._create_fake_instance()
         self.compute.prep_resize(context, instance['uuid'], 1,
@@ -1123,13 +1160,32 @@ class ComputeTestCase(BaseTestCase):
 
     def test_finish_revert_resize(self):
         """Ensure that the flavor is reverted to the original on revert"""
-        context = self.context.elevated()
-        instance = self._create_fake_instance()
-        instance_uuid = instance['uuid']
+        nw_info = fake_network.fake_get_instance_nw_info(self.stubs,
+                                                         spectacular=True)
 
         def fake(*args, **kwargs):
             pass
 
+        def fake_nw_info(*args, **kwargs):
+            return nw_info
+
+        # NOTE(jkoelker) There is a bit of a stubbing issue here.
+        #                fake_network stubs out a bunch of stuff which
+        #                this functional test expects to be acting on
+        #                the db or the stubs it sets.
+        self.stubs.UnsetAll()
+        self.stubs.SmartUnsetAll()
+        self.setUp()
+
+        self.stubs.Set(self.compute.network_api, 'get_instance_nw_info',
+                       fake_nw_info)
+        fake_network.stub_out_nw_api_get_instance_nw_info(self.stubs,
+                                                          func=fake_nw_info)
+
+        context = self.context.elevated()
+        instance = self._create_fake_instance()
+        instance_uuid = instance['uuid']
+
         self.stubs.Set(self.compute.driver, 'finish_migration', fake)
         self.stubs.Set(self.compute.driver, 'finish_revert_migration', fake)
         self.stubs.Set(self.compute.network_api, 'get_instance_nw_info', fake)
@@ -1450,7 +1506,14 @@ class ComputeTestCase(BaseTestCase):
 class ComputeAPITestCase(BaseTestCase):
 
     def setUp(self):
+        def fake_get_nw_info(cls, ctxt, instance):
+            self.assertTrue(ctxt.is_admin)
+            return fake_network.fake_get_instance_nw_info(self.stubs, 1, 1,
+                                                          spectacular=True)
+
         super(ComputeAPITestCase, self).setUp()
+        self.stubs.Set(nova.network.API, 'get_instance_nw_info',
+                       fake_get_nw_info)
         self.compute_api = compute.API()
         self.fake_image = {
             'id': 1,
@@ -2267,17 +2330,9 @@ class ComputeAPITestCase(BaseTestCase):
                                           fixed_address):
             called['associate'] = True
 
-        def fake_get_nw_info(cls, ctxt, instance):
-            self.assertTrue(ctxt.is_admin)
-            return fake_network.fake_get_instance_nw_info(self.stubs, 1, 1,
-                                                          spectacular=True)
-
         self.stubs.Set(nova.network.API, 'associate_floating_ip',
                        fake_associate_ip_network_api)
 
-        self.stubs.Set(nova.network.API, 'get_instance_nw_info',
-                       fake_get_nw_info)
-
         instance = self._create_fake_instance()
         instance_uuid = instance['uuid']
         address = '0.1.2.3'
@@ -2995,12 +3050,6 @@ class ComputeAPITestCase(BaseTestCase):
         self.assertTrue(self.compute_api.get_lock(self.context, instance))
 
     def test_add_remove_security_group(self):
-        def fake_get_nw_info(cls, ctxt, instance):
-            return fake_network.fake_get_instance_nw_info(self.stubs, 1, 1,
-                                                          spectacular=True)
-
-        self.stubs.Set(nova.network.API, 'get_instance_nw_info',
-                       fake_get_nw_info)
         instance = self._create_fake_instance()
 
         self.compute.run_instance(self.context, instance['uuid'])
diff --git a/nova/tests/test_compute_utils.py b/nova/tests/test_compute_utils.py
index 71463fbc0..8e35d1905 100644
--- a/nova/tests/test_compute_utils.py
+++ b/nova/tests/test_compute_utils.py
@@ -29,6 +29,7 @@ import nova.image.fake
 from nova.compute import utils as compute_utils
 from nova.compute import instance_types
 from nova.notifier import test_notifier
+from nova.tests import fake_network
 
 
 LOG = logging.getLogger('nova.tests.compute_utils')
@@ -39,7 +40,15 @@ flags.DECLARE('stub_network', 'nova.compute.manager')
 class UsageInfoTestCase(test.TestCase):
 
     def setUp(self):
+        def fake_get_nw_info(cls, ctxt, instance):
+            self.assertTrue(ctxt.is_admin)
+            return fake_network.fake_get_instance_nw_info(self.stubs, 1, 1,
+                                                          spectacular=True)
+
         super(UsageInfoTestCase, self).setUp()
+        self.stubs.Set(nova.network.API, 'get_instance_nw_info',
+                       fake_get_nw_info)
+
         self.flags(connection_type='fake',
                    stub_network=True,
                    notification_driver='nova.notifier.test_notifier',
diff --git a/nova/tests/test_notifier.py b/nova/tests/test_notifier.py
index 00f367f49..b13f203a4 100644
--- a/nova/tests/test_notifier.py
+++ b/nova/tests/test_notifier.py
@@ -16,6 +16,7 @@
 import stubout
 
 import nova
+import nova.notifier.no_op_notifier
 from nova import log
 import nova.notifier.api
 from nova.notifier.api import notify
@@ -26,6 +27,7 @@ class NotifierTestCase(test.TestCase):
     """Test case for notifications"""
     def setUp(self):
         super(NotifierTestCase, self).setUp()
+        self.flags(notification_driver='nova.notifier.no_op_notifier')
         self.stubs = stubout.StubOutForTesting()
 
     def tearDown(self):
diff --git a/nova/tests/test_quota.py b/nova/tests/test_quota.py
index 8ed38cc92..23660333a 100644
--- a/nova/tests/test_quota.py
+++ b/nova/tests/test_quota.py
@@ -46,7 +46,8 @@ class QuotaTestCase(test.TestCase):
                    quota_cores=4,
                    quota_volumes=2,
                    quota_gigabytes=20,
-                   quota_floating_ips=1)
+                   quota_floating_ips=1,
+                   network_manager='nova.network.manager.FlatDHCPManager')
 
         self.network = self.network = self.start_service('network')
         self.user_id = 'admin'
diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py
index 26e36b911..e034cb3ca 100644
--- a/nova/virt/xenapi_conn.py
+++ b/nova/virt/xenapi_conn.py
@@ -328,13 +328,9 @@ class XenAPIConnection(driver.ComputeDriver):
         for iusage in self._vmops.get_all_bw_usage(start_time, stop_time).\
                       values():
             for macaddr, usage in iusage.iteritems():
-                vi = db.virtual_interface_get_by_address(
-                                    context.get_admin_context(),
-                                    macaddr)
-                if vi:
-                    bwusage.append(dict(virtual_interface=vi,
-                                        bw_in=usage['bw_in'],
-                                        bw_out=usage['bw_out']))
+                bwusage.append(dict(mac_address=macaddr,
+                                    bw_in=usage['bw_in'],
+                                    bw_out=usage['bw_out']))
         return bwusage
 
     def get_console_output(self, instance):