Friday, 2021-07-23

gmanngibi: stephenfin any plan to cut intermediate release for placement ? that will unblock converting the oslo warnings to error https://review.opendev.org/c/openstack/oslo.policy/+/80192900:18
*** bhagyashris__ is now known as bhagyashris04:41
gibigmann: intermediate release from oslo.policy?07:05
bauzasgood morning folks07:17
bauzasgibi: saw your email about the CI issue07:18
bauzasgibi: thanks for following it07:19
gibibauzas: good morning07:29
gibibauzas: it was a full gate block for nova, so it needed to be resolved07:29
bauzasthat's what I saw07:29
bauzasbut the fix is merged, right?07:29
bauzasoh, a new email :)07:30
gibiyepp gtema fixed it while we slept07:30
*** rpittau|afk is now known as rpittau07:35
gibisean-k-mooney: when you are up, melwitt holds her +A on the PR re-parenting patch waiting for your re-review. https://review.opendev.org/c/openstack/placement/+/78402007:44
gibisean-k-mooney: so if you have time then a quick look would be appreciated07:44
opendevreviewFelix Huettner proposed openstack/nova master: Fix duplicate BDMs in compute manager  https://review.opendev.org/c/openstack/nova/+/80199008:48
opendevreviewChristian Rohmann proposed openstack/placement master: Fix SQL query counting the number of individual consumers having allocations by only selecting the aggregated consumer_id column.  https://review.opendev.org/c/openstack/placement/+/80141909:27
sean-k-mooneygibi: a sorry about that was not looking at irc. ill be away for then next 30 mins but ill get it when im back10:50
gibisean-k-mooney: thanks!10:50
sean-k-mooneyok you successfully nerd sniped me i did the review so +1 ok now ill be away for 30 mins :)11:06
gibisean-k-mooney: thanks a lot11:07
opendevreviewLee Yarwood proposed openstack/nova master: WIP: Add functional test for duplicate bdms  https://review.opendev.org/c/openstack/nova/+/80201111:50
*** mgoddard- is now known as mgoddard12:18
sean-k-mooneybauzas: can you prioritiese reviewing https://review.opendev.org/c/openstack/nova/+/797428/2 i would like to see if we can get that landed this week so we can start backporting it12:28
bauzasack12:28
bauzashmpf, a os-vif change12:29
bauzasI'll try12:29
sean-k-mooneywe were waitign for it a nova one12:29
sean-k-mooneyits fixing a bug i had in my patch that always delectas pluging to os-vif for ovs12:29
sean-k-mooneywe were waitign for that pacth to pass ci before mergeing the proceeding patch https://review.opendev.org/c/openstack/nova/+/797142/1 since we wanted to merge them at the same time12:31
bauzassean-k-mooney: I trusted you as I said in the comment : https://review.opendev.org/c/openstack/nova/+/79742812:38
sean-k-mooneythanks i was adding you since your revied the preceeding patch12:39
sean-k-mooneymind adding +w to https://review.opendev.org/c/openstack/nova/+/797142 as well12:39
opendevreviewFelix Huettner proposed openstack/nova master: Fix duplicate BDMs in compute manager  https://review.opendev.org/c/openstack/nova/+/80199012:39
sean-k-mooney* pinging you12:43
opendevreviewKashyap Chamarthy proposed openstack/nova master: Deprecate support for floppy drives in 'Xena'  https://review.opendev.org/c/openstack/nova/+/80202212:53
opendevreviewLee Yarwood proposed openstack/nova master: Add functional test for bug 1937375  https://review.opendev.org/c/openstack/nova/+/80201113:18
opendevreviewLee Yarwood proposed openstack/nova master: compute: Avoid duplicate BDMs during reserve_block_device_name  https://review.opendev.org/c/openstack/nova/+/80199013:18
opendevreviewStephen Finucane proposed openstack/nova master: tests: Speed up 'servers' API tests  https://review.opendev.org/c/openstack/nova/+/77873213:43
*** rpittau is now known as rpittau|afk14:13
opendevreviewStephen Finucane proposed openstack/nova master: Remove redundant service version check  https://review.opendev.org/c/openstack/nova/+/76819514:29
opendevreviewStephen Finucane proposed openstack/nova master: tests: Use correct microversion for server group tests  https://review.opendev.org/c/openstack/nova/+/76819614:29
stephenfinYo, anyone care to look at these two scheduler-related functional tests? bauzas perhaps? https://review.opendev.org/q/topic:%22scheduler-filter-tests%22+(status:open%20OR%20status:merged)14:30
gibistephenfin: I will try before my vacation 14:31
bauzassure, I can try to look14:31
gibistephenfin: one nit in https://review.opendev.org/c/openstack/nova/+/754115/1/nova/tests/functional/test_scheduler.py#10414:39
bauzasstephenfin: you misses a few tests14:48
bauzasbut thanks for the fish14:48
sean-k-mooneyis it me or does this debug log not make sense https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L1032-L103314:49
sean-k-mooneyif we take the else branch we are not applying any migration context14:50
sean-k-mooneylet alone one that does not belog to this instance14:50
gibistephenfin: also one more nit in https://review.opendev.org/c/openstack/nova/+/754116/114:53
gibisean-k-mooney: I guess it try to say that no migration context is set on the instance14:54
sean-k-mooneyright that would make sense14:54
sean-k-mooneyi might submit a 2 liner patch to adress that14:55
gibi:)14:55
sean-k-mooneythe code is form 6 years ago so its possibel that the function signiture has changeed and it was possibel before or something like that and the message just never got fixed14:55
sean-k-mooneyre reading it i can see the interpertation you are suggesting just looking at it in the nova logs it looks odd14:57
gibiI looked at the blame for that reason but I don't see signature changes14:57
stephenfinbauzas: RE: your comment here https://review.opendev.org/c/openstack/nova/+/754116/1/nova/tests/functional/test_scheduler.py#150, is that a thing?15:04
stephenfinthe code says no15:05
stephenfinI don't see us stripping the prefix from the metadata properties15:05
opendevreviewSamuel proposed openstack/nova-specs master: Migrate Instance Between Projects  https://review.opendev.org/c/openstack/nova-specs/+/80203415:05
bauzasdid I miss to provide the URL ? my bad15:05
bauzashttps://github.com/openstack/nova/blob/3508263f236ea6003a76661b9e657ded4d46b413/nova/scheduler/filters/aggregate_instance_extra_specs.py#L55-L5815:05
bauzasstephenfin: this both doesn't verify keys with not the prefix, but also strips the key if the prefix is right15:06
stephenfinit does that for the flavor though15:06
stephenfinnot the aggregate15:06
bauzasoh, whoops, you're absolutely right15:07
stephenfinI don't want to test the unprefixed variant since that behavior is deprecated and would be rejected with recent microversions15:07
bauzasit's a flavor key, not a metadata extra spec15:07
stephenfinyeah15:07
bauzasstephenfin: ah15:07
bauzasdid we deprecated non-prefixed keys ?15:08
sean-k-mooneybauzas: we did i think15:08
sean-k-mooneybauzas: ut the filter still check them15:08
opendevreviewSamuel proposed openstack/nova-specs master: Migrate Instance Between Projects  https://review.opendev.org/c/openstack/nova-specs/+/80203415:08
stephenfinsort of15:08
stephenfinthe docs tell you not to use them, but there's no in-code warning15:09
bauzasmy brain fucked somehow15:09
sean-k-mooneywe deprecated it because you cant use the aggret extra specs filter and compute capablities filter at the same time if you use unprefixed keys15:09
bauzassean-k-mooney: sure, I remember why we have this prefix15:09
bauzasbut I thought we only *documented* to use it if you were using both filters15:09
bauzasnot if you were only using one15:09
stephenfinMultiple values can be given, as a comma-separated list. For backward compatibility, also works with non-scoped specifications; this action is highly discouraged because it conflicts with ComputeCapabilitiesFilter filter when you enable both filters.15:09
stephenfinfrom way back in Mitaka https://docs.openstack.org/mitaka/config-reference/compute/scheduler.html15:10
stephenfin"highly discouraged" is pretty strong :)15:10
bauzasokay, so this is docs15:10
stephenfinI can add the tests if you really want them though?15:10
sean-k-mooneyyes but with the intorctution of the flavor vlaidation it should be even more discuraged15:10
bauzasthe problem is that I know more ops that use this filter than the computecaps one15:10
bauzasas the computecapabilities one is pretty useless now we have traits15:11
sean-k-mooneybauzas: it can do thing tratis cant15:11
bauzasstephenfin: as you wish, I won't both15:11
bauzasbother15:11
bauzasstephenfin: you added code, you proved me I was wrong15:11
bauzasso that's a good start15:11
stephenfinwell I've done it now15:12
stephenfinso you're getting the tests15:12
stephenfinand you'll be happy with them15:12
sean-k-mooneyfor what its worth i suggest we require custom: as a prefix when we were adding the flavor extra spec validation15:12
opendevreviewStephen Finucane proposed openstack/nova master: Add tests for 'AggregateImagePropertiesIsolation'  https://review.opendev.org/c/openstack/nova/+/75411515:12
opendevreviewStephen Finucane proposed openstack/nova master: Add tests for 'AggregateInstanceExtraSpecsFilter'  https://review.opendev.org/c/openstack/nova/+/75411615:12
stephenfingibi, bauzas: comments addressed ^15:12
bauzasstephenfin: looking again, then15:13
stephenfinI'm pretty sure I had a fairly thorough reworking of the scheduler filter docs at some point. I wonder if that merged?15:14
* stephenfin looks15:14
stephenfinnope https://review.opendev.org/c/openstack/nova/+/77364515:14
gibiI'm +2 on both test patch. thanks stephenfin 15:16
stephenfinthank you15:16
bauzassent to the gate, then15:17
bauzas.... and, given this, folks, \o15:17
gibibauzas: o/, please remember I'm off next week, and the meeting chair is your :D15:17
sean-k-mooneystephenfin: there are a few patches before that can it be pulled forward or should we try and merge the others first15:19
sean-k-mooneythe docs patch that is https://review.opendev.org/c/openstack/nova/+/77364515:19
stephenfinI'd say the latter. The docs changes assume the filter scheduler is the only one left, which requires the preceding changes15:19
sean-k-mooneyack ok ill start form the bottom so15:19
bauzasgibi: oh, I remember this, but I haven't said "good vacations" to you :p15:20
bauzasgibi: fwiw, I'll be on PTO between Aug 9 and Aug 3015:21
bauzas... just before end of Xena-3 :)15:21
gibibauzas: thanks15:22
*** akekane_ is now known as abhishekk15:40
opendevreviewBalazs Gibizer proposed openstack/nova master: Support move ops with extended resource request  https://review.opendev.org/c/openstack/nova/+/80008716:06
opendevreviewBalazs Gibizer proposed openstack/nova master: [func test] refactor interface attach with qos  https://review.opendev.org/c/openstack/nova/+/80008816:06
opendevreviewBalazs Gibizer proposed openstack/nova master: Support interaface attach / detach with new resource request format  https://review.opendev.org/c/openstack/nova/+/80008916:07
opendevreviewBalazs Gibizer proposed openstack/nova master: [func test] move unshelve test to the proper place  https://review.opendev.org/c/openstack/nova/+/79362116:10
opendevreviewBalazs Gibizer proposed openstack/nova master: Support boot with extended resource request  https://review.opendev.org/c/openstack/nova/+/80008616:41
opendevreviewBalazs Gibizer proposed openstack/nova master: Support move ops with extended resource request  https://review.opendev.org/c/openstack/nova/+/80008716:41
opendevreviewBalazs Gibizer proposed openstack/nova master: [func test] refactor interface attach with qos  https://review.opendev.org/c/openstack/nova/+/80008816:43
opendevreviewBalazs Gibizer proposed openstack/nova master: Support interaface attach / detach with new resource request format  https://review.opendev.org/c/openstack/nova/+/80008916:45
opendevreviewBalazs Gibizer proposed openstack/nova master: [func test] move unshelve test to the proper place  https://review.opendev.org/c/openstack/nova/+/79362116:46
opendevreviewBalazs Gibizer proposed openstack/nova master: WIP support extended res req in heal port allocation  https://review.opendev.org/c/openstack/nova/+/80206016:48
opendevreviewBalazs Gibizer proposed openstack/nova master: Support move ops with extended resource request  https://review.opendev.org/c/openstack/nova/+/80008717:27
opendevreviewBalazs Gibizer proposed openstack/nova master: [func test] refactor interface attach with qos  https://review.opendev.org/c/openstack/nova/+/80008817:28
opendevreviewBalazs Gibizer proposed openstack/nova master: Support interaface attach / detach with new resource request format  https://review.opendev.org/c/openstack/nova/+/80008917:30
opendevreviewBalazs Gibizer proposed openstack/nova master: [func test] move unshelve test to the proper place  https://review.opendev.org/c/openstack/nova/+/79362117:30
* gibi leaves for a week of vacation17:30
opendevreviewBalazs Gibizer proposed openstack/nova master: WIP support extended res req in heal port allocation  https://review.opendev.org/c/openstack/nova/+/80206017:32
gmanngibi: ah I thought placement is in cycle-with-intermediary model but it is cycle-with-rc so need to wait for rc release.17:45
opendevreviewSamuel proposed openstack/nova-specs master: Migrate Instance Between Projects  https://review.opendev.org/c/openstack/nova-specs/+/80203417:47
opendevreviewSamuel proposed openstack/nova-specs master: Migrate Instance Between Projects  https://review.opendev.org/c/openstack/nova-specs/+/80203417:48
opendevreviewSamuel proposed openstack/nova-specs master: Migrate Instance Between Projects  https://review.opendev.org/c/openstack/nova-specs/+/80203417:50
gmanngibi: stephenfin in that case we will release placement in rc1 right? which is after oslo feature freeze. with that we have to wait for one more cycle to get  https://review.opendev.org/c/openstack/oslo.policy/+/801929 in oslo.policy ?17:57
gmannI think that is how we handle such situation ?17:57
sean-k-mooneyto do a release of placement yes we wait until m3 and then can release a release candiate for placmeent18:00
sean-k-mooneyeven if placment was cycle with intermediary it woudl not affect the oslo patch really18:01
sean-k-mooneygmann: why do you think it would?18:01
gmannsean-k-mooney: we have to release placement first in pypi for fix done for proposed interface changes in oslo 18:02
gmannotherwise placement in pypi fail with the latest oslo release which include the interface change18:02
sean-k-mooneywotn we have the same issue with nova18:03
sean-k-mooneyor keystone18:03
sean-k-mooneyor any other project that are part of the integrated release18:03
gmanntechnically yes but placement is used from pypi in nova tests so it block gate itself18:03
sean-k-mooneywe wont be able to increase upper constirting on master18:03
sean-k-mooneyyou mena placment lib is used18:04
sean-k-mooneyi dont think we use placement directly18:04
gmannif any using repo we use form pypi in gate testing then it would let oslo change merge itself, like placement18:04
sean-k-mooneythe placment fixture shoudl be in placment-lib18:04
sean-k-mooneyeven if placment was release with intermiday it would not be correct to do a release in this case18:06
gmannits in nova fixture18:06
gmannno, with intermediate release model we can release new version of placement anytime in release where code moved to new interface from oslo18:07
gmannso oslo gate would not complain 18:07
sean-k-mooneygmann: we coudl but we should not18:07
sean-k-mooneyit is like release nova early18:08
gmann:) yeah. i am not saying we should change release model18:08
sean-k-mooneygmann: we can merge the oslo change but pin the oslo verion until after xena18:09
gmannI am saying with rc release model, we have to wait lib changing interface for at least one cycle.18:09
sean-k-mooneybut that could cause other isseu18:09
gmannwe cannot merge until we change the usage of placement form master not from pypi18:09
sean-k-mooneygmann: right i am saying the right thing to do would be to wait regardless18:09
gmannyes, at least 1 cycle18:09
sean-k-mooneythe other way is to not chnage the interface18:10
sean-k-mooneyand to support both in parallel18:10
sean-k-mooneydeprecating the old behavior18:10
sean-k-mooneyso add a new parmater to rule default18:11
sean-k-mooneye.g. deprecated_rule_action=warn|raise18:11
sean-k-mooneyhave that parmater default to warn18:11
gmannits after deprecation things not fresh change, warn->error18:11
sean-k-mooneythen next cycle change it to raise18:12
sean-k-mooneyno this is a fresh change18:12
sean-k-mooneyyou care chanign the behavior of oslo_policy18:13
gmannno18:13
sean-k-mooneyso tha tdeprecation become an error18:13
sean-k-mooneyyes that is the api change 18:13
sean-k-mooneyhttps://review.opendev.org/c/openstack/oslo.policy/+/801929/2/oslo_policy/policy.py#124318:13
gmannit is, already deprecated in previous cycle and this cycle changing warning to error18:13
sean-k-mooneyno18:13
sean-k-mooneythe policy rule is18:14
sean-k-mooneybut here we are changing the behavior of the RuleDefault class18:14
sean-k-mooneythe fact a project policy rule is deprecated does not mean the RuleDefault class usage is deprecated18:14
sean-k-mooneyhttps://review.opendev.org/c/openstack/oslo.policy/+/801929 is a new oslo.polify feature18:15
sean-k-mooneywhich is a breaking change18:15
sean-k-mooneyso this need a oslo.policy major viersion bump since its not a additive change18:15
gmannyes, major version bump18:16
sean-k-mooneyyou can make it an addtivie chagne rather simply though18:16
sean-k-mooneyadd a new parmater to the contructor of RuleDefault18:16
gmannwe did that in past via warning, not I am proposing to do a breaking change and remove the old ay18:16
gmann*now I18:17
sean-k-mooneyright i am saying that is not a valid stragy18:17
gmannhumm, so keep supporting old way always?18:17
gmannand keep warnings also?18:17
sean-k-mooneyno you need to wait until all or most of your users have adapted to the new api18:18
gmannwe introduced warning becasue we want usage to mvoe to new way right18:18
sean-k-mooneywhen was the warnign intoduced18:18
gmannsean-k-mooney: I did https://review.opendev.org/q/topic:%22fix-oslo-policy-warnings%22+(status:open%20OR%20status:merged)18:18
gmannhttp://lists.openstack.org/pipermail/openstack-discuss/2021-July/023646.html18:18
sean-k-mooneybasically i dont think you shoudl do this type of change late in the cycle18:18
gmann^^ has all the details18:18
gmannwe cannot do in this cycle, we have to wait for next cycle due to dependencies 18:19
sean-k-mooneyi guess the issue is just the unit tests18:19
sean-k-mooneycan you show me where in nova we are importign placment18:19
opendevreviewSamuel proposed openstack/nova-specs master: Migrate Instance Between Projects  https://review.opendev.org/c/openstack/nova-specs/+/80203418:19
gmannhttps://github.com/openstack/nova/blob/master/nova/tests/functional/fixtures.py#L1718:19
gmannsean-k-mooney: ^^18:19
sean-k-mooneyplacement is not in our test requirements18:19
sean-k-mooneyim trying to figure out where it is listed18:20
sean-k-mooneyhere https://github.com/openstack/nova/blob/master/tox.ini#L9718:20
gmannyes18:21
sean-k-mooneywe should not be importing the fixtuer the way we currently are18:21
sean-k-mooney... ok18:22
sean-k-mooneythat explians it we still have not create placment-lib18:22
sean-k-mooneygmann: we were ment to move the fixture and other common code in a placement-lib repo cycle or two ago18:23
sean-k-mooneylooks like that never happened18:23
gmannyeah18:23
sean-k-mooneyit was to aovid a direct import of placment so we could release the lib ahead of the main release and avoid this18:24
sean-k-mooneythis is kind of tricky18:25
sean-k-mooneythe best way to adress this would be to pin the oslo.plolicy version in nova func test until after placment is released18:26
sean-k-mooneywhich i think we could do with oslo.policy<=${last version without error} here https://github.com/openstack/nova/blob/master/tox.ini#L9618:27
gmannwe should not actually, and keep testing with latest oslo otherwise we can get more surprise. 18:28
gmanneither moving placement fixture to lib side or do like current way 'wait for 1 cycle in oslo side'18:28
sean-k-mooneywell the other way is to deploy placment form master18:28
sean-k-mooneyon master18:28
gmannyeah that too work18:29
sean-k-mooneywe coudl replace openstack-placement>=1.0.0 with -e git+https://opendev.org/openstack/placement.git@master#egg=placement18:29
sean-k-mooneyok lets pick this up next weeks and see if thre is a way forward18:31
gmannactually we should keep testing with released placement and if we want we can do with master version also so that we test both way. 18:35
gmannfor oslo, I feel (from other usage also with pypi released version) is to wait for next cycle. This cycle I fixed almost all the usage in once they are released with oslo version supporting both way then 2, in next cycle remvoe old style in oslo18:36
gmannthat is best way to introduced the breaking change or major version bumo18:36
gmannbump18:36
gmanndoing both in one cycle can break or mixup the things 18:37
opendevreviewSamuel proposed openstack/nova-specs master: Migrate Instance Between Projects  https://review.opendev.org/c/openstack/nova-specs/+/80203419:17
opendevreviewSamuel proposed openstack/nova-specs master: Migrate Instance Between Projects  https://review.opendev.org/c/openstack/nova-specs/+/80203419:59
opendevreviewSamuel proposed openstack/nova-specs master: Migrate Instance Between Projects  https://review.opendev.org/c/openstack/nova-specs/+/80203420:12
opendevreviewMerged openstack/placement master: Add support for RP re-parenting and orphaning  https://review.opendev.org/c/openstack/placement/+/78402020:22
opendevreviewmelanie witt proposed openstack/placement master: Add consumer_types migration, database and object changes  https://review.opendev.org/c/openstack/placement/+/66917020:35
opendevreviewmelanie witt proposed openstack/placement master: Microversion 1.38: API support for consumer types  https://review.opendev.org/c/openstack/placement/+/67944120:35
opendevreviewmelanie witt proposed openstack/placement master: Switch ConsumerType to use an AttributeCache  https://review.opendev.org/c/openstack/placement/+/67948620:35
opendevreviewSamuel proposed openstack/nova-specs master: Migrate Instance Between Projects  https://review.opendev.org/c/openstack/nova-specs/+/80203420:36
opendevreviewSamuel proposed openstack/nova-specs master: Migrate Instance Between Projects  https://review.opendev.org/c/openstack/nova-specs/+/80203420:52
opendevreviewSamuel proposed openstack/nova master: Add REST and compute APIs to migrate instances between projects  https://review.opendev.org/c/openstack/nova/+/80137721:11
opendevreviewmelanie witt proposed openstack/nova master: DNM testing  https://review.opendev.org/c/openstack/nova/+/80213321:53
opendevreviewmelanie witt proposed openstack/nova master: DNM testing  https://review.opendev.org/c/openstack/nova/+/80213321:57
opendevreviewmelanie witt proposed openstack/nova master: DNM testing  https://review.opendev.org/c/openstack/nova/+/80213323:02

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