openstack
/
nova
Code Issues Proposed changes

Fix service_version minimum calculation for compute RPC 

The compute rpcapi module incorrectly specified the binary as "compute"
instead of "nova-compute" when calling get_minimum_version() on the
Service object class. This resulted in always getting back a zero answer.

This patch fixes that, adds a warning and an exception if the method
is called with a binary that doesn't start with "nova-" and corrects
the tests.

Change-Id: I265ee1e14335f837eac4c07dc19dcb7ac5d63886
Closes-Bug: #1506089
This commit is contained in:
Dan Smith 2015-10-14 07:36:53 -07:00
parent 5941f294fb
commit a337a5f1d3
4 changed files with 34 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
nova/compute/rpcapi.py
View File

@ -332,7 +332,7 @@ class ComputeAPI(object):
def _determine_version_cap(self, target):
# FIXME(danms): We should reload this on SIGHUP, or by timer
service_version = objects.Service.get_minimum_version(
context.get_admin_context(), 'compute')
context.get_admin_context(), 'nova-compute')
history = service_obj.SERVICE_VERSION_HISTORY
try:
version_cap = history[service_version]['compute_rpc']

6
nova/objects/service.py
View File

@ -17,6 +17,7 @@ from oslo_log import log as logging
from nova import availability_zones
from nova import db
from nova import exception
from nova.i18n import _LW
from nova import objects
from nova.objects import base
from nova.objects import fields
@ -282,6 +283,11 @@ class Service(base.NovaPersistentObject, base.NovaObject,
@base.remotable_classmethod
def get_minimum_version(cls, context, binary, use_slave=False):
if not binary.startswith('nova-'):
LOG.warning(_LW('get_minimum_version called with likely-incorrect '
'binary `%s\''), binary)
raise exception.ObjectActionError(action='get_minimum_version',
reason='Invalid binary prefix')
version = db.service_get_minimum_version(context, binary,
use_slave=use_slave)
if version is None:

4
nova/tests/unit/compute/test_rpcapi.py
View File

@ -57,7 +57,7 @@ class ComputeRpcAPITestCase(test.NoDBTestCase):
self.flags(compute='auto', group='upgrade_levels')
rpcapi = compute_rpcapi.ComputeAPI()
self.assertEqual('4.4', rpcapi.client.version_cap)
mock_get_min.assert_called_once_with(mock.ANY, 'compute')
mock_get_min.assert_called_once_with(mock.ANY, 'nova-compute')
@mock.patch('nova.objects.Service.get_minimum_version')
def test_auto_pin_fails_if_too_old(self, mock_get_min):
@ -72,7 +72,7 @@ class ComputeRpcAPITestCase(test.NoDBTestCase):
self.flags(compute='auto', group='upgrade_levels')
rpcapi = compute_rpcapi.ComputeAPI()
self.assertEqual('4.0', rpcapi.client.version_cap)
mock_get_min.assert_called_once_with(mock.ANY, 'compute')
mock_get_min.assert_called_once_with(mock.ANY, 'nova-compute')
def _test_compute_api(self, method, rpc_method,
expected_args=None, **kwargs):

34
nova/tests/unit/objects/test_service.py
View File

@ -34,7 +34,7 @@ fake_service = {
'deleted': False,
'id': 123,
'host': 'fake-host',
'binary': 'fake-service',
'binary': 'nova-fake',
'topic': 'fake-service-topic',
'report_count': 1,
'forced_down': False,
@ -148,7 +148,7 @@ class _TestServiceObject(object):
return_value=fake_service)
def test_set_id_failure(self, db_mock):
service_obj = service.Service(context=self.context,
binary='compute')
binary='nova-compute')
service_obj.create()
self.assertRaises(ovo_exc.ReadOnlyFieldError, setattr,
service_obj, 'id', 124)
@ -285,8 +285,8 @@ class _TestServiceObject(object):
mock_get.return_value = None
self.assertEqual(0,
objects.Service.get_minimum_version(self.context,
'compute'))
mock_get.assert_called_once_with(self.context, 'compute',
'nova-compute'))
mock_get.assert_called_once_with(self.context, 'nova-compute',
use_slave=False)
@mock.patch('nova.db.service_get_minimum_version')
@ -294,21 +294,37 @@ class _TestServiceObject(object):
mock_get.return_value = 123
self.assertEqual(123,
objects.Service.get_minimum_version(self.context,
'compute'))
mock_get.assert_called_once_with(self.context, 'compute',
'nova-compute'))
mock_get.assert_called_once_with(self.context, 'nova-compute',
use_slave=False)
@mock.patch('nova.db.service_get_minimum_version')
@mock.patch('nova.objects.service.LOG')
def test_get_minimum_version_checks_binary(self, mock_log, mock_get):
mock_get.return_value = None
self.assertEqual(0,
objects.Service.get_minimum_version(self.context,
'nova-compute'))
self.assertFalse(mock_log.warning.called)
self.assertRaises(exception.ObjectActionError,
objects.Service.get_minimum_version,
self.context,
'compute')
self.assertTrue(mock_log.warning.called)
@mock.patch('nova.db.service_get_minimum_version', return_value=2)
def test_create_above_minimum(self, mock_get):
with mock.patch('nova.objects.service.SERVICE_VERSION',
new=3):
objects.Service(context=self.context, binary='compute').create()
objects.Service(context=self.context,
binary='nova-compute').create()
@mock.patch('nova.db.service_get_minimum_version', return_value=2)
def test_create_equal_to_minimum(self, mock_get):
with mock.patch('nova.objects.service.SERVICE_VERSION',
new=2):
objects.Service(context=self.context, binary='compute').create()
objects.Service(context=self.context,
binary='nova-compute').create()
@mock.patch('nova.db.service_get_minimum_version', return_value=2)
def test_create_below_minimum(self, mock_get):
@ -316,7 +332,7 @@ class _TestServiceObject(object):
new=1):
self.assertRaises(exception.ServiceTooOld,
objects.Service(context=self.context,
binary='compute',
binary='nova-compute',
).create)