| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-28 | |||
| 10:56:52 | sfinucan | I'm still trying to parse how that extension works, but I don't think it has | |
| 10:57:22 | johnthetubaguy | sfinucan: it leaks information to the user about the rxtx limits on a given flavor | |
| 10:57:27 | sfinucan | All the API samples for flavors-detail etc. show a 'rxtx_factor' parameter | |
| 10:57:33 | sfinucan | e.g. doc/api_samples/flavors/flavors-detail-resp.json | |
| 10:57:36 | johnthetubaguy | sfinucan: or rather you can hide that limit from the user | |
| 10:57:48 | johnthetubaguy | sfinucan: correct, the default policy allows you to see that | |
| 10:58:09 | sfinucan | ah, so if we defaulted to false those API samples would need to be updated | |
| 10:58:39 | johnthetubaguy | sfinucan: I believe so | |
| 10:59:23 | johnthetubaguy | sfinucan: its in the change: https://review.openstack.org/#/c/444036/13/nova/api/openstack/compute/flavor_rxtx.py | |
| 11:00:54 | johnthetubaguy | sfinucan: now there is a slightly stupid thing, there is nothing in tree that actually uses that property, AFAIK | |
| 11:02:36 | sfinucan | I had it in my head that a controller == a URL/endpoint though. I was expecting to see something FlavorRxtxController map to '/flavors/{flavorID}/rxtx' or something | |
| 11:03:21 | johnthetubaguy | sfinucan: thats an extension to an existing controller | |
| 11:03:48 | sfinucan | yup - literally *just* saw the @wsgi.extends decorator :) This is starting to make sense | |
| 11:03:59 | johnthetubaguy | its mid evolution in there, it used to be clearer | |
| 11:04:13 | johnthetubaguy | they are all getting rolled into the main API code this cycle | |
| 11:04:22 | johnthetubaguy | to stop the... crazy | |
| 11:06:13 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Pass APIVersionRequest to extensions https://review.openstack.org/425876 | |
| 11:06:56 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: WIP: Specify keymap on server boot (2/2) https://review.openstack.org/460987 | |
| 11:09:23 | sfinucan | johnthetubaguy: Hmm, looking at the various '_flavor_get_from_db' functions in 'nova/objects/flavor.py', I don't see anything that would prevent the 'rxtx_factor' being populated in the standard response | |
| 11:09:34 | sfinucan | This stuff is slightly confusing indeed :) | |
| 11:09:53 | sfinucan | ...after lunch | |
| 11:10:19 | johnthetubaguy | sfinucan: it could be broken, I bet there are no tests for all that | |
| 11:17:22 | openstackgerrit | John Garbutt proposed openstack/nova master: Add description to policies in migrations.py https://review.openstack.org/459727 | |
| 11:30:54 | zioproto | I have a nova bug, I am not sure if I explained by self correctly: https://bugs.launchpad.net/nova/+bug/1686999 | |
| 11:30:54 | openstack | Launchpad bug 1686999 in OpenStack Compute (nova) "Instance with two attached volumes fails to start with error: Duplicate ID 'drive-ide0-0-0' for drive" [Undecided,New] | |
| 12:13:40 | sfinucan | johnthetubaguy: I _think_ that controller did nothing | |
| 12:13:58 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'os-flavor-rxtx' policy https://review.openstack.org/444036 | |
| 12:13:58 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'FlavorRxtxController' extension https://review.openstack.org/461010 | |
| 12:13:59 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove useless API tests https://review.openstack.org/461011 | |
| 12:14:42 | sfinucan | So I've written those ^^^ to test as much (in the gate - weird "no db" issues if I try running 'tox -e api-samples' locally. If so, we can delete the policy as also doing nothing, IMO | |
| 12:44:31 | mriedem | o/ | |
| 12:45:42 | gibi | mriedem: hi! I think I figured out what happens with flavor-delete in https://review.openstack.org/#/c/460377 , I left comments in the patch | |
| 12:46:37 | mriedem | gibi: ah thanks | |
| 12:46:46 | mriedem | i must have been looking at the wrong main db api method | |
| 12:48:02 | mriedem | gibi: i'm not sure about changing it from [] to None | |
| 12:48:14 | mriedem | i'd have to change one of the notification samples at least | |
| 12:48:51 | gibi | mriedem: yes, but I think it is bugfix on that sample | |
| 12:49:08 | gibi | mriedem: before your patch flavor-delete always reported projects=[] | |
| 12:49:20 | gibi | mriedem: even if the flavor had some projects associated before the delete | |
| 12:49:26 | gibi | mriedem: and that is missleading | |
| 12:49:40 | gibi | mriedem: we use None (null) value in the notification to signal if data is missing | |
| 12:49:42 | mriedem | yeah | |
| 12:49:44 | mriedem | ok, | |
| 12:49:55 | mriedem | then i think i'll probably pull that part of the change out and do that first | |
| 12:50:07 | gibi | mriedem: sure, that would be cleaner | |
| 13:01:03 | johnthetubaguy | mriedem: was Nova v1.0 really a thing, I thought only the EC2 API was used back then? | |
| 13:01:55 | mriedem | johnthetubaguy: i found it in the 2010.1 release code in launchpad | |
| 13:02:14 | mriedem | https://launchpad.net/nova/austin/2010.1 | |
| 13:02:16 | johnthetubaguy | mriedem: curious | |
| 13:02:44 | mriedem | nova/api/__init__.py | |
| 13:03:01 | mriedem | http://paste.openstack.org/show/608325/ | |
| 13:06:03 | mriedem | gibi: oh, it can't be None | |
| 13:06:07 | mriedem | flavor.projects is not a nullable field | |
| 13:06:50 | johnthetubaguy | mriedem: crazy stuff, thanks. | |
| 13:07:04 | bauzas | mriedem: dansmith: FWIW, Monday May 1st is a French (and EU) holiday + I'll be on PTO by Tuesday | |
| 13:07:08 | mriedem | johnthetubaguy: yeah really, it was an interesting journey | |
| 13:07:42 | bauzas | mriedem: dansmith: so we should be discussing about the scheduler claims by either today or Wed | |
| 13:08:13 | mriedem | bauzas: ok...i haven't gone through those changes yet, and it's still < 1 cup of coffee here at this point | |
| 13:08:31 | mriedem | and john is already asking me about API versions from the austin release :) | |
| 13:09:02 | gibi | mriedem: :/ | |
| 13:09:43 | bauzas | mriedem: heh okay | |
| 13:09:56 | gibi | mriedem: we somehow ensure that the schema population detects the problem and defaults the payload field to None | |
| 13:10:08 | gibi | gibi: let me play with that patch a bit... | |
| 13:10:17 | mriedem | gibi: it used to do that because of the OrphanedObjectError i thought | |
| 13:10:46 | bauzas | mriedem: dansmith: the main discussion would be around knowing if we sould call placement by either conductor or scheduler (because of the problem I said by https://review.openstack.org/#/c/460455/ | |
| 13:11:17 | gibi | mriedem: yes, so basically what we have to do is to orphan the flavor object | |
| 13:13:09 | gibi | mriedem: so set flavor._context = None; here https://review.openstack.org/#/c/460377/2/nova/objects/flavor.py@544 | |
| 13:13:10 | mriedem | bauzas: gdi you're going to make me talk about this right now aren't you | |
| 13:13:16 | mriedem | gibi: yup did it and testing | |
| 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. | |