| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-03 | |||
| 16:03:29 | artom | mriedem, fair enough, I need to refresh myself on how the above link works then :/ | |
| 16:03:32 | mriedem | ^ been there since liberty | |
| 16:03:59 | mriedem | artom: i think this code is what sets the device names on the bdm objects https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7840 | |
| 16:04:08 | artom | Ah, so there's still a matching name between the bdm and XML | |
| 16:04:16 | artom | But it's libvirt that sets it, not the user | |
| 16:04:19 | mriedem | correct | |
| 16:04:29 | mriedem | it's all very confusing where libvirt sets it, because the libvirt driver doesn't actually set it | |
| 16:04:33 | mriedem | block_device.default_device_names does | |
| 16:04:49 | mriedem | like most of the bdm code, it's a lot of smoke and mirrors | |
| 16:05:14 | artom | Any sentence that matches /.*block.*device.*confusing.*/ is a tautology | |
| 16:05:37 | mriedem | nvm i guess it is specific to the libvirt driver https://github.com/openstack/nova/blob/master/nova/virt/libvirt/blockinfo.py#L466 | |
| 16:05:54 | mriedem | it has those driver bdm wrapper objects | |
| 16:06:02 | mriedem | so it can mutate the underlying bdm object, and then update it in the db | |
| 16:06:05 | mriedem | using driver_bdm.save() | |
| 16:06:20 | mriedem | and get_disk_info(virt_type, instance, image_meta, block_device_info) is not read-only :) | |
| 16:07:02 | artom | Well, get_guest_xml() isn't read-only either IIRC, so it's at least internally consistent | |
| 16:07:21 | mriedem | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/blockinfo.py#L488 | |
| 16:07:50 | mriedem | yeah anyway, point is, the libvirt driver sets the device name for the bdm, the user doesn't | |
| 16:08:43 | mriedem | sdague: johnthetubaguy: dansmith: did you read that thread in the ML on removing user-supplied bdm devices from requests in the API? i had no idea people were using our crappy data model for overriding image bdms | |
| 16:09:12 | openstack | Launchpad bug 1679223 in OpenStack Compute (nova) "tempest.api.compute.servers.test_server_tags.ServerTagsTestJSON fail on centos7 nodes" [Undecided,New] | |
| 16:09:12 | markus_z | felipemonteiro: Would be cool if you could take a look at: https://bugs.launchpad.net/nova/+bug/1679223 | |
| 16:10:43 | dansmith | mriedem: I saw it, didn't read it | |
| 16:11:00 | dansmith | mriedem: but yeah, I knew | |
| 16:11:01 | sdague | mriedem: it didn't fully sink in, I'll have to revisit | |
| 16:14:21 | bauzas | mriedem: jaypipes: I have a thought, and I'd love your feedback about claims | |
| 16:14:28 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Factor out duplicated notification sample data (2) https://review.openstack.org/452819 | |
| 16:14:28 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Factor out duplicated notification sample data https://review.openstack.org/452818 | |
| 16:14:29 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Factor out duplicated notification sample data (3) https://review.openstack.org/452820 | |
| 16:14:44 | bauzas | mriedem: jaypipes: I have a long standing BP targeting to pass the Spec object down to the compute, mostly for rescheduling needs | |
| 16:14:53 | jaypipes | bauzas: I'll trade ya for a review of https://review.openstack.org/#/c/448282/? :) | |
| 16:15:31 | bauzas | mriedem: jaypipes: if we would pass the allocations as part of the Spec object down to the compute, it would prevent us to have a very specific RPC modification for something abstract enough | |
| 16:16:04 | jaypipes | bauzas: yup, makes sense to me. | |
| 16:16:05 | bauzas | mriedem: jaypipes: ie. the conductor gets the Spec object, pass it to the scheduler, and gives back a list of allocations (because the scheduler would claim) | |
| 16:16:07 | mriedem | i'm out of the loop on the claims stuff. | |
| 16:16:21 | bauzas | then, it would pass it down to the compute by setting those claims | |
| 16:16:30 | mriedem | what is the compute going to do with them? | |
| 16:16:50 | mriedem | isn't the whole point that the compute no longer needs to care about setting allocations in the RT? | |
| 16:16:50 | bauzas | jaypipes: it would allow us to change the RPC interface to just accept a spec_obj attribute and read the allocations UUIDs | |
| 16:16:58 | bauzas | mriedem: in case something goes wrong | |
| 16:17:03 | mriedem | what does that mean? | |
| 16:17:06 | bauzas | mriedem: the compute would DELETE those allocations | |
| 16:17:06 | jaypipes | mriedem: if the compute sees the allocations in the request spec, it wouldn't need to do the claims.Claim() build. | |
| 16:17:35 | bauzas | and it would trigger the reschedule by passing back the Spec | |
| 16:17:54 | mriedem | i thought the scheduler was going to delete the allocations if something failed on a given compute host? | |
| 16:18:03 | bauzas | jaypipes: yeah, the Spec object would signal if we support new-style of claiming | |
| 16:18:24 | bauzas | mriedem: the scheduler would POST allocations | |
| 16:18:30 | jaypipes | bauzas: yeah, the presence of allocation objects in the RequestSpec would be that signal. | |
| 16:18:30 | bauzas | mriedem: and get the UUIDs | |
| 16:18:35 | mriedem | jaypipes: so we bump the service version when computes start checking for claims in the scheduler (via the allocations in the request spec?) and then use that service version to determine when the scheduler can start doing this? once all computes are upgraded or what? | |
| 16:19:07 | bauzas | mriedem: I was considering that approach | |
| 16:19:21 | bauzas | mriedem: because we suppose a fully-upgraded cloud | |
| 16:19:24 | jaypipes | mriedem: ya | |
| 16:19:32 | mriedem | i'm not sure i like the compute deleting the allocations | |
| 16:19:36 | mriedem | on a retry | |
| 16:19:49 | dansmith | definitely not | |
| 16:19:55 | mriedem | wouldn't it be better to have the allocation create and cleanup in the same service? | |
| 16:20:01 | bauzas | I dunno | |
| 16:20:02 | mriedem | if we want to just pass a flag down to the compute, sure | |
| 16:20:10 | bauzas | I'm not opiniated | |
| 16:20:24 | dansmith | compute shouldn't be doing that once we're claiming in the scheduler, methinks | |
| 16:20:24 | bauzas | so | |
| 16:20:30 | mriedem | dansmith: agree | |
| 16:20:31 | bauzas | if we say no to that idea | |
| 16:20:39 | bauzas | then, we would just send a signal | |
| 16:20:42 | mriedem | at a high level, we want the compute out of the business of doing this stuff | |
| 16:20:52 | bauzas | but the compute would pass down the Spec by signaling against the signal :) | |
| 16:20:52 | mriedem | yeah send a flag in the reqspec object i guess? | |
| 16:21:04 | bauzas | I mean | |
| 16:21:07 | mriedem | do we pass the reqspec today to the compute | |
| 16:21:08 | mriedem | ? | |
| 16:21:17 | bauzas | mriedem: not yet, but I was planning to | |
| 16:21:22 | mriedem | oy | |
| 16:21:26 | bauzas | it's a specless bp | |
| 16:21:30 | mriedem | why not just pass a simple boolean on the rpc method? | |
| 16:21:35 | dansmith | mriedem: it's leftovers from that whole process still :/ | |
| 16:21:38 | mriedem | do_claims=False | |
| 16:21:54 | dansmith | mriedem: +1.. I think encoding the state in the reqspec is kinda bad | |
| 16:22:00 | bauzas | mriedem: I'd rather prefer to pass a Spec object because we could need it for rescheduling needs | |
| 16:22:01 | dansmith | it's not guaranteed to be right | |
| 16:22:11 | mriedem | bauzas: what rescheduling needs? | |
| 16:22:17 | mriedem | conductor / scheduler handles that | |
| 16:22:24 | bauzas | mriedem: the Retry nested object at least | |
| 16:22:42 | mriedem | in filter_properties? | |
| 16:22:48 | dansmith | bauzas: a call from compute to scheduler will be an upcall, you know | |
| 16:22:58 | bauzas | mriedem: at the moment, we have ugly object-to-dict and dict-to-objects handlers | |
| 16:23:09 | mriedem | let's step back, are you talking about build_and_run_instance? | |
| 16:23:15 | bauzas | dansmith: compute does upcall conductor when rescheduling | |
| 16:23:26 | bauzas | and conductor calls scheduler | |
| 16:23:32 | mriedem | because build_and_run_instance already gets the reqspec | |
| 16:23:48 | bauzas | so when talking compute > scheduler, I'm just summarizing rather compute > conductor > scheduler | |
| 16:24:14 | mriedem | the reqspec is already passed to the compute, but not used at all | |
| 16:24:22 | mriedem | it's a dead parameter in _do_build_and_run_instance in the compute manager | |
| 16:24:26 | dansmith | bauzas: if we're claiming in the scheduler, do we ever reschedule once it's made it to the compute? | |
| 16:24:27 | bauzas | it's passed as a dict | |
| 16:24:35 | bauzas | dansmith: we could | |
| 16:24:48 | bauzas | dansmith: we still need some way to assume something went bad | |
| 16:25:01 | mriedem | yeah we can fail for non-resource related reasons | |
| 16:25:05 | bauzas | libvirt could fail for some foolish BDM things | |
| 16:25:14 | bauzas | or some perms issue | |
| 16:25:18 | mriedem | vif plug can fail, etc | |