| openstackgerrit | Merged openstack/nova master: Make scheduler client allow multiple member_of query parameters https://review.openstack.org/568313 | 00:02 |
|---|---|---|
| openstackgerrit | Merged openstack/nova master: Remove '_apply_instance_name_template' https://review.openstack.org/567257 | 00:03 |
| openstackgerrit | Merged openstack/nova master: XenAPI: Pass expected return codes to resize2fs https://review.openstack.org/568318 | 00:03 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in test_xenapi.py (3) https://review.openstack.org/564645 | 00:36 |
| *** mriedem has joined #openstack-placement | 00:40 | |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Return nested providers in get_by_request https://review.openstack.org/567113 | 01:05 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Add traits check in nested provider candidates https://review.openstack.org/567150 | 01:05 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Support nested alloc cands with sharing providers https://review.openstack.org/567508 | 01:05 |
| openstackgerrit | Merged openstack/nova master: add lower-constraints job https://review.openstack.org/555961 | 01:12 |
| *** mriedem has quit IRC | 01:17 | |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Return all resources in provider_summaries https://review.openstack.org/558045 | 01:54 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in nova/tests/unit/virt/xenapi/stubs.py https://review.openstack.org/568412 | 02:20 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Return all nested providers in tree https://review.openstack.org/559480 | 02:38 |
| openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Use ThreadPoolExecutor for max_concurrent_live_migrations https://review.openstack.org/563505 | 03:35 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Return all resources in provider_summaries https://review.openstack.org/558045 | 04:37 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Return all nested providers in tree https://review.openstack.org/559480 | 04:37 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Support nested allocation candidates in placement https://review.openstack.org/565487 | 04:37 |
| openstackgerrit | Naichuan Sun proposed openstack/nova master: XenAPI: update the document related to vdi streaming https://review.openstack.org/568444 | 05:16 |
| openstackgerrit | Merged openstack/nova master: Added ability to configure default architecture for ImagePropertiesFilter https://review.openstack.org/566425 | 05:26 |
| *** jaypipes has quit IRC | 05:41 | |
| *** jaypipes has joined #openstack-placement | 05:41 | |
| openstackgerrit | Merged openstack/nova stable/queens: Update os_compute_api:os-flavor-extra-specs:index docs for 2.47 https://review.openstack.org/563253 | 05:45 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Add tests for alloc_cands with member_of https://review.openstack.org/561399 | 05:56 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Fix member_of with sharing providers https://review.openstack.org/561400 | 05:56 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Expand member_of functional test cases https://review.openstack.org/566011 | 05:56 |
| *** belmoreira has joined #openstack-placement | 06:19 | |
| *** belmoreira has quit IRC | 06:24 | |
| openstackgerrit | jichenjc proposed openstack/nova-specs master: Add additional information for z/VM spec. https://review.openstack.org/562154 | 06:24 |
| *** tssurya has joined #openstack-placement | 06:25 | |
| *** efried has quit IRC | 06:36 | |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in test_compute_api.py (4) https://review.openstack.org/568462 | 06:42 |
| *** tssurya has quit IRC | 06:51 | |
| *** belmoreira has joined #openstack-placement | 06:55 | |
| *** belmoreira has quit IRC | 06:58 | |
| openstackgerrit | sahid proposed openstack/nova master: libvirt: place emulator threads on CONF.cpu_shared_set https://review.openstack.org/510897 | 07:04 |
| *** diga has joined #openstack-placement | 07:08 | |
| openstackgerrit | Martin Midolesov proposed openstack/nova master: vmware:add support for the hw_video_ram image property https://review.openstack.org/564193 | 07:42 |
| *** avolkov has joined #openstack-placement | 07:57 | |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova-specs master: Complex (Anti)-Affinity Policies https://review.openstack.org/546925 | 08:16 |
| *** finucannot is now known as stephenfin | 08:21 | |
| openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Use ThreadPoolExecutor for max_concurrent_live_migrations https://review.openstack.org/563505 | 08:23 |
| openstackgerrit | jichenjc proposed openstack/nova master: WIP: Remove support for /os-fixed-ips REST API https://review.openstack.org/568516 | 08:25 |
| openstackgerrit | Naichuan Sun proposed openstack/nova master: xenapi(N-R-P):Get vgpu info from `allocations` https://review.openstack.org/521717 | 08:45 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: Simplify instance name generation https://review.openstack.org/516573 | 08:48 |
| *** e0ne has joined #openstack-placement | 08:50 | |
| *** edmondsw has joined #openstack-placement | 08:52 | |
| openstackgerrit | Naichuan Sun proposed openstack/nova master: xenapi(N-R-P): Add API to support compute node resource provider update and create https://review.openstack.org/521041 | 08:54 |
| *** edmondsw has quit IRC | 08:56 | |
| openstackgerrit | Naichuan Sun proposed openstack/nova master: xenapi(N-R-P): support compute node resource provider update https://review.openstack.org/521041 | 08:59 |
| *** cdent has joined #openstack-placement | 08:59 | |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove unnecessary 'to_primitive' call https://review.openstack.org/568532 | 09:03 |
| openstackgerrit | Merged openstack/nova master: libvirt: Report the virtual size of RAW disks https://review.openstack.org/567899 | 09:08 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Support nested alloc cands with sharing providers https://review.openstack.org/567508 | 09:15 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Return all resources in provider_summaries https://review.openstack.org/558045 | 09:15 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Return all nested providers in tree https://review.openstack.org/559480 | 09:15 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Add microversion for nested allocation candidate https://review.openstack.org/565487 | 09:15 |
| openstackgerrit | Zhenyu Zheng proposed openstack/nova master: WIP abort live migration in queue. https://review.openstack.org/568542 | 09:19 |
| *** e0ne has quit IRC | 09:29 | |
| *** e0ne has joined #openstack-placement | 09:33 | |
| *** e0ne has quit IRC | 09:38 | |
| *** e0ne has joined #openstack-placement | 09:59 | |
| openstackgerrit | Chris Dent proposed openstack/nova master: Use nova.db.api directly https://review.openstack.org/543262 | 10:13 |
| *** edmondsw has joined #openstack-placement | 10:40 | |
| *** diga has quit IRC | 10:41 | |
| *** edmondsw has quit IRC | 10:44 | |
| *** cdent has quit IRC | 11:17 | |
| *** edmondsw has joined #openstack-placement | 11:21 | |
| openstackgerrit | Balazs Gibizer proposed openstack/nova master: placement: Fix HTTP error generation https://review.openstack.org/568567 | 11:51 |
| *** efried has joined #openstack-placement | 11:52 | |
| *** cdent has joined #openstack-placement | 11:57 | |
| jaypipes | efried: easy one: https://review.openstack.org/#/c/568567/ | 12:19 |
| efried | I'LL BE THE JUDGE OF THAT! | 12:20 |
| jaypipes | :) | 12:20 |
| *** mriedem has joined #openstack-placement | 12:20 | |
| efried | jaypipes, cdent: I thought we had already put the inventory-in-use error code in place in that spot | 12:22 |
| efried | hm, apparently just for set_inventories. | 12:23 |
| cdent | efried: yeah, when the code stuff was started the first code was only added in one spot | 12:24 |
| efried | cdent: Will the error code be the same for that other spot? | 12:24 |
| cdent | Not sure, but it doesn't really matter for this patch does it? That would be a separate change. | 12:26 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Support nested alloc cands with sharing providers https://review.openstack.org/567508 | 12:27 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Return all resources in provider_summaries https://review.openstack.org/558045 | 12:27 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Return all nested providers in tree https://review.openstack.org/559480 | 12:27 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Add microversion for nested allocation candidate https://review.openstack.org/565487 | 12:27 |
| efried | cdent: Right, separate change, but one I'd like to do now while I'm thinking about it. | 12:28 |
| * cdent forgot that efried is s stack | 12:28 | |
| openstackgerrit | Mohammed Naser proposed openstack/nova stable/queens: Added ability to configure default architecture for ImagePropertiesFilter https://review.openstack.org/568575 | 12:28 |
| cdent | efried: but yeah, I woudl reckon it's the same | 12:28 |
| openstackgerrit | Matt Riedemann proposed openstack/nova stable/ocata: libvirt: Report the virtual size of RAW disks https://review.openstack.org/568382 | 12:31 |
| openstackgerrit | Balazs Gibizer proposed openstack/osc-placement master: CLI for traits (v1.6) https://review.openstack.org/514643 | 12:36 |
| openstackgerrit | Balazs Gibizer proposed openstack/osc-placement master: Resource class set (v1.7) https://review.openstack.org/514644 | 12:36 |
| openstackgerrit | Balazs Gibizer proposed openstack/osc-placement master: Usages per project and user (v1.8, v1.9) https://review.openstack.org/514646 | 12:36 |
| openstackgerrit | Balazs Gibizer proposed openstack/osc-placement master: CLI allocation candidates (v1.10) https://review.openstack.org/514647 | 12:36 |
| openstackgerrit | Balazs Gibizer proposed openstack/osc-placement master: New dict format of allocations (v1.11, v1.12) https://review.openstack.org/542819 | 12:36 |
| openstackgerrit | Balazs Gibizer proposed openstack/osc-placement master: Transactionally update allocations (v1.13) https://review.openstack.org/546674 | 12:36 |
| openstackgerrit | Balazs Gibizer proposed openstack/osc-placement master: Add nested resource providers (v1.14) https://review.openstack.org/546675 | 12:36 |
| openstackgerrit | Balazs Gibizer proposed openstack/osc-placement master: Limit allocation candidates (v1.15, v1.16) https://review.openstack.org/548043 | 12:36 |
| openstackgerrit | Balazs Gibizer proposed openstack/osc-placement master: Allocation candidates parameter: required (v1.17) https://review.openstack.org/548326 | 12:36 |
| efried | jaypipes: Easy one :P https://review.openstack.org/#/c/568353/ | 12:41 |
| gibi | the day of easy patches :) | 12:53 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Add INVENTORY_INUSE to DELETE /rp/{u}/inventories https://review.openstack.org/568578 | 12:55 |
| efried | cdent, gibi, jaypipes: ^ | 12:55 |
| efried | gibi: except for tetsuro's stuff | 12:55 |
| cdent | ✔ | 12:55 |
| gibi | efried: can we now get rid of the nova.scheduler.client.report._RE_INV_IN_USE regex ? | 12:56 |
| efried | gibi: Unfortunately not quite that easy. That code is using the regex to scrape out the name of the conflicting resource class and using it for the message. | 12:57 |
| efried | gibi: So unless we want to dumb down that message, it doesn't really help us to add the check for this code. | 12:58 |
| efried | gibi: See nova.scheduler.client.report._extract_inventory_in_use | 12:58 |
| gibi | efried: as far as i see this regexp only extracts the rc to include it into another exception text | 13:00 |
| efried | correct | 13:00 |
| gibi | can we simply reuse the orignal detail text from the placement in the nova exception? | 13:00 |
| gibi | this is the regexp | 13:01 |
| gibi | ("Inventory for (.+) on resource provider " | 13:01 |
| gibi | "(.+) in us | 13:01 |
| efried | gibi: Well, we don't know which resource class conflicts without scraping the message body. | 13:01 |
| gibi | and this is what we create from it | 13:02 |
| gibi | "Inventory for '%(resource_classes)s' on " | 13:02 |
| gibi | "resource provider '%(resource_provider)s' in use." | 13:02 |
| efried | oh, I see | 13:02 |
| efried | Yeah, that's a good idea :) | 13:02 |
| gibi | :) | 13:02 |
| efried | Was planning to muck with that in a separate change anyway, though. Esp. since the report client doesn't use the delete path at all anyway. | 13:02 |
| gibi | efried: separate change sounds good to me; | 13:03 |
| efried | so in fact the changes are independent. | 13:03 |
| efried | don't even need to be in series. | 13:03 |
| jaypipes | efried: nice. +W | 13:03 |
| efried | jaypipes: thx | 13:03 |
| gibi | efried: so I'm +2 https://review.openstack.org/#/c/568578 | 13:04 |
| gibi | jaypipes: another easy one ^^ | 13:04 |
| efried | cdent: know off the top how I would look for the error code from the client side? | 13:05 |
| cdent | response_json['errors'][0]['code'] <- total guess off the top of my head | 13:05 |
| cdent | is that what you're asking? | 13:06 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Update auth_url in install docs https://review.openstack.org/568002 | 13:06 |
| efried | cdent: Yeah. | 13:06 |
| efried | thanks | 13:06 |
| jaypipes | efried: also +W | 13:07 |
| efried | jaypipes: thanks | 13:07 |
| gibi | what a productive day | 13:09 |
| *** tetsuro has joined #openstack-placement | 13:34 | |
| tetsuro | jaypipes: responsed in https://review.openstack.org/#/c/567150/11 | 13:34 |
| openstackgerrit | sahid proposed openstack/nova master: libvirt: place emulator threads on CONF.compute.cpu_shared_set https://review.openstack.org/510897 | 13:35 |
| openstackgerrit | Vladyslav Drok proposed openstack/nova master: Placement: allow to set reserved value equal to total for inventory https://review.openstack.org/564838 | 13:35 |
| openstackgerrit | Vladyslav Drok proposed openstack/nova master: ironic: Report resources as reserved when needed https://review.openstack.org/517921 | 13:35 |
| openstackgerrit | Vladyslav Drok proposed openstack/nova master: Placement: allow to set reserved value equal to total for inventory https://review.openstack.org/564838 | 13:38 |
| openstackgerrit | Vladyslav Drok proposed openstack/nova master: Ironic: report 0 for vcpus/memory_mb/disk_gb resources https://review.openstack.org/565841 | 13:38 |
| openstackgerrit | Vladyslav Drok proposed openstack/nova master: ironic: Report resources as reserved when needed https://review.openstack.org/517921 | 13:38 |
| openstackgerrit | Vladyslav Drok proposed openstack/nova master: Ironic: report 0 for vcpus/memory_mb/disk_gb resources https://review.openstack.org/565841 | 13:39 |
| openstackgerrit | Merged openstack/osc-placement master: CLI for traits (v1.6) https://review.openstack.org/514643 | 13:43 |
| *** tetsuro has quit IRC | 14:07 | |
| efried | jaypipes, mriedem: What do we need to do to get vdrok's specless bp approved? https://blueprints.launchpad.net/nova/+spec/allow-reserved-equal-total-inventory | 14:08 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Move image conversion to privsep. https://review.openstack.org/554437 | 14:09 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: We don't need utils.trycmd any more. https://review.openstack.org/554439 | 14:09 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: We no longer need rootwrap. https://review.openstack.org/554438 | 14:09 |
| mriedem | https://docs.openstack.org/nova/latest/contributor/blueprints.html | 14:09 |
| efried | vdrok: given that it's a microversion, I suspect you may need a spec. (But didn't we already talk about this?) | 14:14 |
| vdrok | mriedem: ouch, I did not read that before :( efried yeah can do the spec today | 14:15 |
| *** cdent has quit IRC | 14:16 | |
| efried | gibi, jaypipes: Can y'all please hit https://review.openstack.org/#/c/515811/ again today? | 14:17 |
| jaypipes | efried: will try. got a heck of a backlog. | 14:18 |
| gibi | efried: how do you feel about mriedem's comments about the end to end tests? | 14:19 |
| efried | gibi: Agree it's a good idea; agree to do it in a fup. | 14:19 |
| gibi | efried: OK | 14:19 |
| *** cdent has joined #openstack-placement | 14:24 | |
| openstackgerrit | Artom Lifshitz proposed openstack/nova master: Refactor _build_device_metadata https://review.openstack.org/533804 | 14:31 |
| openstackgerrit | Artom Lifshitz proposed openstack/nova master: Consider hostdev devices when building metadata https://review.openstack.org/533805 | 14:31 |
| openstackgerrit | Merged openstack/nova master: __str__ methods for RequestGroup, ResourceRequest https://review.openstack.org/568353 | 14:49 |
| efried | cdent: does this look familiar to you? | 14:50 |
| efried | # Set accept header on every request to ensure we notify placement | 14:50 |
| efried | # service of our response body media type preferences. | 14:50 |
| efried | client.additional_headers = {'accept': 'application/json'} | 14:50 |
| cdent | that used to be required, but isn't so much any more | 14:51 |
| efried | cdent: It doesn't seem to be working =========================vvvvvvvvvvvvvvv | 14:51 |
| efried | (Pdb) resp.request.headers | 14:51 |
| efried | {'Content-Length': '193', 'Accept-Encoding': 'gzip, deflate', 'Accept': '*/*', 'x-auth-token': 'admin', 'OpenStack-API-Version': 'placement latest', 'Connection': 'keep-alive', 'User-Agent': 'run.py keystoneauth1/3.4.0 python-requests/2.18.4 CPython/2.7.6', 'Content-Type': 'application/json', 'X-Openstack-Request-Id': 'req-35195a85-b2c0-43ff-9bd7-1ddbf047c34d'} | 14:51 |
| cdent | if you don't set the accept header at all, then it now defaults to a json response | 14:52 |
| cdent | but it used to be that webob would default to html | 14:52 |
| efried | I'm getting HTML. Which means I can't pull the error code. Because it ain't there afaict. | 14:52 |
| cdent | efried: you got code you can point me at? | 14:52 |
| efried | cdent: I'm running nova.tests.functional.test_report_client.SchedulerReportClientTests.test__set_inventory_for_provider with a breakpoint in nova.scheduler.client.report.SchedulerReportClient._set_inventory_for_provider in the 409 condition. | 14:53 |
| cdent | efried: but from what I can discern there additional headers isn't getting integrated for some reason | 14:53 |
| efried | cdent: Second time it hits (cause I'm mucking with InventoryInUse code) | 14:54 |
| cdent | the fact that your response content-type is application/json is confusion. you're definitely getting html? | 14:54 |
| efried | The response content-type is text/html. The above headers are on the *request* (at least as reported by resp.request.headers) | 14:55 |
| efried | The request was a PUT with a JSON payload, so content-type application/json is expected and correct. | 14:55 |
| efried | Here are the response headers: | 14:55 |
| efried | (Pdb) resp.headers | 14:56 |
| efried | {'openstack-api-version': 'placement 1.25', 'Content-Length': '291', 'Content-Type': 'text/html; charset=UTF-8', 'x-openstack-request-id': 'req-9464f8b2-949b-47e2-b126-c1419f8822b6', 'vary': 'openstack-api-version'} | 14:56 |
| efried | (hm, side note, are those request IDs supposed to match?) | 14:56 |
| cdent | so: something is setting accept: '*/*' which webob and placement will interpret as "give me anything" | 14:57 |
| cdent | placement will only return a json formatted error response if the accept header is not set, or is set to application/json | 14:57 |
| cdent | so you need to figure out what is setting the accept header/why additional headers is not being seen | 14:57 |
| efried | cdent: Okay, I think I see it. Looking at the ksa code for adapter, additional_headers hits the entire 'headers' key with a setdefault(). But report client's put method sets | 15:01 |
| efried | 'headers': {request_id.INBOUND_HEADER: | 15:01 |
| efried | global_request_id} if global_request_id else {} | 15:01 |
| efried | which will cause that setdefault to no-op. | 15:01 |
| efried | but that doesn't actually explain where */* is getting set. | 15:01 |
| cdent | could be requests? | 15:02 |
| efried | oh, wait, I lied above. | 15:02 |
| efried | for k, v in self.additional_headers.items(): | 15:02 |
| efried | kwargs.setdefault('headers', {}).setdefault(k, v) | 15:02 |
| efried | so that *should* set the accept header if it's absent. | 15:02 |
| efried | which means it must be present already and set to */* before we get there. | 15:03 |
| openstackgerrit | Vladyslav Drok proposed openstack/nova-specs master: Allow having placement inventories with reserved value equal to total https://review.openstack.org/568613 | 15:08 |
| efried | cdent: report client's self._client.additional_headers is getting emptied somewhere along the way. | 15:11 |
| openstackgerrit | Merged openstack/nova master: trivial: Explain how the marker works for instance-cell mapping https://review.openstack.org/567597 | 15:12 |
| cdent | any ideas? | 15:13 |
| efried | cdent: Not yet. Deep debug is in order, I guess. I don't relish the thought of delving that far into the bowels of ksa/requests. | 15:13 |
| efried | cdent: I can work around it, but not in pretty ways. | 15:14 |
| cdent | a quick grep of ksa doesn't reveal anything obvious :( | 15:15 |
| efried | though I guess I'll have to, at least temporarily, if I find it's a bug in ksa | 15:15 |
| cdent | do you have any evidence that the problem is present outside of your test? | 15:15 |
| efried | cdent: Well, the fact that additional_headers is a public attr but also a kwarg to the Adapter init is kinda messy. Cause we're not setting it on the init. | 15:15 |
| efried | cdent: um, a reasonable question, because we're probably mocking some part of requests in here, eh? | 15:16 |
| efried | I don't know what an "intercept" is. | 15:16 |
| cdent | https://github.com/cdent/wsgi-intercept doesn't mock the request, just the socket that gets written to | 15:17 |
| cdent | (replaces the socket with a BytesIO (or equivalent) | 15:17 |
| cdent | since wsgi-intercept has been working well for at least 7 years, it's probably not that | 15:18 |
| cdent | but could very well be | 15:18 |
| cdent | I was just thinking that if this problem is present then we would have seen it more in "real" situations | 15:18 |
| cdent | and I was under the impression that we weren't but then I don't read the scheduler logs off | 15:19 |
| cdent | s/off/often/ | 15:19 |
| efried | cdent: Until now, we've never had a reason to resp.json() a non-2xx response. | 15:20 |
| efried | so it could have been broken forever and we wouldn't know. | 15:20 |
| cdent | that's surprising: do we not write error responses in the log when we get them? If not, why not? | 15:20 |
| cdent | writing them to the log is why they exist | 15:21 |
| efried | cdent: I'm sure we do - via resp.text | 15:21 |
| cdent | which should show up as json | 15:21 |
| cdent | but not formatted | 15:21 |
| cdent | it will still be a json string | 15:21 |
| efried | but why would we ever have noticed if it wasn't? | 15:22 |
| cdent | so by looking in the logs | 15:22 |
| cdent | thus my comment above about "I don't read scheduler logs often" | 15:22 |
| openstackgerrit | Vladyslav Drok proposed openstack/nova-specs master: Allow having placement inventories with reserved value equal to total https://review.openstack.org/568613 | 15:29 |
| efried | jaypipes, mriedem: ^ | 15:34 |
| cdent | efried: it's probably the case that placement side should see '*/*' as "default to json" for sake of clean. I'll look into that. | 15:36 |
| jaypipes | efried: are you directing us at vdrok's patch? | 15:37 |
| efried | cdent: Okay, that would be another workaround. Probably require a microversion, though, huh? | 15:37 |
| openstackgerrit | Vladyslav Drok proposed openstack/nova-specs master: Allow having placement inventories with reserved value equal to total https://review.openstack.org/568613 | 15:38 |
| efried | jaypipes: Yes. Spec for reserved=total. Simple one, will unblock his code, which is ready afaict. | 15:38 |
| cdent | efried: if it required a microversion it would be useless | 15:38 |
| cdent | but probably according to the rules it does, but the rules are often wrong | 15:38 |
| cdent | efried: see https://review.openstack.org/#/c/518223/ | 15:39 |
| cdent | which as we know understand things is potentially incomplete if cliens are defaulting to sending */* | 15:40 |
| jaypipes | efried: +2 from me. nice and short. | 15:43 |
| efried | jaypipes: thx | 15:43 |
| efried | cdent: Yeah, I remember that patch. I guess I need to nail down where tf we're setting */*. | 15:44 |
| openstackgerrit | Merged openstack/nova master: placement: Fix HTTP error generation https://review.openstack.org/568567 | 15:46 |
| cdent | efried: made https://bugs.launchpad.net/nova/+bug/1771384 for reference | 15:51 |
| openstack | Launchpad bug 1771384 in OpenStack Compute (nova) "placement api send text/html error responses when accept headers is '*/*'" [Undecided,New] | 15:51 |
| efried | cdent: okay. I guess I'll make one for "we set accept to */* somewhere, somehow" | 15:51 |
| cdent | efried: it's requests: default_headers() in requests.utils | 15:54 |
| cdent | it's a pretty reasonable default | 15:54 |
| efried | cdent: Which would get set if not otherwise set. | 15:54 |
| cdent | so the funkery, presumably, is that additional_headers has gone wild? | 15:55 |
| efried | cdent: Does that mean that the aforementioned patch (to set application/json if unspecified) will never be hit, if the client is using requests? | 15:55 |
| efried | ...because accept will never be unspecified | 15:55 |
| efried | because requests defaults it to */* | 15:55 |
| cdent | right, which is why I made that bug and am fixing it | 15:55 |
| efried | okay. I'm still trying to figure out how additional_headers gets blanked. | 15:55 |
| efried | cdent: ahshit. It's the NoAuthReportClient in the test setup. | 15:57 |
| cdent | wuh? | 15:57 |
| efried | cdent: We're not actually going through SchedulerReportClient._create_client | 15:58 |
| efried | hence not setting the additional_headers. | 15:58 |
| efried | in the test case. | 15:58 |
| efried | which probably means this works fine IRL. | 15:58 |
| cdent | ah. okay. | 15:58 |
| cdent | heh, I read right past that when looking before :( | 15:59 |
| cdent | the fix I'm working on is still worth doing because there will be other clients, always | 15:59 |
| efried | ight | 16:00 |
| openstackgerrit | Chris Dent proposed openstack/nova master: [placement] default to accept of application/json when */* https://review.openstack.org/568630 | 16:01 |
| efried | +2 | 16:14 |
| cdent | thanks | 16:15 |
| cdent | edleafe: for the record, in case it wasn't clear, efried's the catcher on that accept fix. I'm just the labor. | 16:21 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Use placement.inventory.inuse in report client https://review.openstack.org/568639 | 16:30 |
| efried | cdent, jaypipes: ^ | 16:30 |
| cdent | exciting | 16:30 |
| *** e0ne has quit IRC | 16:35 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Don't reschedule on RequestedVRamTooHigh errors https://review.openstack.org/568642 | 16:55 |
| edleafe | cdent: yeah, I was reading the discussion | 17:02 |
| mriedem | why isn't "POST /resource_providers/UUID/inventories" in the API reference again? | 17:05 |
| cdent | mriedem: if I remember right it was because it was decided it was wrong | 17:07 |
| cdent | in the sense that if you want to set just one class of inventory, then PUT to ../inventories/{class} | 17:07 |
| cdent | mriedem: I can probably dredge up some of the discussion if you're interested | 17:08 |
| mriedem | ok https://review.openstack.org/#/c/568613/ was calling it out so i was confused at first because it's not in the API reference | 17:08 |
| mriedem | i know jichen had a patch to add it to the api reference and it was nacked | 17:09 |
| mriedem | as a dirty secret | 17:09 |
| cdent | Yeah, I havent got the reasons for the 'dirty' in my current memory | 17:09 |
| mriedem | https://review.openstack.org/#/c/512215/ | 17:10 |
| openstackgerrit | Merged openstack/nova stable/queens: Added ability to configure default architecture for ImagePropertiesFilter https://review.openstack.org/568575 | 17:10 |
| openstackgerrit | Merged openstack/nova master: Update auth_url in install docs https://review.openstack.org/568002 | 17:11 |
| mriedem | maybe we should use this opportunity for vdrok's microversion to remove the POST inventories endpoint | 17:11 |
| cdent | mriedem: that would require, I suppose, figuring out why it is dirty (I suspect I didn't think it was) | 17:12 |
| cdent | (since I made it) | 17:13 |
| mriedem | the comment from jay in that review explains why | 17:13 |
| cdent | mriedem: hmm, yeah, I never really agreed with that. It was pretty handy back when I was scripting creating shared disk resource providers (something that one of the very early rp specs said was a thing we should do: sample cron jobs of such things) | 17:16 |
| openstackgerrit | Merged openstack/nova master: Add INVENTORY_INUSE to DELETE /rp/{u}/inventories https://review.openstack.org/568578 | 17:16 |
| cdent | It's not actively dangerous | 17:16 |
| cdent | the empty response POSTs for creating stuff are really nice for scripting, less nice for things like ProviderTree | 17:18 |
| efried | and we can't remove it | 17:30 |
| edleafe | we can't? | 17:31 |
| efried | How could we? Let's say we remove it in 1.26. Then we would have to eventually bump the minimum version past 1.26 in order to remove it. | 17:35 |
| efried | and bumping the minimum is something we've said we'll never do. | 17:35 |
| edleafe | never say never | 17:36 |
| edleafe | 410 to the rescue! | 17:37 |
| efried | In any case, we have at least one consumer who claims it's useful, so I don't see why we should remove it. | 17:38 |
| cdent | If we bump the min, that should be a major, and if we're going to a major, we may as well rewrite the api at the same time, using what we learned, and if we're going to do that we should wait for a few years to actually learn something | 17:38 |
| efried | cdent: I responded on https://review.openstack.org/#/c/568639/ -- I'm gonna rebase it now, will look for your feedback. | 17:39 |
| *** e0ne has joined #openstack-placement | 17:39 | |
| cdent | roger | 17:39 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Use placement.inventory.inuse in report client https://review.openstack.org/568639 | 17:41 |
| efried | rebase only ^ | 17:42 |
| cdent | efried: I wrote a noval in response | 17:56 |
| cdent | heh that's a great typo | 17:56 |
| cdent | no val | 17:57 |
| cdent | perhaps | 17:57 |
| cdent | I want one of these for running the nova tests: https://cloudplatform.googleblog.com/2018/05/Introducing-ultramem-Google-Compute-Engine-machine-types.html | 18:01 |
| mriedem | we could deprecate the api, i.e. cap it at a given microversion so everything past that gets a 404 back | 18:06 |
| cdent | yes, we could, but why bother? | 18:13 |
| jaypipes | mriedem: but we can never do that. because, well, we just can't apparently. | 18:15 |
| edleafe | because it would break cdent's fridge | 18:18 |
| * mriedem backs away slowly | 18:18 | |
| *** e0ne has quit IRC | 18:20 | |
| jaypipes | mriedem: you KNOW cdent has a fridge running placement, right? | 18:20 |
| jaypipes | :) | 18:20 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Use placement.inventory.inuse in report client https://review.openstack.org/568639 | 18:20 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Use placement.inventory.inuse in report client https://review.openstack.org/568639 | 18:20 |
| efried | cdent: Good discussion. Removed comment in errors.py. Left import in report client. | 18:21 |
| *** gjayavelu has joined #openstack-placement | 18:26 | |
| cdent | jaypipes: all these resource providers are yours except europa | 18:27 |
| cdent | or, fo ra different turn: I did all this, even the fridge, for you. | 18:27 |
| openstackgerrit | melanie witt proposed openstack/nova-specs master: Propose counting quota usage from placement and API database https://review.openstack.org/509042 | 18:28 |
| efried | jaypipes: Seems like you've got the best handle on why we want to set reserved=total rather than deleting inventory; maybe you could provide words for vdrok at https://review.openstack.org/#/c/568613/3/specs/rocky/approved/allow-reserved-equal-total-inventory.rst@19 ? | 18:28 |
| *** e0ne has joined #openstack-placement | 18:35 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add granular policy rules for resource providers inventories https://review.openstack.org/568666 | 18:37 |
| jaypipes | efried: tried my best. | 18:47 |
| efried | jaypipes: cool man | 18:47 |
| openstackgerrit | melanie witt proposed openstack/nova-specs master: Propose counting quota usage from placement and API database https://review.openstack.org/509042 | 18:49 |
| cdent | jaypipes: is the consumer gen stack stable and ready for re-review? | 18:52 |
| jaypipes | cdent: yes. | 18:54 |
| cdent | rad, will go back to that either a bit later, or before you wake up tomorrow | 18:55 |
| jaypipes | cool, thx | 18:57 |
| cdent | efried: except for queued_for_delete | 19:03 |
| efried | cdent: wha? | 19:04 |
| cdent | queued_for_delete is why consumer_count in /usages won't work | 19:04 |
| *** e0ne has quit IRC | 19:04 | |
| efried | cdent: What's queued_for_delete? | 19:04 |
| efried | as in, what does it do? | 19:05 |
| cdent | efried: it's from the referenced spec: https://blueprints.launchpad.net/nova/+spec/handling-down-cell | 19:05 |
| *** e0ne has joined #openstack-placement | 19:07 | |
| efried | cdent: Is the point that queued_for_delete instances should *not* be counted against quota? | 19:07 |
| cdent | actuall spec here: https://review.openstack.org/#/c/557369/ | 19:07 |
| cdent | efried: that's my understanding based on line 162 of https://review.openstack.org/#/c/509042/3/specs/rocky/approved/count-quota-usage-from-placement.rst | 19:08 |
| cdent | I'm guessing the deal there is that if you try to delete something, and it is currently unable to be deleted beause the cell is down, we should mark it as queued and you should be able to create a new instance somewhere else | 19:09 |
| efried | cdent: Roger that. Wonder if we should go ahead and delete the allocations from placement at that time instead of whenever the cell comes back up. <== melwitt | 19:10 |
| cdent | perhaps they are allowed to change their minds? | 19:10 |
| cdent | I suspect there's PTG context here that I'm now aware of (was probably gone from the nova room by that point) | 19:11 |
| efried | cdent: Are there other reasons we can't assume consumer == instance? | 19:11 |
| cdent | efried: btw, I'm not suggesting that more info isn't needed in the spec to address these questions, just trying to fill in some blanks | 19:11 |
| efried | appreciate that. | 19:12 |
| cdent | well, placement is generation tracker of resource providers and consumption of those resources | 19:12 |
| cdent | we have no idea what people might track in placement | 19:12 |
| cdent | efried: this is _exactly_ the thing I was trying to say in the novel about not fully buying the "why" of being an HTTP api | 19:12 |
| efried | I think it's reasonable for a particular client to make assumptions about who's using its instance of the placement service, though. | 19:13 |
| efried | i.e. nova can assume your fridge isn't going to use its instance of the service. | 19:13 |
| efried | and therefore *nova* assuming that an instance is a consumer is a reasonable thing -- unless that's not true in some scenario, which is what I'm trying to figure out. | 19:14 |
| efried | like is there a neutron thing where an allocation belongs to a port or some such weirdness? | 19:14 |
| cdent | that's a horrible assumption | 19:17 |
| cdent | that nova has an instance of placement is totally contrary to the idea of having a placement service | 19:17 |
| cdent | it is openstack's placement service | 19:17 |
| cdent | not nova's | 19:17 |
| cdent | otherwise how do neutron and nova and cinder conspire to have shared and nested providers | 19:18 |
| melwitt | efried: yeah, that I'm not 100% sure about for queued_for_delete, whether that's a time we should go ahead and delete the allocations | 19:18 |
| cdent | similarly some cloud may decide that they want to have whatever resource provider because they are aware of this useful thing called placement | 19:19 |
| cdent | the gist is: it's not nova's and we should not design in any way that assumes that nova is the only writer | 19:19 |
| efried | cdent: Okay, I can buy that. Thanks for the perspective. | 19:19 |
| cdent | even if it were that the case that nova is the only writer, to assume so would be bad design | 19:20 |
| cdent | efried: In many cases, I'm happy to accept that my assertions are merely perspective, but in this case I'm simply right and I will die on a hill for this one (and probably only this one) | 19:20 |
| efried | Which means we could get a list of consumer_uuids and intersect it with the list of instances from the nova db to get the set of consumer IDs that actually *are* instance UUIDs. | 19:21 |
| efried | ...which will just end up being the list of instances from the nova db. | 19:21 |
| efried | So there's no point. | 19:21 |
| cdent | right | 19:21 |
| melwitt | if we're going to use instance UUIDs where queued_for_delete=False as the instance count, then we would have to delete the allocations in placement upon queued_for_delete=True if instance count and CPU/RAM are going to be in sync | 19:25 |
| melwitt | otherwise we'd have instance count decreased once a delete is requested but the CPU and RAM that go with it would stay allocated. just thinking aloud | 19:25 |
| efried | melwitt: That was my thought. I've concluded (with cdent's help) that it doesn't buy you anything for this particular bp, but it may be useful for other stuff if you were to remove the allocations as soon as it's actually possible rather than having to wait until the cell comes back. | 19:27 |
| openstackgerrit | sunku ranganath proposed openstack/nova-specs master: Submitting blueprint describing usage of resource management daemon to control and use cache as a resource https://review.openstack.org/568678 | 19:27 |
| efried | melwitt: The existing deletion code would simply have to tolerate whatever "already deleted" looks like from placement - which it may already do. | 19:28 |
| melwitt | hm, yeah | 19:28 |
| cdent | it should already be tolerant | 19:28 |
| efried | it actually isn't, which is a tad annoying. | 19:30 |
| cdent | which part do you mean when you say "it"? | 19:30 |
| efried | The report client code. | 19:30 |
| efried | actually | 19:31 |
| efried | I take it back. | 19:31 |
| cdent | server too | 19:31 |
| efried | PUT /rp/{u}/inventories json={} will work even if there are no inventories there. | 19:31 |
| efried | s/work/return 2xx/ | 19:31 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Implement granular policy rules for placement https://review.openstack.org/524425 | 19:32 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add granular policy rules for /resource_classes* https://review.openstack.org/565578 | 19:32 |
| efried | So yeah, it should be transparent from that perspective. | 19:32 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add granular policy rules for resource providers inventories https://review.openstack.org/568666 | 19:32 |
| efried | melwitt: fyi | 19:32 |
| cdent | I reckon DELETE /allocations/{uuid} not being idempotent is a bug, but since that's not what's used in the report client, nbd | 19:32 |
| efried | that's *why* we removed it from report client. | 19:32 |
| cdent | not _that_ kind of idempotent | 19:32 |
| efried | Meaning DELETE when nothing there == DELETE when something there ? | 19:33 |
| cdent | I mean the resty kind of idempotent which is that if you delete once and it works, the second time you try to delete the same thing it should return the same response code | 19:33 |
| efried | I don't know that answer, but yeah, moot. | 19:33 |
| edleafe | I always understood DELETE as meaning "make sure that there aren't any of these things around" | 19:35 |
| edleafe | So DELETE twice should be idempotent | 19:35 |
| cdent | edleafe: you wanna make a bug? | 19:36 |
| efried | Looks like we all hope that would be the case, but not sure if there's any proof. | 19:36 |
| efried | I can find out pretty quick. | 19:36 |
| * edleafe reads back more | 19:36 | |
| cdent | the code is wrong | 19:36 |
| efried | At least this form is not idempotent: DELETE /resource_providers/$ENVIRON['RP_UUID']/inventories/IPV4_ADDRESS | 19:37 |
| efried | we have a gabbit that does ^ twice, second time 404s | 19:37 |
| melwitt | yeah, sometimes delete is idempotent and sometimes not. like if you try to delete an instance that has already been deleted, I think you'd get a 404 | 19:37 |
| efried | Ah, but this form is good: DELETE: /resource_providers/$ENVIRON['RP_UUID']/inventories | 19:37 |
| openstackgerrit | Zack Cornelius proposed openstack/nova-specs master: Libvirt file backed memory https://review.openstack.org/563704 | 19:38 |
| cdent | yay, inconsistency! | 19:38 |
| efried | See inventory.yaml L674 | 19:38 |
| efried | Well, I don't think that's wrong, tbh. | 19:38 |
| efried | The former you're addressing a URI that represents an actual object. If that URI represents something that doesn't exist, you should get a 404, nah? | 19:39 |
| efried | Urgh, I can see it both ways, really. | 19:39 |
| melwitt | same. there are situations where either makes sense. so generally I've seen the low-level API return something different for "not found" and the caller ignores/doesn't treat as error "not found" in order to get idempotence | 19:40 |
| melwitt | if it's an API that's going to be called multiple times potentially (example: volume detach) | 19:41 |
| melwitt | because there are a lot of snags along the way that can ultimately fail the detach. then the user tries again later and steps that succeeded last time should be idempotent so they can complete the entire detach flow | 19:41 |
| efried | melwitt: Well, to summarize this case, we should be good regardless because we're not using DELETE at all - we're using PUT {}, which is idempotent. | 19:43 |
| edleafe | efried: that was the point I was making. DELETE isn't an action; it's a request to set a state | 19:43 |
| melwitt | ah, I see | 19:43 |
| efried | edleafe: Yeah, which is confusing because it's a verb. | 19:43 |
| edleafe | they're all verbs :) | 19:44 |
| efried | edleafe: If it meant "set the state such that this thing is absent" it should really be called ABSENT (which isn't a verb). | 19:44 |
| efried | edleafe: I get it, I'm just being pedantic. DELETE only means "set state to absent" in HTTP. | 19:44 |
| efried | otherwise it means: delete. | 19:44 |
| edleafe | efried: you'll have to take that up the HTTP RFC folks | 19:44 |
| efried | my point is, you can see why not everyone automatically knows/assumes DELETE should be idempotent in this sense when consuming (or for that matter, creating) an API. | 19:45 |
| edleafe | Idempotence refers to the state of the resource *after* the call is completed, so you can have a 204 followed by a 404 and still be idempotent | 19:47 |
| * cdent returns from phone call | 19:56 | |
| cdent | that's a very good point edleafe and, of course, right. delete once and it is still deleted after that but it is now gone. That's how it should be. I'm clearly worn out after dying on a hill, and/or spread too thin. | 19:57 |
| edleafe | cdent: if you actually died on that hill, you wouldn't be worn out, but rather resting in peace :) | 19:58 |
| cdent | trying to die? | 19:58 |
| *** e0ne has quit IRC | 20:02 | |
| *** efried has quit IRC | 20:07 | |
| *** efried has joined #openstack-placement | 20:17 | |
| cdent | 'night all | 20:29 |
| *** cdent has quit IRC | 20:29 | |
| openstackgerrit | sunku ranganath proposed openstack/nova-specs master: Use resource management daemon to manage cache as a resource https://review.openstack.org/568678 | 20:31 |
| *** e0ne has joined #openstack-placement | 20:35 | |
| openstackgerrit | Merged openstack/nova stable/queens: libvirt: Report the virtual size of RAW disks https://review.openstack.org/568363 | 20:36 |
| *** e0ne has quit IRC | 20:40 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add retrying to requirements.txt https://review.openstack.org/568692 | 20:50 |
| openstackgerrit | sunku ranganath proposed openstack/nova-specs master: Use resource management daemon to manage cache as a resource https://review.openstack.org/568678 | 20:52 |
| openstackgerrit | Merged openstack/nova master: Remove mox in tests/unit/api/*/test_volumes.py https://review.openstack.org/564655 | 20:56 |
| openstackgerrit | melanie witt proposed openstack/nova-specs master: Add additional information for z/VM spec. https://review.openstack.org/562154 | 21:15 |
| openstackgerrit | sunku ranganath proposed openstack/nova-specs master: Use resource management daemon to manage cache as a resource https://review.openstack.org/568678 | 21:25 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Implement granular policy rules for placement https://review.openstack.org/524425 | 21:27 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add granular policy rules for /resource_classes* https://review.openstack.org/565578 | 21:27 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add granular policy rules for resource providers inventories https://review.openstack.org/568666 | 21:27 |
| *** avolkov has quit IRC | 21:28 | |
| *** efried has quit IRC | 21:35 | |
| openstackgerrit | sunku ranganath proposed openstack/nova-specs master: Use resource management daemon to manage cache as a resource https://review.openstack.org/568678 | 21:36 |
| *** edmondsw has quit IRC | 21:52 | |
| *** edmondsw has joined #openstack-placement | 21:53 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add granular policy rules for usages https://review.openstack.org/568706 | 21:54 |
| *** efried has joined #openstack-placement | 21:55 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add granular policy rules for usages https://review.openstack.org/568706 | 21:55 |
| *** mriedem is now known as mriedem_afk | 21:55 | |
| *** edmondsw has quit IRC | 21:57 | |
| openstackgerrit | sunku ranganath proposed openstack/nova-specs master: Use resource management daemon to manage cache as a resource https://review.openstack.org/568678 | 22:00 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Debug logs for allocation_candidates filters https://review.openstack.org/568712 | 22:30 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Use GET.get instead of GET.getall in alloc-cands https://review.openstack.org/568713 | 22:35 |
| openstackgerrit | Zack Cornelius proposed openstack/nova master: Implement file backed memory for instances in libvirt https://review.openstack.org/567876 | 22:57 |
| *** edmondsw has joined #openstack-placement | 23:29 | |
| *** edmondsw has quit IRC | 23:34 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!