Thursday, 2020-03-26

openstackgerritmelanie witt proposed openstack/nova master: Add info about affinity requests to the troubleshooting doc  https://review.opendev.org/71509200:02
brinzhang_dansmith: I dont know what you would want to say, you mean we should use update method instead of patch method? We were disscussed in irc meetting, at the beginning I wanted to use `PUT /servers/{server_id}/os-volume_attachments/{volume_id}` to implement this feature, but more people dont agree.00:09
brinzhang_dansmith: Firstly, we need admin_or_owner role to execute this, but the PUT swap volume API default role is administrative.00:09
brinzhang_dansmith: Secondly, we should make the `volumeId` to optional in the request body, that change is to big, and many limits and changes will be add this PUT swap volume API.00:09
brinzhang_dansmith: From talked in the irc meetting before freeze blueprint for Ussuri, we reached a letter of agreement and adopted the alternative scheme[1] of seen-k-mooney to achieve this function.00:09
brinzhang_[1] https://review.opendev.org/#/c/580336/32/specs/ussuri/approved/destroy-instance-with-datavolume.rst@5100:09
brinzhang_dansmith: I know you didn't participate in the irc discussion at the time, but I don't understand why we can't use the PATCH API?00:10
*** macz_ has joined #openstack-nova00:22
*** ociuhandu has joined #openstack-nova00:24
*** macz_ has quit IRC00:27
*** ociuhandu has quit IRC00:30
*** ociuhandu has joined #openstack-nova00:31
*** ociuhandu has quit IRC00:36
*** gyee has quit IRC00:39
*** lbragstad has quit IRC00:46
*** tosky has quit IRC00:52
alex_xusean-k-mooney: I guess we are missing a patch for evacuate00:56
*** zhanglong has joined #openstack-nova01:10
*** Liang__ has joined #openstack-nova01:17
*** TxGirlGeek has quit IRC01:20
*** mlavalle has quit IRC01:22
*** tetsuro has joined #openstack-nova01:24
brinzhang_alex_xu, sean-k-mooney: I think in the libvirt we are lost the accel_info after evacuate, https://review.opendev.org/#/c/631245/62/nova/virt/libvirt/driver.py@578101:44
brinzhang_alex_xu, sean-k-mooney: when we have an evecuate a server, maybe we need try to get the accelerators info for by instance.uuid, rirht?01:45
*** ociuhandu has joined #openstack-nova01:58
*** ociuhandu has quit IRC02:02
*** dswebb has quit IRC02:26
*** zhanglong has quit IRC02:39
*** mkrai has joined #openstack-nova02:40
*** zhanglong has joined #openstack-nova02:49
*** larainema has joined #openstack-nova03:08
sean-k-mooneyalex_xu: we are not planning to have all ops supported initally so evac can be added later if we block it in the final patch03:15
sean-k-mooneybut it would be good to add it sooner rather then later03:15
*** mkrai has quit IRC03:20
*** tetsuro has quit IRC03:46
*** tetsuro has joined #openstack-nova03:49
*** zhanglong has quit IRC03:58
*** mkrai has joined #openstack-nova04:12
*** udesale has joined #openstack-nova04:47
*** tonyb[m] has quit IRC04:49
*** tonyb[m] has joined #openstack-nova04:49
*** udesale has quit IRC05:10
*** links has joined #openstack-nova05:12
*** udesale has joined #openstack-nova05:12
*** ratailor has joined #openstack-nova05:24
*** evrardjp has quit IRC05:36
*** evrardjp has joined #openstack-nova05:36
*** zhanglong has joined #openstack-nova05:37
*** rcernin has quit IRC05:40
*** rcernin has joined #openstack-nova05:41
*** rcernin has quit IRC05:41
*** rcernin has joined #openstack-nova05:42
*** rcernin has quit IRC05:42
*** rcernin has joined #openstack-nova05:47
*** rcernin has quit IRC06:02
*** rcernin has joined #openstack-nova06:02
*** rcernin has quit IRC06:05
*** rcernin has joined #openstack-nova06:05
openstackgerritLuyao Zhong proposed openstack/nova master: support live migration with vpmems  https://review.opendev.org/68785606:08
openstackgerritLuyao Zhong proposed openstack/nova master: Track orphan instances and error migrations in resource tracker  https://review.opendev.org/71465306:08
*** nightmare_unreal has joined #openstack-nova06:51
*** dklyle has quit IRC06:53
*** ociuhandu has joined #openstack-nova07:03
*** ociuhandu has quit IRC07:07
*** ralonsoh has joined #openstack-nova07:09
*** xek_ has joined #openstack-nova07:20
*** dpawlik has joined #openstack-nova07:22
*** factor has quit IRC07:29
*** mkrai has quit IRC07:40
*** belmoreira has joined #openstack-nova07:44
*** tesseract has joined #openstack-nova07:53
*** maciejjozefczyk has joined #openstack-nova07:56
*** vishalmanchanda has joined #openstack-nova07:56
*** sapd1 has joined #openstack-nova07:57
*** evrardjp has quit IRC07:57
*** evrardjp has joined #openstack-nova08:01
gibigood morning nova08:01
*** slaweq has joined #openstack-nova08:01
*** amoralej|off is now known as amoralej08:02
brinzhang_gibi: good morning ^^08:03
* nightmare_unreal waves \008:05
gibio/08:05
brinzhang_gibi: pls add this PATCH API for implement bp/destroy-instance-with-datavolume to today's agenda, I replied dansmith's comments inline, and above in irc08:05
gibibrinzhang_: OK. Will you be able to participate or I could you update me shortly what the disagreemen is about?08:06
*** guilhermesp has quit IRC08:06
*** NostawRm has quit IRC08:06
gibis/I//08:06
brinzhang_gibi: he seems dont want I use PATCH API to do this, but I dont want to have a change.08:06
brinzhang_gibi: it's too later for mee, I am not able to participate08:07
gibibrinzhang_: OK thanks. I will raise it to find a way forward08:07
*** NostawRm has joined #openstack-nova08:07
brinzhang_I think this PATCH reched in the irc meeting, without good reason, I hope it will not stop it from moving forward.08:09
*** belmoreira has quit IRC08:09
brinzhang_gibi: you can see the irc log above, at 8:09:27-8:10:5008:10
gibiI have to, and I will, read back on the disagreement08:10
brinzhang_gibi: thanks ^^08:10
brinzhang_gibi: for bp/action-event-fault-details https://review.opendev.org/#/c/694430/, gmann agreed to add the new policy, but he want we limit it in 4xx error code, I think we also need expose 500 (novalid host exception) to the non-admin if we changed the default policy08:13
brinzhang_gibi: you can review https://review.opendev.org/#/c/694430/, thanks08:13
gibibrinzhang_: I will08:19
*** tkajinam has quit IRC08:19
*** rpittau|afk is now known as rpittau08:26
*** toabctl has quit IRC08:30
*** mkrai has joined #openstack-nova08:44
*** tosky has joined #openstack-nova09:00
*** Liang__ is now known as LiangFang09:03
*** zhanglong has quit IRC09:09
*** aloga has quit IRC09:10
*** macz_ has joined #openstack-nova09:10
*** toabctl has joined #openstack-nova09:14
*** macz_ has quit IRC09:14
*** ociuhandu has joined #openstack-nova09:14
*** derekh has joined #openstack-nova09:37
*** martinkennelly has joined #openstack-nova09:44
*** ociuhandu has quit IRC09:48
*** ociuhandu has joined #openstack-nova09:49
*** rcernin has quit IRC09:51
gibigmann, brinzhang_: responeded in https://review.opendev.org/#/c/694430/09:56
*** ociuhandu has quit IRC09:58
*** LiangFang has quit IRC10:06
*** ociuhandu has joined #openstack-nova10:08
brinzhang_gibi: thanks, the new policy mainly facilitates us to expose the 'details' information to non-admin. We can change it by modifying the default policy check_str. This has been agreed with gmann. Last night at 15:00 UTC in openstac-nova channel discussion .10:14
openstackgerritMarcin Juszkiewicz proposed openstack/nova master: Add default cpu model for AArch64  https://review.opendev.org/70949410:14
hrwkashyap: took your comments and applied10:15
kashyaphrw: Hiya; will look10:15
kashyapThanks!10:15
gibibrinzhang_: yeah I'd like to have the new policy to control the exposure of the 'details' field10:16
brinzhang_if we re-using the  BASE_POLICY_NAME% 'events' policy, we will difficult to distinguish whether 'traceback' or 'details' expose to non-admin10:16
brinzhang_gibi: yeah, thanks10:16
brinzhang_gibi: now gmann want we shuold just catch 4xx error to populate the 'details'10:17
brinzhang_gibi: But I think we also need 500 error, that the non-admin can have a try as soon as possible10:18
luyaostephenfin: Hi, I have addressed alex_xu's comments. :)  https://review.opendev.org/#/c/68785610:18
gibiyeah, I want to expose information, including NoValidHost, but I don't want to collect every possible error to whitelist them.10:18
brinzhang_as you said in commnets, information leakage is inevitable. Since the administrator chooses to modify the default policy, he will accept the change.10:18
gibibrinzhang_: exactly10:19
brinzhang_gibi: yeah, we are same, wait for gmann check, he has -1 on the patch10:19
hrwkashyap: turns out that we need that patch to get kolla-ansible CI running ;(10:20
nightmare_unrealhey how can i create fake cells and create instance in them for functional test. I am looking at nova/tests/functional/test_nova_manage.py10:22
luyaolyarwood: about vpmem cleanup during live migration,  we can check migration_context but not adding a new flag  to migrate_data obj, you can look at https://review.opendev.org/#/c/687856/20/nova/compute/manager.py@819710:24
kashyaphrw: "That patch" is the above default CPU model thing?10:24
*** macz_ has joined #openstack-nova10:26
kashyaphrw: Looks good to me; FWIW; once Zuul blesses it, then it can go through10:26
gibibrinzhang_: responed in the PATCH API discussion in the review and added the topic for the nova meeting accordingly10:27
hrwkashyap: yep10:28
hrwkashyap: or I am going to lose my sanity (as usual when touching nova/libvirt/qemu at once)10:28
brinzhang_gibi: thanks, I will see after dinner ^^10:28
gibibrinzhang_: ack, have a nice dinner10:29
nightmare_unrealnvm found it10:29
*** macz_ has quit IRC10:30
*** tetsuro has quit IRC10:34
*** dtantsur|afk is now known as dtantsur10:48
openstackgerritMerged openstack/nova master: Add Cyborg device profile groups to request spec.  https://review.opendev.org/63124310:49
*** ileixe has quit IRC10:59
*** spatel has joined #openstack-nova10:59
*** spatel has quit IRC11:03
openstackgerritjayaditya gupta proposed openstack/nova master: Support for nova-manage placement heal_allocations --cell  https://review.opendev.org/71445911:11
*** ileixe has joined #openstack-nova11:18
*** ileixe has quit IRC11:19
*** ileixe has joined #openstack-nova11:20
openstackgerritLee Yarwood proposed openstack/nova master: workarounds: Add option to disable native LUKSv1 decryption by QEMU  https://review.opendev.org/70803011:33
openstackgerritLee Yarwood proposed openstack/nova master: workarounds: Connect RBD volumes to the compute host as block devices  https://review.opendev.org/70802911:33
*** ociuhandu has quit IRC11:36
*** ociuhandu has joined #openstack-nova11:38
*** ociuhandu has quit IRC11:43
*** rpittau is now known as rpittau|bbl11:45
lyarwoodstephenfin: https://review.opendev.org/#/c/714956/ - would you mind hitting this?11:47
stephenfinsure11:47
lyarwoodluyao: ack sorry swamped with downstream things, I'll get back to that review today11:48
luyaolyarwood: thanks :)11:49
openstackgerritLee Yarwood proposed openstack/nova master: libvirt: Break up get_disk_mapping within blockinfo  https://review.opendev.org/71496212:07
hrwkashyap: zuul gave +1 for https://review.opendev.org/#/c/709494/ - bumping to +2+w?12:09
*** zhanglong has joined #openstack-nova12:10
*** mkrai has quit IRC12:12
kashyaphrw: I am a mere mortal, can't +2 :)12:12
kashyapgibi: ^ Mind having a gander at hrw's completed patch?12:12
*** nweinber has joined #openstack-nova12:12
gibikashyap, hrw: sure I will try12:12
kashyapgibi: At its core, it is setting a sensible default CPU model for AArch64 VMs12:13
kashyapgibi: We arrived at the sensible default (CPU model 'max') after consulting the QEMU AArch64 maintainers.12:13
*** ileixe has quit IRC12:14
sean-k-mooneykashyap: you could argue that min would be a better sensible default for live migration but max is overall a better default12:14
*** macz_ has joined #openstack-nova12:14
gibikashyap: ack. thanks for bringing in the experts on that12:14
sean-k-mooneyso ya it makes sense even if its not the safest default12:14
kashyap"min"?  There's no such thing as "min"12:14
sean-k-mooneyisnt there. there was an alternitive to max we were considering12:15
*** zhanglong has quit IRC12:15
*** zhanglong has joined #openstack-nova12:15
sean-k-mooneymy understanding of max is it will enable all feature that qemu can emulate on the host12:15
sean-k-mooneynot just the ones the host has at the hardware level12:15
kashyapYou mean, 'host'?12:16
sean-k-mooneyperhaps yes12:16
sean-k-mooneyi think our default should basically be the same as host model is on x8612:16
kashyapAs I noted on the patch, that is still not a good option if you want it to work for _both_ TCG and KVM.12:16
kashyapTo quote myself:12:16
kashyap[quote]12:16
kashyapFor KVM, we essentially have to go with "give the guest what the host CPU has", which you can express either as "host" or as "max" CPU models, the benefit of the latter ("max") being that it also works with TCG.12:16
kashyap[/quote]12:16
kashyapsean-k-mooney: That's what 'max' is; except that it also does the right thing for TCG12:17
kashyapgibi: Thanks!12:17
kashyapsean-k-mooney: Give your ACK on the change, too, please12:17
sean-k-mooneymax will not do the right thing for kvm however right. it will expose more then the host can support in hardware right12:17
*** lpetrut has joined #openstack-nova12:18
sean-k-mooneykashyap: well im not conviced max is the right thing in the kvm case i would prefer to have a different default for both honestly12:18
sean-k-mooneymax i think makes sense in the qemu/tcg case12:18
sean-k-mooneyunless host and max are always identical in the kvm case i woudl prefer it to be host when virt_type=kvm12:18
*** macz_ has quit IRC12:19
kashyapsean-k-mooney: Did you read the above quote?12:19
sean-k-mooneyyes12:19
*** bbowen_ has joined #openstack-nova12:19
*** bbowen has quit IRC12:19
sean-k-mooneyis host always identical to max with kvm12:20
kashyapNo; 'max' will give you the moving-target "all the stuff we can currently emulate".12:20
sean-k-mooneyor if your qemu can emulate feature not present on the host hardware will max result in it doing so even with kvm12:20
kashyapLet's not get worried about theoretical concerns.12:20
sean-k-mooneykashyap: its not a terirectical concern i disagreed with your live migration change and this in my view is one step closer to use not supporting live migration with aarch64 at all12:21
*** udesale_ has joined #openstack-nova12:21
sean-k-mooneyuseing max will have an upgrade impact as we will not be able to live migrate safely form a newer host to an older one12:22
sean-k-mooneyuse host with kvm will allow that12:22
kashyapsean-k-mooney: Hang on; I just asked Peter from QEMU again.12:22
kashyapSo, yes: 'max' is same as 'host' on KVM.  Always12:22
sean-k-mooneyok if that is the case im fine with it :)12:23
kashyapOkido12:23
kashyapgibi: Please go ahead with the patch.  --^12:23
sean-k-mooneyill go +1 it now12:23
kashyapThx!12:23
gibithanks, I will look in a minute12:23
kashyapNo worries; just wanted to say that the loose end (discussion) is tied :)12:24
* kashyap --> offline; back later12:24
gibikashyap: thanks, it helps :)12:24
kashyaplater == April 01 (yes, really; not a joke ;))12:24
*** udesale has quit IRC12:24
gibikashyap: have a nice time off12:25
*** ociuhandu has joined #openstack-nova12:28
*** ociuhandu has quit IRC12:33
*** kashyap has quit IRC12:34
*** sapd1 has quit IRC12:36
hrwthanks12:40
*** lbragstad has joined #openstack-nova12:42
*** ratailor has quit IRC12:44
*** spatel has joined #openstack-nova12:47
*** ociuhandu has joined #openstack-nova12:49
*** spatel has quit IRC12:52
*** amoralej is now known as amoralej|lunch13:01
*** ociuhandu has quit IRC13:10
*** rpittau|bbl is now known as rpittau13:10
*** mkrai has joined #openstack-nova13:11
openstackgerritMerged openstack/nova master: libvirt: Use domain capabilities to get supported device models  https://review.opendev.org/66691513:22
*** PetrTuma has joined #openstack-nova13:22
*** tkajinam has joined #openstack-nova13:22
*** mriedem has joined #openstack-nova13:25
*** ociuhandu has joined #openstack-nova13:26
openstackgerritKevin Zhao proposed openstack/nova master: fix scsi disk unit number of the attaching volume when cdrom bus is scsi  https://review.opendev.org/71260713:26
*** ileixe has joined #openstack-nova13:30
*** zhanglong has quit IRC13:36
*** mkrai has quit IRC13:40
dansmithgibi: yeah, I really just don't like PATCH in general. We don't have it anywhere else in here, and I'm missing the thing that makes it so hard to use PUT that it's worth using PATCH here13:41
dansmithgibi: I wasn't in the discussion where that was decided, which is fine, so until I'm convinced I'm -1 on it, but that doesn't mean you have to stop just for me if you (all) really think it's the only way13:41
gibidansmith: PUT is not use to update the fields of an existing volume connection but to replace the existing volume connection13:41
dansmithgibi:  you don't mean PUT in general, I assume13:42
gibidansmith: nope. I mean https://docs.openstack.org/api-ref/compute/#update-a-volume-attachment13:42
gibithis is an unfortunate use of PUT ^^13:42
gibias it replaces an object not updating the field of an object13:43
dansmithright, so if I put the attachment with the same fields but with delete_on_terminate set differently, then it's easy to determine that it's not new yeah?13:43
gibibut then the PUT with the same volume_id needs to be end user facing, while when the volume_id is new it needs to be admin only13:45
dansmithright, so admin can update volume_id, but users can only update delete_on_termination13:45
dansmithseems pretty straightforward to me13:45
openstackgerritKevin Zhao proposed openstack/nova master: fix scsi disk unit number of the attaching volume when cdrom bus is scsi  https://review.opendev.org/71260713:46
gibidansmith: your argument feels convinving to me but also in the past the discussion in the spec and on the nova meeting convinved me to use the PATCH approach so I now go to the confused state13:48
johnthetubaguyFWIW, consistency with other Nova APIs seems like a big win, i.e. use PUT13:49
dansmithgibi: okay, it seems like we still need to be able to do a policy check on the body of the thing, even if it''s a PATCH, so I can't imagine why we can't do that for PUT as well13:49
dansmithjohnthetubaguy: ++ for sure13:49
*** efried has joined #openstack-nova13:51
*** mkrai has joined #openstack-nova13:54
gibiI'm sad that this discussion happens so late. From reading back I see that another concern was to mix two slightly unrelated API swap, and update into the same PUT method13:56
dansmithfrom the api client's perspective it's the same operation isn't it?13:57
dansmithgibi: I apologize for bringing it up I guess, but ... this is what wide review is for, IMHO, and this is a big step out of existing conventions for nova so it seems worth having wide review on13:58
gibidansmith: I also apologize to saying this but I think such wide review should happen in the spec. this is why we have spec review for13:59
gibifrom me the two operation is different from the client perspective for mutliple reasons13:59
dansmithlike, the internal plumbing of nova turns one of them into a special rpc call to the virt driver and the other into a db update, but that shouldn't affect the external design13:59
dansmithwell, I disagree13:59
dansmith(about them being different)13:59
gibii) the client is different for the two operations13:59
gibifor the swap volume the client is cinder14:00
gibifor the update delete_on_terminate flag the client is the owner of the instance14:00
dansmithin one specific set of cases it is,14:00
gibiii) the swap operation effects the running instance while the update operation does not14:00
dansmithbut a client library used by both wouldn't need to distinguish.. they're both "update the volume attachment" operations14:01
sean-k-mooneythe real issue is swap volume should have been a server action keeping put free for updates.14:01
dansmithsean-k-mooney: I can see that, but I'm not sure it *had* to be.. this is very RESTful as it is, wouldn't you say?14:01
dansmithwe expose a resource, it's changing the resource.. what happens behind the scenes is internal logic14:02
sean-k-mooneyit is not a voilation or the rest design to do it as we do today14:02
*** amoralej|lunch is now known as amoralej14:02
dansmithbut server actions are pretty much all not RESTful :)14:02
sean-k-mooneyso yes from that perspecive swap volume via update if fine14:03
sean-k-mooneydansmith: yes by definition its acutlly a rest anti pattened to have rpc like api actions14:03
dansmithright14:03
dansmithso point is, I just don't think swap-via-update is super terrible in a fundamental way (swap in general may be)14:04
sean-k-mooneyi was ok with the PUT in the spec by the way. gmann was not so i proposed patch as a compormise14:04
dansmithand I don't think that we need should justify using a different method externally because the internal plumbing is currently setup in one specific way14:04
sean-k-mooneysince the api method was free and it allowed used to expresss policy cleanly14:05
dansmiththat is the point of the api.. to abstract those things away14:05
dansmithright, but where does it end? use HEAD for the next one? :)14:05
* johnthetubaguy nods (giggle)14:06
sean-k-mooneyno but patch is for a paritl update which is what we are doing14:06
*** xek__ has joined #openstack-nova14:06
sean-k-mooneywe are setting one atribute and not the rest14:06
sean-k-mooneybut as i said it was a compromise not the ideal solution14:06
dansmithsure, but we're using a different http method because we just want to abuse the WSGI plumbing to call different code for this operation vs. the existing put14:06
dansmithso when the next thing comes along, we could use HEAD and avoid having to abstract that one too :)14:07
sean-k-mooneywell head would only be apporpriate if the new action match the semantics of head but i get your point and i think you get mine14:07
sean-k-mooneydansmith: in your view you would prefer that we keep with PUT correct14:08
dansmithyes14:08
dansmithand yes, HEAD is a silly example for the sake of being silly14:08
sean-k-mooneyi have not looked at gmann specific objection in a while but i belive it was related to the semaintic of the payload14:09
sean-k-mooneydo you have a proposal to adress his concern?14:09
*** xek_ has quit IRC14:09
dansmithI don't know what his concern is14:09
sean-k-mooneyi think there was a question about including the volume id in the payload vs the url and how to correctly apply differnet policies to each sub filed14:10
sean-k-mooneyi belive we can already do a polciy check on the payload so that is not really a concern14:10
openstackgerritSundar Nadathur proposed openstack/nova master: Block unsupported instance operations with accelerators.  https://review.opendev.org/67472614:10
openstackgerritSundar Nadathur proposed openstack/nova master: Add cyborg tempest job.  https://review.opendev.org/67099914:10
dansmithI don't see that as a problem yeah14:10
sean-k-mooneybut there was a question about if the volume shoudl be set in teh payload or just in the url14:10
sean-k-mooneycurrently i think the volume id in the url is the current volume and the one in the payload is the one to swap too14:11
dansmithwell, that makes it even easier to determine what is going on right?14:12
dansmithif they differ, then check admin policy, if they're the same, user policy and only look for user-changeable options in the body14:12
sean-k-mooneyi think we would have to require that its not set in the payload14:12
sean-k-mooneywell ya we could require they are the same14:12
dansmithfurther, PATCH has to be atomic and provides no way to guarantee that the representation hasn't changed since I fetched it on the client side14:13
dansmithso if someone has updated the definition between me fetching it and sending a single field to change, I might not notice14:13
dansmithi.e. there's no generation or other indicator14:13
sean-k-mooneyhonestly i could be miss remebering what gmann objection was14:13
sean-k-mooneydansmith: yes that is also true14:14
dansmithprobably not likely on a volume attachment currently, but certainly could happen in the future14:14
dansmithi.e. some sort of swap-snapshot-swap thing or something14:14
dansmith(this is a major objection of mine to patch in general for most things)14:14
sean-k-mooneyi think the most likely issue would be with doing a volume detach via cinder in parralle14:14
sean-k-mooneythat said we have the same issue with PUT no? unless we have a genertion field in the payload14:15
dansmithno, with put you have all the original fields there, you can check to see that nothing has changed14:15
sean-k-mooneysince put is ment to do a full update replacing all fields at least semantically14:15
dansmithI'm not saying we *have* to for this specific case, but it's at least *possible*14:16
dansmithalso, looking at the api-ref here,14:16
sean-k-mooneyyes that is true14:16
dansmiththere's a big red warning about the whole api being obscure, because people would expect to be able to PUT that.. it would be a lot clearer to revise that and say "changing the volumeId requires special privileges and support" instead of just walling off all of PUT14:16
sean-k-mooneyanyway i was pretty nutral on this orginally so if there is a stong feeling (sounds like there is) that it should be PUT then im ok with that too14:17
dansmithbecause people would say "What? why can't I change this flag without special virt support?"14:17
nightmare_unrealhow can I run nova functional tests ?14:17
sean-k-mooneynightmare_unreal: tox -e functional-py3614:17
nightmare_unrealand for specific file ? -- -n file-name ?14:18
sean-k-mooneyyou can do -- folowed by any arges supported by stestr14:18
sean-k-mooneynormally i just pass a regex for the class name or test name14:18
nightmare_unrealcool :) Thanks14:19
sean-k-mooneye.g. tox -e functional-py36 -- "compute|libvirt"14:19
nightmare_unrealin quotes can i specify exact class name ?14:19
nightmare_unreallike in your example14:19
sean-k-mooneyyes14:20
nightmare_unrealawesome, thanks14:20
sean-k-mooneythe quote are jsut so the shell does not interperate the | as a pipe14:20
sean-k-mooneyso in this case is an or in the regex so that will run all gets with compute or libvirt in the fully qulified function name14:20
*** ileixe has quit IRC14:21
nightmare_unrealunderstood14:21
gibidansmith, sean-k-mooney: from technical perspective I'm OK to have PUT or PATCH as well. I was convinced about both at some point in the past. From process perspective I feel bad that we might change direction of the API design 2 weeks before FF.14:21
gibiI will raise the discussion again on the team meeting as I hope gmann will be there14:22
dansmithgibi: I would hate to change our API so significantly on a process technicality14:22
*** alex_xu has quit IRC14:22
dansmithI'd much rather help brinzhang_ get it right, and/or approve a little extra time14:22
dansmiththe API stability is serious business to me and much more important to get it right than not14:23
gibidansmith: OK that is a good point. Let's double check with gmann but I'm OK to give extra time after FF if brinzhang_ needs time to change14:24
dansmithmeaning, it's not easily changeable later to allow something to land before a deadline14:24
dansmithwe have enough weird "huh, why is this different than *everything* else" warts on the API.. let's not make it worse14:24
openstackgerritKevin Zhao proposed openstack/nova master: fix scsi disk unit number of the attaching volume when cdrom bus is scsi  https://review.opendev.org/71260714:25
PetrTumasean-k-mooney Hello, I had some time this week to test the https://review.opendev.org/#/c/703116/3 and the related issue in Rocky. I was able to reproduce original issue (thanks again for the help last week) and now I'm back to testing of the fix for that issue. Once again I updated code in my nova containers and I'm testing the rebuild between two14:25
PetrTumaimages (that differ in hw_numa_nodes and few other NUMA related properties). So far none of the rebuilds had been blocked. I added some extra logging to see whether the _validate_numa_rebuild method is actually called or not. It looks like it isn't, I don't see my message logged in any logs. I'm quite at loss what might be wrong. Do you have any14:25
PetrTumaidea?14:25
sean-k-mooneyyou updated the nova api container right14:26
PetrTumayes14:26
sean-k-mooneyjust checking you didnt update the compute one :)14:26
sean-k-mooneyPetrTuma: what release is it?14:27
PetrTumaRocky14:27
dansmithgibi: do you want me to circle back on that review and re-affirm my -1 with a summary here, or wait for the meeting or something else?14:28
sean-k-mooneyi tihnk rocky has the chagne that force the image to be checked if it changes14:28
sean-k-mooneyin older releases we did not do that14:28
*** sapd1 has joined #openstack-nova14:28
sean-k-mooneydansmith: you could propose a minor update to the spec with what you want to see?14:29
dansmithI could14:29
*** yankcrime has quit IRC14:29
openstackgerritMerged openstack/nova master: Non-Admin user can filter their instances by more filters  https://review.opendev.org/70160914:29
openstackgerritMerged openstack/nova master: Add default cpu model for AArch64  https://review.opendev.org/70949414:29
*** corvus has quit IRC14:30
*** mmethot has quit IRC14:30
PetrTumasean-k-mooney do you mean other change than the one I use?14:30
*** mkrai has quit IRC14:31
*** mkrai_ has joined #openstack-nova14:31
*** corvus has joined #openstack-nova14:31
hrwyes!14:31
gibidansmith: I would be glad for a summary back on the review14:31
sean-k-mooneyPetrTuma: yes so a cople of release ago we made a change to cause rebuild with a different image to query the schduler to determin if the current host was valid14:32
dansmithgibi: okay14:32
gibidansmith: thank you14:32
sean-k-mooneyPetrTuma: that is one possiblity as to why this code is not beeing called. if it is not in rocky. although i tought that change was older then that14:32
gibidansmith: I will try to reach gmann to comment back14:32
gibidansmith: either on the meeting or separately14:33
PetrTumasean-k-mooney Ok, I'll try to find the change and confirm this, thanks for the moment14:34
*** hrw has quit IRC14:35
gmannreading logs...14:36
*** TxGirlGeek has joined #openstack-nova14:36
sean-k-mooneyPetrTuma: the check for the need to schdule happens here https://zuul.opendev.org/t/openstack/build/cfebf394494246beabb5f417ec1d5c64/logs which is after the check i added14:37
sean-k-mooneyPetrTuma: which is here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3484-L348514:37
sean-k-mooneyPetrTuma: so that is not the issue14:37
*** yankcrime has joined #openstack-nova14:38
sean-k-mooneyPetrTuma: so the only condition that should prevent the _validate_numa_rebuild function running is  if orig_image_ref != image_href:14:39
dansmithgibi: done14:41
openstackgerritjayaditya gupta proposed openstack/nova master: Support for nova-manage placement heal_allocations --cell  https://review.opendev.org/71445914:44
gibidansmith: thanks again. and sorry that I'm more tense than usual14:45
dansmithgibi: np, I understand the pressure14:45
gibiI guess some of it is new to me :)14:46
dansmithI believe you signed up for it :)14:46
gibiI did14:47
gibiand I'm learning every day14:47
gmanndansmith: gibi sean-k-mooney actually PUT is swap volume thing which is not actually updating resources but kind of migrating the storage from your machine. So PUT(swap) is always confusing and complex API from nova. I am not sure how useful that is for people. I remember a bug when some of our customer trying swap back and forth and getting error.14:47
gmannmy main concern on not using PUT was-  not to make this API over scoped and keep it for swap only.14:47
gmanni am ok with any method, PUT also ok for consistency  to our APIs but new one not in existing PUT which is swap14:48
gmanncan we do it via server PUT ?14:49
*** brinzhang_ has quit IRC14:49
*** brinzhang_ has joined #openstack-nova14:49
sean-k-mooneygmann: but thats the thing PUT should not be swap. PUT + changeing the volume id sure14:49
sean-k-mooneybut PUT alone should not have been swap14:49
dansmithI have an important call in ten minutes which I need to get ready for and pay close attention to, but after that I'll be back to discuss further if we need14:50
gmannyeah that is not best design but we cannot change that now but can avoid making it more comlex14:50
gmanndansmith: sure.14:51
dansmithIMHO, anything that requires the client to do something different than the obvious thing of PUTting the resource with delete-on-termination changed is more complex14:51
sean-k-mooneygmann: acutlly we could change it in a microversion but not in ussuri at this point. i do tend to agree with ^ on that point14:52
sean-k-mooney* with dansmith14:53
*** dklyle has joined #openstack-nova14:53
gmannyou mean making swap as action(though action are not good but at least consistent to our API ) and PUT for delete-on-termination changed or any future modification in volume things ?14:54
sean-k-mooneygmann: we dont need to make it an action although that is one option. we just need to stop thing of PUT as swap14:54
johnthetubaguyisn't the simplest just that PUT does different things if the body includes volume_id or delete_on_termination14:54
gmanndansmith: +1 on that. but does not client will be confused with PUT = swap + actual updating..14:54
sean-k-mooneyPUT is just an update and if put change the volume id its sideffect is a swap underneat14:55
gmannjohnthetubaguy: in implementation, there is no challenge it is easy. i am thinking on usage point of view if we are making this API over-scoped14:55
sean-k-mooneyjohnthetubaguy: well it does one thing update the attachment recored, and that update can have other sideffect such as swap14:55
sean-k-mooneyif we consider the attachment ot be declaritive rahter then inperitive this is totally natural14:56
gmanni think swap storage is very explicit things  to use and adding it as side-effect behind some operation is not good API14:56
johnthetubaguyfor me its a consistency thing, PUT for partial update is what we do on other resources14:57
sean-k-mooneyPUT sematicaly is not ment ot be a partil update but it is how we use it yes14:57
gmannyeah, i completely agree on that. but we have used this PUT for swap that is the only things i am worried about.14:57
johnthetubaguygmann: there are two ways of thinking about this though, they are both a partial update of the attachment, one just does more things than the other14:58
*** ociuhandu has quit IRC14:58
johnthetubaguyif we ignore the existing Cinder specific API, we wouldn't be talking about this right, its just a PUT for every other API, and I think that is the simplest thing for 99% of our API users14:59
johnthetubaguybut hey, best to discuss later I suspect14:59
gmanncan we move the swap to action API and use PUT for these kind of updates ? like our host/services/hypervisors APIs were not good and we combined them .14:59
*** ociuhandu has joined #openstack-nova15:00
*** Sundar has joined #openstack-nova15:00
johnthetubaguygmann: certainly an option, but its a cinder only, largely internal to OpenStack API, that frankly I am tempted to remove from the API docs to avoid confusion15:00
sean-k-mooneygmann: we could but what benifit does that serve15:00
johnthetubaguyseems like busy work to me15:01
johnthetubaguywe don't have bandwidth for the pants of fire stuff right now15:01
*** macz_ has joined #openstack-nova15:01
gmannok15:02
*** macz_ has quit IRC15:02
openstackgerritMerged openstack/nova master: Add transform_image_metadata request filter  https://review.opendev.org/66577515:02
*** macz_ has joined #openstack-nova15:02
Sundardansmith, sean-k-mooney, gibi, brinzhang_, alex_xu: I am creating a list of followups to the Cyborg-Nova patch series: https://etherpad.openstack.org/p/cyborg-nova-followup . It is WIP. But, if you have any comments on the structure of the doc or the categories of the tasks, please LMK.15:05
gibiSundar: ack15:06
gmann if we consider volume swap from nova perspective/API as just an update to resource then i am ok and hope it does not confuse users. having clear doc about what this API does as per different ways of request.15:08
gmanngibi: brinzhang_ on instance event things. +1 on having a clear doc to explain how operator can use policy in which situation and with risk.15:10
gmanngibi: brinzhang_ I was waiting for dansmith reply on that- if we can exclude non-nova exception from 'details' and only show nova exception ?. though it will be much clear if we can exclude few nova exceptions also which are not fixable by non-admin.15:11
*** KeithMnemonic has quit IRC15:13
gmannif that is hard to do/decide the gray list of exceptions for non-admin then I am ok with only have a clear policy doc saying the risk and usage of this policy.15:16
nightmare_unrealmriedem:  how will I know what resource value to overwrite ? I am refering to TO-DO overwrite allocations15:18
*** dswebb has joined #openstack-nova15:18
mriedemnightmare_unreal: i guess your test would need to change something about the instance allocations out of band (via the placement API directly) and then run heal_allocations with your new flag which will overwrite those allocations back to the instance flavor15:21
*** gyee has joined #openstack-nova15:21
Sundarsean-k-mooney: Re. https://review.opendev.org/#/c/673735/46/nova/conductor/manager.py@1632, since this is the instance creation code path, failures will cause rescheduling rather than put the instance in error status for the user to clean up, right?15:22
mriedemso i guess you could like simulate a busted same-host resize and double the allocation values for the instance? run heal and then assert the allocations are back to the instance.flavor values15:22
nightmare_unreali have not yet reached to write test :/ , was fixing my previous patch. Yet to write code for the overwrite.15:23
*** tkajinam has quit IRC15:30
mriedemok well i think the actual change is just plumbing a new type of force or overwrite flag or something down to where that conditional is i showed you the other day15:31
mriedemwhere it determines if it should call put_allocations or not15:31
nightmare_unrealyes I think so too :)15:31
nightmare_unrealafk15:31
nightmare_unrealThanks15:32
openstackgerritStephen Finucane proposed openstack/nova master: tox: Integrate mypy  https://review.opendev.org/67620815:32
openstackgerritStephen Finucane proposed openstack/nova master: hardware: Update and correct typing information  https://review.opendev.org/71469415:32
openstackgerritStephen Finucane proposed openstack/nova master: libvirt: Add typing information  https://review.opendev.org/71469515:32
openstackgerritStephen Finucane proposed openstack/nova master: tests: Split instance NUMA object tests  https://review.opendev.org/71469615:32
openstackgerritStephen Finucane proposed openstack/nova master: objects: Replace 'cpu_pinning_requested' helper  https://review.opendev.org/71469715:32
openstackgerritStephen Finucane proposed openstack/nova master: hardware: Don't consider overhead CPUs for unpinned instances  https://review.opendev.org/71469815:32
openstackgerritStephen Finucane proposed openstack/nova master: hardware: Remove handling of pre-Train compute nodes  https://review.opendev.org/71469915:32
openstackgerritStephen Finucane proposed openstack/nova master: hardware: Add validation for 'cpu_realtime_mask'  https://review.opendev.org/46820315:32
openstackgerritStephen Finucane proposed openstack/nova master: hardware: Tweak the 'cpu_realtime_mask' handling slightly  https://review.opendev.org/46145615:32
openstackgerritStephen Finucane proposed openstack/nova master: hardware: Rework 'get_realtime_constraint'  https://review.opendev.org/71470015:32
openstackgerritStephen Finucane proposed openstack/nova master: hardware: Invert order of NUMA topology generation  https://review.opendev.org/71470115:32
gibigmann: do I understand correctly that you also got convinced that the PUT solution is OK15:33
gibi?15:33
*** sapd1 has quit IRC15:36
gmanngibi: yeah. I am ok to add in existing PUT as making other PUT or action API is a big change from implementation and users perspective. having api-ref clear about this PUT = swap + updating resources based on how you use this API.15:36
gibigmann: ack, thanks15:37
*** ociuhandu has quit IRC15:37
gmannthough it makes this API more complex but its tradeoff as dansmith mentioned that any other way is not less complex for client.15:37
*** ociuhandu has joined #openstack-nova15:38
* gmann hope we can deprecate the swap operation some time :)15:38
*** ociuhandu has quit IRC15:39
sean-k-mooneygmann: i think calling it a swap + updating resources is still the wrong way to think about it15:40
gmanngibi: on the instance fault things: i will wai for dansmith to come back and discuss if somehow we can minimize the info leak to non-admin.  other part like policy things is ok for me.15:40
sean-k-mooneygmann: it is just declaritivly updatin the resouces and a sidefect of that can be to trigger swap15:40
*** ociuhandu has joined #openstack-nova15:40
sean-k-mooneythat is how we should explain it to endusers15:40
gibigmann: re instance fault: thanks15:40
johnthetubaguygmann: my preference is to not document the crazy cinder API, and move that to some developer docs or something?15:41
johnthetubaguyshould stop some confusion for the 99% of API users15:41
sean-k-mooneyjohnthetubaguy: do end users ever call that directly15:41
gmannsean-k-mooney: humm, having the swap as side-efect is my concern. that is big change  to enduser machine so it has to be very explicit operation from usage doc side.15:41
*** ociuhandu has quit IRC15:41
johnthetubaguysean-k-mooney: they do, but they never should15:41
johnthetubaguyits for cinder only, else odd things happen15:41
sean-k-mooneygmann: not really that is how i alwasy tought of it already15:42
sean-k-mooneyjohnthetubaguy: ok so this is just for the callback form cinder15:42
johnthetubaguythat is my understanding, yes15:42
johnthetubaguyhence the big red warning15:42
gmannjohnthetubaguy: can we make that internal than ? at least from doc side. i mean api-ref not at all talk about swap operation for users as currently we have ?15:43
sean-k-mooneyjohnthetubaguy: and yes im pretty sure we have had downstream bugs filed as a result of users calling it directly15:43
johnthetubaguygmann: +1 that15:43
gmannsean-k-mooney: yeah we had few in past15:43
johnthetubaguysean-k-mooney: yeah, what they wanted to do was call the Cinder swap API, but they got confused15:43
gibifor me the difference is like change my house (swap) or repaint the kitchen in my house (update)15:43
sean-k-mooneyactully i think our customer wanted to bypass a check in the ciner side a and force it so that is why they used nova15:44
johnthetubaguygibi: or you swap out your oven to a hot one, or just change tell it to self destruct when your pizza is done. its the same thing you are changing.15:45
*** priteau has joined #openstack-nova15:45
openstackgerritStephen Finucane proposed openstack/nova master: api: Add framework for extra spec validation  https://review.opendev.org/70464315:45
openstackgerritStephen Finucane proposed openstack/nova master: api: Add microversion 2.84, extra spec validation  https://review.opendev.org/70843615:45
sean-k-mooneyanyway when we have had issue related to swap volume we havne pretty much always fixed it once for them and told them not to do that again15:45
openstackgerritStephen Finucane proposed openstack/nova master: docs: Add documentation for flavor extra specs  https://review.opendev.org/71003715:45
stephenfingibi: I'd to rebase that ^ after losing the race for 2.83. Could you hit https://review.opendev.org/#/c/708436/9 again?15:45
johnthetubaguysean-k-mooney: yeah, sounds like they were doing something bad. something we shouldn't tell them to do in the docs15:46
stephenfinand johnthetubaguy, any chance you'll stick that on your list? iirc, that was also on your list of gripes ^15:46
gibijohnthetubaguy: yeah like that. replace the whole v.s. tweak a prt of the whole15:46
gibistephenfin: sure15:46
johnthetubaguystephenfin: well played, and yes on both counts15:46
gibistephenfin: I think I caused you to lose :)15:47
stephenfinwell, thankfully it's just lyarwood and I duking it out for 2.84 now, afaict :)15:47
* stephenfin throws shiny thing lyarwood's direction15:48
gibi:)15:48
*** sapd1 has joined #openstack-nova15:49
*** ociuhandu has joined #openstack-nova15:49
*** udesale_ has quit IRC15:50
sean-k-mooneyjohnthetubaguy: have you had any futher issues with the libosinfo feature where we try to set some default based on the disto name/version15:50
*** PetrTuma has quit IRC15:50
johnthetubaguysean-k-mooney: mostly given up on it I think, not had time to dig15:51
sean-k-mooneyjohnthetubaguy: we backported the fix for the last one downstream and fond that new libosinfo broke use in another way15:51
sean-k-mooneyjohnthetubaguy: ok so i would like to remove it in victoria since it was broken from the start given it ignores architeture and machine type15:51
sean-k-mooneywould that cause issues for you if we did that15:52
johnthetubaguysean-k-mooney: it would make my life easier I think15:52
*** ociuhandu has quit IRC15:52
sean-k-mooneywe likely would fix the issue we hit so we can backport then kill it with fire in the next patch15:52
johnthetubaguysounds like a good idea, and just replace it with some decent documentation15:52
sean-k-mooneyya we have explict image properties to set everyting it can set anyway15:53
gibistephenfin: done.15:53
stephenfinthanks15:53
sean-k-mooneyand i alreay recommend people to use those anyway since libosinfo change its default out of our contol anyway15:54
gibinova meeting starts in 5 and a half minutes on #openstack-meeting-315:54
*** mkrai_ has quit IRC15:57
*** mlavalle has joined #openstack-nova16:00
*** ociuhandu has joined #openstack-nova16:00
*** TxGirlGeek has quit IRC16:02
*** TxGirlGeek has joined #openstack-nova16:03
*** spatel has joined #openstack-nova16:10
lyarwoodstephenfin: oOOoOoo shiny.. WAIT A MINUTE!16:10
lyarwoodstephenfin: feel free to land on 2.84 btw, I need to rework something in my series today anyway16:11
lyarwoodstephenfin: if it's just the two of us I'll land on 2.8516:11
openstackgerritStephen Finucane proposed openstack/nova master: Use neutronclient's port binding APIs  https://review.opendev.org/70629516:12
*** ociuhandu has quit IRC16:16
artomstephenfin, for the record, I hate https://review.opendev.org/#/c/631053/ and everything it stands for16:19
artomI'll still review it because I started, but I will forever hold this grudge in my hear.16:19
artom*heart.16:19
stephenfin🕺🕺🕺16:20
artom... man dancing?16:21
openstackgerritHuaqiang Wang proposed openstack/nova master: Take the instance dedicated CPU list from 'cpu_pinning' if possible  https://review.opendev.org/71335216:25
openstackgerritHuaqiang Wang proposed openstack/nova master: libvirt: support to create instance with dedicated and shared CPUs  https://review.opendev.org/71465516:25
openstackgerritHuaqiang Wang proposed openstack/nova master: 'cpu_pinning_requested' property could be derived directly from cpu_policy  https://review.opendev.org/71335316:25
openstackgerritHuaqiang Wang proposed openstack/nova master: Remove 'InstanceNUMACell.cpu_pinning_requested' field  https://review.opendev.org/71465616:25
openstackgerritHuaqiang Wang proposed openstack/nova master: Refactor the code in checking available host CPUs  https://review.opendev.org/71465716:25
openstackgerritHuaqiang Wang proposed openstack/nova master: Introduce 'MIXED' CPU allocation policy for instance  https://review.opendev.org/71335416:25
openstackgerritHuaqiang Wang proposed openstack/nova master: Introduce the interface of creating 'MIXED' policy instance through 'PCPU' and 'VCPU'  https://review.opendev.org/71335516:25
openstackgerritHuaqiang Wang proposed openstack/nova master: metadata: export the vCPU IDs that are pinning on the host CPUs  https://review.opendev.org/68893616:25
openstackgerritHuaqiang Wang proposed openstack/nova master: [WIP] An alternative way for keeping instance dedicated CPUs  https://review.opendev.org/71465816:25
*** ociuhandu has joined #openstack-nova16:26
gmannjohnthetubaguy: you already have noticed this. I added no-deprecation tests for admin-api also which does not make a difference than enable-scope tests but still to check if nothing weird happens when remove deprecation.  - https://review.opendev.org/#/c/715085/2/nova/tests/unit/policies/test_instance_usage_audit_log.py@9916:26
gmannhope that is fine, in some cases it is just removing the deprecatiob from base rule and checks16:26
*** dpawlik has quit IRC16:26
johnthetubaguygmann: yeah, seemed good to check anyways16:26
gmannok16:26
johnthetubaguyits a bit like nova-next tests16:27
gmannyeah16:27
huaqiangstephenfin: I updated the series for 'mixed policy instance', but only the last patch I changed this time16:27
huaqianghttps://review.opendev.org/714658 [WIP] An alternative way for keeping instance dedicated CPUs16:27
huaqiangthis 'pcpuset approach' one,16:28
huaqiangIf you have time please have a look for this patch,16:28
huaqiangI just want your comments for if it is ready to drop the 'cpu_pinning' approach now16:29
huaqiangit the answer is comfirmative, I'll drop it and submit new series.16:30
huaqiangthis patch is still a 'WIP' patch, I still have several 'TODO's, I'll address tomorrow.16:32
openstackgerritJohn Garbutt proposed openstack/nova master: Enforce resource limits using oslo.limit  https://review.opendev.org/61518016:32
openstackgerritJohn Garbutt proposed openstack/nova master: Add legacy limits and usage to unified limits  https://review.opendev.org/71349816:32
openstackgerritJohn Garbutt proposed openstack/nova master: Update quota apis with keystone limits and usage  https://review.opendev.org/71349916:32
*** mgariepy has quit IRC16:36
*** ociuhandu has quit IRC16:36
*** mgariepy has joined #openstack-nova16:43
openstackgerritJohn Garbutt proposed openstack/nova master: Tell oslo.limit how to count nova resources  https://review.opendev.org/71330116:48
*** ociuhandu has joined #openstack-nova16:49
openstackgerritJohn Garbutt proposed openstack/nova master: Enforce resource limits using oslo.limit  https://review.opendev.org/61518016:49
openstackgerritJohn Garbutt proposed openstack/nova master: Add legacy limits and usage to unified limits  https://review.opendev.org/71349816:49
openstackgerritJohn Garbutt proposed openstack/nova master: Update quota apis with keystone limits and usage  https://review.opendev.org/71349916:49
gmanndansmith: johnthetubaguy on  - delete_on_termination via PUT, do we want to control it via separate policy or with swap operation policy which is admin by default - https://review.opendev.org/#/c/693828/19/nova/api/openstack/compute/volumes.py@49416:53
johnthetubaguyits a new policy I think, admin_or_owner one16:53
johnthetubaguyprobably need to rename the old one I guess16:53
*** ociuhandu has quit IRC16:53
gmannok.16:54
dansmithyeah, new policy16:54
johnthetubaguygmann: update -> swap, new one called "put" ?16:55
gmanni was thinking new one to 'update' but that is existing and conflcit at deprecation logic16:56
johnthetubaguyyeah, agreed16:56
gmann'put' will be inconsistentname from other one16:56
gmann'os-volumes-attachments:volume:update' for new ?16:57
gmannvolume is redundant though16:57
johnthetubaguythat seems more inconsistent though16:57
gmann:)16:57
dansmithI would think that it would be okay to rename the existing one, given the conflict16:58
dansmithwith a reno16:58
dansmiththe existing one, if opened to non-admin users wouldn't be terrible if it applied to the delete flag, and probably expected16:59
johnthetubaguyyeah, I am tempted to just reno about the conflict16:59
gmannok, deprecation message saying this old policy is being used for this new operation ?16:59
dansmithin all likelihood nobody has changed this one since it _is_ supposed to be for cinder only16:59
dansmithgmann: maybe not deprecation, but a warning if it's set to non-default?16:59
*** TxGirlGeek has quit IRC16:59
johnthetubaguynah, skip deprecation, make existing "update" PROJECT_MEMBER_OR_SYSTEM_ADMIN, then add new rule for swap16:59
*** TxGirlGeek has joined #openstack-nova17:00
dansmithyeah, that17:00
gmannok, that is better.17:00
johnthetubaguythey shouldn't have changed it, if they did, they should read the reno note17:00
dansmithexactly17:01
dansmithif this was for any other thing I would take more care, but this is suuuper obscure and special17:01
gmannand cinder will keep  working as new defaults keep allowing admin to access17:02
dansmithyeah17:02
gmannok adding comment on the review for brinzhang_17:03
sean-k-mooneygibi: looking at that os-vif repo we have not merged any patches since we did the last release so i think we can stick with the 2.0.0 release for m3 which we created at m217:03
sean-k-mooneygibi: if we do merge something between no and next week i can request a release but for now we are good.17:04
*** Sundar has quit IRC17:08
*** rpittau is now known as rpittau|afk17:12
openstackgerritJohn Garbutt proposed openstack/nova master: Add reno for unified limits  https://review.opendev.org/71527117:14
*** lpetrut has quit IRC17:21
*** tesseract has quit IRC17:30
*** ccamacho has joined #openstack-nova17:31
*** TxGirlGe_ has joined #openstack-nova17:31
*** dtantsur is now known as dtantsur|afk17:32
*** ccamacho has quit IRC17:32
*** TxGirlGeek has quit IRC17:41
*** evrardjp has quit IRC17:49
*** evrardjp has joined #openstack-nova17:49
gmanndansmith: please let me your opinion on this, 'not expose the non-nova exception name to non-amdin' - https://review.opendev.org/#/c/694428/9/nova/objects/instance_action.py@20017:49
*** openstack has quit IRC17:49
*** openstack has joined #openstack-nova17:51
*** ChanServ sets mode: +o openstack17:51
*** ociuhandu has quit IRC17:51
dansmithmnaser: right, which is why we hide the message now17:52
dansmithmnaser: the question is whether or not hiding the actual name of the exception (i.e. LibvirtError) is a problem and I assert that it is not17:52
mnaserdansmith: i agree with that, especially if its a libvirt-specific error17:53
mnasermakes life easier17:53
sean-k-mooneymnaser: to be fair the cve that we had in the past was not acatlly as sever as the bug suggested since the cpeh monitor details specifcaly the ip are also availabel to non admins via the attachment which they can see. but just reporting the name avoids that entirely17:57
sean-k-mooneythe only infomation leak that actully causes was the name of the ceph keyfile, still not ideal.17:59
sean-k-mooneybut that is fixed in that case at least17:59
*** derekh has quit IRC18:00
gmannmnaser: does any user ask about what driver you use for your cloud and my VM will be running on? before they buy :)18:02
gmannor hypervisor18:02
openstackgerritSasha Andonov proposed openstack/nova master: rbd_utils: increase _destroy_volume timeout  https://review.opendev.org/70576418:02
mnasergmann: i don't think we've actually ever had a customer ask what hypervisor/backend storage/etc18:03
sean-k-mooneygmann: in principaly they should not care18:03
gmannyeah, they should not. I was curious if they make the decision based on that.18:04
sean-k-mooneythey might make a dession based on specific features18:04
sean-k-mooneywhich might only work with specific hyperviors but if vmware and libvirt but suppoted the features/api actions they cared about they proably would not care as long as there application worked fine18:05
sean-k-mooneyi mean i think aws uses xen in some from and google cloud suses some form of kvm with its own lightweight qemu alternitive but i dont think most people care when they use either product18:06
gmannin one of our RFP, the customer asked to provide hypervisor-based choice to the user request of VM. not sure how good/bad/usable that was.18:08
gmannthough I am not sure it was user request or just customer thought.18:09
sean-k-mooneywell that is not realy that hard to do you just use multple virt dirvers in one cloud18:09
sean-k-mooneythere are then several filter you can use18:09
sean-k-mooneythe over used example is running windows instnace on hyperv but you can use custom extra specs in flaovr  or a trait today to support that18:11
*** links has quit IRC18:23
*** amoralej is now known as amoralej|off18:30
*** nightmare_unreal has quit IRC18:41
*** ralonsoh has quit IRC18:51
mnasersean-k-mooney: aws has moved away from xen btw :p18:58
*** tbachman has quit IRC19:01
*** maciejjozefczyk has quit IRC19:02
sean-k-mooneymnaser: good to know but im not sure there customer will notice which was kind of my point. to them the use an awx m1.whatever instnace19:03
mnasersean-k-mooney: yep agreed19:04
*** tbachman has joined #openstack-nova19:13
*** ociuhandu has joined #openstack-nova19:44
*** martinkennelly has quit IRC19:48
*** ociuhandu has quit IRC19:55
sean-k-mooneydansmith: im currently testing resculde. it seams to work but sofar i have just added a raise ValueError at the top of spawn in the libvirt driver20:06
dansmithcool20:06
sean-k-mooneythe allocation and arq were correctly created against the second host20:06
sean-k-mooneyis there anything else you want me to check specificlly20:06
dansmithon reschedule?20:08
dansmithI forget, did you try cold migration?20:08
sean-k-mooneyno but i can. i thikn we expect that to fail like evaucate and resize fails.20:09
*** larainema has quit IRC20:09
dansmithah okay.. I can see migration working and resize not, but only in a few scenarios, so if you did that already that's fine20:09
sean-k-mooneydansmith: this is the current list of followups that we need to adress after the current series is merged https://etherpad.openstack.org/p/cyborg-nova-followup20:09
dansmithevac is a little different since it doesn't get help from the original node20:10
dansmithsean-k-mooney: ah sweet, hadn't seen this20:10
sean-k-mooneyya so evac works but it does not claim an fpga on the dest or update the arq20:10
sean-k-mooneydansmith: sundar created it todeay20:10
sean-k-mooney*today20:10
dansmithdoesn't try to update the arq, or fails because the old one isn't deleted?20:11
sean-k-mooneygood question i will check but since the placemnt allocation does not have a device i suspect it doesnt even try20:11
sean-k-mooneydansmith: the rebuild path does not have to do any arq updates20:11
sean-k-mooneyso i suspect it does not try20:11
dansmithbut we scheduled to pick a new host,20:12
dansmithbut yeah, fair point20:12
sean-k-mooneyill go check and let you know20:12
dansmithI'm sure there are plenty of things missing, but yeah it'd be good to know where we're starting from20:14
sean-k-mooneydansmith: ah so cold migration is blocked in the code :)20:17
sean-k-mooneyForbidden with instances that have accelerators. (HTTP 403) (Request-ID: req-c318fb6b-730c-4bd3-9ddf-cb7de6e472a3)20:17
dansmithah okay, I haven't really gotten to that patch at all20:17
dansmithso... good :)20:17
sean-k-mooneyso that works at least although the respocne code still looks wrong to me20:17
sean-k-mooneyit should proably be a 400 or 40920:17
dansmithI thought we agreed on 403?20:22
sean-k-mooneywe proably did20:22
sean-k-mooneyi just find 403 forbiden which is normaly used for auth issues confusing20:23
sean-k-mooneyi would expect to get 403 if i did not have enough permission to do something not because its unsupported20:23
dansmithIIRC, 401 means "authorization required" and 403 means "not permitted" - the latter which may or may not be because of insufficient or missing credentials20:26
sean-k-mooneyhttps://developer.mozilla.org/en-US/docs/Web/HTTP/Status/40320:26
dansmith"401 Unauthorized: If the request already included Authorization credentials, then the 401 response indicates that authorization has been refused for those credentials. 403 Forbidden: The server understood the request, but is refusing to fulfill it"20:26
dansmith"tied to the application logic"20:27
sean-k-mooneyya it gives us leway to use it this way20:27
dansmith"such as (but not limited to) insufficient rights"20:27
sean-k-mooneyim just used to thinking about it as related to authorisation20:28
melwittzzzeek: I've been tracing the gate bug situation with DNM debug logging and the latest thing I found is that this call to _create_session never returns https://github.com/openstack/oslo.db/blob/master/oslo_db/sqlalchemy/enginefacade.py#L657-L658 any ideas how or why that could happen?20:28
dansmithI think a lot of things don't use 401 and 403 properly, so it's common to not have a gut feeling about the difference20:28
*** ociuhandu has joined #openstack-nova20:31
sean-k-mooneyso this is interesting GET /placement/allocation_candidates?limit=1000&resources=DISK_GB%3A1%2CMEMORY_MB%3A64%2CVCPU%3A1&root_required=COMPUTE_ACCELERATORS%2C%21COMPUTE_STATUS_DISABLED20:33
sean-k-mooneythe placement request for the evacuate gets teh trait added by the prefilter20:33
sean-k-mooneybut not the resouce requests20:33
dansmithyeah, that's weird... that means the prefilter is seeing the device profile in the flavor20:37
sean-k-mooneyyep20:37
*** ociuhandu has quit IRC20:37
sean-k-mooneyim not seeing any attepmet to update teh ARQ20:37
sean-k-mooneydid we move to putting the resouce groups form the device profile into the request spec20:38
melwittgmann: I have a followup patch here related to the new policy rule I added earlier in the cycle, would appreciate your review https://review.opendev.org/71329520:38
*** priteau has quit IRC20:41
sean-k-mooneyinteresting the "requested_resources" is null http://paste.openstack.org/show/791213/20:44
sean-k-mooneywe do populate the accel_info in the resouces https://review.opendev.org/#/c/631244/69/nova/compute/manager.py@260420:48
sean-k-mooneybut i guess we never save that sice we call save on line 257020:50
sean-k-mooneyactully saving the instance in not important20:53
*** huaqiang has quit IRC20:54
*** lbragstad has quit IRC20:58
*** amodi has quit IRC20:59
*** zhanglong has joined #openstack-nova21:06
*** huaqiang has joined #openstack-nova21:19
*** ociuhandu has joined #openstack-nova21:22
*** ociuhandu has quit IRC21:26
*** huaqiang has quit IRC21:35
*** zhanglong has quit IRC21:36
*** ociuhandu has joined #openstack-nova21:44
openstackgerritMerged openstack/nova master: nova-net: Remove unused parameters  https://review.opendev.org/70397421:46
sean-k-mooneydansmith: found the issue i think. https://github.com/openstack/nova/blob/9d212738bec63d7490998a2840598d790a3c94fc/nova/conductor/manager.py#L1121-L112821:46
*** mriedem has left #openstack-nova21:56
*** spatel has quit IRC21:58
*** xek__ has quit IRC22:03
*** ociuhandu has quit IRC22:24
*** ociuhandu has joined #openstack-nova22:24
*** rcernin has joined #openstack-nova22:25
*** ociuhandu has quit IRC22:28
*** zhanglong has joined #openstack-nova22:41
*** nweinber has quit IRC22:50
*** tkajinam has joined #openstack-nova22:54
*** slaweq has quit IRC22:59
*** TxGirlGe_ has quit IRC23:03
*** prometheanfire has quit IRC23:04
*** TxGirlGeek has joined #openstack-nova23:04
openstackgerritsean mooney proposed openstack/nova master: [WIP] cyborg evacuate support  https://review.opendev.org/71532623:05
*** macz_ has quit IRC23:06
*** rcernin has quit IRC23:06
*** rcernin has joined #openstack-nova23:07
*** rcernin has quit IRC23:07
*** rcernin has joined #openstack-nova23:08
*** slaweq has joined #openstack-nova23:11
*** slaweq has quit IRC23:15
*** TxGirlGeek has quit IRC23:30
*** TxGirlGeek has joined #openstack-nova23:31
*** tetsuro has joined #openstack-nova23:33
*** lbragstad has joined #openstack-nova23:33
*** tetsuro has quit IRC23:35
*** tetsuro has joined #openstack-nova23:37
*** TxGirlGeek has quit IRC23:37
*** TxGirlGeek has joined #openstack-nova23:38
*** vishalmanchanda has quit IRC23:39
*** tetsuro has quit IRC23:47
gmannmelwitt: +1. lgtm23:48
melwittthanks23:48
*** zhanglong has quit IRC23:48
*** alex_xu has joined #openstack-nova23:53
*** tosky has quit IRC23:53

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