| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-06-21 | |||
| 10:28:42 | gibi | but apperantly we never went there and replaced them with calls pushing down the necessary daya | |
| 10:28:42 | sean-k-mooney | we currently only have 2 i think | |
| 10:28:45 | gibi | data | |
| 10:29:13 | sean-k-mooney | we have the track_instance_changes config option ot push data to the schduler | |
| 10:29:22 | sean-k-mooney | and the group policy check upcall | |
| 10:32:20 | gibi | OK now I can reproduce the issue on master https://paste.opendev.org/show/btK22SLw4UH6oDDu5wKw/ | |
| 10:32:23 | gibi | we go this way https://github.com/openstack/nova/blob/ebe08834f311e8e22bfd9685d7e6e91dab967382/nova/compute/manager.py#L1733 | |
| 10:32:37 | gibi | and there we dont have the except branch | |
| 10:32:48 | gibi | not like in L1741 | |
| 10:33:37 | sean-k-mooney | yep | |
| 10:33:53 | sean-k-mooney | that what i was saying downstream | |
| 10:34:04 | gibi | now we have proof :) | |
| 10:34:15 | sean-k-mooney | we jut need to move the try to the top of the if | |
| 10:34:21 | gibi | bauzas: ^^ I go re-triage the upstream bug | |
| 10:34:39 | sean-k-mooney | so https://github.com/openstack/nova/blob/ebe08834f311e8e22bfd9685d7e6e91dab967382/nova/compute/manager.py#L1738= need to move to https://github.com/openstack/nova/blob/ebe08834f311e8e22bfd9685d7e6e91dab967382/nova/compute/manager.py#L1718= | |
| 10:35:18 | sean-k-mooney | it shoudl be easy to prove that with a unit test too | |
| 10:39:57 | gibi | sean-k-mooney, bauzas: I updated the upstream bug. I agree that we should fix this as suggested ^^. | |
| 10:40:16 | gibi | but now I go get some lunch and go back to the pci tracking patches | |
| 10:42:33 | sean-k-mooney | gibi: do you want me to just submit a unit test repoducer and then move the try | |
| 10:42:41 | gibi | sean-k-mooney: sure you can go | |
| 10:42:59 | gibi | I'm not assigning the bug to me :) | |
| 11:25:47 | opendevreview | sean mooney proposed openstack/nova master: add repoducer test for bug 1890244 https://review.opendev.org/c/openstack/nova/+/847000 | |
| 11:25:47 | opendevreview | sean mooney proposed openstack/nova master: ignore deleted server groups in validation https://review.opendev.org/c/openstack/nova/+/847001 | |
| 11:25:56 | sean-k-mooney | bauzas: gibi ^ | |
| 11:54:39 | opendevreview | Balazs Gibizer proposed openstack/nova master: Basics for PCI Placement reporting https://review.opendev.org/c/openstack/nova/+/846187 | |
| 11:54:39 | opendevreview | Balazs Gibizer proposed openstack/nova master: Extend device_spec with resource_class and traits https://review.opendev.org/c/openstack/nova/+/846218 | |
| 11:54:40 | opendevreview | Balazs Gibizer proposed openstack/nova master: Reject PCI dependent device config https://review.opendev.org/c/openstack/nova/+/846435 | |
| 11:54:40 | opendevreview | Balazs Gibizer proposed openstack/nova master: Reject mixed VF rc and trait config https://review.opendev.org/c/openstack/nova/+/846436 | |
| 11:54:41 | opendevreview | Balazs Gibizer proposed openstack/nova master: Ignore PCI devs with physical_network tag https://review.opendev.org/c/openstack/nova/+/846219 | |
| 11:54:41 | opendevreview | Balazs Gibizer proposed openstack/nova master: Reject devname based device_spec config https://review.opendev.org/c/openstack/nova/+/846466 | |
| 11:54:42 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support [pci]device_spec reconfiguration https://review.opendev.org/c/openstack/nova/+/846470 | |
| 11:54:43 | opendevreview | Balazs Gibizer proposed openstack/nova master: Stop if tracking is disable after it was enabled before https://review.opendev.org/c/openstack/nova/+/847009 | |
| 11:58:01 | opendevreview | Rajesh Tailor proposed openstack/nova stable/wallaby: Add missing condition https://review.opendev.org/c/openstack/nova/+/847011 | |
| 11:58:04 | opendevreview | Balazs Gibizer proposed openstack/nova master: Add more test coverage for devname base dev spec https://review.opendev.org/c/openstack/nova/+/844625 | |
| 11:58:04 | opendevreview | Balazs Gibizer proposed openstack/nova master: Extra tests for remote managed dev spec https://review.opendev.org/c/openstack/nova/+/844626 | |
| 11:58:05 | opendevreview | Balazs Gibizer proposed openstack/nova master: Unparent PciDeviceSpec from PciAddressSpec https://review.opendev.org/c/openstack/nova/+/844491 | |
| 11:58:05 | opendevreview | Balazs Gibizer proposed openstack/nova master: Fix PciAddressSpec descendants to call super.__init__ https://review.opendev.org/c/openstack/nova/+/844565 | |
| 11:58:06 | opendevreview | Balazs Gibizer proposed openstack/nova master: Remove dead code from PhysicalPciAddress https://review.opendev.org/c/openstack/nova/+/844628 | |
| 11:58:06 | opendevreview | Balazs Gibizer proposed openstack/nova master: Poison /sys access via various calls in test https://review.opendev.org/c/openstack/nova/+/844627 | |
| 11:58:07 | opendevreview | Balazs Gibizer proposed openstack/nova master: Clean up mapping input to address spec types https://review.opendev.org/c/openstack/nova/+/845765 | |
| 11:58:07 | opendevreview | Balazs Gibizer proposed openstack/nova master: Remove unused PF checking from get_function_by_ifname https://review.opendev.org/c/openstack/nova/+/845775 | |
| 11:58:08 | opendevreview | Balazs Gibizer proposed openstack/nova master: Fix type annotation of pci.Whitelist class https://review.opendev.org/c/openstack/nova/+/845780 | |
| 11:58:09 | opendevreview | Balazs Gibizer proposed openstack/nova master: Move __str__ to the PciAddressSpec base class https://review.opendev.org/c/openstack/nova/+/845781 | |
| 12:07:18 | gibi | sean-k-mooney, bauzas: I consider the first (first and the half:D) batch of patches of pci tracking in placement ready for review. I posted a rewiew guide to the ML for help http://lists.openstack.org/pipermail/openstack-discuss/2022-June/029156.html | |
| 12:10:02 | gibi | sean-k-mooney: I'm +2 on the evac server group fix | |
| 12:10:06 | gibi | bauzas: easy win ^^ | |
| 12:13:58 | sean-k-mooney | :) i see it in my inbox ill try and take a look at them this week | |
| 12:14:59 | sean-k-mooney | """This covers the first 4 sub chapters of Proposed Change""" | |
| 12:15:35 | sean-k-mooney | have i said how happy i am that you volenterred to work on this :) | |
| 12:17:19 | gibi | yes you have :) | |
| 12:17:48 | gibi | actually after the initial shock, this batch of patches was a nice coding excersize | |
| 12:24:48 | opendevreview | Rajesh Tailor proposed openstack/nova master: Remove unnecessary if condition https://review.opendev.org/c/openstack/nova/+/844418 | |
| 12:42:18 | bauzas | gibi: sean-k-mooney: sorry, was doing other things, will look above | |
| 12:42:29 | gibi | bauzas: no worries | |
| 12:59:47 | bauzas | sean-k-mooney: thanks for having worked on it, it was an easy peasy | |
| 13:01:08 | sean-k-mooney | yep simple fix | |
| 13:02:35 | bauzas | sean-k-mooney: just -1 because of the try clause https://review.opendev.org/c/openstack/nova/+/847001 | |
| 13:03:14 | sean-k-mooney | i will have to duplicate the try | |
| 13:03:39 | sean-k-mooney | the only place that its not needed for is the schdueler_hints.get | |
| 13:04:26 | sean-k-mooney | i can duplicate it but i think this is cleaner but whatever ye prefer | |
| 13:14:27 | opendevreview | sean mooney proposed openstack/nova master: ignore deleted server groups in validation https://review.opendev.org/c/openstack/nova/+/847001 | |
| 13:14:51 | sean-k-mooney | bauzas: ^ | |
| 13:31:36 | bauzas | sean-k-mooney: +2d with a comment on the relnote (sorry missed it at first look) | |
| 14:04:14 | bauzas | gibi: updated the agenda for today's meeting | |
| 14:04:18 | gibi | bauzas: thanks | |
| 14:04:25 | bauzas | gibi: thanks for chairing it until I arrive | |
| 14:04:46 | bauzas | Uggla: missed to review your patches, yet another day with other prios | |
| 14:19:24 | bauzas | but I'll back around the meeting time | |
| 14:36:20 | opendevreview | Rajesh Tailor proposed openstack/nova master: Remove unnecessary if condition https://review.opendev.org/c/openstack/nova/+/844418 | |
| 14:59:50 | Uggla | bauzas, no pb. | |
| 15:00:19 | Uggla | bauzas, sean-k-mooney, please have a quick look at https://review.opendev.org/c/openstack/nova/+/845581/1/nova/compute/api.py and just tell me what you prefer. Longer code with logs (current version) vs shorter one with less logs (proposed by Gibi). If you prefer Gibi's proposal, I will refactor and you will be able to do the review with an updated version. | |
| 15:02:23 | Uggla | bauzas, sean-k-mooney, gibi can we have a look at https://review.opendev.org/c/openstack/nova-specs/+/833669/7..9/specs/zed/approved/libvirt-virtiofs-attach-manila-shares.rst#b94 to get rid of this point tomorrow morning ? | |
| 15:03:44 | sean-k-mooney | Uggla: i think shorter | |
| 15:03:53 | sean-k-mooney | just looking at https://review.opendev.org/c/openstack/nova/+/845581/1/nova/compute/api.py quickly | |
| 15:04:24 | sean-k-mooney | the orginal code had a lot of repaded logs for the differnt pbrances that i dont think will be helpful | |
| 15:04:40 | Uggla | sean-k-mooney, ok thank you. | |
| 15:05:09 | sean-k-mooney | im not against logs but that seams pretyy verbouse with a lot of repetition | |
| 15:12:20 | sean-k-mooney | bauzas: gibi i think https://review.opendev.org/c/openstack/nova/+/847001/2 might be incomplete | |
| 15:13:06 | sean-k-mooney | i think this happens because while we null out the server_group in teh request_spec | |
| 15:13:18 | sean-k-mooney | we dont remove the group form the hint | |
| 15:13:46 | gibi | Uggla: sure, ping me tomorrow morning to talk that through | |
| 15:14:10 | gibi | sean-k-mooney: ahh, so anther way to solve it would be to remove the hint from the request spec if the group is not found | |
| 15:14:40 | sean-k-mooney | gibi: we proably shoudl do both | |
| 15:14:48 | sean-k-mooney | so catch the instance not found | |
| 15:15:00 | sean-k-mooney | but make sure when the group is deleted we remove it form the schdluer hinits | |
| 15:15:37 | sean-k-mooney | gibi: you found where the group was deleted previously yes you dont happen to have the link to that still? | |
| 15:16:31 | gibi | sean-k-mooney: this is where we detect that that the group is gone https://github.com/openstack/nova/commit/94fd36f0582c5dbcf2b9886da7c7bf986d3ad5d1 | |
| 15:17:22 | gibi | sean-k-mooney: this is where the group is deleted in the API https://github.com/openstack/nova/blob/ebe08834f311e8e22bfd9685d7e6e91dab967382/nova/api/openstack/compute/server_groups.py#L145 | |
| 15:17:36 | sean-k-mooney | ya so that sets spec.instace_group to none but does not remvoe the group from the hint | |
| 15:18:06 | sean-k-mooney | so https://github.com/openstack/nova/commit/94fd36f0582c5dbcf2b9886da7c7bf986d3ad5d1#diff-8e61e9d80a8e925e5b0a9209a495ba0e4888e9fd81903ed0ab79edcbdb05901dR458 | |
| 15:18:15 | sean-k-mooney | is incomplete | |
| 15:18:36 | gibi | yeah we should extend that | |
| 15:18:44 | gibi | nuke the group hint | |
| 15:18:59 | gibi | as if the group does not exists it will never exesits in the future | |
| 15:19:11 | sean-k-mooney | yep so ill do both and then adress bauzas nit in the reno | |
| 15:19:15 | gibi | ack | |
| 15:20:16 | sean-k-mooney | do you think this need a func test by the way or are you ok with just unit test to repoduce | |
| 15:20:30 | sean-k-mooney | i could create one i guess | |
| 15:20:54 | sean-k-mooney | i was trying to keep it minimal but it proably does not hurt to add one | |
| 15:21:37 | gibi | I dont think it is hard to add an evact func test for this case. So I suggest to have one | |