Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-12-13
09:42:23 gibi bauzas, sean-k-mooney: this is my last week before vacation and I will not be back until M2 so I'd like to get over with the placement spec https://review.opendev.org/q/topic:any-traits-support this week.
09:42:42 bauzas gibi: ack good point
09:42:46 gibi it was already approved a long time ago so I don't think it is controversioal
09:42:52 bauzas and reminder that we have our spec review day tomorrow
09:43:03 gibi yepp, tomorrow is good for me
09:43:04 gibi :)
09:50:36 gibi as a trade I'm willing to look at many reviews this week as I don't want to start any new on my side just closing down open things
09:50:41 gibi so hit me with reviews
11:03:45 gibi sean-k-mooney: I'm +2 on the recent version of the instance action softdelete spec https://review.opendev.org/c/openstack/nova-specs/+/821387
11:11:25 gibi bauzas: and easy stable/xena backport needing a second core (lyarwood is already on PTO) https://review.opendev.org/q/topic:%2522bug/1945310%2522+branch:stable/xena
11:17:48 bauzas gibi: done
11:18:14 bauzas gibi: I'm just reviewing pslestang's spec already :)
11:50:07 sean-k-mooney gibi: just reading back
11:50:33 sean-k-mooney ill look at the placement one shortly
11:51:27 sean-k-mooney gibi: i was pretty happy with the instance action spec so ill look at that again this morning since its been revised
11:52:57 sean-k-mooney gibi: by the way i dong thave +2 rights on placemt/placemsnt spec directory since its in tree but ill review it anyway
11:56:12 sean-k-mooney gibi: oh this is the spect that like yuou do required=in:TRAIT1,TRAIT2
11:57:55 opendevreview Wenping Song proposed openstack/nova master: Fill the AcceleratorRequestBindingFailed exception msg info https://review.opendev.org/c/openstack/nova/+/817326
12:07:30 gibi sean-k-mooney: yes, that is in :TRAIT1,TRAIT2
12:07:58 sean-k-mooney gibi: i have one open qustion/suggestion for that spec
12:09:52 sean-k-mooney gibi: https://review.opendev.org/c/openstack/placement/+/649992/5/doc/source/specs/yoga/approved/2005346-any-traits-in-allocation_candidates-query.rst#68
12:10:59 sean-k-mooney oh i see the last spec is for mixing which is what i was asking us to support
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

Earlier   Later