| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-15 | |||
| 14:44:53 | bauzas | dansmith: hmmpppfff, instance.host isn't set on the conductor after we schedule | |
| 14:45:04 | bauzas | I thought it was the case but no | |
| 14:45:15 | bauzas | dansmith: we just use that for calling the right compute service | |
| 14:45:27 | dansmith | right, I'll change | |
| 14:45:30 | bauzas | dansmith: AFAICS, instance.host is only set when the instance is spawning | |
| 14:45:34 | dansmith | yeah, yeah | |
| 14:45:57 | bauzas | yeah we unfortunately need to do that change at the same time or we would enter the magical world of race conditions | |
| 14:46:18 | bauzas | but but | |
| 14:46:22 | bauzas | wait | |
| 14:46:41 | dansmith | huh? | |
| 14:46:45 | bauzas | if we need to lookup the aggregate for knowing where the host is | |
| 14:46:59 | bauzas | and that being in the compute section, it would be an upcall right? | |
| 14:47:01 | dansmith | if we haven't build the instance yet, we can just set it | |
| 14:47:18 | dansmith | we know where we're intending for it to go, and if it reschedules, we change it again | |
| 14:48:31 | bauzas | I'm not opposed to that, I'm just saying there is an amount of time where we could show a result where the instance is part of an AZ while it's not yet having an host | |
| 14:48:47 | dansmith | yeah, so? | |
| 14:48:56 | dansmith | also, can we not use this? https://github.com/openstack/nova/blob/master/nova/objects/service.py#L141-L141 | |
| 14:49:14 | dansmith | the host actually knows what AZ it's in right? | |
| 14:51:04 | dansmith | hmm, maybe that's different | |
| 14:51:36 | bauzas | mmm | |
| 14:51:56 | bauzas | I just discover that | |
| 14:52:01 | bauzas | oh wait | |
| 14:52:34 | bauzas | yeah that's because the services are within AZs, not the compute nodes | |
| 14:52:55 | dansmith | but the api hides that az, so it must be different | |
| 14:52:58 | bauzas | so, yeah, service.az should be accurate | |
| 14:53:25 | dansmith | would it? I don't actually see where we're setting that on service | |
| 14:53:31 | bauzas | dansmith: lemme rephrase, if binary is nova-compute, then it should be accurate | |
| 14:56:21 | bauzas | FWIW, I'm digging that | |
| 14:59:52 | bauzas | dansmith: so we update the service AZ in the AZ helper module https://github.com/openstack/nova/blob/master/nova/availability_zones.py#L89 | |
| 15:00:21 | bauzas | which is mostly called in two places | |
| 15:00:29 | bauzas | one when you want to get the Service object | |
| 15:01:03 | bauzas | and one in the nova manage command when you list all the hosts | |
| 15:02:44 | bauzas | the only problem that I see is that we don't persist the Service object which is populated when we call the above | |
| 15:03:16 | bauzas | but if you ServiceList.get_all(), you'll get the right value for the AZ | |
| 15:03:25 | mriedem | jaypipes: replied to the replies, now reviewing the RT tests | |
| 15:06:04 | dansmith | bauzas: that's exactly my point, we can't trust it on Service and we can't get it by get_all() from inside the cell | |
| 15:06:09 | dansmith | bauzas: because it will call out to aggregates | |
| 15:12:45 | bauzas | dansmith: yeah, yet another upcall :( | |
| 15:13:14 | dansmith | bauzas: it seems to me that this is already partially broken and just making instance.az be correct is the solution | |
| 15:13:27 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: guest.BlockDevice should only use the target device of a disk https://review.openstack.org/402163 | |
| 15:15:25 | cfriesen | question...how is the "task_log" table intended to be pruned? Is this intended to be managed by the cloud operator? | |
| 15:17:48 | bauzas | dansmith: well, it would certainly require more than what I originally thought | |
| 15:18:06 | bauzas | dansmith: one way could be to set instance.host in the conductor | |
| 15:18:14 | dansmith | bauzas: right that's what I'm doing | |
| 15:18:15 | bauzas | not by the compute | |
| 15:18:18 | dansmith | no, not host | |
| 15:18:20 | dansmith | instance.az | |
| 15:18:24 | dansmith | we don't need to set host in conductor, | |
| 15:18:28 | dansmith | and that breaks all kinds of stuff | |
| 15:21:35 | bauzas | dansmith: well, it could work given https://github.com/openstack/nova/blob/master/nova/availability_zones.py#L163-L169 | |
| 15:21:43 | cfriesen | another database question....how is stale information in the API DB supposed to be cleaned up to prevent indefinite DB growth? | |
| 15:22:22 | bauzas | cfriesen: we just discussed that, and we're trying to identify all stale contents | |
| 15:22:22 | dansmith | bauzas: I'm not sure what that has to do with it, but setting instance.host in conductor breaks everything (else) so not an option | |
| 15:22:37 | bauzas | dansmith: yeah, lemme rephrase | |
| 15:23:29 | bauzas | dansmith: I'm considering the time between when we set instance.az after a schedule, and when we set instance.host as a temporary situation where we return something on the API that could be different | |
| 15:24:38 | bauzas | dansmith: because we only return the "real" AZ where the host is only only when the compute is setting instance.host | |
| 15:24:59 | bauzas | before that, we return the original instance.az | |
| 15:25:02 | bauzas | anywaty | |
| 15:25:10 | bauzas | let's not overthink that | |
| 15:25:15 | bauzas | do your change and I'll weight it | |
| 15:25:22 | dansmith | it's not wrong in that cast anyway | |
| 15:25:24 | dansmith | *case | |
| 15:25:25 | dansmith | yeah | |
| 15:28:35 | dansmith | gdi, we have too many tests | |
| 15:33:07 | openstackgerrit | Mikhail Feoktistov proposed openstack/nova master: Add is_vz_container function https://review.openstack.org/445947 | |
| 15:34:25 | mriedem | jaypipes: jroll: JayF: cdent: edleafe: ok, final comments are in https://review.openstack.org/#/c/441544/ | |
| 15:34:33 | mriedem | i'm -1 for several issues in test_resource_tracker | |
| 15:37:22 | jroll | mriedem: cool, ty | |
| 15:40:53 | cdent | mriedem: since you're in a reviewing state of mind, getting these from edleafe would clear up some lose ends: https://review.openstack.org/#/c/428800/ https://review.openstack.org/#/c/416752/ https://review.openstack.org/#/c/416751/ (related to inventory handling) | |
| 15:42:06 | mriedem | gonna have to queue up | |
| 15:43:56 | johnthetubaguy | heh, you should ask my wife, she is really picky about queues | |
| 15:45:01 | cdent | my recollection is that the use of "up" there is incorrect and would require a forfeit of some kind. Either running around the quad naked or having to get the next two rounds in, depending on your class. | |
| 15:46:31 | dansmith | bauzas: https://github.com/openstack/nova/blob/master/nova/conductor/tasks/migrate.py#L49-L55 | |
| 15:46:39 | dansmith | bauzas: that ignores the stored az I think :) | |
| 15:46:47 | bauzas | FWIW, using "queue" in French has a terrible meaning | |
| 15:47:32 | bauzas | dansmith: yet another reason to fix that then | |
| 15:47:41 | dansmith | yeah | |
| 15:47:52 | bauzas | those AZ issues are so fun | |
| 15:48:31 | bauzas | dansmith: FWIW, the only nova thing which cares about AZs is just the AZ filter | |
| 15:48:53 | bauzas | if you don't use it, that's just a nice but useless metadata | |
| 15:48:53 | openstack | Launchpad bug 1672624 in OpenStack Compute (nova) "Ceph volumes attached to local deleted instance could not be correctly handled" [Undecided,New] | |
| 15:48:53 | mdbooth | lyarwood johnthetubaguy: re https://bugs.launchpad.net/nova/+bug/1672624 if this were BZ I'd close it NOTABUG. | |
| 15:49:34 | mdbooth | But not because it's the operator's fault, just because it's an ephemeral fault outside Nova's control which will eventually resolve itself automatically. | |
| 15:50:02 | bauzas | dansmith: oh wait | |
| 15:50:20 | bauzas | dansmith: we need to keep the original AZ | |
| 15:50:23 | bauzas | for the Spec object | |
| 15:50:40 | bauzas | dansmith: ie. not having the Spec.az mentioning the "real" AZ | |
| 15:51:24 | dansmith | I'm not changing that | |
| 15:51:39 | bauzas | dansmith: but if you boot an instance, it will change instance.az | |
| 15:51:54 | dansmith | and when you finish the cleanup of the spec object usage, you'll get the original az at that point | |
| 15:52:35 | bauzas | yeah, we need to lookup the spec | |
| 15:52:45 | bauzas | not hydrating from the instance | |
| 15:52:55 | dansmith | bauzas: which is the cleanup step, no? | |
| 15:52:58 | dansmith | the TODO(bauzas) | |
| 15:53:03 | bauzas | yup | |
| 16:06:12 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Remove \r\n line endings from JSON sample https://review.openstack.org/446040 | |
| 16:06:13 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Duplicate JSON line ending check to pep8 https://review.openstack.org/446041 | |
| 16:07:48 | cdent | rfolco: I don't think I'm understanding your question on the DELETE stuff: "Question: Do you think we need a test for 404 and another one for nop (where microversion matches) ?" | |
| 16:09:14 | sfinucan | Could I get someone to look at the above ^ Not possible to build docs locally while things remain this way | |
| 16:11:15 | dansmith | bauzas: explain to me why it makes sense to boot an instance into az foo, land it in az bar, and then resize to foo? | |