| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-09-01 | |||
| 10:05:10 | auniyal | okay, its reverted but still shouldn't /proc/uptime get updated ? | |
| 10:07:39 | auniyal | still great test though it caught it or navigated us to the reason | |
| 10:09:24 | bauzas | no, I mean, we have a problem in Nova : we can't longer hard reboot if you don't deploy Cinder | |
| 10:09:47 | bauzas | we should handle this case | |
| 10:10:09 | auniyal | yes | |
| 10:10:34 | bauzas | not sure sean-k-mooney is around but I'd appreciate his thoughts on the best way to alleviate https://github.com/openstack/nova/blob/88c73e2931dcb276982612dbbdc3b849d4977c16/nova/compute/manager.py#L4177 | |
| 10:12:03 | auniyal | so if cinder.attachment_get_all failed, with could not create client or endpoint not found skip the cinder check | |
| 10:12:45 | auniyal | I'll create a patch for that, | |
| 10:12:56 | bauzas | yeah that sounds the obvious fix, but I wonder whether there is a better way to flag that this environment isn't having cinder" | |
| 10:13:49 | bauzas | I'm just looking at how we manage such case elsewhere in the code | |
| 10:15:25 | bauzas | yeah found | |
| 10:16:04 | bauzas | auniyal: https://github.com/openstack/nova/blob/88c73e2931dcb276982612dbbdc3b849d4977c16/nova/compute/manager.py#L3173-L3178 | |
| 10:16:28 | bauzas | this is gross but meh | |
| 10:16:45 | auniyal | ack bauzas | |
| 10:18:50 | sean-k-mooney | bauzas what the issue | |
| 10:18:57 | bauzas | auniyal: would you be able to propose a patch soon ? | |
| 10:19:05 | auniyal | bauzas yes | |
| 10:19:09 | bauzas | sean-k-mooney: tl;dr: some jobs that don't pull cinder fail | |
| 10:19:21 | sean-k-mooney | ah | |
| 10:19:25 | bauzas | sean-k-mooney: because now we call cinder in reboot | |
| 10:19:41 | bauzas | so we need to wrap the right exceptions | |
| 10:19:44 | bauzas | https://bugs.launchpad.net/nova/+bug/2033752 | |
| 10:19:58 | sean-k-mooney | well we just need a decorator that check if keystone is installed and skipt the function | |
| 10:20:17 | bauzas | sean-k-mooney: keystone is installed, cinder isn't | |
| 10:20:20 | sean-k-mooney | treat this like how we test for api extentions in neutron | |
| 10:20:27 | sean-k-mooney | bauzas: yes | |
| 10:20:39 | sean-k-mooney | oh i said keystoen | |
| 10:20:50 | sean-k-mooney | i ment check in keystone if cinder is installed | |
| 10:20:51 | bauzas | yeah but I got your point | |
| 10:21:04 | bauzas | yeah, that's why I wanted to discuss this with you and dansmith | |
| 10:21:11 | bauzas | we have 100 ways to make it working | |
| 10:21:44 | bauzas | ideally I would think that our internal cinder client package would wrap this, but this sounds a huge work | |
| 10:21:53 | sean-k-mooney | so when discusssing this in the past we noted that if you cant connect ot cinder for any reason we shoudl jsut boot with the bdm we have | |
| 10:22:27 | bauzas | yeah and with shutdown we just have a long list of wrapped exceptions in order to let shutdown succeed anyway | |
| 10:22:33 | bauzas | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L3164-L3188 | |
| 10:23:03 | bauzas | tbc, I agree with it, reboot *has to* be trustable anyway | |
| 10:23:22 | bauzas | so, auniyal is just stacking all the exception handling for now | |
| 10:23:33 | sean-k-mooney | so this is what we do for neutron extnetions | |
| 10:23:36 | sean-k-mooney | https://github.com/openstack/nova/blob/88c73e2931dcb276982612dbbdc3b849d4977c16/nova/network/neutron.py#L1386-L1406 | |
| 10:23:55 | sean-k-mooney | do we get an expction when we try to crate the client or when we try to make the first call? | |
| 10:23:57 | opendevreview | Merged openstack/nova stable/zed: libvirt: At start-up rework compareCPU() usage with a workaround https://review.opendev.org/c/openstack/nova/+/871968 | |
| 10:24:27 | bauzas | sean-k-mooney: when we make the call, that's the problem | |
| 10:24:42 | sean-k-mooney | no thats fine | |
| 10:25:01 | sean-k-mooney | so with the neutron decorators we are caching the results | |
| 10:25:14 | sean-k-mooney | so w eonly do the check once | |
| 10:25:50 | bauzas | sean-k-mooney: yeah sounds better, | |
| 10:26:00 | sean-k-mooney | so what i woul drobaly do is somethign like add a decorator that check the cinder max microversion | |
| 10:26:21 | bauzas | but for the sake of fixing this bug quickly, auniyal is just doing like shutdown and stacking all the exceptions | |
| 10:26:33 | sean-k-mooney | and we can handle the logic with that. i.e. say this cufntion need whatever | |
| 10:26:51 | sean-k-mooney | i would prefer to revert then hack in a fix if we dont do it properly | |
| 10:27:53 | bauzas | sean-k-mooney: well, let's see auniyal's patch | |
| 10:28:19 | bauzas | I don't disagree but I'd then also like to change https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L3164-L3188 | |
| 10:28:31 | bauzas | and possibly others | |
| 10:34:05 | sean-k-mooney | bauzas: we shoudl condider disabling cinder in one fo our jobs by the way | |
| 10:34:27 | bauzas | sean-k-mooney: good point | |
| 10:34:56 | sean-k-mooney | we do not need it in nova-ovs-hybrid-plug | |
| 10:35:15 | bauzas | sean-k-mooney: I was just looking at the jobs list :) | |
| 10:35:24 | bauzas | fancy me creating a patch ? | |
| 10:35:38 | sean-k-mooney | its ok i can do it | |
| 10:35:58 | sean-k-mooney | im looking at the code to see if i can also formulate a patch for the cinder issue | |
| 10:35:59 | bauzas | okay, do it then, you had the copyrights on the idea :) | |
| 10:36:17 | sean-k-mooney | do you have a link to a failing job? | |
| 10:36:37 | bauzas | yeah | |
| 10:36:49 | bauzas | https://1e11be38b60141dbb290-777f110ca49a5cd01022e1e8aeff1ed5.ssl.cf1.rackcdn.com/893401/5/check/neutron-ovn-tempest-ovs-release/f379752/controller/logs/local_conf.txt | |
| 10:38:21 | sean-k-mooney | ok so that is form the constuction of the client not the first call | |
| 10:38:46 | bauzas | sean-k-mooney: https://zuul.openstack.org/job/neutron-ovn-base removes cinder | |
| 10:39:14 | sean-k-mooney | its failing here https://github.com/openstack/nova/blob/master/nova/volume/cinder.py#L247 | |
| 10:39:50 | bauzas | yeah due to https://github.com/openstack/nova/blob/master/nova/volume/cinder.py#L863 | |
| 10:40:13 | bauzas | we're not caching the client | |
| 10:40:30 | bauzas | we just instantiate a client at every call | |
| 10:41:00 | sean-k-mooney | ya that is somehtign we can fix when we replace the client with the sdk | |
| 10:41:10 | bauzas | yup | |
| 10:41:45 | bauzas | anyway, my daughters are starving, I shall start to cook something or they'll burn the house | |
| 10:42:01 | bauzas | fortunately, the school is starting again next week | |
| 10:50:12 | opendevreview | Amit Uniyal proposed openstack/nova master: Addis a try-catch for cinder unavailability. https://review.opendev.org/c/openstack/nova/+/893489 | |
| 11:48:25 | opendevreview | sean mooney proposed openstack/nova master: only attempt to clean up dangeling bdm if cinder is installed https://review.opendev.org/c/openstack/nova/+/893502 | |
| 11:48:50 | sean-k-mooney | bauzas: auniyal ^ is how i woudl fix it | |
| 11:50:13 | opendevreview | sean mooney proposed openstack/nova master: only attempt to clean up dangeling bdm if cinder is installed https://review.opendev.org/c/openstack/nova/+/893502 | |
| 11:50:33 | auniyal | yeah, its looks cleaner than https://review.opendev.org/c/openstack/nova/+/893489 | |
| 11:51:27 | sean-k-mooney | jsut fixing some typos | |
| 11:51:31 | sean-k-mooney | is there a bug for this | |
| 11:51:49 | sean-k-mooney | ah https://bugs.launchpad.net/tempest/+bug/2033752 | |
| 11:52:01 | auniyal | yes | |
| 11:53:23 | opendevreview | sean mooney proposed openstack/nova master: only attempt to clean up dangeling bdm if cinder is installed https://review.opendev.org/c/openstack/nova/+/893502 | |
| 12:07:41 | bauzas | sorry folks, was with kids | |
| 12:08:29 | auniyal | sean-k-mooney, I added 2 comments in patch, its same what I was trying to do locally | |
| 12:08:54 | auniyal | *your patch - https://review.opendev.org/c/openstack/nova/+/893502/3 | |
| 12:15:13 | stephenfin | Uggla: https://review.opendev.org/c/openstack/openstacksdk/+/893505 | |
| 12:15:25 | stephenfin | I'm off for the rest of the afternoon, so hopefully that does the trick for you :) | |
| 12:16:10 | Uggla | oh thank you stephenfin, I'll have a look | |
| 12:16:28 | Uggla | stephenfin, have a great day | |
| 12:21:46 | stephenfin | Also published here https://gist.github.com/stephenfin/5834e4bd3213a2b9a9313ee3a7f56946 | |
| 12:21:50 | stephenfin | Uggla: Cheers | |
| 12:24:42 | Uggla | stephenfin, damned I'have again forgotten to register the options. :( It needs more testing but that looks good. | |
| 12:24:56 | Uggla | stephenfin, thank you so much | |
| 12:51:08 | opendevreview | sean mooney proposed openstack/nova master: only attempt to clean up dangling bdm if cinder is installed https://review.opendev.org/c/openstack/nova/+/893502 | |
| 12:53:02 | sean-k-mooney | bauzas: ^ i think i have adressed your nits stephenfin if your around can you take a look quickly too | |
| 12:53:52 | bauzas | sean-k-mooney: cool do | |
| 13:48:17 | dvo-plv | sean-k-mooney, Hello | |
| 13:48:46 | sean-k-mooney | hi | |