| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-11-17 | |||
| 14:32:56 | bauzas | that's why grenade is not bumping the reqs | |
| 14:33:36 | jangutter | yeah, _however_ etcd in devstack isn't the os-bundled one. | |
| 14:35:04 | bauzas | just checked, we pull tooz-3.2.0 | |
| 14:35:09 | bauzas | https://storage.gra.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_73a/900783/2/check/grenade-skip-level-always/73acbef/controller/logs/old/devstacklog.txt | |
| 14:36:13 | bauzas | and we have etcd3gw==2.1.0 | |
| 14:36:59 | bauzas | oh tonyb +Wd my revert patch https://review.opendev.org/c/openstack/requirements/+/901079 | |
| 14:38:00 | tonyb | it's clearly broken | |
| 14:38:24 | bauzas | oh heh, https://github.com/openstack/grenade#theory-of-upgrade | |
| 14:38:31 | bauzas | tkajinam: jangutter: ^ | |
| 14:38:45 | jangutter | yeah I saw that. | |
| 14:38:54 | tonyb | we should block it in g-r and figure out how to handle things for the longer term | |
| 14:39:30 | bauzas | "Any other required changes on upgrade are an exception and must be called out in the release notes." and "Grenade supports per release specific upgrade scripts (from-juno, from-kilo)." | |
| 14:39:47 | bauzas | tonyb: I don't disagree | |
| 14:40:33 | bauzas | I still think I'd appreciate if tooz may do some fallback mechanism to an older endpoint if the newer endpoint sounds invalid | |
| 14:40:39 | tkajinam | https://review.opendev.org/q/topic:etcd-api-discovery | |
| 14:40:42 | bauzas | just for the sake of easing people's lifes | |
| 14:41:15 | tkajinam | We probably need a bug and link these to the bug, but we can make tooz smart and discover the available api version unless user explicitly requires a specific version | |
| 14:41:22 | bauzas | tkajinam: wow, kudos | |
| 14:41:26 | tkajinam | which is being implemented in these | |
| 14:41:53 | tkajinam | (tooz patch need further work because we first need a new release of etcd3gw... | |
| 14:41:57 | bauzas | sounds a good path to me | |
| 14:42:32 | bauzas | we may ban the current tooz release and just await for a newer release once everything is fine | |
| 14:42:38 | jangutter | tkajinam: one question - if etcd gets updated (v3alpha disappears and v3 appears) - would this case be handled? | |
| 14:44:33 | tkajinam | jangutter, yes. with that code etcd checks the etcd server version using version api and use the latest api version available | |
| 14:44:47 | tkajinam | jangutter, though you may need to restart servers because the detected api is cached after the first detection. | |
| 14:44:57 | tkajinam | s/servers/services/ I mean | |
| 14:45:10 | tkajinam | s/etcd checks/tooz checks/ | |
| 14:45:44 | jangutter | takjinam: so it will appear if cinder "remembers" the version of etcd it first connected to and never try a newer version? | |
| 14:46:37 | tkajinam | there is tradeoff. we can let tooz check api version every time but it always requires additional api call | |
| 14:46:47 | tkajinam | we may probably implement expire but I'd leave it for now | |
| 14:46:58 | tkajinam | jangutter, I probably had to say this first. yes, you are correct | |
| 14:49:19 | jangutter | tkajinam: yeah, gracefully redetecting the endpoint on error is also another option, that adds time to report errors. | |
| 14:50:40 | jangutter | but this covers 95% of the pain. | |
| 14:58:08 | tkajinam | https://bugs.launchpad.net/python-etcd3gw/+bug/2043810 | |
| 14:59:01 | tkajinam | I've created a bug here... in case you want something you can refer to | |
| 14:59:04 | tkajinam | jangutter, we can discuss how we can realize the automated switch without restart, but I'd prefer leaving it for follow-up. | |
| 15:00:05 | jangutter | tkajinam: right, I'm fine with just documenting the behavior (i.e. we expect dependent services to be restarted on etcd upgrade) | |
| 15:04:38 | jangutter | (that's a much more reasonable expectation than requiring operators to update etcd + tooz in lockstep. | |
| 15:15:17 | tonyb | bauzas: You probably need to remove your -W on 901079 | |
| 15:15:36 | bauzas | ack I will | |
| 15:49:43 | bauzas | dansmith: before I shutdown for the weekend and before your PTOs, just a quick question about https://review.opendev.org/c/openstack/nova/+/899625 | |
| 15:50:39 | bauzas | for non-SRIOV GPUs, shall we cap the total of the inventory by the max_instances or should we rather provide total=0 if the GPU has more than the max ? | |
| 15:51:03 | bauzas | for example, a GPU type supporting 8 vGPUs | |
| 15:51:10 | bauzas | and the operator saying max_instances=2 | |
| 15:51:34 | bauzas | shall we then accept one GPU and providing a total=2 or should we rather not accept it? | |
| 15:51:41 | dansmith | we can't *actually* have total=0 right? | |
| 15:51:53 | bauzas | we can now | |
| 15:52:06 | bauzas | with https://review.opendev.org/c/openstack/nova/+/899625/4/nova/virt/libvirt/driver.py | |
| 15:52:16 | bauzas | I'm just writing the unittests | |
| 15:52:20 | dansmith | right but that's just internally, placement won't allow it right? | |
| 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 | |