| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-04 | |||
| 15:25:44 | stephenfin | ack. It'll be back in a runway in a few weeks too | |
| 15:25:47 | efried | stephenfin: We've also got mox, privsep, and fake-libvirt cleanups which ought to be pretty much ready to go. | |
| 15:26:35 | stephenfin | efried: Aye, though privsep is blocked by some comments from you that mikal hasn't address yet. I've been tackling the libvirt version enum cleanup ones from mriedem/kashyap already | |
| 15:27:02 | efried | oh, is that where I left privsep? cool | |
| 15:27:03 | efried | :) | |
| 15:27:34 | efried | looks like there are two I could push | |
| 15:30:02 | cfriesen | efried: is the TPM version actually an issue? given that it's emulated, all hosts can provide both TPM versions. if the guest doesn't care, they can just ask for version 1.2 | |
| 15:35:29 | openstackgerrit | Merged openstack/nova stable/stein: libvirt: Avoid using os-brick encryptors when device_path isn't provided https://review.opendev.org/656462 | |
| 15:40:40 | rm_work | mriedem: seems that we just never enabled notifications to begin with? ¯\_(ツ)_/¯ | |
| 15:40:52 | rm_work | where can I go look in our deployment tooling/config to see? :D | |
| 15:41:02 | rm_work | what config option is it? | |
| 15:41:35 | mriedem | rm_work: https://docs.openstack.org/nova/latest/configuration/config.html#notifications | |
| 15:41:46 | mriedem | https://docs.openstack.org/nova/latest/configuration/config.html#notifications.notification_format specifically is what i'm looking for | |
| 15:42:10 | efried | cfriesen: Well, that's what I was asking you, I guess. | |
| 15:42:18 | mriedem | also this https://docs.openstack.org/nova/latest/configuration/config.html#oslo-messaging-notifications | |
| 15:42:40 | efried | cfriesen: Is it not the case that 2.0 is only available (emulatable) at certain versions of... things (qemu, libvirt, ...)? | |
| 15:43:25 | mriedem | rm_work: notifications are enabled by default so maybe you're just generating them but not consuming them. to disable them, [oslo_messaging_notifications]/driver=noop | |
| 15:43:50 | efried | cfriesen: Basically: is the model the thing the user primarily cares about, or is the version? | |
| 15:44:11 | rm_work | ok, yeah i think that's likely | |
| 15:44:56 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/stein: Include all network devices in nova diagnostics https://review.opendev.org/657125 | |
| 15:46:02 | cfriesen | efried: the version of libvirt that introduced TPM support adds support for both, and the TPM emulator supports both, so they should always be available. I want to double-check with sean-k-mooney and kashyap but I think the initial concern would be the version as the two versions are quite different. | |
| 15:48:45 | efried | cfriesen: Oh, so the implementation (which I haven't gotten to yet, sorry) is using the version trait to effect the proper emulated version? | |
| 15:48:58 | efried | I guess if both versions are available, it would have to. | |
| 15:50:22 | cfriesen | efried: yeah, the virt driver uses the version trait when generating the libvirt XML | |
| 15:51:58 | efried | cfriesen: Okay. This was probably already discussed during design and I'm just late to the party. If hosts are advertising both versions, and users care about the version (are in fact locked to just one) then the current design is fine. | |
| 15:52:17 | mriedem | smcginnis: can you stop in the nova room to talk about openlab for 5 minutes? | |
| 15:52:34 | smcginnis | mriedem: Sure. Right now? | |
| 15:52:39 | mriedem | if possible yeah | |
| 15:52:40 | mriedem | 207 | |
| 15:52:42 | smcginnis | OMW | |
| 15:53:36 | rm_work | mriedem: seems we leave the default | |
| 15:53:45 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove ComputeDriver.macs_for_instance method https://review.opendev.org/652737 | |
| 15:53:45 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove macs kwarg from allocate_for_instance https://review.opendev.org/652749 | |
| 15:54:17 | mriedem | rm_work: ok thanks | |
| 15:55:17 | mriedem | sean-k-mooney: fyi several things are marked as experimental here https://docs.openstack.org/nova/latest/user/feature-classification.html#nfv-cloud-features | |
| 15:55:25 | mriedem | https://docs.openstack.org/nova/latest/user/feature-classification.html#matrix-hpc | |
| 15:55:59 | mriedem | https://docs.openstack.org/nova/latest/admin/huge-pages.html doesn't mention anything about being experimental or not tested | |
| 15:59:18 | cfriesen | efried: A guest could have support for both versions, but unless they're specifically wanting 2.0 functionality then 1.2 should be fine. You raised another issue though, moving operators away from speaking placement-ese in flavors/images. Was that still an issue? This was loosely modelled after the CPU features traits which are explicitly placement-ese. | |
| 16:02:06 | efried | cfriesen: It's not crucial to me that there be zero placement-ese in flavors. For simple things like this (requiring a single trait) it's fine, and is why we enabled support for it. It's complex placement-ese I want to avoid, instead having nova translate more operator-ese thingies to placement-ese. | |
| 16:03:57 | cfriesen | efried: makes sense. I'll reply to your review comment with a summary for others who look at it. | |
| 16:04:30 | efried | cfriesen: Thank you sir. | |
| 16:05:41 | efried | cfriesen: Wait, "A guest could have support for both versions, but unless they're specifically wanting 2.0 functionality then 1.2 should be fine." <== so it's possible I only care about "give me TIS"? | |
| 16:06:25 | efried | If that's the case, I'd still be in favor of making the trait optional (and set up by the request filter) | |
| 16:06:32 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP/DNM objects: Remove get_by_volume_id from BlockDeviceMapping https://review.opendev.org/657127 | |
| 16:07:14 | cfriesen | efried: I think the guest support is different for TIS-on-1.2 vs TIS-on-2.0, but let me confirm with sean/kashyap | |
| 16:08:01 | efried | cfriesen: Okay. None of this should be a huge delta in the impl/spec IIUC. If it turns out to be, I can live with it as is. | |
| 16:08:04 | kashyap | cfriesen: Hi, what exactly you want to double-check? You said "support for both" up-thread -- what are you referring to? | |
| 16:08:31 | efried | kashyap: We're talking about the use cases for TPM. What is it that the operator cares about? | |
| 16:08:47 | efried | kashyap: Is "Give me TIS, I don't care if it's 1.2 or 2.0" a possibility? | |
| 16:09:25 | cfriesen | or do they care primarily about 1.2 vs 2.0, and then crb/tis if they're on 2.0 | |
| 16:09:51 | cfriesen | (the spec was written assuming the second) | |
| 16:09:51 | kashyap | efried: I don't know the answer to your second question -- need to go do some QEMU sluething for it | |
| 16:11:11 | kashyap | efried: The use cases for TPM, my limited understanding is that: they are a "big deal" recently: | |
| 16:11:18 | kashyap | efried: The idea is that you have a tiny co-processor that can do crypto, but with company-specified policies. | |
| 16:12:03 | cfriesen | kashyap: we can talk to sean-k-mooney, he doesn't seem to be paying attention to IRC. :) | |
| 16:13:41 | efried | he's paying attention to the room, like I should be :P | |
| 16:13:47 | kashyap | efried: The use cases are disk encryption — e.g. MS's BitLocker, and "Clevis" (https://github.com/latchset/clevis), TLS key storage (openssl-engine-tpm), etc. | |
| 16:14:25 | kashyap | efried: But I agree: there should be some clear write-up about "use cases from an OpenStack operator PoV" | |
| 16:15:09 | efried | kashyap: That would be nice, but I don't want to go crazy here, or block the work pending said writeup. | |
| 16:15:33 | efried | kashyap: Really the only thing I care about is whether there exists a use case where the VM doesn't care which version it gets. | |
| 16:15:52 | rm_work | On behalf of Octavia, we're a little confused about the "deleting a bound port" problem. johnsom is responding to the email right now, but I wonder if higher bandwidth discussion while a lot of us are here would make sense? I guess the email thread is fine otherwise | |
| 16:16:51 | kashyap | efried: Yeah, noted. And yes, we should explicitly document the answer to your question. | |
| 16:17:10 | efried | rm_work: We have time on the agenda this afternoon if you'd like to get together | |
| 16:17:33 | melwitt | rm_work: it means if you delete a port via the neutron REST API without first detaching it from the server it's attached to. if you do this and the port had QoS settings on it, nova will leak resources in placement (because there's no way for us to delete them if you didn't go through the detach port API or the delete server API) | |
| 16:17:47 | rm_work | efried: Is this really a nova issue, or is it a neutron issue? Or is it totally cross-project? | |
| 16:18:19 | efried | rm_work: The leaked allocations are an issue in placement, which would affect subsequent nova boots with QoS-enabled neutron ports :) | |
| 16:19:14 | melwitt | rm_work: one of the takeaways from the discussion is that neutron can deprecate the ability to delete ports that are still attached (and avoid the issue). as you similarly can't delete a volume that is still attached in cinder | |
| 16:19:56 | efried | rm_work: It's really "always" been bad that you could do this (delete a port without considering what it's attached to), but it never mattered enough to do anything about. Now that the port is tied to real allocations in placement, it constitutes a leak and that kinda pushes it over the edge to where we should really do something. | |
| 16:19:59 | rm_work | the reason we do this directly is that there are several cases where a port detach in nova will simply fail to complete | |
| 16:20:14 | rm_work | at which point, we can either orphan the entire port, or we can just delete it | |
| 16:20:41 | melwitt | I guess if a port detach in nova fails to complete, there's a bug that needs to be fixed | |
| 16:21:11 | rm_work | example A: nova compute node is down | |
| 16:21:11 | efried | When the detach fails to complete, is the instance still deleted? | |
| 16:21:40 | efried | If the failure path still removes the allocations from placement, then it would be fine for you to force-delete the port. | |
| 16:21:54 | melwitt | if compute is down it would get cleaned up at the next run of the 'reap' periodic task on compute | |
| 16:22:03 | rm_work | but we can't wait that long | |
| 16:22:12 | rm_work | this is usually during time-sensitive failover operations | |
| 16:22:18 | melwitt | but this is another case where it would help to add start_immediately=True to the reap periodic | |
| 16:22:27 | rm_work | i think in all cases that we do a port delete the VM that it was attached to WILL be deleted anyway | |
| 16:22:45 | efried | actually, gibi_cape: doesn't the allocation go away whenever the instance is deleted? So the actual problem scenario is when we're detaching a port without deleting the instance? | |
| 16:22:46 | melwitt | that way the first run would happen when compute first comes up | |
| 16:22:49 | rm_work | because we only delete these ports when we are also deleting a VM | |
| 16:23:02 | melwitt | gotcha | |
| 16:23:06 | gibi_cape | efried, rm_work: the leaked allocation is reclaimed when the instance is deleted | |
| 16:23:07 | rm_work | unless I am missing a flow <_< | |
| 16:23:18 | efried | rm_work: If you're deleting the VM, and that succeeds, then we're good. | |
| 16:23:31 | rm_work | ok then so long as that is our workflow, we should not be hard-prevented from doing the port deletion | |
| 16:23:34 | efried | rm_work: So the way the proposed change would affect you is you would have to change the way you do that port detach. | |
| 16:23:37 | rm_work | it comes down to a timing issue | |
| 16:24:04 | rm_work | on our side, we're not going to wait for the detach to succeed in nova before we delete the port | |
| 16:24:10 | rm_work | we're going to move on with our cleanup | |
| 16:24:28 | rm_work | waiting would be problematic when we're in the middle of a time sensitive flow | |
| 16:24:30 | efried | rm_work: "not be hard-prevented" - right; but in order to do it after this change, you would have to change either code or policy (depending on how it's decided to implement) | |
| 16:24:38 | rm_work | yeah, if it's policy, fine | |
| 16:24:48 | rm_work | that's doable (we're already a service admin account) | |
| 16:25:08 | rm_work | if it's a code change, what does that mean -- changing to doing a detach first? because that's unlikely to happen | |
| 16:25:18 | openstackgerrit | garyk proposed openstack/nova master: VMware: populate datastore refs at init https://review.opendev.org/574688 | |
| 16:25:40 | efried | Changing code to do two calls: 1) null the "owner" field, 2) delete op as you have it today | |
| 16:25:42 | rm_work | especially if you're telling us that it's really not a problem anyway given what we're doing in the big picture | |
| 16:26:10 | rm_work | ok, so just an owner null -- assuming there's no real delay in that, then that seems doable, one extra call isn't the end of the world | |