Earlier  
Posted Nick Remark
#openstack-nova - 2023-11-17
14:23:24 tkajinam bauzas, the default in tooz stable/2023.1 is v3alpha, the default in tooz master is v3. Because we keep using old etcd without v3 during upgrade process we have to override api version to v3alpha to keep using v3alpha after upgrade
14:23:58 jangutter if etcd and tooz mismatches, the endpoint has to be specified.
14:24:31 bauzas shouldn't tooz be somehow a bit smarter and do a failback call if etcd isn't new enough ?
14:24:56 tkajinam that's what was proposed above by pas-ha[m]
14:25:02 opendevreview John Garbutt proposed openstack/nova-specs master: Add spec for PCI Groups https://review.opendev.org/c/openstack/nova-specs/+/899719
14:25:57 bauzas what I'm afraid is that operators doing upgrades would face that problem too, right?
14:26:24 bauzas if we start tweaking devstack, there is surely an impact for regular ops that don't deploy with it :)
14:26:43 jangutter that's technically down to etcd3gw backend... and etcd doesn't have a nice version discovery, so, it's the try-different-versions method.
14:26:45 bauzas this means they have to change some config on the fly during upgrades
14:26:57 tkajinam well, we did document the upgrade impact to suggest setting api version in case old api version should be used.
14:27:08 bauzas jangutter: I'm personnally fine with a try/catch clause
14:27:19 tkajinam but what we are learning here is that just documentation may kill some people.
14:27:23 tkajinam including us
14:27:28 bauzas oh yeah
14:27:47 tkajinam https://review.opendev.org/c/openstack/tooz/+/891355/6/releasenotes/notes/etcd-3.4-eee8300c942a1263.yaml
14:28:00 bauzas if you really do breaking upgrades, my personal recommandation is to provide tools that people can consume for checking their states and whether they'll break
14:28:05 jangutter yep, the release notes are the last resort :-/
14:28:34 bauzas tkajinam: we, in nova, create the nova-status upgrade command for that exact reason
14:28:40 bauzas created*
14:29:04 bauzas because even if we spoil the breaking changes in our relnotes, people may be impacted and miss the point
14:29:37 bauzas anyway, I'm not saying tooz shall do a tooz-status upgrade commande
14:29:39 bauzas command*
14:29:44 tkajinam the problem may be that tooz is not the right place to provide that config validation, because tooz itself does not register config options but components using it does and pass the url value
14:29:51 jangutter The problem is that etcd is sometimes regarded as an _externally_ managed service, and sometimes an _internally_ managed service.
14:30:00 tkajinam we can provide the base implementation but every project may need to implement the command using that
14:30:08 bauzas I'm just saying that we need some kind of lockstep mechanism that can handle both etcd/tooz upgrades at once
14:31:25 jangutter yeah: for kolla-ansible that happens because in one release all the elements (config, tooz, etcd) is in lockstep. But that's not necessarily the same case for source-level projects.
14:32:43 bauzas problem is, again, you can't really ask operators to do some OS upgrade while you're upgrading your services
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?

Earlier   Later