Feat/per call nova microversion#8
Draft
msherman64 wants to merge 361 commits into
Draft
Conversation
Implements filter based architexture for usage enforcement. Two filters are included initially. MaxReservationsLengthFilter allows operators to define the maximum reservation length of a lease in seconds. The ExternalServiceFilter sends lease values and allocation candidates to an external HTTP service where policies can be defined. All filters must be enabled in blazar.conf in an enforcement group. When ending a lease, the allocations currently associated with the lease/reservation are sent to the enforcement layers to perform processing. This works fine when deleting a lease, but when a lease naturally ends, that is, when the current time is > the lease's recorded end time, the allocation lookup returns 0 results because we have a filter on the time of the lease/reservation by default, which starts from datetime.now. This is really not what we want to do when explicitly filtering on a lease/reservation, so this updates the generic query function to not apply this filtering in this case. It turns out, it was possible for usage enforcement's on_end hook to run multiple times if the first reservation teardown failed. In this circumstance, the end_lease event would be IN_PROGRESS but the reservations would be in an error state. The code only checks if any reservation is DELETED before trying to run on_end again. To fix this, only try to run on_end if we detected that there was an UNDONE end_lease event; that's the only time in which we'll actually be responsible for running this logic. This also updates the delete logic to be a bit more consistent with the internal way we track states. It is not enough to check dates, because it could be that Blazar never got around to actually starting the leases; we should be checking events. This also makes the logic a bit easier to understand. NOTE:: This also fixes something which has probably been a bug for a very long time. The 'end' event was never set to 'DONE' when a lease was terminated early by the user (via a deletion.) Change-Id: Ic106bee4fc3f5c401c4f8d8ecb1c4e735560bcc2 Co-Authored-By: Pierre Riteau <pierre@stackhpc.com> Co-Authored-By: Jason Anderson <jasonanderson@uchicago.edu> Implements: blueprint flexible-reservation-usage-enforcement
If a particular reservation throws an error during its tear down, the processing of the entire lease and all other reservations halts. This can put the system into a bad state over time. The right thing to do is attempt to tear down each reservation, but then raise an exception at the end of processing all events if any of them failed. TODO: add tests. Change-Id: I605094fb91b549e4ab285a85e6e91b17ebe9646b
Though there was DB functionality already written for this, it was never exposed at the API layer. This prevented hosts from having extra capabilities explicitly removed. To remove an extra capability, specify the capability name and set the value explicitly to `None` (when sending JSON, send `null`.) Existing restrictions apply; it is not possible to remove a capability in use explicitly by an active or pending reservation. Partial-Bug: #1674524 Change-Id: Ic0a555b6f8cd23d213189c1420781dd4e6d4e848
Change-Id: Ic8503479b1efebd29218b0cee08c993f286e5027
Change-Id: I0db77caafb672325eb7bb0903ed0e177aadfc497
Blazar is throwing an exception from a handler b/c it's trying to update a deleted event. Not quite sure how precisely this happens. Change-Id: I6cefc32abf11a20803d50ca7442c47e923739f73
The allocation_candidates function is used on lease create and update to get a list of allocations to send to usage enforcement. However, in the case of physical hosts, if not enough candidates existed, no error would be raised at this point, leading to the entire transaction of a lease create/update failing halfway through, but critically after already sending the information to the enforcement service, which never gets notified that the lease failed to create/update. Allocations should be checked _before_ sending to the enforcment service to make sure that there are enough in the system to satisfy the request. This updates the allocation_candidates function to do just that. It does not update the _matching_hosts helper function, as that is used elsewhere and is assumed to not throw an exception, letting the caller decide what to do based on the number of host_ids it returns. Change-Id: I41bb74e6201bf6e74b3113f7ad88037fd4dea676
Allocation candidates are supposed to be candidates which will be directly reserved as part of a lease, not all possible candidates. Filter the _matching_networks call down to a list of length 1, which is the current (non-configurable) amount of networks which can be reserved as part of a particular network reservation. This also updates it to raise an exception if the number of networks found was <1 Change-Id: Ia2bf660e6f5a316975ef7b9d2c33a61d71b65f78
It is often useful to have a reference to the lease when performing plugin start/end/before_end, in fact several plugins already fetch this information themselves. Lease will now be a keyword argument to all the on_* plugin calls. Change-Id: Ifd98cd7f66a8ebff86b92c567218e965f749171a
Change-Id: Ibd59b936445b5d0e86484bd3b2cbeef5e3d47fe5
Change-Id: I700630802e38c7c930d77f5d23786ad81c42f608
Blazar retrieves a context object from the trust recorded when a lease was created and sends it as part of notification messages. However, if the user who created the lease is deactivated or is not a member of the project anymore, operations modifying the lease and lease events fail to execute with an AuthorizationFailure exception. This is because the trust object is not valid anymore due to the change of permissions for the trustor. This patch removes the context from all notification messages and stops using trusts except for the initial lease creation. This allows project members and OpenStack admins to manage the lease after the creator user is disabled or has been removed from the project, as well as lease events to run properly. Change-Id: I895e9a54948ff5fd4dbfdad71c64631d71ef98fb Closes-Bug: #1712381
Blueprint: blazar-preemptible-instances Change-Id: Ic5fa07c4b1fffc763302b07e8a5f6fe0e15e019e
Change-Id: I039a899b94bdaacbd460aca633c3acfbe259e571
Change-Id: I454d30af37de004e2893dc5fe2c8555c44a98943
Change-Id: Ia32cf44a6a85dcf9dfa0909baab61821ffa35579
- add initial DB models for device resource type and associated entities (custom fields, allocations) - add api endpoints - add device reservation lifecycle functions
Change-Id: I4b8d7e517353cd49eb34e1baa0ce4d4ec70b93cc
Change-Id: I3155d6f41970aee2ebfc94dde85ac6171e4182a4
This should only try to sync RPs for devices that Blazar is managing. Change-Id: Ifdc08ddeb22f9c09edc7ba3a1179dec0aade7181
when lease ends, device is not cleaned up properly.
* Add authorized_projects check when matching devices * Update tests * Verify structure of authorized_projects field, remove extra DB query * Add test for is_project_allowed
If a device lease fails to start, the reservation trait is never added to the parent resource provider. If that lease is deleted, deleting the trait fails because it does not exist. This case will be handled by logging a message rather than failing.
when lease terminates (not deleted by user), there is no rpc call with authentication context. therefore, enforcement on_end fails with error "service identity not found". no enforcement filters will be executed. this fix gets the context from the lease trust.
Change-Id: Ia2a7244ae472bb4b17f2afcaed1ea8cbfd623fec
Change-Id: I072f95bdde4ef5802b124d4595b103de8cf00b2c
This field is a list of extra information to include with allocations so they can be used by the calendar. This should probably be configured by the config file.
Change-Id: I645c632b317893af2e8dfa7324b0d89ef5e6db91
Change-Id: I358c51a9a9d6dc5321b5bf79350da037496893a1
Change-Id: Ie33f8b45ebadfbececa3c0f867314e096243ab3e
We have added logic so that admins may bypass some restrictions on lease creation. However, leases are created in a trust context, and the generation of that context drops roles. Blazar policy enforcement checks for the "admin" role to verify admin-ness, so this means the admin exceptions don't work. This change gets the role names from the trust, pass it through, and verify that the "is_admin" enforcement check passes in a unit-test. Note: create_ctx_from_trust currently looks up values from the current request context `ctx`, the keystone session of the trust `session`, and the session's keystone auth_ref. For consistency, we should probably get everything except request_id and global_request_id from the auth_ref, just in case roles, usernames, or anything else changes betwen trust creation and the lease action being taken.
Change-Id: I4de654a544d2aa4315000805ecc30649eea9df2f
Change-Id: I1ed4ace22acd3ae088097febcca94206044b510e
unittests fail with error 'pkg_resources module not found', because it was removed from setuptools in version 82.0.0, and 2023.1 still relies on it. Pin setuptools<82 to ensure pkg_resources remains available.
Tests in stestr_known_failing.txt are passing, remove it.
When a lease request filters hosts by an extra capability, e.g. `node_type == compute_skylake`, host_get_all_by_queries did the bulk of the work in Python. This involves running `hosts_query.all()` which auto joins to computehost_extra_capabilities via `lazy=joined`. This caused the DB to return the entire table of all hosts and all capabilities, no matter what the query was. This whole-table fetch happens once per extra capability referenced, so it gets progressively more expensive as queries are added (gpus>4, arch=amd64,...). Even in the single-query case, with ~300 hosts the time to load results from the db into python dominated, leading to 100% cpu usage at 1 req/s. This patch moves the matching logic from python into sqlalchemy clauses, letting the DB return only matching hosts.
Add tests for host_get_all_by_queries covering the extra-capability logic: - subset selection across several hosts, including that `!=` matches a host whose value differs but excludes a host that lacks the property entirely (`!=` is not the complement of `==`) - two capability predicates ANDed together - a capability predicate followed by a column predicate
In NovaInventory.get_servers_per_host(), blazar tries to look up whether any nova servers are scheduled to a specific nova hypervisor. However, nova hypervisors may be identified by hypervisor uuid, the hostname of the compute service for that hypervisor (host), or the hostname of the hypervisor itself (hypervisor_hostname). For virtualized nova deployments, host==hypervisor_hostname, but for Ironic backed hosts, many ironic nodes share a single compute service. Nova's api for hypervisors.search with servers=true will return servers for ALL hypervisors sharing a given compute service, rather than the specific one queried. We have carried a patch in Nova to fall back to matching by hypervisor_hostname as well, that is brittle. Since this is the only place in blazar consuming this particular api edge case, we can just do the lookup in two steps. First look up the nova hypervisor by id or hypervisor_hostname, and then look up servers using Nova's servers.list API, which explicitly supports filtering by hypervisor_hostname via the `node` keyword. If we don't need to preserve the "raise on 0 or >1 hosts found" behavior, the call to hypervisors.search could be dropped, simplifying the logic further.
In FakeNovaHypervisors.search(), the mock implies that Nova's os-hypervisors search API can filter by the computehost service name, which is incorrect. The existing tests passed because the service name (`fake_name`) was a substring of (`fake_name.openstack.org`), and the api does support filtering hypervisor_hostname by substring. This removes the incorrect mocked behavior, and changes service name to `fake_service` to disambiguate it from the hypervisor_hostname.
Adds test cases for the alembic migration tree, asserting a single head and single base revision. Pattern is copied from Nova's "tests/unit/db/main/test_migrations.py". With a bad migration, we will get an output like: `testtools.matchers._impl.MismatchError: 1 != 2: expected a single head, found: 553383923ca0, 4fe5e44880da`
on_start and on_end read the project from context.current(), but they run in the manager's event processor, where the current context is the service's, not the lease owner's. This works upstream because they use trusts to impersonate the user, but the Chameleon fork removed that mechanism in commit 5b89659, and 100be4a restored it only for end_lease, but not on_start. This change just reads the project from the lease, which the manager is already passing to the event.
In the past, blazar used to use the same host ID as the nova ID. We stopped doing this, but never updated the nova host monitor code, since we only use ironic hosts. The ironic host handler already uses this. Change-Id: Ic435bc0538070bebe6d357fac443bffaab56bb3b (cherry picked from commit a862af8)
Having a pyproject.toml file means that pip install --editable won't create .egg files. Since 2023.1 devstack doesn't support pep660, this breaks devstack installation of our fork. devstack supports pyproject.toml starting 2024.1 openstack/devstack@4ddd456
blazar-manager called db_api.setup_db() on every start, which runs metadata.create_all(). This silently creates any missing tables, but bypasses alembic_versioning. If a manager with new models starts before `blazar-db-manage upgrade head` is run, the new tables will exist and cause conflicts with the alembic migrations. Alembic should already be the only thing creating and updating the db schema, so this change shouldn't break anything. The call appears to be left over from a long time ago, but since it only causes problem when blazar manager "races" alembic, it could have been missed. Starting the manager against an unmigrated database now fails with missing-table errors instead of silently creating them.
Blueprint: blazar-preemptible-instances Change-Id: Ic5fa07c4b1fffc763302b07e8a5f6fe0e15e019e (cherry picked from commit 9264905)
Add a backward-compatible version= param to BlazarNovaClient/NovaClientWrapper so the flavor plugin can pin microversion 2.55 (required to set a flavor description) without bumping the global nova_client_version.
msherman64
force-pushed
the
feat/per_call_nova_microversion
branch
from
July 24, 2026 00:41
ea6b429 to
6d3b9d5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.