| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-09-16 | |||
| 15:15:41 | gibi | we are under 10 untriaged bugs. I really appreaciate all of your bug triage work! thank you! | |
| 15:43:12 | stephenfin | gibi, bauzas, gmann: https://review.opendev.org/752210 is passing now (the base for the os-server-groups 500 -> 400 change) | |
| 15:45:26 | openstack | Launchpad bug 1775934 in OpenStack Compute (nova) "Cannot run "openstack server list" with instance stuck in scheduling state" [Undecided,New] | |
| 15:45:26 | bauzas | melwitt: based on your last (and old) comment on https://bugs.launchpad.net/nova/+bug/1775934 could we mark the bug as Confirmed ? this is unclear to me whether we need to fix it | |
| 15:45:27 | gmann | stephenfin +2 | |
| 15:45:32 | gibi | +@ | |
| 15:45:41 | gibi | +2+A | |
| 15:45:42 | gibi | ;d | |
| 15:52:03 | melwitt | bauzas: that sounds familiar and I think "probably". I need to look around a bit to find a related/duplicate bug I've seen and add more information | |
| 15:52:31 | bauzas | melwitt: kk, marking it as confirmed for now, but feel free to mark it duplicate if so | |
| 15:52:37 | bauzas | thanks | |
| 15:52:48 | melwitt | sounds good, thanks | |
| 16:03:16 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Make overcommit check for pinned instance pagesize aware https://review.opendev.org/631053 | |
| 17:07:23 | sean-k-mooney | gibi: bauzas melwitt can i get your input on how backportale someting is quickly | |
| 17:08:47 | sean-k-mooney | basically i want to add a call to unplug_vifs in migrate_disk_and_power_off which is caleed in _resize_instance | |
| 17:09:12 | sean-k-mooney | and ideally remove the call to clean up the souce vifs form resize confirm | |
| 17:09:48 | sean-k-mooney | so when we do resize_instance before we go to resize_verify we would unplug the souce vif | |
| 17:09:58 | openstackgerrit | Merged openstack/nova stable/train: post live migration: don't call Neutron needlessly https://review.opendev.org/750670 | |
| 17:10:05 | sean-k-mooney | basicaly right after we call power off just like we remove the cinder attahcments | |
| 17:10:06 | openstackgerrit | Merged openstack/nova master: docs: Improve 'AggregateImagePropertiesIsolation' docs https://review.opendev.org/752236 | |
| 17:10:14 | openstackgerrit | Merged openstack/nova master: functional: Enable real policy fixture by default https://review.opendev.org/745906 | |
| 17:10:23 | openstackgerrit | Merged openstack/nova master: functional: Add and use 'GlanceFixture' https://review.opendev.org/745907 | |
| 17:11:24 | openstack | bug 1895220 in OpenStack Compute (nova) "Fail to cold migrate between two hosts have difference VIF type" [Medium,In progress] https://launchpad.net/bugs/1895220 - Assigned to Dat Le (datlq) | |
| 17:11:24 | sean-k-mooney | it would fix bug #1895220 and i also need to do it for https://review.opendev.org/#/c/602432 to adress bug #1734320 | |
| 17:11:25 | openstack | bug 1734320 in neutron "Eavesdropping private traffic" [High,In progress] https://launchpad.net/bugs/1734320 - Assigned to Rodolfo Alonso (rodolfo-alonso-hernandez) | |
| 17:11:52 | openstackgerrit | melanie witt proposed openstack/nova stable/train: Sanity check instance mapping during scheduling https://review.opendev.org/752279 | |
| 17:13:49 | openstackgerrit | melanie witt proposed openstack/nova stable/train: Sanity check instance mapping during scheduling https://review.opendev.org/752279 | |
| 17:16:42 | sean-k-mooney | lyarwood: elod i guess same question to ye since its about backportablity ^ | |
| 17:17:59 | sean-k-mooney | the current behavior rely on libvirt to unplug the vif so to maintain the correct behavior when we deleate that to os-vif we now need to call unplug_vifs explcitly | |
| 17:18:16 | sean-k-mooney | which we should have been doing along really | |
| 17:21:05 | melwitt | sean-k-mooney: is the unplug_vifs functionality in os-vif available in older branches? | |
| 17:22:16 | sean-k-mooney | yes | |
| 17:22:27 | sean-k-mooney | unplug_vifs is a libvirt driver function | |
| 17:22:45 | sean-k-mooney | but the os-vif support is also there in older branches | |
| 17:23:30 | sean-k-mooney | melwitt: it look like im going to either need to make this change to when we do it | |
| 17:23:50 | sean-k-mooney | e.g. from confirm_resize where it is today to resize_instance | |
| 17:24:13 | sean-k-mooney | or im going to have to alter where and if nova waits for vifs in revert | |
| 17:25:01 | sean-k-mooney | or i guess i could have revert call unplug_vifs before it calls plug_vifs but that is slightly more complicated. | |
| 17:26:09 | sean-k-mooney | melwitt: for the CVE fix we have to do a locksetp backport in neutron and nova. | |
| 17:26:28 | sean-k-mooney | its a publci CVE by the way | |
| 17:26:50 | sean-k-mooney | for the other case of cold migrating/resizign between network backends | |
| 17:27:01 | sean-k-mooney | its technically not something tha was offically supported | |
| 17:27:11 | sean-k-mooney | which i noted in the bug | |
| 17:27:18 | sean-k-mooney | but it works in a limited set of cases | |
| 17:27:29 | melwitt | yeah I guess I'm not immediately seeing a backportability issue (but I could be missing something) but I think we'd want to let bake on master for awhile before backporting everywhere, to catch bugs before it's propagated to more branches | |
| 17:27:31 | sean-k-mooney | e.g. ml2/ovs to ml2/ovn | |
| 17:27:59 | sean-k-mooney | melwitt: there is one edge case im not sure how to adress | |
| 17:28:20 | melwitt | maybe it's just me but the fix sounds complicated and something we'd want to see if problems appear once it's running throughout the gate | |
| 17:29:03 | sean-k-mooney | well the first fix is just one addtion fucntion all in resize_instance to unplug_vifs | |
| 17:29:27 | melwitt | is that consistent with what we do with volumes today too? | |
| 17:29:34 | sean-k-mooney | bit that makes it trickin is to fully solve the diffent backend case i need to remove the current call in confirm | |
| 17:29:56 | sean-k-mooney | melwitt: ya for volumes we remove attacments in resize_instance | |
| 17:30:32 | melwitt | ok. it seems like it would make sense to have them be consistent but I don't know the reason vifs are handled differently | |
| 17:30:55 | sean-k-mooney | well technically in migrate_disk_and_poweroff | |
| 17:30:56 | sean-k-mooney | https://github.com/openstack/nova/blob/e0f088c95d05e9cf32d4af4c7cfc20566b17f8e1/nova/virt/libvirt/driver.py#L10362 | |
| 17:31:22 | sean-k-mooney | and im suggesting adding self.unplug_vifs here https://github.com/openstack/nova/blob/e0f088c95d05e9cf32d4af4c7cfc20566b17f8e1/nova/virt/libvirt/driver.py#L10357 | |
| 17:31:22 | melwitt | yeah, I did see the volume detach there | |
| 17:31:38 | sean-k-mooney | basicly 5 lines up | |
| 17:32:30 | melwitt | yeah, seems to make sense to me | |
| 17:32:48 | sean-k-mooney | melwitt: basically power_off call _destory() not destroy() to delete the domain xml without remving the disk images | |
| 17:32:57 | sean-k-mooney | so we can revert without copying data | |
| 17:33:08 | sean-k-mooney | but _destory() does not call unplug_vifs | |
| 17:33:25 | sean-k-mooney | destroy() does via the call to cleanup() | |
| 17:33:30 | sean-k-mooney | which delete the disk too | |
| 17:33:43 | melwitt | right | |
| 17:34:22 | sean-k-mooney | and today conrim does an rpc from the dest to the source to call unplug_vifs | |
| 17:34:50 | sean-k-mooney | here in cleanup_resize https://github.com/openstack/nova/blob/e0f088c95d05e9cf32d4af4c7cfc20566b17f8e1/nova/virt/libvirt/driver.py#L1665 | |
| 17:35:43 | melwitt | yeah I guess I wonder what are the potential issues if the deployment is in the middle of an upgrade | |
| 17:35:50 | sean-k-mooney | for revert we call plug_vifs in create_instace_with_networks here https://github.com/openstack/nova/blob/e0f088c95d05e9cf32d4af4c7cfc20566b17f8e1/nova/virt/libvirt/driver.py#L6704 https://github.com/openstack/nova/blob/e0f088c95d05e9cf32d4af4c7cfc20566b17f8e1/nova/virt/libvirt/driver.py#L10681-L10683 | |
| 17:36:09 | openstackgerrit | Lee Yarwood proposed openstack/nova-specs master: WIP/DNM - Flavor and image defined ephemeral storage encryption https://review.opendev.org/752284 | |
| 17:36:15 | sean-k-mooney | melwitt: right so if i dont remove the unplug_vifs form the cleanup in confim | |
| 17:36:19 | sean-k-mooney | i think its ok if i do | |
| 17:36:26 | lyarwood | sean-k-mooney: sorry just pushing something and I need to drop again, can we catch up about that backport stuff in the morning? | |
| 17:36:42 | lyarwood | s/morning/your\ morning/g ;) | |
| 17:36:47 | sean-k-mooney | then if i have an old souce and a new dest then i think we dont clean up the vifs | |
| 17:37:00 | sean-k-mooney | lyarwood: ya we can no rush | |
| 17:37:19 | sean-k-mooney | melwitt: im not sure how to handel that other then leave the extra call to unplug_vifs in the backport | |
| 17:37:47 | sean-k-mooney | melwitt: unplug_vifs shoudl be callable twice without any ill effects | |
| 17:38:36 | melwitt | yeah, well wouldn't you have to leave it on master (not only backports) for one cycle? to bridge the N-1 to N upgrade? | |
| 17:39:10 | sean-k-mooney | melwitt: well the proablem is i need to backport the cve issue to like queens eventually | |
| 17:39:43 | sean-k-mooney | what i think i would have to do is keep the call to unplug_vifs in cleanup but wrap it in a try excetp | |
| 17:39:59 | melwitt | I'm just saying re: leaving the extra call, I think you'd have to leave it on master too, not only leave it on the backports. because the upgrade issue exists for master as well | |
| 17:40:08 | sean-k-mooney | oh right | |
| 17:40:17 | sean-k-mooney | i woudl have to leave it on master for a cycle ya | |
| 17:40:46 | sean-k-mooney | ill put the patches up for peopel to see as 3 patches | |
| 17:41:10 | sean-k-mooney | 1 for the new call in resize. one for the cve fix which is alredy up for review and one for the extra call removal | |
| 17:41:28 | sean-k-mooney | and we can defer that last one for a cycle | |
| 17:42:04 | sean-k-mooney | and i guess we can see if peopel think of any other reason not to do it this way. | |
| 17:42:10 | melwitt | yeah, so AFAICT the backport concern here would be the complexity and impact to something that is supposed to be "stable" but it's needed for a CVE so I think that makes it backportable (again unless I'm missing something) | |
| 17:43:03 | sean-k-mooney | ya if we think its backportable and we are happy with it on master i guess i dont have to rush the backport either | |
| 17:43:17 | sean-k-mooney | e.g. we coudl wait a while and let it back | |
| 17:43:20 | sean-k-mooney | *bake | |
| 17:44:24 | melwitt | yeah I'm just concerned about there being a bug or edge case that crops up in the gate and then we have a fix to backport 5 times too | |
| 17:44:51 | melwitt | not the end of the world, we did it recently :P but the goal is to avoid that | |
| 17:45:10 | sean-k-mooney | :) ya im ok with that | |
| 17:45:26 | sean-k-mooney | this partcalar issue has been dragin on since 2018 or longer | |
| 17:45:27 | melwitt | let's see what other ppl think. I worry I'm missing something unbackportable about it | |
| 17:45:46 | sean-k-mooney | 2017-11-24 | |
| 17:46:03 | melwitt | yeah, I think that might be the same neutron security bug I emailed about downstream a long time ago | |
| 17:46:22 | sean-k-mooney | i started working on this when i was still at intel | |