| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-23 | |||
| 19:14:21 | opendevreview | Jay Faulkner proposed openstack/nova stable/ussuri: Ignore plug_vifs on the ironic driver https://review.opendev.org/c/openstack/nova/+/821351 | |
| 19:15:37 | JayF | https://review.opendev.org/c/openstack/nova/+/853546 (to stable/train) needs one more core review, then this one will be backported as far as I plan to take it | |
| 19:16:26 | opendevreview | Jay Faulkner proposed openstack/nova stable/yoga: nova-live-migration tests not needed for Ironic https://review.opendev.org/c/openstack/nova/+/854257 | |
| 19:17:41 | opendevreview | Jay Faulkner proposed openstack/nova stable/yoga: nova-live-migration tests not needed for Ironic https://review.opendev.org/c/openstack/nova/+/854257 | |
| 19:18:36 | opendevreview | Jay Faulkner proposed openstack/nova stable/ussuri: Ignore plug_vifs on the ironic driver https://review.opendev.org/c/openstack/nova/+/821351 | |
| 19:20:11 | JayF | elodilles: took a couple of tries, but I think I got them right now ^^ had to do two rounds, first used `-x` then I realized I needed the `-X` :D | |
| 19:20:19 | JayF | ty for the guidance | |
| 19:22:40 | JayF | https://review.opendev.org/c/openstack/nova/+/821351 and https://review.opendev.org/c/openstack/nova/+/854257 should both be good for reviews now, no-change-backports of stuff already merged, shouldn't be controversial :D | |
| 19:22:53 | JayF | thank you all again for helping me plow thru this ironic-driver-backport tech debt | |
| 22:08:26 | melwitt | sean-k-mooney, gibi: I'm off today and tomorrow, thanks for getting the revert done and sorry for the trouble ☹️ | |
| #openstack-nova - 2022-08-24 | |||
| 03:52:06 | sean-k-mooney[m] | melwitt: sorry didnt realise that enjoy your time off | |
| 05:26:30 | auniyal__ | Hi O/ | |
| 05:26:37 | auniyal__ | please review these | |
| 05:26:39 | auniyal__ | https://review.opendev.org/c/openstack/nova/+/853811 | |
| 05:26:53 | auniyal__ | https://review.opendev.org/c/openstack/nova/+/853812 | |
| 06:46:09 | crohmann | Hey lovely nova folks. I was just about to raise a bug about duplicate indices for tables of Nova and Placement, but then found an old, but unfixed bug: https://bugs.launchpad.net/nova/+bug/1641185 | |
| 06:47:58 | crohmann | Since this is already assigned to ABHAY (since 2018) I believe this might be under the radar. Any chance this could be reassigned or place onto the list of "open isuses" ? | |
| 06:52:33 | crohmann | This also appears to have a simple fix in removing the double definitions of colums as primary indexes as well as them having a unique constraint. | |
| 07:21:37 | gibi | crohmann: hi! thanks for checking before reporting a new bug. Do you plan to proposa a fix? | |
| 07:22:07 | gibi | if so, then feel free to reassing the bug | |
| 07:22:35 | gibi | (or I can reassing it to you if you don't have the rights) | |
| 07:39:53 | crohmann | gibi: I did assign me and also raised a bug for placement at https://storyboard.openstack.org/#!/story/2010251. | |
| 07:40:45 | crohmann | Regarding a fix ... I suppose there are two sides: Fixing the schema for new installs, but also dropping them for existing ones, right? | |
| 07:41:04 | crohmann | "them" = the duplicate index | |
| 07:44:52 | gibi | crohmann: you are correct | |
| 07:45:41 | gibi | you need to drop it from the schema and also propose a db schema migration to drop it from existing dbs during upgrade | |
| 09:04:38 | opendevreview | Gorka Eguileor proposed openstack/nova master: Support os-brick specific lock_path https://review.opendev.org/c/openstack/nova/+/849328 | |
| 09:08:41 | opendevreview | Gorka Eguileor proposed openstack/nova master: Support os-brick specific lock_path https://review.opendev.org/c/openstack/nova/+/849328 | |
| 09:11:22 | opendevreview | Amit Uniyal proposed openstack/nova master: Adds check for VM snapshot fail while quiesce https://review.opendev.org/c/openstack/nova/+/852171 | |
| 09:15:37 | sean-k-mooney | the unique key is the one that should be kept | |
| 09:19:29 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support move operations with PCI tracking in placement https://review.opendev.org/c/openstack/nova/+/854247 | |
| 09:20:45 | opendevreview | ribaudr proposed openstack/nova master: Default Nova persistent objects without soft delete. https://review.opendev.org/c/openstack/nova/+/854355 | |
| 09:23:16 | sean-k-mooney | gibi: since the update userdata feature will need a new trait and a new os-traits reelase i think we shoudl swap the microverions for that and rebuild | |
| 09:23:32 | Uggla | gibi, I did that change ^ then I want to modify act objects like aggregate that do not requires soft delete. Unfortunately it changes the API output. So I guess we need a new microversion at minimum. But do we need a "deprecation" cycle too ? | |
| 09:24:01 | sean-k-mooney | Uggla: waht are you chagining | |
| 09:24:30 | gibi | sean-k-mooney: we have "time" until friday to push an os-traits change. But if the rebuild series is ready then I have no objection to swap | |
| 09:25:02 | sean-k-mooney | i have not reviewed it so i cant say | |
| 09:25:25 | Uggla | sean-k-mooney, https://review.opendev.org/c/openstack/nova/+/854355 trying to make persistent objects without soft delete "by default" | |
| 09:25:35 | gibi | Uggla: let me look at it. For sean-k-mooney, the new ShareMapping object needs to be non-soft-deletable and that needs some new baseclass for Nova ovos as the current one adds the deleted_at field | |
| 09:26:06 | sean-k-mooney | right but we dont need to change all the others | |
| 09:26:11 | gibi | sean-k-mooney: re rebuild series: me neither so for me both userdata and rebuild is in the grey zone but if somebody says that rebuild is ready to land then I'm OK to swap | |
| 09:26:17 | sean-k-mooney | we could eventually | |
| 09:26:31 | gibi | sean-k-mooney: re ovo: yes, we only need to change the base class for the new ShareMapping | |
| 09:27:26 | sean-k-mooney | right so https://review.opendev.org/c/openstack/nova/+/854355/1/nova/objects/base.py#142 is wrong | |
| 09:27:43 | sean-k-mooney | we shoudl not modify the NovaPersistentObject | |
| 09:28:02 | sean-k-mooney | we shoudl leage that the same and add a seperate one that does not use soft delete | |
| 09:28:31 | gibi | ^^ agree | |
| 09:29:06 | sean-k-mooney | maybe call it NovaPersistentObjectHardDelete for now | |
| 09:30:04 | gibi | wondering that the problem is only that the base class change changes the object signature used for versioning | |
| 09:30:31 | sean-k-mooney | it will change the ovo shas | |
| 09:30:41 | Uggla | sean-k-mooney, gibi it means we will keep objects with the removal of delete deleted_at "tricks" forever ? | |
| 09:30:51 | sean-k-mooney | but if Uggla updated them all to point to NovaPersistentSoftDeleteObject it woudl be fine | |
| 09:31:13 | sean-k-mooney | Uggla: if we remove soft delete it will be a sperrate spec | |
| 09:31:14 | Uggla | sean-k-mooney, no the current changes does not change the shas | |
| 09:31:23 | sean-k-mooney | its not something you should do in your current one | |
| 09:33:46 | gibi | Uggla: then what is the exact API change you are worried about? | |
| 09:34:59 | gibi | Uggla: I don't see the reason why the aggregate API output would differ | |
| 09:35:06 | gibi | after your patch | |
| 09:35:23 | gibi | ohh | |
| 09:35:39 | Uggla | gibi, in another patch I wanted to clean the aggregate object | |
| 09:35:41 | gibi | so the aggregate is not soft deleted | |
| 09:35:46 | Uggla | yep | |
| 09:35:53 | gibi | leave that alone for now :) | |
| 09:36:22 | gibi | in your feature you add a new ovo which is not soft deletable, that is good | |
| 09:36:48 | gibi | then in AA if you want, you can work on bumping the Aggregate OVO to 2.0 and add a microversion to hide deleted_at | |
| 09:36:49 | Uggla | so I change it as a test to inherit from NovaPersistantObject (without soft delete) | |
| 09:38:21 | Uggla | gibi but is it something that could be done in AA or later to have a "deprecation" time ? | |
| 09:39:17 | gibi | there is multiple things. The API change needs a microversion bump, it does not need a deprecation period but we need to support old microversions( you can fake deleted_at in the API response for old microversions) | |
| 09:39:36 | gibi | the Aggregate OVO change needs a major ovo version bump to 2.0 to remove a field | |
| 09:39:52 | sean-k-mooney | same for all other objects | |
| 09:40:00 | gibi | that means if we pass Aggregate OVOs over RPC then we need to support Aggregate 1.x and 2.0 parallel for an extra release | |
| 09:40:23 | sean-k-mooney | we cant do the db contraction till CC | |
| 09:40:32 | gibi | yes, that is the 3rd thing | |
| 09:40:34 | sean-k-mooney | because of teh new life cycle | |
| 09:41:12 | gibi | but the Aggregate table has no deleted_at so no need to contract there | |
| 09:41:17 | sean-k-mooney | well technially we can do it in BB if we deperecate teh soft delete feature in AA | |
| 09:41:23 | Uggla | yes so do you think it worth it ? | |
| 09:41:43 | sean-k-mooney | yes long term but i can upgrade my -1 to a -2 if you like | |
| 09:41:57 | sean-k-mooney | you should not be doing this as part of the share change | |
| 09:42:06 | sean-k-mooney | its its own spereate thing | |
| 09:42:22 | Uggla | sean-k-mooney, no this is outside of the share stuff. | |
| 09:42:34 | sean-k-mooney | right it needs a spec | |
| 09:42:36 | gibi | Uggla: it depends. I think the important part is not to add new ovos with soft delete. Cleaning up the old ones is a bit of busy work in my eyes | |
| 09:43:05 | sean-k-mooney | it has a prerrty large upgrad impact so it cant reasonable be done as a bugfix or specless blueprint | |
| 09:43:28 | sean-k-mooney | gibi: this kind of need to be done in tandom with removign shadow tables for it to be useful | |
| 09:43:30 | gibi | yes I agree to draft a spec for it if you want to work on this | |
| 09:43:40 | gibi | sean-k-mooney: good point | |
| 09:44:23 | Uggla | gibi, agree I don't want to work on something if you think the benefits are low. | |
| 09:44:24 | sean-k-mooney | personally removing the shaddow tabels woudl be more useful in my view then the soft delete mechanisum | |
| 09:44:52 | sean-k-mooney | Uggla: its not that its low its that there is a lot more work to do then just that patch | |
| 09:45:22 | gibi | Uggla: you can bring this up on the PTG to get a wider set of opinions about it | |
| 09:45:37 | sean-k-mooney | Uggla: its quite a large change to do right and ensure we dotn break rooling upgrades | |
| 09:47:12 | Uggla | sean-k-mooney, agree I know that changing that is a lot of work due to the API impact etc... But from your point of view is it something that "bring value" ? | |
| 09:47:14 | sean-k-mooney | by the way if an object suppots soft delele is determin by if it has the softDeleteMixin in the db model | |
| 09:47:16 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/main/models.py#L139 | |
| 09:47:19 | sean-k-mooney | not the ovo | |
| 09:47:39 | sean-k-mooney | Uggla: its something i have wanted to do for a few releases | |
| 09:48:20 | sean-k-mooney | basically i want to remove or custom db backup/audit facilaties sicne operator have written there own | |
| 09:48:45 | sean-k-mooney | nova i think is the only project with shadow tabels so they wroge a solution that worked for all of them | |
| 09:50:43 | sean-k-mooney | https://github.com/ovh/osarchiver/ | |