| *** alex_xu has quit IRC | 00:40 | |
| *** alex_xu has joined #openstack-placement | 00:41 | |
| *** tetsuro has joined #openstack-placement | 00:45 | |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Transform instance.rebuild_scheduled notification https://review.openstack.org/473929 | 00:57 |
|---|---|---|
| *** edmondsw has joined #openstack-placement | 01:04 | |
| *** edmondsw has quit IRC | 01:08 | |
| openstackgerrit | Merged openstack/nova stable/pike: Moving more utils to ProviderUsageBaseTestCase https://review.openstack.org/580490 | 01:13 |
| *** edmondsw has joined #openstack-placement | 02:52 | |
| *** edmondsw has quit IRC | 02:57 | |
| *** deepak_mourya has joined #openstack-placement | 03:50 | |
| *** edmondsw has joined #openstack-placement | 04:40 | |
| *** edmondsw has quit IRC | 04:44 | |
| *** tetsuro has quit IRC | 04:58 | |
| *** tetsuro has joined #openstack-placement | 05:16 | |
| *** tetsuro has quit IRC | 05:30 | |
| *** tetsuro_ has joined #openstack-placement | 05:30 | |
| *** tetsuro_ has quit IRC | 05:30 | |
| *** tetsuro has joined #openstack-placement | 05:30 | |
| *** edmondsw has joined #openstack-placement | 06:28 | |
| *** edmondsw has quit IRC | 06:33 | |
| *** tssurya has joined #openstack-placement | 06:53 | |
| *** peereb has joined #openstack-placement | 07:05 | |
| *** giblet is now known as gibi | 07:12 | |
| *** tetsuro has quit IRC | 07:15 | |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add InstanceGroupPolicy object https://review.openstack.org/573628 | 07:15 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Refactor policies to policy in InstanceGroup DB model https://review.openstack.org/579113 | 07:15 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375 | 07:15 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401 | 07:15 |
| *** tetsuro has joined #openstack-placement | 07:15 | |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: DNM: use policy create() https://review.openstack.org/580942 | 07:23 |
| *** ttsiouts has joined #openstack-placement | 07:23 | |
| *** ttsiouts has quit IRC | 07:40 | |
| *** ttsiouts has joined #openstack-placement | 07:46 | |
| openstackgerrit | Balazs Gibizer proposed openstack/nova stable/pike: Fix unbound local when saving an unchanged RequestSpec https://review.openstack.org/580951 | 07:54 |
| openstackgerrit | sahid proposed openstack/nova master: hardware: fix memory check usage for small/large pages https://review.openstack.org/532168 | 08:03 |
| openstackgerrit | sahid proposed openstack/nova master: hardware: fix hugepages memory usage per intances https://review.openstack.org/580657 | 08:06 |
| openstackgerrit | sahid proposed openstack/nova master: hardware: fix memory check usage for small/large pages https://review.openstack.org/532168 | 08:06 |
| *** edmondsw has joined #openstack-placement | 08:16 | |
| *** edmondsw has quit IRC | 08:21 | |
| *** tetsuro has quit IRC | 08:46 | |
| *** tetsuro has joined #openstack-placement | 08:49 | |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Change the ServerGroupAntiAffinityFilter to adapt to new policy https://review.openstack.org/571166 | 08:49 |
| openstackgerrit | Merged openstack/nova stable/pike: factor out compute service start in ServerMovingTest https://review.openstack.org/580473 | 09:22 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465 | 09:47 |
| *** finucannot is now known as stephenfin | 09:53 | |
| openstackgerrit | Chris Dent proposed openstack/nova master: Use nova.db.api directly https://review.openstack.org/543262 | 10:09 |
| *** cdent has joined #openstack-placement | 10:17 | |
| *** ttsiouts has quit IRC | 10:22 | |
| *** cdent has quit IRC | 10:41 | |
| *** tetsuro has quit IRC | 10:49 | |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Add NUMATopologyLimits.network_metadata https://review.openstack.org/575486 | 10:59 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Start accounting for networks in NUMA fitting https://review.openstack.org/564448 | 10:59 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Add RequestSpec.network_metadata https://review.openstack.org/564442 | 10:59 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: scheduler: Start utilizing RequestSpec.network_metadata https://review.openstack.org/564452 | 10:59 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Add '[neutron] physnets' and related options https://review.openstack.org/564440 | 10:59 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Start populating NUMACell.network_metadata field https://review.openstack.org/564441 | 10:59 |
| *** ttsiouts has joined #openstack-placement | 11:02 | |
| *** ttsiouts has quit IRC | 11:35 | |
| *** ttsiouts has joined #openstack-placement | 11:54 | |
| openstackgerrit | karim proposed openstack/nova master: Handle rebuild of instances with image traits https://review.openstack.org/569498 | 12:05 |
| *** jaypipes has joined #openstack-placement | 12:06 | |
| *** cdent has joined #openstack-placement | 12:14 | |
| openstackgerrit | Merged openstack/nova master: Merge server create for user data extension https://review.openstack.org/578709 | 12:18 |
| openstackgerrit | Merged openstack/nova master: Merge server create for security group extension https://review.openstack.org/578714 | 12:18 |
| openstackgerrit | Surya Seetharaman proposed openstack/nova-specs master: Handling a down cell https://review.openstack.org/557369 | 12:20 |
| *** mriedem has joined #openstack-placement | 12:23 | |
| *** edmondsw has joined #openstack-placement | 12:24 | |
| *** edmondsw has quit IRC | 12:29 | |
| openstackgerrit | Merged openstack/nova stable/pike: Add functional test for deleting a compute service https://review.openstack.org/580491 | 12:29 |
| *** edmondsw has joined #openstack-placement | 12:31 | |
| *** edmondsw has quit IRC | 12:35 | |
| *** edmondsw has joined #openstack-placement | 12:37 | |
| *** edmondsw has quit IRC | 12:42 | |
| *** edmondsw has joined #openstack-placement | 12:45 | |
| *** efried_off is now known as efried | 12:46 | |
| *** efried has quit IRC | 12:46 | |
| *** edmondsw has quit IRC | 12:49 | |
| *** edmondsw has joined #openstack-placement | 12:51 | |
| openstackgerrit | huanhongda proposed openstack/nova stable/queens: [Stable Only] Remove soft-deleted instances from quota_usages https://review.openstack.org/579093 | 12:52 |
| *** edmondsw has quit IRC | 12:53 | |
| *** edmondsw has joined #openstack-placement | 12:53 | |
| *** efried has joined #openstack-placement | 13:02 | |
| openstackgerrit | Matthew Booth proposed openstack/nova master: Avoid redundant initialize_connection on source post live migration https://review.openstack.org/551302 | 13:07 |
| openstackgerrit | Merged openstack/nova-specs master: Fix Non-unique network names in Servers IPs API response https://review.openstack.org/558125 | 13:14 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: scheduler: Start utilizing RequestSpec.network_metadata https://review.openstack.org/564452 | 13:15 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Add '[neutron] physnets' and related options https://review.openstack.org/564440 | 13:15 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Start populating NUMACell.network_metadata field https://review.openstack.org/564441 | 13:15 |
| *** ttsiouts has quit IRC | 13:16 | |
| *** ttsiouts has joined #openstack-placement | 13:19 | |
| *** tetsuro has joined #openstack-placement | 13:34 | |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add InstanceGroupPolicy object https://review.openstack.org/573628 | 13:58 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Refactor policies to policy in InstanceGroup DB model https://review.openstack.org/579113 | 13:58 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375 | 13:58 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401 | 13:58 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Change the ServerGroupAntiAffinityFilter to adapt to new policy https://review.openstack.org/571166 | 13:58 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465 | 13:58 |
| efried | cdent: n-sch mtg? | 14:01 |
| cdent | sorry lost track of time | 14:01 |
| efried | gibi: Didn't you propose a patch to start on functional testing of nrp scenarios? I can't find it. | 14:16 |
| gibi | efried: https://review.openstack.org/#/c/527728/ | 14:17 |
| gibi | efried: I took it over | 14:18 |
| efried | aha, that's why I couldn't find it, thanks. | 14:18 |
| gibi | efried: but it is pretty preliminary, only shows the NoValidHost today for nested alloca candidates query | 14:18 |
| gibi | efried: and if you look at the log the you can manually verify that the alloc candidates query string is OK | 14:19 |
| efried | gibi: I just wanted something to point to in response to mriedem's comment on PS17 here: https://review.openstack.org/#/c/515811/ | 14:19 |
| gibi | efried: mriedem asks for granular test. what I did is nrp test with a single group | 14:21 |
| gibi | efried: but can do granular add granular test as well | 14:21 |
| efried | gibi: Right, I kinda wanted to show that we're at least starting down that path with this (series of?) patch(es). | 14:22 |
| gibi | efried: yeah, this patch can be a start of a series including granular | 14:23 |
| efried | ++ | 14:24 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: make incomplete_consumer_project_id a valid UUID https://review.openstack.org/580358 | 14:24 |
| openstackgerrit | Merged openstack/nova stable/pike: api-ref: add a note in DELETE /os-services about deleting computes https://review.openstack.org/580494 | 14:30 |
| *** ttsiouts has quit IRC | 14:42 | |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: api-ref: Example verification for servers.inc https://review.openstack.org/529520 | 14:47 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add InstanceGroupPolicy object https://review.openstack.org/573628 | 14:57 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Refactor policies to policy in InstanceGroup DB model https://review.openstack.org/579113 | 14:57 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375 | 14:57 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401 | 14:57 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Change the ServerGroupAntiAffinityFilter to adapt to new policy https://review.openstack.org/571166 | 14:57 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465 | 14:57 |
| *** takashin has left #openstack-placement | 15:00 | |
| openstackgerrit | Mike Lowe proposed openstack/nova master: Option needed for ceph rbd erasure coding support https://review.openstack.org/581055 | 15:01 |
| *** peereb has quit IRC | 15:03 | |
| mriedem | efried: melwitt: i've posted a change to update the title, description and chair for the scheduler/placement meeting https://review.openstack.org/581057 | 15:07 |
| mriedem | since anyone looking for a placement meeting on http://eavesdrop.openstack.org/ likely wouldn't have found it otherwise | 15:07 |
| *** ttsiouts has joined #openstack-placement | 15:07 | |
| efried | +1 | 15:07 |
| *** tetsuro has quit IRC | 15:08 | |
| *** Jack1 has joined #openstack-placement | 15:09 | |
| openstackgerrit | Chris Dent proposed openstack/nova master: Use nova.db.api directly https://review.openstack.org/543262 | 15:13 |
| *** cdent has quit IRC | 15:13 | |
| Jack1 | Excuse me,if any one can tell me the PCI_DEVICE class 's roadmap in placement api? | 15:14 |
| jaypipes | Jack1: what are you attempting to do? could you tell us what your use case is please? | 15:16 |
| Jack1 | I just want to use it to know the gpu's resource amount and amount of use | 15:20 |
| Jack1 | and how to scheduler to the pci device use placement | 15:20 |
| openstackgerrit | Mathieu Gagné proposed openstack/nova master: Add support for multiple fixed-ips in metadata https://review.openstack.org/580742 | 15:21 |
| *** Jack1 has quit IRC | 15:24 | |
| mriedem | if Jack1 shows up again, point them at https://specs.openstack.org/openstack/nova-specs/specs/queens/implemented/add-support-for-vgpu.html | 15:26 |
| mriedem | oh but maybe they aren't looking for vgpus | 15:26 |
| mriedem | nvm | 15:26 |
| *** Jack1 has joined #openstack-placement | 15:45 | |
| efried | Hi Jack1. | 15:45 |
| efried | Jack1: When you say "know the gpu's resource amount and amount of use" are you referring to *physical* GPUs (in which case the resource amount is 1, and the usage is either "used or not") or *virtual* GPUs? | 15:46 |
| *** Jack1 has quit IRC | 15:47 | |
| *** Jacki has joined #openstack-placement | 15:50 | |
| Jacki | the usage is used,not the vgpu | 15:52 |
| Jacki | I am Jack1 | 15:53 |
| *** Jacki has quit IRC | 16:01 | |
| jaypipes | guh, I was just trying to respond to Jacki... | 16:01 |
| *** ttsiouts has quit IRC | 16:03 | |
| mriedem | fooled me twice....won't get fooled again hehe | 16:04 |
| mriedem | https://www.youtube.com/watch?v=eKgPY1adc0A | 16:04 |
| efried | I always have trouble remembering which way that expression goes too. | 16:09 |
| openstackgerrit | Merged openstack/nova stable/pike: Block deleting compute services which are hosting instances https://review.openstack.org/580496 | 16:11 |
| efried | jaypipes: How would you feel about including the provider name in ProviderTree? | 16:19 |
| jaypipes | efried: what do you mean? there is already the ability to query by name or uuid. | 16:20 |
| efried | jaypipes: Use case is this: as drivers are processing it in update_provider_tree, they need to be able to look for providers to purge - i.e. entries I "own" that should no longer be there. If the thing represented by the RP had a natural UUID that I used when I created the provider, it's cool; but if not, then I need to query placement to discover the names. | 16:20 |
| efried | That's an extra call that I would like to avoid. Also it allows us to preserve the general discouraging of querying placement from the driver. (We've said read-only querys are aight, but still...) | 16:21 |
| jaypipes | efried: I still don't understand... if the virt driver already has a name for the device/resource provider, it should look it up in the provider tree by its name, not uuid. | 16:24 |
| jaypipes | mriedem: running final tests on "delete no-alloc consumers" bug now... should be pushed shortly. | 16:24 |
| jaypipes | mriedem: working on the "update project/user for consumer" after that. | 16:25 |
| efried | jaypipes: Sorry, I see the name is already in there. Why did I think it wasn't? Ignore me. | 16:25 |
| jaypipes | efried: no worries | 16:25 |
| openstackgerrit | Margarita Shakhova proposed openstack/nova master: Do not skip migrations in _destroy_evacuated_instances() https://review.openstack.org/563623 | 16:27 |
| openstackgerrit | Merged openstack/nova master: move lookup of provider from _new_allocations() https://review.openstack.org/579920 | 16:31 |
| mriedem | ack, i'm working on a functional test for https://review.openstack.org/#/c/574488/ | 16:35 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Add RequestSpec.network_metadata https://review.openstack.org/564442 | 16:43 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: scheduler: Start utilizing RequestSpec.network_metadata https://review.openstack.org/564452 | 16:43 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Add '[neutron] physnets' and related options https://review.openstack.org/564440 | 16:43 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Start populating NUMACell.network_metadata field https://review.openstack.org/564441 | 16:43 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: delete consumers which no longer have allocations https://review.openstack.org/581086 | 16:44 |
| jaypipes | efried, mriedem, dansmith: ^ | 16:44 |
| * jaypipes moves on to fixing the auto-created delete consumer patch... | 16:45 | |
| mriedem | jaypipes: i'd consider that lower priority than being able to actually update consumer project/user information | 16:47 |
| mriedem | which is the thing that's blocking me | 16:47 |
| mriedem | this guy https://bugs.launchpad.net/nova/+bug/1779717 | 16:47 |
| openstack | Launchpad bug 1779717 in OpenStack Compute (nova) "No ability to update consumer's project and/or user external ID" [High,In progress] - Assigned to Jay Pipes (jaypipes) | 16:47 |
| mriedem | oh nvm, i see | 16:48 |
| mriedem | you used the wrong bu | 16:48 |
| mriedem | *bug | 16:48 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: delete consumers which no longer have allocations https://review.openstack.org/581086 | 16:51 |
| jaypipes | mriedem: sorry, fixed bug number. | 16:51 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: placement: delete auto-created consumers on fail https://review.openstack.org/579921 | 16:59 |
| jaypipes | dansmith: added the try/except around the POST /allocations with multiple consumers code. | 17:00 |
| jaypipes | ^^ | 17:00 |
| dansmith | got it | 17:02 |
| *** cdent has joined #openstack-placement | 17:16 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Heal allocations with incomplete consumer information https://review.openstack.org/574488 | 17:23 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Refactor _heal_instances_in_cell https://review.openstack.org/577896 | 17:23 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Use consumer generation in _heal_allocations_for_instance https://review.openstack.org/577905 | 17:23 |
| *** tssurya has quit IRC | 17:41 | |
| cdent | jaypipes: you cool with me fixing the failures on https://review.openstack.org/#/c/580358 ? | 18:00 |
| jaypipes | cdent: yes, absolutely. tia | 18:01 |
| cdent | roger | 18:01 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: delete consumers which no longer have allocations https://review.openstack.org/581086 | 18:08 |
| jaypipes | cdent, mriedem, dansmith: ^^ fixed the issue cdent pointed out (thanks cdent!) | 18:08 |
| jaypipes | (I wasn't using .fetchall() to get the dang query results... | 18:09 |
| jaypipes | cdent, mriedem, dansmith: sorry, hold up.. I forgot to handle the AllocationList.delete_all() call site. | 18:11 |
| mriedem | build the dang wall | 18:13 |
| mriedem | get the dang query results | 18:13 |
| openstackgerrit | Chris Dent proposed openstack/nova master: make incomplete_consumer_project_id a valid UUID https://review.openstack.org/580358 | 18:16 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: delete consumers which no longer have allocations https://review.openstack.org/581086 | 18:19 |
| *** mriedem1 has joined #openstack-placement | 18:19 | |
| jaypipes | cdent, mriedem, dansmith: ok, now ready to go. ^^ | 18:19 |
| *** mriedem has quit IRC | 18:21 | |
| *** mriedem1 is now known as mriedem | 18:22 | |
| mriedem | jaypipes: comment inline in https://review.openstack.org/#/c/581086/ about obvious races, but not sure i care that much atm to do anything about that | 18:38 |
| jaypipes | mriedem: is there a self.assertNotRaises() ... ? | 18:44 |
| mriedem | jaypipes: nope | 18:45 |
| mriedem | you don't have to change that | 18:46 |
| mriedem | if you did, i'd just do the get_by_uuid and add a comment that it'll raise if it's not found | 18:46 |
| cdent | jaypipes: can I put you down as someone willing and able to participate in discussing (even if vaguely) cinder+placement at the PTG? | 18:59 |
| efried | cdent: ō/ | 19:02 |
| cdent | efried: is that a "me too, please"? | 19:02 |
| efried | yup | 19:03 |
| cdent | done | 19:03 |
| jaypipes | cdent: fo sho. | 19:03 |
| efried | I prolly won't, like, *lead* the discussion or anything, but I'd like to be in the room. | 19:03 |
| cdent | and done | 19:03 |
| openstackgerrit | Dan Smith proposed openstack/nova master: Add rules column to instance_group_policy table. https://review.openstack.org/560832 | 19:09 |
| openstackgerrit | Dan Smith proposed openstack/nova master: Refactor policies to policy in InstanceGroup DB model https://review.openstack.org/579113 | 19:09 |
| openstackgerrit | Dan Smith proposed openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375 | 19:09 |
| openstackgerrit | Merged openstack/nova stable/pike: Delete allocations from API if nova-compute is down https://review.openstack.org/580498 | 19:31 |
| *** tssurya has joined #openstack-placement | 19:31 | |
| mriedem | jaypipes: efried: tbc, i don't have anything holding me back from +2 on https://review.openstack.org/#/c/581086/ - and since efried looked at it a bit and it looks like there are no alarms going off, i'm ok to +2 now and we can deal with transactional issues later if we hit them | 19:32 |
| efried | I'm taking a closer look, fwiw. | 19:33 |
| efried | jaypipes: Couple of questions in there. | 19:35 |
| efried | Is there some reason we shouldn't just delete all allocation-less consumers when we're done with any allocation-y transaction? Is it appreciably less efficient to say | 19:39 |
| efried | DELETE FROM consumers WHERE uuid IN (SELECT consumers.uuid, allocations.id FROM consumers LEFT JOIN allocations ON consumers.uuid == allocations.consumer_id WHERE allocations.id IS NULL) | 19:39 |
| efried | kind of thing? | 19:39 |
| efried | uh, I think I got my LEFT JOIN backwards, and you'd have to split that up, but you get the idea. | 19:39 |
| jaypipes | efried: no worries, I got the gist of what you're getting at. | 19:40 |
| jaypipes | efried: in general, I don't care for "cleanup orphan stuff even if that orphan stuff isn't related to the operation you are currently performing" strategies. If we do that, I'd prefer it be an online data migration that just does it once and for all... | 19:41 |
| jaypipes | efried: that said, if you and mriedem want to go that route, it's not a terribly hard change to make. | 19:42 |
| jaypipes | just let me know. | 19:42 |
| efried | jaypipes: The "set root_provider_uuid to self.uuid if not set" seems to be roughly in the same spirit. I get that it's not affecting records other than the one we're looking at, but still... | 19:42 |
| efried | mriedem: What do you think? | 19:42 |
| mriedem | i tend to agree with jaypipes in the targeted deleted | 19:44 |
| mriedem | *delete | 19:44 |
| openstackgerrit | Merged openstack/nova stable/pike: Cleanup RP and HM records while deleting a compute service. https://review.openstack.org/580499 | 19:51 |
| efried | okay. So jaypipes what about the thing where we could delete multiple consumers in one shot? | 19:56 |
| efried | Basically same as above, but with a WHERE consumer_uuid IN ($consumer_uuids) | 19:56 |
| jaypipes | efried: I can do that if you'd like, sure. | 19:56 |
| efried | jaypipes: I _would_ like that eavesdrop link in the commit message, so if you're not opposed to respinning... | 19:57 |
| jaypipes | efried: sure. gimme a few. need to git stash and save work and will do that shortly. | 19:59 |
| efried | cool, thx | 20:00 |
| jaypipes | np | 20:00 |
| efried | cdent: https://review.openstack.org/#/c/580373/ <= I'm about to +A this, adding my vote that UUID-ness is clearly what we intended (based on the API ref). But wanted to check whether you feel strongly enough to hold it up for further discussion. | 20:05 |
| openstackgerrit | Merged openstack/nova-specs master: Handling a down cell https://review.openstack.org/557369 | 20:10 |
| cdent | efried: I'm not sure what to say about that. My early memories are of it not always being a uuid and I feel rather strongly that requiring it be a uuid is significantly limiting for the future and unfriendly (that is, imposing a cost on some types of clients) and unflexible on a variety of fronts. That's a loss I will find unfortunate. On the other hand I don't want to hold things up so I'm unable to fight about i | 20:11 |
| cdent | the weight of the pre-existing code is strong | 20:11 |
| efried | cdent: If it wasn't a UUID, would it still be acceptable for it to be limited to 36c? | 20:13 |
| cdent | that's where the "weight of the pre-existing code" comes in. | 20:13 |
| efried | cdent: I'd almost say adding this restriction is microversion-worthy, though, isn't it? | 20:14 |
| cdent | strictly speaking yes: because a PUT that used to work will no longer work | 20:15 |
| efried | cdent: Hypothetically, what would the least intrusive, definitely-not-requiring-a-microversion version of this change look like? | 20:15 |
| * cdent thinks | 20:16 | |
| efried | Switching from UUIDField to StringField (or whatever that's called) internally and updating the docs? | 20:16 |
| cdent | that's at the extreme end, yes | 20:16 |
| cdent | I agree with the assertion that if we are using a UUIDField we should validate it as such (and error) | 20:17 |
| efried | I suppose ideally we would also change all internal symbols s/consumer_uuid/consumer_name/ or similar. | 20:18 |
| cdent | and that by being squishy about in the past we've run ourselves into trouble | 20:18 |
| cdent | well there's the rub | 20:18 |
| efried | mm, because it's not just python vars; it's db columns | 20:18 |
| cdent | in order for consumer ids to work, they need to behave like uuids in the context of the universe of allocations in this placement service | 20:18 |
| efried | oh? How so? | 20:19 |
| efried | They need to be universally unique, but that doesn't mean they need to be UUIDs. | 20:19 |
| cdent | if we say "the universe" is "this placement service", then allocation ids (what we call consumer uuids now) need to be unique | 20:19 |
| cdent | I'm using the term generically | 20:19 |
| *** tssurya has quit IRC | 20:19 | |
| cdent | which is a pretty strong argument for then being UUIDs (strictly) | 20:20 |
| efried | I think I see. There's technically nothing stopping one API user from creating allocations against an existing consumer created by another, but doing that unintentionally is way harder with UUIDs than with <other>. | 20:20 |
| cdent | but a) we haven't done that thus far, b) is it maybe up to the client to decide what universe it is in, but c) a universe with many clients will struggle to agree | 20:20 |
| cdent | yes | 20:20 |
| efried | Yeah, so with all of that in mind, I think this is probably the best approach, even though unfortunately-restrictive in the future potentially. | 20:21 |
| cdent | what I worry about, though, is that we have some systems that us uuids with '-', for example, that we won't consider valid | 20:21 |
| cdent | s/us/use/ | 20:21 |
| cdent | s/with/without/ | 20:21 |
| efried | cdent: I thought is_uuid_like took that into account. | 20:21 |
| * cdent sighs | 20:21 | |
| efried | Though we should (still) be sanitizing those. | 20:22 |
| cdent | exactly | 20:22 |
| cdent | we're not in sync with ourselves | 20:22 |
| efried | separate bug, arguably, though I could certainly get behind fixing that in this patch. | 20:22 |
| efried | um, doesn't it get normalized when you stuff it in the UUIDField? | 20:23 |
| cdent | I don't know | 20:23 |
| *** tssurya has joined #openstack-placement | 20:25 | |
| efried | checking... | 20:25 |
| efried | cdent: Nope. | 20:27 |
| cdent | huh | 20:27 |
| cdent | (sorry for partial attention, working on another thing at same time0 | 20:27 |
| efried | cdent: In fact, if you look at the docstring for UUIDField... | 20:27 |
| efried | ...it provides a recipe for doing what we're doing here. | 20:28 |
| efried | sort of | 20:28 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: delete consumers which no longer have allocations https://review.openstack.org/581086 | 20:35 |
| *** e0ne has joined #openstack-placement | 20:36 | |
| jaypipes | mriedem, efried: k, round 5. ^^ | 20:37 |
| efried | jaypipes: +2, thanks. | 20:39 |
| efried | cdent: I -1'd that patch on the grounds that we need to sanitize the UUID. But referenced our discussion and the overall grudging agreement that we need to do it this way based on momentum. | 20:40 |
| cdent | thanks | 20:40 |
| efried | cdent: Do we have a bug for "sanitize UUIDs"? | 20:41 |
| mriedem | note again that keystone doesn't enforce uuids | 20:41 |
| mriedem | instance.project_id and user_id are 255 in the nova db | 20:41 |
| efried | Yup, noted in the patch. | 20:41 |
| efried | the consumer UUID doesn't go anywhere near keystone, though, does it? | 20:41 |
| cdent | efried: this is the closest: https://bugs.launchpad.net/nova/+bug/1758057 | 20:42 |
| openstack | Launchpad bug 1758057 in OpenStack Compute (nova) "When creating uuid-based entities we can duplicate UUIDs" [Undecided,In progress] - Assigned to Rajat Sharma (tajar29) | 20:42 |
| mriedem | efried: not directly in placement no | 20:42 |
| efried | yup, that's the one. | 20:42 |
| openstackgerrit | Dan Smith proposed openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375 | 20:42 |
| efried | mriedem: Even as instance IDs, from nova, though? | 20:42 |
| mriedem | the instance project_id / user_id come from the request context when creating a server | 20:42 |
| mriedem | and that project_id/user_id come from keystone, | 20:43 |
| mriedem | and those might not be uiuds | 20:43 |
| mriedem | *uuids | 20:43 |
| mriedem | at least for user_id | 20:43 |
| mriedem | according to knikolla | 20:43 |
| mriedem | if you're using ldap or something that's not going to be a uuid, and it's longer than 36 characters you're going to be screwed anyway | 20:43 |
| mriedem | because of the placement data model | 20:44 |
| efried | I get that, for proj & user, but we're not talking about those at all in this patch. We're only talking about consumer_uuid. Which, if we're limiting our field of view to nova, is an instance UUID. And instance UUIDs don't interact with keystone, do they? | 20:44 |
| mriedem | no, if it's instance id (consumer uuid) then yes | 20:45 |
| mriedem | no keystone | 20:45 |
| mriedem | sorry for the noise | 20:45 |
| cdent | mriedem: as far as I can tell the decision is made that project and user id will remain 255 varchar | 20:45 |
| mriedem | ok and projects.external_id is 255 | 20:46 |
| mriedem | whew | 20:46 |
| mriedem | for some reason i thought we had encoded a project_id and user_id of 36 into the placement data model | 20:46 |
| cdent | it's come up, but I think your research made it so it won't happen | 20:47 |
| mriedem | \o/ | 20:52 |
| mriedem | efried: cdent: dansmith: jaypipes: so on this "nova-manage placement sync_aggregates" CLI change i'm about to update, | 20:52 |
| mriedem | i've got the additive part done (add provider aggregates where we have compute host aggregates), | 20:52 |
| mriedem | but i've left the removal as a todo because i think that's something people should opt into with an option, and maybe along with a specific host, | 20:53 |
| mriedem | because if we blindly remove provider aggregates for hosts that aren't in nova aggregates, we could blow up externally managed provider aggregates for things like ironic nodes and shared storage pools | 20:53 |
| mriedem | would y'all agree? | 20:53 |
| cdent | yes | 20:54 |
| efried | absolutely | 20:54 |
| efried | To rephrase: We can't say that a placement aggregate should be deleted just because it doesn't have a corresponding nova host aggregate. | 20:55 |
| mriedem | unless you opt into that | 20:55 |
| efried | You would be crazy to opt into that. | 20:55 |
| mriedem | with a --purge or --remove option | 20:55 |
| mriedem | something like that | 20:55 |
| efried | you're basically saying "make sure the *only* aggregates I have are nova host aggregate mirrors". | 20:56 |
| mriedem | yeah, and if you're not doing shared storage aggregate mgmt yourself, or not using ironic, which are pretty new/advanced things at this point, i don't think it's *crazy* | 20:56 |
| efried | "at this point" | 20:56 |
| mriedem | right | 20:57 |
| efried | If you provide this option, it needs to be crystal clear what's going to happen, in big red letters. | 20:57 |
| mriedem | yeah that's why i left it as a separate todo | 20:57 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add nova-manage placement sync_aggregates https://review.openstack.org/575912 | 20:57 |
| mriedem | thar she blar mateys | 20:57 |
| efried | In the bright future, at least some of the non-nova-host placement aggregates are likely to be created under the covers; the operator would never know about them. And if they blow them away... chaos ensues. | 20:58 |
| efried | Or the oversized sta-puft marshmallow man explodes. | 20:58 |
| mriedem | likely to be created under the covers, by whom? | 20:58 |
| efried | example: PowerVM shared storage pool aggregate created by PowerVM driver. | 20:58 |
| efried | (NOT by admin) | 20:59 |
| mriedem | sure | 20:59 |
| mriedem | maybe it would be a case that we'd only want to remove the placement aggregate iff the aggregate has only 1 provider in it and that provider is not in a nova host aggregate, so you'd avoid removing things like pools managed by the virt driver | 21:00 |
| mriedem | since a shared storage pool aggregate management by powervm should have at least 2 providers in it right? | 21:00 |
| mriedem | one for the compute node and one for the MISC_SHARES_VIA_AGGREGATE DISK_GB provider | 21:01 |
| mriedem | yes? | 21:01 |
| efried | not necessarily | 21:01 |
| efried | um, yeah, probably | 21:01 |
| mriedem | otherwise you'd just have a MISC_SHARES_VIA_AGGREGATE-traited provider in an aggregate by itself | 21:01 |
| mriedem | we could also exclude providers with that trait | 21:01 |
| mriedem | anyway, not something i'm tackling anytime soon really | 21:02 |
| mriedem | i think once we land the basic cli to fill gaps then we can consider the mirror bp done | 21:02 |
| jaypipes | mriedem: yes, agree absolutely. | 21:03 |
| jaypipes | mriedem: that we shouldn't blindly remove stuff. | 21:03 |
| mriedem | ok i think i just have a couple of tweaks to make in the functional happy path test for https://review.openstack.org/575912 then - otherwise it's ready for review | 21:06 |
| mriedem | the change is mostly test coverage | 21:06 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Test for unsanitized consumer UUID https://review.openstack.org/581137 | 21:08 |
| efried | cdent: ^^ | 21:08 |
| mriedem | https://review.openstack.org/#/c/581086/ is on its way to the sarlac pits | 21:08 |
| mriedem | is that the saying now? | 21:08 |
| cdent | efried: noted, but later | 21:09 |
| efried | except for your unforgivable misspelling of sarlacc | 21:10 |
| mriedem | yeah yeah | 21:10 |
| efried | I'm totally bluffing. I actually don't even know if that's from Star Wars or Dune. | 21:12 |
| efried | They're going to take away my nerd card. | 21:12 |
| jaypipes | totally Return of the Jedi. | 21:13 |
| mriedem | paul atreides would be rolling over in his sandy grave | 21:14 |
| efried | That's not the kid from Neverending Story? | 21:22 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: update project/user for consumer in allocation https://review.openstack.org/581139 | 21:28 |
| jaypipes | mriedem: ^^ | 21:28 |
| jaypipes | mriedem: hopefully partly unblocks you. | 21:28 |
| mriedem | jaypipes: ok i'll rebase on top of that and see | 21:29 |
| jaypipes | mriedem: cheers | 21:30 |
| *** e0ne has quit IRC | 21:31 | |
| cdent | efried: I put a very light -1 on that uuid verfiication thing, assuming that you wanted to maximize its utility for eventually fixing the bug | 21:41 |
| *** tssurya has quit IRC | 21:55 | |
| *** tssurya has joined #openstack-placement | 22:02 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401 | 22:10 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Change the ServerGroupAntiAffinityFilter to adapt to new policy https://review.openstack.org/571166 | 22:10 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465 | 22:10 |
| mriedem | jaypipes: does the trick | 22:21 |
| * mriedem makes weedly-woo air guitar motion | 22:21 | |
| mriedem | *weedly-wow that is | 22:21 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Heal allocations with incomplete consumer information https://review.openstack.org/574488 | 22:22 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Refactor _heal_instances_in_cell https://review.openstack.org/577896 | 22:22 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Use consumer generation in _heal_allocations_for_instance https://review.openstack.org/577905 | 22:22 |
| mriedem | jaypipes: rebased on your fix, so you can drop the -2 on mine | 22:23 |
| mriedem | oh and https://review.openstack.org/#/c/581086/ exploded in tempest | 22:24 |
| mriedem | not sure why it passed functional tests | 22:24 |
| mriedem | http://logs.openstack.org/86/581086/5/check/tempest-full/4a79185/controller/logs/screen-placement-api.txt.gz?level=TRACE#_Jul_09_21_05_05_553299 | 22:24 |
| mriedem | [SQL: u'DELETE FROM consumers WHERE consumers.uuid IN (SELECT consumers.uuid \nFROM consumers LEFT OUTER JOIN allocations ON consumers.uuid = allocations.consumer_id \nWHERE allocations.consumer_id IS NULL AND 1 != 1)'] (Background on this error at: http://sqlalche.me/e/2j85): InternalError: (1093, u"You can't specify target table 'consumers' for update in FROM clause") | 22:24 |
| openstackgerrit | Chris Dent proposed openstack/nova master: Update some placement docs to reflect modern times https://review.openstack.org/581151 | 22:30 |
| *** tssurya has quit IRC | 22:30 | |
| efried | jaypipes, cdent: Please confirm: When creating an allocation for a *new* consumer, we expect to receive consumer_generation: null, NOT zero. | 22:37 |
| cdent | correct | 22:37 |
| cdent | efried: is something not playing according to plan? | 22:40 |
| efried | cdent: Yeah, I'm updating that test case as you suggested, sending null, and getting "consumer generation conflict - expected 0 but got None" | 22:40 |
| cdent | that sounds like what https://review.openstack.org/#/c/577914/ was supposed to fix. are you up to date? | 22:43 |
| cdent | efried: rather that ^ demonstrates the bug but I think jay has stuff in flight that fixes it | 22:44 |
| efried | cdent: Okay, no, I'm only based on latest master. | 22:45 |
| efried | cdent: Do you have the fixy patch at your fingertips? | 22:45 |
| jaypipes | mriedem: done. | 22:45 |
| jaypipes | efried: yes, correct. | 22:45 |
| cdent | I had a fix https://review.openstack.org/#/c/577915/ but it was decided it wasn't going to work with other changes and I think jay has something in flight | 22:45 |
| efried | I'll just double-stack the TODOs. | 22:45 |
| efried | It's, like, two all-beef TODOs, special sauce, lettuce, cheese, pickles, onions on a generation conflict bun. | 22:46 |
| jaypipes | cdent: ack. efried, rebase on https://review.openstack.org/#/c/579921/ | 22:46 |
| jaypipes | cdent: that should fix you up. | 22:46 |
| jaypipes | efried: that should fix you up. :) | 22:47 |
| jaypipes | efried: or wait until that patch merges... | 22:47 |
| efried | jaypipes: Roger wilco. Guess we should stuff that patch into the aforementioned bug and close it, if so. | 22:47 |
| cdent | I think I will go to bed | 22:47 |
| * cdent waves goodnight | 22:47 | |
| jaypipes | efried: well... there are subtle diffs in the bug | 22:47 |
| jaypipes | cdent: ciao | 22:47 |
| *** cdent has quit IRC | 22:47 | |
| efried | jaypipes: I'm saying if it fixes it, we should close the bug and note the commit where it was fixed. | 22:48 |
| efried | ...and it does seem to fix it. | 22:49 |
| efried | I'll mark it up. | 22:49 |
| jaypipes | mriedem: sigh, the DELETE multiple consumers revision bombed. | 22:49 |
| jaypipes | mriedem: I couldn't get the DELETE consumers FROM consumers LEFT JOIN allocations... to work in sqlalchemy, so tried with the subquery approach. fail. | 22:51 |
| jaypipes | mriedem: I'll have to end up materializing that subquery into an array and then just specifying WHERE uuid IN <array> | 22:51 |
| jaypipes | damn you efried for your efficiency requests. :P | 22:51 |
| * efried considers self damned | 22:52 | |
| efried | jaypipes: https://review.openstack.org/#/c/579921/ should have removed the xfail at https://github.com/openstack/nova/blob/master/nova/tests/functional/api/openstack/placement/gabbits/allocations-1.28.yaml#L237 | 22:53 |
| jaypipes | efried: no, that's not a new consumer. | 22:54 |
| efried | jaypipes: Well, I'm telling you that test succeeds with your patch. | 22:55 |
| efried | jaypipes: Which apparently looks like this when it contains an xfail: | 22:58 |
| efried | {0} nova.tests.functional.api.openstack.placement.test_placement_api.allocations-1.28_put_that_allocation_to_new_consumer.test_request [0.738921s] ... FAILED | 22:58 |
| efried | {0} nova.tests.functional.api.openstack.placement.test_placement_api.allocations-1.28_put_that_allocation_to_new_consumer.test_request [0.000000s] ... ok | 22:58 |
| efried | ... | 22:58 |
| efried | Ran: 2 tests in 9.0000 sec. | 22:58 |
| efried | - Passed: 1 | 22:58 |
| efried | - Skipped: 0 | 22:58 |
| efried | - Expected Fail: 0 | 22:58 |
| efried | - Unexpected Success: 1 | 22:58 |
| efried | - Failed: 0 | 22:58 |
| jaypipes | efried: why wouldn't those have failed in the patch tests, then? | 22:58 |
| efried | Well, it doesn't seem to cause the suite to fail. That's probably a tox or stestr bug. | 22:59 |
| jaypipes | ah, I see. | 23:00 |
| efried | jaypipes: Here it is in your patch: http://logs.openstack.org/21/579921/4/check/nova-tox-functional/5fb6ee9/job-output.txt.gz#_2018-07-09_17_22_11_846366 and http://logs.openstack.org/21/579921/4/check/nova-tox-functional/5fb6ee9/job-output.txt.gz#_2018-07-09_17_31_07_229271 | 23:00 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: delete consumers which no longer have allocations https://review.openstack.org/581086 | 23:02 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: update project/user for consumer in allocation https://review.openstack.org/581139 | 23:02 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Test for unsanitized consumer UUID https://review.openstack.org/581137 | 23:04 |
| jaypipes | efried: removing the xfail now. perhaps you wouldn't mind +W'ing it after I push? | 23:04 |
| efried | cdent, jaypipes: ^ | 23:04 |
| efried | jaypipes: Okay, you're yanking it from the gate rather than doing a fup? | 23:04 |
| efried | either way es geht | 23:05 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: placement: delete auto-created consumers on fail https://review.openstack.org/579921 | 23:06 |
| jaypipes | efried: ^ | 23:06 |
| efried | jaypipes: +A | 23:07 |
| jaypipes | efried: danke | 23:07 |
| efried | jaypipes: Re+2 https://review.openstack.org/#/c/581137/ if you please? | 23:08 |
| efried | thx | 23:09 |
| jaypipes | efried: done | 23:09 |
| efried | jaypipes: You abandoned the 404 one... what is my response if I ask for allocations for a nonexistent consumer? | 23:10 |
| efried | jaypipes: Looks like mebbe it's 200 {}. That should be 404 IMO. | 23:12 |
| efried | I done restored it, so there. | 23:13 |
| jaypipes | efried: see my comment... | 23:14 |
| efried | jaypipes: testing now | 23:15 |
| jaypipes | efried: no, I mean about the abandon one. | 23:15 |
| efried | jaypipes: Yeah, it's 200 {}, tested on top of the delete patch. | 23:15 |
| jaypipes | efried: the current behaviour (correct) is return 200 {} | 23:15 |
| efried | jaypipes: I'll push this test case. | 23:15 |
| efried | jaypipes: 200 {} is not the behavior we want, is it? | 23:16 |
| efried | or is it? | 23:16 |
| jaypipes | efried: if we change it to a 400 (not a 404), that would require a new microversion. | 23:16 |
| efried | why 400 rather than 404? | 23:16 |
| jaypipes | efried: https://specs.openstack.org/openstack/api-wg/guidelines/http/response-codes.html#failure-code-clarifications | 23:16 |
| efried | I thought we talked about this last week and agreed it made more sense as 404 in this case. | 23:16 |
| jaypipes | efried: third bullet point. | 23:17 |
| efried | jaypipes: Yeah, "a nonexistent resource in the body (not URI)" | 23:17 |
| efried | This is a nonexistent resource in the URI. | 23:17 |
| * efried keeps reading the bullet... | 23:17 | |
| jaypipes | efried: the resource is /allocations | 23:17 |
| jaypipes | efried: the {consumer_uuid} isn't a thing we have as a resource (yet, and AFAICT, for the foreseeable future) | 23:18 |
| jaypipes | efried: either way, the existing behaviour is 200 {} and I'm not sure it's a huge problem to keep it that way now that the delete auto-created consumers and delete consumers w/o allocs bugs are resolved. | 23:19 |
| efried | jaypipes: The guideline isn't specific enough to answer this one unambiguously. IMO if you do anything to <URI> where <URI> represents anything that doesn't exist, that's a 404. In either case I don't think 200 { 'allocations': {} } is a reasonable response, for the same reason we were bugging about that before: namely, it doesn't contain a consumer gen, project ID, or user ID and is therefore inconsistent with the e | 23:21 |
| *** bhagyashri_s has joined #openstack-placement | 23:22 | |
| jaypipes | efried: either way, needs a new microversion, and I ain't up for that effort at the moment. I can consider doing something about that later on, but I don't believe it's a blocker for reshaper or heal allocations? | 23:23 |
| efried | jaypipes: Agree with the latter; let's just keep the patch open, make sure there's a bug, and hash it out later. | 23:23 |
| *** bhagyashris has quit IRC | 23:25 | |
| *** edmondsw has quit IRC | 23:26 | |
| jaypipes | efried: ack, cool with me. | 23:28 |
| openstackgerrit | Eric Fried proposed openstack/nova master: return 404 when no consumer found in allocs https://review.openstack.org/579163 | 23:31 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Update some placement docs to reflect modern times https://review.openstack.org/581151 | 23:44 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!