| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-18 | |||
| 17:38:51 | sean-k-mooney | yes and there is a third field status | |
| 17:39:25 | sean-k-mooney | wait for state chage checks status | |
| 17:39:51 | sean-k-mooney | so is status task_state when it not equal to None and vm_sate otherwise? | |
| 17:40:03 | sean-k-mooney | because that seam to be the behavior of the fixture | |
| 17:40:38 | mriedem | no | |
| 17:40:41 | mriedem | see the link i just posted above | |
| 17:41:41 | mriedem | https://github.com/openstack/nova/blob/c67057dff34a0054977ae3873d33313c0617b308/nova/api/openstack/common.py#L117 | |
| 17:41:59 | sean-k-mooney | ya im reading that now | |
| 17:43:27 | sean-k-mooney | it uses a mapp to do it but its similar to what i said. but there is not a race in the current version then | |
| 17:46:30 | sean-k-mooney | anyway to be extra safe ill push with the wait for state change + notificaitons since that works | |
| 17:48:23 | artom | mriedem, I've had an intuition for https://review.opendev.org/#/c/641453/2 | |
| 17:48:25 | artom | Comments inline | |
| 17:49:57 | openstackgerrit | sean mooney proposed openstack/nova master: make config drives sticky bug 1835822 https://review.opendev.org/669738 | |
| 17:49:58 | openstack | bug 1835822 in OpenStack Compute (nova) "vms loose acess to config drive with CONF.force_config_drive=True after hard reboot" [Medium,In progress] https://launchpad.net/bugs/1835822 - Assigned to sean mooney (sean-k-mooney) | |
| 18:00:47 | mriedem | artom: replied but i still don't really get it | |
| 18:01:00 | mriedem | we know that migrate_data.bdms isn't getting set during pre_live_migratoin | |
| 18:01:03 | mriedem | which should happen on the dest | |
| 18:01:10 | artom | mriedem, to be honest neither do I | |
| 18:01:30 | mriedem | it's probably something really dumb and i'm just overlooking it | |
| 18:02:07 | artom | I mean, that's 4 of us, at this point? | |
| 18:02:11 | artom | We can't all be dumb | |
| 18:02:15 | mriedem | oh we can | |
| 18:02:23 | mriedem | and will be gdi | |
| 18:02:26 | artom | I checked the dumb stuff like args out of order | |
| 18:02:37 | artom | I admire your stubborness | |
| 18:02:55 | mriedem | wonder if the partial with source_bdms is somehow at fault | |
| 18:03:55 | mriedem | OH | |
| 18:03:58 | mriedem | i SEE it | |
| 18:04:04 | artom | mriedem, don't think so - the failure is "in" the migration | |
| 18:04:08 | mriedem | I SEE IT | |
| 18:04:20 | artom | When it's updating the instance XML, migrate_data.bdms is not there | |
| 18:04:23 | artom | TELL US | |
| 18:05:14 | mriedem | IT'S FULL OF STARS | |
| 18:05:35 | mriedem | commented inline to explain the problem, | |
| 18:05:46 | mriedem | but tl;dr we didn't return the migrate_data that we got back from the dest to pass to the driver on the source | |
| 18:05:49 | mriedem | so we passed our stale copy | |
| 18:06:05 | artom | "Thanks Artom, you helped me figure it out." | |
| 18:06:12 | artom | I'm going to use that to fall asleep now | |
| 18:06:22 | artom | Better than any lullaby | |
| 18:07:18 | artom | Ah, I get it | |
| 18:07:22 | artom | *facepalm* | |
| 18:07:34 | artom | Side effects suck | |
| 18:08:10 | mriedem | pycharm might have even warned me locally that i was overwriting a method arg | |
| 18:08:18 | mriedem | i know it does if you shadow imports | |
| 18:08:32 | mriedem | which is helpful in cases where we pass around a thing called 'context' and have a module import of nova.context | |
| 18:09:09 | artom | IIRC importing nova.context you still to access it with nova.context | |
| 18:09:20 | artom | As opposed to from nova import context | |
| 18:10:01 | mriedem | i mean the latter | |
| 18:10:07 | mriedem | from nova import context | |
| 18:10:08 | mriedem | ... | |
| 18:10:23 | mriedem | instances = objects.Instance.get_by_instance_uuid(context, instance_uuid) | |
| 18:10:40 | artom | Yep, that would suck | |
| 18:11:10 | mriedem | we do it all over the place, but i think it's only bit us in the ass once that i know of | |
| 18:11:34 | sean-k-mooney | passing the module instead of a context object | |
| 18:12:09 | sean-k-mooney | that would have intersting sideffect if it did not explode | |
| 18:17:04 | sean-k-mooney | if your really unlucky and only do the assignment to the module in unit test you can end up with the tests passing or fialing dependign on the order that they are run | |
| 18:18:20 | sean-k-mooney | i remember helping to debug an intermitent failture in the neutron gate which was casuse by acidetally assign to the module in a test | |
| 18:20:32 | mloza | hello, is it safe to cleanup failed migrations(those status that are in confirmed or error) in the migration table of the nova database? | |
| 18:22:13 | sean-k-mooney | well those that are in confimed are not failed | |
| 18:23:04 | sean-k-mooney | if you mean can you clean up old migration recored in general i belive so although im not sure if they are needed for teh audit logs. | |
| 18:23:59 | sean-k-mooney | the main sideffect of cleaning up old migration that are confirm or error should be just they are nolonger available for quitying via the api. but maybe mriedem can double check that is correct? | |
| 18:24:27 | sean-k-mooney | i dont know if we have a nova manage command for that but i can see that they would build up over time | |
| 18:24:46 | sean-k-mooney | at least until the vms are delested in any case | |
| 18:27:53 | mriedem | there is no audit log for migrations, in case you're thinking of instance actions | |
| 18:27:56 | mriedem | which are different | |
| 18:28:21 | mriedem | and yeah the downside to removing complete migratoin records is losing any history on those and yeah they won't be archived/pruned until the instance is deleted, | |
| 18:28:40 | sean-k-mooney | yes i was trying to think of what side effect it could have | |
| 18:28:44 | mriedem | we don't have any nova-manage command to archive old migration records - i don't think you can even do that b/c of a foreign key reference to the instance | |
| 18:29:33 | mriedem | mloza: so to answer your questoin i don't think you can do that until the instances for those migrations are also deleted, | |
| 18:29:49 | mriedem | and if you have deleted those instances, than simply running the archive/prune commands will remove them | |
| 18:30:38 | sean-k-mooney | it depend i gues on how we specified the forin key constratint | |
| 18:31:10 | sean-k-mooney | e.g. is it set to cascase on vm deletion or is it a restict key that prevents deleteion untill the instance is deleted | |
| 18:31:24 | mriedem | we don't have any fkeys that do cascading deletes | |
| 18:31:49 | sean-k-mooney | this would be specifid in the modles.py right? | |
| 18:32:12 | sean-k-mooney | we had a customer delete migration recently so i think you can do it | |
| 18:32:45 | mriedem | it's in the models yes | |
| 18:32:50 | mriedem | sean-k-mooney: are you fixing that config drive functional test or what? | |
| 18:33:08 | mloza | sorry, I meant only status error. Just found the confirmed status are for migration and resize action | |
| 18:33:09 | sean-k-mooney | i pushed a version | |
| 18:33:48 | mloza | mriedem: ok, i'll just the migration records in the db for now | |
| 18:33:49 | sean-k-mooney | https://review.opendev.org/#/c/669738/9/nova/tests/functional/regressions/test_bug_1835822.py@66 | |
| 18:33:54 | mloza | leave* | |
| 18:34:06 | sean-k-mooney | i added the wait for notification and kept the state change wait | |
| 18:34:15 | sean-k-mooney | so it cant race on still being active | |
| 18:34:24 | sean-k-mooney | when we start to wait | |
| 18:36:15 | sean-k-mooney | mloza: ok the foreign_key constratin just ensure the instance_uuid exists in the isntace table and the migration instance uuid matche and the instance is not deleted | |
| 18:36:17 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/models.py#L805-L809 | |
| 18:36:44 | sean-k-mooney | so it wont prevent you deleteing the old migration recored while the vm still exists | |
| 18:39:09 | mloza | sean-k-mooney: any sides effect the old migration record that are status error while the vm still exist? | |
| 18:40:30 | sean-k-mooney | the only one im aware of is that you will nolonger be able to retivie the migration info since you deleted it. | |
| 18:41:11 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Refactor pre-live-migration work out of _do_live_migration https://review.opendev.org/641453 | |
| 18:41:11 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Mark "block_migration" arg deprecation on pre_live_migration method https://review.opendev.org/682963 | |
| 18:41:43 | mloza | understand. I don't need it for auditing | |
| 18:41:45 | mloza | Thanks | |
| 18:41:58 | mriedem | don't take that as a recommendation that it's ok | |
| 18:42:08 | mriedem | messing with the db directly is not really a supported thing, so beware | |
| 18:42:39 | mriedem | especially with migration-based allocation consumers since queens i could see us (nova) relying on migration records existing to determine a type of consumer of resource allocatoins in placement | |
| 18:42:47 | mriedem | like in the audit command that bauzas is working on | |
| 18:42:53 | mriedem | mloza: ^ | |
| 18:43:09 | sean-k-mooney | right from a downstream perspecit i think we would normally ask you to file a support exception first and assess if its safe | |