| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-15 | |||
| 13:29:50 | dansmith | gibi: can you look at this? https://review.openstack.org/#/c/445697/ | |
| 13:30:13 | dansmith | gibi: I found another case where we were lazy-loading that from an instance on compute and so I think we probably want to go a little more aggressive on it | |
| 13:32:25 | jaypipes | rfolco: quick request on https://review.openstack.org/#/c/444497/. if you could handle that, I'll re-review quickly. | |
| 13:33:02 | jaypipes | rfolco: besides that, I think we should be able to merge your two patches in that series today. | |
| 13:33:22 | rfolco | jaypipes.requests --> priority -1 | |
| 13:33:27 | cdent | i concur, dr pipes | |
| 13:33:39 | jaypipes | rfolco: :) | |
| 13:34:22 | stvnoyes1 | i was writing a tempest test for swap disk (update volume attachment). The test runs ok on a running server, but I get a libvirt exception (Domain not found: no domain with matching uuid) when run against a stopped server. Is swap not supported on a stopped server? I didn't see anything in the doc about a restriction like that. thanks... | |
| 13:36:08 | mdbooth | stvnoyes1: It is not supported, no. | |
| 13:36:41 | jaypipes | stvnoyes1: when you say "you get a libvirt error", are you saying that that libvirt error is returned to you as a *user*, or that you see that error in the nova-compute log file? | |
| 13:36:56 | stvnoyes1 | in the log | |
| 13:37:14 | mdbooth | johnthetubaguy: ^^^ Incidentally, what stvnoyes1 said is why I was talking about the 'did_copy' response in the swap_volume callback to cinder | |
| 13:38:07 | johnthetubaguy | mdbooth: ack, makes sense, but we can't fix everything in one go | |
| 13:38:35 | mdbooth | johnthetubaguy: Yep, but if we could stick it in the api then we can fix it later more easily. | |
| 13:38:42 | mriedem | jaypipes: if you haven't seen yet, i had some questions in your ironic get_inventory patch | |
| 13:38:50 | mriedem | only thing i have left to review there is the RT functional test | |
| 13:38:57 | jaypipes | mriedem: yup, thanks much for your review and questions. answering now. | |
| 13:39:59 | mriedem | fwiw dansmith explained a bit of earlier conversations to me last night | |
| 13:40:30 | jaypipes | mriedem: dansmith is good like that. :) | |
| 13:43:34 | openstackgerrit | Dan Smith proposed openstack/nova master: Avoid lazy-loading projects during flavor notification https://review.openstack.org/445697 | |
| 13:48:14 | mriedem | johnthetubaguy: you were +2 on this before the cleanups https://review.openstack.org/#/c/429569/ | |
| 13:50:25 | mriedem | bauzas: unless you have a reason not to, i'm going to open a bug about the fact that we never delete request_specs when we delete an instance, | |
| 13:50:28 | mriedem | i'm not sure if that's by design | |
| 13:51:08 | mriedem | maybe i'll start in the ML | |
| 13:51:15 | mriedem | since we wouldn't want to do this for soft delete | |
| 13:53:41 | dansmith | mriedem: we could also do with a bug to clean up all the conversion back and forth to/from the legacy version that never got cleaned up | |
| 13:53:45 | stvnoyes1 | jaypipes: i was interested to see what would happen doing a swap to a stopped vm with the cli. the cli command returned without an error. The volume status of both vols were unchanged. And there was that Domain not found error in the logs. I found a similar non-error using the API, the test only failed because the new volume never became 'in-use'. | |
| 13:53:47 | bauzas | mriedem: I have a branch for that | |
| 13:54:14 | bauzas | mriedem: that's one of the things I have in my plate and never had time for that in Ocata | |
| 13:54:17 | mriedem | dansmith: yeah bauzas has a blueprint for that from ocata, | |
| 13:54:29 | mriedem | i don't remember seeing anything about actually deleting these things though | |
| 13:54:34 | bauzas | mriedem: hence me planning to fix it by Pike | |
| 13:54:41 | mriedem | i'll brain dump in the ML | |
| 13:54:43 | bauzas | mriedem: https://review.openstack.org/#/c/391060/ | |
| 13:54:45 | bauzas | dansmith: ^ | |
| 13:54:51 | mriedem | ah | |
| 13:55:12 | bauzas | it requires a rebase | |
| 13:55:47 | jaypipes | stvnoyes1: gotcha. would you expect instead to have an "unsupported operation on stopped server" error message return instead of no error? | |
| 13:56:51 | mriedem | bauzas: left my concern inline, but that potentially cripples a restored soft deleted instance, | |
| 13:57:04 | bauzas | mriedem: cool, thanks | |
| 13:57:11 | mriedem | i'm not sure how we deal with that, since we don't want the compute doing a call back up to the API to delete the request spec when the instance is hard deleted | |
| 13:57:25 | bauzas | mriedem: the problem is that we never agreed on soft-deleting the Spec with alaski | |
| 13:57:40 | dansmith | he means soft delete of the instance | |
| 13:57:45 | bauzas | yeah | |
| 13:57:45 | dansmith | (right?) | |
| 13:57:49 | dansmith | not soft delete of the instance db record | |
| 13:57:49 | bauzas | I understand that | |
| 13:57:55 | bauzas | sure | |
| 13:58:12 | bauzas | but if we trigger the Spec hard deletion on an instance soft-delete | |
| 13:58:20 | stvnoyes1 | jaypipes: agree, it would be better to get some kind of error. since a server can stop at any time, doing a check in the API code would help a lot but it wouldn't cover all the cases. | |
| 13:58:26 | bauzas | then, how could we restore it if we restore the instance ? | |
| 13:58:34 | dansmith | you don't restore it, | |
| 13:58:49 | dansmith | you have to defer deletion of it until the soft-deleted instance is purged somehow | |
| 13:58:54 | dansmith | although that happens on the compute node | |
| 13:59:04 | bauzas | yeah, the trigger is then totally different | |
| 13:59:33 | bauzas | we could keep the spec record until the instance is hard-deleted | |
| 13:59:41 | bauzas | but I need to see where we trigger that | |
| 13:59:45 | dansmith | right, so you have to just not delete it in that case, and either leak it (as we do now for everything) or at least delete if you hard-delete | |
| 14:00:05 | bauzas | dansmith: you said we hard-delete on the compute side ? | |
| 14:00:14 | bauzas | a periodic I guess ? | |
| 14:00:21 | dansmith | compute is what does the purge yeah, in a periodic | |
| 14:00:30 | mriedem | dansmith: right | |
| 14:00:37 | mriedem | well, SOFT_DELETED in the API sense | |
| 14:00:39 | mriedem | not the DB sense | |
| 14:00:40 | bauzas | okay, that would ease my branch :) | |
| 14:01:11 | bauzas | IIRC, we already merged the .delete() method I added in Newton | |
| 14:01:23 | bauzas | the caller of that .delete() never merged in time in Newton | |
| 14:01:24 | mriedem | which delete() method? | |
| 14:01:28 | bauzas | the Spec one | |
| 14:01:36 | mriedem | yes RequestSpec.destroy() exists, | |
| 14:01:38 | mriedem | but nothing calls it | |
| 14:01:45 | bauzas | exactly | |
| 14:01:53 | bauzas | because I wrote the destroy() | |
| 14:01:58 | bauzas | it was in the same series | |
| 14:01:59 | mriedem | so i think we're left with leaking request specs for soft-deleted instances for now i guess | |
| 14:02:04 | bauzas | yup | |
| 14:02:11 | bauzas | lemme clarify | |
| 14:02:26 | bauzas | in Newton, I discovered that (because I missed that when I reviewed alaski's persistence series) | |
| 14:02:29 | mriedem | we could provide a nova-manage command to purge those if you wanted... | |
| 14:02:37 | bauzas | so I wrote a series | |
| 14:02:45 | bauzas | .destroy() as a bottom patch | |
| 14:02:49 | bauzas | the bottom merged | |
| 14:02:51 | dansmith | mriedem: we have InstanceMappings to purge in that case as well | |
| 14:02:57 | bauzas | but not the top one | |
| 14:03:13 | bauzas | and in Ocata, I had that in my plate, but the placement things diverted me | |
| 14:03:13 | mriedem | dansmith: nova-manage api_db purge_zombies | |
| 14:03:28 | bauzas | so, we're now in Pike and it's still in my plate :) | |
| 14:03:36 | mriedem | eat faster | |
| 14:03:42 | dansmith | mriedem: aye | |
| 14:04:17 | bauzas | mriedem: dansmith: I wrote a db online migrations that was creating the spec records for old instances, could we envisage some hack for purging ? | |
| 14:04:40 | bauzas | ie. we trigger deletes on the periodic task | |
| 14:04:54 | bauzas | but we could also write a db online migration script that would do that | |
| 14:05:19 | dansmith | we shouldn't abuse online migrations for periodic cleanup | |
| 14:05:21 | dansmith | IMHO | |
| 14:05:32 | bauzas | okay | |
| 14:05:34 | bauzas | fine by me | |
| 14:05:40 | bauzas | I'll revisit that change | |
| 14:06:04 | bauzas | if we have a periodic, do we still need a purge command ? | |
| 14:06:15 | bauzas | dansmith: mriedem^ | |
| 14:06:20 | dansmith | we can't have a periodic, IMHO | |