| *** mriedem has quit IRC | 01:06 | |
| *** mriedem has joined #openstack-placement | 01:15 | |
| mriedem | this looks odd http://paste.openstack.org/show/724562/ | 01:16 |
|---|---|---|
| mriedem | why would i have 7982 total memory_mb for inventory | 01:16 |
| mriedem | but then 11205 showing for capacity in provider_summaries? | 01:16 |
| mriedem | oh allocation ratio | 01:16 |
| mriedem | nvm | 01:16 |
| *** mriedem has quit IRC | 01:29 | |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add InstanceGroupPolicy object https://review.openstack.org/573628 | 01:38 |
| openstackgerrit | Merged openstack/nova stable/queens: Fix unit test modifying global state https://review.openstack.org/578722 | 01:40 |
| openstackgerrit | yanpuqing proposed openstack/nova master: Rename auth_uri to www_authenticate_uri https://review.openstack.org/576820 | 01:59 |
| *** tetsuro has joined #openstack-placement | 02:05 | |
| *** takashin has left #openstack-placement | 02:26 | |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: CLI allocation candidates (v1.10) https://review.openstack.org/514647 | 02:37 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: New dict format of allocations (v1.11, v1.12) https://review.openstack.org/542819 | 02:37 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Transactionally update allocations (v1.13) https://review.openstack.org/546674 | 02:37 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Add nested resource providers (v1.14) https://review.openstack.org/546675 | 02:37 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Limit allocation candidates (v1.15, v1.16) https://review.openstack.org/548043 | 02:37 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Allocation candidates parameter: required (v1.17) https://review.openstack.org/548326 | 02:37 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Remove doc/build during tox -e docs https://review.openstack.org/579041 | 02:39 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Fix the 1.6 release note format https://review.openstack.org/579042 | 02:41 |
| openstackgerrit | Merged openstack/nova master: Skip ServerShowV247Test.test_update_rebuild_list_server in nova-cells-v1 job https://review.openstack.org/578125 | 02:44 |
| *** tetsuro has quit IRC | 03:18 | |
| *** alex_xu has quit IRC | 04:15 | |
| *** alex_xu has joined #openstack-placement | 04:15 | |
| openstackgerrit | Merged openstack/nova master: VMware: remove reading resourcePool data https://review.openstack.org/574553 | 05:43 |
| openstackgerrit | Merged openstack/nova master: sync_guest_time: use the proper errno https://review.openstack.org/572346 | 05:43 |
| openstackgerrit | Merged openstack/nova master: Downgrade overquota warning https://review.openstack.org/572028 | 05:44 |
| openstackgerrit | Merged openstack/nova master: Use nova.test.TestingException https://review.openstack.org/575012 | 05:44 |
| openstackgerrit | Merged openstack/nova master: update the description of hypervisor statistics response https://review.openstack.org/462355 | 05:44 |
| *** nicolasbock has joined #openstack-placement | 06:10 | |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Merge server create for scheduler hint extension https://review.openstack.org/579067 | 06:28 |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Merge server create for scheduler hint extension https://review.openstack.org/579067 | 06:32 |
| *** tetsuro has joined #openstack-placement | 06:48 | |
| openstackgerrit | Zhenyu Zheng proposed openstack/nova master: API: add support to abort queued live migration in microversion 2.64 https://review.openstack.org/573136 | 06:58 |
| openstackgerrit | Zhenyu Zheng proposed openstack/nova master: nova-manage db archive_deleted_rows is not multi-cell aware https://review.openstack.org/507486 | 06:59 |
| *** tetsuro has quit IRC | 07:05 | |
| *** tetsuro has joined #openstack-placement | 07:09 | |
| openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata https://review.openstack.org/578019 | 07:12 |
| openstackgerrit | Merged openstack/nova master: conf: libvirt: Make `/dev/urandom` the default for 'rng_dev_path' https://review.openstack.org/577385 | 07:18 |
| *** tetsuro has quit IRC | 07:18 | |
| openstackgerrit | Merged openstack/nova master: Update scheduler to use image-traits https://review.openstack.org/576054 | 07:18 |
| *** tetsuro has joined #openstack-placement | 07:19 | |
| openstackgerrit | Merged openstack/nova master: Test alloc_cands with indirectly sharing RPs https://review.openstack.org/519601 | 07:23 |
| openstackgerrit | Merged openstack/nova master: Powervm configuration cleanup https://review.openstack.org/575171 | 07:23 |
| openstackgerrit | Merged openstack/nova master: placement: s/None/null/ in consumer conflict msg https://review.openstack.org/577171 | 07:23 |
| *** tetsuro has quit IRC | 07:23 | |
| *** tetsuro has joined #openstack-placement | 07:27 | |
| *** tssurya has joined #openstack-placement | 07:32 | |
| openstackgerrit | huanhongda proposed openstack/nova stable/queens: [Stable Only] Remove soft-deleted instances from quota_usages https://review.openstack.org/579093 | 08:27 |
| *** gibi is now known as giblet | 08:33 | |
| *** bauzas is now known as PapaOurs | 08:38 | |
| *** avolkov has joined #openstack-placement | 08:43 | |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Add microversion for nested allocation candidate https://review.openstack.org/565487 | 08:51 |
| openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Fix nits in placement-return-all-resources series https://review.openstack.org/573604 | 08:51 |
| *** stephenfin is now known as finucannot | 08:51 | |
| openstackgerrit | Merged openstack/nova master: libvirt: Fix the rescue race for vGPU instances https://review.openstack.org/577424 | 08:52 |
| openstackgerrit | Merged openstack/nova master: Merge server create schema for legacy BDM extension https://review.openstack.org/577152 | 08:52 |
| openstackgerrit | Merged openstack/nova stable/pike: Add policy rule to block image-backed servers with 0 root disk flavor https://review.openstack.org/563700 | 08:52 |
| *** tetsuro has quit IRC | 08:52 | |
| *** cdent has joined #openstack-placement | 08:53 | |
| openstackgerrit | Merged openstack/nova master: Use ironic-tempest-dsvm-ipa-wholedisk-bios-agent_ipmitool-tinyipa in tree https://review.openstack.org/578878 | 09:08 |
| openstackgerrit | Chris Dent proposed openstack/nova master: Use nova.db.api directly https://review.openstack.org/543262 | 09:08 |
| openstackgerrit | Andrey Volkov proposed openstack/osc-placement master: CLI allocation candidates (v1.10) https://review.openstack.org/514647 | 09:45 |
| openstackgerrit | Andrey Volkov proposed openstack/osc-placement master: New dict format of allocations (v1.11, v1.12) https://review.openstack.org/542819 | 09:45 |
| openstackgerrit | Andrey Volkov proposed openstack/osc-placement master: Transactionally update allocations (v1.13) https://review.openstack.org/546674 | 09:45 |
| openstackgerrit | Andrey Volkov proposed openstack/osc-placement master: Add nested resource providers (v1.14) https://review.openstack.org/546675 | 09:45 |
| openstackgerrit | Andrey Volkov proposed openstack/osc-placement master: Limit allocation candidates (v1.15, v1.16) https://review.openstack.org/548043 | 09:45 |
| openstackgerrit | Andrey Volkov proposed openstack/osc-placement master: Allocation candidates parameter: required (v1.17) https://review.openstack.org/548326 | 09:45 |
| openstackgerrit | Sylvain Bauza proposed openstack/nova master: Fix placement incompatible with webob 1.7 https://review.openstack.org/579110 | 09:46 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add InstanceGroupPolicy object https://review.openstack.org/573628 | 10:11 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Refactor the policies to policy https://review.openstack.org/579113 | 10:11 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: Define common variables for irrelevant-files https://review.openstack.org/578882 | 10:39 |
| openstackgerrit | Stephen Finucane proposed openstack/nova master: manage: Remove dead code https://review.openstack.org/579118 | 10:50 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Update admin/flavors document https://review.openstack.org/573063 | 10:59 |
| openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Handle LM rollback error when detaching volumes from transient domain https://review.openstack.org/579125 | 11:18 |
| *** edmondsw has joined #openstack-placement | 11:32 | |
| *** mriedem has joined #openstack-placement | 12:08 | |
| *** jaypipes is now known as leakypipes | 12:14 | |
| leakypipes | efried: I'm +2 on tetsuro's nested alloc cands API patch. | 12:18 |
| leakypipes | https://review.openstack.org/#/c/565487/ | 12:19 |
| openstackgerrit | Surya Seetharaman proposed openstack/nova stable/queens: Make nova service-list use scatter-gather routine https://review.openstack.org/579135 | 12:22 |
| openstackgerrit | Surya Seetharaman proposed openstack/nova stable/pike: Make nova service-list use scatter-gather routine https://review.openstack.org/579136 | 12:33 |
| *** leakypipes has quit IRC | 12:34 | |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: CLI allocation candidates (v1.10) https://review.openstack.org/514647 | 12:44 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: New dict format of allocations (v1.11, v1.12) https://review.openstack.org/542819 | 12:44 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Transactionally update allocations (v1.13) https://review.openstack.org/546674 | 12:44 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Add nested resource providers (v1.14) https://review.openstack.org/546675 | 12:44 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Limit allocation candidates (v1.15, v1.16) https://review.openstack.org/548043 | 12:44 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Allocation candidates parameter: required (v1.17) https://review.openstack.org/548326 | 12:44 |
| *** peereb has joined #openstack-placement | 12:46 | |
| *** jaypipes has joined #openstack-placement | 12:47 | |
| *** jaypipes is now known as leakypipes | 12:47 | |
| *** openstackgerrit has quit IRC | 12:49 | |
| *** openstackgerrit has joined #openstack-placement | 12:58 | |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Limit allocation candidates (v1.15, v1.16) https://review.openstack.org/548043 | 12:58 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Allocation candidates parameter: required (v1.17) https://review.openstack.org/548326 | 12:58 |
| *** efried is now known as fried_rice | 12:58 | |
| fried_rice | leakypipes: Schweet. | 13:01 |
| fried_rice | leakypipes, alex_xu: The cleanup followon should be a trivial approve: https://review.openstack.org/#/c/573604/ | 13:14 |
| *** edleafe is now known as figleaf | 13:14 | |
| fried_rice | Let's put this bp to bed | 13:14 |
| openstackgerrit | Merged openstack/nova master: [placement] Extract create_allocation_list https://review.openstack.org/577200 | 13:16 |
| cdent | when is something in bed? When the api can do it, or the scheduler can? | 13:16 |
| openstackgerrit | Merged openstack/nova master: [placement] Fix capacity tracking in POST /allocations https://review.openstack.org/578133 | 13:16 |
| openstackgerrit | Merged openstack/nova master: libvirt: Add missing encryption_secret_uuid tests for pre_live_migration https://review.openstack.org/540679 | 13:17 |
| fried_rice | cdent: The blueprint is in bed when the stuff in the spec is finished. This spec didn't mention a thing about the scheduler. It's going to be years before the scheduler has taken full advantage of nrp. | 13:18 |
| cdent | I wasn't asserting it wasn't in bed. More meandering mentally about those "years before" | 13:19 |
| leakypipes | fried_rice: done | 13:23 |
| cdent | fried_rice, leakypipes: since we're all three here at the moment: do we have any synchronizing we need to do on the Reshape stuff? | 13:24 |
| cdent | I've left a slot in https://review.openstack.org/#/c/576927/ to call a metho that takes a dict of inventorylists (by rp) and an AllocationList | 13:27 |
| leakypipes | cdent: I haven't yet gotten to that stuff. Still working on the fix for that "multiple RPs in PUT /allocations requires multiple consumer generations" thing. | 13:31 |
| leakypipes | cdent: hoping to wrap that up this morning and concentrate on the reshaper stuff. | 13:31 |
| cdent | leakypipes: yeah, not trying to rush things, just checking to see if we've got anything we need to catch up on | 13:32 |
| leakypipes | cdent: pep8 error. the horror. | 13:41 |
| cdent | leakypipes: on reshaper? that's expected, see my response to fried_rice | 13:41 |
| mriedem | giblet: i'm +2 on the bottom 2 osc-placement patches now https://review.openstack.org/#/c/514646/ | 13:42 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Allocation candidates parameter: required (v1.17) https://review.openstack.org/548326 | 13:43 |
| leakypipes | cdent: ya | 13:44 |
| *** tssurya is now known as sususuryashines | 13:46 | |
| openstackgerrit | Sylvain Bauza proposed openstack/nova master: Fix placement incompatible with webob 1.7 https://review.openstack.org/579110 | 13:49 |
| openstackgerrit | Jay Pipes proposed openstack/nova master: return 404 when no consumer found in allocs https://review.openstack.org/579163 | 13:49 |
| leakypipes | cdent: your advice sought on ^^ | 13:49 |
| cdent | roger | 13:50 |
| fried_rice | leakypipes, cdent: This seems like a new microversion without question. | 13:52 |
| leakypipes | fried_rice: yeah, I suppose so, but did want to check with you guys. | 13:53 |
| fried_rice | Not necessarily "fixing" the "bug" of missing gen/proj/user for allocation-less consumers - but certainly the 404-vs-empty behavior change is microversion-worthy. | 13:53 |
| leakypipes | fried_rice: what do you mean not fixing the bug? | 13:54 |
| fried_rice | I'm saying the bug fix isn't necessarily microversion-worthy, but the 404 thing is. | 13:54 |
| leakypipes | fried_rice: I don't see how one would fix the bug without changing the response of GET /allocations/{consumer}? | 13:54 |
| fried_rice | leakypipes: Well, I haven't looked at the code at all, just the commit message. | 13:54 |
| fried_rice | leakypipes: But they seem like different things. | 13:55 |
| fried_rice | leakypipes: In one case you've got a consumer that actually does exist, but has no allocations. Previously you were returning an empty payload; now you're returning a payload with gen/proj/user and empty allocations. | 13:55 |
| fried_rice | leakypipes: In the other case you've got no consumer at all, and instead of returning an empty payload you're erroring 404. | 13:55 |
| fried_rice | Regardless of whether the code paths coincide/overlap (I'm sure they do) I'm not suggesting separating those things into different patches or anything. | 13:56 |
| fried_rice | I'm just speaking to the procedural necessity of cutting a new microversion. | 13:56 |
| leakypipes | fried_rice: k | 13:58 |
| fried_rice | yeah, now looking at the code they're totally separate code paths anyway. But again, not suggesting to split them. | 13:58 |
| leakypipes | fried_rice: how are they totally separate code paths? it's all in list_allocations_for_consumer | 13:58 |
| leakypipes | fried_rice: are you referring to doing the Consumer.get_by_uuid() before AllocationList.get_by_consumer()? | 13:59 |
| fried_rice | leakypipes: https://review.openstack.org/#/c/579163/1/nova/api/openstack/placement/handlers/allocation.py The 404 thing is isolated to L163-170 | 13:59 |
| fried_rice | yeah | 13:59 |
| leakypipes | fried_rice: gotcha, ok, yes. | 13:59 |
| leakypipes | fried_rice: there's no way to return the consumer project ID, user ID and generation, though, without grabbing that information first, and that's what the Consumer.get_by_uuid() is for. | 14:00 |
| leakypipes | fried_rice: unless we change AllocationList.get_by_consumer() to be a LEFT JOIN against the consumers, projects, users tables which is exactly what we don't want to do. | 14:01 |
| fried_rice | leakypipes: Gotcha; so if they were split, you would have to do the 404 first. | 14:01 |
| fried_rice | or not, you could do it the other way by ignoring the exception and hacking up an "empty" consumer. | 14:01 |
| fried_rice | But anyway, still not suggesting that. | 14:01 |
| fried_rice | In fact if anything I think we could expand the microversion drop to include some mechanism for deleting a freaking consumer. | 14:02 |
| leakypipes | fried_rice: we don't want to delete the consumer, though... | 14:04 |
| leakypipes | fried_rice: well, only when the AllocationList.create_all() failed. | 14:04 |
| fried_rice | leakypipes: I am not included in your "we". | 14:04 |
| leakypipes | fried_rice: but we don't want to add a DELETE /consumers/{uuid} endpoint, or at least, I wasn't aware of that. | 14:04 |
| leakypipes | fried_rice: oh :) sorry! | 14:04 |
| fried_rice | I want *some* mechanism to delete consumers. | 14:04 |
| openstackgerrit | Merged openstack/nova stable/queens: Use instance project/user when creating RequestSpec during resize reschedule https://review.openstack.org/577918 | 14:05 |
| fried_rice | We're going to end up with thousands of stale records in the consumers table after a relatively short amount of time in a large deployment. | 14:05 |
| fried_rice | Not that that takes up sooo much space, but it's messy and should be fixed. | 14:05 |
| fried_rice | leakypipes: You already expressed that you don't like the idea of auto-deleting a consumer when its last allocation is removed - but I don't know whether that was because you generally don't want to delete consumers or because you don't think it should be done that particular way. | 14:06 |
| fried_rice | leakypipes: The DELETE /consumers/{u} operation is another option, and the one I most prefer. | 14:07 |
| fried_rice | leakypipes: And then there's "create a management script that I can use to periodically go in and scrub out stale consumer records". Which is, IMO, ew. | 14:07 |
| leakypipes | fried_rice: those aren't stale records. they're records, plain and simple. | 14:08 |
| leakypipes | fried_rice: they are a record of past usage. | 14:08 |
| fried_rice | They're stale from the perspective of a know placement use case (i.e. nova). | 14:08 |
| cdent | only that the consunmer existed, not what they did? | 14:08 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Refactor the policies to policy https://review.openstack.org/579113 | 14:08 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add policy to InstanceGroup object and api models. https://review.openstack.org/563375 | 14:08 |
| leakypipes | cdent: yes, correct. | 14:08 |
| fried_rice | Does nova keep track of (hard) deleted instances in the database? | 14:08 |
| cdent | I wrote some related comments on the review itself (about how not deleting impacts flow) | 14:09 |
| leakypipes | cdent: i.e. "X number of consumers have passed through Placement-town" :P | 14:09 |
| leakypipes | fried_rice: yes, that's what the shadow tables are for. | 14:09 |
| leakypipes | fried_rice: I don't particularly care for that type of setup, but it is what it is. | 14:10 |
| cdent | "sherrif, have you seen c37ec127-62c5-45d4-9efd-1776f13e7d50? damn sure he came this way. | 14:10 |
| fried_rice | I don't know what a "shadow table" is. | 14:10 |
| leakypipes | fried_rice: every table in the nova cell DB has a shadow table associated with it. when you run a db purge, deleted records are moved from the active tables to the corresponding shadow table. | 14:11 |
| fried_rice | In any case, leaving a trace of past consumers - weak though that sounds to me - might be an argument to *allow* consumer records to hang around forever. But it should not be a reason to *force* consumer records to hang around forever (i.e. provide no way to remove them). | 14:12 |
| leakypipes | ack, no disagreement from me on that | 14:12 |
| fried_rice | Okay, so then... what's the objection to providing a mechanism to delete consumer records? | 14:13 |
| giblet | mriedem: started reviewing the osc patches but I got distracted. I will go back to there | 14:13 |
| cdent | fried_rice: why would you want to do that over the api, if it is an db cleanup behavior? | 14:14 |
| fried_rice | cdent: I don't see it as being a db cleanup behavior (unless we do it automatically when last allocation goes away) | 14:16 |
| fried_rice | cdent: Particularly since there's now going to be a difference between GETting an allocation-less consumer that never existed vs one that once existed. | 14:16 |
| cdent | right, that was the main thrust of my comment on the review | 14:16 |
| cdent | I think we should delete consumers after their last allocation goes away | 14:17 |
| cdent | but if for some reason we are not doing that, and there are going to be "stale" consumers in the db, then cleaning them is not a job to be done over HTTP, it is a job to be done with a "*-manage" script | 14:18 |
| fried_rice | cdent: Okay, except I think the whole model is done broke and I'd like to clean it up entirely. | 14:19 |
| figleaf | cdent: In a nova-only world, I would agree. But the other day I gave the example (since the fridge thing is SO last year) of using placement to manage a library. In this case, you don't destroy the library card when books are returned. | 14:19 |
| leakypipes | figleaf: ++ | 14:20 |
| leakypipes | figleaf: unless we're talking about the Great Library Card Purge of 2028 | 14:20 |
| cdent | figleaf: have you read my comment on the review? It's not clear to me that a consumer uuid is a library card. it is an instance of checking out a book | 14:20 |
| cdent | leakypipes: that purge is coming much sooner than 2028, sadly | 14:21 |
| leakypipes | cdent: wouldn't the allocations be the book checkouts? | 14:21 |
| leakypipes | cdent: along with the voter roll purges, yep. :( | 14:21 |
| cdent | well, to me a consumer uuid is an allocation uuid. it says so right there in the URI | 14:21 |
| cdent | /allocations/{uuid} | 14:22 |
| cdent | the library card is more accurately the pairing of the project id and user id | 14:22 |
| cdent | that's what I need to show when I go to the library "hi i'm chris, here's my proof that I can check out a book" | 14:23 |
| cdent | oh, okay, here's your book | 14:23 |
| figleaf | So why would consumers need to exist at all? Just put project id and user id in the allocation record. | 14:24 |
| figleaf | since consumers have no meaning outside of a current allocation | 14:25 |
| fried_rice | figleaf: How do you tie that allocation record back to a consumer (instance)? | 14:25 |
| figleaf | fried_rice: ok, put the instance id in there too | 14:26 |
| figleaf | when you delete the allocation, there is no stale consumer | 14:26 |
| fried_rice | That would wfm | 14:26 |
| fried_rice | But other than that, I don't understand why we're talking about all the ways we can ponce around the fringes of having a real GET/PUT/DELETE /consumers/{u} setup. | 14:26 |
| fried_rice | "If you're using microversion 1.30 or above, consumers are a first-class route. You have to PUT /consumers/{u} payload={proj, user, gen} before you can PUT /allocations/{consumer} to that consumer; and when you're done, you may DELETE /consumers/{u} (or not, if you want to keep a record of those for some reason)." | 14:27 |
| cdent | fried_rice: because, at least for me, I have heard no reason for such a thing to exist? | 14:27 |
| figleaf | But cdent is saying that consumers are ephemeral with the allocaiton | 14:27 |
| cdent | It sounds like artificial overhead | 14:27 |
| fried_rice | No reason for such a thing to exist | 14:27 |
| fried_rice | it exists | 14:27 |
| fried_rice | this whole conversation centers around the fact that it exists | 14:28 |
| cdent | "such a thing" == "api for /consumers/{u}" | 14:28 |
| fried_rice | and that we don't have a way to make it not exist when we're done with it. | 14:28 |
| cdent | we have plenty of ways | 14:28 |
| fried_rice | cdent: I don't consider executing SQL commands into the database to be a viable "way". The "management script" option would be the very minimum solution I would accept here, grudgingly. | 14:29 |
| cdent | I don't want that way, but it is one. | 14:30 |
| cdent | If I had my druthers, consumer uuids (and the now associated objecvt) would expire when their allocations go to nil | 14:30 |
| fried_rice | Yes, auto-deleting with last allocation is second least favorite on my list. | 14:30 |
| cdent | but that _may_ be impossible because of the way we are using int generations and the potential for races | 14:31 |
| openstackgerrit | Merged openstack/nova stable/pike: mock utils.execute() in qemu-img unit test https://review.openstack.org/578058 | 14:31 |
| cdent | forcing people to manage consumer uuids, in placement, semi-manually is...I don't know...insulting | 14:31 |
| fried_rice | It's not different than asking them to manage resource providers. | 14:31 |
| figleaf | cdent: So you see it more as an "allocation group id"? I.e., identifying which allocations are for the same thing, and not as a thing that survives those allocations? | 14:32 |
| fried_rice | Why don't we create those automatically when you create inventory? | 14:32 |
| cdent | fried_rice: resource providers persist! | 14:32 |
| cdent | their inventory can change | 14:32 |
| cdent | that's part of the meaning of the api | 14:32 |
| fried_rice | Sure, and a consumer's project/user will "never" change. | 14:32 |
| cdent | figleaf: yes. for instance a consumer uuid identifies a running instance | 14:32 |
| cdent | fried_rice: but the running instance will stop existing | 14:33 |
| cdent | the instance is the consumer, not the project/user | 14:33 |
| fried_rice | The compute node is the provider, not the inventory | 14:34 |
| giblet | mriedem: left some comments in the first osc_placement patch | 14:34 |
| fried_rice | Whatever, I'd be fine with auto-delete with last allocation, can we do that please? | 14:34 |
| fried_rice | as opposed to doing nothing? | 14:34 |
| fried_rice | which to me is completely unacceptable | 14:34 |
| cdent | fried_rice: well, I think we have that review upon which we can discuss things, probalby in a more progress oriented fashion than here. my comments there (about the flow) remain relevant. | 14:35 |
| figleaf | fried_rice: We've made the argument that consumers were ephemeral, and should be treated as such. That argument was rejected, and we have now set that in stone with Consumer objects and all the code around them. Given that path, it makes no sense to auto-remove consumers, as they are now First Class Things. | 14:36 |
| fried_rice | figleaf: If we accept ^ then the right answer is certainly to create a /consumers/{u} route with GET/PUT/DELETE ops. However, I don't necessarily agree that creating a Consumer object internally locks us into consumers being non-ephemeral. | 14:38 |
| mriedem | giblet: thanks, i've pointed those all out before as well | 14:39 |
| fried_rice | Because we haven't exposed any of that in the API. | 14:39 |
| mriedem | replied about the 1.8 test coverage - it's implicitly tested by the 1.9 usages test | 14:39 |
| mriedem | i realize that's not a great answer | 14:39 |
| openstackgerrit | Merged openstack/os-traits master: normalize_name helper https://review.openstack.org/560107 | 14:39 |
| mriedem | figleaf: the consumer object is about modeling the data, as we do for everything else, it's not a first class citizen because there isn't a top level api for consumers | 14:40 |
| figleaf | fried_rice: In a way I can see your point. It's just that it feels to me like we dug ourselves a hole, and adding a full RESTful interface to it is just digging deeper | 14:41 |
| figleaf | mriedem: but fried_rice is saying we should add the api interface | 14:42 |
| fried_rice | figleaf: I'm not going to try to follow the 'hole' analogy, but adding the REST interface is making it look like we did it on purpose. | 14:42 |
| giblet | mriedem: OK, so we have test coverage, I can live with it. Let's merge it to keep the ball rolling in osc placement | 14:42 |
| giblet | mriedem: or more like restart the roll | 14:42 |
| mriedem | giblet: agree - i'd like to keep it moving, i also made the same comment again about the 1.8 tests in a later patch in the series | 14:42 |
| mriedem | https://review.openstack.org/#/c/542819/30/osc_placement/tests/functional/test_allocation.py@100 | 14:42 |
| figleaf | fried_rice: in for a dime, in for a dollar | 14:43 |
| fried_rice | figleaf: The way it is now, consumers are this messy hack halfway between ephemeral and real. We should get off the fence. Either they're ephemeral (and we auto-remove them) or they're real (and we add a route for them). | 14:43 |
| figleaf | fried_rice: vs. when you find yourself in a hole, the first thing to do is stop digging | 14:43 |
| mriedem | what's the argument against auto-removing consumers when they have no more allocations? | 14:44 |
| fried_rice | I'm talking about lining the hole and installing plumbing and a roof. | 14:44 |
| fried_rice | mriedem: leakypipes said you would want to keep the consumer records for "history". | 14:44 |
| cdent | mriedem: that's leakypipes' position, not entirely sure, | 14:44 |
| figleaf | 09:09 < leakypipes>| cdent: i.e. "X number of consumers have passed through Placement-town" : | 14:45 |
| giblet | mriedem: ack | 14:45 |
| mriedem | umm | 14:45 |
| mriedem | i'm not sure what the use case is for that | 14:45 |
| mriedem | plus, | 14:45 |
| mriedem | this means we'd never purge them | 14:45 |
| mriedem | like we purge deleted instances from the nova db | 14:45 |
| fried_rice | correct. Which is bad. | 14:45 |
| mriedem | kind of defeats the purpose of not using soft delete mixin in the placement api | 14:45 |
| mriedem | *api db | 14:45 |
| figleaf | soft delete should DIAF | 14:46 |
| cdent | fire++ | 14:46 |
| mriedem | right, which is why we didn't use it with the api db | 14:46 |
| mriedem | from the start | 14:46 |
| mriedem | i mean, i guess you could always add a placement-manage db purge command for stuff like that, idk | 14:47 |
| mriedem | but you shouldn't really need to | 14:47 |
| fried_rice | Right, so far the candidates we've discussed are: | 14:47 |
| fried_rice | - Do nothing; let consumer records accumulate forever | 14:47 |
| fried_rice | - Management script I can run to periodically purge allocation-less consumers from the db | 14:47 |
| fried_rice | - Auto-delete with last allocation | 14:47 |
| fried_rice | - Make full GET/PUT/DELETE /consumers/{u} ops | 14:47 |
| fried_rice | (or if we want to be less symmetrical about it, we could just make DELETE /consumers/{u}) | 14:49 |
| mriedem | i would auto-delete unless there is a good reason not to | 14:49 |
| fried_rice | ++ | 14:50 |
| mriedem | it's metadata | 14:50 |
| mriedem | think of it like instance system_metadata | 14:50 |
| mriedem | there is no top level api for that, | 14:50 |
| mriedem | and its gone when the instance is deleted | 14:50 |
| leakypipes | cdent: what exactly did you mean by "forcing people to manage consumer UUIDs in placement is insulting"? | 14:51 |
| cdent | leakypipes: having a separate api for $METHOD /consumers/ when they (currently) are just allocation identifiers is overhead and burdensome | 14:53 |
| cdent | by "currently" their meaning, not their implementation | 14:53 |
| openstackgerrit | huanhongda proposed openstack/nova stable/queens: [Stable Only] Remove soft-deleted instances from quota_usages https://review.openstack.org/579093 | 14:54 |
| leakypipes | cdent: you don't believe consumers are things? | 14:54 |
| leakypipes | cdent: i.e. things that have attributes associated with them (like project and user identifiers)? | 14:55 |
| openstackgerrit | Sylvain Bauza proposed openstack/nova master: Fix placement incompatible with webob 1.7 https://review.openstack.org/579110 | 14:55 |
| cdent | leakypipes: i believe there are things which use allocations (such as instances) and those things have projects and users associated with them, and we have currently reified those things in placement as the concept of a consumer. as I've said above I think in placement (as it curently stands now), the concept (the join of a live instance with a project and user) is ephemeral | 14:57 |
| cdent | i'm going to have to go soon, I hope we can hash this out either on the review or in email, rather than IRC. I'm not sure IRC is doing justice to the nuances of the several positions present | 14:58 |
| leakypipes | cdent: sure, no prob | 14:58 |
| leakypipes | cdent: and agree with you that the review is a better place for this convo | 14:58 |
| leakypipes | cdent: thx, and enjoy your weekend | 14:58 |
| cdent | I have the exciting job of moving around a bunch of stuff so an electrician can tear up the floors and walls in various places | 14:59 |
| leakypipes | cdent: furniture live migration. | 15:01 |
| cdent | I think this is "shelve"? Although I've never been sure what that is. The furniture is going to get stacked in an unusable state | 15:02 |
| openstackgerrit | Merged openstack/nova master: Add microversion for nested allocation candidate https://review.openstack.org/565487 | 15:03 |
| cdent | ^ omg | 15:03 |
| mriedem | fried_rice: can https://blueprints.launchpad.net/nova/+spec/nested-resource-providers-allocation-candidates be marked complete now? ^ | 15:03 |
| mriedem | looks like only remaining open change is an approved nit fixing patch | 15:03 |
| mriedem | cdent: shelve means you'd store your furniture in like a storage locker, | 15:04 |
| mriedem | move | 15:04 |
| mriedem | then bring your furniture back | 15:04 |
| mriedem | to the new place | 15:04 |
| fried_rice | mriedem: We have some work to do to prove that pre ^ behavior is the same in Q and R. I don't know whether that warrants keeping the bp open. | 15:04 |
| mriedem | although you're paying to shelve it in that case, which is opposite of how the pricing model should work in openstack... | 15:05 |
| mriedem | fried_rice: once the microversion has merged that's kind of out the window isn't it? | 15:05 |
| fried_rice | mriedem: Not exactly, no. Long conversations have been had about this. We need to make sure that ^ is the case *by the time we cut Rocky* | 15:06 |
| mriedem | or you mean, testing that requests < 1.29 haven't regressed? | 15:06 |
| fried_rice | Yeah, basically that. | 15:06 |
| mriedem | if they have then it's just bugs | 15:06 |
| fried_rice | because there was a point in mid-Rocky where they had. | 15:06 |
| mriedem | so i don't see a reason to hold the bp for that | 15:06 |
| fried_rice | okay. | 15:06 |
| *** alex_xu has quit IRC | 15:06 | |
| *** e0ne has joined #openstack-placement | 15:08 | |
| *** alex_xu has joined #openstack-placement | 15:09 | |
| *** openstack has quit IRC | 15:22 | |
| *** openstack has joined #openstack-placement | 15:27 | |
| *** ChanServ sets mode: +o openstack | 15:27 | |
| openstackgerrit | Chris Dent proposed openstack/nova master: [placement] fix allocation handler docstring typo https://review.openstack.org/579198 | 15:33 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: WIP Usages per project and user (v1.8, v1.9) https://review.openstack.org/514646 | 15:36 |
| openstackgerrit | Chris Dent proposed openstack/nova master: Use 'version2' when syncing placement db https://review.openstack.org/579200 | 15:40 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Add method to get cpu traits https://review.openstack.org/560317 | 15:43 |
| openstackgerrit | Eric Fried proposed openstack/nova master: FakeLibvirtFixture: mock get_fs_info https://review.openstack.org/579201 | 15:43 |
| openstackgerrit | Merged openstack/nova master: Fix nits in placement-return-all-resources series https://review.openstack.org/573604 | 15:44 |
| openstackgerrit | Merged openstack/nova master: trivial: Remove 'tools/releasenotes_tox.sh' https://review.openstack.org/534383 | 15:45 |
| openstackgerrit | Merged openstack/nova master: tox: Document and dedupe mostly everything https://review.openstack.org/578825 | 15:45 |
| openstackgerrit | Merged openstack/nova master: Remove file injection from config drive sample docs https://review.openstack.org/578888 | 15:45 |
| openstackgerrit | Merged openstack/nova master: Fix CLI docs for nova-manage api_db commands https://review.openstack.org/578116 | 15:45 |
| openstackgerrit | Merged openstack/nova master: Remove unused DB API instance_group_delete method https://review.openstack.org/578223 | 15:45 |
| openstackgerrit | Merged openstack/nova master: Mention nova-status upgrade check CLI in upgrade doc https://review.openstack.org/576719 | 15:46 |
| *** edmondsw has quit IRC | 15:48 | |
| *** edmondsw has joined #openstack-placement | 15:52 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: use dest host vif migrate details for live migration https://review.openstack.org/551370 | 15:56 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Annotate flows and handle PortBindingDeletionFailed in ComputeManager https://review.openstack.org/551371 | 15:56 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Port binding based on events during live migration https://review.openstack.org/434870 | 15:56 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: conductor: use port binding extended API in during live migrate https://review.openstack.org/522537 | 15:56 |
| openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Handle LM rollback error when detaching volumes from transient domain https://review.openstack.org/579125 | 16:02 |
| openstackgerrit | Chris Dent proposed openstack/nova master: WIP: [placement] Add /reshaper handler for POST https://review.openstack.org/576927 | 16:13 |
| mriedem | figleaf: question for you here https://review.openstack.org/#/c/537614/ | 16:16 |
| *** smcginnis[m] has quit IRC | 16:18 | |
| figleaf | mriedem: not sure I understand your question | 16:32 |
| *** fried_rice is now known as fried_rolls | 16:51 | |
| figleaf | mriedem: anyways, I'm on PTO this afternoon, so... | 16:55 |
| *** figleaf is now known as edleafe | 16:55 | |
| *** e0ne has quit IRC | 16:58 | |
| *** smcginnis[m] has joined #openstack-placement | 17:01 | |
| *** e0ne has joined #openstack-placement | 17:11 | |
| *** openstack has quit IRC | 17:11 | |
| *** openstack has joined #openstack-placement | 17:16 | |
| *** ChanServ sets mode: +o openstack | 17:16 | |
| mriedem | edleafe: in that we already have test coverage for resize + caching scheduler | 17:43 |
| mriedem | leakypipes: would you consider that we should then name this command "openstack resource allocation candidate list"? https://review.openstack.org/#/c/514647/37/releasenotes/notes/microversion-1.10-03ab71969921a0e4.yaml | 17:44 |
| mriedem | instead of 'openstack allocation candidate list' as he has it today | 17:44 |
| leakypipes | mriedem: I thought "resource" was the namespace all placement stuff was under... | 18:05 |
| leakypipes | mriedem: if that's not the case, I would say no, keep it at openstack allocation candidate list | 18:06 |
| mriedem | well, traits are | 18:06 |
| mriedem | *aren't | 18:06 |
| mriedem | https://docs.openstack.org/osc-placement/latest/index.html | 18:07 |
| mriedem | but resource classes are... | 18:07 |
| mriedem | i guess we could have done resource trait create/delete/list/show | 18:07 |
| leakypipes | mriedem: well "resource class" is the name of the object :) | 18:07 |
| leakypipes | mriedem: like "resource provider". | 18:07 |
| mriedem | i think for the most part the commands are trying to mirror the names in the api | 18:07 |
| mriedem | if we did that, we'd have just 'openstack usage show' | 18:08 |
| leakypipes | mriedem: I think `openstack resource usage show` is the appropriate command, with --project, --user, and --provider options to switch the REST API call appropriately. | 18:08 |
| leakypipes | mriedem: or `openstack usage show`, yes. | 18:09 |
| leakypipes | mriedem: not sure if that wasn't already taken by the os-simple-tenant-usage stuff. | 18:09 |
| openstackgerrit | Merged openstack/nova master: Match ComputeNode.uuid to ironic node uuid in RT https://review.openstack.org/571535 | 18:10 |
| leakypipes | mriedem: my primary point was that it definitely wasn't appropriate to have "consumer" be part of the command. | 18:10 |
| openstackgerrit | Merged openstack/nova master: Mention PowerVM support of config drive https://review.openstack.org/578840 | 18:10 |
| openstackgerrit | Merged openstack/nova master: Merge server create schema for BDM v2 extension https://review.openstack.org/577185 | 18:10 |
| openstackgerrit | Merged openstack/nova master: Merge server create schema for config drive extension https://review.openstack.org/578329 | 18:10 |
| leakypipes | mriedem: if you want to push a fix for that bottom patch to make it `openstack usage show` I can re-review and we can push through much of that series, to keep the momentum up. | 18:13 |
| leakypipes | mriedem: alternately, if you're swamped, I can give the rework a go. | 18:13 |
| leakypipes | mriedem: just let me know your preference. | 18:13 |
| mriedem | i just redid the bottom one to be 'openstack resource usage show' | 18:15 |
| mriedem | dealing with merge conflicts now | 18:15 |
| leakypipes | mriedem: ack | 18:17 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Usages per project and user (v1.8, v1.9) https://review.openstack.org/514646 | 18:17 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: CLI allocation candidates (v1.10) https://review.openstack.org/514647 | 18:17 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: New dict format of allocations (v1.11, v1.12) https://review.openstack.org/542819 | 18:17 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Transactionally update allocations (v1.13) https://review.openstack.org/546674 | 18:17 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Add nested resource providers (v1.14) https://review.openstack.org/546675 | 18:17 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Limit allocation candidates (v1.15, v1.16) https://review.openstack.org/548043 | 18:17 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Allocation candidates parameter: required (v1.17) https://review.openstack.org/548326 | 18:17 |
| cdent | leakypipes, mriedem in the past dtroyer has said that the command names should very much not be namespaces | 18:28 |
| * mriedem cocks the gun | 18:29 | |
| cdent | I think 'resource usages' makes sense | 18:29 |
| mriedem | openstack usage is too generic for my taste | 18:29 |
| cdent | but because that's what they are | 18:29 |
| cdent | yeah, I agree | 18:29 |
| mriedem | the usage is scoped to resource thingies | 18:29 |
| cdent | but for allocation candidates no prefix would be required | 18:30 |
| mriedem | allocation candidate list is random enough it shouldn't overlap with any other namespace | 18:30 |
| mriedem | even though those allocation candidates are resource-specific | 18:30 |
| mriedem | anywho, i'm ok with what we have in the series for those changes | 18:30 |
| cdent | naw mate | 18:30 |
| mriedem | +2s mostly up the board | 18:30 |
| cdent | allocation candidates are just that: allocation candidates. that they have resources in them is just one of them any things in their complex pretty face | 18:31 |
| cdent | or something | 18:31 |
| mriedem | i'm just surprised at avolkov's dedication and persistence on this | 18:31 |
| * mriedem golf claps | 18:31 | |
| cdent | it's awesome isn't it? | 18:31 |
| * cdent goes back outside | 18:32 | |
| leakypipes | mriedem: k, reviewing. | 18:32 |
| *** e0ne has quit IRC | 18:36 | |
| *** e0ne has joined #openstack-placement | 18:37 | |
| *** tssurya has joined #openstack-placement | 18:57 | |
| *** peereb has quit IRC | 19:07 | |
| *** tssurya has quit IRC | 19:23 | |
| *** tssurya has joined #openstack-placement | 19:57 | |
| *** edmondsw has quit IRC | 20:02 | |
| *** avolkov has quit IRC | 20:05 | |
| *** edmondsw has joined #openstack-placement | 20:09 | |
| *** cdent has quit IRC | 20:09 | |
| *** fried_rolls is now known as fried_rice | 20:09 | |
| *** edmondsw has quit IRC | 20:14 | |
| *** edmondsw has joined #openstack-placement | 20:31 | |
| mriedem | leakypipes: screwed up one small thing in the bottom change functional test i added, will fix that, rebase and +W the ones that were already approved | 20:52 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Usages per project and user (v1.8, v1.9) https://review.openstack.org/514646 | 20:52 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: CLI allocation candidates (v1.10) https://review.openstack.org/514647 | 20:52 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: New dict format of allocations (v1.11, v1.12) https://review.openstack.org/542819 | 20:52 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Transactionally update allocations (v1.13) https://review.openstack.org/546674 | 20:52 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Add nested resource providers (v1.14) https://review.openstack.org/546675 | 20:52 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Limit allocation candidates (v1.15, v1.16) https://review.openstack.org/548043 | 20:52 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Allocation candidates parameter: required (v1.17) https://review.openstack.org/548326 | 20:52 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: New dict format of allocations (v1.11, v1.12) https://review.openstack.org/542819 | 20:57 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Transactionally update allocations (v1.13) https://review.openstack.org/546674 | 20:57 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Add nested resource providers (v1.14) https://review.openstack.org/546675 | 20:57 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Limit allocation candidates (v1.15, v1.16) https://review.openstack.org/548043 | 20:57 |
| openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Allocation candidates parameter: required (v1.17) https://review.openstack.org/548326 | 20:57 |
| leakypipes | mriedem: cool with me, thanks man. | 21:10 |
| *** e0ne has quit IRC | 21:13 | |
| *** e0ne has joined #openstack-placement | 21:15 | |
| *** nicolasbock has quit IRC | 21:18 | |
| *** e0ne has quit IRC | 21:46 | |
| *** mriedem has quit IRC | 22:35 | |
| openstackgerrit | Merged openstack/osc-placement master: Usages per project and user (v1.8, v1.9) https://review.openstack.org/514646 | 22:41 |
| openstackgerrit | Merged openstack/osc-placement master: CLI allocation candidates (v1.10) https://review.openstack.org/514647 | 22:41 |
| openstackgerrit | Merged openstack/osc-placement master: New dict format of allocations (v1.11, v1.12) https://review.openstack.org/542819 | 22:41 |
| openstackgerrit | Merged openstack/nova master: Remove mox in nova/tests/unit/virt/xenapi/stubs.py https://review.openstack.org/568412 | 23:13 |
| openstackgerrit | Merged openstack/nova stable/queens: Make nova service-list use scatter-gather routine https://review.openstack.org/579135 | 23:20 |
| openstackgerrit | Merged openstack/nova master: Fix placement incompatible with webob 1.7 https://review.openstack.org/579110 | 23:20 |
| openstackgerrit | Merged openstack/nova master: [placement] fix allocation handler docstring typo https://review.openstack.org/579198 | 23:25 |
| *** tetsuro has joined #openstack-placement | 23:41 | |
| *** tetsuro has quit IRC | 23:42 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!