| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-11-06 | |||
| 14:52:36 | lyarwood | I'm not sure if anyone has been looking at this but I can't see an obvious issue with nova-ceph-multistore aside from instance spawns taking a long time overall | |
| 14:53:03 | lyarwood | I'll look at another example failure after a downstream call and file a bug to track these timeouts | |
| 14:54:13 | gmann | gibi: let me check. we did change one glance policy in nove ceph multistore job. | |
| 14:54:44 | gmann | gibi: ah this is same copy_image policy. checking why it is failing. | |
| 14:55:25 | gibi | gmann: thanks! | |
| 14:56:16 | bauzas | is it only me or you do you get a shit of exceptions when running functional and unittests locally with nova.storage.rbd_utils about the 'rados' and 'rbd' packages missing ? | |
| 14:56:24 | bauzas | or do* | |
| 14:56:27 | openstackgerrit | Ghanshyam Mann proposed openstack/nova stable/victoria: Modify glance's copy_image permission for nova-ceph-multistore https://review.opendev.org/761755 | |
| 14:56:30 | gibi | bauzas: I see it too | |
| 14:56:42 | gibi | and I remember seeing a patch fixing it | |
| 14:56:45 | bauzas | I tested with both py38 and py39 | |
| 14:56:54 | bauzas | but that's mostly annoying | |
| 14:56:55 | gmann | gibi: bauzas lyarwood backporting the nova-ceph-multistore fix https://review.opendev.org/#/c/761755/ | |
| 14:57:13 | gibi | bauzas: https://review.opendev.org/#/c/746873/ | |
| 14:57:15 | gmann | Tempest test merged before we backported this | |
| 14:58:08 | gibi | gmann: thanks! | |
| 14:58:13 | bauzas | gibi: meh, I should have it in my local repo | |
| 14:58:47 | gibi | bauzas: then maybe it is not the fix but it is the patch that generates the exception in the first place | |
| 14:58:58 | lyarwood | gmann: ah did that bork the job on stable? | |
| 14:59:21 | gmann | gibi: lyarwood nova-ceph-multistore job is from victoria so backporting until victoria is needed | |
| 14:59:29 | gmann | lyarwood: yeah - https://zuul.opendev.org/t/openstack/build/bf932888821345c1955cda68226b1a87/ | |
| 14:59:59 | gmann | tempest test which started running in nova-ceph-multistore on master as well on stable/victoria started running and needed this change on nova side | |
| 15:00:16 | bauzas | gibi: hah, that's maybe because the exception is not the same | |
| 15:00:52 | bauzas | ah no | |
| 15:00:54 | bauzas | nevermind | |
| 15:00:58 | bauzas | we log the exception | |
| 15:01:02 | lyarwood | gmann: ack I was going to look at that failure so thanks :) | |
| 15:01:11 | bauzas | hence the stracktrace | |
| 15:01:18 | bauzas | but that's horribly annoying | |
| 15:01:45 | bauzas | lyarwood: any ways for us to not getting a shitty number of exception lines anytime we use tox ? | |
| 15:01:58 | gmann | lyarwood: we added tempest test to copy-image for private images also (https://review.opendev.org/#/c/742546/23) which needed this policy to be open for everyone | |
| 15:02:03 | bauzas | because of rados and rbd missing | |
| 15:02:29 | bauzas | lyarwood: context being https://review.opendev.org/#/c/746873/4/nova/storage/rbd_utils.py | |
| 15:02:59 | lyarwood | gmann: ack | |
| 15:03:37 | lyarwood | bauzas: we could add some logic to skip the check based on an ENV var or something in test envs? | |
| 15:04:29 | lyarwood | bauzas: or we add them as test requirements? | |
| 15:04:36 | lyarwood | bauzas: even though they are virt driver specific | |
| 15:05:06 | bauzas | lyarwood: context is http://paste.openstack.org/show/799789/ | |
| 15:05:19 | lyarwood | bauzas: yeah I;ve seen it | |
| 15:05:32 | bauzas | lyarwood: IMHO we should lazy load the modules | |
| 15:05:42 | bauzas | for rdb and rados | |
| 15:06:03 | bauzas | or making them needed for all *nova* | |
| 15:06:11 | bauzas | but, definitely, the former | |
| 15:06:19 | bauzas | if they are optional | |
| 15:06:29 | bauzas | (and only for a specific driver) | |
| 15:06:39 | lyarwood | bauzas: we lazy loaded before silently and it was a PITA to debug issues when they failed to import | |
| 15:06:46 | lyarwood | bauzas: assuming that's what you mean | |
| 15:07:07 | lyarwood | bauzas: could we not just add them as test deps? | |
| 15:07:16 | bauzas | lyarwood: then, lazy load them by creating the libvirt object | |
| 15:07:26 | bauzas | when initting | |
| 15:07:43 | bauzas | gibi: ^ to be honestly, I'm against https://review.opendev.org/#/c/746873/4/nova/storage/rbd_utils.py | |
| 15:08:07 | bauzas | because we create more problems for folks that don't use ceph than we fix the needed ones | |
| 15:09:07 | openstackgerrit | Merged openstack/nova master: zuul: Merge nova-evacuate into nova-multinode-live-migration https://review.opendev.org/755309 | |
| 15:09:18 | dansmith | bauzas: me too, that seems crazy | |
| 15:10:36 | dansmith | surely we could check images_type and decide whether or not to explode there? | |
| 15:10:37 | gibi | bauzas, lyarwood, dansmith: can we move these import into one of the classes in the rbd_utils? | |
| 15:11:03 | gibi | dansmith: it is not exploding the driver, it is logging the import error | |
| 15:11:10 | dansmith | gibi: I think the point here was to make it explode before those get loaded, but that's not really okay, IMHO | |
| 15:11:13 | bauzas | gibi: the problem is that when using import directly, you *need* rdb | |
| 15:11:22 | bauzas | by any module | |
| 15:11:34 | bauzas | hence either lazy-loading it | |
| 15:11:55 | bauzas | or having other ways to support it like we did by telling none | |
| 15:12:06 | dansmith | yeah, go back to the old way and make something else check to see if the rbd import is None during early init, if we're in a dependent code path | |
| 15:12:16 | bauzas | by providing an exception you say "sorry, but you need it" | |
| 15:12:17 | dansmith | check config, or put it in a class we only load if we're using ceph | |
| 15:13:10 | bauzas | that too | |
| 15:13:19 | bauzas | there are multiple ways to make sure it's optional | |
| 15:16:29 | openstack | Launchpad bug 1903316 in OpenStack Compute (nova) "excessive logging of rbd and rados import errors" [Undecided,New] | |
| 15:16:29 | gibi | filed a bug https://bugs.launchpad.net/nova/+bug/1903316 | |
| 15:19:43 | gibi | feel free to push a fix against it ^^ | |
| 15:20:00 | gibi | if nobody then I might get to it at some point next week | |
| 15:22:04 | bauzas | gibi: I can try to fix it | |
| 15:22:37 | bauzas | but ideally, I'd love to understand the dynamics behind us logging this as an exception and what we wanted to fix previously | |
| 15:22:38 | gibi | ack, thanks | |
| 15:23:00 | gibi | I hope lyarwood can provide such context ^^ | |
| 15:23:04 | bauzas | because I don't see a problem with lazy-loading at init time | |
| 15:23:38 | bauzas | we could even make nova-compute refuse to start if config option says "you should have it" | |
| 15:24:20 | dansmith | right | |
| 15:24:25 | lyarwood | bauzas: the context there is that we previously silently failed without logging any context about why the import failed. | |
| 15:24:30 | dansmith | or .. log the warning if configured and missing | |
| 15:24:54 | lyarwood | bauzas: we had a few downstream reports where this failed even with the modules in the container image etc. | |
| 15:25:25 | lyarwood | hmm I was sure there was a reason we couldn't check the config before doing this import | |
| 15:25:36 | dansmith | lyarwood: well before it was above the CONF, | |
| 15:25:55 | dansmith | lyarwood: but also, where we actually need it, check to see if the import is None, if so, try to import it again and log the exception there just to get the evidence | |
| 15:26:12 | dansmith | i.e. if rbd_utils.rbd is None: import rbd | |
| 15:27:04 | lyarwood | dansmith: yup understood | |
| 15:32:21 | bauzas | dansmith: lyarwood: honestly, I'm on the side we should fix the problem by making rdb and rados mandatory for the use of the right option, and make nova-compute hardstop if not | |
| 15:32:35 | lyarwood | bauzas: it already does | |
| 15:32:52 | bauzas | not in this module then, right? | |
| 15:33:06 | lyarwood | no, we just raise RuntimeError | |
| 15:33:17 | bauzas | when starting the compute service ? | |
| 15:33:41 | lyarwood | from this module that iirc stops the service eventually yes | |
| 15:33:44 | lyarwood | if rbd is used | |
| 15:34:26 | bauzas | the above try/catch is made at import time, not runtime | |
| 15:34:57 | bauzas | so unless we fail ungracefully there, we do check it at run time | |
| 15:35:06 | lyarwood | https://github.com/openstack/nova/blob/c0fe95fcc5aec99a83dd57093dc230ef67b36b39/nova/storage/rbd_utils.py#L139-L140 | |
| 15:35:32 | bauzas | ack, run time | |
| 15:35:43 | bauzas | and when we create the instancez | |
| 15:35:56 | bauzas | which, I suspect, is made at init | |
| 15:36:34 | bauzas | lyarwood: okay, so indeed, we're all good, but why couldn't we just make the imports being made at run time just before this check ? | |