Earlier  
Posted Nick Remark
#openstack-nova - 2019-09-06
21:05:02 mriedem that's better than before, but the list of nodes in json string is not great,
21:05:23 sean-k-mooney is that the output for the toplogy api?
21:05:25 mriedem but if you dumped each node dict as a separate property, what would you name the property? because that host_node field is only in the response for admins,
21:05:29 mriedem yeah, in the cli
21:05:31 mriedem proposed cli
21:05:55 mriedem so if you're an admin, you could name each one as node{host_node} so like node0 and node1
21:06:01 sean-k-mooney can we pretty print the json blob
21:06:11 mriedem but if you're not admin, calling them node0 or node1 could make the user think they are actually on those physical host numa nodes
21:06:57 mriedem hmm, maybe
21:14:58 sean-k-mooney its a litte more readable. http://paste.openstack.org/show/772112/ i didnt put it in the ascis table
21:15:11 sean-k-mooney but you could also do somithing like
21:15:44 mriedem that's what i already have http://paste.openstack.org/show/772111/
21:16:02 sean-k-mooney |node.[0].cpu_pinning | {"0": 0,"1": 5}
21:16:10 mriedem anyway, i've got this better than it was so i'll push it up and someone else can hack on it
21:16:22 mriedem it's not even my patch
21:17:48 sean-k-mooney the latest windows update sucks ...
21:17:57 mriedem 10...?
21:18:02 sean-k-mooney ya
21:18:07 mriedem i'm rocking 7
21:18:28 sean-k-mooney it install an update last night and now its haning if i boot it in a vm
21:18:49 sean-k-mooney if i reboot the host indo the same windwos install its fine
21:21:35 sean-k-mooney i did like 7. i think i still have my 7 ultimate 64bit dvd around somewhere.
21:21:57 mnaser efried_afk: thanks for driving the SIGHUP patches stuff, i ran out of bandwidth
21:22:15 efried mnaser: yahyoubetcha. Glad to have that fixed finally.
21:22:37 prometheanfire os-brick update is breaking nova :( https://review.opendev.org/680339
21:22:59 efried prometheanfire: does cinder know?
21:23:04 efried (I didn't click yet)
21:23:15 mnaser AttributeError: 'module' object has no attribute 'NVMeConnector'
21:23:50 mnaser https://github.com/openstack/os-brick/commit/0f057aee6e0351bdef68fadf1fc56ac315027c98
21:23:53 prometheanfire no, test fails were in nova so pinged you first
21:24:01 mnaser im guessing those are the fails
21:24:11 mnaser the module was renamed
21:24:24 openstackgerrit Matt Riedemann proposed openstack/python-novaclient master: Microversion 2.78 - show server topology https://review.opendev.org/670790
21:25:15 sean-k-mooney are the nova unit test mocking stuff in os-bricks
21:25:20 prometheanfire try import except import?
21:25:33 mnaser btw
21:25:36 mnaser there is some context here https://review.opendev.org/#/c/643421/
21:27:08 sean-k-mooney ok so we are importing the conector object in the test
21:27:41 mnaser `initiator.NVME` that was left around for backwards compat
21:28:01 mnaser but the connector did not keep a backwards compat
21:29:07 mnaser i mean as a work around you could fall back to asserting the initiator used instead of the connector used
21:29:45 sean-k-mooney so the real driver code use the factory method to consurct it
21:29:47 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/virt/libvirt/volume/nvme.py#L34
21:30:04 mnaser sean-k-mooney: it uses `initiator.NVME` which is still around
21:30:23 sean-k-mooney i think the real fix is to not assert thing abot the data type of an external lib
21:30:39 mnaser https://github.com/openstack/os-brick/blob/0f057aee6e0351bdef68fadf1fc56ac315027c98/os_brick/initiator/connector.py#L117-L122
21:30:40 mnaser cause thats still around
21:30:52 sean-k-mooney mnaser: yes but i guess they considerd as part of the public interface
21:31:01 sean-k-mooney and the conector object are not
21:31:30 mnaser maybe the test should mock `connector.InitiatorConnector.factory` and make sure it calls it with initiator.NVME ?
21:31:39 mnaser which is enough to know we are doing the right thing(tm)
21:32:00 sean-k-mooney maybe but its stil mocking stuff we dont own
21:32:06 sean-k-mooney but that seams safer
21:32:16 sean-k-mooney sice we are mocking somethign we are calling
21:32:16 mnaser well at least it makes sure that we're creating a connector with the right type of initiator
21:32:29 mnaser which i think is the biggest thing that test is supported to do..
21:32:43 sean-k-mooney right but that is not novas job to test
21:32:45 openstackgerrit Matt Riedemann proposed openstack/nova master: api-ref: fix server topology "host_numa_node" field param name https://review.opendev.org/680775
21:32:50 sean-k-mooney that is os-bricks job to test
21:33:03 sean-k-mooney we shoudl jsut assert we called ti with the correct input
21:33:18 mnaser exactly thats what i meant about mocking `connector.InitiatorConnector.factory`
21:33:31 mnaser aka make sure we're calling it with initiator.NVME as an argument (instead of $some_other_weird_thing)
21:33:32 sean-k-mooney ya which seams more robust
21:33:41 sean-k-mooney yep
21:33:58 mnaser its friday and im waiting for my food
21:34:01 efried dansmith: responded in https://review.opendev.org/671075
21:34:02 mnaser let me push up a patch quick..
21:35:04 dansmith efried: ack, but I'm EOD
21:35:10 efried nod
21:36:52 dansmith replied for action
21:38:18 efried thanks
21:41:07 mriedem it's in-law o'clock
21:41:07 mriedem o/
21:48:30 openstackgerrit Adam Spiers proposed openstack/nova master: Apply SEV-specific guest config when SEV is required https://review.opendev.org/644565
21:49:16 openstackgerrit Adam Spiers proposed openstack/nova master: Reject live migration and suspend on SEV guests https://review.opendev.org/680158
21:52:17 openstackgerrit Mohammed Naser proposed openstack/nova master: Decouple NVMe tests from os-brick https://review.opendev.org/680779
21:52:28 mnaser sean-k-mooney: ^ unit tests and flake8 passes here locally
21:53:14 openstackgerrit Adam Spiers proposed openstack/nova master: Enable booting of libvirt guests with AMD SEV memory encryption https://review.opendev.org/666616
21:53:29 mnaser efried: fyi the requirements breakage should be solved by that ^
21:53:41 mnaser oh rap
21:53:45 mnaser i have an extra file there
21:53:55 openstackgerrit Mohammed Naser proposed openstack/nova master: Decouple NVMe tests from os-brick https://review.opendev.org/680779
21:54:54 efried mnaser: ugh, do we really need root helper?
21:55:07 mnaser efried: i mean i can skip the mock and import utils ?
21:55:19 mnaser but i mean its being passed down there
21:55:49 efried sorry, I just remember recently seeing where we were trying to get rid of that thing, so adding another reference to it feels like increasing tech debt.
21:55:54 mnaser https://docs.openstack.org/os-brick/stein/reference/os_brick/initiator/connector.html
21:56:03 mnaser well i only touched the tests, it was already referencing it
21:56:13 mnaser and it looks like the library relies on needing it
21:56:30 efried yeah, it just makes me sad.
21:56:58 efried mnaser, prometheanfire: Ima update the req patch to dep-on this guy to make sure it clears up, mkay?
21:57:09 mnaser efried: i mean i can get rid of the mock and have it call the real function but still gonna have a reference at the end of teh day
21:57:24 efried don't worry about it.
21:57:36 mnaser cools
22:07:27 aspiers efried: woot, SEV tempest test is working, exercising many code paths, and even found a bug which is now fixed :)
22:07:39 aspiers now I just have to wait a week for Zuul to catch up :(
22:12:01 efried nice
22:12:10 efried aspiers: did you do the tempest work?
22:12:13 aspiers yes
22:12:30 aspiers I'd never touched tempest before yesterday

Earlier   Later