diff --git a/api-ref/source/v1/parameters.yaml b/api-ref/source/v1/parameters.yaml index ec174c86..08343e8a 100644 --- a/api-ref/source/v1/parameters.yaml +++ b/api-ref/source/v1/parameters.yaml @@ -427,13 +427,13 @@ reservation_allocation: type: array reservation_amount: description: | - The amount of instances to reserve. + The amount of instances to reserve. The value should be greater than 0. in: body required: true type: integer reservation_amount_optional: description: | - The amount of instances to reserve. + The amount of instances to reserve. The value should be greater than 0. in: body required: false type: integer @@ -497,13 +497,13 @@ reservation_lease_id: type: string reservation_max: description: | - The maximum number of hosts to reserve. + The maximum number of hosts to reserve. The value should be greater than or equal to ``min`` value. in: body required: true type: integer reservation_max_optional: description: | - The maximum number of hosts to reserve. + The maximum number of hosts to reserve. The value should be greater than or equal to ``min``. in: body required: false type: integer @@ -521,13 +521,13 @@ reservation_memory_mb_optional: type: integer reservation_min: description: | - The minimum number of hosts to reserve. + The minimum number of hosts to reserve. The value should be greater than 0. in: body required: true type: integer reservation_min_optional: description: | - The minimum number of hosts to reserve. + The minimum number of hosts to reserve. The value should be greater than 0. in: body required: false type: integer diff --git a/blazar/db/api.py b/blazar/db/api.py index 243d9b24..8deeb78b 100644 --- a/blazar/db/api.py +++ b/blazar/db/api.py @@ -46,6 +46,9 @@ IMPL = db_api.DBAPI(cfg.CONF.database.backend, backend_mapping=_BACKEND_MAPPING) LOG = logging.getLogger(__name__) +# The maximum value a signed INT type may have +DB_MAX_INT = 0x7FFFFFFF + def get_instance(): """Return a DB API instance.""" diff --git a/blazar/plugins/instances/instance_plugin.py b/blazar/plugins/instances/instance_plugin.py index d508ad95..d4830d37 100644 --- a/blazar/plugins/instances/instance_plugin.py +++ b/blazar/plugins/instances/instance_plugin.py @@ -15,10 +15,12 @@ import collections import datetime import retrying +import six from novaclient import exceptions as nova_exceptions from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import strutils from oslo_utils.strutils import bool_from_string from blazar import context @@ -381,7 +383,7 @@ class VirtualInstancePlugin(base.BasePlugin, nova.NovaClientWrapper): "for reservation %s", reservation['id']) raise mgr_exceptions.NovaClientError() - def validate_reservation_param(self, values): + def _check_missing_reservation_params(self, values): marshall_attributes = set(['vcpus', 'memory_mb', 'disk_gb', 'amount', 'affinity', 'resource_properties']) @@ -389,8 +391,17 @@ class VirtualInstancePlugin(base.BasePlugin, nova.NovaClientWrapper): if missing_attr: raise mgr_exceptions.MissingParameter(param=','.join(missing_attr)) + def _validate_reservation_params(self, values): + if 'amount' in values: + try: + values['amount'] = strutils.validate_integer( + values['amount'], "amount", 1, db_api.DB_MAX_INT) + except ValueError as e: + raise mgr_exceptions.MalformedParameter(six.text_type(e)) + def reserve_resource(self, reservation_id, values): - self.validate_reservation_param(values) + self._check_missing_reservation_params(values) + self._validate_reservation_params(values) hosts = self.pickup_hosts(reservation_id, values) @@ -457,6 +468,7 @@ class VirtualInstancePlugin(base.BasePlugin, nova.NovaClientWrapper): - If an instance reservation has already started - only amount is increasable. """ + self._validate_reservation_params(new_values) reservation = db_api.reservation_get(reservation_id) lease = db_api.lease_get(reservation['lease_id']) diff --git a/blazar/plugins/oshosts/host_plugin.py b/blazar/plugins/oshosts/host_plugin.py index ebfa416e..3012460c 100644 --- a/blazar/plugins/oshosts/host_plugin.py +++ b/blazar/plugins/oshosts/host_plugin.py @@ -605,13 +605,8 @@ class PhysicalHostPlugin(base.BasePlugin, nova.NovaClientWrapper): return param def _check_params(self, values): - min_hosts = self._convert_int_param(values.get('min'), 'min') - max_hosts = self._convert_int_param(values.get('max'), 'max') - - if 0 <= min_hosts and min_hosts <= max_hosts: - values['count_range'] = str(min_hosts) + '-' + str(max_hosts) - else: - raise manager_ex.InvalidRange() + self._validate_min_max_range(values, values.get('min'), + values.get('max')) if 'hypervisor_properties' not in values: raise manager_ex.MissingParameter(param='hypervisor_properties') @@ -623,14 +618,23 @@ class PhysicalHostPlugin(base.BasePlugin, nova.NovaClientWrapper): if values['before_end'] not in before_end_options: raise manager_ex.MalformedParameter(param='before_end') + def _validate_min_max_range(self, values, min_hosts, max_hosts): + self._convert_int_param(min_hosts, 'min') + self._convert_int_param(max_hosts, 'max') + if min_hosts <= 0 or max_hosts <= 0: + raise manager_ex.MalformedParameter( + param='min and max (must be greater than or equal to 1)') + if max_hosts < min_hosts: + raise manager_ex.InvalidRange() + values['count_range'] = str(min_hosts) + '-' + str(max_hosts) + def _update_allocations(self, dates_before, dates_after, reservation_id, reservation_status, host_reservation, values): - min_hosts = self._convert_int_param(values.get( - 'min', host_reservation['count_range'].split('-')[0]), 'min') - max_hosts = self._convert_int_param(values.get( - 'max', host_reservation['count_range'].split('-')[1]), 'max') - if min_hosts < 0 or max_hosts < min_hosts: - raise manager_ex.InvalidRange() + min_hosts = values.get('min', int( + host_reservation['count_range'].split('-')[0])) + max_hosts = values.get( + 'max', int(host_reservation['count_range'].split('-')[1])) + self._validate_min_max_range(values, min_hosts, max_hosts) hypervisor_properties = values.get( 'hypervisor_properties', host_reservation['hypervisor_properties']) diff --git a/blazar/tests/plugins/instances/test_instance_plugin.py b/blazar/tests/plugins/instances/test_instance_plugin.py index b1c14294..7e717627 100644 --- a/blazar/tests/plugins/instances/test_instance_plugin.py +++ b/blazar/tests/plugins/instances/test_instance_plugin.py @@ -16,6 +16,7 @@ import datetime import uuid +import ddt import mock from novaclient import exceptions as nova_exceptions import six @@ -35,6 +36,7 @@ from oslo_config import fixture as conf_fixture CONF = cfg.CONF +@ddt.ddt class TestVirtualInstancePlugin(tests.TestCase): def setUp(self): @@ -140,6 +142,28 @@ class TestVirtualInstancePlugin(tests.TestCase): 'server_group_id': 2, 'aggregate_id': 3}) + @ddt.data(-1, 0, '0', 'one') + def test_error_with_amount(self, value): + plugin = instance_plugin.VirtualInstancePlugin() + inputs = self.get_input_values(2, 4018, 10, value, False, + '2030-01-01 08:00', '2030-01-01 08:00', + 'lease-1', '') + self.assertRaises(mgr_exceptions.MalformedParameter, + plugin.reserve_resource, 'reservation_id', inputs) + self.assertRaises(mgr_exceptions.MalformedParameter, + plugin.update_reservation, 'reservation_id', inputs) + + @ddt.data('vcpus', 'memory_mb', 'disk_gb', 'amount', 'affinity', + 'resource_properties') + def test_create_reservation_with_missing_param(self, missing_param): + plugin = instance_plugin.VirtualInstancePlugin() + inputs = self.get_input_values(2, 4018, 10, 1, False, + '2030-01-01 08:00', '2030-01-01 08:00', + 'lease-1', '') + del inputs[missing_param] + self.assertRaises(mgr_exceptions.MissingParameter, + plugin.reserve_resource, 'reservation_id', inputs) + def test_filter_hosts_by_reservation_with_exclude(self): def fake_get_reservation_by_host(host_id, start, end): if host_id == 'host-1': diff --git a/blazar/tests/plugins/oshosts/test_physical_host_plugin.py b/blazar/tests/plugins/oshosts/test_physical_host_plugin.py index 66c90bee..a9ea31fc 100644 --- a/blazar/tests/plugins/oshosts/test_physical_host_plugin.py +++ b/blazar/tests/plugins/oshosts/test_physical_host_plugin.py @@ -15,6 +15,7 @@ import datetime +import ddt import mock from novaclient import client as nova_client from novaclient import exceptions as nova_exceptions @@ -101,6 +102,7 @@ class PhysicalHostPluginSetupOnlyTestCase(tests.TestCase): self.assertEqual({}, res) +@ddt.ddt class PhysicalHostPluginTestCase(tests.TestCase): def setUp(self): @@ -643,8 +645,8 @@ class PhysicalHostPluginTestCase(tests.TestCase): now = datetime.datetime.utcnow() values = { 'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c', - 'min': u'1', - 'max': u'1', + 'min': 1, + 'max': 1, 'hypervisor_properties': '["=", "$memory_mb", "256"]', 'resource_properties': '', 'start_date': now, @@ -665,8 +667,8 @@ class PhysicalHostPluginTestCase(tests.TestCase): def test_create_reservation_hosts_available(self): values = { 'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c', - 'min': u'1', - 'max': u'1', + 'min': 1, + 'max': 1, 'hypervisor_properties': '["=", "$memory_mb", "256"]', 'resource_properties': '', 'start_date': datetime.datetime(2013, 12, 19, 20, 00), @@ -706,104 +708,117 @@ class PhysicalHostPluginTestCase(tests.TestCase): ] host_allocation_create.assert_has_calls(calls) - def test_create_reservation_with_missing_param_min(self): + @ddt.data("min", "max", "hypervisor_properties", "resource_properties") + def test_create_reservation_with_missing_param(self, missing_param): values = { 'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c', - 'max': u'2', + 'min': 1, + 'max': 2, + 'before_end': 'default', 'hypervisor_properties': '["=", "$memory_mb", "256"]', 'resource_properties': '', 'start_date': datetime.datetime(2017, 3, 1, 20, 00), 'end_date': datetime.datetime(2017, 3, 2, 20, 00), - 'resource_type': plugin.RESOURCE_TYPE, - } + 'resource_type': plugin.RESOURCE_TYPE} + del values[missing_param] self.assertRaises( manager_exceptions.MissingParameter, self.fake_phys_plugin.reserve_resource, u'441c1476-9f8f-4700-9f30-cd9b6fef3509', values) - def test_create_reservation_with_missing_param_max(self): + @ddt.data({"params": {'max': 0}}, + {"params": {'max': -1}}, + {"params": {'max': 'one'}}, + {"params": {'min': 0}}, + {"params": {'min': -1}}, + {"params": {'min': 'one'}}, + {"params": {'before_end': 'invalid'}}) + @ddt.unpack + def test_create_reservation_with_invalid_param(self, params): values = { 'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c', - 'min': u'2', + 'min': 1, + 'max': 2, + 'before_end': 'default', 'hypervisor_properties': '["=", "$memory_mb", "256"]', 'resource_properties': '', 'start_date': datetime.datetime(2017, 3, 1, 20, 00), 'end_date': datetime.datetime(2017, 3, 2, 20, 00), - 'resource_type': plugin.RESOURCE_TYPE, - } - self.assertRaises( - manager_exceptions.MissingParameter, - self.fake_phys_plugin.reserve_resource, - u'441c1476-9f8f-4700-9f30-cd9b6fef3509', - values) - - def test_create_reservation_with_missing_param_properties(self): - values = { - 'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c', - 'min': u'1', - 'max': u'2', - 'start_date': datetime.datetime(2017, 3, 1, 20, 00), - 'end_date': datetime.datetime(2017, 3, 2, 20, 00), - 'resource_type': plugin.RESOURCE_TYPE, - } - self.assertRaises( - manager_exceptions.MissingParameter, - self.fake_phys_plugin.reserve_resource, - u'441c1476-9f8f-4700-9f30-cd9b6fef3509', - values) - - def test_create_reservation_with_invalid_param_max(self): - values = { - 'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c', - 'min': u'2', - 'max': u'three', - 'hypervisor_properties': '["=", "$memory_mb", "256"]', - 'resource_properties': '', - 'start_date': datetime.datetime(2017, 3, 1, 20, 00), - 'end_date': datetime.datetime(2017, 3, 2, 20, 00), - 'resource_type': plugin.RESOURCE_TYPE, - } + 'resource_type': plugin.RESOURCE_TYPE} + for key, value in params.items(): + values[key] = value self.assertRaises( manager_exceptions.MalformedParameter, self.fake_phys_plugin.reserve_resource, u'441c1476-9f8f-4700-9f30-cd9b6fef3509', values) - def test_create_reservation_with_invalid_param_before_end(self): + @ddt.data({"params": {'max': 0}}, + {"params": {'max': -1}}, + {"params": {'max': 'one'}}, + {"params": {'min': 0}}, + {"params": {'min': -1}}, + {"params": {'min': 'one'}}) + @ddt.unpack + def test_update_reservation_with_invalid_param(self, params): values = { 'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c', - 'min': u'1', - 'max': u'2', + 'min': 1, + 'max': 2, + 'before_end': 'default', + 'hypervisor_properties': '["=", "$memory_mb", "256"]', + 'resource_properties': '', + 'start_date': datetime.datetime(2017, 3, 1, 20, 00), + 'end_date': datetime.datetime(2017, 3, 2, 20, 00), + 'resource_type': plugin.RESOURCE_TYPE} + self.patch(self.db_api, 'reservation_get') + self.patch(self.db_api, 'lease_get') + host_reservation_get = self.patch(self.db_api, + 'host_reservation_get') + host_reservation_get.return_value = { + 'count_range': '1-1', + 'hypervisor_properties': '["=", "$memory_mb", "256"]', + 'resource_properties': '' + } + for key, value in params.items(): + values[key] = value + self.assertRaises( + manager_exceptions.MalformedParameter, + self.fake_phys_plugin.update_reservation, + u'441c1476-9f8f-4700-9f30-cd9b6fef3509', + values) + + def test_create_update_reservation_with_invalid_range(self): + values = { + 'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c', + 'min': 2, + 'max': 1, 'hypervisor_properties': '["=", "$memory_mb", "256"]', 'resource_properties': '', - 'before_end': 'invalid', 'start_date': datetime.datetime(2017, 3, 1, 20, 00), 'end_date': datetime.datetime(2017, 3, 2, 20, 00), 'resource_type': plugin.RESOURCE_TYPE, } - self.assertRaises( - manager_exceptions.MalformedParameter, - self.fake_phys_plugin.reserve_resource, - u'441c1476-9f8f-4700-9f30-cd9b6fef3509', - values) - - def test_create_reservation_with_invalid_range(self): - values = { - 'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c', - 'min': u'2', - 'max': u'1', + self.patch(self.db_api, 'reservation_get') + self.patch(self.db_api, 'lease_get') + host_reservation_get = self.patch(self.db_api, + 'host_reservation_get') + host_reservation_get.return_value = { + 'count_range': '1-1', 'hypervisor_properties': '["=", "$memory_mb", "256"]', - 'resource_properties': '', - 'start_date': datetime.datetime(2017, 3, 1, 20, 00), - 'end_date': datetime.datetime(2017, 3, 2, 20, 00), - 'resource_type': plugin.RESOURCE_TYPE, + 'resource_properties': '' } self.assertRaises( manager_exceptions.InvalidRange, self.fake_phys_plugin.reserve_resource, u'441c1476-9f8f-4700-9f30-cd9b6fef3509', values) + self.assertRaises( + manager_exceptions.InvalidRange, + self.fake_phys_plugin.update_reservation, + u'441c1476-9f8f-4700-9f30-cd9b6fef3509', + values) def test_update_reservation_shorten(self): values = {