| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-05 | |||
| 16:00:56 | mriedem | PciInvalidAlias extends Invalid | |
| 16:01:14 | mriedem | so that should return a 400 | |
| 16:01:43 | cfriesen | yeah, was just trying to track that down | |
| 16:02:20 | mriedem | if it is handled then i don't think we need anything separate | |
| 16:02:48 | mriedem | https://review.openstack.org/#/c/620706/27/nova/api/openstack/compute/servers.py@a857 | |
| 16:02:59 | cfriesen | so Fault will end up being HTTPBadRequest? | |
| 16:03:38 | cfriesen | or I guess Fault(exception.ConvertedException( | |
| 16:04:15 | mriedem | without any testing for it it's hard to say | |
| 16:05:31 | mriedem | so we get that from get_pci_requests_from_flavor which is called today from _validate_and_build_base_options in the api on server create, | |
| 16:05:45 | mriedem | i don't see any existing tests that handle the PciInvalidAlias if it's raised from there | |
| 16:05:58 | mriedem | i suspect it is handled and converted to a 400 response | |
| 16:06:05 | mriedem | but we don't have any testing for that | |
| 16:06:09 | cfriesen | I'll try a test without specially adding it to the list | |
| 16:06:16 | mriedem | unlike https://review.openstack.org/#/c/640841/2/nova/tests/unit/api/openstack/compute/test_serversV21.py | |
| 16:10:45 | cfriesen | mriedem: looks like it fails: http://paste.openstack.org/show/747293/ | |
| 16:11:41 | cfriesen | if I add PciInvalidAlias to the list in ServersController.create() , it passes | |
| 16:11:52 | mriedem | i assume those tests aren't using https://github.com/openstack/nova/blob/c43c1d3fb9da5dd0a13e1f15623a696212f095ff/nova/api/openstack/wsgi.py#L367 | |
| 16:12:03 | mriedem | those are hitting the handler directly | |
| 16:12:10 | mriedem | whereas ^ wraps the handler | |
| 16:12:10 | cfriesen | yeah, just realized that | |
| 16:13:07 | cfriesen | okay, need to hack devstack then | |
| 16:13:09 | mriedem | anywho, i think PciInvalidAlias is probably handled as a 400 and we're OK, and for completeness you could add it to that big ass list of exceptions for the validation stuff | |
| 16:13:16 | cfriesen | works | |
| 16:13:42 | mriedem | you got a 400 for an invalid alias in devstack? | |
| 16:13:52 | cfriesen | haven't done it yet | |
| 16:14:01 | mriedem | oh | |
| 16:16:59 | cfriesen | I get: ERROR (ClientException): Unknown Error (HTTP 500) | |
| 16:17:35 | mriedem | is there a stacktrace in the api logs? | |
| 16:18:39 | cfriesen | whoops...typo. I had "faise" istead of "raise" | |
| 16:19:44 | cfriesen | okay, for real this time I get this: http://paste.openstack.org/show/747294/ checking logs | |
| 16:20:38 | cfriesen | here are the logs: http://paste.openstack.org/show/747295/ | |
| 16:21:37 | cfriesen | adding the exception to the list makes it a 400 as expected | |
| 16:22:40 | cfriesen | report a bug or just put it in the git commit message? | |
| 16:27:01 | gibi | mriedem: regarding https://review.openstack.org/#/c/636360/21/releasenotes/notes/support-neutron-ports-with-resource-request-cb9ad5e9757792d0.yaml@21 I think unshelving is possible after a simple shelve without offload. And that operation is supported with bandwith as that does not need a new scheduling | |
| 16:29:37 | mriedem | efried: can we do an assertEqual(1, len(mdevs)) check in here instead? https://review.openstack.org/#/c/639854/4 | |
| 16:29:42 | jaypipes | mriedem: off it goes. | |
| 16:29:58 | efried | ... | |
| 16:30:19 | mriedem | gibi: yeah true | |
| 16:32:35 | jaypipes | wouldn't it be great if Nvidia developers actually participated in the community of people trying to enable their hardware? :( | |
| 16:33:46 | efried | mriedem: responded | |
| 16:35:15 | mriedem | efried: fwiw i also commented in bauzas's change that we should just do that mdev naming conversion from dashes to underscores in _get_mediated_device_information | |
| 16:35:24 | mriedem | so callers, like the libvirt driver and your test, don't need to care about that conversion | |
| 16:35:37 | mriedem | gah https://review.openstack.org/#/c/639647/ | |
| 16:35:49 | mriedem | i was going to -1 because there are other follow ups from this series i'd like to see | |
| 16:35:51 | mriedem | i guess i'll do those | |
| 16:36:38 | efried | mriedem: So do you want me to fix the test patch, or are you going to fup? | |
| 16:37:34 | mriedem | i'm gonna fup all night long | |
| 16:37:46 | efried | ight | |
| 16:39:00 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Delete orphaned RS and IM records of archived instances https://review.openstack.org/560042 | |
| 16:40:14 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support server create with ports having resource request https://review.openstack.org/636360 | |
| 16:40:52 | gibi | mriedem: fixed your comments in ^^ | |
| 16:41:11 | sean-k-mooney | jaypipes: ya nvida and opensource is not really a thing... | |
| 16:41:38 | sean-k-mooney | jaypipes: unless its there "opensouce" project | |
| 16:42:59 | mriedem | melwitt: there is a lot of stuff in https://blueprints.launchpad.net/nova/stein that is going to be deferred, do you want to just start moving stuff out now to make that page more clear? | |
| 16:43:10 | mriedem | https://etherpad.openstack.org/p/nova-stein-blueprint-status is getting to be kind of a mess | |
| 16:43:32 | mriedem | so would be nice if the bp pages in lp was cleaned up, or https://etherpad.openstack.org/p/nova-stein-blueprint-status was cleaned up to remove stuff that's either done or not going to make it | |
| 16:43:43 | melwitt | mriedem: yeah :( I can try | |
| 16:44:07 | mriedem | well if you're working on the counting stuff that's probably higher priority | |
| 16:46:10 | melwitt | dansmith: I updated the user_id patch last night to remove the loading stuff, fyi. from your comment it sounds like the only thing missing is to do a log message in case someone tries to load an unset user_id | |
| 16:46:41 | sean-k-mooney | am should we just add another heading for defered stuff at the bottom. | |
| 16:46:42 | melwitt | mriedem: oh, sorry I misunderstood "do you want to ...". yeah, I think that is fine | |
| 16:46:56 | bauzas | mriedem: sorry, missed your comments on the naming thing | |
| 16:47:04 | bauzas | and I was off here for an hour | |
| 16:51:51 | openstackgerrit | Chris Friesen proposed openstack/nova master: Handle missing exception in instance creation code https://review.openstack.org/641082 | |
| 16:53:19 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Ensure that bandwidth and VF are from the same PF https://review.openstack.org/623543 | |
| 16:53:20 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support server create with ports having resource request https://review.openstack.org/636360 | |
| 16:54:27 | cfriesen | mriedem: fix for PciInvalidAlias issue. I reported a bug for it too. | |
| 16:54:46 | mriedem | cfriesen: but you got a 500 for that in the api with devstack? | |
| 16:55:00 | cfriesen | mriedem: yes, see above | |
| 16:55:17 | cfriesen | here are the logs: http://paste.openstack.org/show/747295/ | |
| 16:56:18 | cfriesen | CLI output is in the bug: https://bugs.launchpad.net/nova/+bug/1818701 | |
| 16:56:20 | openstack | Launchpad bug 1818701 in OpenStack Compute (nova) "invalid PCI alias in flavor results in HTTP 500 on instance create" [Medium,In progress] - Assigned to Chris Friesen (cbf123) | |
| 16:58:37 | mriedem | cfriesen: ok just robustify the test and i'm +2 | |
| 17:00:11 | openstackgerrit | Theodoros Tsioutsias proposed openstack/nova master: Add requested_networks to RequestSpec https://review.openstack.org/570201 | |
| 17:00:12 | openstackgerrit | Theodoros Tsioutsias proposed openstack/nova master: Enable rebuild for instances in cell0 https://review.openstack.org/570203 | |
| 17:01:45 | mriedem | gibi: can we mark this complete? https://blueprints.launchpad.net/nova/+spec/use-nested-allocation-candidates | |
| 17:02:14 | gibi | mriedem: there is one intentionally open patch https://review.openstack.org/#/c/608298/ | |
| 17:02:32 | gibi | mriedem: it is an edge case that we don't need in nova right now | |
| 17:02:34 | mriedem | "This cannot happen today but will in the future, when cpu and memory resource are moved to the NUMA RP and servers" | |
| 17:02:42 | gibi | mriedem: exactly | |
| 17:02:47 | mriedem | correct, so i'd just say it becomes a part of those bps when they happen | |
| 17:03:01 | gibi | mriedem: OK. I agree. Let's mark the bp completed | |
| 17:04:37 | mriedem | done | |
| 17:04:55 | openstackgerrit | Chris Friesen proposed openstack/nova master: Handle missing exception in instance creation code https://review.openstack.org/641082 | |
| 17:05:12 | cfriesen | mriedem: I also robustified the other PCI test | |
| 17:05:17 | mriedem | oh my | |
| 17:05:20 | mriedem | that's a -5 | |
| 17:05:44 | mriedem | i hope you ran fast8 | |
| 17:05:47 | mriedem | but +2 | |
| 17:06:09 | cfriesen | I can break out the other test as a separate commit if you want... | |
| 17:06:16 | mriedem | i'm just joking | |
| 17:06:45 | mriedem | -5 is a joke unless we're talking about forced cold migration | |
| 17:11:39 | sean-k-mooney | i tought forced migrations tradtional add a couple fo factorals added on like so -5!! | |
| 17:12:47 | mriedem | -5?!?!??!! | |
| 17:12:54 | mriedem | jazz it up however you want | |
| 17:13:49 | melwitt | reshaper + func test is finally on its way to the sarlacc pits \o/ | |
| 17:14:22 | dansmith | melwitt: oh, I saw the comment wasn't on the latest PS, but I hadn't gone back to see what had changed | |
| 17:15:12 | melwitt | dansmith: understood. just giving you the 411 | |
| 17:21:28 | mriedem | mdbooth: you might want to take a look at this https://review.openstack.org/#/c/613039/ | |
| 17:21:33 | mriedem | if it has a chance of making stein | |