Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-12-13
12:11:50 sean-k-mooney the proposal for mixing will work for placment but now for nova/glance
12:12:47 sean-k-mooney i tink just using ; is simpler
12:24:56 gibi sean-k-mooney: it is almost directly following how member_of is modelled in the placement API
12:25:18 gibi so repeat means AND. OR expressed with in: prefix
12:26:02 gibi sure ';' could be used instead of repeat of the required query param but then we would create inconsistency with member_of
12:32:00 opendevreview Pierre-Samuel Le Stang proposed openstack/nova-specs master: Implements: blueprint soft-delete-instance-actions https://review.opendev.org/c/openstack/nova-specs/+/821387
12:33:07 sean-k-mooney gibi: it would yes but the ofllow on proposal of using multiple &required=...&required=...
12:33:17 sean-k-mooney cant be modled in flavors or images
12:33:30 sean-k-mooney since we cannot support multi opts and merge them
12:33:48 sean-k-mooney so we will need to have some other way to mix them in the flavor/image
12:34:20 sean-k-mooney which is why i think either requiring all in parmater to be at the end or using ; makes sense
12:35:40 gibi the current falvor syntax is trait:HW_CPU_X86_AVX2=required , so it can extended to trait:inFOO,BAR=required
12:35:49 opendevreview Merged openstack/nova stable/xena: Reproduce bug 1945310 https://review.opendev.org/c/openstack/nova/+/811405
12:36:32 gibi also trait:inFOO,BAR=required and trait:BAZ=required can be mixed
12:36:35 sean-k-mooney i guess that could work
12:36:38 pslestang gibi: thanks for reviewing the spec, I pushed an other patch to fix all the syntax error you underligned
12:36:44 gibi the flavor syntax is ugly
12:36:47 gibi but that is a different story
12:36:48 gibi :D
12:36:50 sean-k-mooney gibi: this might be nicer trait:FOO,BAR=required_in
12:37:03 gibi sean-k-mooney: yeah, that could be another option
12:37:24 gibi the point is that we have a way to modell repetition as the key contains the trait name
12:37:33 gibi pslestang: I will check soon
12:37:58 sean-k-mooney gibi: ok the image/flavor represnetiaotn is out of scope of the placment spec anyway
12:38:10 sean-k-mooney so i think you have convinced me that this can work
12:38:41 gibi yes, but you had a point, if the flavor have had required=FOO syntax then we would be in deep trouble
12:38:57 sean-k-mooney ya i forgot we reversted it
12:38:57 gibi so it was a good excersize to see how flavor will fit
12:39:19 sean-k-mooney and didn trait:*=required
12:40:33 sean-k-mooney ill update my review with this conversation and change to a +1
12:41:54 gibi sean-k-mooney: thanks
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

Earlier   Later