| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-02 | |||
| 15:31:07 | sfinucan | diana_clarke: ack | |
| 15:31:09 | edleafe | jaypipes: we can do scheduler_client.report_client like we added to RT | |
| 15:32:10 | jaypipes | mriedem: jinx. | |
| 15:32:11 | edleafe | mriedem: that sounds clunky to me | |
| 15:32:11 | mriedem | we could build a new method into the RT to remove it if that's cleaner | |
| 15:32:11 | mriedem | so orphaned and we delete the CN record | |
| 15:32:11 | mriedem | and in this case, it's a not available node | |
| 15:32:11 | mriedem | for an available node | |
| 15:32:11 | mriedem | because we only call rt.update_available_resource(context, nodename) | |
| 15:32:11 | mriedem | it's a chicken/egg, | |
| 15:32:11 | mriedem | jaypipes: well, | |
| 15:32:12 | edleafe | there's nothing magic about the client | |
| 15:32:21 | mriedem | either way, i'm totally fine with a quick rt.report_client.delete_resource_provider from the compute manager in ocata if it gets the job done and saves time | |
| 15:32:33 | jaypipes | edleafe, mriedem: what I'm saying is that all the code in the compute manager's update_available_resource() method should be moved to the RT now. There's no reason to have that in the compute manager. | |
| 15:32:34 | mriedem | and a TODO to move that into RT later | |
| 15:32:46 | mriedem | jaypipes: *now* is not the day to do it | |
| 15:32:49 | jaypipes | mriedem: sure, I'm also fine with a quick fix. | |
| 15:32:52 | jaypipes | mriedem: understood. | |
| 15:32:59 | edleafe | mriedem: gimme a few to get something done | |
| 15:33:07 | jaypipes | edleafe: cheers | |
| 15:33:08 | edleafe | all this IRC chat is slowing me down :) | |
| 15:33:43 | jaypipes | mriedem: in that future cleanup, I will also change the InstanceList.get_all_by_host_and_nodename() to be an InstanceList.get_all_by_host() and fix the performance issues with the Ironic RT once and for all. | |
| 15:33:59 | mriedem | yeah i guess compute manager has self.scheduler_client = scheduler_client.SchedulerClient() | |
| 15:34:04 | jaypipes | mriedem: that was the end goal of moving all the compute_node -> compute_nodes handling in the RT. | |
| 15:34:25 | mriedem | jaypipes: ok | |
| 15:34:53 | jaypipes | mriedem: in the meantime, lemme look at the latest patch from edleafe on the Ironic inventory management. | |
| 15:35:40 | mriedem | the harder part of this change is probably going to be the method in the scheduler to remove the RP b/c we'll also have to make sure it doesn't have allocations and inventory in order to delete the RP, else it will 409 | |
| 15:36:34 | jaypipes | mriedem: yup | |
| 15:36:44 | mriedem | diana_clarke: wow nice catch L( | |
| 15:36:45 | mriedem | :) | |
| 15:36:52 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix typo in cells v2 ocata reno https://review.openstack.org/428198 | |
| 15:36:53 | edleafe | mriedem: add 'force=True'? | |
| 15:37:02 | mriedem | edleafe: is that a thing in the REST API? | |
| 15:37:17 | jaypipes | edleafe: I assigned that bug to you, FYI | |
| 15:37:26 | jaypipes | mriedem: no. | |
| 15:37:26 | edleafe | mriedem: nope - I was joking | |
| 15:37:34 | edleafe | forgot the smiley | |
| 15:37:48 | jaypipes | edleafe: nah, it's easy enough to write code that looks at existence of allocations and deletes them, then inventory, then resource provider. | |
| 15:37:58 | jaypipes | heh | |
| 15:38:06 | mriedem | we already have methods to delete the allocations and inventory, so probably can just re-use those | |
| 15:38:19 | edleafe | jaypipes: it's the same code; doing it in the client just makes more HTTP calls | |
| 15:38:28 | edleafe | mriedem: yep, that's the plan | |
| 15:38:38 | jaypipes | mriedem: indeed. in fact, https://review.openstack.org/#/c/404472 has examples of using those exact calls :) | |
| 15:39:18 | mriedem | i'm going to need to see johnthetubaguy be happy with that before i can look at it probably | |
| 15:39:33 | jaypipes | edleafe: understood. I'd prefer to keep a single HTTP call to a single thing unless there's a strong argument not to. | |
| 15:39:37 | mriedem | jaypipes: we also have 2 bugs for disk inventory/allocation in the report client | |
| 15:39:49 | jaypipes | s/to a single thing/to do a single thing/ | |
| 15:39:55 | jaypipes | mriedem: links pls? | |
| 15:40:02 | mriedem | https://review.openstack.org/#/c/428120/ | |
| 15:40:10 | edleafe | jaypipes: there is no HTTP method that will remove a RP and all of its associated invetory and allocations | |
| 15:40:10 | openstack | Launchpad bug 1659266 in OpenStack Compute (nova) "Disk allocation for instance is not good with swap" [High,Confirmed] - Assigned to Tanvir Talukder (tanvirt16) | |
| 15:40:10 | mriedem | https://bugs.launchpad.net/nova/+bug/1659266 | |
| 15:40:28 | edleafe | jaypipes: so adding one will mean a new microversion :( | |
| 15:40:29 | jaypipes | edleafe: I know, and I think that's appropriate. | |
| 15:41:05 | edleafe | I would certainly prefer to add it that way, but with RC1... | |
| 15:41:16 | jaypipes | edleafe: meaning DELETE /resource-providers/<UUID> should return a 409 if there are existing allocations and inventories. The user should be required to DELETE /allocations|inventories in order to perform the DELETE /r-p | |
| 15:41:35 | edleafe | jaypipes: of course | |
| 15:41:44 | jaypipes | edleafe: I don't care for implementing DELETE CASCADE in a single HTTP call. | |
| 15:42:00 | mriedem | let us rathole on that in ATL | |
| 15:42:04 | jaypipes | yup | |
| 15:42:07 | edleafe | jaypipes: well, that's why I said several HTTP calls | |
| 15:42:27 | edleafe | jaypipes: one to delete allocation, one to delete inventory, and the final to delete the RP | |
| 15:42:45 | jaypipes | edleafe: right, which is how things currently work. | |
| 15:43:09 | mriedem | think if we actually had hard constraints on the instances table and nova didn't auto delete all related things in code... | |
| 15:43:21 | mriedem | *auto soft-delete... | |
| 15:43:41 | dims_ | mriedem : dansmith : melwitt : should ServersNegativeTestJSON have been fixed? (with https://review.openstack.org/#/c/427782)? | |
| 15:43:53 | dansmith | dims_: yes, but we have another patch in the queue for it | |
| 15:43:57 | mriedem | dims_: yes, but https://review.openstack.org/#/c/427775/ | |
| 15:44:51 | mriedem | sfinucan: bauzas: want to send it on https://review.openstack.org/#/c/428198/ ? | |
| 15:44:55 | dims_ | mriedem : dansmith : melwitt : gotcha, saw it pop back up. will track 427775. thanks (http://logs.openstack.org/43/408143/9/gate/gate-tempest-dsvm-neutron-full-ubuntu-xenial/eb7a75d/logs/testr_results.html.gz) | |
| 15:45:47 | jroll | jaypipes: I'm putting this on a plaque "in the future I will fix the performance issues with the Ironic RT once and for all." | |
| 15:46:17 | dims_ | oh boy! | |
| 15:46:27 | jaypipes | jroll: heh. | |
| 15:49:02 | openstackgerrit | Pushkar Umaranikar proposed openstack/nova master: [WIP] Placement api: Add json_error_formatter to defaults https://review.openstack.org/395194 | |
| 15:54:47 | johnthetubaguy | mriedem: jaypipes: I can take a look at that in a few mins | |
| 15:55:54 | jaypipes | johnthetubaguy: actually, just reviewing you patch to fix the MB -> GB thing. running some tests on it locally, but overall the code looks fine (but also bauzas' identified issue with the allocation side also is a good find) | |
| 15:56:37 | openstackgerrit | Lance Bragstad proposed openstack/nova-specs master: Cleanup pike policy spec https://review.openstack.org/428222 | |
| 15:57:08 | bauzas | jaypipes: mriedem: FWIW I'm about to add a PTG bullet about discussing on modifying the disk resources so it's bytes | |
| 15:57:16 | lbragstad | johnthetubaguy ^ feel free to squash that into your original version (it's just cleanup to get your draft passing) | |
| 15:57:56 | johnthetubaguy | lbragstad: oh, cool, thanks | |
| 15:58:05 | jaypipes | bauzas: I support the general idea. but would be a pain in the ass to migrate all the MEMORY_MB and DISK_GB inventory and allocation records to bytes... | |
| 15:58:07 | johnthetubaguy | lbragstad: feel free to just push to my change if you want, thats cool | |
| 15:58:12 | johnthetubaguy | jaypipes: great, thank you | |
| 15:58:20 | lbragstad | johnthetubaguy yep! also - you have a todo in there for the keystone model | |
| 15:58:22 | jaypipes | bauzas: in addition to the fact that MEMORY_MB and DISK_GB are standard resource classes. | |
| 15:58:51 | lbragstad | johnthetubaguy I can take that action item - but where do you want that documented? I felt weird putting keystone things in a nova specific spec | |
| 15:59:16 | bauzas | jaypipes: exactly | |
| 15:59:29 | bauzas | jaypipes: that's why I think we should be discussing about it during the PTG | |
| 15:59:38 | bauzas | jaypipes: or during a hangout | |
| 15:59:51 | bauzas | because yes, changing that could be a PITA | |
| 15:59:56 | jaypipes | cool with me. | |
| 16:00:03 | jaypipes | (the discussions) :) | |
| 16:02:06 | edleafe | jaypipes: cdent just found a flaw in this approach | |
| 16:02:27 | edleafe | jaypipes: we can't delete allocations by RP or ID | |
| 16:02:39 | edleafe | jaypipes: only by consumer_id | |
| 16:02:55 | edleafe | jaypipes: and a consumer might have allocations across multiple RPs | |
| 16:03:36 | jaypipes | edleafe: yes, and? | |
| 16:04:01 | dansmith | so after an evacuate, | |
| 16:04:20 | dansmith | we wouldn't want to delete allocations for not-our-instance-anymore | |
| 16:04:58 | johnthetubaguy | lbragstad: ah, sorry, I missundertood, got you now | |