| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-21 | |||
| 18:00:25 | mriedem | correct, it sucks | |
| 18:00:36 | mriedem | the mox removal series is a prime example | |
| 18:00:45 | mriedem | it also causes issues with merge conflicts for backports | |
| 18:00:53 | sean-k-mooney | well perhaps this is jsut somethin we need to be aware of and think should we be moving things out of the driver into modules used by the driver in the future | |
| 18:01:04 | aspiers | efried: no, blame really can show where it came from | |
| 18:01:25 | sean-k-mooney | aspiers: yes but git blame on github crashes on those files | |
| 18:01:29 | sean-k-mooney | so you have to do it locally | |
| 18:01:43 | aspiers | ? | |
| 18:01:49 | efried | what's the blame trick, without iterating backward across commits? | |
| 18:01:58 | aspiers | mriedem's point about backports is a much more convincing argument against | |
| 18:02:19 | aspiers | however, if our policy is "you can't move stuff around, ever" then we're in for a miserable future | |
| 18:02:50 | mriedem | i'm not against refactoring giant methods in the libvirt driver module, deduplicating test code in test_driver, etc. i just don't want to see random tests moving out of test_driver into some other module when the code being tested is still in driver.py | |
| 18:03:20 | aspiers | mriedem: sure, that's why I said <aspiers> mriedem: I would happily move the corresponding code out from driver.py at the same time if that was feasible | |
| 18:03:21 | mriedem | aspiers: no one said that was the policy | |
| 18:03:24 | sean-k-mooney | aspiers: try opening https://github.com/openstack/nova/blame/master/nova/virt/libvirt/driver.py | |
| 18:03:36 | aspiers | mriedem: ok good :) | |
| 18:03:50 | aspiers | was beginning to sound that way | |
| 18:04:19 | mriedem | as a nova troll i have a reputation to maintain | |
| 18:04:26 | aspiers | :) | |
| 18:04:27 | mriedem | as immovable object | |
| 18:04:28 | sean-k-mooney | i think we just dont want test to be move with out the code also moving and am not sure that patches that only move code are a good idea | |
| 18:04:37 | aspiers | efried: try git gui blame for instance | |
| 18:04:55 | aspiers | some tooltips include "Copied Or Moved Here By:" | |
| 18:05:12 | aspiers | can't remember if github or gitea do the same | |
| 18:05:31 | sean-k-mooney | aspiers: i think i makes sense to consider moving things if you are refacoritn the code already. | |
| 18:05:32 | aspiers | but local is way faster anyway IME | |
| 18:05:39 | aspiers | sean-k-mooney: definitely agree with that | |
| 18:06:02 | aspiers | well, it's an idea to bear in mind anyway | |
| 18:06:28 | aspiers | 23k LoC in one file does not seem sane to me ;-) | |
| 18:07:14 | sean-k-mooney | 23K of code to test 9.75K of code and we still miss things... | |
| 18:07:43 | sean-k-mooney | the driver is not that bad coverage wise but ther are other part that are less awsome... | |
| 18:08:17 | efried | aspiers: git gui appears to be... a GUI :( :( :( | |
| 18:08:30 | aspiers | efried: shocker! who'd have thought? | |
| 18:08:33 | efried | guess it would take some getting used to. | |
| 18:09:01 | sean-k-mooney | aspiers: that is not a standard git command is it a python extentions | |
| 18:09:10 | efried | anyway, I agree enormous files suck, and being stuck with them because of sucky reasons also sucks. | |
| 18:09:19 | efried | sean-k-mooney: apt install git-gui | |
| 18:09:40 | efried | that's if you're running ubuntu. | |
| 18:09:47 | efried | If you're running another distro... just shoot yourself and be done. | |
| 18:10:07 | sean-k-mooney | PopOS which is based on ubuntu yes | |
| 18:10:18 | aspiers | efried: watch your mouth! | |
| 18:10:19 | efried | (sorry, I had a bad experience with rhel a decade ago and still haven't recovered) | |
| 18:10:31 | aspiers | you ran RHEL as a desktop? eww | |
| 18:10:34 | aspiers | Fedora is OK | |
| 18:10:34 | sean-k-mooney | although i have a rhel8 system for the last while... not sure i will keep it but it currently works | |
| 18:10:37 | efried | yeah, that was my problem | |
| 18:10:43 | aspiers | try openSUSE :) | |
| 18:10:50 | efried | is that still a thing?? | |
| 18:10:59 | efried | Is it written in FORTRAN? | |
| 18:11:09 | sean-k-mooney | aspiers: i used tumbelweed for a while and mostly liked it | |
| 18:11:13 | aspiers | ok, good time to leave | |
| 18:11:32 | sean-k-mooney | not sure i liked yast but it was nice to have up to date packages | |
| 18:49:40 | mriedem | melwitt: a couple of things in the archive patch https://review.opendev.org/#/c/507486/ | |
| 18:49:46 | mriedem | and a couple of nits in the nova-next patch on top of that | |
| 18:50:06 | mriedem | both should be small | |
| 18:50:52 | mriedem | melwitt: efried: i think we're ok to merge https://review.opendev.org/#/c/675218/6 now yeah? | |
| 18:51:11 | melwitt | thanks | |
| 18:51:49 | melwitt | yeah I think all is well | |
| 18:55:40 | mriedem | onto openstack resource provider inventory set --aggregate | |
| 18:57:35 | melwitt | heh | |
| 19:10:05 | melwitt | I was panicking with the potential loss of my launchpad bug text after a 503 "TimeoutError" but found when I reloaded the page, it posted everything I had written | |
| 19:10:16 | melwitt | phhheeewwww | |
| 19:13:21 | mriedem | happens a lot, just have to keep refreshing | |
| 19:17:34 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add extra spec parameter and image property for memory encryption https://review.opendev.org/664420 | |
| 19:21:23 | openstackgerrit | Adam Spiers proposed openstack/nova master: Extract SEV-specific bits on host detection https://review.opendev.org/636334 | |
| 19:21:34 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add |
|
| 19:22:51 | mriedem | melwitt: i think i have an idea for how to make one of multiple rps fail to set inventory in aggregate in https://review.opendev.org/#/c/640898/11 | |
| 19:22:56 | mriedem | add allocations to one of them | |
| 19:24:04 | openstackgerrit | melanie witt proposed openstack/nova master: Make a failure to purge_db fail in post_test_hook.sh https://review.opendev.org/677806 | |
| 19:25:01 | melwitt | mriedem: nice! thanks, I was stumped about that | |
| 19:25:19 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add extra spec parameter and image property for memory encryption https://review.opendev.org/664420 | |
| 19:26:28 | openstackgerrit | Adam Spiers proposed openstack/nova master: Extract SEV-specific bits on host detection https://review.opendev.org/636334 | |
| 19:26:42 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add |
|
| 19:28:40 | mriedem | melwitt: same, until about 5 minutes ago :) | |
| 19:29:50 | melwitt | \o/ | |
| 19:29:56 | mriedem | looking at the placement code i'm not totally sure if it handles the "drop inventory below what's allocated but don't remove the resource class" or if it's just "can't remove a resource class inventory that has allocations" - i see the latter in the placement code but not sure about the former, but i'm pretty sure it enforces that | |
| 19:29:58 | mriedem | efried: ^ right?! | |
| 19:31:08 | mriedem | https://review.opendev.org/#/c/640898/11 | |
| 19:31:08 | mriedem | hmm | |
| 19:31:09 | efried | mriedem: You can reduce inventory below what's allocated. You can't set total to zero (I'm 95% sure of that, would have to check). | |
| 19:31:10 | mriedem | ops | |
| 19:31:22 | mriedem | efried: not before 1.26 anyway.. | |
| 19:32:09 | mriedem | yeah so if your allocations exceed capacity it only logs a warning | |
| 19:32:10 | mriedem | https://github.com/openstack/placement/blob/master/placement/objects/resource_provider.py#L619 | |
| 19:32:35 | mriedem | if you try to remove a resource class entirely that has allocations it'll fail https://github.com/openstack/placement/blob/master/placement/objects/resource_provider.py#L84 | |
| 19:33:01 | mriedem | so melwitt i guess one of the providers has to have some resource class inventory and a consumer is using that rc, and then in set inventories you drop/omit that rc, | |
| 19:33:05 | mriedem | it could just be some CUSTOM_FOO in your test | |
| 19:33:25 | melwitt | ah ok | |
| 19:33:47 | melwitt | sophisticated | |
| 19:35:22 | mriedem | efried: can't set total to 0 but you can set reserved = total with 1.26 | |
| 19:35:27 | efried | yes | |
| 19:35:41 | efried | we did that specifically so you could take an ironic node out of the pool | |
| 19:35:46 | mriedem | yeah i know | |
| 19:36:04 | mriedem | mel is trying to trigger a failure on one provider's PUT inventories out of a set of providers in an aggregate | |
| 19:36:32 | mriedem | anyway, think it's figured out, i just didn't realize placement would let you do the other thing here https://github.com/openstack/placement/blob/master/placement/objects/resource_provider.py#L619 | |
| 19:36:41 | efried | does it matter what the failure is? | |
| 19:36:50 | efried | You could PUT a resource class that doesn't exist yet | |
| 19:37:20 | melwitt | like a custom class placement doesn't yet know about? | |
| 19:37:23 | efried | yeah | |
| 19:37:40 | melwitt | ahhh that would be easier | |