Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-11-04
14:56:46 gibi lyarwood: ack, thanks
15:02:23 artom_ bauzas, hey, I think the Ironic folks would be really happy if we made https://review.opendev.org/c/openstack/nova/+/813263 a review priority...
15:03:20 sean-k-mooney i see
15:04:48 sean-k-mooney i think should be safe although it raise the question about oter life cyle events liek power on power off and had/soft reboot
15:05:02 artom sean-k-mooney, the Ironic patch?
15:05:25 sean-k-mooney yes
15:05:29 artom Yeah, I suppose it does, but from what I've seen, use of plug_vifs() is highly limited, so it's safe to make it a noop
15:05:44 sean-k-mooney no its not
15:05:59 sean-k-mooney we need to call it for the inial spawn
15:06:02 artom sean-k-mooney, I mean, look at my review notes inline, and tell me if I've missed something :)
15:06:20 sean-k-mooney we do not need to call it in init_host for ironci
15:06:34 sean-k-mooney but it cant jsut be a noop without change the spwan workflow
15:07:06 sean-k-mooney we use it on inital boot to ensure that the networkign if fully configured by the backend before we power on the ironic host
15:07:27 artom sean-k-mooney, maybe you're thinking of a slightly differently named method?
15:07:35 sean-k-mooney no im not
15:08:00 artom In the compute manager, it's only called from _init_instance(), which is only called from init_host()
15:08:05 artom That's it, nothing on spawn
15:08:14 sean-k-mooney correct its not
15:08:25 sean-k-mooney but we also call plug_vifs during spwan
15:08:31 sean-k-mooney so you cant just make plug_vifs a noop
15:09:03 artom From where?
15:09:06 sean-k-mooney it will mean during spawn we will not actully set up the networking proerly they have hacked around this here https://review.opendev.org/c/openstack/nova/+/813263/3/nova/virt/ironic/driver.py#1606
15:09:28 sean-k-mooney by starting to use _plug_vifs to actully invoke the ironic api
15:09:34 sean-k-mooney for interface attach
15:10:23 artom That's just inlining what plug_vifs() used to do into attach(), no?
15:10:35 bauzas artom: I can mark it as a Review-Priority for me
15:11:40 artom bauzas, PTL's discretion and all that :) I was just making a request
15:12:00 bauzas any core can set this flag
15:12:08 bauzas ... for the moment
15:12:18 artom But you're the core-iest of cores
15:12:19 bauzas I'm about to write a doc change for it, hopefully tomorow
15:12:47 bauzas artom: nah, as sean-k-mooney said, I'm just a "cat herder" or if you prefer, some French guy yelling in the wind
15:13:05 gibi sean-k-mooney, artom: I read that ironic related nova patch, I see that it is correct and does not affect spawn, but now I'm affraid that sean-k-mooney has things I'm missing
15:13:07 artom Meow.
15:13:19 artom gibi, you and me both
15:13:24 gibi we need more cats
15:13:33 bauzas I have a dog
15:13:39 gibi then you are out
15:13:41 gibi :P
15:13:49 bauzas cats are selfish
15:13:50 artom Ionesco says dogs are cats
15:13:56 sean-k-mooney gibi: im mostly uncofrotable with changing the meaing of plug_vifs to be honest
15:13:59 artom He also says that Socrates was a cat
15:14:12 sean-k-mooney im currently reviewing the spawn path
15:14:22 artom sean-k-mooney, I don't think that's our call to make, every driver can do what they want
15:14:26 bauzas artom: the only merit to cats is that they help to prove some theorem
15:14:40 bauzas about quantic nature
15:15:30 artom Only if they're in boxes
15:16:58 sean-k-mooney artom: its used here https://github.com/openstack/nova/blob/fded762f4df26ff5706438a66da33ff966f833c6/nova/virt/ironic/driver.py#L1910
15:17:04 sean-k-mooney which is used by the compute manger here https://github.com/openstack/nova/blob/fded762f4df26ff5706438a66da33ff966f833c6/nova/compute/manager.py#L2597
15:17:17 sean-k-mooney in _build_resources
15:17:26 gibi sean-k-mooney: our virt driver interface has the plug_vif method but we only use that from the computa manager in init_host
15:17:37 gibi sean-k-mooney: that call is transformed out in the proposed patch
15:17:37 sean-k-mooney which is part of _build_and_run_instance
15:17:49 artom sean-k-mooney, yeah, and that's been inlined here: https://review.opendev.org/c/openstack/nova/+/813263/3/nova/virt/ironic/driver.py#1919
15:18:20 gibi sean-k-mooney: and that call paths is totaly ironic specific, libvirt virt driver does not call back to plug_vifs during spawn
15:18:22 artom AFAICT, TheJulia did her homework :)
15:18:26 sean-k-mooney oh that prepare_networks_before_block_device_mapping
15:19:06 sean-k-mooney gibi: well we do use it internally in the virt dirver i think in livemigration and maybe hard reboot
15:19:14 sean-k-mooney but perhaps not directly in spwan
15:19:17 gibi sean-k-mooney: not the virt driver interface
15:19:34 sean-k-mooney gibi not the virt dirver interface not but the funciton in the dirver
15:19:40 artom TheJulia, we're trying to convince sean-k-mooney you're plug_vifs() patch is correct :)
15:19:50 artom *your
15:19:52 TheJulia oh, yes, it is correct as far as I've been able to navigate
15:19:55 artom Oh god, I'm one of them now
15:19:58 TheJulia since ironic handles all attachment management
15:20:16 artom The people who put apostrophe's in plural's and no apostrophe's in possessive's
15:20:37 sean-k-mooney TheJulia: it was the swan path i was concerned about i had not expanded the context lins areound https://review.opendev.org/c/openstack/nova/+/813263/3/nova/virt/ironic/driver.py#1919
15:20:59 sean-k-mooney TheJulia: the change you made in prepare_networks_before_block_device_mapping will ensure we actully plug the vifs still on spwan
15:21:15 TheJulia and due to that, and the duality use of attach_interfaces being present, it is entirely redundant and harmful behavior for the operators to experience with nova-computes running the ironic driver
15:21:28 gibi sean-k-mooney: I see
15:21:54 artom TheJulia, what does plugging the vifs mean in an Ironic context, anyways? Calling out to a physical switch and setting vlans and stuff?
15:22:06 sean-k-mooney TheJulia: what i was really concerned about is regressing spawn such that the compute manger would not wait for the vifs to be configured on the network swtich berofre powering on the server
15:22:18 sean-k-mooney but it looks like that cant happen so ill +1 it shortly
15:22:29 TheJulia artom: recording there should be an attachment, that is only sent to neutron until once the states change to active
15:22:59 TheJulia sean-k-mooney: those are persistant and managed through the workflow and neutron ml2 plugins
15:23:24 TheJulia sean-k-mooney: but totally valid concern not knowing the rest of the mechanics
15:24:27 sean-k-mooney TheJulia: well i mean this is not something that ironic should really mange entrily on its own so jsut making sure we still have the correct mechamins in place on the nova side
15:25:42 sean-k-mooney altough looking at that code path we dont seam to be correctly waiting for the neutron external event in _plug_vifs
15:25:44 sean-k-mooney https://github.com/openstack/nova/blob/fded762f4df26ff5706438a66da33ff966f833c6/nova/virt/ironic/driver.py#L1492-L1525
15:26:10 sean-k-mooney we are just callign the node.vif_attach api
15:26:14 TheJulia sean-k-mooney: it must because of a security lifecycle must be enforced
15:26:19 sean-k-mooney so this looks like there is an existing race
15:26:23 TheJulia and it knows the state of the lifecycle
15:27:25 EugenMayer Anybody in here got novnc working with kolla when using TLS? TLS is working on all sub-systems except when using TLS. lyarwood it seems like they use a haproxy (i got told) which does the SSL offloading, which might be the reason it is not configured. But this would not explain the error message
15:27:46 opendevreview Balazs Gibizer proposed openstack/nova master: Add a WA flag waiting for vif-plugged event during reboot https://review.opendev.org/c/openstack/nova/+/813419
15:29:47 TheJulia sean-k-mooney: on start, but if the vif recrods are already on file, on start it doesn't matter, the nova-compute spins for quite a long time.
15:30:02 TheJulia for running state, that is a separate path and that has been the case for a while, I think.
15:50:42 TheJulia so I think I grok sean-k-mooney's concerns, and just to be on the safe side, I'll go change my "test nova-y tings patch in the ironic repo to pull that vif plug patch in and just make sure that it passes happily again. I believe it did so before, but there is the BFV use case which is a little different
15:54:15 opendevreview Alexey Stupnikov proposed openstack/nova master: WIP: Test aborting queued live migration https://review.opendev.org/c/openstack/nova/+/776250
15:55:03 TheJulia artom: that hash ring handling wip seems to work, which is a good sign. Likely just need to go see if the devstack plugin forces a rebalance... and maybe make it force a rebalance :)
15:55:51 artom TheJulia, yeah, I need to look at it again and properly wrap my head around it
15:57:10 TheJulia artom: yeah, a little different by just reconciling "what is running/active" versuse explicit record checks for each instance, but I'd hate to trigger a few thousand extra DB queries upon rebalance
15:59:04 TheJulia changed https://review.opendev.org/c/openstack/ironic/+/813264 to run the vif plug change
15:59:26 TheJulia sean-k-mooney: ^^ if the ironic bfv job passes, I suspect we're safe and happy for the time being
16:03:16 sean-k-mooney ack tahnks
16:27:01 opendevreview Balazs Gibizer proposed openstack/nova master: Refactor Database fixture https://review.opendev.org/c/openstack/nova/+/815690
16:27:01 opendevreview Balazs Gibizer proposed openstack/nova master: Remove SESSION_CONFIGURED global from DB fixture https://review.opendev.org/c/openstack/nova/+/815689
16:29:50 opendevreview Balazs Gibizer proposed openstack/nova master: Fix interference in db unit test https://review.opendev.org/c/openstack/nova/+/814735

Earlier   Later