| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-12 | |||
| 19:10:34 | finucannot | mriedem: Excellent, thanks :) I'll address all the comments in a follow-up by Monday | |
| 19:10:54 | mriedem | ack | |
| 19:11:01 | mriedem | tell danpb the good news :) | |
| 19:11:07 | mriedem | he is risen | |
| 19:11:14 | finucannot | :D | |
| 19:42:29 | openstackgerrit | Sriharsha Basavapatna proposed openstack/os-vif master: Fix VF-rep lookup routine to use parent PF number https://review.openstack.org/531674 | |
| 20:00:15 | superdan | mriedem: oh you're back | |
| 20:00:31 | superdan | mriedem: wanna conduct a thought experiment with me? | |
| 20:02:52 | mriedem | umm | |
| 20:03:38 | superdan | you can have this lollipop if you get into my van... | |
| 20:05:43 | mriedem | stvnoyes: nice work on https://review.openstack.org/#/c/533285/ | |
| 20:05:54 | mriedem | superdan: shoot | |
| 20:06:03 | mriedem | if it's overly complicated for my vacation brain i'll opt out | |
| 20:06:07 | superdan | mriedem: you wanna do hangout for expediency or here? | |
| 20:06:12 | mriedem | oh... | |
| 20:06:39 | mriedem | i have migrated to the game of thrones final episode of season 7 viewing area... | |
| 20:06:47 | mriedem | i didn't realize i'd have to put professional pants back on | |
| 20:06:54 | superdan | you don't, I can explain here | |
| 20:07:03 | superdan | it'd just be quicker via voice, but up to you | |
| 20:07:19 | mriedem | sec, i'll move back to the office | |
| 20:08:19 | superdan | mriedem: https://hangouts.google.com/call/BPfFHG1eYDY-HJyiblzMAAEE | |
| 20:08:21 | superdan | melwitt ^ | |
| 20:13:25 | mriedem | reminds me of this https://review.openstack.org/#/c/529626/ | |
| 20:21:31 | superdan | sdague: please +W this backport ASAP kthx | |
| 20:21:33 | superdan | https://review.openstack.org/#/c/529635 | |
| 20:28:43 | melwitt | mriedem: couple stable backports needing review https://review.openstack.org/#/q/topic:bug/1713857 and https://review.openstack.org/#/q/topic:bug/1724573 | |
| 20:29:09 | melwitt | oh wait, you did the backports in the first bug so you can't review them. guh | |
| 20:31:47 | fried_rolls | sean-k-mooney I actually tried that, and for some reason it didn't work. I'm actually not super worried about it; I'd almost be tempted to leave the read-only protections off entirely, since the point is for them not to be able to modify the original ProviderTree. | |
| 20:34:17 | mriedem | mtreinish: want to hit this backport? https://review.openstack.org/#/c/523911/ | |
| 20:37:15 | openstackgerrit | Ed Leafe proposed openstack/nova master: Change compute RPC to use alternates for resize https://review.openstack.org/526436 | |
| 20:37:17 | figleaf | mriedem: ^^ addressed your comments | |
| 20:39:56 | openstackgerrit | Merged openstack/nova master: Revert "Deduplicate service status notification samples" https://review.openstack.org/533190 | |
| 20:40:10 | openstackgerrit | Merged openstack/nova master: Add a warning in 'nova-manage cell_v2 delete_cell' https://review.openstack.org/513771 | |
| 20:40:27 | openstackgerrit | Merged openstack/nova master: Update FAQs about listing hosts in cellv2 https://review.openstack.org/531559 | |
| 20:40:41 | openstackgerrit | Merged openstack/nova master: Document lack of side-effects in AllocationList.create_all() https://review.openstack.org/530997 | |
| 20:41:29 | mtreinish | mriedem: +A | |
| 20:49:05 | melwitt | lyarwood: your review needed please for the libvirt driver re-init on hard reboot backport https://review.openstack.org/#/c/531422 | |
| 21:00:47 | openstackgerrit | Eric Fried proposed openstack/nova master: Test helper: validate provider summaries https://review.openstack.org/518982 | |
| 21:00:47 | openstackgerrit | Eric Fried proposed openstack/nova master: Fix accumulated nits in refactor series https://review.openstack.org/521189 | |
| 21:00:48 | openstackgerrit | Eric Fried proposed openstack/nova master: Add aggregates check in allocation candidates https://review.openstack.org/522407 | |
| 21:00:48 | openstackgerrit | Eric Fried proposed openstack/nova master: Fix missing rps in allocation candidates https://review.openstack.org/522409 | |
| 21:00:49 | openstackgerrit | Eric Fried proposed openstack/nova master: placement: func tests for multiple shared RPs https://review.openstack.org/498737 | |
| 21:00:49 | openstackgerrit | Eric Fried proposed openstack/nova master: Fix comments in get_all_with_shared() https://review.openstack.org/533195 | |
| 21:01:28 | fried_rolls | Cores, can I please get re+Ws on ^ after rebasing to pick up gibi fix? | |
| 21:03:09 | mriedem | fried_rice: done | |
| 21:03:15 | fried_rice | mriedem Thanks | |
| 21:03:36 | fried_rice | Considering how to deal with similar on https://review.openstack.org/#/c/521686/ series... | |
| 21:08:36 | mriedem | figleaf: thanks, comments ilnine | |
| 21:08:38 | mriedem | *inline | |
| 21:09:00 | mriedem | fried_rice: w/o rechecking? | |
| 21:09:12 | openstackgerrit | Merged openstack/nova master: Fix typo in release note https://review.openstack.org/531854 | |
| 21:09:32 | fried_rice | mriedem That bottom one has been languishing all week; I kinda don't want to try a recheck only to find that I needed a rebase. I'm gonna rebase the series... | |
| 21:10:16 | openstackgerrit | Eric Fried proposed openstack/nova master: Track provider traits in report client https://review.openstack.org/521686 | |
| 21:10:16 | openstackgerrit | Eric Fried proposed openstack/nova master: Raise conflict exception on RP create 409 https://review.openstack.org/532563 | |
| 21:10:18 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient.set_traits_for_provider https://review.openstack.org/532564 | |
| 21:10:18 | openstackgerrit | Eric Fried proposed openstack/nova master: Track associated sharing RPs in report client https://review.openstack.org/526539 | |
| 21:10:18 | openstackgerrit | Eric Fried proposed openstack/nova master: Raise on API errors getting aggregates/traits https://review.openstack.org/526540 | |
| 21:10:18 | openstackgerrit | Eric Fried proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756 | |
| 21:10:19 | openstackgerrit | Eric Fried proposed openstack/nova master: Track tree-associated providers in report client https://review.openstack.org/526541 | |
| 21:10:19 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098 | |
| 21:10:20 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187 | |
| 21:10:20 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Use update_provider_tree from resource tracker https://review.openstack.org/520246 | |
| 21:10:21 | openstackgerrit | Eric Fried proposed openstack/nova master: Fix nits in update_provider_tree series https://review.openstack.org/531260 | |
| 21:10:24 | fried_rice | mriedem Done, please re+W https://review.openstack.org/#/c/521686/ | |
| 21:25:42 | mriedem | fried_rice: it's just that one on the bottom right? | |
| 21:25:48 | fried_rice | mriedem Yessir. | |
| 21:26:33 | fried_rice | Been bouncing around zuul-land since Tuesday morning. | |
| 21:26:36 | fried_rice | Thank you sir. | |
| 21:37:26 | figleaf | mriedem: about your comment on https://review.openstack.org/#/c/526436/23/nova/conductor/tasks/migrate.py@242 | |
| 21:37:52 | figleaf | The _revert_allocation() method has already been called at that point | |
| 21:38:33 | mriedem | figleaf: _revert_allocation on the compute deletes the source node allocation for the migration record and moves it back to the instance so the instance allocations are doubled up on the source and dest node | |
| 21:38:48 | mriedem | it doesn't remove the allocation that the instance has on the dest node, just moves it back from migration->instance | |
| 21:39:10 | mriedem | but once we're back in conductor, the instance allocation on the dest node is now the wrong dest node (the failed one), and we need to move it from that to the alternate | |
| 21:39:21 | figleaf | I'm not seeing that in the tests | |
| 21:39:42 | mriedem | unless claim_resources for the instance does a total rewrite of what the instance consumer has allocations on, | |
| 21:39:44 | mriedem | that could be the case | |
| 21:39:49 | mriedem | is it an update/overwrite> | |
| 21:39:50 | mriedem | ? | |
| 21:40:00 | figleaf | It's a PUT, so overwrite | |
| 21:40:10 | mriedem | ok, well that's excellent news | |
| 21:40:29 | mriedem | then i think the only major thing is the migration obj lookup | |
| 21:40:44 | figleaf | mriedem: https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1136 | |
| 21:41:26 | figleaf | I added a filter on status, and also added some allocation tests to verify that only the host that succeeds has allocations | |
| 21:41:27 | mriedem | figleaf: https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1096 | |
| 21:41:47 | mriedem | are you sure we don't hit https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1132 and it thinks we're doing a move | |
| 21:41:52 | mriedem | and double the allocations for the instance again? | |
| 21:42:18 | figleaf | mriedem: I put a break on that line and never hit it. Let me run it again to be sure | |
| 21:43:14 | mriedem | and your test is asserting the failed hosts don't have any allocations on them when we're done rescheduling? | |
| 21:43:34 | mriedem | i guess you said that already | |
| 21:43:48 | figleaf | yeah | |
| 21:44:21 | mriedem | the other thing is, dest_alloc_req is going to be in the new format here now https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L180 | |
| 21:44:29 | mriedem | so i'd expect to hit some issues there, like superdan did in his patch | |
| 21:44:40 | figleaf | I just re-ran the functional test with debug output for the current_allocs in the code, and it's empty every time | |
| 21:44:43 | mriedem | i could have sworn we talked about this during the reschedules on the server create path | |
| 21:44:45 | mriedem | *patch | |
| 21:45:39 | mriedem | idk, doesn't make sense to me that that report client code doesn't think we're doing a move operation b/c the instance should still have the allocation on the originally chosen dest node | |
| 21:45:56 | figleaf | mriedem: got the new format covered: https://review.openstack.org/#/c/526436/23/nova/tests/functional/test_servers.py@2730 | |
| 21:46:00 | mriedem | unless something is deleting that before we get there | |
| 21:46:54 | mriedem | figleaf: i don't understand why your functional test needs to stub any of that out | |
| 21:47:01 | mriedem | kind of defeats the purpose of using a functional test | |