Wednesday, 2021-06-23

*** akekane_ is now known as abhishekk05:51
*** alex_xu_ is now known as alex_xu06:37
*** rpittau|afk is now known as rpittau07:07
lyarwoodmorning \o08:05
*** bhagyashris_ is now known as bhagyashris08:27
gibi\o08:59
fricklerlyarwood: https://bugs.launchpad.net/nova/+bug/1452641 just came up in #openstack-dev , are you still planning to proceed with https://review.opendev.org/c/openstack/nova/+/579004/ ? (changing ceph mon addresses)09:15
lyarwoodfrickler: yeah but not as part of that change anymore, I'm writing up a spec at the moment to provide a set of nova-manage commands to allow operators to do refresh this for SHUTOFF instances09:18
lyarwoodfrickler: the alternative is for users to shelve and unshelve the instances09:18
fricklerlyarwood: well from an operator perspective it would be great to have a solution that it transparent to the users and allows them to keep instances running, but I admit that that may not be achievable09:20
lyarwoodyeah that's pretty hard if impossible09:21
* bauzas goes off for lunch a bit early 09:22
bauzaswill be back around 1130UTC09:22
opendevreviewLee Yarwood proposed openstack/nova master: zuul: Add CentOS 8 stream integrated compute tempest job to gate  https://review.opendev.org/c/openstack/nova/+/79761609:57
sean-k-mooneyfrickler: the best way to do that might just be to alwasy put the ceph monds behind haproxy or a keepalived vrrp vip11:58
sean-k-mooneyfrickler: e.g. do not have the actul mon ips present11:58
sean-k-mooneyhave a sperate one that you can move there instead11:58
gibisean-k-mooney, bauzas: I've left my view about vgpu in https://review.opendev.org/c/openstack/nova-specs/+/78045211:59
bauzasgibi: ack12:00
sean-k-mooney"Does OWNER_CYBORG trait on an RP means that _every_ RC on that RP is managed by Cyborg?" so yes traits apply to all invetories in a RP12:01
sean-k-mooneythat is why traits are on the RP not the inventory12:01
bauzasgibi: so your comment would be about providing only traits for Cyborg VGPUs ? I'm OK if so12:02
sean-k-mooneybauzas: we woudl have to use a forbiden trait on nova if we only do if for cyborg12:02
bauzasthe cyborg-agent could create the RPs with the same RCs 12:02
gibi"Based on these assumptions keeping the VGPU RC for nova usage and creating a new standard or custom RC for cyborg works for me. Personally I vote for a new standard trait for cyborg vGPUs." that is my summary12:02
gibishit12:03
gibi Personally I vote for a new 12:03
gibistandard RC12:03
gibi:D12:03
sean-k-mooneyso standard is where i have a problem with that approch12:03
bauzassean-k-mooney: not, not really12:03
sean-k-mooneyare you really suggesting that every time we want 2 service to manage the same thing that we need service speicifc resouce classes12:04
bauzassean-k-mooney: we could have some kind of pre-filter asking for a non-Cyborg, only if we have configuratin options for cyborg12:05
gibisean-k-mooney: I think we should avoid having two service manage the same resource. (you probably know that I'm against duplicating vgpu logic between nova and cyborg)12:05
sean-k-mooneyright but honestly i dont think vgpus should be in cyborg12:05
sean-k-mooneythey are not a prgramable devce12:05
bauzassean-k-mooney: as we discussed, it looks to me it's a non-mixed resource12:06
bauzasif so, having another RC is understoodf12:06
gibisean-k-mooney: I stopped arguing on either side of moving the logic to cyborg or keeping it to nova only. 12:06
gibisean-k-mooney: that would require one side of the table compromise12:07
gibisean-k-mooney: I have no bacon on either side12:07
gibiso I don't want to force either party to accept the compromise12:08
gibithis means we will have a sitation to manage a single type of resource from two services12:09
gibisiutation12:09
gibiI assume this is a rare situation12:09
sean-k-mooneygibi: well  i have been trying to get redhat to support cyborg since i was still working at intel and its not goning to happen anytime soon as in not before the A release and its not even slated for that presently12:09
sean-k-mooneywell not nessiarly12:10
bauzasmy point is to say : either we mix the same RC or not12:10
sean-k-mooneyi do think there will be other cases but in genral yes12:10
bauzasif we mix, we could use the conductor to know whether we would ask cyborg or not12:10
sean-k-mooneyeach RC tends to be specific ot a service12:10
bauzasif we don't mix, we don't really need to have the same RC12:10
sean-k-mooneycinder is the other example 12:10
sean-k-mooneyit should be modeling its capastiy as disk_gb12:11
sean-k-mooneywhich we use for local disk12:11
* gibi needs to jumpt to an urgent call for 10 mins12:11
sean-k-mooneyack12:11
sean-k-mooneybauzas: gibi  it sound like both of ye want to use a new resouce class so if that is what is requird to move this forward so be it12:12
swp20sean-k-mooney: hi, our cyborg tempest failed with the exception: Reason: 'Query' object has no attribute 'with_lockmode' . please help us with the refer:https://fc9dac502ce26d84f9de-05fc50868e17ec6a804428f62cd7e454.ssl.cf1.rackcdn.com/797427/4/check/cyborg-tempest/bd72fc3/job-output.txt12:13
sean-k-mooney that might be form here https://github.com/openstack/cyborg/blob/ba6a35c67ee2e25cccc693d59cae2d2182aefa58/cyborg/db/sqlalchemy/api.py#L25712:16
sean-k-mooneyswp20: i suspect that it has change in the new version fo sqlacamy12:17
sean-k-mooneyi think the excption is comming form here https://github.com/openstack/cyborg/blob/bb35be1b86953c6df5fd9a300221cd45e359e8ec/cyborg/conductor/manager.py#L150-L158 or here https://github.com/openstack/cyborg/blob/bb35be1b86953c6df5fd9a300221cd45e359e8ec/cyborg/conductor/manager.py#L170-L17512:18
swp20it is because the placement changed the version of sqlalermy?12:18
swp20yeah, there are with_lockmode in cyborg project.12:19
sean-k-mooneyno not placment 12:19
sean-k-mooney Collecting SQLAlchemy===1.4.1812:19
sean-k-mooneythere was a new release of sql alchemy not so log ago12:20
sean-k-mooneyand all the pojects need to adapt too it12:20
sean-k-mooneywe updated to 1.4 2 months ago https://github.com/openstack/requirements/commit/dc86260b283dedc3076d7873f5f031f45e3e367112:21
sean-k-mooneyit looks like that did not check for compatiablity in cyborg12:21
sean-k-mooney"When the Query.with_lockmode() method were deprecated in favor of Query.with_for_update()..."12:25
sean-k-mooneylooks like https://docs.sqlalchemy.org/en/14/orm/query.html#sqlalchemy.orm.Query.with_for_update is the replacemnt12:26
sean-k-mooneyswp20: https://docs.sqlalchemy.org/en/14/orm/query.html#sqlalchemy.orm.Query.with_for_update is the replacment for Query.with_lockmode()12:27
swp20sean-k-mooney: ok, i'll try to update. thanks a lot.12:27
sean-k-mooneyswp20: it looks like it was deprecated in 1.1 or 1.2 they just finally got around to droping it in 1.412:29
swp20sean-k-mooney: thanks. the version in cyborg requirement.txt now is SQLAlchemy>=0.9.0,!=1.1.5,!=1.1.6,!=1.1.7,!=1.1.8 # MIT, i have update, hope this will success.12:32
sean-k-mooneywell you should not be capping the version locally in cyborg12:35
sean-k-mooneythat shoudl be managed via the upper constraitns file in the requirement repos12:35
sean-k-mooneysicne all project are ment to be co installable the upper requrieemtn i manged cerntrally 12:36
sean-k-mooneyso cyborg shoudl be updated to supprot 1.412:36
sean-k-mooneyin doing so your minium requiredm will incerease to 1.212:36
sean-k-mooneyfor the Query.with_for_update method12:36
sean-k-mooneyactuly no12:36
sean-k-mooneyit was added in 0.912:37
sean-k-mooneyswp20: so you dont have to udpated your requiremetn.txt in cyborg12:37
* gibi is back12:38
gibibauzas, sean-k-mooney: yeah I think in this specific case we need to allow both service to manage vgpus and in this specific case it is a lot simpler and cleaner to have separate RCs. 12:38
gibisean-k-mooney: for the cinder case. I think the different there is tha cinder's disk_gb is not consumable by nova but consumable by the cinder backned. And this consumability needs to be modelled 12:39
sean-k-mooneywhat about for generic VFs and PFs for neutron sriov12:39
sean-k-mooneybecause there is work to support cyboprg mandaged sriov vfs also12:40
gibigood point12:40
sean-k-mooneythat are assocated with a nutorn port12:40
sean-k-mooneywe can use a differet resouce class there too12:40
sean-k-mooneybut this pattern has many implications if we repeat it12:41
bauzasagain, same thoughts here12:41
bauzasmixed case:  we need to use the same RC12:41
bauzasnon-mixed case : lgtm for another RC12:41
sean-k-mooneywell its not mixed in that we alwasy iknow if its form nova or cyborg12:41
sean-k-mooneythey will have different vnic types12:41
sean-k-mooneyin this case acclerator_direct vs direct12:42
gibidoes devices allocated for accelerator_direct tracked by nova pci tracker? I think it is not it is tracked by cyborg only12:42
sean-k-mooneyits only tracked by cyborg12:43
sean-k-mooneylike the vgpus12:43
sean-k-mooneyyou could whitelist and track it in the pci tracker if it was stateless12:43
sean-k-mooneycyborg only adds the ablity to flash an image on to it if need or do other stateful things12:44
* gibi thinking hard12:45
sean-k-mooneyin the case of intels current propsal they are only supportin staticly pre programed devices but plan to make it dynmaic later12:45
sean-k-mooneythe current hardware does not manage the capailtie at the VF level, its card wide hence static intially until new hardware is release that is more granular12:46
swp20sean-k-mooney: so what's the main reason of the exception?12:46
gibisean-k-mooney: is it actually the first time that nova needs to handle this situation that both nova and another service tracks the same type of resource? 12:49
gibiin case of accelerator_direct we don't have the placement issue yet, as no PCI devices is modelled in placement yet12:49
gibiso vgpu seems to be a first one when we actually do this12:49
sean-k-mooneyswp20: you are using a function that was removed in 1.4 it was deprecated around 0.9 you shoudl jsut use the replacemnt funciton12:50
sean-k-mooneygibi: yes its the first time since cinder did not modle anything in placment yet12:50
sean-k-mooneywe would have the same issue with disk_GB12:51
gibiand we will have the same issue with VFs one we have smartnic in cyborg and PCI devices in placement12:51
gibionce12:51
bauzassean-k-mooney: gibi: that's why I think we should maybe think about some kind of provider type12:52
bauzaswe couldn't just use traits for owners12:53
bauzasor this would mean that we would have traits for all of the resources we currently have12:53
bauzasor, using another RCs12:54
sean-k-mooneywe proposed having provider type before the trait idea12:54
bauzasthat's the alternative12:54
sean-k-mooneythe only reason we went with the trait was peopel did not want to do the work to extend placment to model that12:54
bauzassean-k-mooney: this was 3 years ago, right?12:54
bauzassean-k-mooney: and people weren't thinking not only about cyborg, right?12:55
sean-k-mooneywhen it was first propsoed yes12:55
sean-k-mooneybut it cam up at the ptg sicne12:55
gibiI think we tend to mix provider type with consumer type, the latter is what was discussed in placement12:55
bauzasI'm pretty sure that if cinder was asking to use the same RCs, then we would think about other alternatives than owner traits...12:55
sean-k-mooneygibi: no we have discussed both12:55
bauzasbut here, see12:56
sean-k-mooneyowner triats was inteded to be a version fo provider types that did not reuqire placment code chagnes12:56
bauzasfor me, if cinder wants to support some RCs, those would be like oranges12:56
bauzaswhile nova looks at apples12:56
sean-k-mooneythen we shoudl get rid of the idea of standtar resouce classes12:56
sean-k-mooneythey have 0 value if they cannot be shared12:56
sean-k-mooneyin fact they have negitive value12:57
gibisean-k-mooney: standard means you predefine them and therefore easily standardize them in the flavor extraspec too12:57
sean-k-mooneythat is not how i view them12:58
sean-k-mooneyif standarising them does not bring interoperatbltiy then we can just standarise them in the code of the project that uses them12:58
sean-k-mooneywithout needing a lib to do that12:58
sean-k-mooneythe project could simple have registered there "standard" resocue classes when they first connect to placment and we could have even namespacee them as the owner if we wanted too12:59
gibisean-k-mooney: you are right I withdraw my the above :)13:00
gibios-resource-classes for shering13:00
gibisharing13:00
sean-k-mooneyso we have a few optiops, we can proceed with new "standard" maybe "shared" is better resocues classes for cyborg devices13:01
gibibut sharing does not make too much sense. At least a disk_gb in cinder backend and a disk_gb in nova local storage are not interchangeable13:01
sean-k-mooneywe can use the same with some other owner mechanium13:01
sean-k-mooneyor we could use custom resouce classes right13:01
sean-k-mooneygibi: sharing requires ownership of the resouce13:01
gibidoesn't sharing means we share ownership? 13:02
bauzassharing vs. sharding13:02
bauzaswe discussed this yesterday13:02
sean-k-mooneygibi: no it means to sue it multiple service both mush supprot tracking the ownwership in placment vai some mechanium13:02
bauzasgibi: agreed on the sharing ownership13:03
gibisharing an RC via os-resource-classes only make sense to me if there are two services and both managing that RC and that RC represents an interchangeable resource regardless of which serivice is reported it13:03
bauzasgibi: if the conductor gets some allocation from a shared resource, it should pass the allocation to the right service13:03
sean-k-mooneygibi: to me that is not what that means13:03
stephenfinPython 3.10 looks pretty sweet. It'll be fun to use that in 5 years or whatever :-D https://lwn.net/Articles/860389/13:04
sean-k-mooneytodate we dodn thave any service that share a common resouce class because we have not modeled ownwersyhip yet13:04
bauzassean-k-mooney: we don't need to model ownership for shared resources13:04
sean-k-mooneystephenfin: it has some nice things yest like that swtich statement based on patern matchin13:04
sean-k-mooneystephenfin: we should be able to bump our min python to 3.8 soon13:04
bauzasif we have same resources, this is conceptually the same13:04
opendevreviewRodolfo Alonso proposed openstack/os-vif master: Make explicit the network backend used in the CI jobs  https://review.opendev.org/c/openstack/os-vif/+/79764013:05
sean-k-mooneystephenfin: im hoping that 3.8 bump happens next cycle13:05
bauzasyou're asking for apples, whether they are provided by a grocery or by something else13:05
sean-k-mooneybauzas: what makes a cyborg vgpu different form a nova one13:05
opendevreviewRodolfo Alonso proposed openstack/nova master: Make explicit the network backend used in the CI jobs  https://review.opendev.org/c/openstack/nova/+/79764113:05
sean-k-mooneythey are identical13:06
bauzassean-k-mooney: if you're asking for a resource that's not the same than an apple, this is not an apple13:06
sean-k-mooneyform a user persective13:06
bauzasthe user sees flavors13:06
sean-k-mooneynot in all cases13:06
bauzashe doesn't see resources13:06
sean-k-mooneythe extra specs are not alwasy visable to users13:07
bauzasI'm done with our "power users"13:07
sean-k-mooneyin fact they might not be visable by default that is contoled by policy13:07
bauzassean-k-mooney: but with cyborg, they don't see VGPUs when looking at the flavors, right?13:07
sean-k-mooneyboth are stored in the flavor as an extra spec13:08
bauzasthey see device profiles, right?13:08
sean-k-mooneye.g. resouce:vgpu or device-profile=whatever13:08
bauzasthis is like ironic13:08
sean-k-mooneyor pci_ailais=my-gpu13:08
bauzasresource:vgpu=1 is only a nova syntax for nova-managed vgpus, right?13:08
gibiflavors with resource:vgpu and flavors with device-profile could be interchangeable from the user perspective if he gets a VM with vGPU passed through in both case13:08
bauzasin the flavor, I mean13:09
bauzasgibi: you're mentioning a mixed usecase13:09
sean-k-mooneybauzas: kind of yes. you could use the sriov based vgpus with pci passthough alias but for mdevs yes13:09
bauzasgibi: where users don't care whether vgpus are offered by nova or cyborg13:09
gibibauzas: sort of yes13:09
sean-k-mooneybauzas: yes but 99% they wont13:10
gibibauzas: I want to figure out which resource is interchangeable13:10
sean-k-mooneycyborg is an mostly admin only api13:10
sean-k-mooneyso normally user wont interact with it13:10
bauzasI think we're boiling the ocean and we need to babystep13:10
bauzas*for the moment* cyborg flavors are using device profiles, right?13:10
sean-k-mooneyyes13:10
gibibauzas: we are struging here as we i) want a baby step but ii) doen't want to create a wrong precedence13:11
bauzasok, so they're conceptually different from nova13:11
gibiprecedent13:11
bauzasgibi: agreed13:11
bauzasgibi: that's why I dislike owner traits tbh13:11
bauzasthis looks to me an horrible hack for a single purpose13:11
sean-k-mooneyits not13:12
bauzassean-k-mooney: this is a hack, because this requires to touch the current modeling we have 13:12
gibiif we go with separate RC now, then we probably copy that for accelerator_direct + PCI in placement, and for disk_gb for cinder + nova too13:12
sean-k-mooneyi spent weeks tryign to come up with another way and that was the only way i coudl get jhone and other to consier moving ti forward13:12
gibiand then I agree with sean that os-resource-classes has no use13:12
bauzasgibi: we can make a consensu13:12
bauzasgibi: or a statement if you prefer13:13
sean-k-mooneybauzas: the current modeling does not fit our needs13:13
bauzasgibi: if you have requirements for scheduling decisions that require your resources to be shared, you have to use other RCs13:13
bauzassharded* 13:13
bauzasand absolutely not shared13:13
bauzasgibi: but if you are OK with having scheduling decisions that accept to mix your resources with other resources, then eventually the conductor (or the scheduling client rather) has to place the request to the right service13:14
sean-k-mooneybauzas: we can supprot that but its a lot more work for the cyborg team13:15
gibibauzas: I think the scheduling decision can be made independently from the service providing the resources, the consumption of the resources on the hypervisor needs the information which service tracks the phyisical resource13:15
sean-k-mooneyare you willing to help them do that correctly13:15
bauzasgibi: agreed with your statement, if you're talking about the "mixed case"13:16
bauzassean-k-mooney: for the cyborg team, they wanna do sharding13:16
gibibauzas: even in a not mixed case you have to claim the resource frome the service it is tracking it13:16
sean-k-mooneybauzas: they only want to use the same resouce class because we told them to do that13:17
bauzasgibi: the claim is the allocation13:17
sean-k-mooneybauzas: we told them to do that to not add service specific standard resouce clases13:17
gibibauzas: nope, there is a claim in placement via the allocation, but you also need to talk to the service providint the resource to be able to use it (e.g. neutron to plug, cyborg to program and provide a pci address or mdev, cinder to provide the volume attachment information)13:18
bauzasgibi: right13:18
* sean-k-mooney wonder if we shoudl even use placement at this point since we cannot agree on how to use it and it has repeatbly been a blocker to adding featrure to openstack13:18
bauzasgibi: this is the conductor which would bind the arq13:19
sean-k-mooneybauzas: it can only do that if we have a device profile13:19
bauzasgibi: I mean, this is the conductor which would call cyborg-agent to bind the arq13:19
sean-k-mooneyif we do not we do not have the info requried to create and bind the arq13:19
bauzassean-k-mooney: correct, but we know the flavor13:19
bauzasfor the mixed case13:19
bauzassay, we ask some flavor with a device profile that's turned into VGPUs13:20
bauzasif this is acceptable to get a host not managed by cyborg, then the conductor won't call the agent to bind13:20
sean-k-mooneythat would be incorrct13:20
bauzasbut if the allocated host is cyborg-managed, then the conductor would have to call the agent13:21
sean-k-mooneyno that is not architeutaly valid13:21
sean-k-mooneythe device profile is not just a resouce calss.13:21
sean-k-mooneyits a resouce class, possibel soem traints and optionaly image to program13:22
sean-k-mooneyif the request comes form a device-porfile it __must__ be fulfiled form cyborg13:22
bauzasyou could end up with some non-cyborg managed host using the same RCs and traits, right?13:22
bauzassean-k-mooney: then, a device profile is ABSOLUTELY AND NECESSARLY a request to shard your cloud13:23
sean-k-mooneytoday yes that is why ownwer traits were bting intoduced to make sure that will not happen13:23
bauzasand for this, I think a different RC is the viable option13:23
gibihm13:23
gibiif a device requested by a device profile is not just a pci resource but some extra capability (e.g. programming)13:24
gibithen we should model that capability in placement13:24
gibiand then the scheduling will be based on capability not based on ownership13:24
sean-k-mooneythat does not work for vgpus13:25
gibiif a cyborg vgpu has same extra feature compared to nova vgpu then we shoud modell that13:25
sean-k-mooneythey are not programable13:25
sean-k-mooneynot in the way fpgas are13:25
sean-k-mooneygpus are fixed function asics withthere own isntrcution set13:25
sean-k-mooneythey are just a co processor not a programble hardwarer device13:25
gibiif the cyborg vgpu totally the same device as a nova vgpu then we should be able not to differentiate between them13:25
sean-k-mooneygibi: they are litrally the same model of nvidia gpu13:26
gibiit is just a vgpu resource nothing more13:26
gibiregardless how you request it13:26
gibiyou as a user get the same vgpu in your vm13:26
bauzasthat's my point13:26
sean-k-mooneythat point is incompatbale with "use different RC to tack ownwership" they are identical form a user point of view so shoudl have the same resouce class13:27
bauzasif a device profile is a set of request groups, OK13:27
gibiso if two service represents non interchangeable resource then the difference in capability needs to be modelled in placement (as a trait) 13:27
gibiif the two service represents the interchangeable resource then we use the same RC and we forbid to deploy a mixed cloud13:27
gibias a mixed could would be undecideable13:28
bauzassean-k-mooney: in this case (the mixed one), this is absolutely OK to get allocation candidates from a device profile that AREN'T cyborg managed hosts13:28
sean-k-mooneygibi: well you could use aggreate for that13:28
gibisean-k-mooney: yeah we can allow that13:28
gibimaybe13:28
sean-k-mooneybauzas: i dont agree 13:29
sean-k-mooneyif that is the case then its absolutely ok to get allcoations form a pci passthough alias form cyborg13:29
sean-k-mooneyor to get mdevs from cyborg13:29
sean-k-mooneywhen using generic mdevs13:30
bauzasagreed, with VGPU standard request, you could end up getting a cyborg managed host 13:30
sean-k-mooneywe can support that but if we do it has to be symetirc13:30
bauzasand I'm OK with this13:30
bauzasbut you said it's more than a standard RC13:31
sean-k-mooneylets take that case13:31
sean-k-mooneyyou have resouce:vgpu113:31
bauzasbut as I said, I'm OK with getting results using an explicit request group that maps a device profile13:31
sean-k-mooneyits alocated form a cyborg RP13:31
sean-k-mooneyhow do you boot that vm13:31
sean-k-mooneybauzas: the device profile is in its own requet group by the way13:32
bauzasthe conductor which gets the candidates would see this is cyborg target and accordingly would ask the agent to bind the arq13:32
sean-k-mooneybauzas: what arq13:32
sean-k-mooneywe just had resouces:vgpu13:32
sean-k-mooneythere is not arq or device type referenced in the flavor13:32
sean-k-mooney*device-profile13:33
sean-k-mooneyand without a device-profile you cannot create an arq13:33
bauzasso, again, that means we CAN'T be scheduler agnostic13:34
bauzasin this case, you're asking for orange13:34
bauzasand not appel13:34
bauzasapple13:34
sean-k-mooneybut we agree that the device the guest sees is idential13:34
sean-k-mooneyso they are apples13:34
gibinova implementation needs a distintion but for the user the result is the same13:35
sean-k-mooneythey just have different requiremetn to consume them13:35
sean-k-mooneygibi: exactly13:35
gibithat is bad :/13:35
sean-k-mooneythis is an internal impleemntaiton detail of how the cloud was deployed13:35
gibiwe have two way to get the to the same goal :/13:35
sean-k-mooneytechnially 313:36
gibi:D13:36
sean-k-mooneyamd and nviai now supprot vgpu via palin sriov13:36
sean-k-mooneyso you can jsut use pci passthough13:36
bauzaswhat I'm sad is that I thought cyborg would accept the mixed scenario13:36
bauzasbut they explicitely ask to split the cloud13:36
bauzasdue to implementation details13:37
sean-k-mooneyits not cyborg fault13:37
sean-k-mooneynova is not being expresive enough in its query to placment13:37
bauzasas they're unable to post-create the resources13:37
bauzasno13:37
sean-k-mooneyno we coud but we do not have the info required13:37
bauzasyou're asking placement to give you apples13:37
bauzasit's not nova's fault that you ask for apples coming specially from Tesco13:38
sean-k-mooneyfor example we could list all the device-profile that request the vgpu resouce and select one13:38
sean-k-mooneyand use that to create teh arq13:38
sean-k-mooneyah but that is not what we asked for13:38
sean-k-mooneywe assed for resouce:vgpu=113:38
sean-k-mooneywith no other qulifyer13:38
bauzasok, back to the original thoughts (that's already 2 hours we're tangling the problem)13:39
bauzaswe progressed13:39
sean-k-mooneythe current propsal uses ownwer traits and a prefilter to make that owrk13:39
bauzaswe know we wanna express 'placement, give me apples that come from tesco and not from carrefour"13:39
sean-k-mooneytransparent ot the user or admin by adding a required trati for nova or forbidnint trait for cyborg in the resouce:vgpu case13:39
bauzassean-k-mooney: in this case, I'd prefer the forbidden trait way13:40
sean-k-mooneywe can do that but it makes nova special13:40
bauzas"placement, give me apples from Tesco" in the cyborg case13:40
bauzas"placement, give me apples not coming from Tesco as I know you got some of them"13:40
bauzassean-k-mooney: only if cyborg is configured on this cloud13:40
bauzasthis is the "as I know"13:41
sean-k-mooneythat works if the class is only shared across 2 service but not 3+13:41
bauzasnothing would change for operators who don't give a single penny to cyborg13:41
sean-k-mooneythere is another way i guess13:41
sean-k-mooneywe could use placment aggreate to track ownership13:42
bauzasthe other way is aggregates13:42
bauzas"placement, don't give me apples coming from those supermarkets"13:42
sean-k-mooneyeach proejct woudl add all there rps to an aggate13:42
bauzassean-k-mooney: I insist13:42
sean-k-mooneyand then nova would in the nova case add a member_of requirement to the vgpu request group13:42
bauzassean-k-mooney: whatever the solution is, the prefilter which would have the excluding logic (for the nova case) should only do this if cyborg manages hosts13:43
opendevreviewLee Yarwood proposed openstack/nova master: Add check job for FIPS  https://review.opendev.org/c/openstack/nova/+/79051913:43
sean-k-mooneybauzas: that is trival to do13:43
sean-k-mooneybauzas: we just have a config option for it13:43
sean-k-mooneylike pcpus13:44
bauzassean-k-mooney: I don't want upgrade impacts if the operator doesn't use cyborg yet13:44
bauzassean-k-mooney: can we express "give me hosts that are not from this aggregate" ?13:44
sean-k-mooneythere wont be one if we do this correctly regarless of ownwer traits ro differnt resouce classes13:44
sean-k-mooneybauzas: yes13:44
bauzasok, so there are no operations for nova-managed hosts13:45
sean-k-mooneybut i do not think we shoudl do a negitive member_or for cyborg13:45
bauzassean-k-mooney: the other way13:45
bauzassean-k-mooney: cyborg would manage an aggregate of managed hosts13:45
sean-k-mooneyfor nova the prefilter when enabled would do member_op=nova-aggate or member_of=cyborg-aggreate13:45
bauzasthe prefilter would ask for hosts that are members of this agg in the cyborg case13:45
sean-k-mooneywhen prefilter is off it wont do anything13:46
bauzasbut in the nova case, the prefilter would ask hosts that AREN'T hosts of this agg13:46
bauzassean-k-mooney: again, I don't wanna manage a fleet of nova hosts 13:46
sean-k-mooneybauzas: no i dont think that is the corrct way to do that13:46
sean-k-mooneybauzas: that is short sighted in my opipion13:46
sean-k-mooneywe can have nova automatic regesiter in the nova host aggrate13:47
bauzaseeek13:47
sean-k-mooneywe just need to choose a fixed uuid for it using uuid513:47
bauzasyet again some hack for a non-necessary need in the case of operators don't care about cyborg13:47
gibiI think we should forbid to have two services provide exactly the same thing. As soon as one of the service provides an extra capability like programability then we can schedule based on that. Ownership is an artificial quality we invented as there is no difference between the two vGPU devices. 13:47
sean-k-mooneygibi: ok so we are going to reject the cybrog vgpu spec then13:48
sean-k-mooneygibi: since it provides nothing over novs implemenation ?13:48
gibiso I'm back to my argument that I don't like duplicating capabilities between services13:48
gibias it adds 0 value13:48
bauzasI have to bail out for 20 mins13:48
sean-k-mooneyshould we schdule a call on this topic to get wider input13:49
gibisean-k-mooney: do you know who else cares about one or the other vgpu support?13:49
sean-k-mooneygibi: ill bring up the idea of supporting cyborg in our product again today and see if there is any apitate for that13:49
sean-k-mooneygibi: not not really13:50
bauzassean-k-mooney: I don't see it happening in OSP1813:50
bauzassoooo, 1+13:50
gibias I only argue from the complexity persepective I cannot argue about the business perspective, I have no business in this area13:50
bauzasA+13:50
sean-k-mooneyi was jsut wondering if we want dansmith or johnthetubague13:50
sean-k-mooneybasicaly the wider core team13:50
opendevreviewStephen Finucane proposed openstack/nova stable/wallaby: Test numa and vcpu topologies bug: #1910466  https://review.opendev.org/c/openstack/nova/+/79765213:51
opendevreviewStephen Finucane proposed openstack/nova stable/wallaby: Fix max cpu topologies with numa affinity  https://review.opendev.org/c/openstack/nova/+/79765313:51
dansmithwell, I can't really speak to the business side either,13:51
dansmithbut I totally agree with gibi on a technical level13:51
dansmithit makes no sense to me to build half of cyborg inside nova unless there's some reason it has to be done that way13:52
sean-k-mooneyre no duplication i dont nessisarly dissagree just practicalities13:52
dansmithpracticalities of "it's just easier to hack in the bits we care about into nova than to coordinate with or contribute to another project" right?13:53
sean-k-mooneyok taking downstream out of this i think it would be goind to do a paper exersise of thinking howe we woudl supprot shareing of resouce classes ectra in general between services13:53
dansmithisn't that what the long-awaited consumer types is supposed to help with?13:54
bauzasdansmith: that's my main concern13:54
sean-k-mooneydansmith: well not just its easier because if we are bing honest its not eair to land thing in nova13:54
bauzasadding nova traits for the whole purpose of cyborg seems invasive13:54
sean-k-mooneydansmith: not really no13:54
sean-k-mooneyconsumer type does nto help with shareing resouce classes between services13:55
dansmithsean-k-mooney: well, it does if you have a nested provider so you know where the seam is13:55
sean-k-mooneydansmith: no because cybrog creates RPs under the compute node RP13:56
sean-k-mooneyso we cant use same subtree here13:56
sean-k-mooneyit wont help13:56
sean-k-mooneynot unless we move all rescoue off the root RP13:56
sean-k-mooneyand have each service start there own subtreed form a common shared root rp13:57
gibiconsumer type is like, instance, migration, reservation, etc. What we have here is provider type it is provided (managed by) nova, cyborg, neutron...13:57
sean-k-mooneyyep ^13:57
gibiso I don't think consumer_type helps here13:57
bauzasgibi: only because we need to ask placement to only give us a subset13:57
bauzasfor cyborg-only reasons13:58
sean-k-mooneynot just for cyborg13:58
gibibauzas: but we only need to ask for a subset if that subset is different from the other subset13:58
gibiis the two subsets provide the same capability the why differentiate13:58
gibis/is/if/13:58
bauzassean-k-mooney: at the moment, yes, only because cyborg13:58
bauzasgibi: 100% agreed13:59
bauzasit's a whack-a-mole game13:59
gibiwe differentiate not becasue they provide different capabilities but becuase there is different implementation behind them we need to be aware off during plugging13:59
sean-k-mooneythe usecase predates it the cybrog one and blocked ohter feature in the past13:59
bauzasgibi: again, 100% agreed on your last sentence13:59
sean-k-mooneyi dont like catogoriesing this as a problem cause by cyborg14:00
gibimaybe vgpu is an exception, a historical exception14:00
sean-k-mooneyits a nova/placement limmiation taht we need to solve genericly to enable cyborg and other usecases14:00
sean-k-mooneygibi: well cyborg is doing generic pci passthough also14:00
bauzasgibi: other services could propose resources to consume14:00
gibiand all the other similar cases like accelerator_direct and disk_gb are different as there we have real differences to schedule on14:00
gibiand dont need to invent ownership as a differentiatoer14:01
dansmithso the concern is managing the actual RP and available resource for the cyborg things and not the allocations of those things by another source?14:01
dansmithbecause cyborg reserving some accelerator that is on the compute RP _would_ use a different consumer_type I would think14:01
gibicyborg does not create consumers nova create the consumer after scheduling14:02
gibicyborg only creates inventories14:02
dansmithokay I guess this has all fallen out of my head14:02
gibior we are back to the discussion to split the instance consumer into subconsumers14:02
gibitoday we have a single consumer per instance (except during migration)14:03
dansmithI thought cyborg was going to have to do that because of dynamic devices that may or may not exist until they're scheduled14:03
dansmiththings it programs to create a new device that wasn't actually part of the schedule14:03
sean-k-mooneydansmith: cyborg may have to update the RPs in some cases but we are modelign programabel devices as programable not programed14:05
dansmithokay14:05
sean-k-mooneyfor the cae wehre the admin will use cyborg directly to prgram it out of band that device would be consomed by cyborg14:05
sean-k-mooneybut the resouce provided form ti woudl be consuemd by nova still14:06
dansmithah, so cyborg consumes the programmable device itself, creates a new resource for the programmed thing, which nova is the consumer of, right?14:06
sean-k-mooneyyes in that case14:06
dansmithyeah, okay14:06
sean-k-mooneywhere the consumer of the programabel device is not a vm14:07
dansmithyeah14:07
sean-k-mooneye.g. where you have a many vm to 1 device toplogy 14:07
sean-k-mooneythats the theory at least14:07
sean-k-mooneynot sure they have actully implemente any dirver that does this yet14:07
dansmithI guess that makes the concern over who is managing what less clear to me14:07
dansmithwhich I think was what started this14:08
dansmiththis: [06:53:45]  <sean-k-mooney> ok taking downstream out of this i think it would be goind to do a paper exersise of thinking howe we woudl supprot shareing of resouce classes ectra in general between services14:08
sean-k-mooneywe currently have 4 possibel ideas fo how to do ^ but no concreate propasl and we have not fully tought though all the implcaitons14:09
sean-k-mooneycyborg being the first to need this was trying to propas a way in teh vgpu spec14:10
sean-k-mooneyusing the ownwer_trits approch but bauzas has conserns over that as does gibi 14:10
sean-k-mooneybut really this is a sperate problem form cybrog vgpu supprot14:11
bauzasmy main concern is that we need to add traits for nova hosts14:11
sean-k-mooneyyes but they are not really nova hosts14:11
bauzasnon cyborg managed hosts, if you prefer14:11
bauzasor libvirt-managed hosts14:12
sean-k-mooneywe are supper bias but i have agrued since before nested resouce providers that nova shoudl not own the root rp14:12
sean-k-mooneywell it was one of my argument for intoducing them14:12
sean-k-mooneythe "compute" host is really a shared thing14:12
sean-k-mooneyto which multiple serivce may create resouces14:13
sean-k-mooneybeing totally frank im worried that we will not come to a desicion on this this cycle and cyborg will have to with a third time to make progress14:14
sean-k-mooneyi have personaly see this type of discussion take litrally 2-3 year to progress and i think that is harmful to the openstack comunity as a whole. i also dont want to rush it as its hard to pivort after its released14:16
bauzaswe provided alternatives14:17
sean-k-mooneyyes but we dont agree on any of them14:17
sean-k-mooneyi hate to say this but i think we need a spec for this or atleast an etherpad and some midcycle like real time design session on this14:19
sean-k-mooneythis is exactly the type of thing we woule have worked though in the ptg14:20
sean-k-mooneywith a whiteboard and all the stake holders (where possible) in the same room14:20
bauzasI can't disagree14:21
dansmithwe've also had plenty of those in the past which didn't yield much progress.. specifically about cyborg :)14:21
sean-k-mooneyya the dublin seesion in partcalar was less then useful14:22
* gibi is tired 14:23
sean-k-mooneygibi: thanks for trying to help14:24
sean-k-mooneybut i feel the same14:24
gibiI summarized my lates view in the spec review but I haven't published it yet. 14:24
sean-k-mooneycould we just use a custom resouce class for now an punt on this for now14:25
gibiI'm torn between doing the right thing architecturally and supporting a parallel projec to integrate with nova technically14:25
sean-k-mooneyi know that would requrie a reshape eventually but at least it would unblcok them14:25
gibiit would unblock them and we will never allocate time to do that reshape later14:26
sean-k-mooneywell it would be in cyborg?14:26
gibias it just a lot of work14:26
gibifor basicly nothing14:26
sean-k-mooneymaybe it would be in nova14:26
sean-k-mooneydamb cross project reshapes will be a bitch to fiture out forget i said anything14:26
* gibi published his revised viewpoint in the spec14:37
bauzasgibi: thanks14:38
elodillessean-k-mooney gibi : as we yesterday discussed on the meeting, here is an example of a failure what I saw: test_live_block_migration_with_attached_volume -- https://c5d6a707d1df71acd55f-fbeed0693cac4a6e5441d43111515edc.ssl.cf5.rackcdn.com/787252/4/gate/nova-live-migration/706ec17/testr_results.html14:39
gibielodilles: looking14:40
elodillesI saw this a couple of times at the failures of this patch: https://review.opendev.org/c/openstack/nova/+/78725214:40
gibielodilles: you have a good timing :)14:40
elodillesstable/victoria14:40
elodillesgibi: why? o:)14:40
sean-k-mooney weird 14:40
sean-k-mooneyboth thos test refernce the same volume 7688b3f1-6549-4e0c-a507-9789ddc2eb9514:40
gibielodilles: I've just stopped thinking about the above vGPU problem14:41
sean-k-mooneythat should not happen right14:41
sean-k-mooneytwo tests refering to the same volume uuid 7688b3f1-6549-4e0c-a507-9789ddc2eb9514:41
sean-k-mooneyif they run in parally they would conflict14:41
gibisean-k-mooney: wich two test cases?14:42
gibisean-k-mooney: I see a testcase and a tear down14:42
gibiof the suite14:42
sean-k-mooneyoh ya its the tear down method14:42
elodillesgibi: well I've seen that there was an ongoing discussion so I did not want to interrupt earlier :)14:42
sean-k-mooneyok that make a liit more sense14:42
sean-k-mooneyok so the teardown failed becasue presumable the volume was still attached to the vm14:43
sean-k-mooneyit was in state detaching14:43
gibiit is trying to detach in a loop so I guess it is the original detach problem 14:46
gibifor what we created the notification based solution14:46
gibibut that solution is not in victoria14:46
sean-k-mooneyya the detach starts at Jun 21 09:20:16.007823 14:46
gibiso either we increase the timeout value before we retry in victoria or backport the notification based solution to victoria14:48
sean-k-mooneywe are calling os_brick.initiator.connectors.iscsi.ISCSIConnector.disconnect_volume on it14:48
sean-k-mooneywe would not do that if the detach ahs not commpeted right?14:48
gibisean-k-mooney: I don't know. what I know that the detach is not completed from libvirt perspective, it still reports the device in the live domain after 7 retries14:49
sean-k-mooneythis is happening as part of a live migration14:50
sean-k-mooneythis looks odd what is the test actully doing14:51
sean-k-mooneyit look like we are runign post live migration on the souce which is tearing odwn the volume while its potentioanly still detaching14:51
gibiI think lyarwood has more context on this detach problem 14:51
sean-k-mooneyim wonderign if the tempest test is broken14:52
sean-k-mooneye.g. is it issuing a detach and then a live migrate without waiting for the detach to complete14:52
sean-k-mooneythis is the test https://github.com/openstack/tempest/blob/master/tempest/api/compute/admin/test_live_migration.py#L178-L20414:53
sean-k-mooneythats doing an attach14:54
sean-k-mooneybut its not doing a detach14:54
sean-k-mooneyits also not waiting to ensure its actully attached 14:54
sean-k-mooneyunless self.attach_volume is doint that internally14:55
sean-k-mooneyok if the attach fails its cleanup funciton does a detach im assuming https://github.com/openstack/tempest/blob/53c02181f87804a4ba8ddf6288ea1f7717234c2a/tempest/api/compute/base.py#L554-L59114:56
sean-k-mooneyit is waiting internally14:56
sean-k-mooneyhttps://github.com/openstack/tempest/blob/53c02181f87804a4ba8ddf6288ea1f7717234c2a/tempest/api/compute/base.py#L59014:57
sean-k-mooneycan we see the volume attach complete in the logs14:58
sean-k-mooneywe do the attachment in libvirt at un 21 09:20:00.476733 15:00
lyarwoodif we are still talking about https://c5d6a707d1df71acd55f-fbeed0693cac4a6e5441d43111515edc.ssl.cf5.rackcdn.com/787252/4/gate/nova-live-migration/706ec17/testr_results.html it's just another basic detach failure from the live domain15:10
lyarwoodafter the test has passed and we are cleaning up15:10
lyarwoodI really need to land https://review.opendev.org/c/openstack/tempest/+/794757 so we can get some guestOS logs in this case15:12
gibilyarwood: that run is from stable/victoria so we don't have the new detach code there. Do you think it would help if we try to backport the new detach code? or is it possibly unrelated?15:13
lyarwoodor we could just Depends-On that from a DNM stable/victoria change15:13
elodilleslyarwood: so you are saying that this is similar like bug #1931716 but not the exact same case?15:13
lyarwoodgibi: we've seen failures of this test on master so this time I don't think backporting that code is going to help15:14
lyarwoodbrb15:14
gibilyarwood: OK, then I agree to land the tempest improvement first to get more info15:15
opendevreviewElod Illes proposed openstack/nova stable/victoria: DNM: volume detach test  https://review.opendev.org/c/openstack/nova/+/79767515:18
elodillesI've created the DNM patch what lyarwood suggested ^^^15:19
lyarwoodelodilles: thanks15:21
elodillesnp. let's see what happens :)15:22
lyarwoodelodilles: I have a change to skip the test on master btw that we could also backport if we see the same soft lockups15:30
opendevreviewStephen Finucane proposed openstack/nova stable/victoria: Test numa and vcpu topologies bug: #1910466  https://review.opendev.org/c/openstack/nova/+/79768015:30
opendevreviewStephen Finucane proposed openstack/nova stable/victoria: Fix max cpu topologies with numa affinity  https://review.opendev.org/c/openstack/nova/+/79768115:30
elodilleslyarwood: thanks, good to know that, we might need to backport that then if needed15:35
kashyapgibi: lyarwood: sean-k-mooney: When you get a min, have a look to see if you spot any holes there:  https://blueprints.launchpad.net/nova/+spec/virtio-as-default-display-device15:46
sean-k-mooneythat will need a spec most likely15:50
sean-k-mooneyim not agaisnt it but we need to record the current modele in use15:51
sean-k-mooneyand then only change the default for new instnaces15:51
sean-k-mooneylyarwood: so  in this case we dont actully want to do the detach on succesful migration right15:59
sean-k-mooneythat is jsut an artifcat of the way the cleanup is done16:00
sean-k-mooneywe just want to delete the vm then delete the volume16:00
lyarwoodsean-k-mooney: yeah it's just part of the tempest cleanup code that's added when we initially attach16:00
sean-k-mooneycould we modify that16:00
lyarwoodindeed, we don't need to detach as part of this test16:00
sean-k-mooneyadd a flag to attach e.g cleanup=false16:00
lyarwoodso we could just nuke the instance and move on with our lives16:00
sean-k-mooneyyep16:01
lyarwoodyup, the only issue is that there's duplication in the tempest code base with lots of this utility code16:01
lyarwoodbut I can do this for the compute attach volume part at least16:01
lyarwoodI think there's another helper in the volume api tests16:01
lyarwoodand maybe the scenario manager16:01
sean-k-mooneyit should in thoery decrease test execution time16:02
lyarwoodYup we would however drop some coverage of detaching volumes16:03
*** rpittau is now known as rpittau|afk16:08
sean-k-mooneylyarwood: we would but in principal we have tests for that specifically16:34
sean-k-mooneydetach is not really a part of what we are testing in this case16:34
sean-k-mooneyit does expose racecondition more as we see here16:34
sean-k-mooneybut im not sure that is always a good thing16:34
opendevreviewLee Yarwood proposed openstack/nova master: WIP compute: Avoid calling detach with src connection_info during LM rollback  https://review.opendev.org/c/openstack/nova/+/79772517:19
sean-k-mooneythat was quick17:27
gansolyarwood: hi! is there anything pending on https://review.opendev.org/c/openstack/nova/+/795432 so it can be merged? That patch was the latest one to pass CI. There are several patches (including one with +W) that haven't had a single positive CI run17:39
lyarwoodelodilles: https://review.opendev.org/c/openstack/nova/+/795432 - can you hit this in the morning? I've upgraded my +1 to +2 to move it along.19:37
lyarwoodganso: apologies, I'll work with elodilles and the other stable cores to unblock things in the morning19:38
gansolyarwood: np! thank you very much! I was mostly wondering if something else was pending because I had rebased on top of it and it still failed. See latest comments in https://review.opendev.org/c/openstack/nova/+/79671919:39

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