Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-09
14:55:07 cdent jaypipes: noted!
14:55:27 coreycb cdent, thanks we'll get that fixed up asap
14:55:36 edleafe jaypipes: queued
14:55:42 jaypipes danke
14:57:31 jaypipes mdbooth: you previously reviewed https://review.openstack.org/#/c/399679/. would be great to have a once over from you on that.
14:57:57 mdbooth jaypipes: Sure.
14:58:03 jaypipes cheers
14:58:55 asettle johnthetubaguy: further explanation, package managers test often with automatic setup and that might include updating config files
14:58:57 mdbooth jaypipes: Incidentally, I was looking for validation of this approach for handling RequestContexts for cells: https://review.openstack.org/#/c/443627/
14:59:14 mdbooth Or invalidation, obviously, which is why I haven't tried to do the whole job yet.
14:59:25 jaypipes mdbooth: k, queued.
14:59:37 mdbooth jaypipes: Thanks
15:01:30 openstackgerrit Gábor Antal proposed openstack/nova master: Add json style checking for sample notifications https://review.openstack.org/443677
15:03:05 dansmith mdbooth: I skimmed it briefly
15:03:28 dansmith mdbooth: I think we can go with that approach for things for object contexts, where we iterate over multiple cells
15:03:36 dansmith mdbooth: I don't like it for clearly single-cell-targeted operations on an instance or a host
15:04:03 dansmith mdbooth: because after getting all of this to work, I feel confident we'll have some leaks where we don't realize that we've now got two copies of the context
15:04:06 mdbooth dansmith: Ok. So it's worth adding context.for_cell() and fixing up say get_all() and its users?
15:04:12 dansmith mdbooth: the context _is_ gobal and by reference everywhere
15:04:21 dansmith mdbooth: we get it by reference from the request
15:05:31 dansmith mdbooth: maybe, I also would want to see that change on top of a fully working tempest run of the current stack to prove it doesn't break anything else
15:05:59 jaypipes sfinucan: so https://review.openstack.org/#/c/399653/is going to require the gate to have libvirt 1.2.9
15:06:01 mdbooth dansmith: That was another reason I didn't want to go all the way down this rabbit hole. I expect there are a bunch of tricky cases.
15:06:12 dansmith mdbooth: there are a ton
15:06:31 mdbooth dansmith: However, I was hoping that by removing the whole side-effect of changing the global context there might be significantly less
15:06:41 dansmith mdbooth: I think you might be surprised
15:07:03 sfinucan jaypipes: I thought the point of those tests was to fake out libvirt?
15:07:12 mdbooth dansmith: Ok, I'll limit to get_all() initially and see how it looks.
15:07:14 mdbooth Thanks for looking.
15:07:28 jaypipes sfinucan: see the functional test failure :)
15:07:30 cdent coreycb: I think I've got another one. Is it considered a bug for the service.conf files to have uncommented config settings?
15:08:11 coreycb cdent, not necessarily. some of the packages set defaults.
15:08:24 sfinucan jaypipes: Well that doesn't look right :D
15:08:36 cdent coreycb: in this case it is that os_region_name in the [placement] section is being set
15:08:50 sfinucan Guess we can mock out that version check too
15:08:55 dansmith mdbooth: fwiw, we already had a discussion about whether or not to fork the context in the objects that are tied to cells and decided on this approach to start
15:09:09 dansmith mdbooth: one thing to remember is that if you send an object over rpc, it resets the context on all the objects in the stack
15:09:25 dansmith mdbooth: so if you make a call over to conductor, for anything like boot or resize, contexts get reset anyway
15:09:33 coreycb cdent, that may not need to be set. we set defaults so that the service will can start after the package is installed.
15:10:13 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Temporarily untarget context when deleting from cell0 https://review.openstack.org/443403
15:10:21 cdent coreycb: the reason I noticed was because the docs I was testing changed it, i made all those changs at the top of the section, and then realized it was going to conflict with something that's set further below
15:10:47 cdent coreycb: ah, okay, yeah, that makes sense then
15:10:53 cdent it won't start without that
15:10:57 cdent (there's no default)
15:11:42 mriedem dansmith: ftr, i did give you the business, yes
15:11:44 mriedem :)
15:11:55 dansmith mriedem: yeah, I just totally wasn't paying attention when I wrote that :)
15:11:59 dansmith mriedem: see my rev
15:12:00 dansmith ?
15:13:02 mriedem yes
15:13:08 mriedem wait, what?
15:13:26 mriedem oh i see
15:13:53 dansmith actually, I have to fix target_cell too
15:13:58 mriedem dansmith: you missed on
15:13:59 mriedem *one
15:14:00 dansmith oh no
15:14:01 dansmith it works
15:14:07 mriedem you missed one in the exception block
15:14:09 dansmith ah yeah
15:14:13 dansmith just a sec
15:14:31 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Temporarily untarget context when deleting from cell0 https://review.openstack.org/443403
15:14:48 openstackgerrit Balazs Gibizer proposed openstack/nova master: Using max api version in notificationt sample test https://review.openstack.org/443686
15:15:32 coreycb cdent, ok. we're considering loosening the restriction of having to start services by default because it can cause issues particularly with deployment tools.
15:15:54 dansmith mriedem: so we should be able to write a functional test for this that checks the quota in each after delete right?
15:15:56 coreycb cdent, but we haven't done that yet. if we did i think we'd just use the default config files.
15:16:20 cdent coreycb: it's a reasonable thing to do for machines being directly used by humans but not so much in this case :)
15:16:40 mriedem dansmith: yes
15:16:55 mriedem that's the plan
15:17:00 coreycb cdent, yeah :)
15:17:00 mriedem the tricky thing is going to be,
15:17:16 mriedem we only need one of these fixes for the backport to ocata
15:17:24 mriedem so the functional test won't pass until the top patch in pike
15:17:29 mriedem but it will work for the bottom patch in ocata
15:17:42 mriedem but re-ordering the changes in master doesn't make sense to me
15:17:49 dansmith okay
15:18:02 mriedem so i'm thinking maybe the functional test is in a separate patch
15:18:03 mriedem not sure
15:18:09 dansmith yeah
15:22:06 mriedem so i think the easy thing i can do first is create a functional regression patch that goes at the bottom of the series and recreates the failure, and asserts the error,
15:22:10 mriedem then the fix patches on top of that,
15:22:31 mriedem and the top fix patch changes the functional test to show it working
15:22:35 mriedem that was my plan anyway
15:22:39 dansmith I was just looking at the tests
15:22:44 dansmith give me a few minutes
15:23:00 dansmith we don't really have a way to query into the reservation stuff to look, AFAICT, but I think I can hack something in at least to check
15:23:09 mriedem well,
15:23:16 mriedem for a functional test i think this is pretty straightforward,
15:23:19 mriedem we have cell1 and cell0,
15:23:25 mdbooth dansmith: Incidentally, https://review.openstack.org/#/c/443403/ is pretty much what I was talking about with the side-effect thing.
15:23:29 mriedem we disable nova-compute or do something to make scheduling fail,
15:23:39 mriedem create the server and show it goes into ERROR state,
15:23:47 dansmith mdbooth: it's not at all, since this is a problem across multiple requests
15:23:49 mriedem assert we have quota of instances=9 (default-1)
15:23:55 mriedem then delete the instance,
15:24:03 mriedem and assert that quota of instances=10 (but it won't be until the fix)
15:24:12 mriedem it's basically the same recreate steps as in the bug report
15:24:35 dansmith mriedem: I guess I'm lost in the db api on terminology, knowing which things are returning limits and which are calculated by limit and usage
15:24:44 dansmith mriedem: but yeah, that's what I'm doing
15:24:48 mriedem the functional test just hits the rest api
15:24:54 mriedem like novaclient would

Earlier   Later