Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-04-09
14:50:19 artom bauzas, in practice, feature freeze is today :P
14:50:44 sean-k-mooney nightmare_unreal: that --overwrite kind of acts like a --force would
14:50:53 bauzas gmann: patches seem not to be piled in a branch, but rather only by topic name
14:51:05 bauzas artom: I fuckingly know about it
14:52:03 bauzas gmann: that makes reviewing a bit hard, I don't know where to start and a couple of patches got -1 from Zuul
14:52:03 gmann bauzas: correct. 2 are olso one which I are almost ready to go.
14:52:14 bauzas gmann: so I need guidance for reviews
14:52:34 bauzas gmann: I'd say https://review.opendev.org/#/c/718348/ is crucial before we move on
14:52:35 stephenfin bauzas: anything +2'd by me is passing CI
14:52:37 gmann bauzas: ok so there is always set of series for each API. let me find the review guidelines i sent on ML
14:52:55 stephenfin and good (IMO, of course)
14:52:56 bauzas stephenfin: you're Zuul \o/
14:53:08 bauzas we finally discovered it
14:53:11 stephenfin I wish I was that good at multitasking
14:53:39 bauzas stephenfin: raise 2 kids, get locked down and you'll be surprised about multitasking
14:53:45 sean-k-mooney stephenfin: e.g. it is force updating an allocation even if we dont think it needs healing
14:53:53 sean-k-mooney oh that was for nightmare_unreal ^
14:54:10 gmann bauzas: http://lists.openstack.org/pipermail/openstack-discuss/2019-August/008504.html
14:54:55 bauzas gmann: perfect, thanks
14:55:33 bauzas gmann: you'd understand that I'd hold reviews on the servers API until https://review.opendev.org/#/c/718348/ get merged ?
14:56:36 bauzas in particular given the criticity of the servers API about policy enforcement
14:56:46 gmann bauzas: yeah that make sense. if test coverage is failing somewhere then it is good to wait to have clear pic of what going to be changed in scope and new defaults patches
14:57:50 gmann bauzas: true. that is one of complex one too given the lot of inline policies per attributes .
14:57:51 stephenfin bauzas: I think you missed sean-k-mooney and I's point https://review.opendev.org/#/c/715490/11/nova/virt/libvirt/driver.py@815
14:58:08 bauzas stephenfin: did I ? shit.
14:58:28 stephenfin To be clear, I agree with the change you've made in that PS, but it's not the one we were concerned about so I think there's another one to do
14:58:59 bauzas holy shit.
14:59:09 sean-k-mooney bauzas: basically the current code logs a warning if an instance was using an mdev type that cant be recreated
14:59:13 bauzas stephenfin: I absolutely missed your concern, I apologize.
14:59:16 openstackgerrit Ivaylo Mitev proposed openstack/nova master: Implementing graceful shutdown. https://review.opendev.org/666245
14:59:24 sean-k-mooney i was saying that should cause the agent not to start
14:59:41 sean-k-mooney which i think is what stephenfin ment by make it an error too right
14:59:44 bauzas sean-k-mooney: stephenfin: lemme think about it
15:00:02 bauzas I'm honestly not sure we'd have to hardstop but I could be wrong
15:01:19 bauzas stephenfin: worth accepting a FUP for the doc nit ?
15:01:22 stephenfin bauzas: What would happen in that situation? You wouldn't be able to restart the instance using the mdev because it wasn't recreated?
15:02:16 stephenfin bauzas: Yeah, I'm not sure if we can hard fail for that case (missing '[vgpu_{name}]' sections) after all
15:02:37 stephenfin We already allowed people to list multiple values in 'enabled_vgpu_types', yeah?
15:02:43 stephenfin Even though they did nothing
15:03:18 stephenfin So it's possible there are nova.conf files in the wild with e.g. '[devices] enabled_vgpu_types = nvidia-35, nvidia-36'
15:03:21 sean-k-mooney bauzas: we could not start instance vm at that point right
15:03:45 stephenfin and if they upgrade to Ussuri, they suddenly won't be able to start nova
15:03:49 stephenfin *nova-compute
15:03:58 bauzas stephenfin: I considered this point (having set multiple types while we weren't supporting it)
15:04:22 bauzas stephenfin: I think the hardstop is a reasonable failure
15:04:53 bauzas stephenfin: for operators having set multiple types, they would see that it wasn't really supported before and now they have some action to do
15:05:08 bauzas stephenfin: instead of blindly thinking it was working
15:05:19 stephenfin right, but it was allowed and didn't break anything
15:05:28 bauzas stephenfin: we only refuse to start
15:05:38 bauzas stephenfin: people can just adapt their confs if so
15:05:52 bauzas again, we made it on purpose
15:06:11 bauzas I see this as "we won't impact you if you made things the right way"
15:06:26 bauzas 'but if you made things the wrong way, it's time to change your config to make it work correctly"
15:06:30 stephenfin idk, I thought it was a given that nova.conf from N should continue to work in N+1
15:06:34 stephenfin with warnings, sure
15:06:38 bauzas stephenfin: it will
15:07:03 bauzas stephenfin: but I can honestly revert to PS10
15:07:51 bauzas stephenfin: tbc, there is zero upgrade impact for single-typed configurations
15:08:13 bauzas it's debatable tho and I don't want to hold
15:08:18 sean-k-mooney bauzas: any chace you could add a nova-staus command for this?
15:08:32 bauzas sean-k-mooney: certainly not for Train :)
15:08:41 bauzas which is the case we're discussing about
15:08:46 sean-k-mooney ah ok hehe
15:09:05 sean-k-mooney im also not sure how useful it would be given you would have eto run it on the compute node
15:09:29 stephenfin Yeah, I think what you had before was better. "Your nova.conf is wrong and needs to be fixed. We're reverting to the old Train behaviour until you do that"
15:09:46 bauzas stephenfin: okay here is a trade-off, I can write an upgrade note saying that multiple-typed confs have to adapt their confs
15:10:06 bauzas stephenfin: okay, let's revert to PS10 then
15:10:14 sean-k-mooney that still does not adress https://review.opendev.org/#/c/715490/11/nova/virt/libvirt/driver.py@815
15:10:32 bauzas sean-k-mooney: yup, it will be PS10 with recreate hard stopping
15:10:37 bauzas we all okay ?
15:10:49 sean-k-mooney in that case you have a direct conflict between an instance type used by a guest on the current host and and the config
15:10:52 stephenfin Yeah, change where we do the hard stop and I'm happy
15:10:55 sean-k-mooney bauzas: yep
15:10:57 sean-k-mooney that works for me
15:11:06 bauzas cool, on it
15:11:13 stephenfin that's a straight up misconfiguration and should be a hard stop
15:11:35 stephenfin Might even be worth backporting a variant of that to stable/train
15:11:54 sean-k-mooney the warning
15:11:59 sean-k-mooney maybe
15:21:33 bauzas stephenfin: you're talking of the recreate issue you raised ?
15:21:57 bauzas stephenfin: apologies btw. I fucked up with your point, I thought you were arguing about what I fixed in PS11
15:22:11 bauzas looks like multitasking with kids raises bugs
15:22:27 bauzas -ETOOMANYTHINGS
15:23:53 stephenfin bauzas: Um, I am :) I'm saying I think you need to hard fail at https://review.opendev.org/#/c/715490/11/nova/virt/libvirt/driver.py@815 and *not* hard fail at https://review.opendev.org/#/c/715490/11/nova/virt/libvirt/driver.py@6531
15:24:07 bauzas yup, I finally understood
15:27:57 bauzas stephenfin: worth saying, do you think we should return a InvalidLibvirtGPUConfig within https://review.opendev.org/#/c/715490/11/nova/virt/libvirt/driver.py@815 ?
15:28:02 bauzas or another exception ?
15:28:19 stephenfin It's invalid config, so that makes sense IMO
15:28:21 bauzas I honestly think the operator messed up their config if so
15:28:26 bauzas yup, ok
15:30:29 bauzas stephenfin: to answer your last comment on https://review.opendev.org/#/c/715490/11/nova/virt/libvirt/driver.py@815 ,
15:31:05 bauzas if the operator messes up their config and self._get_vgpu_type_per_pgpu(parent) can't find the right vgpu type because $mess
15:31:25 bauzas then we get a None value and the conditional statement fails
15:32:31 bauzas stephenfin: but unless the operator did provided a section for each of the vGPU types and added devices, you're right, we fall back to only supporting one type, like we did previously
15:34:46 stephenfin cool. gtk I was reading that correctly
15:47:28 openstackgerrit Sylvain Bauza proposed openstack/nova master: Functional test with pGPUs https://review.opendev.org/717975
15:47:28 openstackgerrit Sylvain Bauza proposed openstack/nova master: Support different vGPU types per pGPU https://review.opendev.org/715490
15:47:55 bauzas stephenfin: sean-k-mooney: gibi ^
15:49:23 sean-k-mooney i like how gerrit leaves the filtes ticked if they have not changed form the last revision
15:49:27 sean-k-mooney looking now

Earlier   Later