| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-11 | |||
| 15:16:24 | artom | Public ones | |
| 15:16:30 | sean-k-mooney | what methods are you adding | |
| 15:16:43 | sean-k-mooney | that moduel shoudl really only have methods that call neutron | |
| 15:16:43 | artom | The get_events stuff, see convo with dansmith above | |
| 15:17:01 | sean-k-mooney | its does not interact with the api | |
| 15:17:08 | sean-k-mooney | so why would it be in that module | |
| 15:17:30 | artom | TBH, because dansmith said so, and it makes sense to me as well | |
| 15:17:37 | artom | But mostly because he said so ;) | |
| 15:17:50 | artom | This is not something I have the energy to debate | |
| 15:17:59 | dansmith | artom: don | |
| 15:18:00 | dansmith | artom: don't hedge :) | |
| 15:18:44 | efried | stephenfin: I'm stumped. Are you getting anywhere? If not, I'm going to call time and just disable the test for now. | |
| 15:18:54 | sean-k-mooney | dansmith: why in the api module and not here https://github.com/openstack/nova/blob/master/nova/network/model.py#L460 | |
| 15:18:58 | stephenfin | efried: Yeah, I'm down in pycodestle | |
| 15:19:02 | stephenfin | *pycodestyle | |
| 15:19:17 | dansmith | sean-k-mooney: I said nova/network/something would be fine too | |
| 15:19:47 | artom | I wasn't aware of the "must call neutron API" rule | |
| 15:20:04 | efried | stephenfin: Was that "yeah I'm getting somewhere" or "yeah disable the test"? | |
| 15:20:14 | dansmith | artom: I'm aware of no such rule | |
| 15:20:25 | sean-k-mooney | artom: im not sure it a rule but that module is nova.network.neutronv2.api | |
| 15:20:37 | artom | sean-k-mooney, actually yeah, with the new abstraction suggested by dansmith (plug-time vs bind-time), it fits in model as well | |
| 15:20:37 | stephenfin | efried: Disable it. I'll keep looking | |
| 15:20:43 | stephenfin | Here's our issue though https://github.com/PyCQA/pycodestyle/blob/2.0.0/pycodestyle.py#L1639 | |
| 15:20:57 | stephenfin | We shouldn't be getting there but I don't know yet what's changed | |
| 15:20:59 | sean-k-mooney | pretty much everything it does interacts with the neutron api in one way or another | |
| 15:21:30 | dansmith | sean-k-mooney: we have constants defined in that module, and this detail is completely related to "what to expect from the neutron api" so I have no problem saying it should (or can) be in there | |
| 15:21:34 | stephenfin | Things like this are why I was trying to get us onto a newer hacking version. Things invariably break | |
| 15:21:49 | efried | stephenfin: aren't we on a newer hacking version at this point? | |
| 15:21:49 | sean-k-mooney | artom: plugtime and bind time is more or less what i was thinking about in the neutron spec too so that makes sense to me | |
| 15:22:03 | stephenfin | not really. Hacking is relying on a really old version of flake8 | |
| 15:22:08 | artom | kawn sen suss! | |
| 15:22:11 | stephenfin | Like, super old | |
| 15:23:09 | stephenfin | and I can't update because they broke all the things in flake8 3.x :( https://gitlab.com/pycqa/flake8/issues/545 | |
| 15:23:46 | sean-k-mooney | ok well its not where i would instintivly look for them but i can live with it. | |
| 15:25:05 | sean-k-mooney | artom: to your orginal question https://github.com/openstack/nova/blob/master/nova/network/api.py isthe old nova networks api and the neton one extends it | |
| 15:25:16 | sean-k-mooney | i think you can ignore https://github.com/openstack/nova/blob/master/nova/network/api.py at this point | |
| 15:25:48 | artom | sean-k-mooney, ack, thanks for looking :) | |
| 15:26:04 | artom | (Though tbf my original question was rendered moot when we agreed to put it in model) | |
| 15:27:31 | sean-k-mooney | artom: :) by the way i proposed https://review.opendev.org/#/c/664442/ to test this | |
| 15:28:13 | artom | sean-k-mooney, yep, thanks for that | |
| 15:30:17 | openstackgerrit | Eric Fried proposed openstack/nova master: Skip test_check_doubled_words hacking check UT https://review.opendev.org/664622 | |
| 15:30:20 | efried | stephenfin: ^ | |
| 15:35:24 | efried | dansmith: mind unblocking the gate ^ | |
| 15:45:26 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add extra spec parameter and image property for memory encryption https://review.opendev.org/664420 | |
| 15:45:39 | aspiers | efried: vastly improved patchset based on your feedback ^^^ | |
| 15:47:31 | openstackgerrit | Sylvain Bauza proposed openstack/nova-specs master: Proposes NUMA affinity for vGPUs https://review.opendev.org/650963 | |
| 15:48:26 | efried | aspiers: looking. | |
| 15:49:51 | stephenfin | efried, cdent: Here's what's happening | |
| 15:50:12 | stephenfin | Because of https://bugs.python.org/issue33899, we now insert a NEWLINE token at the end of all lines | |
| 15:50:26 | stephenfin | that manifests itself in pycodestyle | |
| 15:50:45 | stephenfin | which is interating through all the tokens on the test line | |
| 15:50:46 | stephenfin | https://github.com/PyCQA/pycodestyle/blob/2.0.0/pycodestyle.py#L1616 | |
| 15:51:45 | stephenfin | previously, when we got to the end of the line we were falling through to this case https://github.com/PyCQA/pycodestyle/blob/2.0.0/pycodestyle.py#L1650-L1652 | |
| 15:52:09 | stephenfin | but now, because we see this newline token, we hit this case instead https://github.com/PyCQA/pycodestyle/blob/2.0.0/pycodestyle.py#L1632-L1635 | |
| 15:52:26 | stephenfin | note that that case doesn't include a call to 'check_physical' | |
| 15:52:33 | stephenfin | so our test never gets run | |
| 15:53:06 | stephenfin | I don't know how to fix that. It feels like this is now a bug in pycodestyle | |
| 15:54:08 | stephenfin | I note that the latest version of pycodestyle does pretty much the same thing too https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2104-L2107 | |
| 15:54:33 | cdent | stephenfin: sorry stephenfin, I thought most of what you said above was already understood: and it being a bug on pycodestyle was known | |
| 15:54:49 | cdent | or rather: pycodestyle's continued behavior is the root | |
| 15:54:56 | stephenfin | Um, whut? :) | |
| 15:55:02 | stephenfin | Clearly I didn't read far enough | |
| 15:55:37 | cdent | Or maybe I just reached that conclusion based on the discussion on https://bugs.python.org/issue33899 which if I recall right suggests the same thing | |
| 15:56:37 | stephenfin | Yup, now that I search for pycodestyle I see the line of course. Dang it | |
| 15:56:47 | stephenfin | Their fix is rather odd though https://github.com/PyCQA/pycodestyle/pull/801/files | |
| 15:58:06 | openstackgerrit | Adam Spiers proposed openstack/nova master: Pass extra_specs to flavor in vif tests https://review.opendev.org/662556 | |
| 16:01:45 | aspiers | efried: What's supposed to happen when hw:mem_encryption=True but trait:HW_CPU_X86_AMD_SEV=required is missing? | |
| 16:02:05 | efried | aspiers: How could that happen? | |
| 16:02:12 | efried | oh | |
| 16:02:18 | aspiers | efried: Should the filter automatically add it for now, but then later error if we add support for MKTME? | |
| 16:02:32 | efried | No, we should never add the trait. | |
| 16:02:40 | stephenfin | efried: If you apply this to '.tox/py27/lib/python2.7/site-packages/pycodestyle.py' it fixes things, yeah? http://paste.openstack.org/show/752752/ | |
| 16:03:01 | aspiers | OK, so then we need to error if it's missing, which somewhat reduces the user-friendliness benefit of hw:mem_encryption | |
| 16:03:03 | efried | aspiers: You're assured of landing on capable hosts because they're the only ones that can satisfy the MEM_ENCRYPTION_CONTEXT inventory. | |
| 16:03:05 | efried | No | |
| 16:04:05 | efried | if the image/flavor prop is specified, it means I want mem encryption | |
| 16:04:05 | efried | if I care that it's SEV, I specify the SEV trait and land on an AMD host | |
| 16:04:05 | efried | if I care that it's MKTME, I specify the MKTME trait and land on an Intel host | |
| 16:04:05 | efried | if I don't care which, I leave the trait out and land on either one. | |
| 16:04:21 | aspiers | Sure, but I'm talking about the last case | |
| 16:04:35 | aspiers | In that case, there needs to be logic *somewhere* in the code which says "OK, the user wants encrypted memory, and the only way to achieve that currently is SEV" | |
| 16:04:47 | aspiers | so that it triggers usage of all the SEV bits | |
| 16:04:52 | efried | No. | |
| 16:04:52 | efried | I never land on a non-capable host, because only capable hosts have MEM_ENCRYPTION_CONTEXT inventory. | |
| 16:05:13 | efried | Today the only hosts exposing that inventory will be SEV-capable hosts, so that falls into place. | |
| 16:05:15 | aspiers | Sure | |
| 16:05:22 | aspiers | but where does that logic go? ^^^ | |
| 16:05:40 | efried | It's implicit. It's free. It doesn't "go" anywhere. | |
| 16:05:45 | aspiers | It's not enough to land on the right host | |
| 16:05:55 | aspiers | nova needs to know to add all the extra bits to the guest config | |
| 16:06:11 | efried | extra bits | |
| 16:06:21 | aspiers | yes, like <launchSecurity /> and <locked /> | |
| 16:06:28 | aspiers | and a whole bunch of iommu stuff | |
| 16:06:28 | efried | always the same if I'm SEV, right? | |
| 16:07:13 | aspiers | Not sure I understand the question | |
| 16:07:28 | aspiers | That extra config varies depending on a whole bunch of things | |
| 16:07:45 | efried | okay, but the virt driver knows how to twiddle the guest config based on a) mem_encryption flavor/image prop, and b) fact I'm on an AMD host | |
| 16:08:21 | efried | If there are user-/admin-configurable options beyond "yes" or "no", we should talk about where those should go. | |
| 16:08:33 | efried | But my understanding was that from the admin/user pov it was "yes" or "no". | |
| 16:08:39 | aspiers | It is | |