Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-15
14:30:50 dansmith it looks to me like we use instance.az in several other places
14:31:20 bauzas dansmith: for the metadata, yes
14:31:27 dansmith no, for other things too
14:31:36 dansmith nova/network/neutronv2/api.py: zone = 'compute:%s' % instance.availability_zone
14:31:38 bauzas dansmith: for other places, AFAICT, I think we return that too
14:31:42 dansmith nova/virt/block_device.py: return instance.availability_zone
14:32:03 dansmith is having instance.az being incorrect not affecting those places as well?
14:32:11 bauzas yup
14:32:13 dansmith like, shouldn't making it be the actual az the instance is on be better?
14:32:37 bauzas yeah, as I said, I agree, we should update instance.az to the right value
14:32:56 bauzas and use spec_obj.availability_zone for knowing the original request
14:33:14 dansmith okay
14:33:18 dansmith grim
14:33:20 dansmith oops
14:33:50 dane-fichter mriedem: could you unblock this change, please? https://review.openstack.org/#/c/351232/
14:34:09 jaypipes mriedem: so, is there anything on the ironic virt driver patch you'd like me to change?
14:35:01 openstackgerrit Rafael Folco proposed openstack/nova master: [placement] Raising http codes on old microversion https://review.openstack.org/444497
14:35:01 openstackgerrit Rafael Folco proposed openstack/nova master: DELETE all inventory for a resource provider https://review.openstack.org/416669
14:35:04 mriedem dane-fichter: done, left a comment
14:35:08 mriedem jaypipes: let me check your replies
14:37:14 dansmith bauzas: do I really need elevated context here/ https://github.com/openstack/nova/blob/67af1eefc5c1cf8cde4b4becb933808850acec2d/nova/availability_zones.py#L190
14:38:09 jaypipes mriedem: I can add a code comment about raising an exception if resource class is None in Queens, if you'd like.
14:38:46 mriedem jaypipes: let me look first, might be just follow up stuff
14:38:53 mriedem doing a few things at once
14:40:27 bauzas dansmith: I left a proposal in https://review.openstack.org/#/c/439754/23 by saying we could amend instance.az once we schedule a target
14:40:43 bauzas every time we touch instance.host actually
14:41:03 dansmith bauzas: this is what I'm testing right now: http://pastebin.com/raw/iWBb9wt5
14:41:31 dansmith bauzas: for the old and new build operations
14:41:44 bauzas dansmith: for the elevated context, I think it's a legacy from the past
14:41:54 dansmith bauzas: okay I thought so too just checking
14:41:59 bauzas dansmith: now we use objects, we don't really need elevated
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)

Earlier   Later