| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-09-01 | |||
| 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 | |
| 14:00:01 | dansmith | johnthetubaguy: comment here if you could look quickly: https://review.opendev.org/c/openstack/nova/+/886989 | |
| 14:00:30 | dansmith | also bauzas, since you were +2 | |
| 14:00:48 | sean-k-mooney | dansmith: can you take a look at this https://review.opendev.org/c/openstack/nova/+/893502 | |
| 14:01:19 | sean-k-mooney | merging the dangeling bdms serise broke reboot for deploymetns without cinder | |
| 14:01:20 | bauzas | dansmith: shouldn't need to update node, right? | |
| 14:01:27 | bauzas | dansmith: it's not changing | |
| 14:01:47 | bauzas | but, compute_id, mmmm, yeah you're right :( | |
| 14:02:04 | dvo-plv | I would like to talk regarding our proposal | |
| 14:02:22 | dansmith | bauzas: if compute_id is changing then node is changing, however, we're changing host to point to a different node, right? | |
| 14:02:40 | dvo-plv | Currently I see two option. 1 we move back to the separate mech driver. 2 we generate socket in the OpenStack and pass it to the dpdk | |
| 14:03:02 | bauzas | dansmith: well, the compute node uuid is the ironic node uuid | |
| 14:03:13 | bauzas | so, it shouldn't be changing, right? | |
| 14:03:17 | dvo-plv | I belive that 2 option is more suitable for you | |
| 14:03:55 | dansmith | okay I see, we're moving the node between services... | |
| 14:04:04 | dvo-plv | So, how do you see it in terms of the OpenStack, out dpdk team currently analyze this approach with our pmd driver | |
| 14:04:33 | bauzas | dansmith: correct, that being said, now I wonder about compute_id | |
| 14:04:58 | dansmith | bauzas: compute_id should match the node, so only change one if the other | |
| 14:05:15 | bauzas | ah, I was about looking on your series | |
| 14:05:35 | dvo-plv | I belive that using common option vhost-server-path is not acceptable in case dpdk port type, so we should create the new one, and it also will be appropriate parameter | |