| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-18 | |||
| 17:27:51 | sean-k-mooney | i like how this is litrlly all because im swaping the order of two lines | |
| 17:29:07 | mriedem | pick one or the other i don't think it matters, the task_state is None here https://github.com/openstack/nova/blob/c67057dff34a0054977ae3873d33313c0617b308/nova/compute/manager.py#L3529 and the notification is here https://github.com/openstack/nova/blob/c67057dff34a0054977ae3873d33313c0617b308/nova/compute/manager.py#L3536 | |
| 17:29:12 | mriedem | for the purpose of your test either is sufficient | |
| 17:29:45 | mriedem | if you do wait for the task_state to be None, use https://github.com/openstack/nova/blob/c67057dff34a0054977ae3873d33313c0617b308/nova/tests/functional/integrated_helpers.py#L247 | |
| 17:29:49 | mriedem | don't write some new function | |
| 17:30:16 | mriedem | using _wait_for_server_parameter will get you the latest copy of the server with the config drive value in the api so i'd use that myself | |
| 17:30:33 | sean-k-mooney | yep i can do that | |
| 17:30:56 | openstackgerrit | Merged openstack/nova stable/stein: Fix the server group "policy" field type in api-ref https://review.opendev.org/662224 | |
| 17:31:00 | sean-k-mooney | but the reason i pinged you on irc was it look like our fixture are not doing the right thing | |
| 17:31:04 | openstackgerrit | Merged openstack/nova stable/stein: Fixing broken links https://review.opendev.org/681401 | |
| 17:31:11 | openstackgerrit | Merged openstack/nova stable/stein: libvirt: stub logging of host capabilities https://review.opendev.org/682210 | |
| 17:31:20 | openstackgerrit | Merged openstack/nova stable/stein: Fix rebuild of baremetal instance when vm_state is ERROR https://review.opendev.org/680869 | |
| 17:33:38 | sean-k-mooney | well either we really do go from ACTIVE->HARD_REBOOTING->ACTIVE which is why i orginaly thought in which case its not a race or the fixture is reutrning the worng value. ill look at that after i do the taskstae check | |
| 17:34:14 | mriedem | you're seeing HARD_REBOOTING b/c of the task_state not being None | |
| 17:34:17 | sean-k-mooney | for rebuild im getting the same behavior "esttools.matchers._impl.MismatchError: 'ACTIVE' != 'REBUILD'" | |
| 17:34:38 | mriedem | does your test properly stub out the fake notifier in setUp? | |
| 17:34:42 | sean-k-mooney | yes | |
| 17:35:06 | mriedem | this is why you saw HARD_REBOOT https://github.com/openstack/nova/blob/c67057dff34a0054977ae3873d33313c0617b308/nova/api/openstack/common.py#L53 | |
| 17:35:08 | mriedem | b/c the task_state was set | |
| 17:35:09 | sean-k-mooney | i copied the sutb and the example usage form the integrated helpers | |
| 17:37:27 | sean-k-mooney | mriedem: but that shoudl not be reported in the status field it should be in OS-EXT-STS:task_state | |
| 17:37:48 | sean-k-mooney | or is that not how that works | |
| 17:37:59 | sean-k-mooney | there is also OS-EXT-STS:vm_state | |
| 17:38:22 | mriedem | that's not how that works | |
| 17:38:27 | mriedem | status is a mix of vm_state and task_state | |
| 17:38:37 | mriedem | OS-EXT-STS:vm_state and OS-EXT-STS:task_state are separate | |
| 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? | |