| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-12-13 | |||
| 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 | gmann | it end up running tempest in serial | |
| 17:13:10 | sean-k-mooney | yes we do but the locking is slightly different | |
| 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 | gmann | yeah we should not do that as other tempest user still will face the error | |
| 17:24:39 | sean-k-mooney | its what we have done for senario tests for years | |
| 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 | |
| 17:33:06 | gibi | sean-k-mooney: what was actually fixed? | |
| 17:33:13 | gibi | sean-k-mooney: as this bug now has reproduction test | |
| 17:34:19 | gmann | yeah, we never added restriction for add/remove host having server | |
| 17:34:29 | gmann | that is what tempest tests is doing | |
| 17:35:09 | sean-k-mooney | is_safe_to_update_az | |
| 17:35:35 | sean-k-mooney | https://github.com/openstack/nova/commit/8e19ef4173906da0b7c761da4de0728a2fd71e24 | |
| 17:35:39 | sean-k-mooney | https://github.com/openstack/nova/commit/0ad5a64dc9ac4b1cfb8038f9171b6385fdf07f28 | |
| 17:35:46 | gmann | ohk, no two different AZ while adding | |
| 17:36:22 | sean-k-mooney | we currently block you form adding or removing the host to the host-aggreate that has the az metadata | |
| 17:37:12 | gmann | different AZ | |
| 17:37:14 | sean-k-mooney | well more when doign az update | |
| 17:37:33 | sean-k-mooney | but this code was ment to alos cater for this usecase | |
| 17:38:49 | gibi | hm the first one is aggregated update, that is different from add/remove host but yes that could race with instance boot | |
| 17:39:27 | gibi | the second is about one host can only be one AZ case | |
| 17:39:38 | gibi | that I think cannot race with instance boot | |
| 17:39:43 | sean-k-mooney | gibi: well yes and no. ading or removign a host form an aggreate is really an update too in the more general sense | |
| 17:40:14 | sean-k-mooney | my point was that https://review.opendev.org/c/openstack/nova/+/821423/1/nova/compute/api.py#6458 | |
| 17:40:27 | sean-k-mooney | should have been blocking this edgecacse already | |
| 17:40:58 | sean-k-mooney | but it does not hece the need for stephens change | |
| 17:42:24 | gibi | OK, I really need to drop | |
| 17:42:27 | gibi | for today | |
| 17:42:35 | gibi | I will think about it more tomorrow o/ | |
| #openstack-nova - 2021-12-14 | |||
| 00:03:42 | opendevreview | sean mooney proposed openstack/nova-specs master: Add spec for external cpu power managment https://review.opendev.org/c/openstack/nova-specs/+/821419 | |
| 00:19:13 | opendevreview | sean mooney proposed openstack/nova-specs master: add per process healthcheck spec https://review.opendev.org/c/openstack/nova-specs/+/821279 | |
| 01:09:30 | opendevreview | Steve Baker proposed openstack/nova master: Allow remaining in in DELETING for ironic cleaning https://review.opendev.org/c/openstack/nova/+/813729 | |