| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-12-07 | |||
| 18:12:32 | melwitt | I dunno, I don't see that CinderFixture is used anywhere for these tests? | |
| 18:12:52 | melwitt | even on master | |
| 18:13:23 | sean-k-mooney | actully | |
| 18:13:34 | sean-k-mooney | again i keep forgeting this works on python 3 | |
| 18:13:49 | sean-k-mooney | so likely 1 this is just not mocked properly | |
| 18:14:05 | sean-k-mooney | and 2 the deepcopy and majicmock issue was proably fixed in python 3 | |
| 18:14:41 | melwitt | it even works in other tests with that path in the same file on python 2 | |
| 18:15:09 | sean-k-mooney | ya | |
| 18:15:20 | sean-k-mooney | although those were calling idffernt methods like terminate right | |
| 18:15:39 | sean-k-mooney | or was it working in other cases wehre there was a deepcopy | |
| 18:16:01 | melwitt | double checking.. | |
| 18:16:09 | sean-k-mooney | melwitt: in anycase yes i think treaking this on train is vaild. | |
| 18:16:43 | melwitt | sean-k-mooney: like this one, it doesn't fail https://github.com/openstack/nova/blob/master/nova/tests/unit/volume/test_cinder.py#L717 | |
| 18:16:45 | sean-k-mooney | we might also want to fix this on master but not really sure how to go about that | |
| 18:16:51 | melwitt | and it's pretty much exactly the same | |
| 18:17:02 | sean-k-mooney | but it does not raise | |
| 18:17:27 | melwitt | it raises the first call | |
| 18:17:30 | sean-k-mooney | and as i said im wondering if this is the issue with excptions have a circurlar dep | |
| 18:17:43 | sean-k-mooney | right but it does not return the excption | |
| 18:18:02 | sean-k-mooney | are we storign the excption into the magicmock when it does raise | |
| 18:18:04 | melwitt | no, ok | |
| 18:18:26 | melwitt | what about this one https://github.com/openstack/nova/blob/master/nova/tests/unit/volume/test_cinder.py#L731 | |
| 18:20:46 | sean-k-mooney | ok that a 400 so https://review.opendev.org/c/openstack/nova/+/866091/2/nova/volume/cinder.py is not going to discard it | |
| 18:21:23 | melwitt | ohhh ok, I see what you're saying | |
| 18:21:32 | melwitt | ok, so it is "unique" | |
| 18:21:48 | sean-k-mooney | but it caught here i think https://github.com/openstack/nova/blob/85c954444493199c6edb01d9bdaa07fd9cf6d729/nova/virt/block_device.py#L520 | |
| 18:22:00 | sean-k-mooney | actully not 400 is bad request not found | |
| 18:22:47 | sean-k-mooney | well the test that is failing is test_detach_internal_server_error | |
| 18:23:07 | melwitt | yeah | |
| 18:23:34 | melwitt | and it's the only one that raises after a retrying, like you pointed out | |
| 18:23:49 | sean-k-mooney | which you did not modify https://review.opendev.org/c/openstack/nova/+/866091/2/nova/tests/unit/volume/test_cinder.py#706 | |
| 18:23:58 | sean-k-mooney | ya its a 500 so it will hit the retry decorator | |
| 18:24:21 | sean-k-mooney | although it did that before too | |
| 18:24:26 | melwitt | yeah.. exactly | |
| 18:25:58 | melwitt | the change from type(e) == cinder_apiclient.exceptions.InternalServerError to (isinstance(e, cinder_exception.ClientException) and e.code == 500)) makes it fail (in the patch below is where it started failing) | |
| 18:30:16 | sean-k-mooney | this is where you did that chagne | |
| 18:30:19 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/866090/2/nova/tests/unit/volume/test_cinder.py#672 | |
| 18:30:31 | sean-k-mooney | well where that change was made | |
| 18:30:39 | sean-k-mooney | i dont know if you wrote this patch orginailly | |
| 18:30:59 | sean-k-mooney | no it was takashi | |
| 18:31:25 | sean-k-mooney | oh sorry | |
| 18:31:29 | melwitt | yeah. and yeah L672 is the thing I added as a one-off to make this pass | |
| 18:31:33 | sean-k-mooney | so that is where you added the mock | |
| 18:31:38 | melwitt | yes | |
| 18:31:39 | sean-k-mooney | @mock.patch('nova.volume.cinder.API.get', new=mock.MagicMock()) | |
| 18:31:49 | sean-k-mooney | and have you just not rebased the second patch where it failing | |
| 18:31:56 | sean-k-mooney | cause i did not see the mock there | |
| 18:32:16 | melwitt | it should be there ... looking | |
| 18:32:32 | sean-k-mooney | oh sorry it is | |
| 18:32:50 | sean-k-mooney | but its havving issue with deepcopy | |
| 18:32:54 | melwitt | it's still there https://review.opendev.org/c/openstack/nova/+/866091/2/nova/tests/unit/volume/test_cinder.py#704 | |
| 18:33:08 | sean-k-mooney | ya | |
| 18:33:49 | sean-k-mooney | this is really strange | |
| 18:34:36 | melwitt | agreed | |
| 18:35:01 | sean-k-mooney | so this https://review.opendev.org/c/openstack/nova/+/866091/2/nova/volume/cinder.py is the only code change that could affect that test | |
| 18:36:10 | sean-k-mooney | adding extra tests can possibel cause the exsting on to fail unless... | |
| 18:36:39 | sean-k-mooney | can you put a patch on top that remove the extra tests | |
| 18:37:01 | sean-k-mooney | basically revert https://review.opendev.org/c/openstack/nova/+/866091/2/nova/tests/unit/volume/test_cinder.py | |
| 18:37:18 | sean-k-mooney | i really dont think we are leaking any state | |
| 18:40:39 | melwitt | sean-k-mooney: but it's the bottom patch that was failing, it's where this started. before any tests were added | |
| 18:48:08 | sean-k-mooney | well the bottom patch now work with the addtion of the mock right | |
| 18:48:18 | sean-k-mooney | but then the top patch fails | |
| 18:48:27 | sean-k-mooney | on the test you added the mock too | |
| 18:49:05 | sean-k-mooney | Hhttps://review.opendev.org/c/openstack/nova/+/866090/1..2/nova/tests/unit/volume/test_cinder.py | |
| 18:49:25 | sean-k-mooney | that is the only code change between v1 and v2 and it worked | |
| 18:49:47 | sean-k-mooney | and then that exact same test fails on teh next patch | |
| 18:50:47 | sean-k-mooney | oh wait | |
| 18:50:50 | sean-k-mooney | id didnt | |
| 18:51:01 | sean-k-mooney | sorry i tought the -v on https://review.opendev.org/c/openstack/nova/+/866091/2 | |
| 18:51:07 | sean-k-mooney | was for the unit test failure | |
| 18:51:23 | sean-k-mooney | its not its form tempest-slow-py3 | |
| 18:51:58 | sean-k-mooney | melwitt: then yes i think that single mock is fine | |
| 18:52:03 | melwitt | oh, yeah. stable/train is never with it so its CI fails half the time :P | |
| 18:52:47 | sean-k-mooney | test_volume_swap failed in the slow job on the second patch | |
| 18:53:17 | sean-k-mooney | Details: volume 29450c9c-2303-46b1-a3c7-46c22abd2a90 failed to reach available status (current in-use) within the required time (196 s). | |
| 18:53:40 | sean-k-mooney | i dont think that is related to your patch | |
| 18:53:51 | sean-k-mooney | so i guess im +1 on both | |
| 18:54:11 | sean-k-mooney | +1 becasue we have not merged this on the newwer branches | |
| 18:54:28 | melwitt | yeah | |
| 18:54:31 | melwitt | ok, cool, thanks | |
| 18:57:40 | sean-k-mooney | soory it took so long to get to that point | |
| 18:57:55 | melwitt | that's ok :D | |
| 18:58:34 | sean-k-mooney | given its not 7 here its proably a sign that my brain has finshed for today so im going to follw its lead and go have food | |
| 18:58:40 | melwitt | I really wanted to know why it's failing too, like why 2.7 only | |
| 18:58:51 | sean-k-mooney | ya its odd | |
| 18:59:04 | sean-k-mooney | i wonder if it was just flaky | |
| 18:59:22 | sean-k-mooney | like woudl it alwasy fail | |
| 18:59:26 | melwitt | I ran it locally a bunch and it was very consistent | |
| 18:59:44 | sean-k-mooney | weird | |
| 18:59:47 | melwitt | I didn't run it in a long running loop but just while I was messing with it I ran it several times | |
| 19:00:41 | melwitt | if I left it running in a loop overnight, maybe it would pass at some point 😂 | |
| 19:06:42 | sean-k-mooney | thats a lower pass rate then is desireable in ci | |
| 19:06:55 | sean-k-mooney | so i think we are good with your change | |
| 19:06:56 | melwitt | a little | |
| #openstack-nova - 2022-12-08 | |||
| 06:46:21 | opendevreview | yangzhipeng proposed openstack/nova master: when evacuate is performing, and restart compute node, if get instance info early, the instance state is not latest. this will reset instance task to error incorrectly, so refresh instance when modify instance state. https://review.opendev.org/c/openstack/nova/+/866960 | |
| 06:53:50 | opendevreview | yzp proposed openstack/nova master: Remove all tag if instance has beed hard deleted. https://review.opendev.org/c/openstack/nova/+/865362 | |
| 07:54:21 | opendevreview | yzp proposed openstack/nova master: Refresh instance when init instance in rebuilding https://review.opendev.org/c/openstack/nova/+/866960 | |
| 07:58:33 | opendevreview | yzp proposed openstack/nova master: Remove all tag if instance has beed hard deleted. Signed-off-by: yangzhipeng |
|
| 07:58:33 | opendevreview | yzp proposed openstack/nova master: Remove all tag if instance has beed hard deleted. Signed-off-by: yangzhipeng |
|
| 08:00:22 | opendevreview | yzp proposed openstack/nova master: Remove all tag if instance has beed hard deleted. Signed-off-by: yangzhipeng |
|