Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-12-13
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
17:27:20 gmann new bug fix in nova and so does tempest tests need fix
17:29:43 sean-k-mooney this is for stephens bug fix to prevent you updating aggreate when there are isntance on the host?
17:30:00 gibi sean-k-mooney: yes
17:30:02 sean-k-mooney that was previously ment to be blocked but i assume there was an edge case we missed
17:30:04 gmann yeah
17:30:18 gmann https://bugs.launchpad.net/nova/+bug/1907775
17:30:19 gibi sean-k-mooney: https://review.opendev.org/c/openstack/nova/+/821423
17:30:27 sean-k-mooney ok that was "fixed" for a different edgecase many releases ago
17:30:40 sean-k-mooney so im surpised tempest has not hit this already

Earlier   Later