Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-02-02
15:27:52 jaypipes mriedem: yes, and I had some strong words on that patch :)
15:27:59 diana_clarke sfinucan: I heard you like docs ;) https://review.openstack.org/#/c/411441/
15:28:11 diana_clarke sfinucan: (but only if you're not otherwise busy)
15:28:16 jaypipes edleafe: doink.
15:28:16 mriedem http://www.thereluctanttecchie.com/openstack-removing-a-compute-node-in-icehouse/
15:28:26 jaypipes edleafe: right on, brother, nice detective work.
15:28:39 jaypipes edleafe: we need to have the resource tracker do that, not the compute manager.
15:28:45 jaypipes edleafe: I can work on a patch.
15:29:02 edleafe jaypipes: already working on it
15:29:03 jaypipes edleafe: nice find.
15:29:06 mriedem " The fact that the crap hypervisor-list/hypervisor-show REST APIs are still manually attempting to conflate a ComputeNode with a Service is a problem that should be fixed entirely in the REST API, since it is the REST API that is borked."
15:29:10 jaypipes edleafe: oh? even better :)
15:29:11 mriedem those strong words? :)
15:29:22 jaypipes mriedem: heh, yes sir.
15:29:37 edleafe jaypipes: but it's the manager that has to make the call. I don't see the benefit in adding the RT as an indirection layer
15:29:55 jaypipes edleafe: because the RT has the placement client, not the manager.
15:30:09 jaypipes edleafe: and the RT has all the compute node objects now, not the manager.
15:30:11 mriedem the compute manager is the one looking for orphaned computes
15:30:14 mriedem not the RT
15:30:16 edleafe jaypipes: the manager has the scheduler client
15:30:38 mriedem unless you just move that into RT.update_available_resource
15:30:49 jaypipes mriedem: all of that update_available_resource() code in the manager should be in the RT, IMHO.
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 mriedem jaypipes: well,
15:32:11 mriedem it's a chicken/egg,
15:32:11 mriedem because we only call rt.update_available_resource(context, nodename)
15:32:11 mriedem for an available node
15:32:11 mriedem and in this case, it's a not available node
15:32:11 mriedem so orphaned and we delete the CN record
15:32:11 mriedem we could build a new method into the RT to remove it if that's cleaner
15:32:11 edleafe mriedem: that sounds clunky to me
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 edleafe mriedem: nope - I was joking
15:37:26 jaypipes mriedem: no.
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 mriedem https://bugs.launchpad.net/nova/+bug/1659266
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 edleafe jaypipes: there is no HTTP method that will remove a RP and all of its associated invetory and allocations
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

Earlier   Later