Monday, 2018-11-05

*** erlon has joined #openstack-nova00:03
*** slaweq has joined #openstack-nova00:13
*** brinzhang has joined #openstack-nova00:22
*** Dinesh_Bhor has joined #openstack-nova00:31
*** Dinesh_Bhor has quit IRC00:31
*** slaweq has quit IRC00:45
*** bhagyashris has joined #openstack-nova00:55
*** slaweq has joined #openstack-nova01:11
*** licanwei has joined #openstack-nova01:35
*** Dinesh_Bhor has joined #openstack-nova01:37
*** Dinesh_Bhor has quit IRC01:37
*** slaweq has quit IRC01:44
*** jwongz has joined #openstack-nova01:52
openstackgerritJoshua Cornutt proposed openstack/nova master: Added [key_pairs]/fingerprint_algorithm options  https://review.openstack.org/61546001:54
openstackgerritJoshua Cornutt proposed openstack/nova master: Added [key_pairs]/fingerprint_algorithm options  https://review.openstack.org/61546001:56
*** Dinesh_Bhor has joined #openstack-nova01:59
*** hongbin has joined #openstack-nova02:05
*** mhen has quit IRC02:10
*** mhen has joined #openstack-nova02:11
*** slaweq has joined #openstack-nova02:11
*** erlon has quit IRC02:21
*** Dinesh_Bhor has quit IRC02:31
*** Dinesh_Bhor has joined #openstack-nova02:40
*** slaweq has quit IRC02:45
*** mrsoul has quit IRC02:50
*** TuanDA has joined #openstack-nova02:55
*** Cathyz has joined #openstack-nova03:05
*** slaweq has joined #openstack-nova03:13
*** licanwei has quit IRC03:38
*** slaweq has quit IRC03:44
*** Dinesh_Bhor has quit IRC03:47
*** Cathyz has quit IRC03:48
*** Dinesh_Bhor has joined #openstack-nova03:51
*** dave-mccowan has quit IRC04:00
*** slaweq has joined #openstack-nova04:16
*** udesale has joined #openstack-nova04:31
*** janki has joined #openstack-nova04:47
*** slaweq has quit IRC04:48
*** hongbin has quit IRC04:50
*** Nel1x has quit IRC04:58
*** kevinbenton has quit IRC05:00
*** kevinbenton has joined #openstack-nova05:00
*** ratailor has joined #openstack-nova05:07
*** slaweq has joined #openstack-nova05:16
*** pcaruana has joined #openstack-nova05:23
*** Dinesh_Bhor has quit IRC05:24
*** Dinesh_Bhor has joined #openstack-nova05:30
*** pcaruana has quit IRC05:32
*** moshele has joined #openstack-nova05:33
*** ivve has joined #openstack-nova05:33
*** slaweq has quit IRC05:44
*** zul has quit IRC05:49
*** slaweq has joined #openstack-nova06:11
*** moshele has quit IRC06:18
*** tetsuro has joined #openstack-nova06:18
*** fghaas has joined #openstack-nova06:22
*** pvc has joined #openstack-nova06:23
pvchi anyone successfully integrate cyborg to nova?06:23
*** fghaas has quit IRC06:27
*** hshiina has joined #openstack-nova06:33
*** slaweq has quit IRC06:39
*** Luzi has joined #openstack-nova07:03
*** slaweq has joined #openstack-nova07:11
*** dpawlik has joined #openstack-nova07:12
*** jaosorior has joined #openstack-nova07:13
*** slaweq has quit IRC07:16
*** dpawlik has quit IRC07:16
*** jaosorior has quit IRC07:24
*** jaosorior has joined #openstack-nova07:27
*** jangutter has joined #openstack-nova07:39
*** dpawlik has joined #openstack-nova07:42
*** ccamacho has joined #openstack-nova07:44
*** dpawlik has quit IRC07:47
*** ircuser-1 has joined #openstack-nova07:53
*** Dinesh_Bhor has quit IRC07:54
*** hshiina has quit IRC07:58
*** pcaruana has joined #openstack-nova08:06
*** ralonsoh has joined #openstack-nova08:14
*** fghaas has joined #openstack-nova08:16
*** dpawlik has joined #openstack-nova08:17
*** tssurya has joined #openstack-nova08:20
*** helenafm has joined #openstack-nova08:31
*** fanzhang has quit IRC08:42
*** ratailor_ has joined #openstack-nova08:46
*** ratailor has quit IRC08:48
*** bauwser is now known as bauzas08:56
bauzasgood morning stackers08:56
*** jpena|off is now known as jpena08:56
*** Dinesh_Bhor has joined #openstack-nova09:01
*** ccamacho has quit IRC09:04
pvcbauzas09:06
pvcit is possible to implement cybrog to nova09:06
bauzasit's a long story but some people try to do this09:10
*** ccamacho has joined #openstack-nova09:12
jangutterit's also unlikely to happen in the short term.09:13
*** ttsiouts has joined #openstack-nova09:20
*** pvc has quit IRC09:27
johnthetubaguybauzas: just checking, partly for pvc, but I didn't think we had any code in place to integrate cyborg in master (yet)? Apart from maybe PCI pass-through?09:28
bauzasjohnthetubaguy: AFAIK, cyborg directly modifies nova.conf for using PCI passthru, yes09:31
*** pcaruana has quit IRC09:31
johnthetubaguyah, got it, thanks09:31
bauzasjohnthetubaguy: there was a YVC session09:31
johnthetubaguyyeah, I was more checking there wasn't post PTG progress I didn't know about while I was out of it09:32
*** pcaruana has joined #openstack-nova09:32
*** ttsiouts has quit IRC09:36
*** ttsiouts has joined #openstack-nova09:38
*** slaweq has joined #openstack-nova09:41
*** adrianc has joined #openstack-nova09:47
*** derekh has joined #openstack-nova09:49
*** TuanDA has quit IRC10:03
*** panda has joined #openstack-nova10:05
openstackgerritSilvan Kaiser proposed openstack/nova master: [WIP] Added Qemu libquobyte Support to the Quobyte Driver  https://review.openstack.org/54650010:14
*** ccamacho has quit IRC10:15
*** ccamacho has joined #openstack-nova10:17
*** takashin has joined #openstack-nova10:22
*** cdent has joined #openstack-nova10:22
*** bhagyashris has quit IRC10:25
*** naichuans has joined #openstack-nova10:28
*** davidsha has joined #openstack-nova10:30
*** Dinesh_Bhor has quit IRC10:34
*** dtantsur|afk is now known as dtantsur\10:35
*** dtantsur\ is now known as dtantsur10:35
*** lpetrut has joined #openstack-nova10:36
*** aarents has joined #openstack-nova10:41
lpetrutHi, is it acceptable for exceptions to include another exception as message? Asking as exception_to_dict won't handle it properly: https://github.com/openstack/nova/blob/5859741f4d5e08ec15169b9c8d1aae1836442fd2/nova/compute/utils.py#L58-L8710:41
*** fghaas has left #openstack-nova10:43
*** maciejjozefczyk has quit IRC10:50
*** maciejjozefczyk has joined #openstack-nova10:54
openstackgerritTakashi NATSUME proposed openstack/nova master: Use links to placement docs in nova docs  https://review.openstack.org/61405611:01
openstackgerritTakashi NATSUME proposed openstack/nova master: Remove Placement API reference  https://review.openstack.org/61443711:02
*** k_mouza has joined #openstack-nova11:03
openstackgerritTakashi NATSUME proposed openstack/nova master: Fix best_match() deprecation warning  https://review.openstack.org/61120411:06
*** aarents has quit IRC11:18
*** sean-k-mooney has joined #openstack-nova11:27
sean-k-mooneybauzas: can you reivew https://review.openstack.org/#/c/610034/11:36
*** dpawlik has quit IRC11:38
adriancsean-k-mooney: Hi, i would like to co-author the following commits as i am now working towards aligning the POC code for libvirt sriov live migration: https://review.openstack.org/#/c/607365/ https://review.openstack.org/#/c/607365/11:41
sean-k-mooneyadrianc: hi yes that is fine feel free too. i have been held up with other work for the last 2 weeks so have not made much progress on this since then11:44
sean-k-mooneyim currently adressing stephens feedback on the spec but i should get that done today11:44
adriancgreat, thanks sean-k-mooney11:45
*** erlon has joined #openstack-nova11:52
*** beekneemech has quit IRC11:53
*** bnemec has joined #openstack-nova11:57
*** dave-mccowan has joined #openstack-nova12:04
*** janki has quit IRC12:05
*** brinzhang has quit IRC12:13
*** udesale has quit IRC12:14
*** dpawlik has joined #openstack-nova12:16
*** dpawlik has quit IRC12:18
*** dpawlik has joined #openstack-nova12:18
*** tetsuro has quit IRC12:20
*** jpena is now known as jpena|lunch12:20
*** dpawlik has quit IRC12:23
*** dpawlik has joined #openstack-nova12:26
*** dpawlik has quit IRC12:27
*** dpawlik has joined #openstack-nova12:28
openstackgerritLucian Petrut proposed openstack/nova master: Convert exception messages to strings  https://review.openstack.org/61555012:29
*** dpawlik_ has joined #openstack-nova12:30
*** dpawlik has quit IRC12:31
*** k_mouza has quit IRC12:31
*** dpawlik_ has quit IRC12:32
*** dpawlik has joined #openstack-nova12:32
*** jroll has quit IRC12:32
*** jroll has joined #openstack-nova12:34
*** dpawlik_ has joined #openstack-nova12:35
*** dpawlik has quit IRC12:37
*** dpawlik_ has quit IRC12:42
*** dpawlik has joined #openstack-nova12:42
*** ratailor__ has joined #openstack-nova12:46
*** aarents has joined #openstack-nova12:47
*** ratailor_ has quit IRC12:49
*** dtantsur is now known as dtantsur|brb12:51
aarentsHi there, can someone may evaluate this issue: https://bugs.launchpad.net/nova/+bug/180170212:54
openstackLaunchpad bug 1801702 in OpenStack Compute (nova) "Spawn may fail when cache=none on block device with logical block size > 512" [Undecided,New]12:54
bauzassean-k-mooney: ack, will look13:03
*** Sundar has joined #openstack-nova13:05
*** ttsiouts has quit IRC13:05
Sundarcdent: Please LMK when you have some time, regarding https://review.openstack.org/#/c/603955/13:06
cdentSundar: I'm unlikely to have much time to talk synchronously, a lot going on, but if you leave questions and comments on the review I will respond as soon as I can13:07
cdentbut if it is something quick?13:08
Sundarcdent: OK. I was hoping to get and provide clarification on the general comment: "In general it seems like the resources and representations thereof are too closely tied to the specific needs of the API"13:08
SundarI will respond in the review itself, as best as I can13:09
SundarThanks13:09
cdentthank you13:09
*** zul has joined #openstack-nova13:16
*** tetsuro has joined #openstack-nova13:19
*** k_mouza has joined #openstack-nova13:25
*** jpena|lunch is now known as jpena13:25
bauzassean-k-mooney: +2d with a few nits13:28
sean-k-mooneybauzas: thanks ill check them out13:28
*** davidsha has quit IRC13:31
*** jistr is now known as jistr|call13:32
*** ratailor__ has quit IRC13:34
*** panda is now known as panda|bbl13:36
*** ttsiouts has joined #openstack-nova13:43
tssuryamelwitt: is there any reason on why we calculate the quota for instances/ram/cores per user per project ? (https://review.openstack.org/#/c/569055/2/nova/quota.py@1342), like do we practically use the counts for per user over projects anywhere ?13:46
sean-k-mooneytssurya: i belive we used to use the per user qoats in the past i assuemd we still do13:47
tssuryasean-k-mooney: we now do per project quotas right ? I am gessing only for keypairs we do per user ?13:47
sean-k-mooneytssurya: i was under the impression we had both13:48
tssuryaah okay13:48
tssuryalet me check the documentation13:48
sean-k-mooneytssurya: personally i would consider it a fairly big regression if we drop the user quotas13:48
sean-k-mooneyit was not that uncommon for people to deploy one project per department/team in private cloude and then have a user level quota too13:49
tssuryasean-k-mooney: hmm correct looks like we have user quotas also heh,13:49
sean-k-mooneyso the test team project can have 100 instances but each tester can only have 10 personally13:50
tssuryaokay sounds fair enough that restricting resources on users also is valid13:50
sean-k-mooneyi dont know what the plans for this were with unified limits however13:50
sean-k-mooneybest to ask melwitt et al for comment13:50
tssuryathanks sean-k-mooney , yea I don't know about tha unified limits stuff althought there is a spec worked upon by John13:51
* johnthetubaguy waves13:51
* tssurya waves back13:51
johnthetubaguytssurya: currently the operator can specify per user limits, there are no plans to support this post unified limits13:52
johnthetubaguywell basically, the plan is hierarchical limits replace that feature13:52
sean-k-mooneyjohnthetubaguy: really13:52
tssuryajohnthetubaguy: ah thanks, so we are going to drop user quotas altogether ?13:52
leakypipesjohnthetubaguy: I'm getting close...13:53
johnthetubaguytssurya: that is what I propose in the spec and my patches, yeah13:53
leakypipesjohnthetubaguy: sigh, these quota unit test are ugly-as-f...13:53
sean-k-mooneyjohnthetubaguy: so each user would have there own project in that case as a subproject of there teams project13:53
johnthetubaguyleakypipes: not been in them yet!13:53
tssuryajohnthetubaguy: thanks I will go read the spec13:53
*** leakypipes is now known as jaypipes13:53
jaypipesjohnthetubaguy: bring a decanter of scotch.13:53
jaypipesyou'll need it.13:53
jaypipeshopefully I'll have the worst of them cleaned up shortly.13:54
jaypipes9 failures left to fix (after converting them from test.TestCase to test.NoDBTestCase)13:54
johnthetubaguyOK, nice13:54
sean-k-mooneyjaypipes: johnthetubaguy so has there been any push back on the fact that users with acess to multiple project will nolong have a global limit on the resouce that user can use13:58
sean-k-mooneyi know you can kindo of model that with per user project nested under a larger project but with only 2 levels of nesting that is not as flexible as what could be done before13:59
*** janki has joined #openstack-nova13:59
johnthetubaguysean-k-mooney: so I should be more precise, it was per user within a project, if my memory is correct13:59
*** lpetrut has quit IRC13:59
jaypipessean-k-mooney: no, no pushback.14:00
johnthetubaguysean-k-mooney: the only users we know about, prefer and want to move to two level project quotas (they used the user thing as a workaround)14:00
sean-k-mooneyjohnthetubaguy: wait could you have a per user qoat that was different form the  users global  quota  for a specific porject before14:00
jaypipessean-k-mooney: what johnthetubaguy said.14:00
johnthetubaguyyeah, the opposite, we have support for this direction14:00
jaypipessean-k-mooney: what's a "qoat"? Quota Of All Time? :)14:01
johnthetubaguysean-k-mooney: there was no global user quota, it was per user within a specific project. Maybe I answered tssurya's question badly14:01
sean-k-mooneyoh then i totally missunder stood how that option worked and always gave my team memebr more instance then i planned lol14:02
tssuryajohnthetubaguy: no I think you answered my question correctly. we currently support per user quota within projects, after unified limits we won't right ?14:03
johnthetubaguytssurya: yeah, that's right14:03
sean-k-mooneyi assumed the user quota applied across all project so when i gave them acess to a second project outside there main one i used the user quotat to limit there use of th unlimited project14:03
tssuryayea so I guess sean-k-mooney was worried of this to be a big bad regression ?14:03
johnthetubaguytssurya: the folks who used that also modified policy so you couldn't delete someone elses instance, so its all a bit strange around those parts14:04
tssuryajohnthetubaguy: oh yea that is strange if its all in the same project14:04
sean-k-mooneytssurya: from what johnthetubaguy and jaypipes explained there is no regression that i can see so all good14:04
johnthetubaguysean-k-mooney: yeah, I thought that too till I reviewed melwitt's patches, turns out it doesn't do that14:04
sean-k-mooneyjohnthetubaguy: well it was a private developer cloud so no harm in my case but ya14:05
sean-k-mooneyhaving to adjust both the project and user limits was always a bit of a pain so it soundls like its an improvment overall14:06
johnthetubaguycool14:06
tssuryasean-k-mooney: okay :)14:06
*** jistr|call is now known as jistr14:08
*** jdillaman has quit IRC14:09
*** jdillaman has joined #openstack-nova14:09
*** mriedem has joined #openstack-nova14:11
*** davidsha has joined #openstack-nova14:14
*** panda|bbl is now known as panda14:15
*** dtantsur|brb is now known as dtantsur14:19
*** fried_rice is now known as efried14:25
*** k_mouza has quit IRC14:25
*** k_mouza has joined #openstack-nova14:25
*** SteelyDan is now known as dansmith14:37
*** mlavalle has joined #openstack-nova14:42
efriedtssurya: I saw Belmiro respond on the ML; does this mean y'all have tried the patch in your env somewhere?14:44
tssuryaefried: well no all we do it increase the config's value to a very high number, ever since we added the config in queens14:45
tssuryaso if you allow setting the config to "0"14:45
tssuryathat will disable it its good for us14:45
efriedtssurya: The patch actually goes quite a bit further than that.14:45
efriedI believe with the refresh interval very high, you're still getting the inventory polls.14:46
efriedNow you won't even get those anymore.14:46
openstackgerritJohn Garbutt proposed openstack/nova-specs master: Add Unified Limits Spec  https://review.openstack.org/60220114:46
efried...I think.14:46
*** mvkr has quit IRC14:47
tssuryaefried: oh just read the commit (message)14:47
*** awaugama has joined #openstack-nova14:47
tssuryayea we don't currently have any of those yet downstream (meaning our code is the same as upstream at the tracker level)14:47
tssuryabut it would be cool to optimize the calls to placement to the maximum14:48
sean-k-mooneyefried: by the way if you have time can you review https://review.openstack.org/#/c/610034/14:49
tssuryabecause in rocky you have more stuff than in queens right ? the way the updates are done with pulling the prodiver tree info and everything (meaing the new update_to_placement function)?14:49
*** hshiina has joined #openstack-nova14:49
efriedtssurya: What I think we'll be really looking for in order to have enough confidence to merge this is for deployments such as yours and mnaser's to put this in place and verify, not so much that the number of placement calls drops off a cliff - that should be a given - but that we don't wind up with the RT getting out of sync with placement's view.14:49
efriedtssurya: I don't believe there's much more in rocky than queens, no.14:49
*** Sundar has quit IRC14:50
*** awaugama has quit IRC14:50
tssuryawe are also investigating on our end14:50
*** ccamacho has quit IRC14:50
tssuryaI can see if we can test this patch in our deployment14:50
tssuryaand give feedback14:50
efriedOne thing about mnaser's comments, which I will also mention in a response on the thread, is that this should not affect *allocations* at all. IIUC, it was allocations getting out of sync that mnaser observed. I will be interested to know if he or you have ever observed anything else (inventories, etc.) getting out of sync.14:51
efriedEspecially you, since you've been running the long-poll in production for a while, right?14:51
*** awaugama has joined #openstack-nova14:51
tssuryaefried: well the long poll used to only switch off traits and aggregates14:51
efriedtssurya: It would also affect inventory updates.14:52
efriedoh14:52
*** ccamacho has joined #openstack-nova14:52
efriedyeah, I see what you're saying now. I think you're right.14:52
efriedand we're not doing much with those in q/r.14:52
tssuryayea I am pretty sure it was only the traits and aggregates syn that was off14:53
tssuryathe inventory sync was like normal for us14:53
efriedthe poll you're stretching would also update inventories, but there was an additional, separate inventory update that was happening outside of that one, so yeah.14:53
tssuryaah yea that makes sense14:53
tssuryabecause we didn't have that much out of syn inventory issues14:54
tssuryasync*14:54
efriedtssurya: The theory behind the patch is that you shouldn't have those issues anyway, even if we *never* poll.14:55
efriedbut14:55
efriedI'm not sure I hit the code paths that do that separate inventory update14:55
efriedsad face14:55
jaypipesthree more unit test failures to address... closing in...14:55
*** tetsuro has quit IRC14:55
tssuryahaha, I am trying to go through your commit now... but14:55
tssuryaso idea is a inventory refresh only if something changes right ?14:56
efriedtssurya: well, at least in this code path, yes. Basically, if you set the refresh interval to zero, the only time you would get a refresh is if e.g. the virt driver pushes a change via update_provider_tree.14:57
efriedjaypipes: IYO, should there be a bp and/or spec for this? And should it be multiple patches?14:57
jaypipesefried: the cache change stuff?14:58
efriedy14:58
jaypipesefried: I think it would be useful to have a bp for tracking purposes, sure. spec, not so much.14:58
efriedight, we'll start there.14:58
*** k_mouza has quit IRC14:59
*** munimeha1 has joined #openstack-nova15:01
*** janki has quit IRC15:01
mnasertssurya: I can give you a script I wrote to audit placement (I think it’s in a paste somewhere). Interesting to see if you see things get out of sync too15:02
tssuryaefried: hmm yea makes sense as for the syncing issues which I am sure we would hit considering our size, can't we slowly build a heal/sync tool like we have for allocations/aggregates that deployments can run when they want ?15:02
openstackgerritJohn Garbutt proposed openstack/nova master: WIP: Integrating with unified limits  https://review.openstack.org/61518015:03
tssuryalike a placement sync tool for plausible things that would go out of sync15:03
tssuryamnaser: ah thanks so far we haven't like disabled any major sync updates15:04
*** k_mouza has joined #openstack-nova15:04
*** takashin has left #openstack-nova15:04
tssuryabut we surely do have out of sync issues15:04
efriedtssurya: I think healing allocations is a separate issue. If we can show that inventories/traits/aggregates don't get out of sync when we don't refresh them, that's goodness. And then separately, if we can write something to heal allocations - or better yet, to identify why they're getting out of sync in the first place and close that gap - also goodness.15:04
mnasertssurya: this is for allocations being out of sync tho, so it’d be interesting if this is something we have broken I guess15:04
*** jistr is now known as jistr|call15:04
efriedmnaser: Do you have any suspicions (or better) about where the allocation drift is happening? Like, is it on instances that fail a migration or similar? Resizes? Evacuations? Or (eek) just steady state?15:05
openstackgerritMerged openstack/nova master: Minimal construct plumbing for nova show when a cell is down  https://review.openstack.org/59165815:06
mnaserI think it might be around live migrations, I suspect that if live migrations fail on a machine that somehow already has issues talking to placement then it won’t be able to revert the allocation or whatever15:06
mnaserDo the compute nodes make all the claims during a resize or live migration?15:07
mnaserI.e is it possible scheduler does something that compute cannot revert because of an intermittent issue15:07
tssuryaefried, mnaser: there is no way we don't have out of sync issues I am literally already working on a consistency tool but I can confirm more after looking further about the statistics regarding allocations/inventories - about how much those are out of sync15:07
efriedmnaser: certainly, although I thought we had (a bunch of really ugly) code to clean up that mess.15:07
sean-k-mooneymnaser: i was under the impression the cpu ram and disk were still claimed in the schduler on migration15:08
sean-k-mooneybut pci device would be cliamied by the compute nodes15:08
sean-k-mooneywell that is a bad example15:08
efriedmnaser: I can't think of a way inventories would get out of sync unless a third party is mucking with them (e.g. CLI).15:08
efriedmnaser: But failed migrations of various types - absolutely.15:08
efried^^ for allocations15:09
mnasertssurya: let me share a tool that does exactly that!15:09
mnaserYeah I don’t think I ever had issue with inventories15:09
tssuryamnaser: that would be great then I can get some statistics15:09
efriedmnaser: But as we were discussing above, even if you max out the refresh interval, we are still polling (and "healing") the inventories every periodic.15:09
efriedso if the inventories did drift, we would have fixed them.15:10
tssuryaefried: true15:10
efriedSo there's really no way to know if drift issues exist there.15:10
mnaseryeah but i dont think those would drift in our use cases15:10
efried...until I kill that.15:10
mnasereven if they didnt sync15:10
mnaser:P15:10
*** jistr|call is now known as jistr15:10
efriedI think I have a pretty major rework to do on this patch now that I'm thinking in terms of the inventory refreshes...15:10
mriedemtssurya: https://bugs.launchpad.net/nova/+bug/179356915:11
openstackLaunchpad bug 1793569 in OpenStack Compute (nova) "Add placement audit commands" [Wishlist,Confirmed]15:11
tssuryamriedem: thanks15:11
mnaserhttp://paste.openstack.org/show/734146/15:12
mnasereasier to digest paste because of launchpad's wrapping15:12
mnasertssurya: ^15:12
openstackgerritJohn Garbutt proposed openstack/nova-specs master: Add Unified Limits Spec  https://review.openstack.org/60220115:13
*** jiaopengju has quit IRC15:14
*** jiaopengju has joined #openstack-nova15:14
*** mvkr has joined #openstack-nova15:15
*** k_mouza has quit IRC15:15
tssuryamnaser: thanks :)15:18
mriedemefried: fwiw, i would at least split out the change to disable the refresh interval (config value of 0)15:18
mriedemsince that's pretty straight forward i imagine15:18
efriedmriedem: ack, thx15:18
*** cfriesen has joined #openstack-nova15:21
*** d34dh0r53 has quit IRC15:22
*** ttsiouts has quit IRC15:22
*** d34dh0r53 has joined #openstack-nova15:23
*** ttsiouts has joined #openstack-nova15:25
*** lyarpwood is now known as lyarwood15:26
*** Luzi has quit IRC15:32
*** tbachman has joined #openstack-nova15:32
*** ttsiouts has quit IRC15:33
openstackgerritMaciej Jozefczyk proposed openstack/nova master: Force refresh instance info_cache during heal  https://review.openstack.org/59160715:50
openstackgerritMaciej Jozefczyk proposed openstack/nova master: Add fill_virtual_interface_list online_data_migration script  https://review.openstack.org/61416715:50
*** ttsiouts has joined #openstack-nova15:54
openstackgerritMaciej Jozefczyk proposed openstack/nova master: Add fill_virtual_interface_list online_data_migration script  https://review.openstack.org/61416715:55
*** dklyle has joined #openstack-nova16:02
*** dpawlik has quit IRC16:05
*** dpawlik_ has joined #openstack-nova16:05
*** artom has quit IRC16:06
*** hshiina has quit IRC16:06
*** pcaruana has quit IRC16:06
*** ccamacho has quit IRC16:08
openstackgerritEric Fried proposed openstack/nova master: DNM: Trust the report client cache more  https://review.openstack.org/61488616:11
*** dpawlik_ has quit IRC16:12
*** dpawlik has joined #openstack-nova16:12
*** ttsiouts has quit IRC16:14
*** ttsiouts has joined #openstack-nova16:14
*** maciejjozefczyk has quit IRC16:16
*** imacdonn has quit IRC16:16
*** imacdonn has joined #openstack-nova16:17
*** dpawlik has quit IRC16:17
*** ttsiouts has quit IRC16:19
*** dpawlik has joined #openstack-nova16:28
*** k_mouza has joined #openstack-nova16:30
*** dpawlik has quit IRC16:32
efriedstephenfin: Does :oslo.config:option: not work in renos yet?16:33
stephenfinefried: https://github.com/openstack/nova/blob/master/doc/source/conf.py vs. https://github.com/openstack/nova/blob/master/releasenotes/source/conf.py16:33
stephenfinefried: It's a different build. You'd need to configure it for reno16:34
stephenfinBut I strongly advise against doing so16:34
efriedstephenfin: ack. I think I don't care that much.16:34
efriedwhy would you advise against it?16:34
stephenfinIf we removed/renamed the option in the future, we'd break the build16:34
efriedas we should16:35
efriedoh16:35
efriedthe reno should be able to stay forever16:35
efriedgot it.16:35
stephenfinYup16:35
efriedbutbutbut16:35
sean-k-mooneyi was about to say we cant fix rennos after teh fact right16:35
efriedshouldn't it therefore point to the respective version of the config.html?16:35
sean-k-mooneyor at least its non trivail to change them after a release is done16:35
efriedsean-k-mooney: You asked me to review https://review.openstack.org/#/c/610034/ -- I'm out of my depth on that one, I'm afraid.16:36
sean-k-mooneyefried: it would if you had the commit checked out16:36
stephenfinsean-k-mooney: We couldn't in the past but we can now (reno fixed that). That said, why make life hard for yourself16:36
stephenfin*?16:36
stephenfinefried: I guess it should, yes16:36
sean-k-mooneyefried: ok no worries16:36
stephenfinBut that ship has long since sailed. All our doc links in renos point to latest16:37
efriedsean-k-mooney: You spelled "outer" wrong! -2!16:37
efriedthat's about the best I can do on that one.16:38
sean-k-mooneywhere in the patch16:38
efriedhttps://review.openstack.org/#/c/610034/10/nova/utils.py@132216:38
sean-k-mooney...16:38
sean-k-mooneyyes i did ...16:39
sean-k-mooneythat means its incorrect in the placement version too which is a pain.16:40
sean-k-mooneyill repin hopefully for the last time16:40
sean-k-mooneyefried: thanks for finding it it woudl have been even more of a pain when backporting this16:41
efriedsean-k-mooney: Well, it's really not a big deal, especially since that method isn't exposed anywhere outside of the decorator. But sure, if you're going to respin; I imagine the other non-blocking nits are worse than that :)16:42
sean-k-mooneywell i wasnt but i can if needed16:42
sean-k-mooneythis has been part of an a downstream ci blocking bug for almost a month so i would like to finally get it fixed16:43
*** ttsiouts has joined #openstack-nova16:47
openstackgerritLance Bragstad proposed openstack/nova master: WIP: experiment with oslo.limit interface  https://review.openstack.org/61560216:48
lbragstadjohnthetubaguy i took a wild crack at bringing the oslo.limit logic closer to the actual enforcement point, based on the commit you have ^16:48
*** openstackgerrit has quit IRC16:48
johnthetubaguylbragstad: ah, interesting, will have a look16:49
lbragstadi guess i wanted to try and see if it was possible to make oslo.limit simple enough to not require a nova.limit module for dynamic limits16:49
lbragstadbut - what you have there for static limits totally makes sense16:49
lbragstadi was just thinking that the original direction of the context manager might be hard to take advantage of if we go from api code -> limit module -> oslo limit16:50
lbragstadsince oslo.limit and the Enforcer context manager was written to be as close to the actual code consuming resources as possible16:50
johnthetubaguythe problem is the retry check is always in another process at the moment16:51
johnthetubaguyso not sure where the context manager will help, I am hoping jaypipes's patch will be different though16:51
lbragstaddumb question, but what do you mean by the retry logic being in a different process?16:52
lbragstadi was under the assumption that retry logic was in the same area of code that the resource consumption happens16:52
sean-k-mooneywhat are people feeilngs about adding more debug logs in the schduler16:53
sean-k-mooneyor rather in the numa_toplogy_filter to be specific16:53
sean-k-mooneyim trying to debug a no valid host form schduler logs and the is nothing to go on to determin why the numa toplogy filter decied it was invlaid16:54
johnthetubaguylbragstad: it isn't at the moment sadly16:54
lbragstadis the retry logic the thing that protects against race conditions between clients/16:54
lbragstador is that something else?16:55
jaypipessean-k-mooney: if a deployer is using the NUMATopologyFilter, they don't care about quickness of the scheduler. I say go for it.16:55
johnthetubaguysean-k-mooney: I prefer debug logs only on the reject path, if possible, but what jaypipes said too16:55
johnthetubaguylbragstad: it is that thing, let me link to an example16:55
sean-k-mooneyya it wanted to pring the host toplogy and requested guest topology as a debug message only on failure16:56
johnthetubaguylbragstad: this is the recheck for build requests: https://review.openstack.org/#/c/615180/5/nova/conductor/manager.py16:56
sean-k-mooneyat the moment we jsut asy it did not fit16:57
sean-k-mooneythat could be a little noisy however...16:57
johnthetubaguylbragstad: the first check is in the API process, here: https://github.com/openstack/nova/blob/8d089111c8554e94e117ada3a7f51a42df59e84f/nova/compute/api.py#L86816:57
lbragstadahhh16:57
johnthetubaguylbragstad: the recheck is in the conductor, after calling the scheduler16:57
lbragstadinteresting... so that's not API code16:57
lbragstadi see what you mean16:57
melwittsean-k-mooney, tssurya: looks like this already got explained but I can also pile on and say that per user quotas are per project only. so the two-level hierarchy in unifed limits in keystone gets you the same functionality from a nested quota standpoint16:58
sean-k-mooneymelwitt: ya that was news to me that user quotas are per project16:58
melwittgotcha16:58
johnthetubaguylbragstad: sadly the simplest examples of the recheck basically don't get moved to unified limits16:59
lbragstadbecause it's not in nova-api?16:59
johnthetubaguylbragstad: well this is one that could move to unified limits with a context manager, but it doesn't really make sense for other reasons: https://review.openstack.org/#/c/615180/5/nova/api/openstack/compute/server_groups.py17:00
stephenfinjaypipes: Out of curiosity, did you ever post your slides for "Scheduler Wars: A New Hope" anywhere?17:00
lbragstadbecause it's not really a consumable resource, right?17:00
johnthetubaguylbragstad: yeah17:00
lbragstadlimiting server groups is more of a rate limiting thing17:00
johnthetubaguyyeah, its a db bloat protection17:00
*** helenafm has quit IRC17:01
lbragstadgot it17:01
*** openstackgerrit has joined #openstack-nova17:02
openstackgerritEric Fried proposed openstack/nova master: Allow resource_provider_association_refresh=0  https://review.openstack.org/61560617:02
johnthetubaguylbragstad: just to confuse things, I actually propose we remove the rechecks for these rate-limit like things in the spec, since the check is being demoted lets tidy the code up a bit more if we can17:02
*** gyee has joined #openstack-nova17:02
lbragstadbased on my super vague understand of all this, that seems reasonable17:02
johnthetubaguylbragstad: I am planning on keeping the recheck for everything in unified limits, but right now one part is in API the other is in the conductor process17:02
melwittjohnthetubaguy: is the conductor recheck the only problem area for the oslo.limit verify? because I've had a TODO in my head to move the recheck back to nova-api17:03
lbragstadoh, nice...17:03
johnthetubaguymelwitt: not sure its too much of a problem really, but that is the only recheck that is relevant in the end. Isn't the issue that we need to write into the correct DB before we recheck?17:04
lbragstadfor historical context, what was the reason for moving the recheck logic to conductor? Just to have it closer to the database?17:06
melwittjohnthetubaguy: it is, but related to behavior change we have where multi-create causes potentially a lot of instances to fall into ERROR state if recheck fails, I had been thinking we should instead count build requests + instances for the recheck and do that in nova-api instead of doing the conductor thing17:06
melwittlbragstad: because that's where the instance record is created, yes17:06
lbragstadgot it17:06
johnthetubaguylbragstad: would it not be OK just to make the context manager optional, because when verify=false the context manager is really strange?17:07
melwittI can propose a WIP patch today to show what I mean. I just hadn't gotten around to trying it out17:07
johnthetubaguymelwitt: oh good thinking... build requests17:07
lbragstadyeah - i guess you're right17:07
lbragstadoriginally, we were thinking it would make adoption easier, but in this case it might not...17:07
lbragstadwe just didn't want to have to require developers to have to remember to put specific ordering in their API code (e.g., don't forget to recheck *here*)17:08
lbragstadseemed like an interesting opportunity to encapsulate some of that behind the __exit__ of a context manager17:09
johnthetubaguylbragstad: if we make the recheck compulsory, I think it works (like oslo.limit has the configuration about if you recheck or not)17:10
melwittyeah, I suppose other people might run into a similar issue if they've got a resource create happening in a separate service/process as well. I'm sorry I missed this case at previous oslo.limit sessions :\17:10
lbragstadwell - i think this is just one of those things where you don't really see it fall apart until you're knee-deep in the code17:11
johnthetubaguyyeah, I have similar regrets, that PoC patch has really made me think again on a few things17:11
lbragstadwhich is good, who knows, maybe the context manager is just an over-optimization at this point17:12
*** ivve has quit IRC17:12
melwittI think it's definitely a nice option where it fits17:12
johnthetubaguyyeah, I like the option of having it17:12
lbragstadwhat if we (oslo.limit) give nova a public API for enforce and recheck (which you can call optionall)?17:12
jaypipesstephenfin: yeah, they're on good drive... one sec.17:12
lbragstadand forget the context manager for now17:13
lbragstadin the future, we could expose a context manager that consumes enforce() on __enter__ and recheck() or reverify() on __exit__() for compatibility17:13
johnthetubaguylbragstad: I think that works, although it makes "claim" seem like the wrong word, its more a resource request17:14
jaypipesstephenfin: https://bit.ly/scheduler-wars-a-new-hope and https://bit.ly/scheduler-wars-revenge-of-the-split17:14
cfriesenThe vTPM spec is up at https://review.openstack.org/#/c/571111/ if any cores feel like taking a look.  I believe all comments have been addressed.17:15
lbragstadjohnthetubaguy yeah - i'm not tied to the terminology, at the time it seemed easy to think of it as "I'm attempting to claim 4 cores on this project"17:15
stephenfinjaypipes: Awesome. Thanks very much17:15
jaypipesnp17:15
johnthetubaguylbragstad: I would be tempted to say include the context manager from the beginning, but its hard to justify if we have no one to use it in v117:17
lbragstaddo you think it's going to be hard to design enforce() and verify() with the hopes that there will be a context manager in the future?17:18
johnthetubaguylbragstad: that sounds good though, some kind of limit.check call sounds good. Not sure you need a recheck one, depending on the arguments you go for.17:19
lbragstaddoesn't nova need a verify() function that isn't associated to the context manager?17:19
lbragstadthat way nova-conductor can call it?17:20
johnthetubaguysorry, I just confused things there17:20
johnthetubaguyif we have the recheck configuration move into olso.limit we need a verify call that takes a list of resource_names to check, only if configured to17:22
openstackgerritJay Pipes proposed openstack/nova master: quota: remove QuotaEngine.register_resources()  https://review.openstack.org/61561317:22
openstackgerritJay Pipes proposed openstack/nova master: quota: remove defaults kwarg in get_project_quotas  https://review.openstack.org/61561417:22
openstackgerritJay Pipes proposed openstack/nova master: quota: remove get_quota_classes() driver method  https://review.openstack.org/61561517:22
openstackgerritJay Pipes proposed openstack/nova master: quota: remove Context.quota_class  https://review.openstack.org/61561617:22
openstackgerritJay Pipes proposed openstack/nova master: quota: remove FakeContext from quota unit tests  https://review.openstack.org/61561717:22
openstackgerritJay Pipes proposed openstack/nova master: quota: remove _no_class tests  https://review.openstack.org/61561817:22
openstackgerritJay Pipes proposed openstack/nova master: quota: clean up DbQuotaDriver unit tests  https://review.openstack.org/61561917:22
johnthetubaguyso that is what I waiting to see ^17:22
jaypipesjohnthetubaguy: that's mostly just cleanups and test refactoring. still have the new LimitsDriver stuff to push.17:23
johnthetubaguyjaypipes: did you see the thing I did here, for the direction I was thinking: https://review.openstack.org/#/c/61518017:23
lbragstadyeah - i was thinking the main reason for not using a context manager was because nova-conductor does the verification step if configured to do so17:23
jaypipesjohnthetubaguy: the series there is just slowly chipping away at the bloated quota driver interface, attempting to simplify it as much as possible before introducing further changes.17:24
johnthetubaguylbragstad: yeah, its just right now we use a single call, we pass +0 extra resources in the conductor17:24
johnthetubaguyjaypipes: yeah, my thinking was to put a new stack next to the old one, and remove the old stack later, but open to ideas17:24
johnthetubaguy(took me all friday to work out that is what I was actually thinking)17:25
lbragstadjohnthetubaguy right - and that should still be possible if oslo.limit gives you enforce() and verify() functions, right?17:27
lbragstadyou'd just be calling them from two different places17:27
johnthetubaguylbragstad: yeah totally, I just was wondering if you needed the separation, I think you probably do17:27
melwittyeah, you're just thinking allow another option where the APIs are decoupled right? and then if they are co-located in some part of code, you could use the context manager17:28
lbragstad^ yeah17:28
melwittbecause enforce == first check and verify == second check with +/- 017:28
johnthetubaguyyeah, +117:28
lbragstadthe context manager could use the same exact public APIs17:28
johnthetubaguytotally agreed with that17:28
melwittthat actually sounds like it fits nicely17:28
johnthetubaguy+117:29
lbragstadok - sounds like we have some things we can improve in oslo.limit then17:29
johnthetubaguyI wondered if you could just have a single check API that is used in both sides of the context manager, but possibly not17:29
johnthetubaguyi.e. use check in enter and exit17:29
lbragstadyou mean make enforce() generic enough to handle all cases?17:30
johnthetubaguybut I think I prefer oslo.limit deciding if a recheck is done or not17:30
johnthetubaguyyeah17:30
lbragstadyeah - i think wxy-xiyuan tried to do something like that initially17:30
* lbragstad doesn't mind having clear separation of purpose 17:31
jaypipesjohnthetubaguy: did you want a review on the patch above?17:31
jaypipesjohnthetubaguy: I'm not a fan of mixing usage and limit information in the same function/object17:31
*** davidsha has quit IRC17:32
jaypipesjohnthetubaguy: lemme push up what I've been working on. perhaps it will make a bit more sense.17:32
johnthetubaguyjaypipes: from a direction point of view, that would be great17:32
johnthetubaguyjaypipes: yeah, that would be cool17:32
johnthetubaguyI have to run now, but will catch up later17:33
johnthetubaguylbragstad: yeah, the clear separation is almost certainly a win over a smaller API17:33
melwittnot to complicate things but thinking about the APIs, I do wonder how the they'll look in the future when other backends are added, like the etcd locking idea as another way to deal with races17:37
lbragstad^ a couple people also had ideas about using etcd as a way to implement more than two levels of hierarchical checking17:38
melwittto be clear, just thinking to keep those in mind to avoid painting ourselves into a corner for the future. but decoupling APIs shouldn't affect that really17:40
lbragstadideally - the usage of etcd would be an implementation detail for oslo.limit to handle17:41
lbragstadi would think - i can't really think of a reason why the data coming from nova would change if etcd were being used17:42
melwittI think it would too, just thinking where it fits. it would be in enforce right? and then verify wouldn't be used in that case?17:46
melwittlike, is verify() valid depending on backend?17:47
*** jpena is now known as jpena|off17:48
*** dave-mccowan has quit IRC17:49
*** ttsiouts has quit IRC17:50
*** dave-mccowan has joined #openstack-nova17:50
*** ttsiouts has joined #openstack-nova17:51
*** dave-mccowan has quit IRC17:55
*** ttsiouts has quit IRC17:55
efriedbauzas: LazyLoader?17:58
bauzasefried: wat?18:00
efriedbauzas: I'm wondering if there's a reason functools.partial was necessary there.18:00
bauzasoh about my concern?18:00
bauzasyou mean functools.wraps ?18:00
*** derekh has quit IRC18:00
bauzas.partial is different18:01
efriedbauzas: This is unrelated to anything18:01
bauzasnot sure I understand you18:01
efriedbauzas: I'm running into a unit test snafu due to LazyLoader because I'm trying to access self.compute.reportclient._provider_tree18:01
efriedLazyLoader is returning _provider_tree as a functools.partial18:02
efriedbecause it's only set up to return *callable* attributes from the thing it's shimming.18:02
efriedSo I'm trying to figure out if there's a way for me to do18:02
efriedif callable:18:02
efried  do the thing it does now18:02
efriedelse18:02
efried  just return the attribute, not a functools.partial18:02
efriedbut in order to do that, I'm going to have to collapse it18:03
efriedbauzas: https://review.openstack.org/#/c/104556/9..14/nova/scheduler/client/__init__.py18:05
bauzasah this18:05
efriedI fully expect you to 100% remember all the reasoning that went on behind this change from four years ago.18:06
bauzasefried: .partial() is because we don't want to pass __name18:07
bauzasexample https://stackoverflow.com/questions/15331726/how-does-the-functools-partial-work-in-python18:08
efriedbauzas: butbutbut, why wouldn't it work to just do this:18:08
efried    def __getattr__(self, name):18:08
efried        if self.instance is None:18:08
efried            self.instance = self.klass(*self.args, **self.kwargs)18:08
efried        return getattr(self.instance, name)18:08
efried(FWIW, it seems to do what I need)18:10
bauzasyup, but then that's not a lazy loader ;)18:12
efriedbauzas: Say wha?18:12
efriedbauzas: The only difference in laziness is if a caller pulled a method but didn't call it.18:13
bauzasefried: calling __getattr__ in your case will run getattr() of the instance, right?18:13
efriedyes18:13
bauzasso, it's executing synchronously18:14
*** adrianc has quit IRC18:14
bauzasthe problem is when importing the module18:14
bauzasif you don't lazy load it, then you're loopîng18:14
bauzassee why I lazy-load it below18:15
efriedYou mean getattr looping on itself?18:15
bauzasnope18:15
bauzaswe will only import the modules when we call select_destinations()18:15
bauzas*not when we import nova.scheduler.client* :)18:16
efriedthat... still happens?18:16
*** Swami has joined #openstack-nova18:17
efriedNever mind about why it matters that we wait to import the report client...18:17
bauzasefried: see this blogpost https://snarky.ca/lazy-importing-in-python-3-7/18:17
bauzasefried: anyway, test it18:19
bauzasefried: when I wrote this, we were having an import loop18:19
bauzashence the lazyload18:19
efriedbauzas: Test it how? Is there test code somewhere that verifies that the importing is being done lazily?18:19
bauzasbut now we changed a lot of the client, so maybe it's not needed18:19
bauzasefried: you can pdb it, right?18:20
efriedI would think so.18:20
bauzasor you can just remove the lazyloader and test whether we still have the import loop18:20
bauzasif you have concerns by this class18:20
*** ralonsoh has quit IRC18:20
bauzasanyway, 7:20pm here18:20
bauzas++18:20
efried    def __init__(self):18:20
efried        self.queryclient = LazyLoader(importutils.import_class(18:20
efried            'nova.scheduler.client.query.SchedulerQueryClient'))18:20
efried        self.reportclient = LazyLoader(importutils.import_class(18:20
efried            'nova.scheduler.client.report.SchedulerReportClient'))18:20
efriedPretty sure that -^ is actually doing the import right away.18:21
efriedYou're calling LazyLoader with the *result* of running importutils.import_class(), which is the imported class.18:21
lbragstadmelwitt that's a good question, verify is going to need the current usage and limit information, which means it might need to use etcd18:24
lbragstadto get that information18:24
cdentefried: sure looks that way to me too18:24
cdentthe reason it avod the import loop is simply because it is called "later"18:24
cdentbut it isn't lazy18:24
efriedyuh. I guess I need to pull this part out into a separate patch, sigh.18:24
melwittlbragstad: I was wondering, is verify() even useful though if we are distributed locking on enforce + create? I dunno, just unhelpfully thinking aloud :P18:26
lbragstadoh... i missed the distributed lock part18:26
lbragstadusing etcd as a distributed lock would prevent the need for verify(), then?18:27
melwittthat's what I was thinking. this came up at... was it the forum in vancouver? someone asked why do the recheck thing to avoid races, why not use distributed locking instead, and people had some ideas about using etcd as part of distributed locking. and then we were thinking that could be just one of many potential approaches that oslo.limit could provide underneath18:30
*** jmlowe has quit IRC18:30
melwitt*to handle races18:30
lbragstadoh - sure18:31
lbragstadi haven't thought about that specifically, yet18:31
melwittand I was just thinking whether that affects the API we're thinking. maybe it would just make verify() optional in the case of that backend18:31
efriedcdent, bauzas: oic now. The lazy loader is deferring the *instantiation* of the class until some method is called on it. The import of the module is still happening as soon as you instantiate SchedulerClient. But the arg being passed to the LazyLoader is the class object, not an instance of that class.18:31
efriedAnd IMO that doesn't really save you anything over just instantiating the class right away in the SchedulerClient init.18:31
cdentright18:31
cdentIt's not clear why you'd want to wait18:32
melwittlbragstad: yeah. too early to think too much about it but at the same time, throwing it out there in case it would cause a big problem with what we're thinking about for now18:32
lbragstadin that same session i want to say we were talking about using etcd as a way to communicate events between nova (oslo.limit) and keystone18:32
melwittah, gotcha18:32
lbragstadbecause - once you go past the 2nd layer of projects, it becomes harder to make assumptions about the resources in the tree, i think18:32
dansmithand if you take the locking approach,18:33
dansmithyou have to figure out how high to lock18:33
lbragstadhttp://specs.openstack.org/openstack/keystone-specs/specs/keystone/rocky/strict-two-level-enforcement-model.html#limit-and-usage-awareness-across-endpoints18:34
lbragstaddansmith yeah - that's another important bit18:34
melwittlbragstad: thanks for the link, I had missed that18:35
lbragstadyeah, turns out that was an important bit to include as justification for why we decided to keep things at 2 levels18:36
*** k_mouza has quit IRC18:44
*** jmlowe has joined #openstack-nova18:53
*** mvkr has quit IRC18:55
*** tssurya has quit IRC18:56
*** ivve has joined #openstack-nova18:57
*** tekfiabderrahman has joined #openstack-nova18:59
*** tekfiabderrahman has quit IRC19:00
*** dtantsur is now known as dtantsur|afk19:24
*** zul has quit IRC19:24
openstackgerritJay Pipes proposed openstack/nova master: quota: rename arguments to clarify they are limits  https://review.openstack.org/61563319:30
lbragstaddoes nova allow users to create multiple instances in multiple projects in one request?19:31
melwittno19:31
lbragstadok - cool, just wanted to double check19:31
openstackgerritMerged openstack/nova master: Minimal construct plumbing for nova service-list when a cell is down  https://review.openstack.org/58482919:33
openstackgerritMatt Riedemann proposed openstack/nova master: Update compute API.get() mocks in test_server_metadata  https://review.openstack.org/61534119:38
openstackgerritMatt Riedemann proposed openstack/nova master: Update compute API.get() stubs in test_serversV21  https://review.openstack.org/61534219:38
openstackgerritMatt Riedemann proposed openstack/nova master: Update compute API.get() stubs in test_server_actions  https://review.openstack.org/61534319:38
openstackgerritMatt Riedemann proposed openstack/nova master: Update compute API.get() stubs for test_*security_groups  https://review.openstack.org/61534419:38
openstackgerritMatt Riedemann proposed openstack/nova master: Update compute API.get() stubs for test_disk_config  https://review.openstack.org/61534519:38
openstackgerritMatt Riedemann proposed openstack/nova master: Update compute API.get() stubs in test_access_ips  https://review.openstack.org/61534619:38
openstackgerritMatt Riedemann proposed openstack/nova master: Drop pre-cellsv2 compat in compute API.get()  https://review.openstack.org/61534719:38
openstackgerritMatt Riedemann proposed openstack/nova master: Remove "API Service Version" upgrade check  https://review.openstack.org/61534819:38
*** tbachman has quit IRC20:01
mriedemi'm pretty sure we don't cleanup allocations held by a migration record if we delete a server while it's in VERIFY_RESIZE status, but my recreate test isn't failing for that, and i don't see where in the code we'd cleanup allocatoins for migration records when deleting a server20:07
cdentmriedem: I told someone internally to make a bug about that recently20:09
mriedemit came to mind in the ML thread with mnaser about leaked/incorrect allocations20:09
mriedembut can't seem to reproduce it20:09
cdent(upstream bug) but guess they didn't.20:09
cdentI think they may have convinced themselves it wasn't happening. I'll see if I can find a reference20:09
mriedemi can't see how it *can't* be happening20:10
mriedemif you delete while in VERIFY_RESIZE status20:10
openstackgerritEric Fried proposed openstack/nova master: Remove LazyLoad of Scheduler Clients  https://review.openstack.org/61564120:10
efriedbauzas, johnthetubaguy, cdent: I couldn't find a circular import, but I guess CI will tell ^20:11
mnaserI’m curious if deleting an instance in ERROR state after failed live migrate or resize doesn’t delete it too20:12
mriedemas far as i can tell, for a resize, we only cleanup the allocations held by the migration record when confirming the server (we delete the source node allocations held by the migration) or on revert we swap the allocations held by the migratoin record on the source node with the instance consumer and drop the allocations held by the instance on the target node20:12
mriedemmnaser: we should always cleanup allocations held by at least the instance, even if error state, either on the compute or in the api (if the compute is down)20:13
mriedemi'm more worried that we're leaking allocations held by the migration record20:13
melwittyeah, we should be ok on the delete in error state case (the local delete path) will take care of the instance allocation20:14
melwittbut I agree, I'm not seeing where we take care of the migration related allocations20:14
*** maciejjozefczyk has joined #openstack-nova20:14
mriedemi do not see where _rollback_live_migration cleans up allocations held by the migration record20:14
mnaserI’ve definitely seen it log the rollback message20:15
mnaserin terms of removing destination allocation20:15
mriedemoh there it is for rollback20:19
*** jmlowe has quit IRC20:19
mriedemhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L679620:20
mriedemwhere i'd expect migration allocations to get cleaned up is here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L76120:20
mriedemon server delete i mean20:20
*** maciejjozefczyk has quit IRC20:21
melwittyeah. guess VERIFY_RESIZE is the only case where we'd have the situation of having outstanding migration allocations? but you said you are trying to recreate the bug and not seeing outstanding migration allocations when the instance is in VERIFY_RESIZE :\20:23
mriedemcorrect20:24
mriedemi can push the test up20:24
mriedemmaybe my test is busted20:24
melwittI'm wondering if there's maybe something different about how migrations are handled in a functional test environment (like is anything being faked in a way that covers it up?). but I thought we've been able to demonstrate allocation cleanup bugs with func tests before20:26
cdentmriedem: you could do a pure api driven integration test as a child of https://review.openstack.org/#/c/613386/ pretty easy/quick20:27
openstackgerritLance Bragstad proposed openstack/nova master: WIP: experiment with oslo.limit interface  https://review.openstack.org/61560220:27
mriedemdoes that have 2 nodes?20:27
mriedem$.hypervisors.`len`: 120:28
mriedemnope20:28
cdentoh yeah, that20:29
mriedemresize to same host will also create migration-based allocations (that's a separate bug)20:29
mriedemso it's still probably doable with your thing, but what i've got (in functional tests with python) is easier/faster for me20:29
cdentthis was in response to melwitt suggesting that there was some change that functional might be doing something odd20:29
mriedemthe functional tests assert that the source node contains the migration allocations after the resize,20:30
mriedemso i think they are ok,20:30
mriedemthey just aren't asserting the migration allocations are removed after the server is deleted20:30
lbragstadmelwitt jaypipes johnthetubaguy https://review.openstack.org/#/c/615643/ is a quick stab at the oslo.limit changes we talked about (sans requiring a context manager in the initial implementation)20:30
mriedemnor do i see tests that delete the server while it's in VERIFY_RESIZE state20:30
lbragstadhttps://review.openstack.org/#/c/615602/ is a nova patch based on johnthetubaguy's that tries to use the new changes20:31
melwittcool lbragstad20:34
openstackgerritMatt Riedemann proposed openstack/nova master: Add functional test to delete a server while in VERIFY_RESIZE  https://review.openstack.org/61564420:35
jaypipeslbragstad: I'm not really following the oslo.limits part, frankly...20:35
jaypipeslbragstad: one of the parts that is incredibly confusing and frustrating about the current nova quota engine is its coupling of limits with usage into the same objects (what it calls a "Quota")20:35
openstackgerritEric Fried proposed openstack/nova master: SIGHUP n-cpu to refresh provider tree cache  https://review.openstack.org/61564620:36
jaypipeslbragstad: I was thinking that oslo.limits would stick to the limits stuff and stay out of the usage bits.20:36
jaypipeslbragstad: in short, I don't think oslo.limits needs an Enforcer thing.20:36
melwittjaypipes: I had the same frustration and it didn't occur to me to rename things as you have :P20:38
melwittI've been pedantic on any new code comments or discussions we've had on the difference between limits and usage20:38
*** jmlowe has joined #openstack-nova20:39
melwittbut, I think with oslo.limit the idea was to try and abstract away as much as possible and let it handle the hierarchical enforcement, and all you do is provide it a callback20:41
*** awaugama has quit IRC20:44
lbragstadoh - sure...20:46
lbragstadi can see where that can get muddy20:46
lbragstadi guess the only reason we have usage in the oslo.limits stuff is because we need to calculate enforcement, which checks what users are asking for against what they already have20:47
lbragstadand to melwitt's point, the callback is what oslo.limit is relying on for the usage of X in a given project20:50
melwittI think what jaypipes is saying is that it's not technically required for oslo.limit to take care of enforcement, since enforcement could be done in projects in whatever way they want. but I thought oslo.limit was also trying to provide a common and simple interface for every project to use and let the callbacks be the only pieces each project would have to write separately. and otherwise use the common oslo.limit interface20:54
mriedemi think i found what cleans up the migration-based allocation during server delete of a VERIFY_RESIZE server20:54
mriedemhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L209620:55
melwittmriedem: a-ha! nice find. what a sneaky code20:56
melwittnever knew resizes were confirmed right before deleting20:56
melwittmakes sense though20:56
jaypipeslbragstad: I still don't see why oslo.limits needs to "calculate enforcement".20:58
melwittI don't think it "needs to" but if it does, it makes it so projects don't have to implement the same thing separately. they just have to write callbacks20:58
melwittmaybe that commonized code is minimal though, which I guess is your point20:59
lbragstadyeah - i think we were operating under the assumption that service would give oslo.limit a project_id + resource type and oslo.limit would return a yes or no based on the hierarchy and usage20:59
lbragstadbecause we didn't want to make service re-implement logic to understand this complex project hierarchy21:00
lbragstadservices*21:00
lbragstadbut - putting that aside... how do you see nova's interaction with oslo.limit jaypipes?21:01
openstackgerritMatt Riedemann proposed openstack/nova master: Add functional test to delete a server while in VERIFY_RESIZE  https://review.openstack.org/61564421:01
jaypipeslbragstad: I see oslo.limit as basically the client for Keystone's GET /limits API.21:03
jaypipeslbragstad: and maybe the definition of some common objects. that's about it.21:03
lbragstadok21:04
lbragstadso - to be clear, you except oslo.limit to still handle the tree of projects from keystone and their respective limits?21:04
lbragstadexpect*21:04
*** erlon has quit IRC21:13
*** maciejjozefczyk has joined #openstack-nova21:16
*** awaugama has joined #openstack-nova21:18
jaypipeslbragstad: I don't necessarily think so, no...21:21
*** maciejjozefczyk has quit IRC21:21
jaypipeslbragstad: all I need is a flattened dict of project limit amounts for a set of resource types21:21
jaypipeslbragstad: I figured Keystone would do the needful when it came to flattening the returned dict of resource type to limit amount.21:22
jaypipeslbragstad: still trying to finish up this code. I hope things will be clearer with the last patch in this series.21:22
lbragstadack21:22
lbragstadwe kinda do that already - http://specs.openstack.org/openstack/keystone-specs/specs/keystone/rocky/strict-two-level-enforcement-model.html#fetching-project-hierarchy21:23
lbragstadwhich was required for the limit work21:23
*** itlinux has joined #openstack-nova21:23
*** tbachman has joined #openstack-nova21:34
*** markmcclain has quit IRC21:42
openstackgerritMatt Riedemann proposed openstack/nova-specs master: Add support for emulated virtual TPM  https://review.openstack.org/57111121:43
*** ivve has quit IRC21:51
*** k_mouza has joined #openstack-nova21:52
cfriesenmriedem: thanks for that tweak ^21:56
*** awaugama has quit IRC22:02
openstackgerritEric Fried proposed openstack/nova master: WIP: update_from_provider_tree: fast fail, clear cache  https://review.openstack.org/61567722:10
mriedemefried: cdent: jaypipes: have we found a compromise on https://review.openstack.org/#/c/570847/10/nova/rc_fields.py@45 ?22:11
mriedemNET_BW_EGR_KILOBIT_PER_SEC and NET_BW_IGR_KILOBIT_PER_SEC ?22:12
cdentmriedem: ooph, I had managed to forget about that22:14
mriedemi'm rebasing his series so if we are cool with that naming then i'll make the change22:14
* cdent is rereading22:15
*** markmcclain has joined #openstack-nova22:17
cdentdid we consider NET_BW_EGRESS_KBPS or is that too ambiguous for bits and bytes? (although historicall *bps has always been bits?)22:17
cdentmriedem: I don't have a huge opinion on that actual name, just that the symbol and the string in the canonical location be the same thing22:18
cdentgiven the canonical location is going to change soonish *shrug*22:18
mriedemgibi mentioned the bits vs bytes thing22:19
mriedem"I needed to include the unit of the resource as it is not trivial and as  the name needs to be all upper case the bit and byte difference (the  capital B in KB means byte officially) in the unit cannot be expressed  if abbreviated."22:20
mriedemi'll just change to NET_BW_EGR_KILOBIT_PER_SEC and NET_BW_IGR_KILOBIT_PER_SEC so we can shit and get off the pot22:21
cdentwfm22:22
efriedsince I think the discussion is all my fault, I might as well weigh back in.22:30
efriedI don't see why the const has to be the same as the string.22:30
efriedBut jaypipes has a point that you can always alias it if you need to.22:30
efriedSo effit, we can just go back to the original names and suck up literally half the line width Python allows us.22:30
mriedemno backsies now22:32
mriedemi've already changed it22:32
efriedidgas22:39
*** munimeha1 has quit IRC22:42
*** itlinux has quit IRC22:45
openstackgerritMatt Riedemann proposed openstack/nova master: Add request_spec.RequestGroup versioned object  https://review.openstack.org/56884022:47
openstackgerritMatt Riedemann proposed openstack/nova master: Add requested_resources field to RequestSpec  https://review.openstack.org/56726722:47
openstackgerritMatt Riedemann proposed openstack/nova master: Add bandwidth related standard resource classes  https://review.openstack.org/57084722:47
openstackgerritMatt Riedemann proposed openstack/nova master: Transfer port.resource_request to the scheduler  https://review.openstack.org/56726822:47
openstackgerritMatt Riedemann proposed openstack/nova master: Reject interface attach with QoS aware port  https://review.openstack.org/57007822:47
openstackgerritMatt Riedemann proposed openstack/nova master: Reject networks with QoS policy  https://review.openstack.org/57007922:47
*** mvkr has joined #openstack-nova22:51
mriedemUH OH http://logs.openstack.org/47/615347/5/check/tempest-full/bf73d04/controller/logs/screen-n-sch.txt.gz#_Nov_05_20_39_53_55440823:01
mriedemhttp://logstash.openstack.org/#dashboard/file/logstash.json?query=message%3A%5C%22AllocationUpdateFailed%3A%20Failed%20to%20update%20allocations%20for%20consumer%5C%22%20AND%20message%3A%5C%22Error%3A%20another%20process%20changed%20the%20consumer%5C%22%20AND%20message%3A%5C%22after%20the%20report%20client%20read%20the%20consumer%20state%20during%20the%20claim%5C%22%20AND%20tags%3A%5C%22screen-n-sch.txt%5C%22&from=10d23:02
efriedmriedem: Tell me we have a retry in place for that23:08
*** lbragstad has quit IRC23:09
*** lbragstad has joined #openstack-nova23:10
efriedmriedem: Is the code path for is_new_compute_node guaranteed to run only on startup?23:13
*** mlavalle has quit IRC23:17
*** mriedem has quit IRC23:20
openstackgerritEric Fried proposed openstack/nova master: Remove LazyLoad of Scheduler Clients  https://review.openstack.org/61564123:56
openstackgerritEric Fried proposed openstack/nova master: SIGHUP n-cpu to refresh provider tree cache  https://review.openstack.org/61564623:56
openstackgerritEric Fried proposed openstack/nova master: WIP: Reduce calls to placement from _ensure  https://review.openstack.org/61567723:56

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!