Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-04-02
10:33:09 stephenfin brinzhang_: Actually, in the setUp, is there any reason not to configure 'self.req' with a version argument?
10:33:32 stephenfin You're testing policy, not microversion support, so you'll always want to use a known working version, right?
10:33:52 brinzhang_ yes
10:35:45 brinzhang_ stephenfin: you mean I should change self.req to req in setup()?
10:35:52 stephenfin no
10:36:24 stephenfin instead of self.req = fakes.HTTPRequest.blank('')
10:36:33 stephenfin do self.req = fakes.HTTPRequest.blank('', version='2.85')
10:36:49 stephenfin here https://review.opendev.org/#/c/711194/12/nova/tests/unit/policies/test_volumes.py@78
10:37:19 brinzhang_ no, it dosenot only for v2.85
10:37:35 brinzhang_ so I just used req in v2.85 policy tests
10:37:36 stephenfin ah, I see now, this is only for the 'update_volume' case
10:37:41 brinzhang_ yes
10:37:49 stephenfin gotcha
10:37:57 brinzhang_ req.environ['nova.context'] = self.system_foo_context
10:38:11 stephenfin yup, that'll do so ^
10:38:13 brinzhang_ I added this, it run ok now, will update
10:38:18 brinzhang_ thansk stephenfin, gibi
10:47:28 openstackgerrit Brin Zhang proposed openstack/nova master: Separate update and swap volume policies https://review.opendev.org/711194
10:47:33 brinzhang_ gibi, stephenfin: done thanks
10:48:43 brinzhang_ stephenfin: can you +A for this patch https://review.opendev.org/#/c/693828/25? it's ready for everything, that comment inline, I will fixed by FUP
10:50:39 stephenfin brinzhang_: I haven't looked at that in a while so I'll need a while
10:50:52 openstackgerrit Stephen Finucane proposed openstack/nova master: api: Add framework for extra spec validation https://review.opendev.org/704643
10:50:53 openstackgerrit Stephen Finucane proposed openstack/nova master: api: Add microversion 2.85, extra spec validation https://review.opendev.org/708436
10:50:53 openstackgerrit Stephen Finucane proposed openstack/nova master: api: Add support for new cyborg extra specs https://review.opendev.org/716222
10:50:54 openstackgerrit Stephen Finucane proposed openstack/nova master: docs: Add documentation for flavor extra specs https://review.opendev.org/710037
10:51:35 stephenfin gibi: addressed the comments from Sundar and removed those errant prints from the last doc patch ^
10:51:38 brinzhang_ stephenfin, gibi, gmann: thanks, hope you have a greate day ^^
10:52:47 openstackgerrit Sylvain Bauza proposed openstack/nova master: Provide the parent pGPU when creating a new vGPU https://review.opendev.org/715489
10:52:48 openstackgerrit Sylvain Bauza proposed openstack/nova master: WIP: Support different vGPU types per pGPU https://review.opendev.org/715490
10:52:53 bauzas sean-k-mooney: ^
11:03:45 openstackgerrit Merged openstack/nova master: Add new default roles in remote console policies https://review.opendev.org/716484
11:38:31 gibi stephenfin: ack, will look shortly
11:41:27 gibi stephenfin: I'm all the way +2 on that series. Do you have a novaclient change as well?
12:25:26 stephenfin bauzas: You think you could review https://review.opendev.org/#/c/710037/8/doc/ext/extra_specs.py ? It's not the least bit complicated, if that helps :)
12:25:38 bauzas oh yeah
12:25:42 stephenfin Happy to trade reviews
12:34:20 sean-k-mooney bauzas: at the moment i assume you are not allowing multiple vGPU types on a single pGPU
12:34:41 sean-k-mooney bauzas: there is noting in princiapl that prevents that form working even if nvida does not support that
12:35:21 sean-k-mooney the vfio mediated device framework fully support using multiple mdev_types on the same devcie concurrnetly
12:35:26 alex_xu anyone know why we can swap the same volume before https://review.opendev.org/#/c/693828/25/nova/api/openstack/compute/volumes.py@495...
12:36:35 sean-k-mooney alex_xu: lyarwood is on pto i think but i think we can may do that for a volume retype on the cinder side
12:36:38 alex_xu oh, I guess we can't, that will fail at later voluem status check
12:36:53 alex_xu sean-k-mooney: ah, thanks
12:37:25 sean-k-mooney alex_xu: im not sure if my guess above was correct by the way. we could very well just fail later as you said
12:37:41 alex_xu yea
12:38:13 sean-k-mooney but i think cinder allows use to retype a voule while its attached to an instance and this is the only way i can think if for it to tell nova that it has updated
12:39:20 alex_xu sean-k-mooney: ok, thanks
12:41:26 sean-k-mooney alex_xu: i didnt review the rest of that patch but just -1'd it since its aliasing the id builtin fucntion and using id as a varible which is wrong
12:41:44 sean-k-mooney in this context it wont break anything but we should never do that
12:51:28 alex_xu sean-k-mooney: I'm not sure we can check that, I need to check the api code. we use 'id' in many API. but I agree that seems wrong
12:53:22 sean-k-mooney im not sure we call id directly in nova since we normally dont care about the objects address in memory
12:53:42 sean-k-mooney but its just not good practic to alias builtins in general
12:54:20 sean-k-mooney it would only really be an issue if you did "id=dir" or something at module scope
12:54:47 sean-k-mooney inside a function it will jsut make id unavialbel in that function body
12:57:37 sean-k-mooney but if you do python3 -c "id=dir; help(id)" it will show the help for dir so if you do that at module scope anything that uses id after will get dir instead
12:58:16 alex_xu yea, agree with that
12:58:39 alex_xu sean-k-mooney: this is where id coming from https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/routes.py#L820
13:00:12 sean-k-mooney ah i see ya we proably shoucl change that to <resouce>_id for all of the usages and add a hacking check.
13:00:33 sean-k-mooney that is a seperate change however so ill remove my -1
13:00:46 sean-k-mooney this is an existing bug
13:02:43 openstackgerrit Huaqiang Wang proposed openstack/nova master: hardware: Add support for 'hw:cpu_dedicated_mask' extra spec https://review.opendev.org/714706
13:02:43 openstackgerrit Huaqiang Wang proposed openstack/nova master: Introduce the 'CPUAllocationPolicy.MIXED' enum https://review.opendev.org/716267
13:02:44 openstackgerrit Huaqiang Wang proposed openstack/nova master: Introduce 'pcpuset' field for InstanceNUMACell https://review.opendev.org/716984
13:19:44 artom stephenfin, could you take a look at the bottom patch in the NUMA LM series? Dan Smith is +2, if you agree, we can at least get the first 2 patches in that series in the gate
13:19:54 stephenfin sure
13:20:06 artom The rest can wait until after FF I guess, since there are probably more important series to review
13:20:26 artom stephenfin, err, link: https://review.opendev.org/#/c/714228/2
13:21:43 bauzas sean-k-mooney: that's a correct assumption, I prevent such thing (supporting multiple types concurrently for *each* pGPU)
13:22:38 sean-k-mooney bauzas: ok. well that is not limitation of mdevs just of the nova/nvida usage
13:22:52 sean-k-mooney bauzas: that is fine for now in anycase
13:22:53 bauzas sean-k-mooney: like I also doesn't want to support yet asking for multiple allocations using groups
13:23:16 bauzas i limit to only one allocation for the VGPU resource class
13:23:25 bauzas I mean, people can ask VGPU>1
13:23:29 bauzas (of course)
13:23:40 bauzas but not with numbered request groups
13:23:45 sean-k-mooney ya that is fine
13:24:04 sean-k-mooney i dont think we shoudl use numbered request groups in teh flavor anyway so im fine with that
13:24:50 bauzas I don't return an exception for this
13:24:54 bauzas I just take the first one
13:25:25 sean-k-mooney that could be an improvement to add as a followup
13:25:39 sean-k-mooney bauzas: its documented which is the important thing
13:25:46 bauzas yup
13:28:21 bauzas sean-k-mooney: yeah I know the mdev framework does that
13:28:41 bauzas (I mean having multiple vGPU types per pGPU=
13:29:13 bauzas but then if you create one mdev with the nvidia driver, then automatically and magically the available instances of vGPU for the other types turn to 0 :D
13:29:28 bauzas (at least GRID10 AFAIK)
13:30:23 bauzas anyway, rushing to finish my change so that I can decently review stephenfin's extra spec validation thingies https://review.opendev.org/#/c/704643/20
13:53:36 openstackgerrit jayaditya gupta proposed openstack/nova master: Support for nova-manage placement heal_allocations --cell https://review.opendev.org/714459
13:54:57 nightmare_unreal mriedem: if you get some time can you see this : https://review.opendev.org/#/c/714459/ . I can't seem to figure out the issue with the unit test case I wrote. Thank you :) . I have fixed rest of the points you mentioned
14:15:16 mriedem nightmare_unreal: this? "Exception: This test uses methods that set internal oslo_db state, but it does not claim to use the database. This will conflict with the setup of tests that do use the database and cause failures later."
14:15:40 mriedem that means the unit test class is using NoDBTestCase or whatever it's called, which means something in the test is trying to make a database connection and isn't allowed,
14:16:10 mriedem so either move that test to the functional test class where there is a DB connection, or you have to mock out the DB call to raise the NotFound error or whatever for the test
14:16:46 mriedem your test can mock objects.CellMapping.get_by_uuid to raise CellMappingNotFound
14:17:59 mriedem @mock.patch('nova.objects.CellMapping.get_by_uuid', side_effect=exception.CellMappingNotFound('uuid')
14:18:05 mriedem something like that as a decorator to the test case method
14:20:19 openstackgerrit Merged openstack/nova master: Combine the limits policies in single place https://review.opendev.org/715678
14:28:28 nightmare_unreal Thanks !!
14:35:11 openstackgerrit jayaditya gupta proposed openstack/nova master: Support for nova-manage placement heal_allocations --cell https://review.opendev.org/714459
14:35:37 dansmith brinzhang_: are you working on feedback on the PUT patch?
14:41:43 francoisp stephenfin, just to draw your attention, I answered your last question on https://review.opendev.org/#/c/669674/10 - thanks!

Earlier   Later