| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-05 | |||
| 18:38:08 | dansmith | but if you start an evacuation, it fails, and then the original source comes back alive, you can keep it there, barring bugs and timing and all kinds of crazy | |
| 18:38:08 | artom | Even if the symptoms are the same, it has to be a different root cause, because upstream doens't use hybrid plug | |
| 18:38:13 | sean-k-mooney | artom: that was the bug we intoduce the wait on revert to fix | |
| 18:38:14 | dansmith | which I wish wasn't the case | |
| 18:38:48 | artom | dansmith, that couldn't have been on purpose... | |
| 18:39:20 | sean-k-mooney | dansmith: im assuming that only works if we have not updated the instance host yet | |
| 18:39:21 | dansmith | artom: nothing about evacuate was done with a great sense of purpose, but it was decided not to make it explicitly disallowed (to stay put) | |
| 18:39:26 | sean-k-mooney | e.g. we failed before that point | |
| 18:39:36 | dansmith | personally I'd prefer that the instant you start an evac, the instance _has_ to move somewhere | |
| 18:39:40 | sean-k-mooney | and it jsut happend to work if the host became alive again? | |
| 18:39:44 | dansmith | sean-k-mooney: yeah | |
| 18:40:42 | sean-k-mooney | ok so its just happening because we dont keep retrying to move the instance after the failure | |
| 18:40:55 | sean-k-mooney | and when it fails we dont update the db | |
| 18:41:39 | dansmith | it depends on when we fail | |
| 18:41:41 | dansmith | .of course | |
| 18:41:48 | dansmith | which is why it seems broken to me, | |
| 18:42:13 | sean-k-mooney | so i proably know the answer to this but do we really care about that edgecase? | |
| 18:42:14 | dansmith | especially because HA stuff that has made a decision to rebuild the instance is easier knowing it has fenced the original node, etc | |
| 18:42:36 | sean-k-mooney | i mean we proably should but other stuff is proably broken at that point | |
| 18:42:43 | dansmith | sean-k-mooney: IIRC, mriedem (and maybe others) really want to keep the "can keep it on the source if it's not too late" behavior | |
| 18:43:06 | sean-k-mooney | dansmith: no im kind of ok with that | |
| 18:43:12 | sean-k-mooney | what i ment was for artom patch | |
| 18:43:27 | dansmith | oh, yeah, I think that's worth investigating and fixing if it's related | |
| 18:43:35 | sean-k-mooney | do we need to add in extra code to handel that case | |
| 18:44:35 | artom | sean-k-mooney, the evacuate case? Maybe in a different patch, but why put it in mine? | |
| 18:44:35 | sean-k-mooney | im trying to think how we would even fake that out in a functional test | |
| 18:45:00 | artom | dansmith, so, turns out I was being too paranoid too soon about mocking things, and it actully works out pretty clean | |
| 18:45:01 | sean-k-mooney | artom: just because dansmith asked about the other move operation in yours | |
| 18:45:21 | sean-k-mooney | but it could be in another patch | |
| 18:45:31 | dansmith | sean-k-mooney: and when mriedem reviews it, that's going to be one of the first things he says :) | |
| 18:46:16 | sean-k-mooney | well i dont think artoms patch makes that case any worse but it also might not fix things in that case | |
| 18:46:32 | sean-k-mooney | assuming its currently broken in that case | |
| 18:47:15 | dansmith | it's a matter of making sure we don't re-have this conversation in a year when someone reports it for evacuate, and maybe fix it a different way | |
| 18:47:36 | sean-k-mooney | ya thats fair | |
| 18:47:41 | dansmith | also, I think hard reboot used to wait, now doesn't, potentially because we had this convo already and didn't go deep enough | |
| 18:47:54 | dansmith | https://review.opendev.org/#/c/553035/ | |
| 18:48:01 | artom | But... I thought the whole thing here is that hybrid plug is special in how it wires vifs | |
| 18:48:03 | dansmith | heh, about a year ago | |
| 18:48:06 | sean-k-mooney | well not quite | |
| 18:48:24 | artom | And resize is special in that it leaves in the instance paused on the source until confirm or revert | |
| 18:48:39 | sean-k-mooney | some network backeds only send events on port bind and we dont bind the ports on hard reboot or rebuild | |
| 18:48:39 | dansmith | no, it's not paused on the source | |
| 18:48:42 | dansmith | it's off | |
| 18:48:43 | artom | So.. how would that be applicable for evacuate, when the instance was never on the dest to begin with? | |
| 18:48:51 | artom | dansmith, sorry, off (aka destoryed) | |
| 18:49:28 | artom | sean-k-mooney, ah, so some backends might send the event as soon as we change the port binding to a host, regardless if it's wired or not? | |
| 18:49:44 | sean-k-mooney | artom: yes | |
| 18:49:51 | artom | I'd say that's a bug in those backends | |
| 18:50:00 | sean-k-mooney | it was by design | |
| 18:50:06 | artom | It shouldn't be sending the event if the thing isn't actually plugged/wired | |
| 18:50:39 | sean-k-mooney | not all networking backend can support that or at least could back when this was firt intoduced | |
| 18:50:58 | artom | That doesn't invalidate my argument :) | |
| 18:51:08 | sean-k-mooney | odl took 2 years to catch up and intoduce a way for odl to send events to neutron which neutron could send to nova | |
| 18:51:14 | artom | The idea was for Nova to be sure we have networking for instance by the time it spawns | |
| 18:51:17 | artom | So things like DHCP work | |
| 18:51:27 | artom | If we get the event before that's happened, that's not for Nova to fix | |
| 18:51:49 | sean-k-mooney | yep i had this argument with the odl folk back in 2014 | |
| 18:51:53 | artom | Because we'd be acting like networking is ready, when in reality it isn't | |
| 18:52:01 | artom | (yey, 5 years late) | |
| 18:52:39 | sean-k-mooney | for what its worth im pretty sure netorking-aci still does not send it when the port is wireded up an send it on bind | |
| 18:52:57 | sean-k-mooney | and ovn is not doing the right thing either in all cases | |
| 18:52:58 | artom | The Cisco thing? | |
| 18:53:02 | sean-k-mooney | yes | |
| 18:53:14 | artom | That's just one more argument for people not to buy Cisco ;) | |
| 18:53:36 | mriedem | "IIRC, mriedem (and maybe others) really want to keep the "can keep it on the source if it's not too late" behavior" | |
| 18:53:40 | mriedem | i have no opinion on that | |
| 18:53:42 | mriedem | that i'm aware of | |
| 18:54:14 | mriedem | i'm also assuming i don't want to cruft up an already complicated change to cover other cases in the same patch if we're doing backports | |
| 18:54:23 | mriedem | b/c there is a >0% chance this will introduce a regression somehow | |
| 18:54:27 | mriedem | b/c this is all very brittle sounding | |
| 18:54:30 | mriedem | and super duper latent | |
| 18:55:12 | mriedem | how would one even recreate this hybrid ovs resize thing in the gate? configure devstack for that backend (is that easy?) and then put a sleep in the nova code between migrate_instance_start(finish?) and the call to the virt driver? | |
| 18:55:16 | artom | Like osteoporosis | |
| 18:56:14 | sean-k-mooney | mriedem: we have patches to recreate. you configure ovs of iptables and then we can race on teh revert tests | |
| 18:56:39 | sean-k-mooney | *ovs to use the iptables firewall driver | |
| 18:56:53 | artom | https://review.opendev.org/#/c/660782/5 specifically | |
| 18:57:59 | sean-k-mooney | our downstream ci default to iptables firewall still on older releases so that is why we are seeing it downstream. upstream use the openvswtich contrack firewall driver by default | |
| 18:58:53 | mriedem | why does https://review.opendev.org/#/c/653498/ come after the fix in the series? | |
| 18:59:27 | artom | mriedem, because without hybrid plug it's actually irrelevant to the fix :/ | |
| 18:59:32 | artom | Well | |
| 18:59:39 | artom | I suppose it tests the non-hybrid-plug case | |
| 19:00:12 | mriedem | correct | |
| 19:00:30 | mriedem | you said in the ML it was the only job we run that is multi-node for the resize revert stuff right? | |
| 19:00:39 | mriedem | or would be | |
| 19:00:57 | artom | Yeah | |
| 19:01:00 | artom | I can move it below | |
| 19:02:16 | mriedem | you know, | |
| 19:02:23 | sean-k-mooney | artom: actuly i think you used to have it below and inverted it in a recent revision | |
| 19:02:28 | mriedem | test_server_connectivity_cold_migration_revert in the tempest-slow-py3 job would run resize revert in a multi-node job, | |
| 19:02:31 | mriedem | if you removed the skip on the test | |
| 19:02:38 | mriedem | and the test is skipped b/c of https://bugs.launchpad.net/nova/+bug/1788403 | |
| 19:02:40 | openstack | Launchpad bug 1788403 in OpenStack Compute (nova) "test_server_connectivity_cold_migration_revert randomly fails ssh check" [Medium,Fix released] - Assigned to Matt Riedemann (mriedem) | |
| 19:02:46 | mriedem | which sounds very familiar to your issue yeah? | |
| 19:03:13 | artom | It would, except again, upstream gate doesn't use hybrid plug, so it's a different root cause | |
| 19:03:19 | dansmith | mriedem: you emotionally scarred me with discussion of that opinion in the past, which is why I remember it | |
| 19:04:06 | mriedem | dansmith: logs or it never happened! | |
| 19:04:06 | dansmith | mriedem: and yeah, not saying fixing the other operations needs to be in this patch, but if we're undoing not only this wait, but also making previous ones (like the hard reboot) no longer accurate, I think fixing them all in the same way (re-using helpers if they apply, etc) is a good idea | |
| 19:04:21 | dansmith | mriedem: I can show you the scars. those' | |
| 19:04:24 | dansmith | re good enough yeah? | |
| 19:04:34 | mriedem | pictures or it didn't happen! | |