| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-09-01 | |||
| 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 | |
| 14:05:53 | bauzas | (b/c while I reviewed it, honestly, I didn't thought about Ironic) | |
| 14:06:59 | sean-k-mooney | dvo-plv: ya so option 2 woudl be my prefernce. but add a new dpdkarg or extra_config to hold the path | |
| 14:07:29 | sean-k-mooney | dvo-plv: we have kind of a chicken and egg problem | |
| 14:07:41 | dansmith | sean-k-mooney: really? we have no jobs without cinder? | |
| 14:07:48 | sean-k-mooney | dansmith: apprently | |
| 14:07:59 | sean-k-mooney | dansmith: its enabled by default in the devstack base job | |
| 14:08:24 | dansmith | sean-k-mooney: that doesn't make sense to me.. if we have no cinder, we should have no bdms with type==volume | |
| 14:09:07 | sean-k-mooney | dvo-plv: os-vif does not have a way to return data ot nova, nova need to knwo the socket path to put it in the libvirt xml so qemu cna create it, and neutron does not knwo what vendor nic is being used | |
| 14:09:13 | bauzas | dansmith: ok, indeed compute_id == compute_node.id (found it in the code) so are you +W then ? | |
| 14:09:28 | bauzas | dansmith: see https://bugs.launchpad.net/nova/+bug/2033752 | |
| 14:09:43 | dansmith | bauzas: it was already +W, I just -Wd to make sure we weren't missing something | |
| 14:10:15 | sean-k-mooney | dansmith: hum... that is a very good point | |
| 14:10:22 | sean-k-mooney | oh | |
| 14:10:33 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/893502/4/nova/compute/manager.py#4217 | |
| 14:10:42 | sean-k-mooney | dansmith: that line is alwasy called | |
| 14:10:47 | dansmith | ah | |