| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-07 | |||
| 15:21:51 | bauzas | mriedem: and there are fields that aren't persisted already | |
| 15:22:08 | bauzas | so, maybe the work consists of making the line clearer in between what requires to be persisted and what's not | |
| 15:22:24 | bauzas | I could do that | |
| 15:22:31 | stephenfin | sdague: Nice little doc cleanup here, if you're interested https://review.openstack.org/#/c/501342/ | |
| 15:23:33 | mriedem | def select_destinations(self, ctxt, spec_obj, instance_uuids): | |
| 15:23:36 | mriedem | bauzas: ^ is a long list? | |
| 15:24:42 | bauzas | mriedem: you know we had a problem with instance_uuids being different from what we have in the spec object, so I'd say one parameter is maybe too much | |
| 15:25:16 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove dest node allocation if evacuate MoveClaim fails https://review.openstack.org/499878 | |
| 15:25:30 | bauzas | mriedem: the real crux of the problem is that I never explicited whether the RequestSpec object you pass is either the original spec or the amended spec in case of a move | |
| 15:26:01 | mriedem | gdi now my git review push failed | |
| 15:26:20 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove dest node allocation if evacuate MoveClaim fails https://review.openstack.org/499878 | |
| 15:26:20 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass migration from API to conductor for evacuate https://review.openstack.org/500176 | |
| 15:26:21 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add a test to make sure failed evacuate cleans up dest allocation https://review.openstack.org/499877 | |
| 15:26:22 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add recreate test for evacuate claim failure https://review.openstack.org/499874 | |
| 15:26:22 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Create allocations against forced dest host during evacuate https://review.openstack.org/499399 | |
| 15:26:23 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Refactor out claim_resources_on_destination into a utility https://review.openstack.org/499718 | |
| 15:26:23 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Modernize set_vm_state_and_notify https://review.openstack.org/499799 | |
| 15:26:39 | bauzas | do folks agree with the fact that we should disable reschedules if someone is requesting a destination when cold migrating ? | |
| 15:26:43 | mriedem | the bottom change there is going to need to be re-approved | |
| 15:26:54 | bauzas | mriedem: ack will do | |
| 15:27:31 | mriedem | don't we already restrict reschedules like that in some other case? | |
| 15:28:17 | bauzas | mriedem: we don't reschedule for both evacuate and live-migrate | |
| 15:28:28 | bauzas | we only reschedule for two cases : boot and resize | |
| 15:28:38 | mriedem | well, we do reschedule for live migrate | |
| 15:28:40 | mriedem | from conductor | |
| 15:28:42 | mriedem | if the pre-checks fails | |
| 15:28:43 | bauzas | well, and cold migrate since resize shares the same codepath | |
| 15:28:43 | mriedem | *fail | |
| 15:29:00 | bauzas | mriedem: you mean at the conductor level ? | |
| 15:29:04 | mriedem | yes | |
| 15:29:09 | bauzas | mriedem: ah yeah | |
| 15:29:12 | bauzas | that's recent | |
| 15:29:41 | bauzas | I was talking of the classical reschedule situation where a compute raises an exception and then calls out conductor by amending the retry dict | |
| 15:29:44 | mriedem | evacuate in the conductor doesn't reschedule | |
| 15:30:17 | mriedem | if you specify a host for cold migrate and that host fails, we shouldn't reschedule IMO | |
| 15:30:39 | bauzas | the situation I mention in takashi's spec is that if you allow 3 reschedules by default, you could have the scenario of a requested destination call, and then if the compute fails, then ending up finding another compute node | |
| 15:30:48 | bauzas | mriedem: that's my point | |
| 15:30:58 | mriedem | yes i realize, it's ComputeManager._reschedule_resize_or_reraise | |
| 15:31:02 | bauzas | I just wanted to make sure we all agree | |
| 15:32:21 | mriedem | now, live migration allows you to specify a host, not force it, and it will retry migrate_max_retries times | |
| 15:32:25 | mriedem | which defaults to -1 for unlimited | |
| 15:34:03 | mriedem | although i think in the case of live migration, if you specify a host (without force) and the scheduler raises NoValidHost, we're done | |
| 15:34:06 | mriedem | there is no retry | |
| 15:34:33 | mriedem | otherwise if you don't specify a host, we just keep trying until we find a host or run out of hosts and the scheduler raises NoValidHost | |
| 15:34:33 | bauzas | that's fine then | |
| 15:34:39 | bauzas | for live migrate | |
| 15:35:10 | mriedem | yeah so for cold migrate if i specify a host and the scheduler kicks it out with NoValidHost, you're done | |
| 15:35:22 | mriedem | if the scheduler says it's ok and the compute fails, we don't reschedule to another host via prep_resize | |
| 15:35:24 | mriedem | you're done | |
| 15:35:28 | bauzas | okay, so I +1d the spec | |
| 15:35:41 | bauzas | once takashi amends the spec for mentioning that, I +2 | |
| 15:35:47 | mriedem | did he remove the stuff about adding the force flag to the cold migrate api? | |
| 15:35:52 | bauzas | he deserves more attention than last cycle :) | |
| 15:35:55 | bauzas | he did | |
| 15:36:19 | mriedem | well, what i'd also like to see with this is to not do the same underhanded request spec stuff that live migrate and evacuate do when a host is specified, | |
| 15:36:31 | mriedem | i.e. the API does some logic and shoves stuff in the request spec, which passes through conductor to the scheduler | |
| 15:36:47 | mriedem | i'd prefer if that was all more explicit between api->conductor-scheduler | |
| 15:37:07 | mriedem | because whenever i look at the conductor code i have to know exactly how the api code works | |
| 15:38:09 | bauzas | mriedem: when I wrote the original blueprint, there was a reason | |
| 15:38:45 | bauzas | mriedem: I necessarly needed to set the Spec fields by the API because the force or not force logic is there | |
| 15:39:22 | bauzas | mriedem: plus the fact that you can call again the conductor by the compute if you resize | |
| 15:39:48 | bauzas | mriedem: those reasons led me to set the fields there in the API | |
| 15:40:11 | bauzas | but I agree with the fact that is spaghetti code | |
| 15:42:06 | mriedem | i've tried to document the hell out of all of these areas that i've had to touch for live migration and evacuate to help clarify some of it | |
| 15:42:19 | mriedem | but we still need some detailed documentation in the request spec object itself per my ML email | |
| 15:42:40 | mriedem | plus the fact that we don't convert some things to primitive in the request spec | |
| 15:42:42 | mriedem | which seems like a bug | |
| 15:43:09 | mriedem | but, i don't really know enough about it or if it's something we should care about | |
| 15:43:25 | mriedem | i.e. if you only have to convert to a primitive for legacy stuff due to an old scheduler client, then we probably don't care | |
| 15:45:52 | bauzas | when you say primitives, you mean legacy dictionaries ? | |
| 15:46:17 | bauzas | not the primitived dictionary in terms of o.vo ? | |
| 15:46:55 | bauzas | honestly, the problem is that we still have lots of back-and-forths between those legacy dicts and the new object, which is errorprone | |
| 15:47:24 | bauzas | so, my goal is to reduce those by changing the scheduler helper methods to accept a spec object | |
| 15:51:20 | mriedem | the legacy dict | |
| 15:53:36 | mriedem | gibi: in https://review.openstack.org/#/c/417882/23/doc/notification_samples/instance-resize-error.json it looks weird in a few places where there are empty dicts or lists and there is a blank line in between | |
| 15:54:22 | bauzas | by reading the news, all my thoughts go to folks in Florida, Georgia and South Carolina. Hope jaypipes will be all good | |
| 15:54:49 | jaypipes | bauzas: it's a logistical nightmare :( | |
| 15:55:18 | bauzas | jaypipes: you're far from me, buddy so I can hardly help | |
| 15:55:29 | jaypipes | bauzas: heh, I know :) it's cool, dude. | |
| 15:55:35 | bauzas | jaypipes: but in case you need anything I can do, just lemme know | |
| 15:55:41 | jaypipes | bauzas: thx :) | |
| 15:56:12 | bauzas | jaypipes: you're temporarly relocating, as you said ? | |
| 15:57:38 | jaypipes | bauzas: yeah, though plans have changed... now looking to got to Asheville, NC for a night, drop the girls off with my parents and then catch flights out of ATL. | |
| 15:57:47 | jaypipes | bauzas: it's a clusterf**k | |
| 15:59:20 | bauzas | :( | |
| 16:17:10 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Remove 'vendordata_driver' opt https://review.openstack.org/397835 | |
| 16:17:14 | openstackgerrit | John Garbutt proposed openstack/nova master: Enable test_iscsi_volume in live migration job https://review.openstack.org/459316 | |
| 16:17:36 | stephenfin | mriedem: Per mikal's comment earlier, could you remove the -2 on this, please? https://review.openstack.org/397835 | |
| 16:18:30 | mriedem | stephenfin: what was mikal's comment earlier? | |
| 16:19:05 | stephenfin | ||
| 16:19:17 | stephenfin | <mikal> stephenfin: yeah, I think we could take another pass at that now | |
| 16:19:53 | stephenfin | mriedem: I assume that means the vendordata v2 stuff is done? idk for sure, personally | |
| 16:20:20 | mriedem | mikal made improvements in ocata | |
| 16:20:33 | mriedem | https://specs.openstack.org/openstack/nova-specs/specs/ocata/implemented/vendordata-reboot-ocata.html | |
| 16:21:52 | stephenfin | Ultimately though, does that mean configurability of vendordata driver is no longer required? | |
| 16:24:01 | openstackgerrit | Stephen Finucane proposed openstack/nova-specs master: DNM! Make "reproposals" more obvious to readers https://review.openstack.org/501797 | |
| 16:24:01 | openstackgerrit | Stephen Finucane proposed openstack/nova-specs master: DNM: Unformatted file https://review.openstack.org/501798 | |
| 16:24:24 | efried | mriedem Bitter. So bitter. | |
| 16:27:01 | mriedem | gdi i can't find the vendordata v2 stuff in the docs nova | |
| 16:27:02 | mriedem | *now | |