| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-18 | |||
| 16:02:30 | mriedem | dansmith: i thought about that yesterday too | |
| 16:02:42 | mriedem | select_destinations could just return the hosts | |
| 16:02:52 | dansmith | ...yeah | |
| 16:03:08 | mriedem | i'd return whatever CONF.max_attempts is | |
| 16:03:11 | bauzas | there are 2 implementation possibilites | |
| 16:03:13 | dansmith | it already returns multiples now if num_instances is >1, so just return N plus some.. yeah | |
| 16:03:32 | bauzas | either we put POST in consume_from_request() | |
| 16:03:50 | mriedem | jaypipes: cdent: bauzas: https://bugs.launchpad.net/nova/+bug/1683858 | |
| 16:03:50 | openstack | Launchpad bug 1683858 in OpenStack Compute (nova) "Allocation records do not contain overhead information" [High,Triaged] | |
| 16:03:50 | bauzas | or, we do POST right after scheduler returns a destination to conductor | |
| 16:04:08 | mriedem | bauzas: the post isn't the problem, it's where to retry | |
| 16:04:11 | jaypipes | mriedem: thx | |
| 16:04:19 | bauzas | either way, like I discussed with mriedem, we need to DELETE allocations in the conductor on a reschedule/move operation | |
| 16:04:40 | bauzas | mriedem: like I said to you, the scheduler doesn't know whether it's a move or a single boot | |
| 16:05:05 | bauzas | for it, it just answers a specific question which is "which host(s) matches my request" | |
| 16:05:24 | bauzas | *why* we ask the scheduler this question is totally unrelated to it | |
| 16:05:41 | edleafe | bauzas: either way, the resources on the selected host will have to be claimed | |
| 16:06:00 | bauzas | edleafe: in the PS I'm writing, I'm decoupling the DELETE and the POST | |
| 16:06:56 | bauzas | edleafe: in the conductor, I'm planning to GET allocations for that instance and DELETE those if existing, before calling scheduler which would POST the new allocations for that instance | |
| 16:07:09 | openstackgerrit | Jon Bernard proposed openstack/nova master: Add tempest-dsvm-ceph-rc https://review.openstack.org/456292 | |
| 16:08:57 | efried | sfinucan Got a sec to talk through https://review.openstack.org/#/c/455710/8/nova/api/openstack/placement/handler.py@184 ? (andymccr cdent) | |
| 16:09:26 | sfinucan | efried: Sure | |
| 16:09:40 | efried | sfinucan First, assertion that we don't need .get(..., '') | |
| 16:09:57 | efried | Whyzat? Is something upstack guaranteeing that var will be present (even if empty) in the env? | |
| 16:10:55 | sfinucan | efried: '.get' will return 'None' even if the key isn't a dictionary | |
| 16:11:06 | sfinucan | which is different from the dict[key] notation | |
| 16:11:18 | sfinucan | *isn't in a given dictionary | |
| 16:11:38 | efried | mm, fair | |
| 16:12:04 | cdent | this has been through enough iterations I'm not sure if we haven't lost the original point | |
| 16:12:09 | cdent | or maybe points | |
| 16:13:03 | andymccr | sfinucan: so if i understand your point correctly, the subsequent .get()'s after line 183 are not required in that format since we are certain by now that the var does exist. | |
| 16:13:15 | andymccr | although the last one is probably required still since that's for CONTENT_TYPE rather than CONTENT_LENGTH? | |
| 16:13:18 | efried | I wasn't a fan of leaving the int() to raise TypeError/ValueError, but that was the original behavior that supposedly wasn't wanting to be changed. | |
| 16:13:36 | efried | andymccr Right, but leave out the , '' | |
| 16:13:47 | cdent | efried: no we don't want that. we don't want a lose exception to raise | |
| 16:13:53 | cdent | loose | |
| 16:14:04 | andymccr | efried: because it would return None anyway- ok cool i understand | |
| 16:14:05 | efried | cdent Yeah, just thought we might want it to be a better exception | |
| 16:14:25 | efried | cdent Like HTTPBadRequest | |
| 16:14:28 | andymccr | i can add in a try/raise if needed | |
| 16:15:06 | cdent | andymccr: my advice would be to start from a test that demonstrates the bug you're experiencing | |
| 16:15:15 | cdent | make that, make it fail, then fix it | |
| 16:15:40 | cdent | you can do that by faking an environ that you pass to the PlacementHandler class | |
| 16:15:44 | cdent | (as a callable) | |
| 16:16:22 | efried | Yeah, mahbad for suggesting all the , '' / , 0 stuff. There's some other builtin that raises if you don't use that second var (can't remember which, now) that I was confusing with dict.get(). | |
| 16:17:32 | andymccr | cdent: yeah my plan is to cover a few test cases there. i think it should be doable i just need to carve out some time. but to confirm when i propose the next patch set it would be better to have a try block around the int() | |
| 16:17:58 | cdent | I can't remember :) | |
| 16:18:03 | efried | andymccr IMO, yes; cdent may not agree. | |
| 16:18:18 | cdent | go for it | |
| 16:18:26 | andymccr | efried: it makes sense to me so i'll do it and can always revise :) | |
| 16:18:27 | efried | Basically, we don't want a CONTENT_LENGTH that's not an int. | |
| 16:18:35 | cdent | i think once you get tests in place, the solutions will be pretty clear | |
| 16:18:43 | andymccr | yeah agreed | |
| 16:19:25 | efried | Thanks guys. Felt like we were going around in circles a bit trying to have this conversation via gerrit. | |
| 16:20:13 | andymccr | haha yeah probably - and thanks for the pointers/help on that one. | |
| 16:21:23 | openstackgerrit | dane-fichter proposed openstack/nova master: Add trusted certificates to InstanceExtras https://review.openstack.org/457711 | |
| 16:21:44 | efried | andymccr cdent sfinucan Aha - it was getattr() I was confusing with. If you don't provide the default there, it raises. <facepalm> | |
| 16:22:33 | sfinucan | efried: Yup, I only realized last week that you could even use a default. Had been using a 1-2 hasattr/getattr combo up to that point | |
| 16:27:22 | cdent | efried: thanks for that linking | |
| 16:27:32 | efried | yahyoubetcha | |
| 16:33:00 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Add three new '[libvirt] live_migration_*' options https://review.openstack.org/456571 | |
| 16:33:00 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Gather 'live_migration_scheme', 'live_migration_inbound_addr' https://review.openstack.org/456572 | |
| 16:33:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Convert 'live_migration_inbound_addr' to HostAddressOpt https://review.openstack.org/456573 | |
| 16:33:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Set default for live_migration_scheme https://review.openstack.org/457716 | |
| 16:34:40 | johnthetubaguy | mriedem: I have some questions on https://review.openstack.org/#/c/452577 I just took a look | |
| 16:34:54 | johnthetubaguy | mriedem: I am trying to workout why the existing logic doesn't work correctly | |
| 16:46:21 | efried | mriedem jaypipes Clarification on https://review.openstack.org/#/c/454983/ (Service catalog for endpoints): Do we *want* to include config options like [cinder]catalog_info for all the services, or just use a hardcoded service_type? | |
| 16:48:04 | jaypipes | efried: I'd prefer to use a hard-coded service type, from the service-types-authority: https://github.com/openstack/service-types-authority/ | |
| 16:48:19 | jaypipes | efried: unfortunately cinder isn't in there :( | |
| 16:48:37 | efried | jaypipes Okay, I dig the idea of having fewer conf settings. But edmondsw also had concerns about hardcoding the endpoint_type. | |
| 16:49:00 | jaypipes | efried: but cinder is proposed for "block-storage": https://review.openstack.org/#/c/436178/ | |
| 16:49:40 | jaypipes | efried: for Cinder, specifically, we'll need to have a list of hard-coded service types and try them in order. | |
| 16:50:05 | jaypipes | efried: for instance, try "block-storage", then "volumev2", then "volume", etc | |
| 16:50:25 | jaypipes | efried: because Cinder is the odd man out of all this. | |
| 16:52:09 | mriedem | efried: the whole point is we don't want catalog_info | |
| 16:52:18 | mriedem | so we're going to deprecate that once we start using ksa | |
| 16:52:22 | jaypipes | right | |
| 16:52:25 | mriedem | and we just have a service_type and interface option | |
| 16:52:29 | jaypipes | bingo | |
| 16:52:32 | mriedem | so service_type=cinderv3, interface=public | |
| 16:52:34 | mriedem | or whatever | |
| 16:52:43 | jaypipes | and frankly, even the interface is questionable... but meh. :) | |
| 16:52:44 | mriedem | interface defaults to None in nova.conf, which defaults to public in ksa | |
| 16:52:47 | mriedem | sure | |
| 16:52:58 | mriedem | i only mention interface b/c of the one we added for placement for andymccr | |
| 16:53:01 | jaypipes | ya | |
| 16:53:19 | mriedem | johnthetubaguy: ok will look after the notifications meeting | |
| 16:53:35 | mriedem | where i'm sure we'll talk about loading instance.tags by default | |
| 16:53:40 | mriedem | so dansmith might want to be there | |
| 16:53:56 | dansmith | "want" | |
| 16:54:09 | mriedem | "so i want dansmith to be there" | |
| 16:54:11 | mriedem | better? | |
| 16:54:13 | dansmith | yes | |
| 16:54:29 | mriedem | sorry i'm midwestern | |
| 16:54:33 | mriedem | and therefore passive aggressive | |
| 16:54:58 | openstackgerrit | Sylvain Bauza proposed openstack/nova-specs master: Claims in the scheduler https://review.openstack.org/437424 | |
| 16:55:11 | bauzas | mriedem: dansmith: edleafe: cdent: jaypipes: BOOOM ^ | |
| 16:55:19 | bauzas | that said, \o | |
| 16:55:33 | jaypipes | bauzas: k, thx. will re-review shortly. | |
| 16:55:58 | mordred | TIL novaclient validates payloads more in python3 than in python2 | |