| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-11-04 | |||
| 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 | |
| 16:32:36 | melwitt | gibi: just read your comments, I might be wrong but if I am then I don't understand why it's needed. I'll look at it some more | |
| 16:33:19 | gibi | melwitt: I followed your suggestion and things are still passing, so I think you are right | |
| 16:33:49 | melwitt | oh ok | |
| 16:34:07 | gibi | so you were right that I don't need to patch the per connection case | |
| 16:34:34 | melwitt | ack | |
| 18:00:55 | EugenMayer | lyarwood FYI - kolla supports tls for novnc but uses a haproxy as SSL offloaded and LB in front of it. My issues was an internal vs external VIP IP. So kolla has support for it | |
| 18:01:19 | lyarwood | cool cool | |
| 18:34:34 | opendevreview | Lee Yarwood proposed openstack/nova master: nova-next: Deploy noVNC from source instead of packages https://review.opendev.org/c/openstack/nova/+/816738 | |
| 18:41:56 | opendevreview | Lee Yarwood proposed openstack/nova master: nova-next: Drop NOVA_USE_SERVICE_TOKEN from subnode https://review.opendev.org/c/openstack/nova/+/816740 | |
| 20:31:31 | opendevreview | Gustavo Santos proposed openstack/nova master: Reattach mdevs to guest on resume https://review.opendev.org/c/openstack/nova/+/815373 | |
| 22:07:36 | opendevreview | sean mooney proposed openstack/nova master: [WIP] Add extra tests for pinning with partial siblings https://review.opendev.org/c/openstack/nova/+/816758 | |
| 23:30:48 | opendevreview | Merged openstack/nova stable/wallaby: Store old_flavor already on source host during resize https://review.opendev.org/c/openstack/nova/+/810913 | |
| #openstack-nova - 2021-11-05 | |||
| 01:46:46 | opendevreview | Brin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from List/Update Servers APIs https://review.opendev.org/c/openstack/nova/+/764292 | |
| 02:01:38 | opendevreview | Brin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from List/Update Servers APIs https://review.opendev.org/c/openstack/nova/+/764292 | |
| 02:12:37 | opendevreview | Brin Zhang proposed openstack/nova master: [Trival] Fix wrong microversion in TestClass name https://review.opendev.org/c/openstack/nova/+/816778 | |
| 02:47:30 | opendevreview | Brin Zhang proposed openstack/nova master: Replace all_tenants with all_projects in List Server APIs https://review.opendev.org/c/openstack/nova/+/765311 | |
| 02:58:25 | opendevreview | Brin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from Rebuild Server API https://review.opendev.org/c/openstack/nova/+/766380 | |