Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix instance cross AZ check when attaching volumes
For CONF.cinder_cross_az_attach=False, we currently only check the desired AZ
during scheduling but don't actually check the final AZ of the node.

When attaching volumes to running instances we need to check the actual
AZ on the instance and not the desired AZ at boot time. The desired AZ
attribute is optional at boot and may not be set or the desired AZ
could be different from the final AZ.
When we are booting with a volume however we don't know the final AZ so we
check on the desired AZ.

Closes-Bug: #1255347
Change-Id: Ied912c171100a450754d315027bf5c407ac067bb
(cherry picked from commit 18a1486)
  • Loading branch information
sorrison authored and Roman Podoliaka committed Oct 2, 2014
1 parent c04b6ea commit 64ec1bf
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 11 deletions.
45 changes: 35 additions & 10 deletions nova/tests/volume/test_cinder.py
Expand Up @@ -16,6 +16,7 @@
import mock

from cinderclient import exceptions as cinder_exception
import mock

from nova import context
from nova import exception
Expand Down Expand Up @@ -110,22 +111,46 @@ def test_check_attach_volume_already_attached(self):
def test_check_attach_availability_zone_differs(self):
volume = {'status': 'available'}
volume['attach_status'] = "detached"
instance = {'availability_zone': 'zone1'}
volume['availability_zone'] = 'zone2'
cinder.CONF.set_override('cinder_cross_az_attach', False)
self.assertRaises(exception.InvalidVolume,
self.api.check_attach, self.ctx, volume, instance)
volume['availability_zone'] = 'zone1'
self.assertIsNone(self.api.check_attach(self.ctx, volume, instance))
cinder.CONF.reset()
instance = {'availability_zone': 'zone1', 'host': 'fakehost'}

with mock.patch.object(cinder.az, 'get_instance_availability_zone',
side_effect=lambda context,
instance: 'zone1') as mock_get_instance_az:

cinder.CONF.set_override('cinder_cross_az_attach', False)
volume['availability_zone'] = 'zone1'
self.assertIsNone(self.api.check_attach(self.ctx,
volume, instance))
mock_get_instance_az.assert_called_once_with(self.ctx, instance)
mock_get_instance_az.reset_mock()
volume['availability_zone'] = 'zone2'
self.assertRaises(exception.InvalidVolume,
self.api.check_attach, self.ctx, volume, instance)
mock_get_instance_az.assert_called_once_with(self.ctx, instance)
mock_get_instance_az.reset_mock()
del instance['host']
volume['availability_zone'] = 'zone1'
self.assertIsNone(self.api.check_attach(
self.ctx, volume, instance))
mock_get_instance_az.assert_not_called()
volume['availability_zone'] = 'zone2'
self.assertRaises(exception.InvalidVolume,
self.api.check_attach, self.ctx, volume, instance)
mock_get_instance_az.assert_not_called()
cinder.CONF.reset()

def test_check_attach(self):
volume = {'status': 'available'}
volume['attach_status'] = "detached"
volume['availability_zone'] = 'zone1'
instance = {'availability_zone': 'zone1'}
instance = {'availability_zone': 'zone1', 'host': 'fakehost'}
cinder.CONF.set_override('cinder_cross_az_attach', False)
self.assertIsNone(self.api.check_attach(self.ctx, volume, instance))

with mock.patch.object(cinder.az, 'get_instance_availability_zone',
side_effect=lambda context, instance: 'zone1'):
self.assertIsNone(self.api.check_attach(
self.ctx, volume, instance))

cinder.CONF.reset()

def test_check_detach(self):
Expand Down
10 changes: 9 additions & 1 deletion nova/volume/cinder.py
Expand Up @@ -26,6 +26,7 @@
from cinderclient.v1 import client as cinder_client
from oslo.config import cfg

from nova import availability_zones as az
from nova import exception
from nova.openstack.common.gettextutils import _
from nova.openstack.common import log as logging
Expand Down Expand Up @@ -230,7 +231,14 @@ def check_attach(self, context, volume, instance=None):
msg = _("already attached")
raise exception.InvalidVolume(reason=msg)
if instance and not CONF.cinder_cross_az_attach:
if instance['availability_zone'] != volume['availability_zone']:
# NOTE(sorrison): If instance is on a host we match against it's AZ
# else we check the intended AZ
if instance.get('host'):
instance_az = az.get_instance_availability_zone(
context, instance)
else:
instance_az = instance['availability_zone']
if instance_az != volume['availability_zone']:
msg = _("Instance and volume not in same availability_zone")
raise exception.InvalidVolume(reason=msg)

Expand Down

0 comments on commit 64ec1bf

Please sign in to comment.