Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-08
15:31:32 mriedem ok, currently re-reviewing superdan's live migration allocations thing
15:34:00 melwitt sahid: would you be able to take a look at this review where there's a bug with disk device addresses being changed during a live migration? https://review.openstack.org/#/c/518022
15:35:15 melwitt it looks like it's a regression caused by a different bug fix from the past https://review.openstack.org/#/c/459741
15:36:24 openstackgerrit Chris Dent proposed openstack/nova master: [placement] add name to resource provider create error https://review.openstack.org/526710
15:36:55 sahid melwitt: i think i did that this morning
15:37:09 sahid is there something not clear you want to discuss?
15:37:15 melwitt mdbooth: it would be good to have your review on this change for fixing the cache value with regard to O_DIRECT support during an image conversion https://review.openstack.org/#/c/523554
15:38:03 mdbooth melwitt: I vaguely recall having looked at something similar, if not this
15:38:09 melwitt sahid: oh geez, sorry. I didn't see that you reviewed it this morning. I had it on my todo to ask you the past few days and just did it blindly. thanks
15:43:04 openstackgerrit Merged openstack/nova master: Avoid stashed connector lookup for new style detach https://review.openstack.org/526182
15:43:46 melwitt mdbooth: most of the patch is moving the supports_direct_io check to nova/utils.py so that it could be used by both virt/images and libvirt/driver. but the main part of the fix is in here https://review.openstack.org/#/c/523554/6/nova/virt/images.py
15:47:06 mriedem superdan: done
15:47:19 mriedem however, it's an easy one
15:47:30 superdan received
15:50:34 leakypipes mriedem: I'm trying to square your code comment here: https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4591-L4595 with the following in the functional test case setup here: https://github.com/openstack/nova/blob/master/nova/tests/functional/compute/test_host_api.py#L76-L79
15:51:12 leakypipes mriedem: the reason is I have some PoC code I'm working on locally that would allow host aggregates to be associated to compute nodes (via UUID) in addition to compute *services* by service hostname
15:51:19 cdent superdan: not sure if you saw my earlier ping on this potentially cells related bug that needs some evaluation: https://bugs.launchpad.net/nova/+bug/1736101
15:51:20 openstack Launchpad bug 1736101 in OpenStack Compute (nova) "nova placement resource_providers DBDuplicateEntry when name repeat" [Undecided,New]
15:51:51 superdan I did not
15:51:53 mriedem leakypipes: https://github.com/openstack/nova/blob/master/nova/tests/functional/compute/test_host_api.py#L78 is just test data, the hostmapping.host wouldn't be a uuid
15:52:01 mriedem leakypipes: at least, not a uuid that matches the computenode.uuid
15:52:31 leakypipes mriedem: sorry, I'm not following you...
15:52:55 leakypipes mriedem: you're creating a HostMapping with the host attribute of the mapping set to the compute node's UUID.
15:53:02 superdan cdent: so, two hosts with the same name is not a valid thing, but it sounds like they only have that situation because they're moving between cells?
15:53:07 leakypipes mriedem: which is what the code comment in HostsAPI says we're not doing?
15:53:16 mriedem leakypipes: https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4591-L4595 is just saying, we're looking up the compute node (in a cell) by uuid, (microversion >=2.53), if we had the uuid stored in the host mapping (api db), then we wouldn't have to iterate the cells querying each db for a compute node with that uuid
15:53:26 mriedem we could just get the host mapping from the api db which would tell us which cell that compute node is in
15:53:37 cdent superdan: I struggled to parse it and wasn’t entirely clear on the naming constraints
15:53:55 mriedem leakypipes: https://github.com/openstack/nova/blob/master/nova/tests/functional/compute/test_host_api.py#L78 - the hostmapping.host field could be "turd.ferguson"
15:53:56 superdan cdent: the conflict is in the placement db, right?
15:54:07 cdent yes, rp names are supposed to be unique
15:54:17 superdan cdent: the rp name comes from what though?
15:54:18 leakypipes mriedem: ok. so are we adding HostMapping records for compute nodes now or are we not doing that?
15:54:19 mriedem leakypipes: the test is just using something random, which in this case is the uuid that matches the compute node in the cell - confusing the test i guess
15:54:30 leakypipes mriedem: note: compute nodes, not service hosts.
15:54:31 mriedem leakypipes: we are adding host mappings for service hosts
15:54:32 mriedem not compute nodes
15:54:53 cdent superdan: that’s what I’m not clear on. some portion of the host? If it’s not the fqdn, then that’s a clear potential for problems
15:54:53 leakypipes mriedem: ok, that's what I needed to know, thanks man
15:55:11 leakypipes mriedem: I'm writing func tests for this code I'm proposing, thus the question...
15:55:14 mriedem leakypipes: this is what creates the host mappings http://git.openstack.org/cgit/openstack/nova/tree/nova/objects/host_mapping.py#n192
15:55:31 superdan cdent: I wonder if it is something like the hostname, and that moving between cells has caused the name to be the same but the uuid change?
15:55:35 mriedem http://git.openstack.org/cgit/openstack/nova/tree/nova/objects/host_mapping.py#n177
15:55:40 leakypipes mriedem: I copied test_hosts_api.py and saw that after reading the comment in api.py about us not storing compute node records in host mappings table and was like... uh?
15:55:41 mriedem host_mapping = HostMapping( ctxt, host=compute.host, cell_mapping=cm) host_mapping.create()
15:56:26 mriedem leakypipes: the api comment says "We wouldn't have to loop over cells if we stored the ComputeNode.uuid in the HostMapping" - specifically the uuid, but yeah i can see how that's confusing now
15:56:33 mriedem because even if we did, it'd be wrong
15:56:37 mriedem we should probably remove that comment
15:57:07 leakypipes mriedem: right. I'm hacking around the host mapping stuff, though, and calling ComputeNode.get_all() on all cells via the context.scatter_gather_skip_cell0() function, since I figured it would not be possible to look up compute node information in the host_mappings table.
15:57:19 cdent superdan: I haven’t had much of a chance to think about it, I was just going through placement tagged bugs and that one was making a dance between invalid and new and thought I better try to make sure it wasn’t lost if it is a real issue
15:57:22 leakypipes s/get_all()/get_by_uuid()
15:57:44 mriedem leakypipes: yeah correct - only the compute_nodes.host field is in there, and if you're ironic that doesn't help you
15:57:47 mriedem because 1:M
15:58:09 superdan cdent: well, I imagine it's a real issue, but I'm not sure exactly what to tell them.. I guess you could provide them the curl commands to grab a token and delete the old provider record to confirm that it heals up properly afterwards right?
15:58:17 leakypipes mriedem: FYI, this is to support PUT /os-aggregates/{agg_id} and passing a compute node UUID instead of a service hostname.
15:58:41 leakypipes mriedem: since our friends in Ironic can't use host aggregates due to the service host coupling.
15:59:24 leakypipes mriedem: and our Ironic friends can't use any aggregate or affinity filter that relies on host aggregates being associated with an Ironic resource node.
16:00:09 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove confusing comment in compute_node_get API method https://review.openstack.org/526718
16:00:10 mriedem leakypipes: see if this floats your boat ^
16:00:18 leakypipes mriedem: of course, I am going to need to backport all of this to Ocata, when none of the context.scatter_gather stuff was in. so that should be super-fun.
16:00:23 mriedem leakypipes: you can't add aggregates support for ironic in nova because mogan
16:00:33 sean-k-mooney leakypipes: FYI i talked to infra and at least for now they are ok with enableing legacy jobs so ill submit a patch for os-vif fuctional jobs on monday. ill see if i can migrate os-vif to in repo zuul v3 jobs before the PTG
16:00:35 mriedem leakypipes: you mean backport internally...
16:00:35 leakypipes mriedem: hmm?
16:00:41 leakypipes mriedem: correcto.
16:00:44 mriedem fun
16:00:47 mriedem oath?
16:00:50 leakypipes yuppers.
16:00:53 cdent superdan: when I say “ I haven’t had much of a chance to think about it” I really mean that. I’ve not processed what they are doing at all. If you’re trying to encourage me to take some responsibility for flagging something up, that’s great, and I can, but it’s not on my mental stack, despite the presumption of trying to stick ti on yours… :)
16:00:53 mriedem great
16:01:00 leakypipes I blame the penick.
16:01:05 mriedem leakypipes: oh right you weren't at the mogen session in sydney
16:01:18 leakypipes mriedem: I wasn't in Sydney. period. :)
16:01:18 mriedem aggregates support in mogan was one of the differentiators
16:01:28 mriedem i could have sworn...
16:01:47 mriedem leakypipes: so,
16:01:57 mriedem are you going to do this upstream with a new microversion?
16:02:11 leakypipes mriedem: well, frankly, a placement aggregate doesn't have the service host coupling that a nova host-aggregate currently does, so I'm just trying to get a patch in place for Ocata Nova that kind of gets nova host-aggs looking/functioning more like placement aggs
16:02:32 mriedem what happens when oath upgrades to pike or queens?
16:02:39 leakypipes mriedem: I'll throw my code up and have you take a looksie, of course.
16:02:45 mriedem well,
16:03:10 leakypipes mriedem: when oath upgrades to pike/queens, we'll likely forward-port this thing I'm wokring on
16:03:20 leakypipes mriedem: until placement aggs are fully utilized.
16:03:20 mriedem my point is, backporting microversions opens up some bad issues unless you backport all intermediate microversions too
16:03:32 leakypipes mriedem: I'm not doing this as a microversion :)
16:03:38 leakypipes mriedem: I'm not changing the API at all.
16:03:38 mriedem ok
16:03:53 mriedem "this is to support PUT /os-aggregates/{agg_id} and passing a compute node UUID instead of a service hostname."
16:03:58 leakypipes mriedem: just looking to see if hostname is like a UUID and if so, looking up a compute node instead of a service host.
16:04:02 mriedem ok,
16:04:06 mriedem was just going to say that :)
16:04:08 leakypipes it's cheeky, I know.
16:04:09 mriedem tricksy
16:04:19 leakypipes cheeky cheeky monkey.
16:04:59 mriedem does oath also want to be able to cold migrate/resize baremetal instances?
16:05:02 sean-k-mooney superdan: i was under the impression that we required the hostID to be gloally unique across the entire cloud not jsut within a cell so if they are useing hostnames as the hostid .i.e the default then they would be required to have unique host names across all cells
16:05:26 superdan sean-k-mooney: correct, but they're moving a host between cells
16:06:09 mriedem leakypipes: because https://review.openstack.org/#/c/449155/
16:06:42 sean-k-mooney superdan: yes but i was under the impression that the host id had to be unique period e.g. its invalid to have to hosts with the same hostname in different cells if you are using the hostname as the hostid
16:06:54 leakypipes mriedem: that hasn't been brought up, no

Earlier   Later