| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-18 | |||
| 17:11:23 | melwitt | KeithMnemonic1: ^ (late) | |
| 17:15:39 | KeithMnemonic1 | lol | |
| 17:18:53 | sean-k-mooney | im not sure why i try to make my tests robost... | |
| 17:20:24 | sean-k-mooney | gibi: mriedem http://paste.openstack.org/show/777439/ | |
| 17:20:48 | sean-k-mooney | so you know why stat is not ment to be HARD_REBOOT ... | |
| 17:22:50 | sean-k-mooney | im going to add backin self._wait_for_state_change(self.api, shelved_server, 'ACTIVE') | |
| 17:23:46 | sean-k-mooney | or active_server in this case | |
| 17:24:20 | mriedem | idk what you are doing there | |
| 17:24:43 | sean-k-mooney | im using the fake notifiyer to wait for the reboot end notifcaiont | |
| 17:25:00 | mriedem | but why do you need a whole separate _get_server_info | |
| 17:25:07 | sean-k-mooney | the same one that is used in the allocation fucntional tests | |
| 17:25:26 | mriedem | self._wait_for_state_change(self.api, active_server, 'ACTIVE') does the same thing you added | |
| 17:25:27 | sean-k-mooney | because i was nolonger checking for the server being active | |
| 17:25:49 | sean-k-mooney | right but the whole point was the status was ment to always be active | |
| 17:26:01 | sean-k-mooney | or the previous patch would have been correct | |
| 17:26:11 | sean-k-mooney | its actully HARD_REBOOT | |
| 17:26:25 | mriedem | after the task_state is None? | |
| 17:26:34 | mriedem | if you wait for ACTIVE and task_state=None you shouldn't have a problem | |
| 17:26:43 | sean-k-mooney | no im not checking for that im waiting for the notificaiton | |
| 17:26:54 | sean-k-mooney | ya i can do that instead | |
| 17:27:05 | mriedem | i look forward to at least 5 more patch sets for this fix | |
| 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 | |