From 222085dcafb6d94adbf20ce82e3245c46735f105 Mon Sep 17 00:00:00 2001 From: Sylvain Bauza Date: Mon, 13 Apr 2015 15:03:06 +0200 Subject: [PATCH] Invalidate AZ cache when the instance AZ information is different When a new instance is created, the host value is set to None. As a corrolar, its AZ field is set to the default_availability_zone value. That's only when the host field is provided (ie. once the instance is started) that the AZ information is also updated. Unfornately, as the AZ module is using a cache system, it leaves the old AZ unchanged which can make the instance AZ data wrong. In order to fix that corner case, we need to compare both cache and instance values and if different, invalidate the cache key and leave it updated correctly by calling the AZ related information of the aggregate the host belongs to. Change-Id: I1b47ae07cf8a4245df00b9dc25e6000428bdc4f2 Closes-Bug: #1390033 --- nova/availability_zones.py | 14 ++++++++++++++ nova/tests/unit/test_availability_zones.py | 20 ++++++++++++++++++-- nova/tests/unit/test_metadata.py | 3 ++- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/nova/availability_zones.py b/nova/availability_zones.py index 11b5aef7c7db..dbd8697c3723 100644 --- a/nova/availability_zones.py +++ b/nova/availability_zones.py @@ -178,6 +178,20 @@ def get_instance_availability_zone(context, instance): cache_key = _make_cache_key(host) cache = _get_cache() az = cache.get(cache_key) + az_inst = instance.get('availability_zone') + if az_inst is not None and az != az_inst: + # NOTE(sbauza): Cache is wrong, we need to invalidate it by fetching + # again the right AZ related to the aggregate the host belongs to. + # As the API is also calling this method for setting the instance + # AZ field, we don't need to update the instance.az field. + # This case can happen because the cache is populated before the + # instance has been assigned to the host so that it would keep the + # former reference which was incorrect. Instead of just taking the + # instance AZ information for refilling the cache, we prefer to + # invalidate the cache and fetch it again because there could be some + # corner cases where this method could be called before the instance + # has been assigned to the host also. + az = None if not az: elevated = context.elevated() az = get_host_availability_zone(elevated, host) diff --git a/nova/tests/unit/test_availability_zones.py b/nova/tests/unit/test_availability_zones.py index ca5ce1e805b7..30449c6003bd 100644 --- a/nova/tests/unit/test_availability_zones.py +++ b/nova/tests/unit/test_availability_zones.py @@ -17,6 +17,7 @@ Tests for availability zones """ +import mock from oslo_config import cfg import six @@ -239,7 +240,8 @@ class AvailabilityZoneTestCases(test.TestCase): def test_get_instance_availability_zone_default_value(self): """Test get right availability zone by given an instance.""" - fake_inst = objects.Instance(host=self.host) + fake_inst = objects.Instance(host=self.host, + availability_zone=None) self.assertEqual(self.default_az, az.get_instance_availability_zone(self.context, fake_inst)) @@ -250,11 +252,25 @@ class AvailabilityZoneTestCases(test.TestCase): service = self._create_service_with_topic('compute', host) self._add_to_aggregate(service, self.agg) - fake_inst = objects.Instance(host=host) + fake_inst = objects.Instance(host=host, + availability_zone=self.availability_zone) self.assertEqual(self.availability_zone, az.get_instance_availability_zone(self.context, fake_inst)) + @mock.patch.object(az._get_cache(), 'get') + def test_get_instance_availability_zone_cache_differs(self, cache_get): + host = 'host170' + service = self._create_service_with_topic('compute', host) + self._add_to_aggregate(service, self.agg) + cache_get.return_value = self.default_az + + fake_inst = objects.Instance(host=host, + availability_zone=self.availability_zone) + self.assertEqual( + self.availability_zone, + az.get_instance_availability_zone(self.context, fake_inst)) + def test_get_instance_availability_zone_no_host(self): """Test get availability zone from instance if host not set.""" fake_inst = objects.Instance(host=None, availability_zone='inst-az') diff --git a/nova/tests/unit/test_metadata.py b/nova/tests/unit/test_metadata.py index 2dd147232220..10aa5e72ca83 100644 --- a/nova/tests/unit/test_metadata.py +++ b/nova/tests/unit/test_metadata.py @@ -83,7 +83,8 @@ def fake_inst_obj(context): metadata={}, default_ephemeral_device=None, default_swap_device=None, - system_metadata={}) + system_metadata={}, + availability_zone=None) nwinfo = network_model.NetworkInfo([]) inst.info_cache = objects.InstanceInfoCache(context=context, instance_uuid=inst.uuid,