| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-25 | |||
| 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:06 | mriedem | for the instance | |
| 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 | |
| 14:37:15 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Accept all standard resource classes in flavor extra specs https://review.openstack.org/514942 | |
| 14:37:15 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: VGPU_support: add enabled white list https://review.openstack.org/512580 | |
| 14:37:16 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: WIP: libvirt: Provide VGPU inventory for a single GPU type https://review.openstack.org/514763 | |
| 14:41:43 | efried | How's the new hire stuff going jaypipes? Winding down? | |
| 14:43:36 | jaypipes | efried: it's going. | |
| 14:46:03 | dansmith | jaypipes: that well huh? | |
| 14:46:16 | mriedem | dansmith: sorry for getting to this late, but questions inline https://review.openstack.org/#/c/508595/ | |
| 14:47:52 | dansmith | mriedem: standard operating procedure for you, I know | |
| 14:48:00 | mriedem | :( | |
| 14:48:05 | dansmith | mriedem: that comment was added recently by you right? maybe just rebase noise | |
| 14:48:11 | mriedem | yes | |
| 14:48:17 | mriedem | when cleaning out the system_metadata stuff | |
| 14:48:33 | mriedem | i'm not -1 for that, that could be added back on top later | |
| 14:48:38 | dansmith | gonna pull it out of the gate? I'm still reading | |
| 14:48:46 | mriedem | -1 is for the lack of testing with limits, unless i'm missing something | |
| 14:48:52 | mriedem | *build_requests + limits in the sortmaster | |
| 14:52:09 | dansmith | mriedem: you think it's broken or just want more coverage? | |
| 14:52:25 | dansmith | since it's all the same logic (no special limit decrementing) it's not much of a gap | |
| 14:52:44 | mriedem | there was a test for build requests + limits which was removed, | |
| 14:52:48 | mriedem | so i think we want that coverage yeah | |
| 14:53:10 | mriedem | you could replay it on top of this change if you want (not the series, just this change) | |
| 14:54:56 | dansmith | I would rather not keep that test the way it was, since it tests the instance_list module from above the compute api, but I'm happy to slap a limit base test into instance_list | |
| 14:55:25 | dansmith | mriedem: it's fairly deep into the gate (spot 15 out of 42) but if you want to pull it out, I'll add it to that patch, else I can pile the coverage on top | |
| 14:55:43 | mriedem | just put it on top | |
| 14:55:44 | mriedem | that's fine | |
| 15:04:23 | dansmith | did they just restart zuul or something? | |
| 15:07:49 | jaypipes | gibi: please see my explanation on https://review.openstack.org/#/c/377215/53/nova/tests/functional/db/test_resource_provider.py | |