| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-12-13 | |||
| 16:50:39 | gibi | OK, I will gather the full picutre and fix up the cleanup issue | |
| 16:50:46 | gmann | gibi: let me update test and add your change as depends-on to see if all pass. and later we can merge the temest fix before nova change as per bug fix procedure | |
| 16:51:22 | gibi | gmann: sure, if you have time then I can appreciate the tempest fix | |
| 16:51:35 | gmann | those tempest test play on same host but we have only single tests doing server boot but I will check if we have any case like race condition you explained | |
| 16:51:37 | gmann | sure | |
| 16:53:32 | gibi | gmann: I think test_aggregate_basic_ops failure in https://zuul.opendev.org/t/openstack/build/83aad9cdd5b849d48f4ffb463880d259/logs is a race | |
| 16:53:41 | gibi | as that test case does not boot a server | |
| 16:53:51 | gibi | but it fails as probably other parallel test booted one | |
| 16:54:36 | gmann | gibi: ohk, did not see that in latest run. let me check | |
| 16:55:42 | gmann | we should not need lock here instead test should check host with AZ etc carefully | |
| 16:58:09 | gibi | gmann: so mean skip the AZ test if the host has instance? | |
| 16:58:21 | gibi | or wait in the test until the host has no instnace? | |
| 16:59:04 | gmann | gibi: skip like we do here https://github.com/openstack/tempest/blob/7e96c8e854386f43604ad098a6ec7606ee676145/tempest/api/compute/admin/test_aggregates.py#L228 | |
| 16:59:48 | gibi | gmann: but would not that mean we sometimes run the test sometimes does not depending on which other parallel test runs at the same time | |
| 16:59:51 | gibi | ? | |
| 17:00:03 | gibi | so in theory we might lose test coverage by that | |
| 17:00:39 | gmann | gibi: i mean skip if there is no such host in env. test creating instance has to be cleaned up so we can add wait or so | |
| 17:02:23 | gmann | gibi: ah, we do have lock there but not sure why race is happening https://github.com/openstack/tempest/blob/master/tempest/scenario/test_aggregates_basic_ops.py#L119 | |
| 17:02:58 | gmann | gibi: ohk but any other non agg test might be picking that host and creating server | |
| 17:03:09 | gibi | gmann: exactly ^^ | |
| 17:03:25 | gibi | so the lock only avoid two AZ test to race | |
| 17:03:58 | gmann | yeah | |
| 17:04:34 | gibi | can we somehow make the AZ tests always run _after_ all the other tests? | |
| 17:04:42 | gmann | and skip if host has server also does not solve the issue as server can be created in between | |
| 17:05:37 | gmann | gibi: we can configure that way in our CI but as tempest run it is still bug that this test canot be run in parallal | |
| 17:06:59 | sean-k-mooney | we do run the senario test after all the others in serial | |
| 17:07:07 | sean-k-mooney | so we could put the az test in that group | |
| 17:07:13 | sean-k-mooney | at least for tempest-full | |
| 17:07:38 | sean-k-mooney | https://github.com/openstack/tempest/blob/master/tox.ini#L112-L113 | |
| 17:07:54 | sean-k-mooney | we could exclude the az test the same way and run them with the senario and slow tests | |
| 17:08:22 | sean-k-mooney | its not the ideal solution but its an option | |
| 17:08:40 | gmann | gibi: one best we can do in tests 'check for host has no servers and then only add/remove tests' but that does not solve race completly | |
| 17:09:18 | sean-k-mooney | gmann: we donbt really want to make the dest depend on thing that are dynamic like that really | |
| 17:09:45 | gmann | gibi: or we can add try except and if conflict error due to server present then skip the test otherwise pass | |
| 17:09:59 | gibi | sean-k-mooney: you mean AZ test would be a 3rd group? | |
| 17:10:08 | gibi | to separate them from all the others | |
| 17:10:17 | sean-k-mooney | gibi: not entirly just put them with the senario tests | |
| 17:10:24 | sean-k-mooney | we could have them be a third group | |
| 17:10:27 | gmann | otherwise we have only choice of remvoe the test | |
| 17:10:35 | sean-k-mooney | but only if we could run them in paralle in that group | |
| 17:10:42 | gmann | sean-k-mooney: that does not solve the issue, it is just our CI fix | |
| 17:10:59 | gmann | anyone running tempest will face the same issue | |
| 17:11:01 | sean-k-mooney | gmann: right but the only way to solve the issue it so use a lock really | |
| 17:11:18 | gibi | sean-k-mooney: does the scenario / show runs in a single thread? if so we can add the AZ test there but if those also run in parallel then the same race can happen there too | |
| 17:11:26 | gmann | sean-k-mooney: we cannot use lock as it end up making all tests running serially | |
| 17:11:42 | sean-k-mooney | gibi: for tempest full its serical so one thread yes | |
| 17:11:43 | gmann | gibi: yeah that is happening now. | |
| 17:12:13 | gmann | scenario test also create server. | |
| 17:12:44 | gibi | yeah, locking means we basically need to lock agains any test that boots an instance, that is pretty restrictive. | |
| 17:12:51 | gmann | yeah | |
| 17:12:54 | sean-k-mooney | gibi: not quite | |
| 17:13:10 | sean-k-mooney | yes we do but the locking is slightly different | |
| 17:13:10 | gmann | it end up running tempest in serial | |
| 17:13:25 | sean-k-mooney | basiclly we need somihing like a reader writer lock | |
| 17:13:49 | sean-k-mooney | the aggreate tests need to grab the writer lock but the boot test need only a reader lock | |
| 17:14:18 | gmann | I see these are good option if we want to keep tests - 1. test will 'check for host has no servers and then only add/remove tests' 2. add try except and if conflict error due to server present then skip the test otherwise pass | |
| 17:14:34 | sean-k-mooney | so the build test can run in parallel with the read lock until a aggreate test try to modify thing in which case it grabes the writer lock | |
| 17:16:00 | gmann | we do have same kind of try except for image tests too for some cases, that is best tempest can test these APi race operations | |
| 17:16:27 | gmann | and which matches to the real operation scenario | |
| 17:16:54 | sean-k-mooney | perhaps but i still think a reader/writere lock would be a good fit here | |
| 17:17:43 | gmann | sean-k-mooney: but boot tests having reader lock will still create server on host as aggregate test having write lock. correct? | |
| 17:18:28 | gmann | if we want to isolate them the it has to be same lock | |
| 17:18:31 | sean-k-mooney | it depend on the sematics of the lcok. you can have it perfer reads so that you cant get the write lock until all readers relese | |
| 17:18:44 | sean-k-mooney | https://en.wikipedia.org/wiki/Readers%E2%80%93writer_lock | |
| 17:19:16 | gmann | yeah which is same thing right. end up running in serial as per many current boot tests | |
| 17:19:40 | sean-k-mooney | well we have to run in serial | |
| 17:19:47 | gmann | let me try the above check and then we can see how it looks like | |
| 17:19:56 | sean-k-mooney | we can run in parallel only if aggreates are not being changed | |
| 17:20:15 | sean-k-mooney | if they are then only that test can happen as its global state | |
| 17:21:19 | sean-k-mooney | we actully need "Write-preferring RW lock" semantics if we were to use this correctly | |
| 17:22:13 | gibi | as we have a small amount of AZ test I still think separating them out to the end would be easier than addin a reader lock to each test that boots an instance | |
| 17:22:27 | gibi | of course if such separation is possible | |
| 17:22:38 | sean-k-mooney | gibi: yep it would be but it need to be done by teh caller of tempest | |
| 17:22:54 | sean-k-mooney | we can do it via tox with the regex support | |
| 17:23:01 | sean-k-mooney | but every client would have to do that | |
| 17:23:02 | gibi | sean-k-mooney: then we might need to invent someting inside tempest to order cases? | |
| 17:23:14 | sean-k-mooney | unless we add a new feature to tempest to tag things as "must be serial" | |
| 17:23:18 | gibi | yeah | |
| 17:23:20 | gibi | like that | |
| 17:23:34 | sean-k-mooney | if we could do that as a decorator that would be nice | |
| 17:23:50 | gibi | OK, so we have couple of options a) RW lock b) @serial decorator | |
| 17:23:51 | sean-k-mooney | i just dont know if we can. i suspect its posible | |
| 17:24:05 | sean-k-mooney | c) tox.ini for now | |
| 17:24:19 | gibi | yeah temporarily tox.ini but I don't really like that | |
| 17:24:39 | sean-k-mooney | its what we have done for senario tests for years | |
| 17:24:39 | gmann | yeah we should not do that as other tempest user still will face the error | |
| 17:24:45 | sean-k-mooney | it works but you have to know to do it | |
| 17:25:18 | gibi | I will sleep on this. I don't need to rush to land the nova bug fix so we have time | |
| 17:25:21 | gmann | sean-k-mooney: we did that only for env/timeout restriction they can be run in parallel like we do in tempest-parallel job | |
| 17:25:38 | gibi | thanks for the input | |
| 17:25:45 | sean-k-mooney | gmann: they can if you have enough resouces in the could yes | |
| 17:25:47 | gibi | and I will read back if you still continue discussiing this | |
| 17:25:48 | gibi | :) | |
| 17:25:50 | gmann | sean-k-mooney: here we are making tempest cannot be run in parallel so we have to fix test not the way we run those | |
| 17:26:00 | sean-k-mooney | we do it because with our default concurance tehy wont fit in the ci vms | |
| 17:26:14 | sean-k-mooney | gmann: yep i understand | |
| 17:26:31 | sean-k-mooney | however if this is a gate blocker it better to unblock the gate and then fix it properly | |
| 17:26:44 | gmann | gibi:sean-k-mooney let me add test modification with checks and try/except. and later we can see if tagging a tests to run serial can be done or not | |
| 17:26:55 | sean-k-mooney | ack | |
| 17:27:00 | gmann | sean-k-mooney: this is not gate blokcer but change need with nova fix | |