| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-11-17 | |||
| 15:52:49 | bauzas | dansmith: if we would say total=0 then the driver would ask to remove the GPU | |
| 15:53:10 | dansmith | right, so just internal notation for us to know what to do when we update placement | |
| 15:53:22 | dansmith | I guess I'm not sure what you're asking then | |
| 15:53:38 | bauzas | okay, then nevermind | |
| 15:53:51 | bauzas | I'll create a test and we'll discuss with gerrit | |
| 15:53:59 | dansmith | I was also going to ask on that patch why you continue to have providers above the limit, but with total=0, but I thought maybe that was related to cleanup for people who already have more providers than they should | |
| 15:54:05 | dansmith | okay | |
| 15:54:07 | bauzas | tbc, sriov gpus only have a total=1 for each of the VFs | |
| 15:54:23 | bauzas | so it's simple | |
| 15:54:34 | bauzas | either we accept a VF or we don't | |
| 15:54:57 | bauzas | but for non-sriov gpus, we can have a capacity for say 8 or 16 | |
| 15:55:12 | bauzas | so we create an inventory with a total=16 | |
| 15:55:39 | bauzas | now, the question is, what would happen if the operator would add a max_instances value for the type ? | |
| 15:55:57 | dansmith | just total=min(actual, max_instances) right? | |
| 15:56:10 | bauzas | that's my question | |
| 15:56:15 | dansmith | why not? | |
| 15:56:25 | bauzas | because for the moment, we have https://review.opendev.org/c/openstack/nova/+/899625/4/nova/virt/libvirt/driver.py#8144 | |
| 15:56:36 | bauzas | we calculate per GPU (or VF) | |
| 15:57:09 | bauzas | and if the number of possible vGPU for it is above the max_instances value, then we eventually say "heh, no, total=0" | |
| 15:57:18 | bauzas | so we don't really cap the GPU, we rather don't accept it | |
| 15:57:28 | bauzas | which is fine too | |
| 15:57:46 | dansmith | oh I see, you're saying your current patch doesn't add the inventory _at_all_ if they specify max_instances<total instead of just capping it? | |
| 15:58:01 | dansmith | I didn't pick up on that when I looked, but.. why would we do that? | |
| 15:58:28 | dansmith | isn't the point of this to be able to say "I know it says 16 available, but we can really only allocate 2 of this mdev_type because it takes so much of the card's resource" ? | |
| 15:58:40 | dansmith | if you don't allow them to set max_instances below total, we lose the ability to do that, no? | |
| 15:59:05 | bauzas | you know what ? I can upload my change | |
| 15:59:11 | bauzas | I'm done with the unitteests | |
| 15:59:45 | bauzas | I can create a functional test (and I want to) but for the sake of discussing, let me just upload it now | |
| 16:00:13 | opendevreview | Sylvain Bauza proposed openstack/nova master: WIP: libvirt: Cap with max_instances GPU types https://review.opendev.org/c/openstack/nova/+/899625 | |
| 16:01:57 | dansmith | okay, but .. the above question is what I need answered to really say anything... | |
| 16:02:33 | dansmith | but I can put that comment in gerrit if you prefer | |
| 16:04:19 | opendevreview | Sylvain Bauza proposed openstack/nova master: WIP: libvirt: Cap with max_instances GPU types https://review.opendev.org/c/openstack/nova/+/899625 | |
| 16:04:28 | bauzas | dansmith: I added some comments ^ | |
| 16:04:53 | dansmith | okay I'll just drop this comment/question in gerrit and we can iterate from there.. I have a call starting in a bit anyway | |
| 16:05:51 | bauzas | dansmith: fwiw, haven't seen your comments from PS4 but we run the periodic just after starting the service | |
| 16:05:56 | bauzas | so this works | |
| 16:06:33 | bauzas | given operators would modify the option, they would restart the nova-compute service so it will then delete the non-needed inventories then | |
| 16:07:11 | dansmith | yeah, but we may have boot requests pending for us when we start, and it means we're technically doing the cleanup on each run right? | |
| 16:17:57 | bauzas | dansmith: that's a tradeoff but I see your point | |
| 16:18:26 | bauzas | we could force a update_rp() right after init_host in the compute manager before we run RPC | |
| 16:18:41 | bauzas | but I think the problem is the same for all things | |
| 16:19:53 | bauzas | oh wait | |
| 16:20:17 | bauzas | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1778 | |
| 16:20:20 | bauzas | we're fine then | |
| 16:22:13 | bauzas | I just replied | |
| 16:22:23 | bauzas | anyway, I'll need to go errand | |
| 16:22:34 | bauzas | my patch is up, with a proposition | |
| 16:23:06 | bauzas | an alternative could be to cap the GPU inventory instead of refusing it as a whole, but meh, people can review | |
| 16:23:22 | bauzas | I'll add functests on Monday when I'm back | |
| 16:26:43 | dansmith | ack | |
| 23:36:33 | tonyb | bauzas: your revert has merged | |
| #openstack-nova - 2023-11-18 | |||
| 14:48:46 | opendevreview | Merged openstack/nova master: Fix missing oslo.versionedobjects library option https://review.opendev.org/c/openstack/nova/+/890718 | |
| 15:18:53 | opendevreview | Takashi Kajinami proposed openstack/os-vif master: Deprecate Windows support https://review.opendev.org/c/openstack/os-vif/+/901382 | |
| 17:37:29 | opendevreview | Takashi Kajinami proposed openstack/os-vif master: Deprecate Windows support https://review.opendev.org/c/openstack/os-vif/+/901382 | |
| #openstack-nova - 2023-11-19 | |||
| 13:49:22 | opendevreview | Merged openstack/nova master: Fix wrong description about minimum values https://review.opendev.org/c/openstack/nova/+/874061 | |
| #openstack-nova - 2023-11-20 | |||
| 09:36:53 | opendevreview | Takashi Kajinami proposed openstack/os-vif master: Clean up remaining python 2.7 support https://review.opendev.org/c/openstack/os-vif/+/896598 | |
| 09:36:58 | opendevreview | Takashi Kajinami proposed openstack/os-vif master: Clean up remaining python 2.7 support https://review.opendev.org/c/openstack/os-vif/+/896598 | |
| 10:40:19 | tkajinam | wondering if placement can/should be added to https://docs.openstack.org/project-team-guide/stable-branches.html#project-teams-which-asserted-they-follow-the-stable-branch-policy | |
| 12:26:05 | opendevreview | Rajesh Tailor proposed openstack/python-novaclient master: Fix typos https://review.opendev.org/c/openstack/python-novaclient/+/901457 | |
| 12:42:12 | opendevreview | Takashi Kajinami proposed openstack/nova master: Detect maximum number of SEV guests automatically https://review.opendev.org/c/openstack/nova/+/899381 | |
| 12:42:58 | opendevreview | Takashi Kajinami proposed openstack/nova master: Detect maximum number of SEV guests automatically https://review.opendev.org/c/openstack/nova/+/899381 | |
| 12:44:13 | opendevreview | Takashi Kajinami proposed openstack/nova master: Detect maximum number of SEV guests automatically https://review.opendev.org/c/openstack/nova/+/899381 | |
| 12:48:05 | opendevreview | Takashi Kajinami proposed openstack/nova master: Detect maximum number of SEV guests automatically https://review.opendev.org/c/openstack/nova/+/899381 | |
| 12:54:32 | sahid | o? | |
| 12:54:36 | sahid | o/ | |
| 12:54:50 | sahid | if some of you have a moment to look at this one: https://review.opendev.org/c/openstack/nova/+/896512 | |
| 12:55:11 | sahid | bauzas, dansmith ^ you already have context on it :-) | |
| 14:01:22 | opendevreview | Danylo Vodopianov proposed openstack/nova master: Packed virtqueue support was added. https://review.opendev.org/c/openstack/nova/+/876075 | |
| 14:27:38 | sean-k-mooney | sahid: before proceeding with that i would like you to justify why we cant just do this with https://docs.openstack.org/nova/latest/admin/aggregates.html#tenant-isolation-with-placement | |
| 14:28:38 | sean-k-mooney | we were ment to discuss this during the ptg but i dont recall doing it so | |
| 14:33:23 | opendevreview | Takashi Kajinami proposed openstack/nova master: Detect maximum number of SEV guests automatically https://review.opendev.org/c/openstack/nova/+/899381 | |
| 14:34:32 | opendevreview | Takashi Kajinami proposed openstack/nova master: Detect maximum number of SEV guests automatically https://review.opendev.org/c/openstack/nova/+/899381 | |
| 14:35:14 | opendevreview | Takashi Kajinami proposed openstack/nova master: Detect maximum number of SEV guests automatically https://review.opendev.org/c/openstack/nova/+/899381 | |
| 14:38:59 | sahid | sean-k-mooney: the behavior are differents, give me a moment I will point you my reponse on gerrit | |
| 14:40:21 | sahid | sean-k-mooney: https://review.opendev.org/c/openstack/nova/+/896512/comment/68ae77da_5f69a5f7/ | |
| 14:40:38 | sahid | It was a concerne from dansmith | |
| 14:41:05 | sahid | (line 39, marked as resolved) | |
| 14:44:50 | sean-k-mooney | the flilter is know to be buggy in some cases | |
| 14:45:06 | sean-k-mooney | i dont think we want to continue to support that buggy behavior and should remove the filter | |
| 14:45:42 | sean-k-mooney | with the placement version you can contole if a teant must use the aggreate or not vai a config option | |
| 14:45:57 | sahid | sean-k-mooney: I don't think it is buggy, it addresses a real use-case that the placement don't | |
| 14:46:43 | sahid | with placement the instance *is only* scheduled on that specified az | |
| 14:47:03 | sean-k-mooney | https://docs.openstack.org/nova/latest/configuration/config.html#scheduler.placement_aggregate_required_for_tenants | |
| 14:47:38 | sahid | it's not the behavior that we want. We want to allow certain tenant to schedule on that specified az, which means they can be scheduled on other | |
| 14:51:05 | sean-k-mooney | so you want all those that have not tenat assocated with them + the hosts that are allowed explcitly for this tenant | |
| 14:52:18 | sean-k-mooney | i woudl prefer to support that in the placment version honestly then extend the filter but your correct we cant express that in teh placment version today | |
| 14:59:14 | sahid | do you have an idea of the work needed to implement it in placement? | |
| 14:59:23 | sahid | I don't know much about placement | |
| 15:00:52 | sahid | As filter are standalone piece of code we can keep our change in filter as downstream only until the placement part get implemented | |
| 15:01:37 | sean-k-mooney | the problem with the filter approch is placmenet may not return hosts that could fit the vm depening on the alocation candate limit | |
| 15:01:57 | sean-k-mooney | i belvie the default limit is 1000 allocation candiates but we may have more then one allcoation candiate per host | |
| 15:02:40 | sean-k-mooney | so even if you have less then 1000 host some hosts that dont have any tennated assocated with them can be filtered out before we hit the schduler filters | |
| 15:03:11 | sean-k-mooney | this is made worse where you limit the reustls. cern limit placment ot less then 20 allcoation candiates | |
| 15:03:54 | sean-k-mooney | in such a case it very likely that all 20 hosts would be exclude by the filter even if you have many hosts with capsity | |
| 15:04:02 | sean-k-mooney | that woudl other wise be valid | |
| 15:05:20 | sahid | this limittation is valable for all scheduler filters, right? | |
| 15:06:53 | sean-k-mooney | yes which is why filters taht filter host based on critia that placement has not considerd in terms of resouce constraits can be problmatic if the placemetn limit is low | |
| 15:07:58 | sean-k-mooney | supporting the usecase you want to enabel is not particalarly striat forward in placment | |
| 15:09:15 | sean-k-mooney | its doable but somewhat complex. basically we would have to create a special aggreated that initally contaiend all comptues. then we would have to remove hots form that if they are later added to a tenent sepcific aggreated. | |
| 15:09:47 | sean-k-mooney | that is non tivial to ensure is correct as new hosts are added and or as aggreates are updated. | |