Thursday, 2023-08-31

opendevreviewmelanie witt proposed openstack/nova master: Enable use of native threads in scatter_gather_cells  https://review.opendev.org/c/openstack/nova/+/65017202:00
opendevreviewMerged openstack/nova master: libvirt: Add 'COMPUTE_ADDRESS_SPACE_*' traits support  https://review.opendev.org/c/openstack/nova/+/87322102:47
opendevreviewOpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata  https://review.opendev.org/c/openstack/nova/+/89164804:58
opendevreviewAmit Uniyal proposed openstack/nova master: Reproducer for dangling bdms  https://review.opendev.org/c/openstack/nova/+/88994706:11
opendevreviewAmit Uniyal proposed openstack/nova master: Delete dangling bdms  https://review.opendev.org/c/openstack/nova/+/88228406:11
auniyalthanks sean-k-mooney for the reviews06:50
*** ralonsoh_ooo is now known as ralonsoh07:18
opendevreviewMerged openstack/nova master: Remove deprecated AZ filter.  https://review.opendev.org/c/openstack/nova/+/88677907:21
opendevreviewAmit Uniyal proposed openstack/nova master: Delete dangling bdms  https://review.opendev.org/c/openstack/nova/+/88228408:08
opendevreviewJohn Garbutt proposed openstack/nova master: Deprecate ironic.peer_list  https://review.opendev.org/c/openstack/nova/+/88334608:55
opendevreviewJohn Garbutt proposed openstack/nova master: Deprecate ironic.peer_list  https://review.opendev.org/c/openstack/nova/+/88334608:56
auniyalHi sean-k-mooney, tat for +W this https://review.opendev.org/c/openstack/nova/+/889947/7,11:21
auniyalbut the dependent patch is not +W yet, its has all +2's https://review.opendev.org/c/openstack/nova/+/881457/3511:22
opendevreviewsean mooney proposed openstack/nova master: increase job timeout for ocational slow nodes  https://review.opendev.org/c/openstack/nova/+/89336011:34
opendevreviewsean mooney proposed openstack/nova master: increase job timeout for ocational slow nodes  https://review.opendev.org/c/openstack/nova/+/89336011:41
sean-k-mooneyauniyal: thats intentionaly we will +w the previsou patch after we are happy with the final one11:41
sean-k-mooneyand merge the seriese  together11:41
auniyalack, 11:42
auniyalbut it won't fail merging the reproducer one (something like merge conflict) - because reproducer is over  that one11:43
sean-k-mooneyno it should not11:46
bauzasauniyal: have you seen the -1 from sean-k-mooney ?12:42
bauzason the top change ?12:42
dvo-plvsean-k-mooney, Hello. Are you here ?12:42
Ugglahello, I have some troubles with the service token, my assumption is cinder.py, neutron.py, ... were modified to use service token by calling service_auth.get_auth_plugin method. But I think this was not done for the services using the sdk. Can you confirm this assumption ?12:48
sean-k-mooneybauzas: if your around can you take a look at https://review.opendev.org/c/openstack/nova/+/893360 i have seen 1 or two timeouts so this might be good to land 12:52
auniyalbauzas, yes, updating 12:54
auniyalwas afk so late reply, 12:54
bauzassean-k-mooney: well, today, I'm litterrally sitting in front of Gerrit12:55
bauzasI still have melwitt's series on unified-limits nova-manage to look at12:55
sean-k-mooneyi can respin it now it will take 30 seconds12:57
bauzascool12:59
opendevreviewsean mooney proposed openstack/nova master: increase job timeout for occasional slow nodes  https://review.opendev.org/c/openstack/nova/+/89336012:59
sean-k-mooneyok i need to swap to somehting else. just an fyi your num instance change and my change to remove the  az filter have merged since yesterday13:00
sean-k-mooneybauzas: kasyaps min libivrt bump is still in the queue. it hit the rbac job timeout the last time13:01
bauzasyup saw13:01
bauzasI need to CC the min libvirt bump13:01
bauzasbut I'll track it anyway for RC113:01
bauzas(not impacted by FF)13:01
Ugglasean-k-mooney, bauzas any advices about my question above ?13:02
sean-k-mooneysorry didnt see it13:02
* sean-k-mooney looks back13:02
sean-k-mooneyis manial requireign service tokens13:02
sean-k-mooneywhen using the sdk ya you would have to handel that yourself am stephenfin might be able to point you in the right direction13:03
sean-k-mooneyalthough i belive he is off today13:03
sean-k-mooneylet me check his ironic series and see if he has done that convertion13:03
sean-k-mooneyUggla: so for ironic stpehin is using 13:06
sean-k-mooney            self._ironic_connection = utils.get_sdk_adapter(13:06
sean-k-mooney                'baremetal', check_service=True)13:06
sean-k-mooneywhich is https://github.com/openstack/nova/blob/master/nova/utils.py#L969-L99613:06
Ugglayep, I use the same "helper". But I think it does not manage service token.13:07
sean-k-mooneyso its calling ks_loading.load_session_from_conf_options13:08
Ugglayes13:09
sean-k-mooneyso no that does not look like it load the service user section13:11
sean-k-mooneyit ends up calling https://opendev.org/openstack/keystoneauth/src/commit/8d24892f9dc2742d9bd837b3a71fce260a5ec326/keystoneauth1/loading/session.py#L235-L25813:11
Ugglasean-k-mooney, yep so no service token right ?13:13
sean-k-mooneyno so you will need to figure out how to do htat13:13
sean-k-mooneyim looking at there docs quickly to see if they have an example13:14
sean-k-mooneyi think its part of the session but that just a guess13:14
Ugglasean-k-mooney, no worries at least it confirms my thoughts.13:17
sean-k-mooneyi belive there is an #openstack-sdk or #openstack-clinets channel?13:19
frickleractually #openstack-sdks for added confusion13:22
sean-k-mooneyfrickler: well the sdk does not import service_token form kestoneauth...13:23
sean-k-mooneyso it looks liek it does not supprot them at all 13:23
sean-k-mooneywhich is a probelm13:23
bauzasooooh ?13:24
kashyapsean-k-mooney: Thanks for this other-kind-of-bump :) : https://review.opendev.org/c/openstack/nova/+/89336013:24
kashyap(And the recheck on the version-bump patch itself)13:25
sean-k-mooneybauzas: https://codesearch.opendev.org/?q=from+keystoneauth1+import+service_token&i=nope&literal=nope&files=&excludeFiles=&repos=13:25
fricklerI don't know anything about these details, just wanted to point out the proper channel name. gtema and stephenfin over there can probably be much more helpful than me13:25
sean-k-mooneyfrickler: no worries knwoign it #openstack-sdks helps :)13:25
sean-k-mooneyso this is hwo nova enabels service tokens 13:26
sean-k-mooneyhttps://opendev.org/openstack/nova/src/branch/master/nova/service_auth.py#L38-L5413:26
bauzassean-k-mooney: doh13:26
sean-k-mooneywe might just be able to externally wrap the sdk object13:28
sean-k-mooneybut the sdk shoudl be update to supprot this nativly in conenct13:28
Ugglasean-k-mooney, I will ask in the sdks channel.13:30
opendevreviewJohn Garbutt proposed openstack/nova master: Limit nodes by ironic shard key  https://review.opendev.org/c/openstack/nova/+/88698013:31
bauzasdansmith: could you save me a couple of mins ? do you remember top of your head which helper method in o.vo can do a deep copy of a list of objects ?13:35
bauzascopy.deepcopy() can't work correctly13:35
bauzasand obj_clone() is IIRC only for single objects13:36
bauzasbut I can try13:36
dansmithbauzas: um, not sure there is anything other than obj_clone13:38
bauzasthen that's weird13:38
dansmithyou could obj_from_primitive(obj_to_primitive())13:39
bauzashttps://paste.opendev.org/show/b1nUZLuwjL1jJ0eFsTA5/13:39
bauzasso basically the containers aren't identical13:39
dansmithwell, if it made a copy of the objects inside then bdm1 is not in the copy list by definition13:40
bauzasdamn, right13:40
bauzasit's a deepcopy, the reference is different13:40
* bauzas facepalms13:40
dansmithI'll say again, I really don't think there's anything wrong with modifying the list in place13:41
bauzasauniyal: you know what ? instead of removing items from a copied list, just add items into a new empty list at start13:41
auniyalcopy.copy worked13:41
dansmithin fact it seems *more* right to me to do so13:41
dansmithplease don't copy.copy() an object13:41
auniyalenpty list - not of BlockDeviceMapping13:42
auniyalis it okay13:42
auniyalthis works13:42
auniyalhttps://paste.opendev.org/show/bRTqAnAkcUd9XQEhLiqP/13:42
dansmithplease don't do that13:43
bauzasauniyal: as dansmith said, this is wrong, you're also deleting the original object13:43
auniyalno it didn't other failed13:43
auniyalack13:43
auniyalbauzas, dansmith and return the list ?13:46
bauzasso, I'm wrapping my head for a simple pattern13:46
bauzasyou have many possibilities :13:47
bauzas1/ iterate over the list, mark the ones to delete, delete them at once in a comprehension list13:47
bauzas2/ generate an empty object list, iterate over the list, add every single bdm that's correct13:48
bauzasI'm in favor of 1/13:48
dansmithme too13:48
bauzas3/ you can use python's filter() with a lambda method13:49
dansmithand I'll say, you don't even need to do the comprehension list part,13:49
bauzas:)13:49
dansmithjust make the list of ones to delete, then go back and delete them13:49
bauzasthat too13:49
dansmithwhich I believe is what he already had before we startedin on this13:49
auniyal1 - iterate over the list; delete(bdm) if culprit else empty_list.append(bdm); return empty_list ??13:51
auniyaldansmith, so delete part is not happening, 13:52
auniyalthats I am doing here - https://review.opendev.org/c/openstack/nova/+/882284/53/nova/compute/manager.py13:52
dansmithno, don't return an empty list of course13:52
dansmithauniyal: just return nothing, as was said there would be fine13:53
dansmithto "make it clear that we're modifying in place". I don't think it's important to do that but if that matters to someone else, then fine13:53
dansmith"for now i would prefer if you just drop the return value and we can adress this later."13:54
dansmiththat's all you have to do13:54
auniyalack, thanks dansmith13:54
bauzasin other words, step back to PS5x-something13:55
dansmith*facepalm* yeah13:55
bauzasactually, not13:55
bauzasI stupidely gave you the wrong IndexError pattern which is to drop objects in a loop13:55
dansmithjust roll from here, delete the return value and it's good right?13:56
bauzasI'm lost between versions tbh, but yeah13:56
sean-k-mooneydelete the return and let it update in palce as it was and its fine14:00
sean-k-mooneyits not quite the same as the old patchset as that was missing assert in the unit tests i think14:02
auniyalyeah it passing all required tests14:02
auniyalyes, I updated tests14:02
opendevreviewAmit Uniyal proposed openstack/nova master: Delete dangling bdms  https://review.opendev.org/c/openstack/nova/+/88228414:03
sean-k-mooneyauniyal: you forgot the doc string14:05
sean-k-mooneyreturns: BlockDeviceMappingList list object.14:05
auniyaloh 14:05
auniyalsean-k-mooney, can you please here as well https://review.opendev.org/c/openstack/nova/+/882284/53..54/doc/source/admin/manage-volumes.rst14:07
auniyalif its fixed as per comments14:07
sean-k-mooney yep i already reviewd it and marked it doen14:07
auniyalack, thanks14:07
sean-k-mooneythe doc string is all that left so if you fix that ill add a +214:08
opendevreviewJohn Garbutt proposed openstack/nova master: Add nova-manage ironic-compute-node-move  https://review.opendev.org/c/openstack/nova/+/88698914:08
opendevreviewJohn Garbutt proposed openstack/nova master: Make compute node rebalance safter  https://review.opendev.org/c/openstack/nova/+/88715114:08
opendevreviewAmit Uniyal proposed openstack/nova master: Delete dangling bdms  https://review.opendev.org/c/openstack/nova/+/88228414:09
sean-k-mooneydansmith: bauzas: is there any reason to not +w the first partch in the ironic shard key serise. i.e. the rename/deprecaton of peer_list14:13
bauzasI was leaving this to the core who was +2/+W the top one14:14
johnthetubaguybauzas: thank you for all the reviews on that sharding chain (and after having turned the spell checker back on in my editor...) I have refreshed all those patches now.14:14
bauzasjohnthetubaguy: 've seen that, I'm now on the unified-limits nova-manage series, but I'll give yours a look before I stop14:15
sean-k-mooneyok i was going to +w to get it moving though ci since it looks fine and both you and dan +2'd it14:15
johnthetubaguysean-k-mooney: I think JayF was wanting to make sure the rest of series looked plauable first. Hopefully that is the case now.14:15
sean-k-mooneyno worries we can leave it there until the rest is reviewd14:15
JayFyeah I do not want us to deprecate peer list until we're sure sharding is going to land14:15
bauzassean-k-mooney: oh sorry, read too fast, I thought you were referring to amit's series14:15
sean-k-mooneybut i think we coudl merge that in this cycle even if the rest was not merged14:15
bauzasand yeah what johnthetubaguy said14:15
JayFdo not want to have to put our operators through two migrations on how nova-computes work :)14:15
johnthetubaguybauzas: awesome, thank you.14:16
bauzasJayF: I was surprised we didn't catch the ironic microversion bit, was it working when you tested ?14:16
sean-k-mooneyJayF: deprecating does not force them to migrate14:16
JayFI tested it with my lessee patch which also raised the microversion14:16
JayFsean-k-mooney: that's a good point14:16
bauzasJayF: ahah !14:16
JayFbauzas: I am planning on dedicating a day or two during RC to testing this, migrating nodes around, etc14:16
sean-k-mooneywe will have the option until the 2024.2 release at a minium14:17
JayFbauzas: just always forget how hectic this week in with my Ironic hat on :)14:17
JayFs/in/is/14:17
dansmithoh I just +Wd it..14:17
bauzasJayF: I don't see at all what you mean :p 14:17
dansmithI thought it was critical to get the deprecation into place this cycle,14:17
dansmithI haven't looked at the patches above14:17
bauzasthe two next ones are quite easy to review14:17
bauzasthe last one (the reshuffle) is the hardest14:18
sean-k-mooneywell i would like to do it this cycle but with new skip level porcess it actully has less of an impact14:18
johnthetubaguydansmith: FWIWI, that was my take too, tell people its broken, do active-passive if you can, keep using the old way if you must.14:19
dansmithyeah14:19
bauzasI thought we agreed on that even in the spec14:19
johnthetubaguy(It might have just been in PTG chat we said that)14:19
bauzasyeah14:19
sean-k-mooneythat why i was onderign why it had 2 +2s and no +w and why i ask14:19
sean-k-mooneyanyway im going to review the next two patches now before my next meeting14:20
dansmithsean-k-mooney: I just forgot to hit the +W14:20
JayFProbably because of my request, but you all are right that a deprecation won't force operator action immediately.14:20
opendevreviewAmit Uniyal proposed openstack/nova master: Delete dangling bdms  https://review.opendev.org/c/openstack/nova/+/88228414:20
johnthetubaguyJayF: we can always tweak the wording in the release note if we don't land the second half this cycle14:20
sean-k-mooneyjohnthetubaguy: JayF  everythign is in place for the shards on the ironic side yes?14:20
JayFsean-k-mooney: very yes :) We released it last cycle, specifically ahead of any of the nova work :D14:21
sean-k-mooneyim assuming so since these do not have depends on14:21
sean-k-mooneycool14:21
johnthetubaguyI think so, the API has been shipped.14:21
JayFjohnthetubaguy: if you wanted to test this yourself in devstack; I recently updated our doc here to have a plug+play devstack config (our old doc had bitrotted) https://docs.openstack.org/ironic/latest/contributor/devstack-guide.html#ironic-with-nova this is the config I'll start from when I play with it in RC14:21
auniyalbauzas,  it was not failing by default - missed one test :-(14:22
auniyalits done now14:22
johnthetubaguyJayF: I was trying to get a minion to do that, but I failed, that looks handy though :)14:22
JayFjohnthetubaguy: well, my goal was to make that doc minion-friendly ;) 14:22
sean-k-mooneyJayF  this is goinng to create 3 ironic vms on the host this is run on right14:23
JayFsean-k-mooney: yes, fronted by virtualbmc in this case (because it's ipmi)14:23
sean-k-mooneyi know there was test automation in ci to do that14:24
sean-k-mooneyi was not aware devstack could actully do that14:24
JayFIt's all in the devstack Ironic plugin14:24
sean-k-mooneyya that makes sense14:24
ozzzo_workWhat's the best way to determine whether a VM is locked?14:24
sean-k-mooneyozzzo_work: you can tell in the api with a server show14:24
ozzzo_workI'm on Train. Was that added in a later version?14:26
sean-k-mooneyi dont think so but i dont think it is shown in openstack client by default14:27
ozzzo_workI'm comparing "server show" output from locked and unlocked VMs, and I can't find a difference14:27
sean-k-mooneyis there a --long option for that command14:27
ozzzo_workno14:28
ozzzo_workaha I can see it with "nova show"14:28
johnthetubaguyMaybe also try update to the very latest openstack CLI and SDK so it uses the latest microversion by default, it was added in 2.9 apparently: https://docs.openstack.org/nova/latest/reference/api-microversion-history.html#id814:28
sean-k-mooneyyou can do "openstack sever list --locked"14:29
sean-k-mooneyhttps://docs.openstack.org/python-openstackclient/latest/cli/command-objects/server.html#cmdoption-openstack-server-list-locked14:30
ozzzo_workah very nice, ty!14:42
opendevreviewPavlo Shchelokovskyy proposed openstack/nova master: Replace -- as 3-4 char in hostname with -  https://review.opendev.org/c/openstack/nova/+/89307215:40
alaysdHey everyone16:20
sean-k-mooneybauzas: fyi i put a -2 on https://review.opendev.org/c/openstack/nova/+/893072 for https://bugs.launchpad.net/nova/+bug/203340116:38
sean-k-mooneythis is an api change so it would need a spec but in general im not sure we want to do any mroe vlaidation fo hostname with all the pain we had last time16:38
bauzasNOOOOOOOOO16:38
bauzas(not no because of your -2 but because of the rathole we entered when we went with supporting FQDNs16:39
sean-k-mooneyright so if we add even more santaisation to ensure we comply with https://www.rfc-editor.org/rfc/rfc5891#section-4.2.3.116:40
bauzasmy personal take : fix it with docs16:40
sean-k-mooneyit will break someone who does not care about it16:40
sean-k-mooneyim not enven sure it needs to be in our docs but we could jus tadd a note16:40
bauzaschange as much as you want on clients (incl SDK and OSC), API-ref and the likes, but please please please, never ever touch this again16:41
sean-k-mooneybut basically i think we shoudl leave this up to the user to do16:41
bauzaswe have api-reference16:41
bauzasand I know 1% of our users read it16:41
bauzasbut that's the place where we can put 'doh, we don't support rfc5891'16:42
sean-k-mooneyya a note in api-ref would be ok but it woudl basically say. if you do not follwo the relvente RFCs for hostname formats thigns might break16:42
sean-k-mooneyits not that we dont suport it16:42
bauzastrue, nova is just passing thru the hostname16:42
bauzasso you're due to respect this16:42
sean-k-mooneyits that we dont auto manage names that are invlaid to make it pass16:42
opendevreviewAmit Uniyal proposed openstack/nova master: Allow swap resize from non-zero to zero  https://review.opendev.org/c/openstack/nova/+/85733916:43
bauzascores, there are two series that still require love from you in order to be accepted in time for the FF : https://review.opendev.org/q/topic:ironic-shards+project:openstack/nova+is:open and https://review.opendev.org/q/topic:bp%252Funified-limits-nova-tool-and-docs16:51
bauzasboth have all their changes at least having a single +216:51
sean-k-mooneybauzas: this is already approved but we shoudld track this for rc1 https://review.opendev.org/c/openstack/nova/+/88334417:19
opendevreviewJohn Garbutt proposed openstack/nova master: Add nova-manage ironic-compute-node-move  https://review.opendev.org/c/openstack/nova/+/88698917:19
bauzassean-k-mooney: I'll start creating an etherpad17:20
sean-k-mooneyits running in zuul now so we can just keep an eye on it but i tought this was already merged17:21
opendevreviewJohn Garbutt proposed openstack/nova master: Make compute node rebalance safter  https://review.opendev.org/c/openstack/nova/+/88715117:21
johnthetubaguymelwitt: bauzas sorry, my rebase broke the functional tests without me noticing, I fixed that up now.17:22
bauzasjohnthetubaguy: already sent back to the gate17:22
johnthetubaguythank you.17:23
* bauzas has two screens, one for Gerrit and one for inbocx17:23
melwittdual wielding17:23
sean-k-mooneyso given how close this is if it does not merge today i would be open to an FFE for the ironic shard stuff17:23
bauzaswe don't need to define a RFE process 17:25
bauzasthe ironic shard one can be late approved IMHO, one core is already giving his go, and two patches of 4 are already +W17:25
bauzaseventually, the change surface is quite restricted to the ironic driver17:25
sean-k-mooneywell more just indrectly noting that today is thehnicall feature freeze 17:26
sean-k-mooneybut as you note it had +2 across the borad17:26
bauzasso, provided JayF or other ironic folks can hardly stress-test this thing and can provide bugfixes those 2 weeks, I'm good17:26
melwittthis patch has only one +2 but was +W'ed, was that intentional? https://review.opendev.org/c/openstack/nova/+/88715117:26
bauzassean-k-mooney: I'm very well aware of today's FF :)17:26
johnthetubaguyin perfect timing, I am going to the beach tomorrow... (obvs to celebrate feature freeze with an ice cream)17:27
bauzasmelwitt: doh, my fault !17:27
sean-k-mooneyjohnthetubaguy: nice17:27
sean-k-mooneyjohnthetubaguy: enjoy it 17:27
sean-k-mooneyim also not planning to be here at this time tomrorrow evening17:27
* sean-k-mooney normally works later but im not planning to work late teh firday after FF17:28
bauzasmelwitt: I don't know why but when I proxy'd your +W on the dependent patch, I messed up and added it too to the top patch17:28
johnthetubaguyThanks for the reviews, I am back Monday if there is stuff to help push it across the line17:28
bauzasapologies, no pun intended17:28
melwittah ok17:28
bauzashttps://etherpad.opendev.org/p/nova-bobcat-blueprint-status tells me we are mostly done with all the blueprints we were able to review17:29
bauzasexcept the large manila series and the ephemeral storage17:29
bauzasso, again, we still have some patches requiring second +2 on https://review.opendev.org/q/topic:ironic-shards and https://review.opendev.org/q/topic:bp%252Funified-limits-nova-tool-and-docs17:32
bauzasfor me, my day is almost over17:32
bauzasbut I can hang for a bit17:33
zer0c00lI think this change in libvirt 9.5.0 (shipped with centos stream 9) breaks nova, (atleast yoga) https://github.com/libvirt/libvirt/commit/a2ae3d299cf Any one seen this before?17:37
sean-k-mooneyzer0c00l: with what network backend17:39
sean-k-mooneycalico?17:39
sean-k-mooneythe intree network backends do not create taps nor does os-vif17:40
sean-k-mooneythey are only created by libvirt17:40
sean-k-mooneyzer0c00l: i am not aware of any supported backend where the tap is precreated17:41
sean-k-mooneyso i dont think this will impact nova17:41
zer0c00lsean-k-mooney: yeah we are running calico17:41
sean-k-mooneywell even in the calico case libvirt is ment to create the tap17:41
sean-k-mooneycalico just manges the routing17:42
sean-k-mooneycalico is apprently still using vif type tap https://github.com/projectcalico/calico/blob/cf7fa35475eba84f5afcd7f53ac7d07dcb403202/networking-calico/networking_calico/plugins/ml2/drivers/calico/test/lib.py#L66C31-L66C3417:54
JayFbauzas: Yeah, I am dedicating at least one full day next week to testing sharding in devstack, and potentially writing a tempest test if I get far enough :) 17:54
sean-k-mooneyvif type tap is not supported by our os-vif code so its usign the legacy fallback17:54
sean-k-mooneyhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L595-L59617:55
sean-k-mooneyhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L420-L43017:55
sean-k-mooneyhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/designer.py#L44-L5517:56
sean-k-mooneyzer0c00l: with that said the tap was always ment to be created by libvirt so it sound like calico might have been doing things it shoudl not have been17:57
zer0c00lsean-k-mooney: Thanks for looking into this. :(17:57
sean-k-mooneywe could proably correct this with a bug fix17:58
sean-k-mooneyjsut setting managed='no'17:58
sean-k-mooneyhere https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L42717:58
sean-k-mooneythe problem is that the there is no way to test this really upstream17:59
sean-k-mooneywell beyond unit/fucntional tests17:59
sean-k-mooneybut we dont have any calico ci17:59
sean-k-mooneycalico should be the only backend using vif_type=tap18:00
sean-k-mooneybut im not sure if we woudl need a config option in the workarounds section for this or not18:00
zer0c00lsean-k-mooney: yeah, my team member is doing exactly that.18:00
sean-k-mooneyim kind of reluctant to set managed='no' because that might eb a security issue18:01
zer0c00l        if self.target_dev is not None:18:01
zer0c00l            dev.append(etree.Element("target", dev=self.target_dev, managed=self.managed))18:01
sean-k-mooneyzer0c00l: can you file a bug for this.18:02
zer0c00lsean-k-mooney: i will do that. thank you.18:02
sean-k-mooneyjohnthetubaguy: JayF  https://review.opendev.org/c/openstack/nova/+/886980/7/nova/conf/ironic.py should that really be mutable?18:08
JayFI think that's an astute observation by you, and it's likely better not mutable18:09
JayFsean-k-mooney: I can add a follow up to flip it back if you wanna land that?18:09
sean-k-mooneyim not going to hold the series on that but ya a follow up would likely be a good idea18:09
JayFack, I might wait for it to land and do the follow up directly against master, just to ensure I don't accidentally restack the whole set18:09
JayFbut I am making a note in my calendar to check for it tomorrow morning18:10
sean-k-mooneywe have 2 +2 on everything now18:10
sean-k-mooneyso ill add a +w to that shortly 18:10
JayFexactly why I don't wanna risk accidentally re-pushing them :D18:10
sean-k-mooneyim just going to review the one patch i did not look at18:10
sean-k-mooneyheheh -R is useful to prevent change but ya we can fix this after FF18:10
* JayF goes and looks at a git-review doc18:11
JayFI'd love to learn that trick anyway :D18:11
sean-k-mooneyit just prevents it doing any rebases18:12
sean-k-mooneyok food just arrived. i skimed the 3rd patch and didnt see anything at a glance so im going to rely on the other reviwes and i added +w to the second patch18:14
sean-k-mooneyso the seriese shoudl have +2w all the way18:15
sean-k-mooneymelwitt: ill try and find time later tonight to look at https://review.opendev.org/q/topic:bp%252Funified-limits-nova-tool-and-docs18:16
melwittUggla: re: your service token question from earlier. I dunno if you are using a "[manila]" conf section or similar, but the get_sdk_adapter helper "as-is" only considers a conf section for the given service type. the nova/service_auth.py is a helper method that will use the [service_user] conf group (allowing for sharing of the settings and not have to duplicate them for each service's conf section, for example)18:16
sean-k-mooneyif other get to it before me however dont wait for my review18:16
sean-k-mooneyo/18:16
melwittsean-k-mooney: thanks 🙏 seeya later18:19
melwittUggla: I _think_ if you added send_service_user_token = True in the [manila] section alongside the other [manila] auth it will work as-is (assuming you are wanting to send a service user token with a request). to make it work with [service_user] I think you would have to update get_sdk_adapter to accept an auth'ed session (for example Session(auth=service_auth.get_auth_plugin()) and then pass that to Connection(session=<session>)18:22
melwitt https://docs.openstack.org/openstacksdk/latest/user/connection.html#from-existing-authenticated-session18:22
zer0c00lsean-k-mooney: FYI https://bugs.launchpad.net/nova/+bug/203368118:55
opendevreviewMerged openstack/nova master: Deprecate ironic.peer_list  https://review.opendev.org/c/openstack/nova/+/88334619:06
opendevreviewMerged openstack/nova master: Add function to get all attachments in Cinder.API module  https://review.opendev.org/c/openstack/nova/+/88145719:06
opendevreviewMerged openstack/nova master: Reproducer for dangling bdms  https://review.opendev.org/c/openstack/nova/+/88994719:06
opendevreviewMerged openstack/nova master: Limit nodes by ironic shard key  https://review.opendev.org/c/openstack/nova/+/88698021:00
opendevreviewMerged openstack/placement master: Modify the comment that is confused  https://review.opendev.org/c/openstack/placement/+/80994821:00
opendevreviewMerged openstack/nova master: Delete dangling bdms  https://review.opendev.org/c/openstack/nova/+/88228421:02
opendevreviewMerged openstack/nova master: Update serial console example client for py3  https://review.opendev.org/c/openstack/nova/+/87808021:02
opendevreviewMerged openstack/nova master: Remove unused mocks  https://review.opendev.org/c/openstack/nova/+/86778921:02
opendevreviewMerged openstack/nova master: Fix tox docs target  https://review.opendev.org/c/openstack/nova/+/89103321:02
sean-k-mooneymelwitt: o/21:48
melwittsean-k-mooney: o/21:49
sean-k-mooneyso the way the doc and porting tool are managing vcpu quota is not correct21:50
sean-k-mooneyhttps://review.opendev.org/c/openstack/nova/+/891646/5/nova/cmd/manage.py#346121:50
sean-k-mooneybut i think we can adress both of those issues in a followup patch21:51
sean-k-mooneywhen migrating the quota on flavor.vcpu it should not be mapped 1:1 to the vcpu resouce class limit exclively 21:51
sean-k-mooneywe need to also set the same limit for PCPUs21:51
sean-k-mooneysince flavor.vcpu can be either VCPU alloctions , PCPU alloctions or both in the case of a vm with mixed cpus21:52
sean-k-mooneyso while that has the downside of effectivly doubling the qutoa for cpus21:52
sean-k-mooneyi think its the only way to avoid over quota issues in general21:53
melwittsean-k-mooney: hm, ok21:53
sean-k-mooneysince we have not made the new quota driver the default21:54
sean-k-mooneyjust deprecated the old one21:54
sean-k-mooneyim fine with doing that in a followup21:54
melwittsean-k-mooney: you've reminded me there's a config option around this https://docs.openstack.org/nova/latest/configuration/config.html#workarounds.unified_limits_count_pcpu_as_vcpu21:54
melwittas well21:54
sean-k-mooneyoh ya there is/was21:54
melwittack. I'm working on follow up stuff21:54
sean-k-mooneyi have approved the two patches as they are since for most peopel it will do what they need21:55
melwittthank you21:56
sean-k-mooneyno worreis i came online to do that before i head to bed so ill chat to you soon o/21:57
melwittsean-k-mooney: thanks 🙏 ttyl o/21:57
opendevreviewMerged openstack/nova master: nova-manage: Add 'limits migrate_to_unified_limits'  https://review.opendev.org/c/openstack/nova/+/89164623:54
opendevreviewMerged openstack/nova master: Add documentation for unified limits  https://review.opendev.org/c/openstack/nova/+/89312723:54

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