| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-28 | |||
| 13:13:46 | mriedem | bauzas: i think doing it in conductor is ok, we had also talked about this as an alternative, | |
| 13:13:58 | mriedem | and said we could do the retries in conductor if we return the hosts from the scheduler via select_destinations | |
| 13:15:10 | jaypipes | figleaf: ah, great point on the negative tests on that SQL. will add. | |
| 13:16:44 | mriedem | bauzas: comments inline | |
| 13:17:09 | openstackgerrit | John Garbutt proposed openstack/nova master: Update description to policies in remaining flavor APIs https://review.openstack.org/460509 | |
| 13:17:10 | mriedem | bauzas: fwiw dansmith always wanted to do it in conductor anyway, because that's normally where we do retries, and it makes scheduler more of a library, which i think is a long-term nice to have | |
| 13:19:21 | cdent | figleaf: can you sanity check the comments on the same thing you just reviewed? I feel like I'm maybe missing something, or something big is missing. | |
| 13:19:44 | bauzas | mriedem: okay | |
| 13:19:47 | bauzas | mriedem: looking | |
| 13:19:58 | bauzas | mriedem: also, you asked to rebase https://review.openstack.org/#/c/460113/1/nova/compute/resource_tracker.py | |
| 13:20:24 | bauzas | mriedem: shouldn't we rather maybe merge first ^ and then possibly add a new change for adding the needed modification ? | |
| 13:20:50 | bauzas | mriedem: I wouldn't like to wait for https://review.openstack.org/#/c/457782/ for that | |
| 13:21:13 | bauzas | mriedem: or I could write a follow-up that would be on top of the above if you agree | |
| 13:23:03 | mriedem | bauzas: i think https://review.openstack.org/#/c/457782/ is basically in, i was just waited for leakypipes to reply | |
| 13:23:14 | mriedem | about the spooning that's happening between the virt drivers and the RT | |
| 13:23:31 | bauzas | mriedem: of course, I tend to agree | |
| 13:24:19 | bauzas | mriedem: I just think that https://review.openstack.org/#/c/460113/ could just add the opt, and https://review.openstack.org/#/c/457782/ would have a follow-up adding the needed reserved stuff | |
| 13:24:43 | bauzas | mriedem: given I'm pretty operators won't use the new opt by the next weeks :) | |
| 13:24:57 | leakypipes | mriedem: I did reply... | |
| 13:25:00 | mriedem | leakypipes: i know | |
| 13:25:07 | leakypipes | ah, ok. thx luv. | |
| 13:25:08 | mriedem | leakypipes: and now i'm going to confront you about it | |
| 13:25:11 | mriedem | leakypipes: so https://review.openstack.org/#/c/457782/2/nova/virt/libvirt/driver.py | |
| 13:25:22 | leakypipes | heh, ok | |
| 13:25:38 | mriedem | leakypipes: would you be opposed to not setting reserved=0 for VCPU inventory in the libvirt driver and default that in the RT like we're doing for MEMORY_MB and DISK_GB? | |
| 13:25:45 | mriedem | that's i think the main thing bugging me, | |
| 13:25:58 | mriedem | is that the libvirt driver is setting reserved=0 for VCPU b/c it knows that the RT helper method isn't doing it | |
| 13:26:40 | mriedem | i think i'd like to get to the point that either the driver provides everything (which would be redundant between drivers), or the drivers only override what they need to, and the RT fills in all the blanks | |
| 13:27:14 | leakypipes | mriedem: I'd be totes ok with that. | |
| 13:27:27 | mriedem | if you make that change i'm +10 | |
| 13:27:35 | leakypipes | mriedem: we just never had any vcpus reserved option until bauzas' recent patch. | |
| 13:27:43 | leakypipes | mriedem: so it was just a hard-coded ugh. | |
| 13:27:50 | leakypipes | mriedem: happy to move it into the RT. | |
| 13:27:51 | bauzas | oh snap, it's Friday | |
| 13:29:01 | mriedem | leakypipes: ok cool, | |
| 13:29:09 | mriedem | left a comment and -1 for posterity :) | |
| 13:30:12 | bauwser | so many political folks want posterity... | |
| 13:30:24 | bauwser | could I just -1 them ? | |
| 13:31:23 | bauwser | life would be so much simple if our Parliaments were using Gerrit for voting laws | |
| 13:32:31 | figleaf | cdent: to which review are you referring? | |
| 13:33:12 | cdent | figleaf: sorry, I was thinking that would land on your brain at the same time as I typied, which is stupid: https://review.openstack.org/#/c/460798/ | |
| 13:33:43 | figleaf | cdent: on it | |
| 13:33:43 | cdent | figleaf: the key bit is not the bit about how the query works (unless you care) but the bit about aggregates not playing a part | |
| 13:35:00 | mriedem | gibi: ok got this working, patch coming shortly | |
| 13:35:07 | mriedem | once i can wordsmith the commit message | |
| 13:36:12 | cdent | mriedem, sdague: to follow up on the thing that sdague already fixed by changing the flags, my patch which does a greenthread sleep around the requests has passed 14 rechecks (as expected): https://review.openstack.org/#/c/459733/ | |
| 13:36:15 | figleaf | cdent: Hmmm... I see your point. I guess if the unit tests covered cases with multiple aggregates available, it would have shown up | |
| 13:36:21 | cdent | figleaf: yes | |
| 13:36:21 | gibi | mriedem: nice | |
| 13:36:41 | figleaf | cdent: I missed the missing stuff :) | |
| 13:36:51 | mriedem | cdent: do we need that if the keep_alive thing fixed it? | |
| 13:37:09 | leakypipes | bauwser: feel like +2ing https://review.openstack.org/#/c/460281/ and/or figleaf's https://review.openstack.org/#/c/453862/? | |
| 13:37:27 | cdent | mriedem: no, I'm just mentioning it for sake of data (that using eventlet in tests is somewhat bad...) | |
| 13:37:41 | bauwser | leakypipes: I *could* | |
| 13:37:43 | mriedem | cdent: ah ok | |
| 13:37:47 | bauwser | just need to rebase some stuff :) | |
| 13:38:21 | cdent | mriedem: and with that done, I'll abandon | |
| 13:38:40 | leakypipes | Vgq | |
| 13:38:42 | leakypipes | :wq | |
| 13:38:45 | leakypipes | oh ffs | |
| 13:39:10 | figleaf | leakypipes: I didn't know 'ffs' was a vim command | |
| 13:39:17 | leakypipes | lmao | |
| 13:39:28 | bauwser | leakypipes: so, you'd be killing me | |
| 13:39:34 | bauwser | leakypipes: /me runs | |
| 13:40:03 | bauwser | because https://review.openstack.org/#/c/460281/ will have a bad day because of https://review.openstack.org/#/c/460281/ | |
| 13:40:36 | figleaf | bauwser: we need one to merge so that the other can rebase on top of it | |
| 13:40:47 | bauwser | figleaf: I know | |
| 13:40:50 | leakypipes | bauwser: yup, I know. it's a quick rebase for me, so feel free to +2 ed's | |
| 13:41:06 | bauwser | leakypipes: figleaf: FWIW, I did +2 ed's one | |
| 13:41:12 | leakypipes | coolio, thx! | |
| 13:41:15 | bauwser | hence my point about you needing extra work :) | |
| 13:41:28 | leakypipes | not a prob :) | |
| 13:41:37 | bauwser | I'll just +2 yours and so, we could +2/+W quickly after the rebase | |
| 13:41:37 | figleaf | bauwser: thanks. Always good to make jay work harder | |
| 13:41:44 | leakypipes | heh | |
| 13:41:45 | sfinucan | johnthetubaguy: Turns out the DB isn't what populates the output. A ViewBuilder does, which appears to be doing exactly as expected. Seems we do need a deprecation cycle for that policy, as you suggest | |
| 13:42:05 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Don't lazy-load flavor.projects during destroy() https://review.openstack.org/461032 | |
| 13:42:06 | mriedem | gibi: ^ | |
| 13:42:56 | johnthetubaguy | sfinucan: yeah, view builder extracts from the DB object, else DB changes would leak out the API by acciedent | |
| 13:48:44 | mriedem | johnthetubaguy: is there anything holding you up on https://review.openstack.org/#/c/459111/ and below? | |
| 13:49:45 | johnthetubaguy | mriedem: was doing TC stuff this morning, just been thinking of getting a coffee and looking at that now | |
| 13:49:53 | leakypipes | bauwser: k, rebased https://review.openstack.org/460281 | |
| 13:50:05 | mriedem | johnthetubaguy: cool | |
| 13:50:17 | mriedem | johnthetubaguy: i applaud you for actually doing TC stuff | |
| 13:50:33 | bauwser | leakypipes: +wapit | |
| 13:51:42 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove compatibility code for flavors https://review.openstack.org/460377 | |
| 13:52:58 | mriedem | bauwser: you wanted to do a newton release soon yeah? | |
| 13:53:03 | mriedem | we have two changes with +2s https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:stable/newton | |
| 13:53:10 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove useless API tests https://review.openstack.org/461011 | |
| 13:53:10 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'FlavorRxtxController' extension https://review.openstack.org/461010 | |
| 13:53:11 | bauwser | mriedem: k | |
| 13:53:15 | bauwser | mriedem: looking at those | |
| 13:55:05 | bauwser | figleaf: not sure you understood my comment on https://review.openstack.org/#/c/460177/1/nova/compute/resource_tracker.py@1004 | |
| 13:55:34 | cdent | thanks sfinucan, TIL | |
| 13:55:53 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: implement get_inventory() for libvirt https://review.openstack.org/457782 | |
| 13:55:54 | leakypipes | bauwser, mriedem, cdent, figleaf: ^ | |
| 13:56:10 | sfinucan | cdent: Intel's favourite paradox, last I checked ;) | |
| 13:56:48 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: WIP: Delete existing allocations for move operations or retries https://review.openstack.org/460433 | |
| 13:56:48 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: WIP: Don't delete allocation if instance being scheduled https://review.openstack.org/460177 | |
| 13:56:49 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: WIP: Allocate instance by the conductor https://review.openstack.org/460455 | |