Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-08-30
14:37:27 bauzas (which I do understand is somehow risky)
14:37:52 sean-k-mooney its not that its risky just less common in our code base
14:37:57 sean-k-mooney the end result
14:38:02 sean-k-mooney we want in ether case
14:38:05 sean-k-mooney is a single rpc
14:38:13 bauzas I just don't want us to push this feature under the FeatureFreeze bus because of a code pattern argument :)
14:38:16 sean-k-mooney so aslong as we get ther its fine
14:38:38 bauzas which is the case in the current latest PS
14:38:45 bauzas so, +2
14:38:54 sean-k-mooney ack
14:56:10 sean-k-mooney elodilles: by the way the os-vif ci has been pretty green https://zuul.openstack.org/builds?project=openstack%2Fos-vif&skip=0 bu twe also dont see many executions there either
14:57:23 sean-k-mooney sounds like you are concerredn about hte os-vif-ovs-iptables job sepcifically which has had a few failures https://zuul.openstack.org/builds?job_name=os-vif-ovs-iptables&project=openstack%2Fos-vif
15:07:33 elodilles sean-k-mooney: yepp, os-vif-ovs-iptables failed 2 times on the generated 'ci health checking' patch, but i saw that it passed recently
15:08:19 elodilles i'm not really concerned, but wanted to see that the DNM patch is passing so that i can abandon it o:)
16:02:50 sean-k-mooney elodilles: i think this failrues were mainly related to volumes
16:03:02 sean-k-mooney so ya we will see what happens iwht the dnm
16:03:13 sean-k-mooney honestly we likely sould disable the volume tests in os-vif
16:04:24 sean-k-mooney its also possible that those test are not configured for verification
16:05:27 sean-k-mooney its not disabled expleictly however so i dont thin kthis is the issue
16:54:08 bauzas folks, I'm about to EOB, but I'd appreciate if some cores around may look in particular at https://review.opendev.org/c/openstack/nova/+/873221 , https://review.opendev.org/q/topic:dangling-volumes+project:openstack/nova+is:open and https://review.opendev.org/q/topic:ironic-shards+project:openstack/nova+is:open which both have my +2 (mostly, or just asking for nits)
16:54:38 bauzas as a reminder, FF is tomorrow and I'd very appreciate if we could somehow give a go to those series
16:59:12 opendevreview Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284
17:01:06 auniyal bauzas, dansmith, sean-k-mooney can you please have another look dangling patches
17:07:46 dansmith bauzas: there are so many typos in that bottom ironic sharing patch including doc text and renos, I think it's worth a respin.. what do you think?
18:01:32 sean-k-mooney auniyal: im -1 on the reproducer https://review.opendev.org/c/openstack/nova/+/889947/6/nova/tests/functional/compute/test_attached_volumes.py
18:08:31 sean-k-mooney bauzas: dansmith im confilcited on what to do with the dangeleing volume seriese
18:09:07 dansmith I gotta run in a sec, but I don't think it's anything approaching critical
18:09:28 dansmith and it doesn't yet address concerns about the cinder volume being detached and reattached outside nova, AFAIK
18:09:46 sean-k-mooney right so i didnt think we wanted to suport that
18:09:55 sean-k-mooney this would actully enabel detach to work
18:10:04 sean-k-mooney albeit with a hard reboot
18:10:13 sean-k-mooney adn needing a service user token
18:10:57 dansmith I mean it doesn't catch that case, but should
18:11:54 sean-k-mooney so the bit im conflicted by is Verify if volume exists and cinder has an attachment ID else delete its BDMS data from nova DB and vice versa.
18:12:18 sean-k-mooney the "and cinder has an attachment ID" part
18:13:10 dansmith what about it?
18:13:35 sean-k-mooney i was not expecting use to delete the BDM if the volume existiend but the atachment was gone
18:13:57 dansmith why not? that means it's been detached underneath us
18:14:10 dansmith (/me will go poof in a few minutes without warning)
18:14:10 sean-k-mooney whihc we dont allow so we shoudl repair it?
18:14:20 sean-k-mooney or error if you attached it to somethign else
18:14:36 dansmith we should heal ourselves and not require admin intervention
18:14:39 dansmith which is what is needed right now
18:15:20 sean-k-mooney in the case the volume is deleted i agree
18:15:35 sean-k-mooney im unconfrotabel with just the case wehre the attachment is deleted
18:15:44 sean-k-mooney perhaps that is valid
18:15:45 dansmith why? from our perspective it's the same
18:16:13 dansmith the state of the volume has been mucked with undereath us, so bail
18:16:31 dansmith don't try to hook it back up or recover in any other way than just avoiding our own problems in the future
18:16:40 sean-k-mooney so i woudl error in the attachment delete case personslly and say no you srewed with thsi you need ot manuall fix it
18:16:52 dansmith that's exactly what we're trying to avoid
18:17:30 dansmith because the user can't manually fix it
18:17:58 dansmith just like when we go to delete something and it's already deleted, we say "okay fair enough" and move on
18:18:16 dansmith these are distributed systems.. nova's DB could have been restored from ten minutes ago, etc
18:18:20 sean-k-mooney well the admin can with the attachemtn refesh command. i was debating if we shoudl do the refresh in the case the voluem is aviabale but i get you dont want that complexity
18:19:05 sean-k-mooney i guess the fact the attachment api needs service user
18:19:09 sean-k-mooney tokens now
18:19:26 sean-k-mooney fixes most of the concerns i have
18:19:50 sean-k-mooney as long as we are still very clear that if you detach a volume form cidner or a prot form neutorn your isntance is tainted
18:20:13 sean-k-mooney and all we are doing is minimal efffort to boot again
18:20:16 sean-k-mooney i guess that is ok
18:25:57 sean-k-mooney ill look at this again tomorooww but without the service user chagne i dont this woudl have been the correct approch to take.
18:35:11 dansmith sean-k-mooney: right the point in this is very specifically to *not* require the admin to intervene when the only thing they can reasonably do is run a command to do the same thing, which is eliminate the BDM from nova's side
18:35:22 dansmith we wouldn't want the admin to "fix" it (IMHO)
18:36:04 dansmith and especially with the cinder policy change, the only real thing that could cause this workflow-wise would be a database restore from checkpoint such that the two sides are out of sync, or an admin operation on the cinder side that just then needs to be mirrored with admin operations on the nova side
18:36:30 dansmith it's not to _enable_ any sort of bad behavior by the user, it's to avoid the user being stuck and unable to get their instance back
18:36:55 dansmith for self-service reasons, I'd much prefer my volume to just become detached and me still be able to work on my instance without waiting in an admin's queue
18:46:12 opendevreview Merged openstack/nova master: Add a new NumInstancesWeigher https://review.opendev.org/c/openstack/nova/+/886232
19:23:52 opendevreview melanie witt proposed openstack/nova master: Add documentation for unified limits https://review.opendev.org/c/openstack/nova/+/893127
19:23:52 opendevreview melanie witt proposed openstack/nova master: nova-manage: Add 'limits migrate_to_unified_limits' https://review.opendev.org/c/openstack/nova/+/891646
19:34:05 opendevreview melanie witt proposed openstack/nova master: Add documentation for unified limits https://review.opendev.org/c/openstack/nova/+/893127
19:36:11 sean-k-mooney dansmith: ok i can bye that. i kind of feel like in addtion to this we show have a external event form cidner saying its detach or something like that to make it properly automatic but again with the service token change that shoudl not be reuqired either.
19:36:46 dansmith to be clear, I do not think we should _ever_ tolerate detach via cinder, thus no need for an event
19:36:58 dansmith this is a corruption recovery mechanism and nothing else
19:36:59 sean-k-mooney so ill remove my -1 on the repoducer. i still have not looked at the actual final patch however
19:37:14 sean-k-mooney ack
#openstack-nova - 2023-08-31
02:00:15 opendevreview melanie witt proposed openstack/nova master: Enable use of native threads in scatter_gather_cells https://review.opendev.org/c/openstack/nova/+/650172
02:47:02 opendevreview Merged openstack/nova master: libvirt: Add 'COMPUTE_ADDRESS_SPACE_*' traits support https://review.opendev.org/c/openstack/nova/+/873221
04:58:50 opendevreview OpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata https://review.opendev.org/c/openstack/nova/+/891648
06:11:37 opendevreview Amit Uniyal proposed openstack/nova master: Reproducer for dangling bdms https://review.opendev.org/c/openstack/nova/+/889947
06:11:38 opendevreview Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284
06:50:23 auniyal thanks sean-k-mooney for the reviews
07:21:34 opendevreview Merged openstack/nova master: Remove deprecated AZ filter. https://review.opendev.org/c/openstack/nova/+/886779
08:08:05 opendevreview Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284
08:55:57 opendevreview John Garbutt proposed openstack/nova master: Deprecate ironic.peer_list https://review.opendev.org/c/openstack/nova/+/883346
08:56:11 opendevreview John Garbutt proposed openstack/nova master: Deprecate ironic.peer_list https://review.opendev.org/c/openstack/nova/+/883346
11:21:59 auniyal Hi sean-k-mooney, tat for +W this https://review.opendev.org/c/openstack/nova/+/889947/7,
11:22:19 auniyal but the dependent patch is not +W yet, its has all +2's https://review.opendev.org/c/openstack/nova/+/881457/35
11:34:45 opendevreview sean mooney proposed openstack/nova master: increase job timeout for ocational slow nodes https://review.opendev.org/c/openstack/nova/+/893360
11:41:14 opendevreview sean mooney proposed openstack/nova master: increase job timeout for ocational slow nodes https://review.opendev.org/c/openstack/nova/+/893360
11:41:43 sean-k-mooney auniyal: thats intentionaly we will +w the previsou patch after we are happy with the final one
11:41:57 sean-k-mooney and merge the seriese together
11:42:05 auniyal ack,
11:43:46 auniyal but it won't fail merging the reproducer one (something like merge conflict) - because reproducer is over that one
11:46:19 sean-k-mooney no it should not
12:42:00 bauzas auniyal: have you seen the -1 from sean-k-mooney ?
12:42:05 bauzas on the top change ?
12:42:13 dvo-plv sean-k-mooney, Hello. Are you here ?
12:48:05 Uggla hello, I have some troubles with the service token, my assumption is cinder.py, neutron.py, ... were modified to use service token by calling service_auth.get_auth_plugin method. But I think this was not done for the services using the sdk. Can you confirm this assumption ?
12:52:30 sean-k-mooney bauzas: if your around can you take a look at https://review.opendev.org/c/openstack/nova/+/893360 i have seen 1 or two timeouts so this might be good to land

Earlier   Later