Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-12-13
12:41:58 gibi pslestang: +2
12:43:01 sean-k-mooney pslestang: ill review that sortly after i grab a coffee
12:43:31 sean-k-mooney i unexpectly have meeting for the next 4 hours but i should have time to review your spec before i have to join them
12:44:27 sean-k-mooney its almost as if there is pending holidays that people will be taking on mass and everything is beeing cramed into this week :)
12:44:29 gibi sean-k-mooney: I hope that 4 hour meeting is some light gathering before the vacation time to chill out and slow down :)
12:44:44 pslestang gibi: thanks
12:44:49 plibeau3 lyarwood: I have apply a part of proposition. I have put some question on patchset3. -> https://review.opendev.org/c/openstack/nova/+/820531
12:44:55 pslestang sean-k-mooney: sure, don't worry
12:45:06 gibi pslestang: thank you for the nicely written spec!
12:45:20 gibi plibeau3: lyarwood is on PTO already
12:45:31 gibi plibeau3: so be prepared for slow response :)
12:46:00 plibeau3 gibi: oki :)
12:47:50 opendevreview Merged openstack/nova stable/xena: Query ports with admin client to get resource_request https://review.opendev.org/c/openstack/nova/+/811407
12:54:58 sean-k-mooney bauzas: unless you want to to specifically look at the instance-action sepc im also not +2 on it and was going to +w
12:55:29 sean-k-mooney pslestang: thanks for adding the lines regardding restore
13:08:10 pslestang gibi: sean-k-mooney thanks!
13:29:02 gibi bauzas: I've responded in https://review.opendev.org/c/openstack/nova/+/820549 , could you upgrade your vote to +2? :)
14:04:19 bauzas gibi: sean-k-mooney: sorry, was afk for getting a 3rd jab
14:04:35 gibi bauzas: nice
14:04:44 sean-k-mooney bauzas: no worries i just +w'd the instance action spec
14:04:58 sean-k-mooney i can porbly pull it out of the gate if you want to review?
14:05:10 sean-k-mooney gibi and i are both +2 on it
14:07:13 bauzas sean-k-mooney: nah, eventually I just +wd it too
14:07:26 bauzas I'm OK with the way
14:07:35 bauzas so we wouldn't have any upgrade issues
14:11:38 sean-k-mooney ack
14:16:51 opendevreview Merged openstack/nova-specs master: Implements: blueprint soft-delete-instance-actions https://review.opendev.org/c/openstack/nova-specs/+/821387
14:38:18 gibi gmann: due to https://review.opendev.org/c/openstack/nova/+/821423 the AZ related tempest tests cannot be run in parallel with tests that are booting instances (you can see many example failures in the run on that patch). Is there an easy way to separate these tests out?
14:39:10 gibi I see fixtures.LockFixture('availability_zone') in these tests, but I guess that only protects the AZ test from each other
14:39:32 gibi and it would be hard to add the same lock to all the other tests that boot instances
14:44:25 gibi gmann: I'm thinking about separating out the AZ tests to a separate tempest run command in all tox targets, but that is also a bit of a pain to do properly
15:37:24 gmann gibi: looking
16:15:25 opendevreview yuval proposed openstack/nova master: Lightbits LightOS driver https://review.opendev.org/c/openstack/nova/+/821606
16:36:53 jhartkopf Hi there, I need another +2 on my patch: https://review.opendev.org/c/openstack/nova/+/813672 It's just a small fix. I'd appreciate if someone could take time for review.
16:46:40 gmann gibi: I got the problem, its tempest test also need to modify as per your change.
16:47:29 gibi gmann: what would be the good way to separate the AZ tests from the rest?
16:47:32 gmann gibi: test is trying to remove the host which has servers (bug you are fixing). you can see remove host request - https://zuul.opendev.org/t/openstack/build/5a9aad45498d45cda1eabd2b87b743d2/log/controller/logs/screen-n-api.txt#2268
16:47:41 gmann and delete server request is after that https://zuul.opendev.org/t/openstack/build/5a9aad45498d45cda1eabd2b87b743d2/log/controller/logs/screen-n-api.txt#2568
16:48:00 gmann gibi: I am fixing test to wait for server delete before it does remove host request
16:48:51 gmann there is no race condition here its just tests did not wait for server to be deleted before remove_host is called in cleanup
16:49:01 gibi gmann: OK, if that is in a single test case I can fix that up. But I think there are cases when we have two parallel tempest tests 1) does some AZ testing by moving host between aggregates 2) boot instance on host. If this two happens in parallel then the 1) will fail after the bugfix
16:50:10 gmann gibi: I can check those if there is any such case. I think that is only test we have to add/remove host to agg with server
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 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

Earlier   Later