openstackgerrit | melanie witt proposed openstack/nova master: Add info about affinity requests to the troubleshooting doc https://review.opendev.org/715092 | 00: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@51 | 00: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-nova | 00:22 | |
*** ociuhandu has joined #openstack-nova | 00:24 | |
*** macz_ has quit IRC | 00:27 | |
*** ociuhandu has quit IRC | 00:30 | |
*** ociuhandu has joined #openstack-nova | 00:31 | |
*** ociuhandu has quit IRC | 00:36 | |
*** gyee has quit IRC | 00:39 | |
*** lbragstad has quit IRC | 00:46 | |
*** tosky has quit IRC | 00:52 | |
alex_xu | sean-k-mooney: I guess we are missing a patch for evacuate | 00:56 |
*** zhanglong has joined #openstack-nova | 01:10 | |
*** Liang__ has joined #openstack-nova | 01:17 | |
*** TxGirlGeek has quit IRC | 01:20 | |
*** mlavalle has quit IRC | 01:22 | |
*** tetsuro has joined #openstack-nova | 01: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@5781 | 01: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-nova | 01:58 | |
*** ociuhandu has quit IRC | 02:02 | |
*** dswebb has quit IRC | 02:26 | |
*** zhanglong has quit IRC | 02:39 | |
*** mkrai has joined #openstack-nova | 02:40 | |
*** zhanglong has joined #openstack-nova | 02:49 | |
*** larainema has joined #openstack-nova | 03:08 | |
sean-k-mooney | alex_xu: we are not planning to have all ops supported initally so evac can be added later if we block it in the final patch | 03:15 |
sean-k-mooney | but it would be good to add it sooner rather then later | 03:15 |
*** mkrai has quit IRC | 03:20 | |
*** tetsuro has quit IRC | 03:46 | |
*** tetsuro has joined #openstack-nova | 03:49 | |
*** zhanglong has quit IRC | 03:58 | |
*** mkrai has joined #openstack-nova | 04:12 | |
*** udesale has joined #openstack-nova | 04:47 | |
*** tonyb[m] has quit IRC | 04:49 | |
*** tonyb[m] has joined #openstack-nova | 04:49 | |
*** udesale has quit IRC | 05:10 | |
*** links has joined #openstack-nova | 05:12 | |
*** udesale has joined #openstack-nova | 05:12 | |
*** ratailor has joined #openstack-nova | 05:24 | |
*** evrardjp has quit IRC | 05:36 | |
*** evrardjp has joined #openstack-nova | 05:36 | |
*** zhanglong has joined #openstack-nova | 05:37 | |
*** rcernin has quit IRC | 05:40 | |
*** rcernin has joined #openstack-nova | 05:41 | |
*** rcernin has quit IRC | 05:41 | |
*** rcernin has joined #openstack-nova | 05:42 | |
*** rcernin has quit IRC | 05:42 | |
*** rcernin has joined #openstack-nova | 05:47 | |
*** rcernin has quit IRC | 06:02 | |
*** rcernin has joined #openstack-nova | 06:02 | |
*** rcernin has quit IRC | 06:05 | |
*** rcernin has joined #openstack-nova | 06:05 | |
openstackgerrit | Luyao Zhong proposed openstack/nova master: support live migration with vpmems https://review.opendev.org/687856 | 06:08 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Track orphan instances and error migrations in resource tracker https://review.opendev.org/714653 | 06:08 |
*** nightmare_unreal has joined #openstack-nova | 06:51 | |
*** dklyle has quit IRC | 06:53 | |
*** ociuhandu has joined #openstack-nova | 07:03 | |
*** ociuhandu has quit IRC | 07:07 | |
*** ralonsoh has joined #openstack-nova | 07:09 | |
*** xek_ has joined #openstack-nova | 07:20 | |
*** dpawlik has joined #openstack-nova | 07:22 | |
*** factor has quit IRC | 07:29 | |
*** mkrai has quit IRC | 07:40 | |
*** belmoreira has joined #openstack-nova | 07:44 | |
*** tesseract has joined #openstack-nova | 07:53 | |
*** maciejjozefczyk has joined #openstack-nova | 07:56 | |
*** vishalmanchanda has joined #openstack-nova | 07:56 | |
*** sapd1 has joined #openstack-nova | 07:57 | |
*** evrardjp has quit IRC | 07:57 | |
*** evrardjp has joined #openstack-nova | 08:01 | |
gibi | good morning nova | 08:01 |
*** slaweq has joined #openstack-nova | 08:01 | |
*** amoralej|off is now known as amoralej | 08:02 | |
brinzhang_ | gibi: good morning ^^ | 08:03 |
* nightmare_unreal waves \0 | 08:05 | |
gibi | o/ | 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 irc | 08:05 |
gibi | brinzhang_: OK. Will you be able to participate or I could you update me shortly what the disagreemen is about? | 08:06 |
*** guilhermesp has quit IRC | 08:06 | |
*** NostawRm has quit IRC | 08:06 | |
gibi | s/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 participate | 08:07 |
gibi | brinzhang_: OK thanks. I will raise it to find a way forward | 08:07 |
*** NostawRm has joined #openstack-nova | 08: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 IRC | 08:09 | |
brinzhang_ | gibi: you can see the irc log above, at 8:09:27-8:10:50 | 08:10 |
gibi | I have to, and I will, read back on the disagreement | 08: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 policy | 08:13 |
brinzhang_ | gibi: you can review https://review.opendev.org/#/c/694430/, thanks | 08:13 |
gibi | brinzhang_: I will | 08:19 |
*** tkajinam has quit IRC | 08:19 | |
*** rpittau|afk is now known as rpittau | 08:26 | |
*** toabctl has quit IRC | 08:30 | |
*** mkrai has joined #openstack-nova | 08:44 | |
*** tosky has joined #openstack-nova | 09:00 | |
*** Liang__ is now known as LiangFang | 09:03 | |
*** zhanglong has quit IRC | 09:09 | |
*** aloga has quit IRC | 09:10 | |
*** macz_ has joined #openstack-nova | 09:10 | |
*** toabctl has joined #openstack-nova | 09:14 | |
*** macz_ has quit IRC | 09:14 | |
*** ociuhandu has joined #openstack-nova | 09:14 | |
*** derekh has joined #openstack-nova | 09:37 | |
*** martinkennelly has joined #openstack-nova | 09:44 | |
*** ociuhandu has quit IRC | 09:48 | |
*** ociuhandu has joined #openstack-nova | 09:49 | |
*** rcernin has quit IRC | 09:51 | |
gibi | gmann, brinzhang_: responeded in https://review.opendev.org/#/c/694430/ | 09:56 |
*** ociuhandu has quit IRC | 09:58 | |
*** LiangFang has quit IRC | 10:06 | |
*** ociuhandu has joined #openstack-nova | 10: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 |
openstackgerrit | Marcin Juszkiewicz proposed openstack/nova master: Add default cpu model for AArch64 https://review.opendev.org/709494 | 10:14 |
hrw | kashyap: took your comments and applied | 10:15 |
kashyap | hrw: Hiya; will look | 10:15 |
kashyap | Thanks! | 10:15 |
gibi | brinzhang_: yeah I'd like to have the new policy to control the exposure of the 'details' field | 10:16 |
brinzhang_ | if we re-using the BASE_POLICY_NAME% 'events' policy, we will difficult to distinguish whether 'traceback' or 'details' expose to non-admin | 10:16 |
brinzhang_ | gibi: yeah, thanks | 10: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 possible | 10:18 |
luyao | stephenfin: Hi, I have addressed alex_xu's comments. :) https://review.opendev.org/#/c/687856 | 10:18 |
gibi | yeah, 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 |
gibi | brinzhang_: exactly | 10:19 |
brinzhang_ | gibi: yeah, we are same, wait for gmann check, he has -1 on the patch | 10:19 |
hrw | kashyap: turns out that we need that patch to get kolla-ansible CI running ;( | 10:20 |
nightmare_unreal | hey how can i create fake cells and create instance in them for functional test. I am looking at nova/tests/functional/test_nova_manage.py | 10:22 |
luyao | lyarwood: 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@8197 | 10:24 |
kashyap | hrw: "That patch" is the above default CPU model thing? | 10:24 |
*** macz_ has joined #openstack-nova | 10:26 | |
kashyap | hrw: Looks good to me; FWIW; once Zuul blesses it, then it can go through | 10:26 |
gibi | brinzhang_: responed in the PATCH API discussion in the review and added the topic for the nova meeting accordingly | 10:27 |
hrw | kashyap: yep | 10:28 |
hrw | kashyap: 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 |
gibi | brinzhang_: ack, have a nice dinner | 10:29 |
nightmare_unreal | nvm found it | 10:29 |
*** macz_ has quit IRC | 10:30 | |
*** tetsuro has quit IRC | 10:34 | |
*** dtantsur|afk is now known as dtantsur | 10:48 | |
openstackgerrit | Merged openstack/nova master: Add Cyborg device profile groups to request spec. https://review.opendev.org/631243 | 10:49 |
*** ileixe has quit IRC | 10:59 | |
*** spatel has joined #openstack-nova | 10:59 | |
*** spatel has quit IRC | 11:03 | |
openstackgerrit | jayaditya gupta proposed openstack/nova master: Support for nova-manage placement heal_allocations --cell https://review.opendev.org/714459 | 11:11 |
*** ileixe has joined #openstack-nova | 11:18 | |
*** ileixe has quit IRC | 11:19 | |
*** ileixe has joined #openstack-nova | 11:20 | |
openstackgerrit | Lee Yarwood proposed openstack/nova master: workarounds: Add option to disable native LUKSv1 decryption by QEMU https://review.opendev.org/708030 | 11:33 |
openstackgerrit | Lee Yarwood proposed openstack/nova master: workarounds: Connect RBD volumes to the compute host as block devices https://review.opendev.org/708029 | 11:33 |
*** ociuhandu has quit IRC | 11:36 | |
*** ociuhandu has joined #openstack-nova | 11:38 | |
*** ociuhandu has quit IRC | 11:43 | |
*** rpittau is now known as rpittau|bbl | 11:45 | |
lyarwood | stephenfin: https://review.opendev.org/#/c/714956/ - would you mind hitting this? | 11:47 |
stephenfin | sure | 11:47 |
lyarwood | luyao: ack sorry swamped with downstream things, I'll get back to that review today | 11:48 |
luyao | lyarwood: thanks :) | 11:49 |
openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Break up get_disk_mapping within blockinfo https://review.opendev.org/714962 | 12:07 |
hrw | kashyap: zuul gave +1 for https://review.opendev.org/#/c/709494/ - bumping to +2+w? | 12:09 |
*** zhanglong has joined #openstack-nova | 12:10 | |
*** mkrai has quit IRC | 12:12 | |
kashyap | hrw: I am a mere mortal, can't +2 :) | 12:12 |
kashyap | gibi: ^ Mind having a gander at hrw's completed patch? | 12:12 |
*** nweinber has joined #openstack-nova | 12:12 | |
gibi | kashyap, hrw: sure I will try | 12:12 |
kashyap | gibi: At its core, it is setting a sensible default CPU model for AArch64 VMs | 12:13 |
kashyap | gibi: We arrived at the sensible default (CPU model 'max') after consulting the QEMU AArch64 maintainers. | 12:13 |
*** ileixe has quit IRC | 12:14 | |
sean-k-mooney | kashyap: you could argue that min would be a better sensible default for live migration but max is overall a better default | 12:14 |
*** macz_ has joined #openstack-nova | 12:14 | |
gibi | kashyap: ack. thanks for bringing in the experts on that | 12:14 |
sean-k-mooney | so ya it makes sense even if its not the safest default | 12:14 |
kashyap | "min"? There's no such thing as "min" | 12:14 |
sean-k-mooney | isnt there. there was an alternitive to max we were considering | 12:15 |
*** zhanglong has quit IRC | 12:15 | |
*** zhanglong has joined #openstack-nova | 12:15 | |
sean-k-mooney | my understanding of max is it will enable all feature that qemu can emulate on the host | 12:15 |
sean-k-mooney | not just the ones the host has at the hardware level | 12:15 |
kashyap | You mean, 'host'? | 12:16 |
sean-k-mooney | perhaps yes | 12:16 |
sean-k-mooney | i think our default should basically be the same as host model is on x86 | 12:16 |
kashyap | As 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 |
kashyap | To quote myself: | 12:16 |
kashyap | [quote] | 12:16 |
kashyap | For 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 |
kashyap | sean-k-mooney: That's what 'max' is; except that it also does the right thing for TCG | 12:17 |
kashyap | gibi: Thanks! | 12:17 |
kashyap | sean-k-mooney: Give your ACK on the change, too, please | 12:17 |
sean-k-mooney | max will not do the right thing for kvm however right. it will expose more then the host can support in hardware right | 12:17 |
*** lpetrut has joined #openstack-nova | 12:18 | |
sean-k-mooney | kashyap: well im not conviced max is the right thing in the kvm case i would prefer to have a different default for both honestly | 12:18 |
sean-k-mooney | max i think makes sense in the qemu/tcg case | 12:18 |
sean-k-mooney | unless host and max are always identical in the kvm case i woudl prefer it to be host when virt_type=kvm | 12:18 |
*** macz_ has quit IRC | 12:19 | |
kashyap | sean-k-mooney: Did you read the above quote? | 12:19 |
sean-k-mooney | yes | 12:19 |
*** bbowen_ has joined #openstack-nova | 12:19 | |
*** bbowen has quit IRC | 12:19 | |
sean-k-mooney | is host always identical to max with kvm | 12:20 |
kashyap | No; 'max' will give you the moving-target "all the stuff we can currently emulate". | 12:20 |
sean-k-mooney | or if your qemu can emulate feature not present on the host hardware will max result in it doing so even with kvm | 12:20 |
kashyap | Let's not get worried about theoretical concerns. | 12:20 |
sean-k-mooney | kashyap: 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 all | 12:21 |
*** udesale_ has joined #openstack-nova | 12:21 | |
sean-k-mooney | useing max will have an upgrade impact as we will not be able to live migrate safely form a newer host to an older one | 12:22 |
sean-k-mooney | use host with kvm will allow that | 12:22 |
kashyap | sean-k-mooney: Hang on; I just asked Peter from QEMU again. | 12:22 |
kashyap | So, yes: 'max' is same as 'host' on KVM. Always | 12:22 |
sean-k-mooney | ok if that is the case im fine with it :) | 12:23 |
kashyap | Okido | 12:23 |
kashyap | gibi: Please go ahead with the patch. --^ | 12:23 |
sean-k-mooney | ill go +1 it now | 12:23 |
kashyap | Thx! | 12:23 |
gibi | thanks, I will look in a minute | 12:23 |
kashyap | No worries; just wanted to say that the loose end (discussion) is tied :) | 12:24 |
* kashyap --> offline; back later | 12:24 | |
gibi | kashyap: thanks, it helps :) | 12:24 |
kashyap | later == April 01 (yes, really; not a joke ;)) | 12:24 |
*** udesale has quit IRC | 12:24 | |
gibi | kashyap: have a nice time off | 12:25 |
*** ociuhandu has joined #openstack-nova | 12:28 | |
*** ociuhandu has quit IRC | 12:33 | |
*** kashyap has quit IRC | 12:34 | |
*** sapd1 has quit IRC | 12:36 | |
hrw | thanks | 12:40 |
*** lbragstad has joined #openstack-nova | 12:42 | |
*** ratailor has quit IRC | 12:44 | |
*** spatel has joined #openstack-nova | 12:47 | |
*** ociuhandu has joined #openstack-nova | 12:49 | |
*** spatel has quit IRC | 12:52 | |
*** amoralej is now known as amoralej|lunch | 13:01 | |
*** ociuhandu has quit IRC | 13:10 | |
*** rpittau|bbl is now known as rpittau | 13:10 | |
*** mkrai has joined #openstack-nova | 13:11 | |
openstackgerrit | Merged openstack/nova master: libvirt: Use domain capabilities to get supported device models https://review.opendev.org/666915 | 13:22 |
*** PetrTuma has joined #openstack-nova | 13:22 | |
*** tkajinam has joined #openstack-nova | 13:22 | |
*** mriedem has joined #openstack-nova | 13:25 | |
*** ociuhandu has joined #openstack-nova | 13:26 | |
openstackgerrit | Kevin Zhao proposed openstack/nova master: fix scsi disk unit number of the attaching volume when cdrom bus is scsi https://review.opendev.org/712607 | 13:26 |
*** ileixe has joined #openstack-nova | 13:30 | |
*** zhanglong has quit IRC | 13:36 | |
*** mkrai has quit IRC | 13:40 | |
dansmith | gibi: 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 here | 13:41 |
dansmith | gibi: 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 way | 13:41 |
gibi | dansmith: PUT is not use to update the fields of an existing volume connection but to replace the existing volume connection | 13:41 |
dansmith | gibi: you don't mean PUT in general, I assume | 13:42 |
gibi | dansmith: nope. I mean https://docs.openstack.org/api-ref/compute/#update-a-volume-attachment | 13:42 |
gibi | this is an unfortunate use of PUT ^^ | 13:42 |
gibi | as it replaces an object not updating the field of an object | 13:43 |
dansmith | right, 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 |
gibi | but 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 only | 13:45 |
dansmith | right, so admin can update volume_id, but users can only update delete_on_termination | 13:45 |
dansmith | seems pretty straightforward to me | 13:45 |
openstackgerrit | Kevin Zhao proposed openstack/nova master: fix scsi disk unit number of the attaching volume when cdrom bus is scsi https://review.opendev.org/712607 | 13:46 |
gibi | dansmith: 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 state | 13:48 |
johnthetubaguy | FWIW, consistency with other Nova APIs seems like a big win, i.e. use PUT | 13:49 |
dansmith | gibi: 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 well | 13:49 |
dansmith | johnthetubaguy: ++ for sure | 13:49 |
*** efried has joined #openstack-nova | 13:51 | |
*** mkrai has joined #openstack-nova | 13:54 | |
gibi | I'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 method | 13:56 |
dansmith | from the api client's perspective it's the same operation isn't it? | 13:57 |
dansmith | gibi: 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 on | 13:58 |
gibi | dansmith: I also apologize to saying this but I think such wide review should happen in the spec. this is why we have spec review for | 13:59 |
gibi | from me the two operation is different from the client perspective for mutliple reasons | 13:59 |
dansmith | like, 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 design | 13:59 |
dansmith | well, I disagree | 13:59 |
dansmith | (about them being different) | 13:59 |
gibi | i) the client is different for the two operations | 13:59 |
gibi | for the swap volume the client is cinder | 14:00 |
gibi | for the update delete_on_terminate flag the client is the owner of the instance | 14:00 |
dansmith | in one specific set of cases it is, | 14:00 |
gibi | ii) the swap operation effects the running instance while the update operation does not | 14:00 |
dansmith | but a client library used by both wouldn't need to distinguish.. they're both "update the volume attachment" operations | 14:01 |
sean-k-mooney | the real issue is swap volume should have been a server action keeping put free for updates. | 14:01 |
dansmith | sean-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 |
dansmith | we expose a resource, it's changing the resource.. what happens behind the scenes is internal logic | 14:02 |
sean-k-mooney | it is not a voilation or the rest design to do it as we do today | 14:02 |
*** amoralej|lunch is now known as amoralej | 14:02 | |
dansmith | but server actions are pretty much all not RESTful :) | 14:02 |
sean-k-mooney | so yes from that perspecive swap volume via update if fine | 14:03 |
sean-k-mooney | dansmith: yes by definition its acutlly a rest anti pattened to have rpc like api actions | 14:03 |
dansmith | right | 14:03 |
dansmith | so 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-mooney | i was ok with the PUT in the spec by the way. gmann was not so i proposed patch as a compormise | 14:04 |
dansmith | and I don't think that we need should justify using a different method externally because the internal plumbing is currently setup in one specific way | 14:04 |
sean-k-mooney | since the api method was free and it allowed used to expresss policy cleanly | 14:05 |
dansmith | that is the point of the api.. to abstract those things away | 14:05 |
dansmith | right, but where does it end? use HEAD for the next one? :) | 14:05 |
* johnthetubaguy nods (giggle) | 14:06 | |
sean-k-mooney | no but patch is for a paritl update which is what we are doing | 14:06 |
*** xek__ has joined #openstack-nova | 14:06 | |
sean-k-mooney | we are setting one atribute and not the rest | 14:06 |
sean-k-mooney | but as i said it was a compromise not the ideal solution | 14:06 |
dansmith | sure, 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 put | 14:06 |
dansmith | so when the next thing comes along, we could use HEAD and avoid having to abstract that one too :) | 14:07 |
sean-k-mooney | well head would only be apporpriate if the new action match the semantics of head but i get your point and i think you get mine | 14:07 |
sean-k-mooney | dansmith: in your view you would prefer that we keep with PUT correct | 14:08 |
dansmith | yes | 14:08 |
dansmith | and yes, HEAD is a silly example for the sake of being silly | 14:08 |
sean-k-mooney | i have not looked at gmann specific objection in a while but i belive it was related to the semaintic of the payload | 14:09 |
sean-k-mooney | do you have a proposal to adress his concern? | 14:09 |
*** xek_ has quit IRC | 14:09 | |
dansmith | I don't know what his concern is | 14:09 |
sean-k-mooney | i 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 filed | 14:10 |
sean-k-mooney | i belive we can already do a polciy check on the payload so that is not really a concern | 14:10 |
openstackgerrit | Sundar Nadathur proposed openstack/nova master: Block unsupported instance operations with accelerators. https://review.opendev.org/674726 | 14:10 |
openstackgerrit | Sundar Nadathur proposed openstack/nova master: Add cyborg tempest job. https://review.opendev.org/670999 | 14:10 |
dansmith | I don't see that as a problem yeah | 14:10 |
sean-k-mooney | but there was a question about if the volume shoudl be set in teh payload or just in the url | 14:10 |
sean-k-mooney | currently i think the volume id in the url is the current volume and the one in the payload is the one to swap too | 14:11 |
dansmith | well, that makes it even easier to determine what is going on right? | 14:12 |
dansmith | if they differ, then check admin policy, if they're the same, user policy and only look for user-changeable options in the body | 14:12 |
sean-k-mooney | i think we would have to require that its not set in the payload | 14:12 |
sean-k-mooney | well ya we could require they are the same | 14:12 |
dansmith | further, PATCH has to be atomic and provides no way to guarantee that the representation hasn't changed since I fetched it on the client side | 14:13 |
dansmith | so if someone has updated the definition between me fetching it and sending a single field to change, I might not notice | 14:13 |
dansmith | i.e. there's no generation or other indicator | 14:13 |
sean-k-mooney | honestly i could be miss remebering what gmann objection was | 14:13 |
sean-k-mooney | dansmith: yes that is also true | 14:14 |
dansmith | probably not likely on a volume attachment currently, but certainly could happen in the future | 14:14 |
dansmith | i.e. some sort of swap-snapshot-swap thing or something | 14:14 |
dansmith | (this is a major objection of mine to patch in general for most things) | 14:14 |
sean-k-mooney | i think the most likely issue would be with doing a volume detach via cinder in parralle | 14:14 |
sean-k-mooney | that said we have the same issue with PUT no? unless we have a genertion field in the payload | 14:15 |
dansmith | no, with put you have all the original fields there, you can check to see that nothing has changed | 14:15 |
sean-k-mooney | since put is ment to do a full update replacing all fields at least semantically | 14:15 |
dansmith | I'm not saying we *have* to for this specific case, but it's at least *possible* | 14:16 |
dansmith | also, looking at the api-ref here, | 14:16 |
sean-k-mooney | yes that is true | 14:16 |
dansmith | there'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 PUT | 14:16 |
sean-k-mooney | anyway 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 too | 14:17 |
dansmith | because people would say "What? why can't I change this flag without special virt support?" | 14:17 |
nightmare_unreal | how can I run nova functional tests ? | 14:17 |
sean-k-mooney | nightmare_unreal: tox -e functional-py36 | 14:17 |
nightmare_unreal | and for specific file ? -- -n file-name ? | 14:18 |
sean-k-mooney | you can do -- folowed by any arges supported by stestr | 14:18 |
sean-k-mooney | normally i just pass a regex for the class name or test name | 14:18 |
nightmare_unreal | cool :) Thanks | 14:19 |
sean-k-mooney | e.g. tox -e functional-py36 -- "compute|libvirt" | 14:19 |
nightmare_unreal | in quotes can i specify exact class name ? | 14:19 |
nightmare_unreal | like in your example | 14:19 |
sean-k-mooney | yes | 14:20 |
nightmare_unreal | awesome, thanks | 14:20 |
sean-k-mooney | the quote are jsut so the shell does not interperate the | as a pipe | 14:20 |
sean-k-mooney | so in this case is an or in the regex so that will run all gets with compute or libvirt in the fully qulified function name | 14:20 |
*** ileixe has quit IRC | 14:21 | |
nightmare_unreal | understood | 14:21 |
gibi | dansmith, 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 |
gibi | I will raise the discussion again on the team meeting as I hope gmann will be there | 14:22 |
dansmith | gibi: I would hate to change our API so significantly on a process technicality | 14:22 |
*** alex_xu has quit IRC | 14:22 | |
dansmith | I'd much rather help brinzhang_ get it right, and/or approve a little extra time | 14:22 |
dansmith | the API stability is serious business to me and much more important to get it right than not | 14:23 |
gibi | dansmith: 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 change | 14:24 |
dansmith | meaning, it's not easily changeable later to allow something to land before a deadline | 14:24 |
dansmith | we have enough weird "huh, why is this different than *everything* else" warts on the API.. let's not make it worse | 14:24 |
openstackgerrit | Kevin Zhao proposed openstack/nova master: fix scsi disk unit number of the attaching volume when cdrom bus is scsi https://review.opendev.org/712607 | 14:25 |
PetrTuma | sean-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 two | 14:25 |
PetrTuma | images (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 any | 14:25 |
PetrTuma | idea? | 14:25 |
sean-k-mooney | you updated the nova api container right | 14:26 |
PetrTuma | yes | 14:26 |
sean-k-mooney | just checking you didnt update the compute one :) | 14:26 |
sean-k-mooney | PetrTuma: what release is it? | 14:27 |
PetrTuma | Rocky | 14:27 |
dansmith | gibi: 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-mooney | i tihnk rocky has the chagne that force the image to be checked if it changes | 14:28 |
sean-k-mooney | in older releases we did not do that | 14:28 |
*** sapd1 has joined #openstack-nova | 14:28 | |
sean-k-mooney | dansmith: you could propose a minor update to the spec with what you want to see? | 14:29 |
dansmith | I could | 14:29 |
*** yankcrime has quit IRC | 14:29 | |
openstackgerrit | Merged openstack/nova master: Non-Admin user can filter their instances by more filters https://review.opendev.org/701609 | 14:29 |
openstackgerrit | Merged openstack/nova master: Add default cpu model for AArch64 https://review.opendev.org/709494 | 14:29 |
*** corvus has quit IRC | 14:30 | |
*** mmethot has quit IRC | 14:30 | |
PetrTuma | sean-k-mooney do you mean other change than the one I use? | 14:30 |
*** mkrai has quit IRC | 14:31 | |
*** mkrai_ has joined #openstack-nova | 14:31 | |
*** corvus has joined #openstack-nova | 14:31 | |
hrw | yes! | 14:31 |
gibi | dansmith: I would be glad for a summary back on the review | 14:31 |
sean-k-mooney | PetrTuma: 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 valid | 14:32 |
dansmith | gibi: okay | 14:32 |
gibi | dansmith: thank you | 14:32 |
sean-k-mooney | PetrTuma: 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 that | 14:32 |
gibi | dansmith: I will try to reach gmann to comment back | 14:32 |
gibi | dansmith: either on the meeting or separately | 14:33 |
PetrTuma | sean-k-mooney Ok, I'll try to find the change and confirm this, thanks for the moment | 14:34 |
*** hrw has quit IRC | 14:35 | |
gmann | reading logs... | 14:36 |
*** TxGirlGeek has joined #openstack-nova | 14:36 | |
sean-k-mooney | PetrTuma: the check for the need to schdule happens here https://zuul.opendev.org/t/openstack/build/cfebf394494246beabb5f417ec1d5c64/logs which is after the check i added | 14:37 |
sean-k-mooney | PetrTuma: which is here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3484-L3485 | 14:37 |
sean-k-mooney | PetrTuma: so that is not the issue | 14:37 |
*** yankcrime has joined #openstack-nova | 14:38 | |
sean-k-mooney | PetrTuma: so the only condition that should prevent the _validate_numa_rebuild function running is if orig_image_ref != image_href: | 14:39 |
dansmith | gibi: done | 14:41 |
openstackgerrit | jayaditya gupta proposed openstack/nova master: Support for nova-manage placement heal_allocations --cell https://review.opendev.org/714459 | 14:44 |
gibi | dansmith: thanks again. and sorry that I'm more tense than usual | 14:45 |
dansmith | gibi: np, I understand the pressure | 14:45 |
gibi | I guess some of it is new to me :) | 14:46 |
dansmith | I believe you signed up for it :) | 14:46 |
gibi | I did | 14:47 |
gibi | and I'm learning every day | 14:47 |
gmann | dansmith: 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 |
gmann | my main concern on not using PUT was- not to make this API over scoped and keep it for swap only. | 14:47 |
gmann | i am ok with any method, PUT also ok for consistency to our APIs but new one not in existing PUT which is swap | 14:48 |
gmann | can we do it via server PUT ? | 14:49 |
*** brinzhang_ has quit IRC | 14:49 | |
*** brinzhang_ has joined #openstack-nova | 14:49 | |
sean-k-mooney | gmann: but thats the thing PUT should not be swap. PUT + changeing the volume id sure | 14:49 |
sean-k-mooney | but PUT alone should not have been swap | 14:49 |
dansmith | I 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 need | 14:50 |
gmann | yeah that is not best design but we cannot change that now but can avoid making it more comlex | 14:50 |
gmann | dansmith: sure. | 14:51 |
dansmith | IMHO, anything that requires the client to do something different than the obvious thing of PUTting the resource with delete-on-termination changed is more complex | 14:51 |
sean-k-mooney | gmann: acutlly we could change it in a microversion but not in ussuri at this point. i do tend to agree with ^ on that point | 14:52 |
sean-k-mooney | * with dansmith | 14:53 |
*** dklyle has joined #openstack-nova | 14:53 | |
gmann | you 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-mooney | gmann: we dont need to make it an action although that is one option. we just need to stop thing of PUT as swap | 14:54 |
johnthetubaguy | isn't the simplest just that PUT does different things if the body includes volume_id or delete_on_termination | 14:54 |
gmann | dansmith: +1 on that. but does not client will be confused with PUT = swap + actual updating.. | 14:54 |
sean-k-mooney | PUT is just an update and if put change the volume id its sideffect is a swap underneat | 14:55 |
gmann | johnthetubaguy: in implementation, there is no challenge it is easy. i am thinking on usage point of view if we are making this API over-scoped | 14:55 |
sean-k-mooney | johnthetubaguy: well it does one thing update the attachment recored, and that update can have other sideffect such as swap | 14:55 |
sean-k-mooney | if we consider the attachment ot be declaritive rahter then inperitive this is totally natural | 14:56 |
gmann | i think swap storage is very explicit things to use and adding it as side-effect behind some operation is not good API | 14:56 |
johnthetubaguy | for me its a consistency thing, PUT for partial update is what we do on other resources | 14:57 |
sean-k-mooney | PUT sematicaly is not ment ot be a partil update but it is how we use it yes | 14:57 |
gmann | yeah, i completely agree on that. but we have used this PUT for swap that is the only things i am worried about. | 14:57 |
johnthetubaguy | gmann: 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 other | 14:58 |
*** ociuhandu has quit IRC | 14:58 | |
johnthetubaguy | if 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 users | 14:59 |
johnthetubaguy | but hey, best to discuss later I suspect | 14:59 |
gmann | can 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-nova | 15:00 | |
*** Sundar has joined #openstack-nova | 15:00 | |
johnthetubaguy | gmann: 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 confusion | 15:00 |
sean-k-mooney | gmann: we could but what benifit does that serve | 15:00 |
johnthetubaguy | seems like busy work to me | 15:01 |
johnthetubaguy | we don't have bandwidth for the pants of fire stuff right now | 15:01 |
*** macz_ has joined #openstack-nova | 15:01 | |
gmann | ok | 15:02 |
*** macz_ has quit IRC | 15:02 | |
openstackgerrit | Merged openstack/nova master: Add transform_image_metadata request filter https://review.opendev.org/665775 | 15:02 |
*** macz_ has joined #openstack-nova | 15:02 | |
Sundar | dansmith, 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 |
gibi | Sundar: ack | 15: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 |
gmann | gibi: 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 |
gmann | gibi: 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 IRC | 15:13 | |
gmann | if 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_unreal | mriedem: how will I know what resource value to overwrite ? I am refering to TO-DO overwrite allocations | 15:18 |
*** dswebb has joined #openstack-nova | 15:18 | |
mriedem | nightmare_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 flavor | 15:21 |
*** gyee has joined #openstack-nova | 15:21 | |
Sundar | sean-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 |
mriedem | so 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 values | 15:22 |
nightmare_unreal | i have not yet reached to write test :/ , was fixing my previous patch. Yet to write code for the overwrite. | 15:23 |
*** tkajinam has quit IRC | 15:30 | |
mriedem | ok 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 day | 15:31 |
mriedem | where it determines if it should call put_allocations or not | 15:31 |
nightmare_unreal | yes I think so too :) | 15:31 |
nightmare_unreal | afk | 15:31 |
nightmare_unreal | Thanks | 15:32 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: tox: Integrate mypy https://review.opendev.org/676208 | 15:32 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Update and correct typing information https://review.opendev.org/714694 | 15:32 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Add typing information https://review.opendev.org/714695 | 15:32 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Split instance NUMA object tests https://review.opendev.org/714696 | 15:32 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Replace 'cpu_pinning_requested' helper https://review.opendev.org/714697 | 15:32 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Don't consider overhead CPUs for unpinned instances https://review.opendev.org/714698 | 15:32 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Remove handling of pre-Train compute nodes https://review.opendev.org/714699 | 15:32 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Add validation for 'cpu_realtime_mask' https://review.opendev.org/468203 | 15:32 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Tweak the 'cpu_realtime_mask' handling slightly https://review.opendev.org/461456 | 15:32 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Rework 'get_realtime_constraint' https://review.opendev.org/714700 | 15:32 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Invert order of NUMA topology generation https://review.opendev.org/714701 | 15:32 |
gibi | gmann: do I understand correctly that you also got convinced that the PUT solution is OK | 15:33 |
gibi | ? | 15:33 |
*** sapd1 has quit IRC | 15:36 | |
gmann | gibi: 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 |
gibi | gmann: ack, thanks | 15:37 |
*** ociuhandu has quit IRC | 15:37 | |
gmann | though 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-nova | 15:38 | |
* gmann hope we can deprecate the swap operation some time :) | 15:38 | |
*** ociuhandu has quit IRC | 15:39 | |
sean-k-mooney | gmann: i think calling it a swap + updating resources is still the wrong way to think about it | 15:40 |
gmann | gibi: 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-mooney | gmann: it is just declaritivly updatin the resouces and a sidefect of that can be to trigger swap | 15:40 |
*** ociuhandu has joined #openstack-nova | 15:40 | |
sean-k-mooney | that is how we should explain it to endusers | 15:40 |
gibi | gmann: re instance fault: thanks | 15:40 |
johnthetubaguy | gmann: my preference is to not document the crazy cinder API, and move that to some developer docs or something? | 15:41 |
johnthetubaguy | should stop some confusion for the 99% of API users | 15:41 |
sean-k-mooney | johnthetubaguy: do end users ever call that directly | 15:41 |
gmann | sean-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 IRC | 15:41 | |
johnthetubaguy | sean-k-mooney: they do, but they never should | 15:41 |
johnthetubaguy | its for cinder only, else odd things happen | 15:41 |
sean-k-mooney | gmann: not really that is how i alwasy tought of it already | 15:42 |
sean-k-mooney | johnthetubaguy: ok so this is just for the callback form cinder | 15:42 |
johnthetubaguy | that is my understanding, yes | 15:42 |
johnthetubaguy | hence the big red warning | 15:42 |
gmann | johnthetubaguy: 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-mooney | johnthetubaguy: and yes im pretty sure we have had downstream bugs filed as a result of users calling it directly | 15:43 |
johnthetubaguy | gmann: +1 that | 15:43 |
gmann | sean-k-mooney: yeah we had few in past | 15:43 |
johnthetubaguy | sean-k-mooney: yeah, what they wanted to do was call the Cinder swap API, but they got confused | 15:43 |
gibi | for me the difference is like change my house (swap) or repaint the kitchen in my house (update) | 15:43 |
sean-k-mooney | actully i think our customer wanted to bypass a check in the ciner side a and force it so that is why they used nova | 15:44 |
johnthetubaguy | gibi: 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-nova | 15:45 | |
openstackgerrit | Stephen Finucane proposed openstack/nova master: api: Add framework for extra spec validation https://review.opendev.org/704643 | 15:45 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: api: Add microversion 2.84, extra spec validation https://review.opendev.org/708436 | 15:45 |
sean-k-mooney | anyway 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 again | 15:45 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Add documentation for flavor extra specs https://review.opendev.org/710037 | 15:45 |
stephenfin | gibi: 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 |
johnthetubaguy | sean-k-mooney: yeah, sounds like they were doing something bad. something we shouldn't tell them to do in the docs | 15:46 |
stephenfin | and johnthetubaguy, any chance you'll stick that on your list? iirc, that was also on your list of gripes ^ | 15:46 |
gibi | johnthetubaguy: yeah like that. replace the whole v.s. tweak a prt of the whole | 15:46 |
gibi | stephenfin: sure | 15:46 |
johnthetubaguy | stephenfin: well played, and yes on both counts | 15:46 |
gibi | stephenfin: I think I caused you to lose :) | 15:47 |
stephenfin | well, thankfully it's just lyarwood and I duking it out for 2.84 now, afaict :) | 15:47 |
* stephenfin throws shiny thing lyarwood's direction | 15:48 | |
gibi | :) | 15:48 |
*** sapd1 has joined #openstack-nova | 15:49 | |
*** ociuhandu has joined #openstack-nova | 15:49 | |
*** udesale_ has quit IRC | 15:50 | |
sean-k-mooney | johnthetubaguy: have you had any futher issues with the libosinfo feature where we try to set some default based on the disto name/version | 15:50 |
*** PetrTuma has quit IRC | 15:50 | |
johnthetubaguy | sean-k-mooney: mostly given up on it I think, not had time to dig | 15:51 |
sean-k-mooney | johnthetubaguy: we backported the fix for the last one downstream and fond that new libosinfo broke use in another way | 15:51 |
sean-k-mooney | johnthetubaguy: ok so i would like to remove it in victoria since it was broken from the start given it ignores architeture and machine type | 15:51 |
sean-k-mooney | would that cause issues for you if we did that | 15:52 |
johnthetubaguy | sean-k-mooney: it would make my life easier I think | 15:52 |
*** ociuhandu has quit IRC | 15:52 | |
sean-k-mooney | we likely would fix the issue we hit so we can backport then kill it with fire in the next patch | 15:52 |
johnthetubaguy | sounds like a good idea, and just replace it with some decent documentation | 15:52 |
sean-k-mooney | ya we have explict image properties to set everyting it can set anyway | 15:53 |
gibi | stephenfin: done. | 15:53 |
stephenfin | thanks | 15:53 |
sean-k-mooney | and i alreay recommend people to use those anyway since libosinfo change its default out of our contol anyway | 15:54 |
gibi | nova meeting starts in 5 and a half minutes on #openstack-meeting-3 | 15:54 |
*** mkrai_ has quit IRC | 15:57 | |
*** mlavalle has joined #openstack-nova | 16:00 | |
*** ociuhandu has joined #openstack-nova | 16:00 | |
*** TxGirlGeek has quit IRC | 16:02 | |
*** TxGirlGeek has joined #openstack-nova | 16:03 | |
*** spatel has joined #openstack-nova | 16:10 | |
lyarwood | stephenfin: oOOoOoo shiny.. WAIT A MINUTE! | 16:10 |
lyarwood | stephenfin: feel free to land on 2.84 btw, I need to rework something in my series today anyway | 16:11 |
lyarwood | stephenfin: if it's just the two of us I'll land on 2.85 | 16:11 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: Use neutronclient's port binding APIs https://review.opendev.org/706295 | 16:12 |
*** ociuhandu has quit IRC | 16:16 | |
artom | stephenfin, for the record, I hate https://review.opendev.org/#/c/631053/ and everything it stands for | 16:19 |
artom | I'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 |
openstackgerrit | Huaqiang Wang proposed openstack/nova master: Take the instance dedicated CPU list from 'cpu_pinning' if possible https://review.opendev.org/713352 | 16:25 |
openstackgerrit | Huaqiang Wang proposed openstack/nova master: libvirt: support to create instance with dedicated and shared CPUs https://review.opendev.org/714655 | 16:25 |
openstackgerrit | Huaqiang Wang proposed openstack/nova master: 'cpu_pinning_requested' property could be derived directly from cpu_policy https://review.opendev.org/713353 | 16:25 |
openstackgerrit | Huaqiang Wang proposed openstack/nova master: Remove 'InstanceNUMACell.cpu_pinning_requested' field https://review.opendev.org/714656 | 16:25 |
openstackgerrit | Huaqiang Wang proposed openstack/nova master: Refactor the code in checking available host CPUs https://review.opendev.org/714657 | 16:25 |
openstackgerrit | Huaqiang Wang proposed openstack/nova master: Introduce 'MIXED' CPU allocation policy for instance https://review.opendev.org/713354 | 16:25 |
openstackgerrit | Huaqiang Wang proposed openstack/nova master: Introduce the interface of creating 'MIXED' policy instance through 'PCPU' and 'VCPU' https://review.opendev.org/713355 | 16:25 |
openstackgerrit | Huaqiang Wang proposed openstack/nova master: metadata: export the vCPU IDs that are pinning on the host CPUs https://review.opendev.org/688936 | 16:25 |
openstackgerrit | Huaqiang Wang proposed openstack/nova master: [WIP] An alternative way for keeping instance dedicated CPUs https://review.opendev.org/714658 | 16:25 |
*** ociuhandu has joined #openstack-nova | 16:26 | |
gmann | johnthetubaguy: 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@99 | 16:26 |
gmann | hope that is fine, in some cases it is just removing the deprecatiob from base rule and checks | 16:26 |
*** dpawlik has quit IRC | 16:26 | |
johnthetubaguy | gmann: yeah, seemed good to check anyways | 16:26 |
gmann | ok | 16:26 |
johnthetubaguy | its a bit like nova-next tests | 16:27 |
gmann | yeah | 16:27 |
huaqiang | stephenfin: I updated the series for 'mixed policy instance', but only the last patch I changed this time | 16:27 |
huaqiang | https://review.opendev.org/714658 [WIP] An alternative way for keeping instance dedicated CPUs | 16:27 |
huaqiang | this 'pcpuset approach' one, | 16:28 |
huaqiang | If you have time please have a look for this patch, | 16:28 |
huaqiang | I just want your comments for if it is ready to drop the 'cpu_pinning' approach now | 16:29 |
huaqiang | it the answer is comfirmative, I'll drop it and submit new series. | 16:30 |
huaqiang | this patch is still a 'WIP' patch, I still have several 'TODO's, I'll address tomorrow. | 16:32 |
openstackgerrit | John Garbutt proposed openstack/nova master: Enforce resource limits using oslo.limit https://review.opendev.org/615180 | 16:32 |
openstackgerrit | John Garbutt proposed openstack/nova master: Add legacy limits and usage to unified limits https://review.opendev.org/713498 | 16:32 |
openstackgerrit | John Garbutt proposed openstack/nova master: Update quota apis with keystone limits and usage https://review.opendev.org/713499 | 16:32 |
*** mgariepy has quit IRC | 16:36 | |
*** ociuhandu has quit IRC | 16:36 | |
*** mgariepy has joined #openstack-nova | 16:43 | |
openstackgerrit | John Garbutt proposed openstack/nova master: Tell oslo.limit how to count nova resources https://review.opendev.org/713301 | 16:48 |
*** ociuhandu has joined #openstack-nova | 16:49 | |
openstackgerrit | John Garbutt proposed openstack/nova master: Enforce resource limits using oslo.limit https://review.opendev.org/615180 | 16:49 |
openstackgerrit | John Garbutt proposed openstack/nova master: Add legacy limits and usage to unified limits https://review.opendev.org/713498 | 16:49 |
openstackgerrit | John Garbutt proposed openstack/nova master: Update quota apis with keystone limits and usage https://review.opendev.org/713499 | 16:49 |
gmann | dansmith: 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@494 | 16:53 |
johnthetubaguy | its a new policy I think, admin_or_owner one | 16:53 |
johnthetubaguy | probably need to rename the old one I guess | 16:53 |
*** ociuhandu has quit IRC | 16:53 | |
gmann | ok. | 16:54 |
dansmith | yeah, new policy | 16:54 |
johnthetubaguy | gmann: update -> swap, new one called "put" ? | 16:55 |
gmann | i was thinking new one to 'update' but that is existing and conflcit at deprecation logic | 16:56 |
johnthetubaguy | yeah, agreed | 16:56 |
gmann | 'put' will be inconsistentname from other one | 16:56 |
gmann | 'os-volumes-attachments:volume:update' for new ? | 16:57 |
gmann | volume is redundant though | 16:57 |
johnthetubaguy | that seems more inconsistent though | 16:57 |
gmann | :) | 16:57 |
dansmith | I would think that it would be okay to rename the existing one, given the conflict | 16:58 |
dansmith | with a reno | 16:58 |
dansmith | the existing one, if opened to non-admin users wouldn't be terrible if it applied to the delete flag, and probably expected | 16:59 |
johnthetubaguy | yeah, I am tempted to just reno about the conflict | 16:59 |
gmann | ok, deprecation message saying this old policy is being used for this new operation ? | 16:59 |
dansmith | in all likelihood nobody has changed this one since it _is_ supposed to be for cinder only | 16:59 |
dansmith | gmann: maybe not deprecation, but a warning if it's set to non-default? | 16:59 |
*** TxGirlGeek has quit IRC | 16:59 | |
johnthetubaguy | nah, skip deprecation, make existing "update" PROJECT_MEMBER_OR_SYSTEM_ADMIN, then add new rule for swap | 16:59 |
*** TxGirlGeek has joined #openstack-nova | 17:00 | |
dansmith | yeah, that | 17:00 |
gmann | ok, that is better. | 17:00 |
johnthetubaguy | they shouldn't have changed it, if they did, they should read the reno note | 17:00 |
dansmith | exactly | 17:01 |
dansmith | if this was for any other thing I would take more care, but this is suuuper obscure and special | 17:01 |
gmann | and cinder will keep working as new defaults keep allowing admin to access | 17:02 |
dansmith | yeah | 17:02 |
gmann | ok adding comment on the review for brinzhang_ | 17:03 |
sean-k-mooney | gibi: 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 m2 | 17:03 |
sean-k-mooney | gibi: 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 IRC | 17:08 | |
*** rpittau is now known as rpittau|afk | 17:12 | |
openstackgerrit | John Garbutt proposed openstack/nova master: Add reno for unified limits https://review.opendev.org/715271 | 17:14 |
*** lpetrut has quit IRC | 17:21 | |
*** tesseract has quit IRC | 17:30 | |
*** ccamacho has joined #openstack-nova | 17:31 | |
*** TxGirlGe_ has joined #openstack-nova | 17:31 | |
*** dtantsur is now known as dtantsur|afk | 17:32 | |
*** ccamacho has quit IRC | 17:32 | |
*** TxGirlGeek has quit IRC | 17:41 | |
*** evrardjp has quit IRC | 17:49 | |
*** evrardjp has joined #openstack-nova | 17:49 | |
gmann | dansmith: 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@200 | 17:49 |
*** openstack has quit IRC | 17:49 | |
*** openstack has joined #openstack-nova | 17:51 | |
*** ChanServ sets mode: +o openstack | 17:51 | |
*** ociuhandu has quit IRC | 17:51 | |
dansmith | mnaser: right, which is why we hide the message now | 17:52 |
dansmith | mnaser: 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 not | 17:52 |
mnaser | dansmith: i agree with that, especially if its a libvirt-specific error | 17:53 |
mnaser | makes life easier | 17:53 |
sean-k-mooney | mnaser: 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 entirely | 17:57 |
sean-k-mooney | the only infomation leak that actully causes was the name of the ceph keyfile, still not ideal. | 17:59 |
sean-k-mooney | but that is fixed in that case at least | 17:59 |
*** derekh has quit IRC | 18:00 | |
gmann | mnaser: does any user ask about what driver you use for your cloud and my VM will be running on? before they buy :) | 18:02 |
gmann | or hypervisor | 18:02 |
openstackgerrit | Sasha Andonov proposed openstack/nova master: rbd_utils: increase _destroy_volume timeout https://review.opendev.org/705764 | 18:02 |
mnaser | gmann: i don't think we've actually ever had a customer ask what hypervisor/backend storage/etc | 18:03 |
sean-k-mooney | gmann: in principaly they should not care | 18:03 |
gmann | yeah, they should not. I was curious if they make the decision based on that. | 18:04 |
sean-k-mooney | they might make a dession based on specific features | 18:04 |
sean-k-mooney | which 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 fine | 18:05 |
sean-k-mooney | i 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 product | 18:06 |
gmann | in 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 |
gmann | though I am not sure it was user request or just customer thought. | 18:09 |
sean-k-mooney | well that is not realy that hard to do you just use multple virt dirvers in one cloud | 18:09 |
sean-k-mooney | there are then several filter you can use | 18:09 |
sean-k-mooney | the over used example is running windows instnace on hyperv but you can use custom extra specs in flaovr or a trait today to support that | 18:11 |
*** links has quit IRC | 18:23 | |
*** amoralej is now known as amoralej|off | 18:30 | |
*** nightmare_unreal has quit IRC | 18:41 | |
*** ralonsoh has quit IRC | 18:51 | |
mnaser | sean-k-mooney: aws has moved away from xen btw :p | 18:58 |
*** tbachman has quit IRC | 19:01 | |
*** maciejjozefczyk has quit IRC | 19:02 | |
sean-k-mooney | mnaser: 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 instnace | 19:03 |
mnaser | sean-k-mooney: yep agreed | 19:04 |
*** tbachman has joined #openstack-nova | 19:13 | |
*** ociuhandu has joined #openstack-nova | 19:44 | |
*** martinkennelly has quit IRC | 19:48 | |
*** ociuhandu has quit IRC | 19:55 | |
sean-k-mooney | dansmith: 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 driver | 20:06 |
dansmith | cool | 20:06 |
sean-k-mooney | the allocation and arq were correctly created against the second host | 20:06 |
sean-k-mooney | is there anything else you want me to check specificlly | 20:06 |
dansmith | on reschedule? | 20:08 |
dansmith | I forget, did you try cold migration? | 20:08 |
sean-k-mooney | no but i can. i thikn we expect that to fail like evaucate and resize fails. | 20:09 |
*** larainema has quit IRC | 20:09 | |
dansmith | ah okay.. I can see migration working and resize not, but only in a few scenarios, so if you did that already that's fine | 20:09 |
sean-k-mooney | dansmith: 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-followup | 20:09 |
dansmith | evac is a little different since it doesn't get help from the original node | 20:10 |
dansmith | sean-k-mooney: ah sweet, hadn't seen this | 20:10 |
sean-k-mooney | ya so evac works but it does not claim an fpga on the dest or update the arq | 20:10 |
sean-k-mooney | dansmith: sundar created it todeay | 20:10 |
sean-k-mooney | *today | 20:10 |
dansmith | doesn't try to update the arq, or fails because the old one isn't deleted? | 20:11 |
sean-k-mooney | good question i will check but since the placemnt allocation does not have a device i suspect it doesnt even try | 20:11 |
sean-k-mooney | dansmith: the rebuild path does not have to do any arq updates | 20:11 |
sean-k-mooney | so i suspect it does not try | 20:11 |
dansmith | but we scheduled to pick a new host, | 20:12 |
dansmith | but yeah, fair point | 20:12 |
sean-k-mooney | ill go check and let you know | 20:12 |
dansmith | I'm sure there are plenty of things missing, but yeah it'd be good to know where we're starting from | 20:14 |
sean-k-mooney | dansmith: ah so cold migration is blocked in the code :) | 20:17 |
sean-k-mooney | Forbidden with instances that have accelerators. (HTTP 403) (Request-ID: req-c318fb6b-730c-4bd3-9ddf-cb7de6e472a3) | 20:17 |
dansmith | ah okay, I haven't really gotten to that patch at all | 20:17 |
dansmith | so... good :) | 20:17 |
sean-k-mooney | so that works at least although the respocne code still looks wrong to me | 20:17 |
sean-k-mooney | it should proably be a 400 or 409 | 20:17 |
dansmith | I thought we agreed on 403? | 20:22 |
sean-k-mooney | we proably did | 20:22 |
sean-k-mooney | i just find 403 forbiden which is normaly used for auth issues confusing | 20:23 |
sean-k-mooney | i would expect to get 403 if i did not have enough permission to do something not because its unsupported | 20:23 |
dansmith | IIRC, 401 means "authorization required" and 403 means "not permitted" - the latter which may or may not be because of insufficient or missing credentials | 20:26 |
sean-k-mooney | https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/403 | 20: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-mooney | ya it gives us leway to use it this way | 20:27 |
dansmith | "such as (but not limited to) insufficient rights" | 20:27 |
sean-k-mooney | im just used to thinking about it as related to authorisation | 20:28 |
melwitt | zzzeek: 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 |
dansmith | I think a lot of things don't use 401 and 403 properly, so it's common to not have a gut feeling about the difference | 20:28 |
*** ociuhandu has joined #openstack-nova | 20:31 | |
sean-k-mooney | so 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_DISABLED | 20:33 |
sean-k-mooney | the placement request for the evacuate gets teh trait added by the prefilter | 20:33 |
sean-k-mooney | but not the resouce requests | 20:33 |
dansmith | yeah, that's weird... that means the prefilter is seeing the device profile in the flavor | 20:37 |
sean-k-mooney | yep | 20:37 |
*** ociuhandu has quit IRC | 20:37 | |
sean-k-mooney | im not seeing any attepmet to update teh ARQ | 20:37 |
sean-k-mooney | did we move to putting the resouce groups form the device profile into the request spec | 20:38 |
melwitt | gmann: 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/713295 | 20:38 |
*** priteau has quit IRC | 20:41 | |
sean-k-mooney | interesting the "requested_resources" is null http://paste.openstack.org/show/791213/ | 20:44 |
sean-k-mooney | we do populate the accel_info in the resouces https://review.opendev.org/#/c/631244/69/nova/compute/manager.py@2604 | 20:48 |
sean-k-mooney | but i guess we never save that sice we call save on line 2570 | 20:50 |
sean-k-mooney | actully saving the instance in not important | 20:53 |
*** huaqiang has quit IRC | 20:54 | |
*** lbragstad has quit IRC | 20:58 | |
*** amodi has quit IRC | 20:59 | |
*** zhanglong has joined #openstack-nova | 21:06 | |
*** huaqiang has joined #openstack-nova | 21:19 | |
*** ociuhandu has joined #openstack-nova | 21:22 | |
*** ociuhandu has quit IRC | 21:26 | |
*** huaqiang has quit IRC | 21:35 | |
*** zhanglong has quit IRC | 21:36 | |
*** ociuhandu has joined #openstack-nova | 21:44 | |
openstackgerrit | Merged openstack/nova master: nova-net: Remove unused parameters https://review.opendev.org/703974 | 21:46 |
sean-k-mooney | dansmith: found the issue i think. https://github.com/openstack/nova/blob/9d212738bec63d7490998a2840598d790a3c94fc/nova/conductor/manager.py#L1121-L1128 | 21:46 |
*** mriedem has left #openstack-nova | 21:56 | |
*** spatel has quit IRC | 21:58 | |
*** xek__ has quit IRC | 22:03 | |
*** ociuhandu has quit IRC | 22:24 | |
*** ociuhandu has joined #openstack-nova | 22:24 | |
*** rcernin has joined #openstack-nova | 22:25 | |
*** ociuhandu has quit IRC | 22:28 | |
*** zhanglong has joined #openstack-nova | 22:41 | |
*** nweinber has quit IRC | 22:50 | |
*** tkajinam has joined #openstack-nova | 22:54 | |
*** slaweq has quit IRC | 22:59 | |
*** TxGirlGe_ has quit IRC | 23:03 | |
*** prometheanfire has quit IRC | 23:04 | |
*** TxGirlGeek has joined #openstack-nova | 23:04 | |
openstackgerrit | sean mooney proposed openstack/nova master: [WIP] cyborg evacuate support https://review.opendev.org/715326 | 23:05 |
*** macz_ has quit IRC | 23:06 | |
*** rcernin has quit IRC | 23:06 | |
*** rcernin has joined #openstack-nova | 23:07 | |
*** rcernin has quit IRC | 23:07 | |
*** rcernin has joined #openstack-nova | 23:08 | |
*** slaweq has joined #openstack-nova | 23:11 | |
*** slaweq has quit IRC | 23:15 | |
*** TxGirlGeek has quit IRC | 23:30 | |
*** TxGirlGeek has joined #openstack-nova | 23:31 | |
*** tetsuro has joined #openstack-nova | 23:33 | |
*** lbragstad has joined #openstack-nova | 23:33 | |
*** tetsuro has quit IRC | 23:35 | |
*** tetsuro has joined #openstack-nova | 23:37 | |
*** TxGirlGeek has quit IRC | 23:37 | |
*** TxGirlGeek has joined #openstack-nova | 23:38 | |
*** vishalmanchanda has quit IRC | 23:39 | |
*** tetsuro has quit IRC | 23:47 | |
gmann | melwitt: +1. lgtm | 23:48 |
melwitt | thanks | 23:48 |
*** zhanglong has quit IRC | 23:48 | |
*** alex_xu has joined #openstack-nova | 23:53 | |
*** tosky has quit IRC | 23:53 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!