Add support for cinder-manage online data migrations#624
Conversation
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/62ff833b1f554bd99cebdadb07f49999 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 21m 36s |
Enhanced the job interface to support multiple cinder-manage commands: - Added generic CinderManageJob() function with job type parameter - Implemented OnlineDataMigrationsJob() for upgrade scenarios - Maintained backward compatibility with existing DbSyncJob() - Added proper command routing and configuration logic This enables running cinder-manage db online_data_migrations during upgrades as documented in upstream OpenStack upgrade procedure [1]. [1] https://docs.openstack.org/cinder/latest/admin/upgrades.html Co-Authored-By: Claude <noreply@anthropic.com>
…job.go Simplified cinder-manage job execution by removing unnecessary kolla layer: - Updated DBSyncCommand to use direct cinder-manage command - Removed kolla-specific environment variables (KOLLA_CONFIG_STRATEGY, KOLLA_BOOTSTRAP) - Eliminated kolla config.json mount (/var/lib/kolla/config_files/config.json) - Renamed dbsync.go to job.go to better reflect multi-command purpose - Maintained existing volume structure with filtered config items Both dbsync and online_data_migrations now execute cinder-manage commands directly without kolla abstraction, simplifying execution and reducing dependencies while preserving backward compatibility. Co-Authored-By: Claude <noreply@anthropic.com>
Integrate online data migrations execution in cinder controller and update envTests to verify the new condition. This enables cinder-manage db online_data_migrations to run automatically during cinder deployment initialization, supporting proper OpenStack upgrade procedures as documented upstream. Co-Authored-By: Claude <noreply@anthropic.com>
|
I rebased this patch and I'm wondering if @abays || @stuggi can take a look at this. Do you have any concern about adding this step? [1] https://docs.openstack.org/cinder/latest/admin/upgrades.html#database-upgrades |
| }, | ||
| CinderOnlineDataMigration: types.NamespacedName{ | ||
| Namespace: cinderName.Namespace, | ||
| Name: fmt.Sprintf("%s-online-data-migrations", cinderName.Name), |
There was a problem hiding this comment.
Why is plural used in the name here when we're using singular everywhere else? Not a blocker by any means, just curious if I'm missing something.
There was a problem hiding this comment.
I think the plural is to reflect the original command cinder-manage db online_data_migrations, and because I originally called it online-data-migrations in job.go, I had to align the envTest accordingly. I could move it back to singular, but the original idea was to convey in the name the equivalent command run behind the scenes.
| } | ||
|
|
||
| dbSyncExtraMounts := []cinderv1beta1.CinderExtraVolMounts{} |
There was a problem hiding this comment.
This was formerly needed because of the shared GetVolumeMounts and GetVolumes functions, right? It wasn't because db syncs ever attached extra mounts? I'm just wondering if there could be a scenario where a previous version had extra mounts here and then for some reason db sync would run again and be missing the extra data.
There was a problem hiding this comment.
Correct, and because dbsync and any variation of cinder-manage command usually needs only db connection, I simplified this part to only get what's rendered in /etc/cinder/cinder.conf.d, with no kolla in the middle (we don't need it). ExtraMounts was part of that interface but it's not supposed to be used (hence the empty struct instance).
|
+1 from me. Will defer approval to let @stuggi have a look as well |
stuggi
left a comment
stuggi
left a comment
There was a problem hiding this comment.
/lgtm I think we need something similar across service operators for upgrades. good to have it as a reference on how we could do it.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fmount, stuggi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit f93527c
into
openstack-k8s-operators:main
|
Thanks both for the review!! |
3 participants
Enhance the existing job interface to support multiple
cinder-managecommands.The main purpose of this patch is to introduce the ability to run
online_data_migration, required in the upgrade context and described in the upstream documentation [1].For this reason, this patch:
CinderManageJob()function with job type parameterOnlineDataMigrationsJob(), useful for upgrade scenariosDbSyncJob()Jira: OSPRH-1559
[1] https://docs.openstack.org/cinder/latest/admin/upgrades.html