Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-09
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 mriedem the tricky thing is going to be,
15:17:00 coreycb cdent, yeah :)
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
15:25:04 mriedem i really don't want to be going into the actual databases
15:25:32 mriedem i was going to do this in nova/tests/functional/regressions
15:25:54 dansmith mriedem: well, you're not really validating that exactly what you think is happening really is, but okay
15:26:09 dansmith I'll leave it to you then
15:26:36 mriedem as long as the regression test asserts the failure (the recreate), and the fix has to change the test to show it now passing, i think that's what we care about
15:27:02 dansmith I just want to see them in the wrong database, but okay
15:27:33 dansmith the quota stuff is so opaque to me that I'd want to see that we're out of sync for the reason we think and not one of the other reasons quotas gets out of sync
15:27:46 dansmith but that's fine, you're far more familiar, so go for it
15:28:04 mriedem why did you change set_target_cell?
15:28:09 mriedem we're calling target_cell and passing None
15:28:35 dansmith target_cell calls set_target_cell
15:29:42 mriedem gotcha
15:32:05 johnthetubaguy mdbooth: I am looking at your comments on migrate_volume_completion
15:32:30 mdbooth johnthetubaguy: I spent half a day or so reading the cinder side code for that, btw
15:32:32 johnthetubaguy mdbooth: I don't feel like I am understanding what that is trying to do, and if it should die/get replaced in this new model
15:32:43 mdbooth I'll never get that back
15:32:50 johnthetubaguy yeah, I did that about 6-8 months ago, and its wondered out my head again
15:33:36 mdbooth johnthetubaguy: I think it needs to continue to exist, because cinder needs to know that the copy is complete so it can cleanup the old volume.
15:34:01 johnthetubaguy mdbooth: yeah, we totally need that bit
15:34:04 mdbooth However, I suspect it could similarly exist in a new form
15:34:09 johnthetubaguy yeah, that
15:34:56 mdbooth johnthetubaguy: It would be great if the new form included a flag for 'we didn't do the copy'
15:35:21 mdbooth So that in cases where the driver can't do it, eg because the instance is stopped, cinder knows it needs to do it
15:35:26 mdbooth And we don't need to fail the operation
15:36:23 johnthetubaguy mdbooth: my problem is how do we know what to do with the attachments, does it alway delete the correct attachment for us?
15:37:05 mdbooth Well cinder will have its own migration record which it hasn't exposed to Nova, which iirc is how it decides which 'mode' it's in
15:37:14 mdbooth Can cinder just work it out?
15:37:38 johnthetubaguy mdbooth: I think it knows, I just wonder if it always does the same thing from our view of the world
15:37:57 johnthetubaguy like it always deletes the correct attachment
15:38:05 johnthetubaguy so we don't need to delete any attachments
15:38:29 mdbooth johnthetubaguy: We say: we're done with the swap volume of att_A to att_b for instance X, and we did the copy
15:38:53 mdbooth That's enough info for cinder in all cases, right?
15:39:05 mdbooth And I don't think we need a return value any more
15:39:09 johnthetubaguy what I mean is, what do we have to do?
15:39:14 mdbooth Because we don't need to do connection_info munging
15:39:27 johnthetubaguy cinder gives us the volume-uuid of the finally attached volume to update in the BDM
15:39:31 mdbooth I think the only thing Nova has to do is callback cinder with ^^^
15:39:34 johnthetubaguy maybe we just don't delete any attachments
15:40:01 mdbooth It actually occurred to me that all we really needed to do was delete the attachment
15:40:05 mdbooth the source attachment, that is
15:40:11 mdbooth And that's actually enough info for cinder
15:40:24 mdbooth However, I vastly prefer to be explicit
15:40:42 mdbooth I think we delete the attachment, then make the callback
15:40:47 gibi dansmith, melwitt: Hi! could you please look at the bugfix of the instance.delete regression? https://review.openstack.org/#/c/437222/
15:40:48 johnthetubaguy Oh, true, cinder could delete the volume if it knows it has to
15:41:08 johnthetubaguy but something nice about migrate being explicit, in case that gets messed up
15:41:13 mdbooth Yeah
15:41:19 asettle jaypipes: thanks for reviewing the doc :) were you able to test or just take a look?
15:41:34 johnthetubaguy mdbooth: I have no idea how this could work with multi-attached volumes?? who does the delete?
15:41:40 jaypipes asettle: lol, just on a hangout with cdent talking about that right now :)
15:41:54 jaypipes asettle: apparently there *is* an openstack-nova-api-placement package...
15:41:56 asettle jaypipes: wow you guys have all the fun don't you :p
15:42:00 mdbooth johnthetubaguy: Whoa, I hadn't even considered that
15:42:06 mdbooth johnthetubaguy: Who does the copy?
15:42:13 asettle jaypipes: oh. so. what's next?
15:42:31 jaypipes asettle: well, other than that one comment from me, I think all my other comments were correct ;)
15:43:15 johnthetubaguy mdbooth: everyone? oh dear... yeah.
15:43:33 mdbooth johnthetubaguy: Ok, I think that problem can be solved if we add a 'please do the copy' flag to nova's swap_volume call
15:43:53 asettle jaypipes: yeah just reading htem all now
15:43:53 mdbooth I think cinder then has enough tools in the box to do this
15:44:04 mdbooth Hmm, no, you'd need a quiesce operation too

Earlier   Later