| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-22 | |||
| 16:07:06 | mriedem | let me make sure we don't have GET /os-services/{service_id} | |
| 16:07:24 | mriedem | yeah we don't | |
| 16:07:40 | mriedem | ok so change all 4 PUTs to POSTs and modify the URL to include the {service_id} | |
| 16:07:48 | mriedem | and drop host/binary from the POST body | |
| 16:07:54 | mriedem | since those are replaced by the service_id | |
| 16:08:28 | mriedem | i'll make those changes to the spec | |
| 16:08:35 | mriedem | thanks y'all | |
| 16:08:55 | cdent | I was under the impression we didn't want to make that breadth of change, but since we are willing, that sounds like the right way to go | |
| 16:09:23 | sdague | actually, I'm going to raise a bigger question | |
| 16:09:52 | sdague | why isn't this just a PUT/PATCH of /os-services/{service_id} | |
| 16:10:20 | sdague | every one of these urls is really an attribute in the response | |
| 16:10:22 | mriedem | with a body with the action to perform? | |
| 16:10:42 | mriedem | or PUT with 'status' in the req body? | |
| 16:10:44 | cdent | that's what I think is the long term right way, but I thought we didn't want that because people did want the attribute free way of doing stuff | |
| 16:10:48 | sdague | http://paste.openstack.org/show/603789/ | |
| 16:11:11 | sdague | mriedem: yeh, just like how we can update the description in a server | |
| 16:12:42 | mriedem | we could do that too | |
| 16:12:47 | cdent | the debate surrounding that kind of thing in the past has been that PUT is supposed to be a full representation, and PATCH is supposed to be a diff, not a partial representation. <- just reporting, not presenting that as my position | |
| 16:13:21 | cdent | and some people think a full rep PUT to change one thing is wasteful | |
| 16:13:24 | sdague | which would also let you do things like set {"status": "disabled", "disabled_reason": "...."} | |
| 16:13:25 | mriedem | all of these actions are actualy served by the update method in the controller anyway | |
| 16:13:26 | cdent | because there are only so many bytes in the universe | |
| 16:13:31 | mriedem | with the id being the action | |
| 16:13:36 | sdague | so, we already do PUT with partials, and I think that's fine | |
| 16:14:02 | sdague | I honestly am anti PATCH, because I think it is really for something more like what swift uses it for | |
| 16:14:07 | sdague | not for this stuff | |
| 16:14:23 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in console_auth_tokens.py https://review.openstack.org/448663 | |
| 16:14:26 | mriedem | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/services.py#L212 is what handles all of those PUTs | |
| 16:14:30 | mriedem | the id is the action | |
| 16:14:31 | sdague | mriedem: well, it would be PUT /os-services/{service_id} ... stuff you want to change | |
| 16:14:36 | mriedem | sdague: right | |
| 16:14:47 | mriedem | so we'd have another microversioned update method, | |
| 16:14:48 | sdague | just like severs today | |
| 16:14:53 | mriedem | where id becomes the service uuid | |
| 16:15:00 | mriedem | and the body contains the things you want to change | |
| 16:15:06 | mriedem | like status or forced-down | |
| 16:15:47 | sdague | yeh | |
| 16:16:11 | sdague | especially because those things are reflected in the representation when you get it | |
| 16:16:24 | mriedem | i'd kind of like to split that into a separate spec/microversion, but doing that gets weird when we also want to change the url | |
| 16:16:44 | sdague | yeh, I think it should all change at once, otherwise it's just painful for users | |
| 16:16:46 | mriedem | so probably can't split | |
| 16:16:48 | mriedem | right | |
| 16:17:02 | mriedem | since we don't test any of this in tempest, i worry | |
| 16:17:10 | mriedem | but anyway | |
| 16:17:51 | cdent | someday | |
| 16:18:26 | mriedem | it's just a lot of cases for the single PUT to handle, but we'll see | |
| 16:18:37 | mriedem | plus there is already another microversion in this flow already, 2.11 | |
| 16:18:38 | mriedem | for forced-down | |
| 16:18:39 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in cloudpipe.py https://review.openstack.org/448271 | |
| 16:18:40 | sdague | we don't test this stuff in tempest because it would break the world | |
| 16:18:44 | mriedem | sdague: i know | |
| 16:18:49 | mriedem | i know why, i'm just saying | |
| 16:18:50 | mriedem | you know | |
| 16:18:57 | sdague | you can't start randomly turning off the compute service in the middle of tests | |
| 16:19:03 | sdague | it's all testable on the functional side | |
| 16:19:07 | mriedem | yes i know | |
| 16:19:25 | mriedem | can't you just let me worry | |
| 16:22:32 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in certificates.py https://review.openstack.org/448267 | |
| 16:24:01 | mriedem | replied in https://review.openstack.org/#/c/447149/1/specs/pike/approved/service-hyper-uuid-in-api.rst@148 and left examples of what the single PUT would look like | |
| 16:24:11 | mriedem | i can detail how a single PUT replaces the existing 4 PUT actions | |
| 16:25:50 | openstackgerrit | Gábor Antal proposed openstack/nova master: Add json style checking for sample notifications https://review.openstack.org/443677 | |
| 16:30:11 | cfriesen | maybe an odd question....when we download a qcow2-formatted image we convert it to raw before storing it under "_base". Is there a way to leave it as-is? I'm pretty sure qemu understands using qcow2 images as backing files. | |
| 16:30:19 | cfriesen | Or does that cause a performance hit at runtime? | |
| 16:53:24 | johnthetubaguy | cfriesen: I think there is a spec about that, looking at similar things with ploop | |
| 16:56:30 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in deferred_delete.py https://review.openstack.org/448681 | |
| 16:56:48 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in create_backup.py https://review.openstack.org/448682 | |
| 16:57:00 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in consoles.py https://review.openstack.org/448683 | |
| 16:57:19 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in console_output.py https://review.openstack.org/448684 | |
| 17:06:56 | openstackgerrit | Thomas Stewart proposed openstack/nova master: Add lan9118 as valid nic for hw_vif_model property for qemu https://review.openstack.org/393489 | |
| 17:37:42 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Use uuids in services and os-hypervisors APIs (spec) https://review.openstack.org/447149 | |
| 17:37:54 | mriedem | johnthetubaguy: sdague: cdent: dansmith: alex_xu: ^ has the updates we talked about earlire | |
| 17:37:56 | mriedem | *earlier | |
| 17:39:19 | cdent | ✔ | |
| 18:07:07 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Reserve NUMA nodes with PCI devices attached https://review.openstack.org/364468 | |
| 18:08:53 | mriedem | johnthetubaguy: you can drop the -2 on https://review.openstack.org/#/c/379524/ now, the bp is approved | |
| 18:21:38 | mriedem | dansmith: i've got a pickle, i'm removing the aggregate.uuid migrate on load in the object https://review.openstack.org/#/c/448579/ | |
| 18:21:49 | mriedem | that's failing because there is a test for the online data migration which uses the object, | |
| 18:22:04 | mriedem | and because i've removed that code, the online data migration blows up since it can't use the object anymore | |
| 18:22:16 | dansmith | ...right | |
| 18:22:29 | dansmith | we can remove the migration now | |
| 18:22:31 | mriedem | i can't remove the online data migration thing from nova-manage can i? | |
| 18:22:33 | mriedem | oh | |
| 18:22:45 | mriedem | so i guess if you jump from what, mitaka to pike, well, sorry | |
| 18:22:46 | mriedem | derp | |
| 18:22:56 | dansmith | ...for this and many other reasons | |
| 18:23:18 | mriedem | ok i retract the pickle | |
| 18:23:32 | dansmith | you .. retract... the pickle .... | |
| 18:31:40 | johnthetubaguy | mriedem: ack | |
| 18:33:07 | johnthetubaguy | mdbooth: I should catch up with your -1 on this patch, I don't understand it: https://review.openstack.org/#/c/437597/11 | |
| 20:29:18 | oomichi | mriedem: about http://lists.openstack.org/pipermail/openstack-dev/2017-March/114487.html | |
| 20:30:08 | oomichi | mriedem: I am not sure the difference happens between os-hosts and os-services if a single ironic *compute* service manages multiple nodes | |
| 20:30:46 | oomichi | mriedem: I don't have such envs TBH, but it is just a concern | |
| 20:31:02 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove aggregate uuid generation on load from DB https://review.openstack.org/448579 | |
| 20:32:02 | oomichi | Can't we see multipe ironic nodes on os-hosts and single ironic compute service on os-services? | |
| 20:33:10 | mriedem | oomichi: looking at the index() methods in both os-services and os-hosts, they both use HostAPI.service_get_all | |
| 20:33:16 | mriedem | and both filter out API services from the response | |
| 20:33:39 | openstackgerrit | Sujitha proposed openstack/nova master: [WIP] Add BDM to InstancePayload https://review.openstack.org/448779 | |
| 20:34:10 | mriedem | oomichi: the nodes are actually shown in the os-hypervisors API | |
| 20:35:52 | oomichi | mriedem: oh, nice investigation. that should be the same. | |
| 20:35:58 | mriedem | as far as i can tell on listing, | |