| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-04 | |||
| 16:04:03 | gibi | bauzas: sure, I also want to make sure, hence the big commit message | |
| 16:04:15 | bauzas | (fwiw, I have the same problem with any NUMA affinity for vGPUs ;) ) | |
| 16:04:26 | bauzas | changing the filter is difficult | |
| 16:04:58 | gibi | if we end up fearing the re-schedule then I can drop this patch and document the limitation | |
| 16:05:33 | bauzas | gibi: I'm not really bad with this | |
| 16:05:54 | bauzas | also, https://review.openstack.org/#/c/636360/19 is the API microversion that really creates the new feature, right? | |
| 16:06:18 | gibi | bauzas: yes, everything is rejected until https://review.openstack.org/#/c/636360/19 | |
| 16:06:37 | gibi | bauzas: everything involving ports with resource request | |
| 16:06:45 | bauzas | yeah, I'm seeing with https://review.openstack.org/#/c/636360/19/nova/api/openstack/common.py | |
| 16:06:45 | gibi | bauzas: except server delete | |
| 16:07:06 | gibi | bauzas: yepp, that is the key | |
| 16:15:17 | stephenfin | gibi: Left a series of nits plus a suggestion to s/pf_interface_name/parent_ifname/, but otherwise +2 on https://review.openstack.org/#/c/623543/ Thanks for tackling that | |
| 16:16:21 | bauzas | gibi: just to make it clear, if you solve https://review.openstack.org/#/c/623543/39//COMMIT_MSG by the compute claim, that means that you check the compute version for all the move ops ? | |
| 16:17:05 | gibi | stephenfin: I'm OK to rename the key to parent_ifname | |
| 16:17:20 | stephenfin | and I'm OK to re-review if you do :) | |
| 16:17:25 | bauzas | don't see any of this in https://specs.openstack.org/openstack/nova-specs/specs/stein/approved/bandwidth-resource-provider.html#upgrade-impact | |
| 16:17:50 | gibi | bauzas: I think you found a problem | |
| 16:18:18 | gibi | bauzas: we did not planned to check the compute version | |
| 16:19:08 | gibi | bauzas: right now if you boot a server and the scheduler select a host that is still Rocky then the pci claim fails as the new pf_interface_name key will be missing from the pci spec | |
| 16:19:25 | gibi | bauzas: so no resource inconsistency but more re-schedule will happen | |
| 16:20:41 | bauzas | gibi: wait | |
| 16:20:42 | gibi | bauzas: ohh no. the InstancePCIRequest will not request the new key either if the request lands on an old compute | |
| 16:20:58 | bauzas | gibi: yeah that | |
| 16:21:08 | bauzas | it's old code so it doesn't know about anything | |
| 16:21:31 | gibi | bauzas: yeah, the InstancePCIRequest change is also in the compute/manager | |
| 16:22:40 | gibi | BUT if we have an old compute then the old compute will not have bandwidth inventory | |
| 16:22:49 | gibi | so placement will not select it | |
| 16:23:07 | gibi | (assuming that nova-compute is updated along with the neutron agents) | |
| 16:23:10 | bauzas | gibi: that was the original reason why we didn't need a check | |
| 16:23:22 | bauzas | because placement checks it for free | |
| 16:23:23 | bauzas | but | |
| 16:23:49 | bauzas | for a classic move operation, you can end up with the late-check not being read | |
| 16:24:03 | mriedem | there is no move support for this in stein | |
| 16:24:30 | gibi | bauzas: ^^ | |
| 16:24:35 | mriedem | gibi: and we enforce that in the api right? | |
| 16:24:39 | mriedem | yeah i remember | |
| 16:24:47 | gibi | mriedem: yes, the moves are rejected | |
| 16:24:56 | gibi | mriedem: even after the 2.72 | |
| 16:24:56 | bauzas | mriedem: what happens with my instance ? am I able to migrate it with dropping the bandwidth check, or do i get some ERROR ? | |
| 16:25:07 | mriedem | you get a 400 or 409 | |
| 16:25:08 | bauzas | ah, that's an API enforcement | |
| 16:25:09 | mriedem | i can't remember which | |
| 16:25:10 | bauzas | good then | |
| 16:25:46 | mriedem | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/evacuate.py#L123 400 | |
| 16:25:54 | bauzas | so that works, at least if neutron agent is upgraded with compute service, which is usually what's done | |
| 16:26:13 | mriedem | if the neutron agent is upgraded before nova-compute it could be reporting inventory before the compute is ready to handle those types of allocations... | |
| 16:26:18 | mriedem | but i'm not sure how realistic that is | |
| 16:26:41 | gibi | bauzas: even if they ugrade the neutron agent they can skip the bw config in the agent until nova-compute is upgraded | |
| 16:26:45 | bauzas | worth a note at least | |
| 16:26:54 | gibi | bauzas: the whole neutron bw inventory is config based | |
| 16:26:57 | bauzas | yeah | |
| 16:27:12 | mriedem | how much of the compute-level stuff is virt-driver specific? | |
| 16:27:16 | bauzas | so, ops can upgrade whenever they want, and once they're done, they set configs | |
| 16:27:17 | mriedem | just the multi-VF on same PF thing? | |
| 16:27:37 | bauzas | gibi: planned to write some ops docs btw. ? | |
| 16:27:51 | bauzas | that would also help me reviewing all of this :) | |
| 16:27:55 | gibi | bauzas: yeah, rubasov already started it for neutron and I will chime in | |
| 16:28:19 | bauzas | I'd be more than happy to review it | |
| 16:28:23 | gibi | mriedem: good question, I think the SRIOV + bandwidth support is now become virt driver specify | |
| 16:28:41 | gibi | bauzas: here is a neutron WIP doc patch https://review.openstack.org/#/c/640390/ | |
| 16:28:45 | dansmith | mriedem: hmm, aren't the two things in runway slot 2 and 3 part of the same effort? | |
| 16:28:59 | mriedem | dansmith: yeah, spot instances | |
| 16:29:02 | dansmith | yeah | |
| 16:29:04 | mriedem | separate blueprints for whatever reason | |
| 16:29:10 | dansmith | hrm | |
| 16:29:37 | dansmith | they're part of the same stack in gerrit, so.. kinda seems like that's one thing:) | |
| 16:29:42 | gibi | mriedem: if there is a InstancePCIRequest then it will have the new tag included even if the compute host only has a single PF | |
| 16:29:48 | mriedem | gibi: but maybe a compute without a supporting virt driver wouldn't configure the neutron agent to report that inventory | |
| 16:29:52 | artom | mriedem, I wasn't checking the database instance_numa_topology | |
| 16:29:58 | artom | So the pins are updated and correct | |
| 16:30:14 | gibi | mriedem: SRIOV without bw is kept intact | |
| 16:30:15 | artom | But Windriver are telling me the DB isn't updated, despite apply_migration_context() being called | |
| 16:30:43 | gibi | mriedem: so if no resource request in the sriov port then we don't need the special key from the virt driver | |
| 16:30:57 | mriedem | artom: are you saving the instance changes after calling apply_migration_context()? | |
| 16:31:04 | mriedem | b/c ^ doesn't persist the changes, it just updates the object fields | |
| 16:31:15 | artom | mriedem, wait, seriously? | |
| 16:31:35 | artom | mriedem, huh, but instance.save() is still called after in post_live_migration | |
| 16:31:36 | mriedem | don't take my word for it | |
| 16:31:43 | bauzas | gibi: if that's virt-specific, you should document it in https://docs.openstack.org/nova/latest/user/feature-classification.html | |
| 16:31:53 | gibi | bauzas: thanks for the pointer | |
| 16:32:16 | artom | mriedem, ah, you're right, it's not saved | |
| 16:32:21 | mriedem | i'm more worried that we send a server create request to a compute that doesn't support it | |
| 16:32:33 | mriedem | and we don't have any kind of feature capability for this port requested resources stuff | |
| 16:32:44 | mriedem | gibi: bauzas: ^ | |
| 16:32:48 | mriedem | and we just drop the request on the floor | |
| 16:33:01 | dansmith | artom: so does that mean some testing gap exists because we never re-migrate the instance a second time or something? | |
| 16:33:09 | mriedem | thinking about https://review.openstack.org/#/c/538498/ coincidentally | |
| 16:33:11 | bauzas | mriedem: that's my -1 | |
| 16:33:18 | dansmith | artom: meaning.. how could we be failing to save that and think that this works? | |
| 16:33:22 | artom | But there's an instance.save 5 lines after https://review.openstack.org/#/c/634606/37/nova/compute/manager.py@6925 | |
| 16:33:23 | gibi | mriedem: yeah if neutron agent is configured to report bandwidth but the nova virt driver does not support it then it will be NoValidHost after a bunch of reschedule | |
| 16:33:35 | bauzas | mriedem: but the question is : should we leave it as docs, or enforce it ? | |
| 16:33:37 | artom | dansmith, I was just testing the XML is updated correctly | |
| 16:33:52 | dansmith | artom: oh :( | |
| 16:34:03 | artom | dansmith, yeah, testing coverage gap :( | |
| 16:34:52 | mriedem | gibi: bauzas: at this point it's probably best to just doc it and clean it up in train with a capability tag a la https://review.openstack.org/#/c/538498/ | |
| 16:35:16 | bauzas | I'm good with that | |
| 16:35:19 | mriedem | aspiers: efried: btw who is on the hook for documenting ^ now that it's going to be codified? | |
| 16:35:19 | gibi | bauzas, mriedem, sean-k-mooney, stephenfin: this is the cons of the auto detection of the pf_interface_name. It makes the feature virt driver specific. If we go with the a new tag in the passthrough_whitelist then the solution is virt driver agnostic | |
| 16:35:55 | mriedem | but then you have to configure more crap | |