From a038592cb614630d33d20b23942c6c927cf944b7 Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Mon, 1 Jul 2024 12:19:07 +0100 Subject: [PATCH 01/10] Extend enforcement to add resources With the flavor reservation plugin, it has made some calcuation on what the flavor chosen means in terms of resources. This is exactly the sort of information that is needed by an external enfrocement service, to track how many resource hours are being comsumted by each reservation. As such, we find a way to send this extra information to the external enforcement service. Change-Id: I168a2e90735226676cd573fc1a3c316efde1f6d1 --- blazar/enforcement/enforcement.py | 25 ++++-- blazar/manager/service.py | 55 +++++++++++-- blazar/plugins/base.py | 5 ++ blazar/plugins/flavor/flavor_plugin.py | 60 ++++++++------ blazar/tests/enforcement/test_enforcement.py | 49 +++++++---- blazar/tests/manager/test_service.py | 11 ++- .../plugins/flavor/test_flavor_plugin.py | 82 ++++++++++++++----- 7 files changed, 209 insertions(+), 78 deletions(-) diff --git a/blazar/enforcement/enforcement.py b/blazar/enforcement/enforcement.py index 232f1fdd2..3f70294b8 100644 --- a/blazar/enforcement/enforcement.py +++ b/blazar/enforcement/enforcement.py @@ -59,7 +59,8 @@ def format_context(self, context, lease_values): project_id=lease_values['project_id'], auth_url=auth_url, region_name=region_name) - def format_lease(self, lease_values, reservations, allocations): + def format_lease(self, lease_values, reservations, allocations, + resource_requests=None): lease = lease_values.copy() lease['reservations'] = [] @@ -69,31 +70,39 @@ def format_lease(self, lease_values, reservations, allocations): res['allocations'] = allocations[resource_type] lease['reservations'].append(res) + if resource_requests: + lease['resource_requests'] = resource_requests + return lease - def check_create(self, context, lease_values, reservations, allocations): + def check_create(self, context, lease_values, reservations, allocations, + resource_requests=None): context = self.format_context(context, lease_values) - lease = self.format_lease(lease_values, reservations, allocations) + lease = self.format_lease( + lease_values, reservations, allocations, resource_requests) for _filter in self.enabled_filters: _filter.check_create(context, lease) def check_update(self, context, current_lease, new_lease, current_allocations, new_allocations, - current_reservations, new_reservations): + current_reservations, new_reservations, + current_resource_requests=None, + new_resource_requests=None): context = self.format_context(context, current_lease) current_lease = self.format_lease(current_lease, current_reservations, - current_allocations) + current_allocations, + current_resource_requests) new_lease = self.format_lease(new_lease, new_reservations, - new_allocations) + new_allocations, new_resource_requests) for _filter in self.enabled_filters: _filter.check_update(context, current_lease, new_lease) - def on_end(self, context, lease, allocations): + def on_end(self, context, lease, allocations, resource_requests=None): context = self.format_context(context, lease) lease_values = self.format_lease(lease, lease['reservations'], - allocations) + allocations, resource_requests) for _filter in self.enabled_filters: _filter.on_end(context, lease_values) diff --git a/blazar/manager/service.py b/blazar/manager/service.py index d19ac59f6..8f4f5e2c9 100644 --- a/blazar/manager/service.py +++ b/blazar/manager/service.py @@ -378,8 +378,11 @@ def create_lease(self, lease_values): allocations = self._allocation_candidates( lease_values, reservations) try: + resource_requests = self._get_enforcement_resources( + lease_values, reservations) self.enforcement.check_create( - context.current(), lease_values, reservations, allocations) + context.current(), lease_values, reservations, allocations, + resource_requests) except common_ex.NotAuthorized as e: LOG.error("Enforcement checks failed. %s", str(e)) raise common_ex.NotAuthorized(e) @@ -562,10 +565,17 @@ def update_lease(self, lease_id, values): new_allocs = existing_allocs try: - self.enforcement.check_update(context.current(), lease, values, - existing_allocs, new_allocs, + current_resource_requests = self._get_enforcement_resources( + values, existing_reservations) + new_resource_requests = self._get_enforcement_resources( + values, new_reservations) + self.enforcement.check_update(context.current(), lease, + values, existing_allocs, + new_allocs, existing_reservations, - new_reservations) + new_reservations, + current_resource_requests, + new_resource_requests) except common_ex.NotAuthorized as e: LOG.error("Enforcement checks failed. %s", str(e)) raise e @@ -682,7 +692,10 @@ def delete_lease(self, lease_id): # lease is no longer in play. allocations = self._existing_allocations(reservations) try: - self.enforcement.on_end(ctx, lease, allocations) + resource_requests = self._get_enforcement_resources( + lease, reservations) + self.enforcement.on_end(ctx, lease, allocations, + resource_requests) except Exception as e: LOG.error(e) @@ -716,7 +729,10 @@ def end_lease(self, lease_id, event_id): with trusts.create_ctx_from_trust(lease['trust_id']) as ctx: allocations = self._existing_allocations(lease['reservations']) try: - self.enforcement.on_end(ctx, lease, allocations) + resource_requests = self._get_enforcement_resources( + lease, lease['reservations']) + self.enforcement.on_end(ctx, lease, allocations, + resource_requests) except Exception as e: LOG.error(e) @@ -807,6 +823,33 @@ def _allocation_candidates(self, lease, reservations): return allocations + def _get_enforcement_resources(self, lease, reservations): + """Returns dict by resource type of reservation candidates.""" + resources = defaultdict(int) + + for reservation in reservations: + res = reservation.copy() + resource_type = reservation['resource_type'] + res['start_date'] = lease['start_date'] + res['end_date'] = lease['end_date'] + + if resource_type not in self.plugins: + raise exceptions.UnsupportedResourceType( + resource_type=resource_type) + + plugin = self.plugins.get(resource_type) + + if not plugin: + raise common_ex.BlazarException( + 'Invalid plugin names are specified: %s' % resource_type) + + reservation_resources = plugin.get_enforcement_resources(res) + + for resource, amount in reservation_resources.items(): + resources[resource] += amount + + return resources + def _existing_allocations(self, reservations): allocations = {} diff --git a/blazar/plugins/base.py b/blazar/plugins/base.py index 74e54def6..4accc2963 100644 --- a/blazar/plugins/base.py +++ b/blazar/plugins/base.py @@ -87,6 +87,11 @@ def allocation_candidates(self, lease_values): """Get candidates for reservation allocation.""" pass + def get_enforcement_resources(self, reservation_values): + """Get enforcement details for the lease.""" + # default to the existing behavior of sending lease values + return {} + @abc.abstractmethod def update_reservation(self, reservation_id, values): """Update reservation.""" diff --git a/blazar/plugins/flavor/flavor_plugin.py b/blazar/plugins/flavor/flavor_plugin.py index b80f26cea..107b18b8c 100644 --- a/blazar/plugins/flavor/flavor_plugin.py +++ b/blazar/plugins/flavor/flavor_plugin.py @@ -70,9 +70,10 @@ def allocation_candidates(self, reservation): def _pick_hosts(self, reservation): self._validate_reservation_params(reservation) - flavor_id = reservation['flavor_id'] - resource_request, resource_traits, source_flavor = \ - self._get_flavor_details(flavor_id) + source_flavor = self._get_flavor_details(reservation) + + resource_request, resource_traits = \ + self._estimate_flavor_resources(source_flavor) affinity = strutils.bool_from_string( reservation['affinity'], default=None) @@ -94,6 +95,7 @@ def _pick_hosts(self, reservation): while len(candidates) > req_amount: candidates.pop() host_ids = [host['id'] for host in candidates] + return host_ids, source_flavor def _validate_reservation_params(self, values): @@ -204,25 +206,29 @@ def has_free_slot(): return hosts_list def _get_cached_flavor(self, instance_reservation): - source_flavor = instance_reservation["resource_properties"] + source_flavor = instance_reservation.get("resource_properties") if source_flavor and "OS-FLV-EXT-DATA:ephemeral" in source_flavor: return json.loads(source_flavor) + def _get_total_resource_request(self, instance_reservation, source_flavor): + request_count = instance_reservation["amount"] + flavor_resource_inventory, _ = \ + self._estimate_flavor_resources(source_flavor) + return { + rc: amount * request_count + for rc, amount in flavor_resource_inventory.items() + if amount > 0 + } + def _max_usages(self, reservations): """For reservation list for a host, find resource high watermark.""" def resource_usage_by_event(event): instance_reservation = event['reservation']['instance_reservation'] - request_count = instance_reservation["amount"] source_flavor = self._get_cached_flavor(instance_reservation) - if source_flavor: - flavor_resource_inventory, _ = \ - self._estimate_flavor_resources(source_flavor) - return { - rc: amount * request_count - for rc, amount in flavor_resource_inventory.items() - if amount > 0 - } - raise mgr_exceptions.ReservationTypeConflict() + if not source_flavor: + raise mgr_exceptions.ReservationTypeConflict() + return self._get_total_resource_request( + instance_reservation, source_flavor) # Get sorted list of events for all reservations # that exist in the target time window @@ -257,23 +263,14 @@ def resource_usage_by_event(event): f"max is: {max_usage}") return max_usage - def _get_flavor_details(self, flavor_id): - # access nova using the user token, - # to ensure we can only see flavors they can see + def _get_flavor_details(self, instance_reservation): + flavor_id = instance_reservation['flavor_id'] user_client = nova.NovaClientWrapper() flavor = user_client.nova.nova.flavors.get(flavor_id) source_flavor = flavor.to_dict() # TODO(johngarbutt): use newer api to get this above source_flavor["extra_specs"] = flavor.get_keys() - if 'links' in source_flavor.keys(): - del source_flavor['links'] - - # NOTE(johngarbutt): we are only partially reproducing all the - # options that are available in a flavor. - resource_request, resource_traits = \ - self._estimate_flavor_resources(source_flavor) - - return (resource_request, resource_traits, source_flavor) + return source_flavor def _estimate_flavor_resources(self, source_flavor): resource_request = {} @@ -329,6 +326,7 @@ def reserve_resource(self, reservation_id, values): 'affinity': None, 'resource_properties': json.dumps(source_flavor) } + LOG.debug(f"instance_reservation_val {instance_reservation_val}") instance_reservation = db_api.instance_reservation_create( instance_reservation_val) @@ -419,3 +417,13 @@ def on_start(self, resource_id): def on_end(self, resource_id): self._instance_plugin.on_end(resource_id) + + def get_enforcement_resources(self, reservation_values): + source_flavor = self._get_cached_flavor(reservation_values) + if not source_flavor: + # when called before writing to db, fetch flavor again + source_flavor = self._get_flavor_details(reservation_values) + if not source_flavor: + raise mgr_exceptions.ReservationTypeConflict() + return self._get_total_resource_request( + reservation_values, source_flavor) diff --git a/blazar/tests/enforcement/test_enforcement.py b/blazar/tests/enforcement/test_enforcement.py index 08208abde..0a76d437f 100755 --- a/blazar/tests/enforcement/test_enforcement.py +++ b/blazar/tests/enforcement/test_enforcement.py @@ -146,13 +146,16 @@ def setUp(self): def tearDown(self): super(EnforcementTestCase, self).tearDown() - def get_formatted_lease(self, lease_values, rsv, allocs): + def get_formatted_lease(self, lease_values, rsv, allocs, + resource_requests=None): expected_lease = lease_values.copy() if rsv: expected_lease['reservations'] = rsv for res in expected_lease['reservations']: res['allocations'] = allocs[res['resource_type']] + if resource_requests: + expected_lease['resource_requests'] = resource_requests return expected_lease @@ -177,25 +180,30 @@ def test_format_context(self): def test_format_lease(self): lease_values, rsv, allocs = get_lease_rsv_allocs() + resource_requests = {"VCPUS": 4, "MEMORY_MB": 8192, "DISK_GB": 10} - formatted_lease = self.enforcement.format_lease(lease_values, rsv, - allocs) + formatted_lease = self.enforcement.format_lease( + lease_values, rsv, allocs, resource_requests) - expected_lease = self.get_formatted_lease(lease_values, rsv, allocs) + expected_lease = self.get_formatted_lease(lease_values, rsv, allocs, + resource_requests) self.assertDictEqual(expected_lease, formatted_lease) def test_check_create(self): lease_values, rsv, allocs = get_lease_rsv_allocs() ctx = context.current() + resource_requests = {"VCPUS": 4, "MEMORY_MB": 8192, "DISK_GB": 10} check_create = self.patch(self.enforcement.enabled_filters[0], 'check_create') - self.enforcement.check_create(ctx, lease_values, rsv, allocs) + self.enforcement.check_create(ctx, lease_values, rsv, allocs, + resource_requests) formatted_lease = self.enforcement.format_lease(lease_values, rsv, - allocs) + allocs, + resource_requests) formatted_context = self.enforcement.format_context(ctx, lease_values) check_create.assert_called_once_with(formatted_context, @@ -205,7 +213,8 @@ def test_check_create(self): region_name=self.region, auth_url='https://fakeauth.com') - expected_lease = self.get_formatted_lease(lease_values, rsv, allocs) + expected_lease = self.get_formatted_lease(lease_values, rsv, allocs, + resource_requests) self.assertDictEqual(expected_context, formatted_context) self.assertDictEqual(expected_lease, formatted_lease) @@ -222,7 +231,8 @@ def test_check_create_with_exception(self): self.assertRaises(exceptions.BlazarException, self.enforcement.check_create, context=ctx, lease_values=lease_values, - reservations=rsv, allocations=allocs) + reservations=rsv, allocations=allocs, + resource_requests=None) def test_check_update(self): lease, rsv, allocs = get_lease_rsv_allocs() @@ -230,6 +240,7 @@ def test_check_update(self): new_lease_values = get_fake_lease(end_date='2014-02-07 13:37') new_reservations = list(new_lease_values['reservations']) allocation_candidates = {'virtual:instance': [get_fake_host('2')]} + resources_requests = {"VCPUS": 4, "MEMORY_MB": 8192, "DISK_GB": 10} del new_lease_values['reservations'] ctx = context.current() @@ -239,20 +250,24 @@ def test_check_update(self): self.enforcement.check_update( ctx, lease, new_lease_values, allocs, allocation_candidates, - rsv, new_reservations) + rsv, new_reservations, resources_requests, resources_requests) formatted_context = self.enforcement.format_context(ctx, lease) - formatted_lease = self.enforcement.format_lease(lease, rsv, allocs) + formatted_lease = self.enforcement.format_lease(lease, rsv, allocs, + resources_requests) new_formatted_lease = self.enforcement.format_lease( - new_lease_values, new_reservations, allocation_candidates) + new_lease_values, new_reservations, allocation_candidates, + resources_requests) expected_context = dict(user_id='111', project_id='222', region_name=self.region, auth_url='https://fakeauth.com') - expected_lease = self.get_formatted_lease(lease, rsv, allocs) + expected_lease = self.get_formatted_lease(lease, rsv, allocs, + resources_requests) expected_new_lease = self.get_formatted_lease( - new_lease_values, new_reservations, allocation_candidates) + new_lease_values, new_reservations, allocation_candidates, + resources_requests) check_update.assert_called_once_with( formatted_context, formatted_lease, new_formatted_lease) @@ -283,16 +298,17 @@ def test_check_update_with_exception(self): def test_on_end(self): allocations = {'virtual:instance': [get_fake_host('1')]} + resource_requests = {"VCPUS": 4, "MEMORY_MB": 8192, "DISK_GB": 10} lease = get_fake_lease() ctx = context.current() on_end = self.patch(self.enforcement.enabled_filters[0], 'on_end') - self.enforcement.on_end(ctx, lease, allocations) + self.enforcement.on_end(ctx, lease, allocations, resource_requests) formatted_context = self.enforcement.format_context(ctx, lease) formatted_lease = self.enforcement.format_lease( - lease, lease['reservations'], allocations) + lease, lease['reservations'], allocations, resource_requests) on_end.assert_called_once_with(formatted_context, formatted_lease) @@ -300,7 +316,8 @@ def test_on_end(self): region_name=self.region, auth_url='https://fakeauth.com') - expected_lease = self.get_formatted_lease(lease, None, allocations) + expected_lease = self.get_formatted_lease(lease, None, allocations, + resource_requests) self.assertDictEqual(expected_context, formatted_context) self.assertDictEqual(expected_lease, formatted_lease) diff --git a/blazar/tests/manager/test_service.py b/blazar/tests/manager/test_service.py index 7a6a5769a..6105a5f26 100644 --- a/blazar/tests/manager/test_service.py +++ b/blazar/tests/manager/test_service.py @@ -490,10 +490,17 @@ def test_list_leases(self): def test_create_lease_now(self): lease_values = self.lease_values - lease = self.manager.create_lease(lease_values) + resources = { + "CUSTOM_FAKE": 3, "VCPU": 1 + } + self.fake_plugin.get_enforcement_resources.return_value = resources - self.enforcement.check_create.assert_called_once() + lease = self.manager.create_lease(lease_values) + self.enforcement.check_create.assert_called_once_with( + self.context.current(), lease_values, mock.ANY, mock.ANY, + resources + ) self.trust_ctx.assert_called_once_with(lease_values['trust_id']) self.lease_create.assert_called_once_with(lease_values) self.assertEqual(lease, self.lease) diff --git a/blazar/tests/plugins/flavor/test_flavor_plugin.py b/blazar/tests/plugins/flavor/test_flavor_plugin.py index 20194480e..283dfb978 100644 --- a/blazar/tests/plugins/flavor/test_flavor_plugin.py +++ b/blazar/tests/plugins/flavor/test_flavor_plugin.py @@ -60,8 +60,10 @@ def test_query_allocations(self, mock_query): self.assertEqual("fake", result) mock_query.assert_called_once_with(['123'], '2001', None) + @mock.patch.object(flavor_plugin.FlavorPlugin, + '_estimate_flavor_resources') @mock.patch.object(flavor_plugin.FlavorPlugin, '_get_flavor_details') - def test_allocation_candidates(self, mock_get_flavor): + def test_allocation_candidates(self, mock_get_flavor, mock_resources): self._create_fake_host() fake_inventory_values = { 'computehost_id': 123, @@ -82,17 +84,21 @@ def test_allocation_candidates(self, mock_get_flavor): 'start_date': datetime.datetime(2030, 1, 1, 8, 00), 'end_date': datetime.datetime(2030, 1, 1, 12, 00) } - mock_get_flavor.return_value = ({"PCPU": 2}, {}, - {"flavor_id": "fake"}) + mock_resources.return_value = ({"PCPU": 2}, {}) + mock_flavor = {"flavor_id": "fake"} + mock_get_flavor.return_value = mock_flavor result = plugin.allocation_candidates(reservation) self.assertEqual(4, len(result)) - mock_get_flavor.assert_called_once_with( - "34eb7166-0e9b-432c-96fd-dff37f22e36e") + mock_get_flavor.assert_called_once_with(reservation) + mock_resources.assert_called_once_with(mock_flavor) + @mock.patch.object(flavor_plugin.FlavorPlugin, + '_estimate_flavor_resources') @mock.patch.object(flavor_plugin.FlavorPlugin, '_get_flavor_details') - def test_allocation_candidates_fails_no_space(self, mock_get_flavor): + def test_allocation_candidates_fails_no_space(self, mock_get_flavor, + mock_resources): self._create_fake_host() fake_inventory_values = { 'computehost_id': 123, @@ -113,17 +119,20 @@ def test_allocation_candidates_fails_no_space(self, mock_get_flavor): 'start_date': datetime.datetime(2030, 1, 1, 8, 00), 'end_date': datetime.datetime(2030, 1, 1, 12, 00) } - mock_get_flavor.return_value = ({"PCPU": 2}, {}, - {"flavor_id": "fake"}) + mock_get_flavor.return_value = {"flavor_id": "fake"} + mock_resources.return_value = ({"PCPU": 2}, {}) self.assertRaises(mgr_exceptions.NotEnoughHostsAvailable, plugin.allocation_candidates, reservation) + @mock.patch.object(flavor_plugin.FlavorPlugin, + '_estimate_flavor_resources') @mock.patch.object(flavor_plugin.FlavorPlugin, '_create_resources') @mock.patch.object(flavor_plugin.FlavorPlugin, '_get_flavor_details') def test_allocation_candidates_avoids_reservations(self, mock_get_flavor, - mock_create): + mock_create, + mock_resources): self._create_fake_host() fake_inventory_values = { 'computehost_id': 123, @@ -133,7 +142,7 @@ def test_allocation_candidates_avoids_reservations(self, mock_get_flavor, 'min_unit': 1, 'max_unit': 10, 'step_size': 1, - 'allocation_ratio': 1.0 + 'allocation_ratio': 1.0, } db_api.host_resource_inventory_create(fake_inventory_values) plugin = flavor_plugin.FlavorPlugin() @@ -142,7 +151,7 @@ def test_allocation_candidates_avoids_reservations(self, mock_get_flavor, 'amount': 3, 'affinity': None, 'start_date': datetime.datetime(2030, 1, 1, 8, 00), - 'end_date': datetime.datetime(2030, 1, 1, 12, 00) + 'end_date': datetime.datetime(2030, 1, 1, 12, 00), } fake_flavor = { "disk": 0, # GiB @@ -152,10 +161,10 @@ def test_allocation_candidates_avoids_reservations(self, mock_get_flavor, "ram": 0, # MB "swap": 0, "vcpus": 2, - "extra_specs": {'hw:cpu_policy': 'dedicated'} + "extra_specs": {'hw:cpu_policy': 'dedicated'}, } - mock_get_flavor.return_value = ({"PCPU": 2}, {}, - fake_flavor) + mock_get_flavor.return_value = fake_flavor + mock_resources.return_value = ({"PCPU": 2}, {}) old_reservation = new_reservation.copy() old_reservation['amount'] = 2 fake_phys_reservation = new_reservation.copy() @@ -217,14 +226,12 @@ def test__get_flavor_details(self, mock_get): 'resources:VGPU': '1', } mock_flavor.get_keys.return_value = fake_extra_specs + fake_instance_reservation = { + "flavor_id": "34eb7166-0e9b-432c-96fd-dff37f22e36e" + } - resource_request, resource_traits, source_flavor = \ - plugin._get_flavor_details("34eb7166-0e9b-432c-96fd-dff37f22e36e") + source_flavor = plugin._get_flavor_details(fake_instance_reservation) - self.assertDictEqual({ - 'DISK_GB': 10, 'MEMORY_MB': 1024, 'PCPU': 1, 'VCPU': 0, 'VGPU': 1 - }, resource_request) - self.assertDictEqual({'HW_CPU_X86_AVX': 'required'}, resource_traits) expected = fake_flavor.copy() expected["extra_specs"] = fake_extra_specs self.assertDictEqual(expected, source_flavor) @@ -404,3 +411,38 @@ def test__query_available_hosts(self): } ret = plugin._query_available_hosts(**query_params) self.assertEqual(2, len(ret)) + + @mock.patch.object(flavor_plugin.FlavorPlugin, 'get_enforcement_resources') + @mock.patch.object(flavors.FlavorManager, 'create') + def test_get_enforcement_resources(self, mock_create_flavor, + mock_get_resources): + plugin = flavor_plugin.FlavorPlugin() + fake_flavor = { + "disk": 10, # GiB + "OS-FLV-EXT-DATA:ephemeral": 100, # GiB + "id": "34eb7166-0e9b-432c-96fd-dff37f22e36e", + "name": "test1", + "ram": 1024, # MB + "swap": 0, + "vcpus": 2, + "extra_specs": {} + } + fake_reservation = { + 'reservation_id': "12345", + 'vcpus': fake_flavor["vcpus"], + 'memory_mb': fake_flavor["ram"], + 'disk_gb': fake_flavor["disk"], + 'amount': 2, + 'affinity': None, + 'resource_properties': json.dumps(fake_flavor) + } + expected_resources = { + 'DISK_GB': 220, + 'MEMORY_MB': 2048, + 'VCPU': 4 + } + mock_get_resources.return_value = expected_resources + mock_flavor = mock.Mock() + mock_create_flavor.return_value = mock_flavor + rsv = plugin.get_enforcement_resources(fake_reservation) + self.assertEqual(rsv, expected_resources) From e807f3e5c5664caf1d1d3f0fd843188c249b8165 Mon Sep 17 00:00:00 2001 From: scrungus Date: Thu, 25 Jul 2024 17:58:57 +0100 Subject: [PATCH 02/10] Support bearer token for external enforcement This change allows for users to configure blazar to call out to an external enforcement API with a bearer token. This is very handy when you create an enforcement API using the Django Rest Framework. Change-Id: I772db7c85620d7a9f0fc1b7aa10f73a96bc86a5f --- .../filters/external_service_filter.py | 16 +++++++++++----- .../filters/test_external_service_filter.py | 19 +++++++++++++++++++ doc/source/admin/usage-enforcement.rst | 3 ++- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/blazar/enforcement/filters/external_service_filter.py b/blazar/enforcement/filters/external_service_filter.py index 1b5850398..70f29f0a3 100644 --- a/blazar/enforcement/filters/external_service_filter.py +++ b/blazar/enforcement/filters/external_service_filter.py @@ -67,7 +67,11 @@ class ExternalServiceFilter(base_filter.BaseFilter): cfg.StrOpt( 'external_service_token', default="", - help='Token used for authentication with the external service.') + help='Token used for authentication with the external service.'), + cfg.StrOpt( + 'external_service_bearer_token', + default="", + help='Bearer token for authentication with the external service.') ] def __init__(self, conf=None): @@ -96,7 +100,8 @@ def __init__(self, conf=None): raise ExternalServiceMisconfigured( message=_("ExternalService has no endpoints set.")) - self.token = conf.enforcement.external_service_token + self.x_auth_token = conf.enforcement.external_service_token + self.bearer_token = conf.enforcement.external_service_bearer_token @staticmethod def _validate_url(url): @@ -124,9 +129,10 @@ def _construct_url(self, method, replacement_url): def _get_headers(self): headers = {'Content-Type': 'application/json'} - if self.token: - headers['X-Auth-Token'] = self.token - + if self.x_auth_token: + headers['X-Auth-Token'] = self.x_auth_token + elif self.bearer_token: + headers['Authorization'] = "Bearer " + self.bearer_token return headers def _post(self, url, body): diff --git a/blazar/tests/enforcement/filters/test_external_service_filter.py b/blazar/tests/enforcement/filters/test_external_service_filter.py index 9aee96a62..b3e57f4d3 100644 --- a/blazar/tests/enforcement/filters/test_external_service_filter.py +++ b/blazar/tests/enforcement/filters/test_external_service_filter.py @@ -167,6 +167,25 @@ def test_check_create_allowed(self, post_mock): data='{"context": {"is_context": true}, ' '"lease": {"is_lease": true}}') + @mock.patch("requests.post") + def test_check_create_allowed_token(self, post_mock): + CONF.set_override("external_service_bearer_token", "bearer_token", + group='enforcement') + self.addCleanup(CONF.clear_override, + 'external_service_bearer_token', + group='enforcement') + self.filter = external_service_filter.ExternalServiceFilter(CONF) + post_mock.return_value = FakeResponse204() + + self.filter.check_create(self.ctx, self.lease) + + post_mock.assert_called_with( + "http://localhost/check-create", + headers={'Content-Type': 'application/json', + 'Authorization': 'Bearer bearer_token'}, + data='{"context": {"is_context": true}, ' + '"lease": {"is_lease": true}}') + @mock.patch("requests.post") def test_check_create_denied(self, post_mock): post_mock.return_value = FakeResponse403WithMessage() diff --git a/doc/source/admin/usage-enforcement.rst b/doc/source/admin/usage-enforcement.rst index b37d940bb..b0e082ff1 100644 --- a/doc/source/admin/usage-enforcement.rst +++ b/doc/source/admin/usage-enforcement.rst @@ -61,7 +61,8 @@ ExternalServiceFilter This filter delegates the decision for each API to an external HTTP service. The service must use token-based authentication, accepting (or ignoring) -the static token sent by Blazar in the ``X-Auth-Token`` header. +the static token sent by Blazar in either the ``X-Auth-Token`` header, or +the ``Authorization: Bearer`` header, depending on your configuration. The following endpoints should be implemented: * ``POST /check-create`` From 728ef497f33166a40abe25a32087f3b457a26ca1 Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Thu, 26 Sep 2024 17:04:48 +0100 Subject: [PATCH 03/10] Ensure check_create gets the lease_id To keep track of resource usage in an external enforcement service, its really important for the lease_id to be passed to the external service. Change-Id: I953cf73891f27bd649ba27db9769f11f8fe98d59 --- blazar/manager/service.py | 23 ++++++++++++++--------- blazar/tests/manager/test_service.py | 12 +++++++++--- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/blazar/manager/service.py b/blazar/manager/service.py index 8f4f5e2c9..c07e36835 100644 --- a/blazar/manager/service.py +++ b/blazar/manager/service.py @@ -377,15 +377,6 @@ def create_lease(self, lease_values): allocations = self._allocation_candidates( lease_values, reservations) - try: - resource_requests = self._get_enforcement_resources( - lease_values, reservations) - self.enforcement.check_create( - context.current(), lease_values, reservations, allocations, - resource_requests) - except common_ex.NotAuthorized as e: - LOG.error("Enforcement checks failed. %s", str(e)) - raise common_ex.NotAuthorized(e) events.append({'event_type': 'start_lease', 'time': start_date, @@ -430,6 +421,20 @@ def create_lease(self, lease_values): with save_and_reraise_exception(): LOG.exception('Cannot create a lease') else: + # check enforcement only after the lease_id + # has been created + try: + lease_values['id'] = lease['id'] + resource_requests = self._get_enforcement_resources( + lease_values, reservations) + self.enforcement.check_create( + context.current(), lease_values, reservations, + allocations, resource_requests) + except common_ex.NotAuthorized as e: + db_api.lease_destroy(lease_id) + LOG.error("Enforcement checks failed. %s", str(e)) + raise common_ex.NotAuthorized(e) + try: for reservation in reservations: reservation['lease_id'] = lease['id'] diff --git a/blazar/tests/manager/test_service.py b/blazar/tests/manager/test_service.py index 6105a5f26..bab40f4e7 100644 --- a/blazar/tests/manager/test_service.py +++ b/blazar/tests/manager/test_service.py @@ -489,16 +489,20 @@ def test_list_leases(self): self.lease_list.assert_called_once_with() def test_create_lease_now(self): - lease_values = self.lease_values + lease_values = self.lease_values.copy() + del lease_values['id'] resources = { "CUSTOM_FAKE": 3, "VCPU": 1 } self.fake_plugin.get_enforcement_resources.return_value = resources + self.lease_create.return_value = self.lease lease = self.manager.create_lease(lease_values) + lease_values_extra = lease_values.copy() + lease_values_extra['id'] = self.lease['id'] self.enforcement.check_create.assert_called_once_with( - self.context.current(), lease_values, mock.ANY, mock.ANY, + self.context.current(), lease_values_extra, mock.ANY, mock.ANY, resources ) self.trust_ctx.assert_called_once_with(lease_values['trust_id']) @@ -776,6 +780,7 @@ def test_create_lease_without_required_params(self): def test_create_lease_with_filter_exception(self): lease_values = self.lease_values.copy() + self.lease_create.return_value = self.lease self.enforcement.check_create.side_effect = ( enforcement_ex.MaxLeaseDurationException(lease_duration=200, @@ -784,7 +789,8 @@ def test_create_lease_with_filter_exception(self): self.assertRaises(exceptions.NotAuthorized, self.manager.create_lease, lease_values=lease_values) - self.lease_create.assert_not_called() + self.assertEqual(1, self.lease_create.call_count) + self.lease_destroy.assert_called_once_with(self.lease["id"]) def test_update_lease_completed_lease_rename(self): lease_values = {'name': 'renamed'} From 827cdb048c7700e623cd9281f334d6e3f69eef93 Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Fri, 27 Sep 2024 18:17:52 +0100 Subject: [PATCH 04/10] Simplify create_lease by removing some indenting The follow on change was cusing problems with the tox complexity checker. This gives that change some head room. Change-Id: Ic2378b6b7e856f99b31fcb2c8eb1dd1e0742b05d --- blazar/manager/service.py | 87 +++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 44 deletions(-) diff --git a/blazar/manager/service.py b/blazar/manager/service.py index c07e36835..c8fcdffba 100644 --- a/blazar/manager/service.py +++ b/blazar/manager/service.py @@ -420,53 +420,52 @@ def create_lease(self, lease_values): except db_ex.BlazarDBException: with save_and_reraise_exception(): LOG.exception('Cannot create a lease') - else: - # check enforcement only after the lease_id - # has been created - try: - lease_values['id'] = lease['id'] - resource_requests = self._get_enforcement_resources( - lease_values, reservations) - self.enforcement.check_create( - context.current(), lease_values, reservations, - allocations, resource_requests) - except common_ex.NotAuthorized as e: + + # check enforcement only after the lease_id + # has been created + try: + lease_values['id'] = lease['id'] + resource_requests = self._get_enforcement_resources( + lease_values, reservations) + self.enforcement.check_create( + context.current(), lease_values, reservations, + allocations, resource_requests) + except common_ex.NotAuthorized as e: + db_api.lease_destroy(lease_id) + LOG.error("Enforcement checks failed. %s", str(e)) + raise common_ex.NotAuthorized(e) + + try: + for reservation in reservations: + reservation['lease_id'] = lease['id'] + reservation['start_date'] = lease['start_date'] + reservation['end_date'] = lease['end_date'] + self._create_reservation(reservation) + except Exception: + with save_and_reraise_exception(): + LOG.exception("Failed to create reservation for a " + "lease. Rollback the lease and " + "associated reservations") db_api.lease_destroy(lease_id) - LOG.error("Enforcement checks failed. %s", str(e)) - raise common_ex.NotAuthorized(e) - try: - for reservation in reservations: - reservation['lease_id'] = lease['id'] - reservation['start_date'] = lease['start_date'] - reservation['end_date'] = lease['end_date'] - self._create_reservation(reservation) - except Exception: - with save_and_reraise_exception(): - LOG.exception("Failed to create reservation for a " - "lease. Rollback the lease and " - "associated reservations") - db_api.lease_destroy(lease_id) + try: + for event in events: + event['lease_id'] = lease['id'] + db_api.event_create(event) + except (exceptions.UnsupportedResourceType, + common_ex.BlazarException): + with save_and_reraise_exception(): + LOG.exception("Failed to create event for a lease. " + "Rollback the lease and associated " + "reservations") + db_api.lease_destroy(lease_id) - try: - for event in events: - event['lease_id'] = lease['id'] - db_api.event_create(event) - except (exceptions.UnsupportedResourceType, - common_ex.BlazarException): - with save_and_reraise_exception(): - LOG.exception("Failed to create event for a lease. " - "Rollback the lease and associated " - "reservations") - db_api.lease_destroy(lease_id) - - else: - db_api.lease_update( - lease_id, - {'status': status.lease.PENDING}) - lease = db_api.lease_get(lease_id) - self._send_notification(lease, ctx, events=['create']) - return lease + db_api.lease_update( + lease_id, + {'status': status.lease.PENDING}) + lease = db_api.lease_get(lease_id) + self._send_notification(lease, ctx, events=['create']) + return lease def _add_resource_type(self, reservations, existing_reservations): rsvns_by_id = {} From db69073828f76dadc2a7d915484ee1f2a6f9bcbc Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Fri, 27 Sep 2024 18:20:48 +0100 Subject: [PATCH 05/10] Add extra on_end calls on reservation failure If we use check_create to keep state about reserverations we need an on_end callback to be attempted when the lease is deleted early due to a failure, such as reserve_resource failing due to some race with another reservation request. In other error cases, we would expect a lease db object to still be created at the end of the create lease call, such that delete will be called that triggers on_end. One alternative is to move check_create after operations such as reserve_resource, but in that case we need to make all of the plugins capable of cleaning up the resources they have just created, which seems a bad idea when it is not uncommon that a request is rejected due to the enforcement logic. Change-Id: If4ff1c8da32c3be47bf1523058f83effd7340f02 --- blazar/manager/service.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/blazar/manager/service.py b/blazar/manager/service.py index c8fcdffba..fddb95f0e 100644 --- a/blazar/manager/service.py +++ b/blazar/manager/service.py @@ -447,6 +447,9 @@ def create_lease(self, lease_values): "lease. Rollback the lease and " "associated reservations") db_api.lease_destroy(lease_id) + self._call_enforcement_on_end( + context.current(), lease_values, reservations, + allocations) try: for event in events: @@ -459,6 +462,9 @@ def create_lease(self, lease_values): "Rollback the lease and associated " "reservations") db_api.lease_destroy(lease_id) + self._call_enforcement_on_end( + context.current(), lease_values, reservations, + allocations) db_api.lease_update( lease_id, @@ -467,6 +473,16 @@ def create_lease(self, lease_values): self._send_notification(lease, ctx, events=['create']) return lease + def _call_enforcement_on_end(self, ctx, lease, reservations, allocations): + # allow external enforcement know create reservation failed + try: + resource_requests = self._get_enforcement_resources( + lease, reservations) + self.enforcement.on_end(ctx, lease, allocations, + resource_requests) + except Exception as e: + LOG.error(e) + def _add_resource_type(self, reservations, existing_reservations): rsvns_by_id = {} From 097ba6746061f4978305f05f6e92800c69014218 Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Mon, 30 Sep 2024 13:46:04 +0100 Subject: [PATCH 06/10] WIP: add dry_run option on lease create Option to just do the enforcement check and resource allocations check, but not actually create the resources. Change-Id: I150e3f0caad1ab5196d54e75d0ff366ec66797a7 --- blazar/api/v1/leases/service.py | 4 ++++ blazar/manager/service.py | 14 ++++++++++++++ blazar/tests/manager/test_service.py | 21 +++++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/blazar/api/v1/leases/service.py b/blazar/api/v1/leases/service.py index 0a63762b9..a3cafac7a 100644 --- a/blazar/api/v1/leases/service.py +++ b/blazar/api/v1/leases/service.py @@ -14,6 +14,7 @@ # limitations under the License. from oslo_log import log as logging +from oslo_utils import strutils from blazar import context from blazar.manager.leases import rpcapi as manager_rpcapi @@ -54,6 +55,9 @@ def create_lease(self, data): # two lines ctx = context.current() data['user_id'] = ctx.user_id + # TODO(johngarbutt) we need to agree a senible API! + data["dry_run"] = strutils.bool_from_string( + data.get('dry_run', "false")) return self.manager_rpcapi.create_lease(data) @policy.authorize('leases', 'get') diff --git a/blazar/manager/service.py b/blazar/manager/service.py index fddb95f0e..22dbe5a40 100644 --- a/blazar/manager/service.py +++ b/blazar/manager/service.py @@ -408,6 +408,20 @@ def create_lease(self, lease_values): self._update_before_end_event_date(event, before_end_date, lease_values) + # Exit early if this was just a dry run + if lease_values.get("dry_run", False): + resource_requests = self._get_enforcement_resources( + lease_values, reservations) + # Make sure enforcement is happy with the request + self.enforcement.check_create( + context.current(), lease_values, reservations, + allocations, resource_requests) + # Tell the exteral system we change our mind + self._call_enforcement_on_end( + context.current(), lease_values, reservations, + allocations) + return None + try: if trust_id: lease_values.update({'trust_id': trust_id}) diff --git a/blazar/tests/manager/test_service.py b/blazar/tests/manager/test_service.py index bab40f4e7..704b6ce83 100644 --- a/blazar/tests/manager/test_service.py +++ b/blazar/tests/manager/test_service.py @@ -515,6 +515,27 @@ def test_create_lease_now(self): notifier_api.format_lease_payload(lease), 'lease.create') + def test_create_lease_dry_run(self): + lease_values = self.lease_values.copy() + del lease_values['id'] + lease_values['dry_run'] = True + resources = { + "CUSTOM_FAKE": 3, "VCPU": 1 + } + self.fake_plugin.get_enforcement_resources.return_value = resources + self.lease_create.return_value = self.lease + + result = self.manager.create_lease(lease_values) + + self.assertIsNone(result) + self.enforcement.check_create.assert_called_once_with( + self.context.current(), lease_values, mock.ANY, mock.ANY, + resources + ) + self.trust_ctx.assert_called_once_with(self.lease_values['trust_id']) + self.lease_create.assert_not_called() + self.fake_notifier.assert_not_called() + def test_create_lease_some_time(self): lease_values = self.lease_values.copy() self.lease['start_date'] = '2026-11-13 13:13' From 57aad94ee00798aa34c3fe0d228c4861fa4a7003 Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Tue, 5 Nov 2024 18:00:07 +0000 Subject: [PATCH 07/10] Don't fail hard on missing traits --- blazar/plugins/flavor/flavor_plugin.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/blazar/plugins/flavor/flavor_plugin.py b/blazar/plugins/flavor/flavor_plugin.py index 107b18b8c..eb6bcfe76 100644 --- a/blazar/plugins/flavor/flavor_plugin.py +++ b/blazar/plugins/flavor/flavor_plugin.py @@ -118,9 +118,11 @@ def _query_available_hosts(self, start_date, end_date, # we should be able to exclude hosts that don't match the # resource requests, e.g. baremetal vs virtual # or missing traits - if resource_traits: - raise mgr_exceptions.NotImplemented( - error="Resource traits not supported yet") + # TODO(johngarbutt) explore supporting custom traits + # used to group hosts, e.g. for different types of hardware + # if resource_traits: + # raise mgr_exceptions.NotImplemented( + # error="Resource traits not supported yet") hosts = db_api.reservable_host_get_all_by_queries([]) # find reservations for each host in our time period From fbcaed93cd21f456e5e07473b644dcc6e28d964b Mon Sep 17 00:00:00 2001 From: stackhpc-ci <22933334+stackhpc-ci@users.noreply.github.com> Date: Tue, 4 Feb 2025 16:29:17 +0000 Subject: [PATCH 08/10] feat: automatic update of workflows stackhpc/master --- .github/workflows/tag-and-release.yml | 12 ++++++++++++ .github/workflows/tox.yml | 7 +++++++ 2 files changed, 19 insertions(+) create mode 100644 .github/workflows/tag-and-release.yml create mode 100644 .github/workflows/tox.yml diff --git a/.github/workflows/tag-and-release.yml b/.github/workflows/tag-and-release.yml new file mode 100644 index 000000000..cff2f940d --- /dev/null +++ b/.github/workflows/tag-and-release.yml @@ -0,0 +1,12 @@ +--- +name: Tag & Release +'on': + push: + branches: + - stackhpc/master +permissions: + actions: read + contents: write +jobs: + tag-and-release: + uses: stackhpc/.github/.github/workflows/tag-and-release.yml@main diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml new file mode 100644 index 000000000..8713f0e02 --- /dev/null +++ b/.github/workflows/tox.yml @@ -0,0 +1,7 @@ +--- +name: Tox Continuous Integration +'on': + pull_request: +jobs: + tox: + uses: stackhpc/.github/.github/workflows/tox.yml@main From c83d8e502d138225a7c9d930ef39e35803e2a8a3 Mon Sep 17 00:00:00 2001 From: stackhpc-ci <22933334+stackhpc-ci@users.noreply.github.com> Date: Tue, 4 Feb 2025 16:29:50 +0000 Subject: [PATCH 09/10] feat: automatic update of community files stackhpc/master --- .github/CODEOWNERS | 1 + 1 file changed, 1 insertion(+) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 000000000..a914011ae --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1 @@ +* @stackhpc/openstack From 1dc71eece3a2c50c6d6f11e2636cb0eeee259a5b Mon Sep 17 00:00:00 2001 From: Seunghun Lee Date: Wed, 5 Feb 2025 11:47:24 +0000 Subject: [PATCH 10/10] Add C901 to ignore list of pep8 C901 Function is too complex is catching ManagerService.create_lease() because the function has more than 5 potential branches. Because the function is in WIP in master branch, ignore C901 at least until the function is in stable state. --- tox.ini | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 52374cd75..c4544d263 100644 --- a/tox.ini +++ b/tox.ini @@ -64,7 +64,10 @@ exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,tools,api-ref # W503 line break before binary operator # W504 line break after binary operator # E402 module level import not at top of file -ignore=H105,H238,E123,E402,W503,W504 +# NOTE(seunghun1ee): Remove C901 when ManagerService.create_lease() +# in /blazar/manager/service.py get refactored +# C901 Function is too complex +ignore=H105,H238,E123,E402,W503,W504,C901 # [H904] Delay string interpolations at logging calls. enable-extensions=H904 # To get a list of functions that have a complexity of 17 or more, set