summaryrefslogtreecommitdiff
path: root/nova/db
Commit message (Collapse)AuthorAgeFilesLines
* db: Remove legacy migrationsStephen Finucane2023-02-0153-3262/+4
| | | | | | | | | sqlalchemy-migrate does not (and will not) support sqlalchemy 2.0. We need to drop these migrations to ensure we can upgrade our sqlalchemy version. Change-Id: I7756e393b78296fb8dbf3ca69c759d75b816376d Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* db: Drop redundant indexes on instances and console_auth_tokens tablesChristian Rohmann2022-09-152-2/+35
| | | | | | | | | | | | * There were two unique constrains on the same column uuid of instances. This change drops one of them. The second constraint was introduced with https://review.opendev.org/c/openstack/nova/+/97946, but was pending cleanup since. * In console_auth_tokens there was a unique constraint and another index on column token_hash. Closes-Bug: #1641185 Change-Id: I0ffa47d2afbfbfa63651991b3791dfad3e1832e1
* BlockDeviceMapping: Add encryption fieldsLee Yarwood2022-08-022-3/+68
| | | | | | | | This change adds the `encrypted`, `encryption_secret_uuid`, `encryption_format` and `encryption_options` to the BlockDeviceMapping object and associated table in the database. Change-Id: I6178c9bc249ef4d448de375dc16d82b2d087fc90
* Merge "db: Resolve additional SAWarning warnings"Zuul2022-04-291-0/+7
|\
| * db: Resolve additional SAWarning warningsStephen Finucane2022-04-081-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Resolving the following SAWarning warnings: Coercing Subquery object into a select() for use in IN(); please pass a select() construct explicitly SELECT statement has a cartesian product between FROM element(s) "foo" and FROM element "bar". Apply join condition(s) between each element to resolve. While the first of these was a trivial fix, the second one is a little more involved. It was caused by attempting to build a query across tables that had no relationship as part of our archive logic. For example, consider the following queries, generated early in '_get_fk_stmts': SELECT instances.uuid FROM instances, security_group_instance_association WHERE security_group_instance_association.instance_uuid = instances.uuid AND instances.id IN (__[POSTCOMPILE_id_1]) SELECT security_groups.id FROM security_groups, security_group_instance_association, instances WHERE security_group_instance_association.security_group_id = security_groups.id AND instances.id IN (__[POSTCOMPILE_id_1]) While the first of these is fine, the second is clearly wrong: why are we filtering on a field that is of no relevance to our join? These were generated because we were attempting to archive one or more instances (in this case, the instance with id=1) and needed to find related tables to archive at the same time. A related table is any table that references our "source" table - 'instances' here - by way of a foreign key. For each of *these* tables, we then lookup each foreign key and join back to the source table, filtering by matching entries in the source table. The issue here is that we're looking up every foreign key. What we actually want to do is lookup only the foreign keys that point back to our source table. This flaw is why we were generating the second SELECT above: the 'security_group_instance_association' has two foreign keys, one pointing to our 'instances' table but also another pointing to the 'security_groups' table. We want the first but not the second. Resolve this by checking if the table that each foreign key points to is actually the source table and simply skip if not. With this issue resolved, we can enable errors on SAWarning warnings in general without any filters. Change-Id: I63208c7bd5f9f4c3d5e4a40bd0f6253d0f042a37 Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* | db: Close connection on early returnStephen Finucane2022-04-221-1/+3
|/ | | | | | | | | During review of change Ic43c21038ee682f9733fbde42c6d24f8088815fc, we noticed that we were leaking connections if we had an early return from '_archive_deleted_rows_for_table'. Correct this. Change-Id: I748d962b6c7012e9bc2b8c91519da99d2d4bd240 Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* db: Don't rely on autocommit behaviorStephen Finucane2022-04-081-10/+18
| | | | | | | | | | | | | | | | | | Resolve the following RemovedIn20Warning warning: The current statement is being autocommitted using implicit autocommit, which will be removed in SQLAlchemy 2.0. Use the .begin() method of Engine or Connection in order to use an explicit transaction for DML and DDL statements. I genuinely expected this one to be more difficult to resolve, but we weren't using this as much as expected (thank you, non-legacy enginefacade). With this change, we appear to be SQLAlchemy 2.0 ready. Change-Id: Ic43c21038ee682f9733fbde42c6d24f8088815fc Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* db: Replace use of Column.copy() methodStephen Finucane2022-04-082-2/+6
| | | | | | | | | | | | | | | | Resolve the following RemovedIn20Warning warning: The Column.copy() method is deprecated and will be removed in a future release. The recommended solution here (by zzzeek himself) is to use the private method. This method isn't perfect (hence why the public version was deprecated) but it's more than okay for what we want. The alternative is to effectively vendor a variant of the 'Column.copy()' code, which means we'll lose out on any future bug fixes. Change-Id: Ia663251dfa7cf8f7d33f19902a92bcc586ae9f43 Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* db: Remove inplicit coercion of SELECTsStephen Finucane2022-04-081-2/+2
| | | | | | | | | | | | | Resolve the following RemovedIn20Warning warning: Implicit coercion of SELECT and textual SELECT constructs into FROM clauses is deprecated; please call .subquery() on any Core select or ORM Query object in order to produce a subquery object. This one was easy. Change-Id: Ifeab2aa8cef7ad151d5d5f92937e90ab34b96e8a Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* db: Replace use of strings in join, defer operationsStephen Finucane2022-04-081-113/+212
| | | | | | | | | | | | | | | | | | | | | Resolve the following RemovedIn20Warning warnings: Using strings to indicate column or relationship paths in loader options is deprecated and will be removed in SQLAlchemy 2.0. Please use the class-bound attribute directly. Using strings to indicate relationship names in Query.join() is deprecated and will be removed in SQLAlchemy 2.0. Please use the class-bound attribute directly. This is rather tricky to resolve. In most cases, we can simply make use of getattr to fetch the class-bound attribute, however, there are a number of places were we were doing "nested" joins, e.g. 'instances.info_cache' on the 'SecurityGroup' model. These need a little more thought. Change-Id: I1355ac92202cb504a7814afaa1338a4a511f9b54 Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* db: Remove use of 'bind' argumentsStephen Finucane2021-11-154-11/+21
| | | | | | | | | | | | | | | | | | Resolve the following RemovedIn20Warning warnings: The MetaData.bind argument is deprecated and will be removed in SQLAlchemy 2.0. The ``bind`` argument for schema methods that invoke SQL against an engine or connection will be required in SQLAlchemy 2.0. We must retain a couple of workarounds to keep sqlalchemy-migrate happy. This shouldn't be an issue since this library is not compatible with SQLAlchemy 2.x and will have to be removed (along with the legacy migration files themselves) when we eventually support 2.x. Change-Id: I946b4c7926ba2583b84ab95a1fe7aedc6923d9f8 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* db: Don't use legacy 'Row()' methodsStephen Finucane2021-11-121-1/+1
| | | | | | | | | | | | | | | Resolve the following RemovedIn20Warning warnings: The Row.keys() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. Use the namedtuple standard accessor Row._fields, or for full mapping behavior use row._mapping.keys() An additional warning that appears to have been resolved in the interim is also removed. Change-Id: I0c33130a745b986f1bcd2ec177f78e3bb68d2271 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* db: Don't pass strings to 'Connection.execute'Stephen Finucane2021-11-121-19/+26
| | | | | | | | | | | | Resolve the following RemovedIn20Warning warning: Passing a string to Connection.execute() is deprecated and will be removed in version 2.0. Use the text() construct, or the Connection.exec_driver_sql() method to invoke a driver-level SQL string. Change-Id: I44d6bf1ebfaf24f00a21389364456bceaae7c4d1 Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* db: Replace 'insert.inline' parameter with 'Insert.inline()' methodStephen Finucane2021-11-121-4/+4
| | | | | | | | | | Resolve the following RemovedIn20Warning warning: The insert.inline parameter will be removed in SQLAlchemy 2.0. Please use the Insert.inline() method. Change-Id: Ib5fcb841294b4e20fe085e8603d4132e97be7db9 Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* db: Replace use of legacy select() calling styleStephen Finucane2021-11-121-16/+23
| | | | | | | | | | | Resolve the following RemovedIn20Warning warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0. Please use the new calling style described at select(). Change-Id: I36e43e30e07f4904c7b49925cefe804af45cff6c Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* db: Replace use of 'autoload' parameterStephen Finucane2021-11-122-5/+8
| | | | | | | | | | | Resolve the following RemovedIn20Warning warnings: The autoload parameter is deprecated and will be removed in version 2.0. Please use the autoload_with parameter, passing an engine or connection. Change-Id: I10e9bbf14706aece2a59ebb5861004c5b852a592 Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* Merge "Set "cache_ok=True" in "TypeDecorator" inheriting classes"Zuul2021-11-111-0/+2
|\
| * Set "cache_ok=True" in "TypeDecorator" inheriting classesRodolfo Alonso Hernandez2021-10-141-0/+2
| | | | | | | | | | | | | | | | By setting this flag to True, it is assumed that the parameters passed to the object are safe to be used as a cache. Closes-Bug: #1942621 Change-Id: I582c89d8999db304923a51eb28c25f12046a4ebe
* | Merge "db: Remove nova-network models"Zuul2021-11-101-190/+15
|\ \
| * | db: Remove nova-network modelsStephen Finucane2021-11-031-190/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are a number of nova-network tables which we can now drop. nova-network Feature removed entirely in Ussuri. - dns_domains - fixed_ips - floating_ips - networks - provider_fw_rules - security_group_default_rules Unfortunately we can't get rid of the security group-related entries due to the unfortunate presence of the 'security_groups' attribute on the 'Instance' object and corresponding table, which in turn brings in a load of other tables. We'll address that separately. For now, just drop what we can easily drop. Change-Id: I8858faa14119f4daa9630b0ff6dcf082d0ff8fba Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* | | Merge "db: Remove models for removed services, features"Zuul2021-11-101-143/+20
|\ \ \ | |/ /
| * | db: Remove models for removed services, featuresStephen Finucane2021-11-031-143/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We've built up a large amount of tables that are no longer used for anything, given the removal of their users in past releases. We'd like to remove those, but before we do that we need to drop the models. This means there are no references to the tables come N+2, at which point we can remove the tables themselves. XenAPI virt driver Feature removed entirely in Victoria. - agent_builds - bw_usage_cache - console_pools - consoles Cells v1 Feature removed entirely in Train. - cells Volume Snapshots Feature removed entirely in Liberty. - snapshots EC2 API Feature removed entirely in Mitaka. Note that these tables are *not* used by the separate ec2-api project. - snapshot_id_mappings - volume_id_mappings There are still some tables related to nova-network left here. Those are unfortunately referenced from elsewhere, so we need to clean them up separately. Change-Id: I5e3d022fdf7328a1132f6e00998a3286b19be69a Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* | | Merge "objects: Remove 'bandwidth' fields from notifications"Zuul2021-11-101-83/+0
|\ \ \ | |/ /
| * | objects: Remove 'bandwidth' fields from notificationsStephen Finucane2021-11-031-83/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Finish up removing these entries from the versioned instance notifications. They're useless since we dropped support for the XenAPI virt driver. The underlying model is retained for now: that will be handled separately. Change-Id: I774c50fca99bc655ca5010e3b9d8247b739293b3 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* | | Merge "db: Remove models that were moved to the API database"Zuul2021-11-102-341/+22
|\ \ \ | |/ /
| * | db: Remove models that were moved to the API databaseStephen Finucane2021-11-032-341/+22
| | | | | | | | | | | | | | | | | | | | | | | | Remove all of the models that were moved to the API database many many cycles ago. Change-Id: Ib327f47b889dbccd5279f43c39203ed27689748b Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* | | Merge "Add autopep8 to tox and pre-commit"Zuul2021-11-081-1/+0
|\ \ \ | |/ / |/| |
| * | Add autopep8 to tox and pre-commitSean Mooney2021-11-081-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | autopep8 is a code formating tool that makes python code pep8 compliant without changing everything. Unlike black it will not radically change all code and the primary change to the existing codebase is adding a new line after class level doc strings. This change adds a new tox autopep8 env to manually run it on your code before you submit a patch, it also adds autopep8 to pre-commit so if you use pre-commit it will do it for you automatically. This change runs autopep8 in diff mode with --exit-code in the pep8 tox env so it will fail if autopep8 would modify your code if run in in-place mode. This allows use to gate on autopep8 not modifying patches that are submited. This will ensure authorship of patches is maintianed. The intent of this change is to save the large amount of time we spend on ensuring style guidlines are followed automatically to make it simpler for both new and old contibutors to work on nova and save time and effort for all involved. Change-Id: Idd618d634cc70ae8d58fab32f322e75bfabefb9d
* | | objects: Stop querying the main DB for keypairsStephen Finucane2021-10-181-72/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This was migrated to the API DB during the 14.0.0 (Newton) release [1] and the 345 migration introduced during the 15.0.0 (Ocata) release [2] ensures we should no longer have any entries left in the main database. The actual model isn't removed yet. That will be done separately. [1] I5f6d88fee47dd87de2867d3947d65b04f0b21e8f [2] Iab714d9e752c334cc1cc14a0d524cc9cf5d115dc Change-Id: I15efc38258685375284d8a97004777385023c6e8 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* | | db: Remove legacy placement modelsStephen Finucane2021-10-181-230/+17
|/ / | | | | | | | | | | | | | | These are now managed separately via the extracted placement project. We no longer need to include them in the API database. Change-Id: If18344034b31d9f6168e7a1c7d49bdcb3d53113e Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* | db: Remove unused build_requests columnsStephen Finucane2021-10-182-32/+46
| | | | | | | | | | | | | | | | These fields were never used in the API database. They can be removed now, some years after originally intended. Change-Id: I781875022d37d2c0626347f42c87707a29a9ab21 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* | db: De-duplicate list of removed table columnsStephen Finucane2021-10-184-50/+95
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We have a policy of removing fields from SQLAlchemy models at least one cycle before we remove the underlying database columns. This can result in a discrepancy between the state that our newfangled database migration tool, alembic, sees and what's actually in the database. We were ignoring these removed fields (and one foreign key constraint) in two different locations for both databases: as part of the alembic configuration and as part of the tests we have to ensure our migrations are in sync with our models (note: the tests actually use the alembic mechanism to detect the changes [1]). De-duplicate these. [1] https://github.com/sqlalchemy/alembic/issues/724#issuecomment-672081357 Change-Id: I978b4e44cf7f522a70cc5ca76e6d6f1a985d5469 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* | db: Enable auto-generation of API DB migrationsStephen Finucane2021-10-182-11/+67
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change I18846a5c7557db45bb63b97c7e8be5c4367e4547 enabled auto-generation of migrations for the main database. Let's now extend this to the API database using the same formula. While we're here, we also enable "batch" migrations for SQLite [1] by default, which allow us to work around SQLite's inability to support the ALTER statement for all but a limited set of cases. As noted in the documentation [2], this will have no impact on other backends where "we'd see the usual 'ALTER' statements done as though there were no batch directive". [1] https://stackoverflow.com/a/31140916/613428 [2] https://alembic.sqlalchemy.org/en/latest/batch.html Change-Id: I51c3a53286a0eced4bf57ad4fc13ac5f3616f7eb Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* | db: Add migration to resolve shadow table discrepanciesStephen Finucane2021-09-242-9/+77
|/ | | | | | | | | | | | | | | | | | | | | | | As part of the migration to alembic, a number of mistakes were noted in the migrations. These were all a result of doing things differently for the shadow table compared to the main table. Resolve some of these in a new migration to both address the bugs and test out the alembic machinery. Note that we don't actually resolve all of the mismatches. A large number of these mismatches were caused by the addition of 'nullable=False' to a column in a main table but not the equivalent shadow table. With the benefit of hindsight, this is probably wise and possibly even intended. If we were to apply the new constraint to the shadow table also, we'd need a data migration to populate any remaining NULL fields in the shadow tables. This data migration could be very costly and may not even possible as we don't necessarily have all available context required to do this population. As a result, the wording around these differences is changed to indicate that we will likely never address these "bugs". Change-Id: Icdc683d534a4fd35af399561253888abb9193c7d Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* Add missing __init__.py in nova/db/apiThomas Goirand2021-09-201-0/+0
| | | | | Change-Id: I455d91b17e17c9525112067e765b368150729262 Closes-Bug: #1944111
* Fix nova-manage db versionBalazs Gibizer2021-09-131-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | When nova switched to alembic implementation was added to nova-manage db version CLI to query the current db revision from alembic. However multiple mistake was made. The code called alembic_api.current[1] with an Engine object while that call expects a Config object instead. This leads to but/1943436. Also the same code expected that this call returns the revision. But that call just prints the revision to the standard output instead. So the implementations has been change from calling the alembic command API which is mostly created for CLI consumption to MigrationContext.get_current_revision() call that is intended to be used as a python API instead. [1] https://alembic.sqlalchemy.org/en/latest/api/commands.html#alembic.command.current Co-Authored-By: Sean Mooney <smooney@redhat.com> Closes-Bug: #1943436 Change-Id: I9fa7c03310c5bdb82e9a9c39727edb12eeae77f0
* Merge "db: Handle parameters in DB strings"Zuul2021-08-311-4/+9
|\
| * db: Handle parameters in DB stringsStephen Finucane2021-08-271-4/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As part of 'nova.db.migration', it is necessary to override the 'sqlalchemy.url' config option used by alembic. We were doing this but we weren't handling the fact that we could receive encoded URL strings. This causes issues for the ConfigParser instead used under the hood, as noted in the alembic docs [1]: value – the value. Note that this value is passed to ConfigParser.set, which supports variable interpolation using pyformat (e.g. %(some_value)s). A raw percent sign not part of an interpolation symbol must therefore be escaped, e.g. %%. The given value may refer to another value already in the file using the interpolation format. Resolve the issue by escaping % with %%. The engine.url is always encoded therefore this can be done unconditionally. [1] https://alembic.sqlalchemy.org/en/latest/api/config.html#alembic.config.Config.set_main_option Closes-Bug: #1940555 Change-Id: I74de55107a80af13df348f2bce49415b08028746 Signed-off-by: Stephen Finucane <stephenfin@redhat.com> Co-Authored-By: Sean Mooney <work@seanmooney.info>
* | Merge "Remove module level caching"Zuul2021-08-311-17/+7
|\ \ | |/
| * Remove module level cachingSean Mooney2021-08-271-17/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | This change removes the module level caches _MIGRATE_REPO and _ALEMBIC_CONF from nova.db.migration These caches are not correctly reset in current testing and dont currently provide much value as it is atypical for db_sync or db_version to be invoked multiple times outside of tests within the same process. As such the caches will typically be empty. Change-Id: I7a59c29f27dc16a0a2b3bb688a52da3c962a0b1b
* | Fix inactive session error in compute node creationMark Goddard2021-08-201-2/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the fix for bug 1839560 [1][2], soft-deleted compute nodes may be restored, to ensure we can reuse ironic node UUIDs as compute node UUIDs. While this seems to largely work, it results in some nasty errors being generated [3]: InvalidRequestError This session is in 'inactive' state, due to the SQL transaction being rolled back; no further SQL can be emitted within this transaction. This happens because compute_node_create is decorated with pick_context_manager_writer, which begins a transaction. While _compute_node_get_and_update_deleted claims that calling a second pick_context_manager_writer decorated function will begin a new subtransaction, this does not appear to be the case. This change removes pick_context_manager_writer from the compute_node_create function, and adds a new _compute_node_create function which ensures the transaction is finished if _compute_node_get_and_update_deleted is called. The new unit test added here fails without this change. This change marks the removal of the final FIXME from the functional test added in [4]. [1] https://bugs.launchpad.net/nova/+bug/1839560 [2] https://git.openstack.org/cgit/openstack/nova/commit/?id=89dd74ac7f1028daadf86cb18948e27fe9d1d411 [3] http://paste.openstack.org/show/786350/ [4] https://review.opendev.org/#/c/695012/ Change-Id: Iae119ea8776bc7f2e5dbe2e502a743217beded73 Closes-Bug: #1853159 Related-Bug: #1853009
* | Prevent deletion of a compute node belonging to another hostMark Goddard2021-08-201-5/+18
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | There is a race condition in nova-compute with the ironic virt driver as nodes get rebalanced. It can lead to compute nodes being removed in the DB and not repopulated. Ultimately this prevents these nodes from being scheduled to. The main race condition involved is in update_available_resources in the compute manager. When the list of compute nodes is queried, there is a compute node belonging to the host that it does not expect to be managing, i.e. it is an orphan. Between that time and deleting the orphan, the real owner of the compute node takes ownership of it ( in the resource tracker). However, the node is still deleted as the first host is unaware of the ownership change. This change prevents this from occurring by filtering on the host when deleting a compute node. If another compute host has taken ownership of a node, it will have updated the host field and this will prevent deletion from occurring. The first host sees this has happened via the ComputeHostNotFound exception, and avoids deleting its resource provider. Co-Authored-By: melanie witt <melwittt@gmail.com> Closes-Bug: #1853009 Related-Bug: #1841481 Change-Id: I260c1fded79a85d4899e94df4d9036a1ee437f02
* db: Enable auto-generation of migrationsStephen Finucane2021-08-0910-118/+118
| | | | | | | | | | | | Let's take advantage of the features that alembic provides [1]. We use this opportunity to clean up how we do our base model creation and remove the downgrade section from the migration script template, since we don't support downgrades. [1] https://alembic.sqlalchemy.org/en/latest/autogenerate.html Change-Id: I18846a5c7557db45bb63b97c7e8be5c4367e4547 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* db: Integrate alembicStephen Finucane2021-08-097-110/+186
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This looks more complicated than it is, but it's really quite simple. Essentially we have to deal with two possible configurations: - For existing deployments, the DB sync operation should apply any outstanding sqlalchemy-migrate-based migrations, dummy apply the initial alembic migration, and then apply any additional alembic-based migrations requested (or any available, if no version is specified). - For new deployments, the DB sync operation should apply the initial alembic migration and any additional alembic-based migrations requested (or any available, if no version is specified). No sqlalchemy-migrate-based migrations will ever be applied. While we continue to allow users to request a specific database migration version to upgrade to, we *do not* allow them to request a sqlalchemy-migrate-based migration version. There's no good reason to do this - the deployment won't run with an out-of-date DB schema (something that's also true of the alembic migration, fwiw) - and we want to get people off of sqlalchemy-migrate as fast as possible. A change in a future release can remove the sqlalchemy-migrate-based migrations once we're sure that they'll have upgraded to a release including all of the sqlalchemy-migrated-based migrations (so Wallaby). Tests are modified to validate the sanity of these operations. They're mostly trivial changes, but we do need to do some funky things to ensure that (a) we don't use logger configuration from 'alembic.ini' that will mess with our existing logger configuration and (b) we re-use connection objects as necessary to allow us to run tests against in-memory databases, where a different connection would actually mean a different database. We also can't rely on 'WalkVersionsMixin' from oslo.db since that only supports sqlalchemy-migrate [1]. We instead must re-invent the wheel here somewhat. [1] https://github.com/openstack/oslo.db/blob/10.0.0/oslo_db/sqlalchemy/test_migrations.py#L42-L44 Change-Id: I850af601f81bd5d2ecc029682ae10d3a07c936ce Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* db: Add initial alembic migration for API DBStephen Finucane2021-08-095-0/+823
| | | | | | | | | | | | The pattern here is the same as the one used for the main database in change I1fa2feaee78213ad81f1889ce54888696f58d98c. Once again, we're mostly copy-pasting from the existing schema. Reviewers can easily diff 'nova/db/api/legacy_migrations/versions/067_train.py' against 'nova/db/api/migrations/versions/d67eeaabee36_initial_version.py' in order to sanity check the new schema files. Change-Id: Ic65ac5cec46ace7bd49243d052deb2a434bb1099 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* db: Add initial alembic migration for main DBStephen Finucane2021-08-096-3/+1848
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This wasn't as complicated as feared. We're mostly able to copy-paste the existing sqlalchemy-migrate migration and simply changes the pattern of calls from monkey-patched 'create' methods such as: agent_builds = Table('agent_builds', meta, sa.Column('created_at', sa.DateTime), ... mysql_charset='utf8' ) agent_builds.create() to explicit alembic APIs like: op.create_table( 'agent_builds', sa.Column('created_at', sa.DateTime), ... mysql_charset='utf8' ) Reviewers are encouraged to diff the old migration file, 'nova/db/main/legacy_migrations/versions/402_train.py' against the new one, 'nova/db/main/migrations/versions/8f2f1571d55b_initial_version.py', to ease their job. The only significant divergences are the removal of a single reference to the 'migrate_version' table created by sqlalchemy-migrate, which obviously won't exist in an alembic-only world, along with some reordering of tables. The latter step is necessary since Alembic is not smart enough to correctly order the creation of tables so that tables that reference (via a foreign key) one or more other tables are created after the table(s) they reference. Since we now have to create tables using the 'create_table' API as noted above, rather than by creating a 'Table' instance and calling the sqlalchemy-migrate-provided 'create' API as we could previously, we must reorder our calls. The alternative would be to leave the creation of any 'ForeignKeyConstraint' until later but that seems no better and would arguably be harder to read. Note that this isn't yet wired up to anything: users can run it manually but the nova-manage commands we provide are still connected to the sqlalchemy-migrate commands. This will change shortly when we add shims to handle the conversion. Change-Id: I1fa2feaee78213ad81f1889ce54888696f58d98c Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* db: Avoid use of ALTER in initial migrationStephen Finucane2021-08-091-59/+68
| | | | | | | | | | | | | | | | | | | | | | | | | | | | We have a class of bugs in our current database schema that were introduced by now-squashed migrations, whereby we applied a new constraint to a column but forgot to apply the same change to the column in the shadow table. Currently, we preserve the behavior of these bugs through use of the 'ALTER' statement. This results in more complex SQL for the initial migration, however, more importantly, the ALTER statement is not actually supported by SQLite and the underlying migration engine must instead mimic behavior by creating a new table with the updated schema and a temporary name, copy across all data from the old table to the new table, delete the old table and rename the new table. While sqlalchemy-migrate handled this transparently for us, therefore allowing us to mostly ignore the issue, alembic makes this lack of support in SQLite explicit by insisting on the 'batch_alter_table' context manager [1] wrapping any ALTER statements. This is context manager is "a little tricky" (zzzeek's words, not mine) and is therefore best avoided. Fortunately, this is easily done through the addition of a couple of checks within the '_create_shadow_tables' helper that simply modify the affected columns before creating the table. [1] https://alembic.sqlalchemy.org/en/latest/ops.html#alembic.operations.Operations.batch_alter_table Change-Id: Ib3c8ed876791cda96486d43384f7aec0c487fd2f Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* db: Post reshuffle cleanupStephen Finucane2021-08-094-119/+179
| | | | | | | | | | | | Introduce a new 'nova.db.api.api' module to hold API database-specific helpers, plus a generic 'nova.db.utils' module to hold code suitable for both main and API databases. This highlights a level of complexity around connection management that is present for the main database but not for the API database. This is because we need to handle the complexity of cells for the former but not the latter. Change-Id: Ia5304c552ce552ae3c5223a2bfb3a9cd543ec57c Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* db: Move remaining 'nova.db.sqlalchemy' modulesStephen Finucane2021-08-0930-4/+2
| | | | | | | | The two remaining modules, 'api_models' and 'api_migrations', are moved to the new 'nova.db.api' module. Change-Id: I138670fe36b07546db5518f78c657197780c5040 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* db: Unify 'nova.db.api', 'nova.db.sqlalchemy.api'Stephen Finucane2021-08-096-1480/+19
| | | | | | | | | | | | | | | | | | | | | | Merge these, removing an unnecessary layer of abstraction, and place them in the new 'nova.db.main' directory. The resulting change is huge, but it's mainly the result of 's/sqlalchemy import api/main import api/' and 's/nova.db.api/nova.db.main.api/' with some necessary cleanup. We also need to rework how we do the blocking of API calls since we no longer have a 'DBAPI' object that we can monkey patch as we were doing before. This is now done via a global variable that is set by the 'main' function of 'nova.cmd.compute'. The main impact of this change is that it's no longer possible to set '[database] use_db_reconnect' and have all APIs automatically wrapped in a DB retry. Seeing as this behavior is experimental, isn't applied to any of the API DB methods (which don't use oslo.db's 'DBAPI' helper), and is used explicitly in what would appear to be the critical cases (via the explicit 'oslo_db.api.wrap_db_retry' decorator), this doesn't seem like a huge loss. Change-Id: Iad2e4da4546b80a016e477577d23accb2606a6e4 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
View Lorry Controller Status