| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-06-21 | |||
| 09:46:41 | sean-k-mooney | is there a reason we allow that | |
| 09:46:57 | sean-k-mooney | unlike the flavor for example we are not embdeding a copy fo the instance group | |
| 09:47:16 | sean-k-mooney | so we really should not allow ti to be remvoed if its in use | |
| 09:47:23 | sean-k-mooney | do we know why we do today? | |
| 09:48:44 | bauzas | sean-k-mooney: because Instance Groups are a terrible concept ?N | |
| 09:49:03 | sean-k-mooney | not nessiarly they may be implemented badly | |
| 09:49:23 | sean-k-mooney | the concept is not a bad one but the way we implemeted them is not good | |
| 09:52:56 | gibi | bauzas: could this be the reason we bail out https://github.com/openstack/nova/blob/ebe08834f311e8e22bfd9685d7e6e91dab967382/nova/compute/manager.py#L1742 | |
| 09:53:11 | gibi | (still instrumenting the code with logs then I will rerun the repro attempt) | |
| 09:55:00 | gibi | btw I agree with sean that group is a better concept than same_host / different_host hints, as the hints were not reflexive | |
| 09:55:24 | gibi | strike reflexive | |
| 09:55:28 | gibi | symmetric | |
| 09:55:34 | gibi | they wasnt symmetirc | |
| 09:56:01 | bauzas | let's discuss this tomorrow at the bug call | |
| 09:56:04 | bauzas | shit, d/s | |
| 10:11:03 | gibi | I have /etc/nova/nova-cpu.conf:disable_group_policy_check_upcall = True in my env, restaring repro .... | |
| 10:12:25 | sean-k-mooney | isnt that our default | |
| 10:14:45 | gibi | nope | |
| 10:14:49 | gibi | the default is false | |
| 10:14:57 | gibi | I had an explicit True set for some reason | |
| 10:17:21 | sean-k-mooney | devstack set it to true by defualt i think | |
| 10:17:37 | gibi | yepp devastack by default does not even support the upcall | |
| 10:17:54 | gibi | so If I enable the upcall nova blows as no DB access | |
| 10:19:00 | sean-k-mooney | wait what | |
| 10:19:08 | sean-k-mooney | it shoudl be doing that via rpc to conductor | |
| 10:19:20 | sean-k-mooney | is it trying to use the old local conductor path | |
| 10:19:23 | sean-k-mooney | and directly access the db | |
| 10:20:37 | gibi | https://paste.opendev.org/show/bxWFPkh1OoBfWlM15mS0/ | |
| 10:20:59 | gibi | it calls up to the cell conductor | |
| 10:21:06 | gibi | and cell conductor fails to read the api db | |
| 10:21:13 | gibi | as cell conductors has no access to the api db | |
| 10:21:14 | sean-k-mooney | oh | |
| 10:21:20 | sean-k-mooney | right | |
| 10:21:27 | gibi | the upcall is between the cell and the super conductor | |
| 10:21:27 | sean-k-mooney | so this will only work in singel conductor mode | |
| 10:21:32 | sean-k-mooney | which devstack support | |
| 10:21:35 | sean-k-mooney | but its not the default | |
| 10:21:59 | gibi | I can simply promote my cell conductor to a super one by adding the api db settings to it | |
| 10:22:10 | sean-k-mooney | yep that would work | |
| 10:22:21 | gibi | but this really feels like something that upstream we dont test hence dont really support | |
| 10:22:37 | gibi | anyhow I do the propomotion and retry | |
| 10:22:38 | sean-k-mooney | well we do have supprot to test this in devstack | |
| 10:22:46 | gibi | do we have jobs testing it? | |
| 10:22:51 | sean-k-mooney | i dobt it | |
| 10:22:56 | gibi | we have code, we dont have test :D | |
| 10:23:09 | sean-k-mooney | ill check with codesearch | |
| 10:23:13 | sean-k-mooney | and see if anything enables it | |
| 10:23:30 | gibi | anyhow I go and do the promotion now and retry. I still believe that nova handles the missing group gracefully | |
| 10:24:20 | sean-k-mooney | so kolla-ansible disable the upcall out of the box i didnt get any hits form tripleo or pupet so i assuem they have it enabled | |
| 10:24:32 | sean-k-mooney | ooo actully would work since it does not supprot super conductors | |
| 10:25:10 | sean-k-mooney | well if you do multi cell its usign the cell1 conductor as the super conductor | |
| 10:25:18 | sean-k-mooney | so it would work only in cell1 | |
| 10:26:04 | gibi | intersting setup | |
| 10:26:04 | gibi | :) | |
| 10:26:19 | sean-k-mooney | https://opendev.org/openstack/grenade/src/branch/master/.zuul.yaml#L188= | |
| 10:26:31 | sean-k-mooney | so our grenade job apprently uses singel conductor mode | |
| 10:27:12 | sean-k-mooney | so we could renable the upcall check there if we wanted | |
| 10:27:52 | sean-k-mooney | gibi: could we consider changing the default for this or removing the call eventually | |
| 10:27:58 | gibi | yeah but there we dont do evac testing | |
| 10:28:19 | gibi | sean-k-mooney: I don't know what is the startegy I think originally we wanted to get rid of all the upcall | |
| 10:28:22 | gibi | s | |
| 10:28:36 | sean-k-mooney | yes we did | |
| 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 | |