Tuesday, 2020-07-28

« Monday, 2020-07-27 Index Wednesday, 2020-07-29 »
*** zhanglong has joined #openstack-nova00:13
*** hamalq has quit IRC00:41
*** gmann has quit IRC00:51
*** gmann has joined #openstack-nova00:51
*** CeeMac has quit IRC00:52
*** pas-ha has quit IRC00:53
*** CeeMac has joined #openstack-nova00:55
*** pas-ha has joined #openstack-nova00:55
*** zhanglong has quit IRC00:59
*** zhanglong has joined #openstack-nova00:59
*** ociuhandu has joined #openstack-nova01:01
*** ociuhandu has quit IRC01:05
*** tetsuro has joined #openstack-nova01:13
openstackgerritBrin Zhang proposed openstack/nova master: Optimize _create_and_bind_arqs logic in conducor  https://review.opendev.org/72656401:26
*** mkrai has joined #openstack-nova01:44
*** yaawang has quit IRC01:45
*** yaawang has joined #openstack-nova01:46
*** zhanglong has quit IRC02:18
*** zhanglong has joined #openstack-nova02:19
*** sapd1 has joined #openstack-nova02:33
*** tetsuro has quit IRC02:45
*** ircuser-1 has joined #openstack-nova02:46
*** nweinber has quit IRC02:46
openstackgerritTony Su proposed openstack/nova master: Provider Config File: YAML file loading and schema validation  https://review.opendev.org/67334102:48
*** rcernin has quit IRC02:59
*** hongbin has joined #openstack-nova03:00
*** rcernin has joined #openstack-nova03:03
*** nweinber has joined #openstack-nova03:12
*** nweinber has quit IRC03:15
*** tetsuro has joined #openstack-nova03:51
*** tetsuro has quit IRC04:01
*** tetsuro has joined #openstack-nova04:02
*** tetsuro has quit IRC04:06
*** gyee has quit IRC04:10
*** hongbin has quit IRC04:20
*** vishalmanchanda has joined #openstack-nova04:22
*** evrardjp has quit IRC04:33
*** evrardjp has joined #openstack-nova04:33
*** ratailor has joined #openstack-nova04:37
*** ociuhandu has joined #openstack-nova05:03
*** sapd1 has quit IRC05:04
*** ociuhandu has quit IRC05:07
*** mtreinish has quit IRC05:08
*** udesale has joined #openstack-nova05:27
openstackgerritBrin Zhang proposed openstack/nova master: Optimize _create_and_bind_arqs logic in conducor  https://review.opendev.org/72656405:36
*** sapd1 has joined #openstack-nova05:47
*** zhanglong has quit IRC05:49
*** mtreinish has joined #openstack-nova05:54
*** bhagyashris|away is now known as bhagyashris06:01
*** lpetrut has joined #openstack-nova06:04
*** tetsuro has joined #openstack-nova06:06
*** jsuchome has joined #openstack-nova06:08
*** maciejjozefczyk has joined #openstack-nova06:10
*** tetsuro has quit IRC06:11
*** sapd1 has quit IRC06:28
*** kevinz has joined #openstack-nova06:28
*** links has joined #openstack-nova06:30
*** mkrai has quit IRC06:43
*** slaweq has joined #openstack-nova06:48
*** nightmare_unreal has joined #openstack-nova06:56
*** tesseract has joined #openstack-nova07:11
*** dklyle has quit IRC07:19
*** sapd1 has joined #openstack-nova07:22
*** rcernin has quit IRC07:32
*** tosky has joined #openstack-nova07:42
*** ociuhandu has joined #openstack-nova07:52
*** ociuhandu has quit IRC07:52
*** ociuhandu has joined #openstack-nova07:53
*** ralonsoh has joined #openstack-nova07:53
*** brinzhang0 has joined #openstack-nova07:55
*** brinzhang_ has quit IRC07:58
*** bhagyashris is now known as bhagyashris|lunc08:10
*** xek has joined #openstack-nova08:13
*** brinzhang_ has joined #openstack-nova08:13
*** brinzhang0 has quit IRC08:16
*** derekh has joined #openstack-nova08:41
*** rcernin has joined #openstack-nova08:48
*** priteau has joined #openstack-nova08:50
*** ociuhandu_ has joined #openstack-nova08:53
*** rcernin has quit IRC08:54
openstackgerritBrin Zhang proposed openstack/nova master: Add instance project_id for cyborg arq  https://review.opendev.org/73842808:54
*** ociuhandu has quit IRC08:56
*** tetsuro has joined #openstack-nova08:58
*** awalender has joined #openstack-nova09:02
awalenderHi there, using ubuntu stein packages for OpenStack and recently we are having troubles with randomly aborting live-migrations with libvirt and qemu. Target hosts periodacly shows: Timed out during operation: cannot acquire state change lock (held by monitor=remoteDispatchDomainMigratePrepare3Params)09:06
awalenderFollowed by:  internal error: Missing monitor reply object.....afterwards libvirtd crashes with SEGV. Both hosts have same packages (libvirtd: 5.0.0-1). Anyone had similiar issues with this?09:07
*** jangutter has joined #openstack-nova09:10
*** dtantsur|afk is now known as dtantsur09:12
*** jangutter_ has quit IRC09:14
openstackgerritTony Su proposed openstack/nova master: Provider Config File: YAML file loading and schema validation  https://review.opendev.org/67334109:18
lyarwoodstephenfin: https://review.opendev.org/#/q/topic:bug/1889108 - would you mind taking a swing at this today?09:19
tony_sustephenfin: alex_xu: I unified the exception message coding style, hopefully for readibiliy, and updated patch according to other comments.09:21
*** ociuhandu_ has quit IRC09:25
brinzhang_gibi, sean-k-mooney: did you see this patch https://review.opendev.org/#/c/7384? Adding project_id to the binding API, when server call _create_and_and_arq that we can verify user permissions,09:29
brinzhang_gibi, sean-k-mooney: we will introduce it to cyborg too, see https://review.opendev.org/#/c/738427/09:30
*** ociuhandu has joined #openstack-nova09:31
*** ociuhandu has quit IRC09:31
brinzhang_if you are free, please review these patch, the cyborg patch depends-on the nova patch, otherwise the create_server test in cyborg cannot pass.09:31
*** ociuhandu has joined #openstack-nova09:31
*** tetsuro has quit IRC09:32
*** brinzhang0 has joined #openstack-nova09:35
stephenfinbrinzhang0: gibi is on PTO for the next two weeks09:36
stephenfinjust FYI09:36
brinzhang0stephenfin: thanks, got it~09:37
brinzhang0stephenfin: hope you can review that patch too, it just add an field we request accelerator with boot instance09:38
*** brinzhang_ has quit IRC09:38
openstackgerritLee Yarwood proposed openstack/nova master: Add regression test for bug #1889108  https://review.opendev.org/74328909:39
openstackbug 1889108 in OpenStack Compute (nova) "failures during driver.pre_live_migration remove source attachments during rollback" [High,In progress] https://launchpad.net/bugs/1889108 - Assigned to Lee Yarwood (lyarwood)09:39
openstackgerritLee Yarwood proposed openstack/nova master: compute: Don't delete the original attachment during live migration rollback  https://review.opendev.org/74331909:39
*** tetsuro has joined #openstack-nova09:40
openstackgerritLee Yarwood proposed openstack/nova stable/queens: ironic: add instance_uuid before any other spawn activity  https://review.opendev.org/74349309:40
*** bhagyashris|lunc is now known as bhagyashris09:41
*** Luzi has joined #openstack-nova09:45
*** tetsuro has quit IRC09:48
stephenfintony_su: Reviewed again :) Let me know if anything doesn't make sense09:50
*** tetsuro has joined #openstack-nova09:51
*** tetsuro has quit IRC09:53
*** k_mouza has joined #openstack-nova09:57
*** udesale_ has joined #openstack-nova09:58
*** udesale has quit IRC10:01
*** brinzhang_ has joined #openstack-nova10:02
sean-k-mooneybrinzhang0: let me take a look but you should not need to add project_id to the api itsself it will be valiable in the keystone atuth token so i need to see what you have acatully done10:05
*** brinzhang0 has quit IRC10:05
sean-k-mooneybrinzhang_: ^10:05
sean-k-mooneybrinzhang_: my initall feeling is that you should not really need the project id10:07
brinzhang_sean-k-mooney: Cyborg is about to introduce a new policy, I think this is needed.10:13
sean-k-mooneybrinzhang_: is cyborg currently using api microversions10:13
brinzhang_https://review.opendev.org/#/q/project:openstack/cyborg+branch:master+topic:policy-popup10:13
sean-k-mooneybecause it need to continue to supprot old novas that dont send it10:14
sean-k-mooneymeaning they need to either do a major api verion bump and maintian both10:14
sean-k-mooneyor the need to add a microversions10:14
sean-k-mooneyor treat it as optional in someother way so that older novas can work with it10:14
brinzhang_Yes, this will change the cyborg api, we need a new microversion to support project_id in cyborg arq api10:16
sean-k-mooneyi dont see that in https://review.opendev.org/#/c/738427/210:16
sean-k-mooneyin factr i dont see any microversion code in cyborg10:17
sean-k-mooneyi have not looked very hard10:17
brinzhang_I will ask him to add microversion, we have introduced microversion in v2 API, this must be done.10:17
sean-k-mooneyit looks like it will be the first microversion https://github.com/openstack/cyborg/blob/369abe8dd06aa6648298c3256f444a63ee6268d0/cyborg/api/controllers/v2/api_version_request.py#L4910:19
*** rcernin has joined #openstack-nova10:19
sean-k-mooneyis this the first api change since v210:20
brinzhang_I would like this patch can add the microversion too https://review.opendev.org/#/c/698190/10:20
brinzhang_I am trying my best..10:20
sean-k-mooneybrinzhang_: i know10:21
sean-k-mooneybrinzhang_: its not just your responsiblity10:21
sean-k-mooneyall cyborg cores shoudl be reviewing for this10:21
sean-k-mooneyeven non cores10:21
brinzhang_sean-k-mooney: yes, thanks10:21
brinzhang_hope you can leave comment inline too10:21
brinzhang_In contrast, you have more experience in this area, which is worth learning from.10:22
stephenfinlyarwood: Reviewed those, btw10:22
sean-k-mooneyyes i can i was hoping to point them to a patch that previously raised the microversion so that they had an example but since they are the first i guess i cant10:22
sean-k-mooneywell nova does i try to avoid making api change when i can. its a lot less work if you dont have to modify the api to implemente a feature10:23
sean-k-mooney:)10:23
sean-k-mooneyill leave a comment on both patches shortly10:24
brinzhang_sean-k-mooney: yeah, I can understand, but the current two patches inevitably need to modify the API.10:24
brinzhang_Thanks, so that I can ask again^^10:25
sean-k-mooneybrinzhang_: basically we need to copy https://docs.openstack.org/nova/latest/contributor/microversions.html into the cyborg docs and do a find and replace10:31
sean-k-mooneybrinzhang_: gmann and alex_xu are two of the experts on api microversioning10:32
brinzhang_sean-k-mooney: thanks, I will review this, and do a copy later10:32
brinzhang_yeah, if I have some problem, Iill ask them for help10:33
lyarwoodstephenfin: ta10:37
*** rcernin has quit IRC10:43
*** xek has quit IRC10:49
stephenfinsean-k-mooney: Left comments on https://review.opendev.org/#/c/739131/. I have those type hints worked out locally if you want me to push them somewhere?10:56
sean-k-mooneyoh i just fixed the ones you commented on before i proablyshould have checked all the types10:59
sean-k-mooneyon the styple nit putting each paramater on its onw line is something im happy to fight you over10:59
sean-k-mooneyi hate that more then anything else10:59
sean-k-mooneywasting vertical space like that is deeply wrong in my view.11:00
*** brinzhang_ has quit IRC11:00
sean-k-mooneyim assumeing that is what you want here https://review.opendev.org/#/c/739131/8/nova/virt/libvirt/driver.py@689411:01
*** bhagyashris is now known as bhagyashris|away11:06
*** sapd1 has quit IRC11:07
*** k_mouza has quit IRC11:13
*** mgariepy has quit IRC11:18
*** k_mouza has joined #openstack-nova11:19
*** k_mouza has quit IRC11:19
*** k_mouza has joined #openstack-nova11:20
stephenfinsean-k-mooney: I think putting it on its own line helps visually group the function definition stuff neatly together. It also allows for maximum width wrt annotating the return type (which can often be quite lengthy, as you're seeing)11:22
sean-k-mooneyright but i think your thinking about type hints wrong11:23
sean-k-mooneywe should not in all cases fully qualify the type11:23
sean-k-mooneythat is not how they are ment to be used11:23
stephenfinwhy ever not?11:23
sean-k-mooneyhave you ever used c++ templates11:23
sean-k-mooneyand tried to fully quallify a template that multipel template args11:24
stephenfinno, I've barely touched C++11:24
sean-k-mooneytype hints are most useful when they specify the interface of the type you expect11:24
*** xek has joined #openstack-nova11:24
stephenfinright11:24
sean-k-mooneyyou can be more specific too but our brains are not compilers so adding too much detail can be harmful11:25
sean-k-mooneyif you add too much detail to the type trait and you have multiple parmater it gets hard to keep all that context in your brain11:25
stephenfinthe point of the type hints is offload the context though11:26
sean-k-mooneybut you still need to read them11:26
stephenfinyou don't need to figure out what param_a is: it's written right there11:26
sean-k-mooneyyes we agree on this11:26
sean-k-mooneybut is ty.Dict[str,ty.Union[str, ty.Dict[str, ty.Union[str, ty.List[str], None]]]]11:27
sean-k-mooneyuseful?11:27
sean-k-mooneyvs ty.Dict[str,ty.Union[str, ty.Dict]]11:27
stephenfinNo, not at all. That should be a typed dict11:27
*** whoami-rajat has joined #openstack-nova11:27
stephenfinyou'll note in most places we have that I've just used ty.Dict[str, ty.Any]11:27
stephenfinso we're on the same page there11:28
stephenfinhowever, 'list' also isn't useful11:28
sean-k-mooneywell that is actully the fully correct type11:28
stephenfinUnless it's truly a generic container for anything, then I want to know what's in it11:28
sean-k-mooneyits technically ty.Dict[str,ty.Union[str, ty.Dict[str, ty.Union[str, ty.List[str], None]]]]11:28
stephenfinI don't think I've written anything approaching that level of detail anywhere11:29
sean-k-mooneyty.Dict is an alis for dict which is treated as ty.Dict[Any, Any]11:29
sean-k-mooneyyou havent11:29
whoami-rajathi #openstack-nova , is there a way to use pdb in n-cpu service (DEVSTACK), i tried starting the service manually on terminal but it doesn't stop on the pdb11:29
stephenfinand I'm not suggesting we do that here either11:29
sean-k-mooneybut that is what i would hav too change https://review.opendev.org/#/c/739131/8/nova/virt/libvirt/driver.py@6909 too11:30
sean-k-mooneywhoami-rajat: that is because we use eventlets11:30
stephenfinnah, I didn't say that. I just said what was there was wrong. 'ty.Dict[str, ty.Any]' would work perfectly fine11:31
sean-k-mooneyto use it with pdb you have to disable monkeypatching of the threading api11:31
stephenfinor ty.Dict, if you prefer11:31
sean-k-mooneyright so i have been using dict which is the same as ty.Dict which is ty.Dict[ty.Any, ty.Any]11:32
whoami-rajatsean-k-mooney, ah, does disabling it has any side effect?11:32
*** jangutter_ has joined #openstack-nova11:32
sean-k-mooneywhoami-rajat: unfortunetly yes, we actullly have a couple of infinet loops11:32
sean-k-mooneywe normally only exit them via yeilding11:32
sean-k-mooneyso in practic it is quite hard to debug n-cpu directly11:32
whoami-rajat:( , is rpdb a solution here? sean-k-mooney11:33
sean-k-mooneyi have never used that11:33
stephenfinsean-k-mooney: Okay, then I retract my request not to use 'dict' in that case then11:33
whoami-rajatack. thanks for the info sean-k-mooney11:34
stephenfinI'd still rather we did use 'ty.Dict[str, ty.Any]', since that's at least a little more information than we might otherwise have11:34
stephenfinbut it's not necessary11:34
*** k_mouza has quit IRC11:34
sean-k-mooneyoh am well we can run with remote debuging but basically you have to deal with either eventlet monkey patching which means breakpoint might not work or disable monkey patch  whcih means you might not get to the code you want11:35
stephenfinI think my comments on the other places that 'ty.Any' is still used are valid though, since they're easy to type and actually helpful11:35
sean-k-mooneystephenfin: ill go make most of the change you asked for11:35
*** jangutter has quit IRC11:35
sean-k-mooneyand then when the type get a bit too long11:35
sean-k-mooneyill do type erasure11:35
sean-k-mooneyi can do ty.Dict[str, ty.Any]11:36
stephenfinsounds good to me11:36
*** bhagyashris|away is now known as bhagyashris11:36
sean-k-mooneyto me by the way needing to do ty.Dict[str, ty.Any] is kind fo a code smell11:36
sean-k-mooneyeither we should be returning classes rather then dicts11:37
sean-k-mooneyor we shoudl be breaking down the function as it likely has too many differnt posable return types11:37
stephenfintotally agree11:37
sean-k-mooneyam other then typing and the comment you left11:38
sean-k-mooneyare you happy with the change other then that11:38
sean-k-mooneyi am hoping this will be the last revision so i can move on to the numa in placemnt stuff for a while11:38
stephenfinI do think we could fold some of the functions into each other, since the separation feels a bit artificial rn, but I've said as much in the review and it's easy fix later11:39
stephenfinso yeah, lgtm otherwise11:39
sean-k-mooneyok im also planning to backport this at least to train so i was trying to keep this relitivly small11:40
* sean-k-mooney failed at that11:40
stephenfinMaybe drop the type hints or put them in a separate patch in that case? :)11:40
sean-k-mooneywell we had this debate a few versions ago11:41
sean-k-mooneythat we did not want to have to write worse code with out type hint jsut because we wanted to backport11:41
stephenfinfair point11:41
*** brinzhang has joined #openstack-nova11:42
sean-k-mooneyso im hesitent to set that precident as if i do ill be asked to do it for every bugfix11:42
sean-k-mooneyat which point its much less motivating to ever write type hints and i used them while writing the patch a few times so they are really useful IMO11:43
sean-k-mooneye.g. i forgot what i was passing an looked at the type hint11:43
*** lbragstad has quit IRC11:45
*** k_mouza has joined #openstack-nova11:45
*** ratailor has quit IRC11:45
*** spatel has joined #openstack-nova11:50
*** k_mouza has quit IRC11:53
*** spatel has quit IRC11:54
*** priteau has quit IRC11:55
sean-k-mooneystephenfin: by the way im not going to do it in this patch but what are your feeling on doing ty.Dict vs importing Dict directly and just using it11:56
sean-k-mooneystephenfin: you seamed to be doing ty.<whatever>11:56
sean-k-mooneyso that is what im doing but the ty. gets a little tedious after a while11:56
*** raildo has joined #openstack-nova11:56
*** priteau has joined #openstack-nova11:57
sean-k-mooneymost of the examples i have seen do "from typeing import Dict,List,..."11:57
sean-k-mooneylong term do you think we should stick with import typeing as ty or move over to the other from typing style11:58
*** evrardjp has quit IRC12:00
*** tkajinam has quit IRC12:02
*** k_mouza has joined #openstack-nova12:19
*** sapd1 has joined #openstack-nova12:20
*** k_mouza has quit IRC12:24
*** mgariepy has joined #openstack-nova12:33
*** ralonsoh has quit IRC12:34
*** ralonsoh has joined #openstack-nova12:34
*** k_mouza has joined #openstack-nova12:37
*** k_mouza has quit IRC12:41
lyarwoodstephenfin: so I've actually managed to hit the same issue with live_migration as pre_live_migration in my func tests btw, looks like I also need the fix to cover this case12:46
lyarwoodoh wait I borked the test12:48
lyarwoodOS_DEBUG++12:48
*** xek_ has joined #openstack-nova12:48
*** lbragstad has joined #openstack-nova12:50
*** xek has quit IRC12:51
sean-k-mooneyOS_DEBUG is for what disabling monkeypatching or extra output12:53
*** sapd1 has quit IRC12:53
sean-k-mooneyits the latter right?12:53
sean-k-mooneywhen running test with tox12:54
lyarwoodlatter, had a typo in a mock and was confused until I saw the extra output12:54
sean-k-mooneyya i have used it once or twice12:54
sean-k-mooneyi normally got to pdb instead12:54
*** ociuhandu has quit IRC12:57
*** brinzhang_ has joined #openstack-nova12:57
*** ociuhandu has joined #openstack-nova12:57
*** whoami-rajat has quit IRC12:57
*** brinzhang has quit IRC12:57
*** k_mouza has joined #openstack-nova12:58
*** rcernin has joined #openstack-nova12:59
jsuchomelyarwood: https://review.opendev.org/#/c/743220/ is green, hooray!13:00
*** k_mouza has quit IRC13:02
*** rcernin has quit IRC13:03
*** brinzhang0 has joined #openstack-nova13:10
*** k_mouza has joined #openstack-nova13:11
*** brinzhang_ has quit IRC13:12
*** nweinber has joined #openstack-nova13:15
*** k_mouza has quit IRC13:16
openstackgerritLee Yarwood proposed openstack/nova master: Add regression tests for bug #1889108  https://review.opendev.org/74328913:22
openstackbug 1889108 in OpenStack Compute (nova) "failures during driver.pre_live_migration remove source attachments during rollback" [High,In progress] https://launchpad.net/bugs/1889108 - Assigned to Lee Yarwood (lyarwood)13:22
openstackgerritLee Yarwood proposed openstack/nova master: compute: Don't delete the original attachment during pre LM rollback  https://review.opendev.org/74331913:22
openstackgerritLee Yarwood proposed openstack/nova master: func: Add live migration rollback volume attachment tests  https://review.opendev.org/74353413:22
openstackgerritLee Yarwood proposed openstack/nova master: func: Add CinderFixture to _IntegratedTestBase  https://review.opendev.org/74353513:22
*** k_mouza has joined #openstack-nova13:23
lyarwoodjsuchome: ack, I'll sort the actual test for that out later this eveningn13:23
*** k_mouza has quit IRC13:27
jsuchomethanks13:32
*** whoami-rajat has joined #openstack-nova13:38
*** jangutter_ is now known as jangutter13:39
*** Luzi has quit IRC13:40
openstackgerritAlexandre Arents proposed openstack/nova master: Make _rebase_with_qemu_img() generic  https://review.opendev.org/74353713:41
*** brinzhang_ has joined #openstack-nova13:52
*** brinzhang0 has quit IRC13:55
*** k_mouza has joined #openstack-nova13:59
*** maciejjozefczyk has quit IRC14:03
*** k_mouza has quit IRC14:03
*** k_mouza has joined #openstack-nova14:04
*** mlavalle has joined #openstack-nova14:04
*** sapd1 has joined #openstack-nova14:12
*** brinzhang_ has quit IRC14:13
*** brinzhang_ has joined #openstack-nova14:14
*** mriedem has joined #openstack-nova14:20
*** brinzhang0 has joined #openstack-nova14:23
*** brinzhang_ has quit IRC14:26
*** lpetrut has quit IRC14:40
*** dklyle has joined #openstack-nova14:41
*** maciejjozefczyk has joined #openstack-nova14:46
gmannbrinzhang0: let me know if you need help to introduce microverison in cyborg there are common code we can share from nova (copy for now but we should have it somewhere in common lib like oslo etc)14:52
*** brinzhang_ has joined #openstack-nova15:09
*** brinzhang0 has quit IRC15:12
openstackgerritGhanshyam Mann proposed openstack/nova master: Add new default roles in volumes policies  https://review.opendev.org/74277715:12
gmannstephenfin: thanks for all review on policy patches. ^^ updated one. rest nit i will fix if i need to re-spin let me know if anything you want me to fix in same commit.15:16
*** brinzhang_ has quit IRC15:19
openstackgerritGhanshyam Mann proposed openstack/nova master: Add scope and new default roles in extensions policies  https://review.opendev.org/74304615:22
*** maciejjozefczyk has quit IRC15:27
*** links has quit IRC15:34
stephenfingmann: ack15:35
*** udesale_ has quit IRC15:40
kashyapstephenfin: Now I know why you deferred that live migration options doc :D the sheer amount of scouring through the code to get the "web of relations" :D15:41
kashyapMore seriously, I've been looking through history and code reading to figure out WTH is going on15:42
kashyapEither way, 'fun'15:42
openstackgerritAlexandre Arents proposed openstack/nova master: Rebase qcow2 images when unshelving an instance  https://review.opendev.org/69608415:43
stephenfinkashyap: I assume that means you've left something somewhere that I should go read? :)15:43
kashyapstephenfin: Not yet; just writing an additional comment, besides what I wrote yesterday15:44
kashyapstephenfin: I might need a better phrasing of the existing one from the author.  It took me a few careful readings to parse it :D15:44
stephenfinAha, gotcha15:45
stephenfinLooking forward to it :)15:45
openstackgerritStephen Finucane proposed openstack/nova master: Handle multiple 'vcpusched' elements during live migrate  https://review.opendev.org/74356815:45
stephenfinartom: When you've a chance, could you sanity check that for me?15:46
stephenfinI'll poke bauzas or lyarwood to review later15:47
artomstephenfin, for sure15:47
stephenfinta15:48
artomstephenfin, also, aren't you glad for those LOG.debug calls with the full XML dump? :)15:48
stephenfinfor sure15:49
stephenfinlogging ftw15:49
kashyapstephenfin: Here, right now I don't have a better phrasing; you might say it's not even required, let's see -- https://review.opendev.org/#/c/741473/1/nova/conf/libvirt.py@26115:50
openstackgerritGhanshyam Mann proposed openstack/nova master: Pass the actual target in volumes policy  https://review.opendev.org/74277915:54
artomstephenfin, seems sane, left a note15:57
*** noonedeadpunk has joined #openstack-nova16:00
stephenfinartom: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/config.py#L66-L6716:00
stephenfinhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/config.py#L53-L5916:00
stephenfinthe name is poor, admittedly16:00
*** k_mouza has quit IRC16:02
*** mgariepy has quit IRC16:04
*** xek_ has quit IRC16:06
*** priteau has quit IRC16:08
artomstephenfin, doh :( A ctrl-] would not have been that hard, on my part16:14
sean-k-mooneywhat does ctrl-] do16:15
sean-k-mooneya find?16:15
sean-k-mooneygot do definition16:15
artomGo to definition, in vim with ctags16:15
sean-k-mooneyah ok16:15
artomAdmittedly, it sometimes messes up if there are multiple methods called the same thing16:16
openstackgerritStephen Finucane proposed openstack/nova master: Track CPU scheduler policy during live migration  https://review.opendev.org/74358816:22
*** maciejjozefczyk has joined #openstack-nova16:22
stephenfinartom: follow-up there, if you're feeling generous16:22
*** k_mouza has joined #openstack-nova16:23
artomstephenfin, so... does it really make sense to do this, given we only support 1 policy now?16:25
stephenfinMaybe. Maybe not. Given how trivial it is to do, I'd rather knock it off now than have to worry about it when we really do need it16:27
stephenfinat that point we'd be down to needing service version checks, or whatever the new hotness is by then16:27
*** k_mouza has quit IRC16:28
artomstephenfin, so... this would be for a case where the scheduler is set on a per-host basis?16:29
artomAnd we want to support live migration between different hosts/schedulers?16:29
artomWouldn't we do it as an image property/flavor extra spec?16:29
sean-k-mooneystephenfin: given we dont support other schduler polcies and dont plan to intoduce that in the near term do we really need this cange16:30
stephenfinif we did have different scheduler policies they'd likely be done via an image prop/extra spec, yes16:30
sean-k-mooneystephenfin: right but we dont plan on allowing user to request that16:30
artomstephenfin, so if it's set in the flavor, do we need this change?16:31
artomAs you can see, I'm having trouble being convinced :)16:31
*** k_mouza has joined #openstack-nova16:31
sean-k-mooneyif it was in the flavor or image no16:31
sean-k-mooneywe woudl only need it if it was set by a host level config option16:31
artomsean-k-mooney, yep, and I think you and I would be against such a thing16:32
stephenfinartom: I'm confused. What would change?16:32
artomPrecisely because it would require stephenfin's patch :P16:32
*** dtantsur is now known as dtantsur|afk16:32
artomstephenfin, I'm saying - if the scheduler is set via flavor extra spec, we don't need to update it as part of the live migration XML update16:32
sean-k-mooneystephenfin: i guess the issue is right now i dont think your change adds anything useful16:33
artomBecause the destination host should be able to accept the existing scheduler16:33
artom(Presumably because we've scheduled to it because of a trait or something)16:33
sean-k-mooneywe have no plans to expose sched_scheduler to the user or operator as something that can be set in anyway16:33
stephenfinartom: oh, perhaps the difference here is that I'm saying we shouldn't be _updating_ the XML elements16:33
stephenfinwe should be regenerating them based on authoritative sources that tell us what they should be set to16:34
stephenfinI figured the value of doing this is to provide such an authoritative source for what the scheduler policy should be16:34
sean-k-mooneystephenfin: this value will always be fifo if realtime is enabled16:34
sean-k-mooneyotherwise it will be unset16:34
sean-k-mooneyso we shoudl not need any addtion info in the migrate_data16:34
artomAnd if we ever want to support something other than fifo, it won't change during a live migration16:34
stephenfinright, at the moment that's the case16:35
artomOnly during a rebuild with a different image16:35
stephenfinartom: okay, but if it doesn't change how do I get that information16:35
artomstephenfin, you don't need to? Keep the existing one16:35
stephenfinI'm not keeping the original elements though16:35
artomOh, *facepalm*16:35
artomI get it now16:35
sean-k-mooneystephenfin: if hw:cpu_realtime=true or hw_cpu_realtime=true its fifo]16:35
artomYou're clobbering first16:36
stephenfinI'm throwing them away and recalculating them16:36
*** k_mouza has quit IRC16:36
stephenfinyessss16:36
artomSo... don't clobber them :)16:36
*** maciejjozefczyk has quit IRC16:36
stephenfinand parsing the existing XML feels gross16:36
artomOr stash the stuff we know won't chang16:36
artom*change16:36
stephenfinthat's what I thought I was doing16:36
stephenfinstashing it in the migrate_data object16:36
openstackgerritTakashi Natsume proposed openstack/python-novaclient master: Add a cleanup for a server in a functional test  https://review.opendev.org/74358916:36
sean-k-mooneystephenfin: you dont need to stash this16:36
stephenfini.e. providing an authoritative source16:36
artomIsn't that overkill?16:37
sean-k-mooneyyou can compute it form the flavor and image16:37
stephenfinsean-k-mooney: Yes, that's exactly what I do16:37
stephenfinon the source side16:37
stephenfinthen I stash it in the migrate data16:37
*** k_mouza has joined #openstack-nova16:37
stephenfin:)16:37
sean-k-mooneyby they would you do that16:37
sean-k-mooneyyou can comptue it form the falvor and image anyhwere its needed16:37
artomYou're proposing we add a a field to an object that does, like, 42 transits over the wire, instead of parsing some XML in a single line?16:38
stephenfinyes16:38
artomOK :)16:38
artomI disagree :)16:38
sean-k-mooneyme too16:38
stephenfinI'm saying our flavor and image metadata code is authoritative and the only thing we can trust16:39
sean-k-mooneyalso you dont need to parse16:39
*** tesseract has quit IRC16:39
stephenfinas evidenced by the fact this bug exists16:39
sean-k-mooneywhat bug16:39
stephenfinlibvirt can do anything to our XML once we pass it off16:39
artomstephenfin, true, but we can still have some expectations16:39
sean-k-mooneyoh we are generating  <vcpusched vcpus="2-3" scheduler="fifo" priority="1"/>16:40
sean-k-mooney      <vcpusched vcpus="3" scheduler="fifo" priority="1"/>16:40
stephenfinwe should be trying to avoid introspecting that XML to try guess what was done previously16:40
stephenfinsean-k-mooney: yup16:40
sean-k-mooneyya you dont need to introspect the xml16:40
sean-k-mooneyor pass any data16:40
sean-k-mooneyyou jst need to check the flavor and image extraspecs16:40
stephenfinthen I need to hardcode the policy16:40
sean-k-mooneyyes16:40
sean-k-mooneyits hardcoded today based on if you enable realtime16:40
artomstephenfin, I mean, you're already "parsing" the XML when you're removing the existing vcpusched elements16:41
artomUse that to stash the scheduler16:41
*** maciejjozefczyk has joined #openstack-nova16:41
stephenfinsean-k-mooney: right, in a function that's miles away16:41
artomsean-k-mooney, I don't think we have the flavor/image in libvirt/migration.py, there would be plumbing required16:41
sean-k-mooneywe do16:41
sean-k-mooneyif we have the instance we have the flavor and image16:42
*** k_mouza has quit IRC16:42
stephenfinartom: I'm not parsing it though. I'm finding it and totally replacing it with my own freshly baked XML16:42
sean-k-mooneystephenfin: if you plan to backprot this bug fix16:42
sean-k-mooneyyou can change the object16:42
artomsean-k-mooney, I don't think we have the instance either16:42
sean-k-mooneyi think we woudl want to backport it to train16:42
stephenfinsean-k-mooney: that's why it's a separate change  :)16:42
*** k_mouza has joined #openstack-nova16:42
sean-k-mooneyso form my point of vew we cant add the new fiedl16:42
artomstephenfin, agree to disagree I guess? We're obviously doing a poor job of convincing the other :P16:43
*** vinay_m has joined #openstack-nova16:43
stephenfinI've hardcoded it for expediency, but I think it's a bad idea to be doing that16:43
artomWhoever the cores are that review that can make the call16:43
sean-k-mooneystephenfin: why im asserting we actvly never want to make the schduler policy configurable16:43
stephenfinartom: it would appear so. Guess I best go request cookies be sent to gibi in advance of his return ;)16:43
sean-k-mooneyits not a case of we dont expose this to day and might want to in the futre it we dont support it today and dont want to in the future16:44
artomstephenfin, lemme try another angle. If we push your logic, we end up regenerating the *whole* XML upon live migration, including potentially shuffling PCI addresses16:45
*** ociuhandu has quit IRC16:45
sean-k-mooneywell we cant do that16:45
artomsean-k-mooney, my point exactly :)16:45
artomSo we have to admit that *some* of the existing XML is authoritative in a live migraiton16:45
sean-k-mooneyregenrating all the xml is out of the question for a live migration16:45
artomSo why not the realtim scheduler?16:46
sean-k-mooneyregenreat the cputune element is a differnt matter16:46
*** hamalq has joined #openstack-nova16:46
artom(Btw, Real Tim is my MC name)16:46
stephenfinWe can't do it yet16:47
stephenfinIf we properly tracked everything, we could totally do that16:47
stephenfinwhich would require adding more stuff to migrate data16:47
stephenfinlike I'm doing here16:47
*** k_mouza has quit IRC16:47
sean-k-mooneyyes but i dont think we should do that16:47
sean-k-mooneythis is not something that should be change in a bug16:48
*** mlavalle has quit IRC16:48
sean-k-mooneyits a deeper desigin choice that need to be consierded carfully16:48
stephenfinI'm not proposing that. Just saying it's something we should aspire to16:48
artomstephenfin, so we're back to the whole "Is XML canonical" debate?16:49
*** vinay_m has quit IRC16:49
stephenfinand that this is a tiny step in that direction that we'd do well to take, IMO of course16:49
sean-k-mooneyartom: well its not and i dont think it should be16:49
*** mlavalle has joined #openstack-nova16:49
stephenfinartom: yes, but we've just made a brief stop on our way somewhere else16:50
artomstephenfin, so I think that's part of the issue - we first need to decide whether that's a direction we'd *all* like to take :)16:50
sean-k-mooneystephenfin: technically its a step a way form it since you are not modifying it and regenrating it16:50
stephenfinsean-k-mooney: how so?16:50
sean-k-mooneymaking the xml canonical mean not regenerating it ever16:50
artomOK, the new angle has clearly not worked :)16:50
sean-k-mooneyso that modifcatin done out of band are preserved16:50
artomI'll register my disagreement on the patch, then leave it up to the powers that be, and go feed myself and the kiddos16:51
sean-k-mooneyso regenreating the entire xml form info passed back is the opisce of makeing the xml cannonical16:51
stephenfinsean-k-mooney: oh, gotcha. Yeah, I'm on the "nova is authoritative" side of the fence16:51
sean-k-mooneysame16:51
sean-k-mooneywhich is why i would be totally happy to always regenerate the xml form nova datastuctures16:52
sean-k-mooneykind of like the migrate data but im asserting you already have the info you need with out having to pass it16:52
*** k_mouza has joined #openstack-nova16:52
sean-k-mooneynot that the approch is wrong16:52
stephenfinit really sounds like you're advocating for the same position as me16:52
stephenfinah16:52
sean-k-mooneyjsut that you should not have two places to get the info16:52
sean-k-mooneythe falvor/image and the migrate data object16:53
stephenfinso I'm for it, artom is against it, and sean-k-mooney is kind of for it but not the way I've done it16:53
stephenfinlovely :)16:53
sean-k-mooneywell 3 enginerrs your normally expect at least 4 oppinions16:53
artomstephenfin, if we end up doing a push towards "Nova is authoritative, XML can suck it" I wont' stand in anyone's way16:53
sean-k-mooney3 is light :P16:53
artomstephenfin, I just think in the current context it's premature16:54
sean-k-mooneyartom: that is the status quoe today16:54
sean-k-mooneyso the null hypotous is that nova is authritavive and xml is not16:54
artomsean-k-mooney, well, except when it isn't? Like live-migration? :)16:54
artomIn practice, I mean16:54
stephenfinOkay, I figured I had the information to hand when generating the migration data object, so it made sense to slot it in there, even if I could technically generate it on the other end16:54
sean-k-mooneynova is still athuritive16:54
sean-k-mooneyif you ever modify the xml then the vm is not supproted anymore16:55
artomsean-k-mooney, I guess? Is it really though, if it's using the existing XML to pull info?16:55
sean-k-mooneynova created the exsting xml16:55
artomHeh, true16:55
sean-k-mooneyand nothing outside of nova is allowed to modify it16:55
artomTurtles all the way down!16:55
sean-k-mooneywell bar libvirt16:55
stephenfinartom: As before. Maybe. Maybe not. It seemed fairly trivial, consistent and would always be correct, even it was slightly overengineered16:55
artomstephenfin, all true. My beef is about the last part.16:56
sean-k-mooneyok so https://review.opendev.org/#/c/743568/1 is artoms backportable change16:56
stephenfinYup, got that now16:56
sean-k-mooneyand https://review.opendev.org/#/c/743588/1 is your cleanup16:56
stephenfinsean-k-mooney: nah, that's me too16:56
stephenfinAdd a TODO, resolve TODO in follow-up16:57
sean-k-mooneyok well the first is intended to be backported and then a master only followup16:57
*** k_mouza has quit IRC16:57
stephenfinyup16:57
stephenfinthat's the overengineered idea, anyway16:57
stephenfin:)16:57
sean-k-mooneyhow did we end up with 2 elements16:58
stephenfinread the bug report16:58
stephenfinlibvirt does it16:58
sean-k-mooneyreally because it looks like we shoudl be updating the xml to only have one right16:59
sean-k-mooneywith the orginal code16:59
stephenfinnope, we just update the first one we find16:59
stephenfinnot expecting there to be more16:59
sean-k-mooneyoh libvirt splits them16:59
sean-k-mooneyso  <vcpusched vcpus="2-3" scheduler="fifo" priority="1"/>17:00
*** derekh has quit IRC17:00
sean-k-mooneybecomes  <vcpusched vcpus="2" scheduler="fifo" priority="1"/>17:00
sean-k-mooney      <vcpusched vcpus="3" scheduler="fifo" priority="1"/>17:00
sean-k-mooneythen we update the first one17:00
stephenfinyup17:00
sean-k-mooneythen why can we not delete the vcpushed element and just not add the new one17:01
stephenfinthat's precisely what I'm doing17:01
*** maciejjozefczyk has quit IRC17:01
sean-k-mooneyoh that is what your doing17:02
sean-k-mooneyin https://review.opendev.org/#/c/743568/1/nova/virt/libvirt/migration.py17:02
stephenfinyuuup17:02
stephenfinloads of context in the bug report and commit message itself, and it's pretty trivial to reproduce17:02
stephenfinand with that, I'm out17:02
stephenfino/17:02
sean-k-mooneyso i would just read the policy form the element before we remove it17:03
sean-k-mooneyhere https://review.opendev.org/#/c/743568/1/nova/virt/libvirt/migration.py@13017:03
sean-k-mooneywe can even assert that its the same for all element if we want17:03
stephenfin<stephenfin> we should be trying to avoid introspecting that XML to try guess what was done previously17:03
sean-k-mooneywell yes which is why i said we shoulc caluated it form the flaovr/image17:04
sean-k-mooneybut if you dont want to do that and dont want to hardcode tehn introscpect is the only other option17:04
*** k_mouza has joined #openstack-nova17:05
sean-k-mooneywait17:05
sean-k-mooneythe check is "if 'sched_vcpus' and 'sched_priority' in info:"17:05
sean-k-mooneywhat is info['sched_priority']17:06
sean-k-mooneyits the dest numa toplogy object17:06
sean-k-mooneywhich is calulated form the flavor and image17:06
sean-k-mooneyoh but that does not have the schulder right17:06
sean-k-mooneyjust the priority17:07
*** k_mouza has quit IRC17:09
sean-k-mooneyactully no info is this https://github.com/openstack/nova/blob/master/nova/objects/migrate_data.py#L113-L13017:09
*** awalender has quit IRC17:10
*** k_mouza has joined #openstack-nova17:11
*** mgariepy has joined #openstack-nova17:14
*** k_mouza has quit IRC17:16
sean-k-mooneystephenfin: so looking at the code you would have to pass the flavor/image  down two function calls form17:20
sean-k-mooneyhttps://opendev.org/openstack/nova/src/branch/master/nova/virt/libvirt/driver.py#L893917:20
sean-k-mooneyor you could pass the instance object17:20
sean-k-mooneyi would proably pass the instance17:20
sean-k-mooneystephenfin: im +1 on the first patch and -0.5 on the second17:21
sean-k-mooneyi think changing 2 internal function calls and caluating it form the flavor/image is cleaner then changing the objects17:22
*** k_mouza has joined #openstack-nova17:28
*** k_mouza has quit IRC17:32
*** nightmare_unreal has quit IRC17:33
*** k_mouza has joined #openstack-nova17:36
*** k_mouza has quit IRC17:41
*** redrobot has quit IRC17:41
*** sapd1 has quit IRC17:42
*** k_mouza has joined #openstack-nova17:45
*** k_mouza has quit IRC17:49
*** ralonsoh has quit IRC17:49
*** ociuhandu has joined #openstack-nova17:54
*** k_mouza has joined #openstack-nova17:54
*** ociuhandu has quit IRC17:58
*** k_mouza has quit IRC17:59
*** k_mouza has joined #openstack-nova18:12
*** k_mouza has quit IRC18:16
*** k_mouza has joined #openstack-nova18:20
*** k_mouza has quit IRC18:24
*** tbachman has joined #openstack-nova18:27
*** jamesden_ has joined #openstack-nova18:27
*** k_mouza has joined #openstack-nova18:30
*** jamesdenton has quit IRC18:30
*** k_mouza has quit IRC18:39
lyarwoodstephenfin / artom ; https://review.opendev.org/#/q/topic:bug/1889108 updated btw if you have time this evening18:49
*** xek_ has joined #openstack-nova18:51
artomlyarwood, left a comment on one of them. Not sure it's -1 worthy18:55
lyarwoodartom: well the issue itself isn't tied to any microversion but pinning on anything later than stable/queens and 2.60 just introduces pointless churn in the backports18:57
artomlyarwood, so we could just remove the microversion= line altogether?18:58
lyarwoodartom: wouldn't it pin to latest then introducing churn in the backports?18:58
artomHow so?18:59
* lyarwood was sure we default to latest somewhere18:59
* lyarwood checks18:59
artomI honestly don't know, but if we do, 'latest' for queens isn't 'latest' for master...18:59
lyarwoodmicroversion = None18:59
lyarwoodfun19:00
artomlyarwood, so I went looking to remember how I did func tests for NUMA live migration19:01
artomWhich I've proposed as a U backport19:01
artomhttps://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/libvirt/test_numa_live_migration.py#L41 appears to run just fine in U19:02
artomAnd then https://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/libvirt/test_numa_live_migration.py#L387 is the "functional" reason I'm talking about it - ie, there's a need for something a specific microversion provides19:02
artomSo I would suspect 'latest' will be fine all the way down to queens19:03
lyarwoodartom: so microversion = None actually means the oldest?19:03
artomI guess?19:03
*** bbowen has quit IRC19:04
*** bbowen has joined #openstack-nova19:04
lyarwoodartom: urgh this sucks19:07
artomHow so?19:07
lyarwoodartom: leaving it set to None breaks my use of loads of the helper methods from _IntegratedTestBase19:07
lyarwoodartom: _live_migrate etc19:07
sean-k-mooneylatest will break some tests19:08
lyarwoodfeels weird writing functional tests against a version of the API we don't even support anymore tbh19:08
artomlyarwood, how does it break them?19:08
sean-k-mooneyalthough those proably would be the ones testing older microverions19:08
lyarwoodright so I've picked the max version in stable/queens19:08
artomlyarwood, my point is that microversio = 'latest' works just fine in queens19:08
sean-k-mooneyartom: what that technically means is use the latest version the nova client know about19:09
sean-k-mooneynot the latest version the api know about19:09
artomsean-k-mooney, in func tests, so no novaclient, but yeah19:09
lyarwoodartom: latest in queens != latest in master19:09
artomlyarwood, yep, so?19:09
artomI fail to see the problem :)19:09
sean-k-mooneylyarwood: artom means actully use "latest" not fine the latest microverion and use that number19:10
artomRight, that19:10
sean-k-mooneyartom: does the "latest" string actully work in the api19:10
sean-k-mooneyi tought it was just in the client code19:10
* lyarwood tilts head19:10
artomsean-k-mooney, https://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/libvirt/test_numa_live_migration.py#L4119:10
lyarwoodI'm writing this against master at the moment19:10
sean-k-mooney cool so ya19:11
lyarwoodand I want to write it once and not have to update it with each backport as latest changes19:11
sean-k-mooney  microversion = 'latest' will be the latest version for any given release if you packport19:11
lyarwoodyeah but that's going to drop support for various things along the way meaning I'll need to constantly update the func test as I backport19:11
lyarwoodlike the create args change loads between master and queens19:12
gmannyes, 'latest' keyword is supported in API too19:12
lyarwoodnetworks for example19:12
sean-k-mooneywell you will have to do that anyway right19:12
gmannit is api_version.max_api_version()19:12
lyarwoodnot if I just picked the latest in queens when landing it in master19:12
artomlyarwood, yeah, if you write your logic against APIs that only exist in U and up...19:12
artomBut I didn't see anything like that that jumped out19:12
sean-k-mooneylyarwood: you could do that but you proably should also have a test for master then no?19:13
sean-k-mooneywhat were you trying to test by the way19:13
lyarwoodnothing microversion specific19:13
lyarwoodthis is just to keep the setup code sane19:14
lyarwoodand stop chrun in the backports19:14
sean-k-mooneythen use 2.1 if you want19:14
lyarwoodthen I'm writing tests against a version of the API we don't even support anymore19:14
sean-k-mooneywe do support it because of microversion but my point was more dont set a microverison and only popluate the minium filed in the create request you need19:15
lyarwoodright sorry I mean with microversion = None19:15
sean-k-mooneywe teachnicaly have to support very micoversion untill we go to nova v419:15
lyarwoodyeah true19:16
sean-k-mooneymicroversion = None is the same as 2.119:16
sean-k-mooneyi think19:16
gmannfunctional tests are with 'latest' by default i think doing with 'latest' make sense19:16
lyarwoodgmann: it's set to None in _IntegratedTestBase19:16
sean-k-mooneygmann: that is how i have always written them unless i was testing a feature19:16
artomgmann, I think they're none, if I have the right code in front of me: https://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/api/client.py#L13519:17
sean-k-mooneyin which case i might use the microverion it was added in19:17
* lyarwood sets it to latest and moves on19:17
sean-k-mooneylyarwood: is _IntegratedTestBase the new base of the functional test or the old one that we should not use anymore19:18
sean-k-mooneythere are a few baseclasses we can use in the funcational tests19:18
lyarwoodsean-k-mooney: the former I think, stephenfin is talking about replacing ProviderUsageBaseTestCase with it19:18
sean-k-mooneywaith isnte ProviderUsageBaseTestCase the thing we were ment to be moving too19:18
lyarwoodhttps://github.com/openstack/nova/blob/master/nova/tests/functional/integrated_helpers.py#L1062-L106319:18
gmannah right, it is in ProviderUsageBaseTestCase not base and many tests side too19:22
sean-k-mooney oh sorry _IntegratedTestBase is not the one with the weird implematnion of _wait_for_server_allocations19:22
artomsean-k-mooney, that got fixed a while ago19:22
artomstephenfin unified all the things19:22
sean-k-mooneyya i rememebr19:22
sean-k-mooneyi just was not sure if we still had some remnetes19:23
openstackgerritLee Yarwood proposed openstack/nova master: func: Add live migration rollback volume attachment tests  https://review.opendev.org/74353419:23
openstackgerritLee Yarwood proposed openstack/nova master: Add regression tests for bug #1889108  https://review.opendev.org/74328919:23
openstackbug 1889108 in OpenStack Compute (nova) "failures during driver.pre_live_migration remove source attachments during rollback" [High,In progress] https://launchpad.net/bugs/1889108 - Assigned to Lee Yarwood (lyarwood)19:23
openstackgerritLee Yarwood proposed openstack/nova master: compute: Don't delete the original attachment during pre LM rollback  https://review.opendev.org/74331919:23
openstackgerritLee Yarwood proposed openstack/nova master: func: Add CinderFixture to _IntegratedTestBase  https://review.opendev.org/74353519:23
sean-k-mooneywhat is confution me is that _IntegratedTestBase has a preceding underscore19:23
sean-k-mooneyand i tought we were not ment to be using it directly anymore19:24
sean-k-mooneyane moving to the public base class19:24
sean-k-mooneyoh that todo was added 2 weeks ago https://github.com/openstack/nova/commit/c60f90cb2f8e2a30ffee5abeebd78f9eee3c6b2519:25
*** tosky has quit IRC19:25
sean-k-mooneyso ya one of the main deltas right now is   microversion = None in _IntegratedTestBase and  microversion = 'latest' in ProviderUsageBaseTestCase19:26
sean-k-mooneyhttps://github.com/openstack/nova/blob/master/nova/tests/functional/integrated_helpers.py#L978 vs https://github.com/openstack/nova/blob/master/nova/tests/functional/integrated_helpers.py#L107519:27
gmannboth are used in same amount so it is like half of functional tests run with 'None' and half with 'latest'19:29
sean-k-mooneymany other set it expcitly19:30
sean-k-mooneyor dont use either19:31
sean-k-mooneythe last regression i wrote did not use either of the base classes since the kept getting rewritten19:31
sean-k-mooneyhttps://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/test_bug_1835822.py#L22-L2319:31
gmannrunning with 'latest' can give better coverage of latest code set and capture if something effecting functionality in latest (as new version on top of old one)code path19:31
sean-k-mooneyi think latest is generally more corerct unless you are testing older version explcitly19:32
sean-k-mooneyor testing something that is not version related19:32
sean-k-mooneyin which can none and latest are qually valid19:33
sean-k-mooneyif you use none it wont catch if a new cahnge alters the behavior19:33
sean-k-mooneywell it will if you alter it without actounting for microverison i guess19:33
gmanncorrect.19:34
*** vishalmanchanda has quit IRC20:41
openstackgerritMerged openstack/python-novaclient master: Add a cleanup for a server in a functional test  https://review.opendev.org/74358920:52
*** evrardjp has joined #openstack-nova20:55
*** gyee has joined #openstack-nova21:02
*** raildo has quit IRC21:23
*** slaweq has quit IRC21:34
*** jsuchome has quit IRC21:40
*** mriedem has left #openstack-nova21:41
*** redrobot has joined #openstack-nova21:43
*** xek_ has quit IRC22:03
*** rcernin has joined #openstack-nova22:09
*** aj_mailing has joined #openstack-nova22:11
*** aj_mailing has quit IRC22:14
*** rcernin has quit IRC22:22
*** rcernin has joined #openstack-nova22:37
*** ircuser-1 has quit IRC22:48
*** rcernin has quit IRC22:50
*** rcernin has joined #openstack-nova22:50
*** tkajinam has joined #openstack-nova22:52
*** mlavalle has quit IRC23:01
*** ircuser-1 has joined #openstack-nova23:05
*** zer0c00l has joined #openstack-nova23:10
*** brinzhang has joined #openstack-nova23:46
brinzhanggmann: we already introduced microversion in Ussuri, this is the patch https://review.opendev.org/#/c/696860/23:47
brinzhanggmann: I know this is still very simple and needs further optimization. If you have better suggestions, we are looking forward to it23:48
*** hamalq has quit IRC23:48
brinzhanggmann: you are an expert in api, you can do the api change in cyborg too, and we will be very happy to see you improve this ^23:52
« Monday, 2020-07-27 Index Wednesday, 2020-07-29 »

Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!