| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-25 | |||
| 13:34:02 | mriedem | so the instance will have an allocation on the dest host, and the migration will have an allocation on the source ohst | |
| 13:34:04 | mriedem | *host | |
| 13:34:46 | mriedem | as i pointed out in https://review.openstack.org/#/c/506420/ this morning, until we have cdent's POST allocations stuff, we'll actually be quadrupling the allocations temporarily when resizing to the same host | |
| 13:34:54 | dtantsur | hi mriedem! do I get it right that we're past spec freeze? people ping me re https://review.openstack.org/#/c/449155/, do they have to move it to Rocky at this point? I also wonder if that even needs a spec, but dunno | |
| 13:35:52 | mriedem | dtantsur: i think any move operation involving the ironic driver is going to require a spec yes, simply because of the resource tracking considerations that have to be made with the 1:M host:node mapping with ironic | |
| 13:36:03 | efried | mriedem Okay, so there will still need to be some affordance for adjusting doubled (or quadrupled) allocations for the forseeable future. | |
| 13:36:15 | mriedem | dtantsur: because we have assumptions all over nova when getting compute nodes from the db with only the host that there is only 1 node because we're not supporting moves with ironic yet | |
| 13:36:28 | mriedem | efried: yes, per the FIXME note from dan in that change | |
| 13:36:38 | mriedem | the FIXME means replace that with cdent's POST allocations stuff | |
| 13:36:54 | cdent | just in the midst of writing the docs for that stuff | |
| 13:36:58 | efried | mriedem The problem is in the way we're discovering how much adjustment to make, and to which RP. At the moment the code assumes in several ways that there's only one RP - the compute node. | |
| 13:36:59 | dtantsur | mriedem: mm, this is something I've never considered | |
| 13:37:34 | mriedem | dtantsur: also, | |
| 13:38:05 | mriedem | we have a spec from takashin to pass a dest host for cold migration, and that won't really work for ironic because you're only passed the host in the api, not the host+node | |
| 13:38:30 | mriedem | today when the admin specifies a host, we just take the ComputeNodeList[0] entry b/c we assume it's not an ironic instance | |
| 13:38:39 | dtantsur | yep, I think the plan was to let nova find a host | |
| 13:39:01 | mriedem | find a node? | |
| 13:39:10 | mriedem | the admin specifies a host, nova just randomly picks a node | |
| 13:39:28 | mriedem | well, i guess i'm thinking of the force case with live migration and evacuate | |
| 13:39:46 | mriedem | for cold migration we said we wouldn't add a force option, so you can specify a host and we'll still go through the scheduler to find a node | |
| 13:40:05 | mriedem | evacuate + forced host + ironic is probably totally busted though :) | |
| 13:40:58 | dtantsur | yep :) | |
| 13:41:26 | dtantsur | we were discussing cold migration + resize in context of nova. I was under impression this may work | |
| 13:41:36 | dtantsur | well, assuming people boot from volume and do not rely on local disks too much :) | |
| 13:41:55 | dansmith | mriedem: his question was if it'll remove the need for that special allocation-slicing thing and it will | |
| 13:42:25 | dansmith | mriedem: once we're past queens we won't have any of that | |
| 13:43:04 | mriedem | dansmith: the scheduler still picks the dest host and makes the doubled allocation yeah? | |
| 13:43:06 | mriedem | for the instance | |
| 13:43:06 | dtantsur | mriedem: anyway, could you please leave a short comment on that spec with the freeze status and the question you'd see covered? | |
| 13:43:13 | dansmith | mriedem: no | |
| 13:43:15 | mriedem | dtantsur: doing so now | |
| 13:43:24 | dtantsur | thanks mriedem | |
| 13:43:30 | dansmith | mriedem: before we call the scheduler we replace the instance allocation with the migration one so no doubling happens when the scheduler does its thing | |
| 13:44:23 | mriedem | dansmith: yeah sorry you're right - _preallocate_migration is called before select_destinations() | |
| 13:45:01 | dansmith | right | |
| 13:45:31 | mriedem | ok i was +2 on that change since last night anyway | |
| 13:46:02 | mriedem | back to crushing everyone's queens spec freeze dreams | |
| 13:46:26 | dansmith | efried: catch that? | |
| 13:52:10 | sean-k-mooney | dansmith: mriedem today we claim PCI devices on the compute node not the scheduler/conductor correct? we only do placement allocation for cpu,ram and disk in the scheduler today. | |
| 13:52:26 | dansmith | correct | |
| 13:54:41 | sean-k-mooney | ok that's what i taught. we may need to pass more the the host-id when creating the new portbinding unless neutron are happy to allow use to update the binding profile with the claimed pci devices when we activate the binding | |
| 13:55:07 | openstackgerrit | Radoslav Gerganov proposed openstack/nova master: VMware: implement get_inventory() driver method https://review.openstack.org/506175 | |
| 13:55:32 | mriedem | sean-k-mooney: i thought some of the dvr hackery migration code we have in nova.network.neutronv2.api already dealt with pci devices in the port binding profile? | |
| 13:56:37 | sean-k-mooney | it may i was just trying to think if we need to have the binding profile in the new migrtion object or not. | |
| 13:57:47 | sean-k-mooney | mriedem: that said there is no code to regenerate the interfaces at all currently so if it was updating pci devices it was not updating the xml for sriov devices | |
| 13:58:24 | mriedem | sean-k-mooney: ok - definitely going into moshe / cfriesen territory here | |
| 13:59:26 | sean-k-mooney | mriedem: ya ill check with them, as far as i know i think moshe had patches to make livemigration with macvtap mode work but i dont recall if they ever merged | |
| 14:00:54 | sean-k-mooney | mriedem: vnic_type=direct or direct_physical do not support live migration currently at all as far as i know so this is an edge case of an edge case | |
| 14:07:25 | mriedem | dansmith: better to do a new remotable class method here? https://review.openstack.org/#/c/514904/3/nova/objects/migration.py | |
| 14:07:29 | mriedem | i think that's what we do in InstanceList | |
| 14:08:24 | dansmith | mriedem: the reason we did it for instance is because we had sort_key already and they wanted to replace it with sort_keys | |
| 14:08:49 | dansmith | we do add arguments like this all the time and I think the behavior of them being unset makes sense in this case (i.e. =None means default sort) | |
| 14:09:07 | mriedem | ok | |
| 14:09:17 | dansmith | I shall comment | |
| 14:10:23 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix AttributeError in BlockDeviceMapping.obj_load_attr https://review.openstack.org/514825 | |
| 14:10:26 | mriedem | melwitt: i just removed the obj_reset_changes thing in the bdm object here; didn't want that holding up this otherwise simple fix ^ | |
| 14:20:42 | efried | dansmith Got it, thanks. I just saw your commentary at https://review.openstack.org/#/c/506420/29/nova/conductor/tasks/migrate.py@61 which confirms some stuff I "discovered" last night as I was trying to figure out how to make flavors take numbered groupings. | |
| 14:21:07 | dansmith | ack | |
| 14:21:23 | efried | dansmith Getting resources from the flavor is going to break down *very* quickly for everything other than scheduling. | |
| 14:21:48 | efried | We're gonna have to get it from placement - but that's a chicken/egg for these scenarios. | |
| 14:22:05 | dansmith | well, that's why I was suggesting we go ahead with passing the allocation into the virt drier, | |
| 14:22:12 | dansmith | instead of setting the precedent of ripping that from the flavor | |
| 14:22:32 | efried | dansmith Does that help, though? Unless compute caches it somewhere... | |
| 14:22:56 | dansmith | help what? | |
| 14:23:23 | dansmith | it helps us not have to reconstruct the request to placement for things like "which device on which numa node should we give them a vgpu from" | |
| 14:23:27 | efried | Help with getting rid of remove_provider_from_instance_allocation | |
| 14:23:40 | dansmith | no, but that's not an issue after my set anyway | |
| 14:23:54 | efried | Let me come at it from the other angle: What all needs to merge for us to get rid of remove_provider_from_instance_allocation ? Just your thing, or also cdent's POST thing? | |
| 14:24:18 | dansmith | yes, and then a cycle has to pass, because we still need that for the old path where we have pike computes doing the old thing | |
| 14:24:30 | dansmith | so we merge my thing, snap queens, then we can remove all that junk | |
| 14:24:58 | dansmith | merging cdent's thing is orthogonal to being able to stop doubling.. it's just a correctness/atomicity thing | |
| 14:25:14 | efried | dansmith Ayee, okay. So we'll still need to figure out how to make remove_provider_from_instance_allocation work with e.g. nested. | |
| 14:25:42 | dansmith | well, it kinda depends on how that turns out | |
| 14:25:48 | dansmith | my first response is to say no, we don't | |
| 14:25:59 | dansmith | because pike computes won't have any nesting | |
| 14:26:12 | dansmith | and just the presence of the nested stuff doesn't break us, | |
| 14:26:39 | dansmith | what would cause a problem is if we could be migrating between a pike and queens node where the queens node has some nested resources that we have to account for in that thing | |
| 14:28:33 | efried | dansmith Presumably the scheduler on the pike node would just request the resources from whatever RPs are available on the pike node, which won't be nested. | |
| 14:28:57 | mriedem | scheduler is global | |
| 14:29:12 | efried | Right, point remains. | |
| 14:29:13 | mriedem | you upgrade scheduler before computes | |
| 14:29:15 | dansmith | efried: we don't do that new behavior until all the computes are upgraded | |
| 14:29:32 | dansmith | efried: because the old ones assume doubled allocations | |
| 14:29:48 | mriedem | and pike computes don't know how to cleanup allocations using the migration uuid | |
| 14:29:56 | mriedem | which is another way of saying what dan just said i think | |
| 14:29:59 | dansmith | right | |
| 14:32:03 | efried | Just so my brain doesn't explode, I need to zoom out a little bit and frame all of this in terms of how to grab numbered resource groupings from the flavor for the sake of scheduling. | |
| 14:32:27 | openstackgerrit | Pavel Gluschak (scsnow) proposed openstack/nova master: Fixed concurrent access to direct io test file https://review.openstack.org/515091 | |
| 14:33:03 | efried | I think for now I just need to sum up all the resources and continue to pass in the single dict to remove_provider_from_instance_allocation | |
| 14:33:47 | cdent | efried: overhangs are the best part | |
| 14:33:48 | efried | And only keep them split up for the scheduler side of things, which is resources_from_request_spec | |
| 14:34:16 | dansmith | efried: currently I think that's right yeah | |
| 14:34:42 | gibi | mriedem: As you suggested last week I reached out to the author of bp service-create-destroy-notification and he stated he is still planning to work on the implementation. | |
| 14:34:44 | efried | This keeps us throwing good money after bad, digging deeper into the shared RP tech debt hole, but I don't see an alternative without boiling the ocean. | |
| 14:35:00 | efried | dansmith mriedem Thanks y'all. | |
| 14:35:22 | cdent | bonus points for boiling the ocean | |
| 14:35:28 | mriedem | gibi: cool thanks | |
| 14:35:46 | efried | cdent No penalties for mixing metaphors? | |
| 14:36:00 | cdent | good lord from me? no way. | |
| 14:36:06 | cdent | mixed metaphors are the best | |