| opendevreview | melanie witt proposed openstack/nova master: Enable use of native threads in scatter_gather_cells https://review.opendev.org/c/openstack/nova/+/650172 | 02:00 |
|---|---|---|
| opendevreview | Merged openstack/nova master: libvirt: Add 'COMPUTE_ADDRESS_SPACE_*' traits support https://review.opendev.org/c/openstack/nova/+/873221 | 02:47 |
| opendevreview | OpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata https://review.opendev.org/c/openstack/nova/+/891648 | 04:58 |
| opendevreview | Amit Uniyal proposed openstack/nova master: Reproducer for dangling bdms https://review.opendev.org/c/openstack/nova/+/889947 | 06:11 |
| opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | 06:11 |
| auniyal | thanks sean-k-mooney for the reviews | 06:50 |
| *** ralonsoh_ooo is now known as ralonsoh | 07:18 | |
| opendevreview | Merged openstack/nova master: Remove deprecated AZ filter. https://review.opendev.org/c/openstack/nova/+/886779 | 07:21 |
| opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | 08:08 |
| opendevreview | John Garbutt proposed openstack/nova master: Deprecate ironic.peer_list https://review.opendev.org/c/openstack/nova/+/883346 | 08:55 |
| opendevreview | John Garbutt proposed openstack/nova master: Deprecate ironic.peer_list https://review.opendev.org/c/openstack/nova/+/883346 | 08:56 |
| auniyal | Hi sean-k-mooney, tat for +W this https://review.opendev.org/c/openstack/nova/+/889947/7, | 11:21 |
| auniyal | but the dependent patch is not +W yet, its has all +2's https://review.opendev.org/c/openstack/nova/+/881457/35 | 11:22 |
| opendevreview | sean mooney proposed openstack/nova master: increase job timeout for ocational slow nodes https://review.opendev.org/c/openstack/nova/+/893360 | 11:34 |
| opendevreview | sean mooney proposed openstack/nova master: increase job timeout for ocational slow nodes https://review.opendev.org/c/openstack/nova/+/893360 | 11:41 |
| sean-k-mooney | auniyal: thats intentionaly we will +w the previsou patch after we are happy with the final one | 11:41 |
| sean-k-mooney | and merge the seriese together | 11:41 |
| auniyal | ack, | 11:42 |
| auniyal | but it won't fail merging the reproducer one (something like merge conflict) - because reproducer is over that one | 11:43 |
| sean-k-mooney | no it should not | 11:46 |
| bauzas | auniyal: have you seen the -1 from sean-k-mooney ? | 12:42 |
| bauzas | on the top change ? | 12:42 |
| dvo-plv | sean-k-mooney, Hello. Are you here ? | 12:42 |
| Uggla | hello, 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-mooney | bauzas: 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 |
| auniyal | bauzas, yes, updating | 12:54 |
| auniyal | was afk so late reply, | 12:54 |
| bauzas | sean-k-mooney: well, today, I'm litterrally sitting in front of Gerrit | 12:55 |
| bauzas | I still have melwitt's series on unified-limits nova-manage to look at | 12:55 |
| sean-k-mooney | i can respin it now it will take 30 seconds | 12:57 |
| bauzas | cool | 12:59 |
| opendevreview | sean mooney proposed openstack/nova master: increase job timeout for occasional slow nodes https://review.opendev.org/c/openstack/nova/+/893360 | 12:59 |
| sean-k-mooney | ok 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 yesterday | 13:00 |
| sean-k-mooney | bauzas: kasyaps min libivrt bump is still in the queue. it hit the rbac job timeout the last time | 13:01 |
| bauzas | yup saw | 13:01 |
| bauzas | I need to CC the min libvirt bump | 13:01 |
| bauzas | but I'll track it anyway for RC1 | 13:01 |
| bauzas | (not impacted by FF) | 13:01 |
| Uggla | sean-k-mooney, bauzas any advices about my question above ? | 13:02 |
| sean-k-mooney | sorry didnt see it | 13:02 |
| * sean-k-mooney looks back | 13:02 | |
| sean-k-mooney | is manial requireign service tokens | 13:02 |
| sean-k-mooney | when using the sdk ya you would have to handel that yourself am stephenfin might be able to point you in the right direction | 13:03 |
| sean-k-mooney | although i belive he is off today | 13:03 |
| sean-k-mooney | let me check his ironic series and see if he has done that convertion | 13:03 |
| sean-k-mooney | Uggla: 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-mooney | which is https://github.com/openstack/nova/blob/master/nova/utils.py#L969-L996 | 13:06 |
| Uggla | yep, I use the same "helper". But I think it does not manage service token. | 13:07 |
| sean-k-mooney | so its calling ks_loading.load_session_from_conf_options | 13:08 |
| Uggla | yes | 13:09 |
| sean-k-mooney | so no that does not look like it load the service user section | 13:11 |
| sean-k-mooney | it ends up calling https://opendev.org/openstack/keystoneauth/src/commit/8d24892f9dc2742d9bd837b3a71fce260a5ec326/keystoneauth1/loading/session.py#L235-L258 | 13:11 |
| Uggla | sean-k-mooney, yep so no service token right ? | 13:13 |
| sean-k-mooney | no so you will need to figure out how to do htat | 13:13 |
| sean-k-mooney | im looking at there docs quickly to see if they have an example | 13:14 |
| sean-k-mooney | i think its part of the session but that just a guess | 13:14 |
| Uggla | sean-k-mooney, no worries at least it confirms my thoughts. | 13:17 |
| sean-k-mooney | i belive there is an #openstack-sdk or #openstack-clinets channel? | 13:19 |
| frickler | actually #openstack-sdks for added confusion | 13:22 |
| sean-k-mooney | frickler: well the sdk does not import service_token form kestoneauth... | 13:23 |
| sean-k-mooney | so it looks liek it does not supprot them at all | 13:23 |
| sean-k-mooney | which is a probelm | 13:23 |
| bauzas | ooooh ? | 13:24 |
| kashyap | sean-k-mooney: Thanks for this other-kind-of-bump :) : https://review.opendev.org/c/openstack/nova/+/893360 | 13:24 |
| kashyap | (And the recheck on the version-bump patch itself) | 13:25 |
| sean-k-mooney | bauzas: https://codesearch.opendev.org/?q=from+keystoneauth1+import+service_token&i=nope&literal=nope&files=&excludeFiles=&repos= | 13:25 |
| frickler | I 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 me | 13:25 |
| sean-k-mooney | frickler: no worries knwoign it #openstack-sdks helps :) | 13:25 |
| sean-k-mooney | so this is hwo nova enabels service tokens | 13:26 |
| sean-k-mooney | https://opendev.org/openstack/nova/src/branch/master/nova/service_auth.py#L38-L54 | 13:26 |
| bauzas | sean-k-mooney: doh | 13:26 |
| sean-k-mooney | we might just be able to externally wrap the sdk object | 13:28 |
| sean-k-mooney | but the sdk shoudl be update to supprot this nativly in conenct | 13:28 |
| Uggla | sean-k-mooney, I will ask in the sdks channel. | 13:30 |
| opendevreview | John Garbutt proposed openstack/nova master: Limit nodes by ironic shard key https://review.opendev.org/c/openstack/nova/+/886980 | 13:31 |
| bauzas | dansmith: 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 |
| bauzas | copy.deepcopy() can't work correctly | 13:35 |
| bauzas | and obj_clone() is IIRC only for single objects | 13:36 |
| bauzas | but I can try | 13:36 |
| dansmith | bauzas: um, not sure there is anything other than obj_clone | 13:38 |
| bauzas | then that's weird | 13:38 |
| dansmith | you could obj_from_primitive(obj_to_primitive()) | 13:39 |
| bauzas | https://paste.opendev.org/show/b1nUZLuwjL1jJ0eFsTA5/ | 13:39 |
| bauzas | so basically the containers aren't identical | 13:39 |
| dansmith | well, if it made a copy of the objects inside then bdm1 is not in the copy list by definition | 13:40 |
| bauzas | damn, right | 13:40 |
| bauzas | it's a deepcopy, the reference is different | 13:40 |
| * bauzas facepalms | 13:40 | |
| dansmith | I'll say again, I really don't think there's anything wrong with modifying the list in place | 13:41 |
| bauzas | auniyal: you know what ? instead of removing items from a copied list, just add items into a new empty list at start | 13:41 |
| auniyal | copy.copy worked | 13:41 |
| dansmith | in fact it seems *more* right to me to do so | 13:41 |
| dansmith | please don't copy.copy() an object | 13:41 |
| auniyal | enpty list - not of BlockDeviceMapping | 13:42 |
| auniyal | is it okay | 13:42 |
| auniyal | this works | 13:42 |
| auniyal | https://paste.opendev.org/show/bRTqAnAkcUd9XQEhLiqP/ | 13:42 |
| dansmith | please don't do that | 13:43 |
| bauzas | auniyal: as dansmith said, this is wrong, you're also deleting the original object | 13:43 |
| auniyal | no it didn't other failed | 13:43 |
| auniyal | ack | 13:43 |
| auniyal | bauzas, dansmith and return the list ? | 13:46 |
| bauzas | so, I'm wrapping my head for a simple pattern | 13:46 |
| bauzas | you have many possibilities : | 13:47 |
| bauzas | 1/ iterate over the list, mark the ones to delete, delete them at once in a comprehension list | 13:47 |
| bauzas | 2/ generate an empty object list, iterate over the list, add every single bdm that's correct | 13:48 |
| bauzas | I'm in favor of 1/ | 13:48 |
| dansmith | me too | 13:48 |
| bauzas | 3/ you can use python's filter() with a lambda method | 13:49 |
| dansmith | and I'll say, you don't even need to do the comprehension list part, | 13:49 |
| bauzas | :) | 13:49 |
| dansmith | just make the list of ones to delete, then go back and delete them | 13:49 |
| bauzas | that too | 13:49 |
| dansmith | which I believe is what he already had before we startedin on this | 13:49 |
| auniyal | 1 - iterate over the list; delete(bdm) if culprit else empty_list.append(bdm); return empty_list ?? | 13:51 |
| auniyal | dansmith, so delete part is not happening, | 13:52 |
| auniyal | thats I am doing here - https://review.opendev.org/c/openstack/nova/+/882284/53/nova/compute/manager.py | 13:52 |
| dansmith | no, don't return an empty list of course | 13:52 |
| dansmith | auniyal: just return nothing, as was said there would be fine | 13:53 |
| dansmith | to "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 fine | 13:53 |
| dansmith | "for now i would prefer if you just drop the return value and we can adress this later." | 13:54 |
| dansmith | that's all you have to do | 13:54 |
| auniyal | ack, thanks dansmith | 13:54 |
| bauzas | in other words, step back to PS5x-something | 13:55 |
| dansmith | *facepalm* yeah | 13:55 |
| bauzas | actually, not | 13:55 |
| bauzas | I stupidely gave you the wrong IndexError pattern which is to drop objects in a loop | 13:55 |
| dansmith | just roll from here, delete the return value and it's good right? | 13:56 |
| bauzas | I'm lost between versions tbh, but yeah | 13:56 |
| sean-k-mooney | delete the return and let it update in palce as it was and its fine | 14:00 |
| sean-k-mooney | its not quite the same as the old patchset as that was missing assert in the unit tests i think | 14:02 |
| auniyal | yeah it passing all required tests | 14:02 |
| auniyal | yes, I updated tests | 14:02 |
| opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | 14:03 |
| sean-k-mooney | auniyal: you forgot the doc string | 14:05 |
| sean-k-mooney | returns: BlockDeviceMappingList list object. | 14:05 |
| auniyal | oh | 14:05 |
| auniyal | sean-k-mooney, can you please here as well https://review.opendev.org/c/openstack/nova/+/882284/53..54/doc/source/admin/manage-volumes.rst | 14:07 |
| auniyal | if its fixed as per comments | 14:07 |
| sean-k-mooney | yep i already reviewd it and marked it doen | 14:07 |
| auniyal | ack, thanks | 14:07 |
| sean-k-mooney | the doc string is all that left so if you fix that ill add a +2 | 14:08 |
| opendevreview | John Garbutt proposed openstack/nova master: Add nova-manage ironic-compute-node-move https://review.opendev.org/c/openstack/nova/+/886989 | 14:08 |
| opendevreview | John Garbutt proposed openstack/nova master: Make compute node rebalance safter https://review.opendev.org/c/openstack/nova/+/887151 | 14:08 |
| opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | 14:09 |
| sean-k-mooney | dansmith: bauzas: is there any reason to not +w the first partch in the ironic shard key serise. i.e. the rename/deprecaton of peer_list | 14:13 |
| bauzas | I was leaving this to the core who was +2/+W the top one | 14:14 |
| johnthetubaguy | bauzas: 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 |
| bauzas | johnthetubaguy: 've seen that, I'm now on the unified-limits nova-manage series, but I'll give yours a look before I stop | 14:15 |
| sean-k-mooney | ok i was going to +w to get it moving though ci since it looks fine and both you and dan +2'd it | 14:15 |
| johnthetubaguy | sean-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-mooney | no worries we can leave it there until the rest is reviewd | 14:15 |
| JayF | yeah I do not want us to deprecate peer list until we're sure sharding is going to land | 14:15 |
| bauzas | sean-k-mooney: oh sorry, read too fast, I thought you were referring to amit's series | 14:15 |
| sean-k-mooney | but i think we coudl merge that in this cycle even if the rest was not merged | 14:15 |
| bauzas | and yeah what johnthetubaguy said | 14:15 |
| JayF | do not want to have to put our operators through two migrations on how nova-computes work :) | 14:15 |
| johnthetubaguy | bauzas: awesome, thank you. | 14:16 |
| bauzas | JayF: I was surprised we didn't catch the ironic microversion bit, was it working when you tested ? | 14:16 |
| sean-k-mooney | JayF: deprecating does not force them to migrate | 14:16 |
| JayF | I tested it with my lessee patch which also raised the microversion | 14:16 |
| JayF | sean-k-mooney: that's a good point | 14:16 |
| bauzas | JayF: ahah ! | 14:16 |
| JayF | bauzas: I am planning on dedicating a day or two during RC to testing this, migrating nodes around, etc | 14:16 |
| sean-k-mooney | we will have the option until the 2024.2 release at a minium | 14:17 |
| JayF | bauzas: just always forget how hectic this week in with my Ironic hat on :) | 14:17 |
| JayF | s/in/is/ | 14:17 |
| dansmith | oh I just +Wd it.. | 14:17 |
| bauzas | JayF: I don't see at all what you mean :p | 14:17 |
| dansmith | I thought it was critical to get the deprecation into place this cycle, | 14:17 |
| dansmith | I haven't looked at the patches above | 14:17 |
| bauzas | the two next ones are quite easy to review | 14:17 |
| bauzas | the last one (the reshuffle) is the hardest | 14:18 |
| sean-k-mooney | well i would like to do it this cycle but with new skip level porcess it actully has less of an impact | 14:18 |
| johnthetubaguy | dansmith: 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 |
| dansmith | yeah | 14:19 |
| bauzas | I thought we agreed on that even in the spec | 14:19 |
| johnthetubaguy | (It might have just been in PTG chat we said that) | 14:19 |
| bauzas | yeah | 14:19 |
| sean-k-mooney | that why i was onderign why it had 2 +2s and no +w and why i ask | 14:19 |
| sean-k-mooney | anyway im going to review the next two patches now before my next meeting | 14:20 |
| dansmith | sean-k-mooney: I just forgot to hit the +W | 14:20 |
| JayF | Probably because of my request, but you all are right that a deprecation won't force operator action immediately. | 14:20 |
| opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | 14:20 |
| johnthetubaguy | JayF: we can always tweak the wording in the release note if we don't land the second half this cycle | 14:20 |
| sean-k-mooney | johnthetubaguy: JayF everythign is in place for the shards on the ironic side yes? | 14:20 |
| JayF | sean-k-mooney: very yes :) We released it last cycle, specifically ahead of any of the nova work :D | 14:21 |
| sean-k-mooney | im assuming so since these do not have depends on | 14:21 |
| sean-k-mooney | cool | 14:21 |
| johnthetubaguy | I think so, the API has been shipped. | 14:21 |
| JayF | johnthetubaguy: 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 RC | 14:21 |
| auniyal | bauzas, it was not failing by default - missed one test :-( | 14:22 |
| auniyal | its done now | 14:22 |
| johnthetubaguy | JayF: I was trying to get a minion to do that, but I failed, that looks handy though :) | 14:22 |
| JayF | johnthetubaguy: well, my goal was to make that doc minion-friendly ;) | 14:22 |
| sean-k-mooney | JayF this is goinng to create 3 ironic vms on the host this is run on right | 14:23 |
| JayF | sean-k-mooney: yes, fronted by virtualbmc in this case (because it's ipmi) | 14:23 |
| sean-k-mooney | i know there was test automation in ci to do that | 14:24 |
| sean-k-mooney | i was not aware devstack could actully do that | 14:24 |
| JayF | It's all in the devstack Ironic plugin | 14:24 |
| sean-k-mooney | ya that makes sense | 14:24 |
| ozzzo_work | What's the best way to determine whether a VM is locked? | 14:24 |
| sean-k-mooney | ozzzo_work: you can tell in the api with a server show | 14:24 |
| ozzzo_work | I'm on Train. Was that added in a later version? | 14:26 |
| sean-k-mooney | i dont think so but i dont think it is shown in openstack client by default | 14:27 |
| ozzzo_work | I'm comparing "server show" output from locked and unlocked VMs, and I can't find a difference | 14:27 |
| sean-k-mooney | is there a --long option for that command | 14:27 |
| ozzzo_work | no | 14:28 |
| ozzzo_work | aha I can see it with "nova show" | 14:28 |
| johnthetubaguy | Maybe 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#id8 | 14:28 |
| sean-k-mooney | you can do "openstack sever list --locked" | 14:29 |
| sean-k-mooney | https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/server.html#cmdoption-openstack-server-list-locked | 14:30 |
| ozzzo_work | ah very nice, ty! | 14:42 |
| opendevreview | Pavlo Shchelokovskyy proposed openstack/nova master: Replace -- as 3-4 char in hostname with - https://review.opendev.org/c/openstack/nova/+/893072 | 15:40 |
| alaysd | Hey everyone | 16:20 |
| sean-k-mooney | bauzas: fyi i put a -2 on https://review.opendev.org/c/openstack/nova/+/893072 for https://bugs.launchpad.net/nova/+bug/2033401 | 16:38 |
| sean-k-mooney | this 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 time | 16:38 |
| bauzas | NOOOOOOOOO | 16:38 |
| bauzas | (not no because of your -2 but because of the rathole we entered when we went with supporting FQDNs | 16:39 |
| sean-k-mooney | right so if we add even more santaisation to ensure we comply with https://www.rfc-editor.org/rfc/rfc5891#section-4.2.3.1 | 16:40 |
| bauzas | my personal take : fix it with docs | 16:40 |
| sean-k-mooney | it will break someone who does not care about it | 16:40 |
| sean-k-mooney | im not enven sure it needs to be in our docs but we could jus tadd a note | 16:40 |
| bauzas | change as much as you want on clients (incl SDK and OSC), API-ref and the likes, but please please please, never ever touch this again | 16:41 |
| sean-k-mooney | but basically i think we shoudl leave this up to the user to do | 16:41 |
| bauzas | we have api-reference | 16:41 |
| bauzas | and I know 1% of our users read it | 16:41 |
| bauzas | but that's the place where we can put 'doh, we don't support rfc5891' | 16:42 |
| sean-k-mooney | ya 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 break | 16:42 |
| sean-k-mooney | its not that we dont suport it | 16:42 |
| bauzas | true, nova is just passing thru the hostname | 16:42 |
| bauzas | so you're due to respect this | 16:42 |
| sean-k-mooney | its that we dont auto manage names that are invlaid to make it pass | 16:42 |
| opendevreview | Amit Uniyal proposed openstack/nova master: Allow swap resize from non-zero to zero https://review.opendev.org/c/openstack/nova/+/857339 | 16:43 |
| bauzas | cores, 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-docs | 16:51 |
| bauzas | both have all their changes at least having a single +2 | 16:51 |
| sean-k-mooney | bauzas: this is already approved but we shoudld track this for rc1 https://review.opendev.org/c/openstack/nova/+/883344 | 17:19 |
| opendevreview | John Garbutt proposed openstack/nova master: Add nova-manage ironic-compute-node-move https://review.opendev.org/c/openstack/nova/+/886989 | 17:19 |
| bauzas | sean-k-mooney: I'll start creating an etherpad | 17:20 |
| sean-k-mooney | its running in zuul now so we can just keep an eye on it but i tought this was already merged | 17:21 |
| opendevreview | John Garbutt proposed openstack/nova master: Make compute node rebalance safter https://review.opendev.org/c/openstack/nova/+/887151 | 17:21 |
| johnthetubaguy | melwitt: bauzas sorry, my rebase broke the functional tests without me noticing, I fixed that up now. | 17:22 |
| bauzas | johnthetubaguy: already sent back to the gate | 17:22 |
| johnthetubaguy | thank you. | 17:23 |
| * bauzas has two screens, one for Gerrit and one for inbocx | 17:23 | |
| melwitt | dual wielding | 17:23 |
| sean-k-mooney | so given how close this is if it does not merge today i would be open to an FFE for the ironic shard stuff | 17:23 |
| bauzas | we don't need to define a RFE process | 17:25 |
| bauzas | the ironic shard one can be late approved IMHO, one core is already giving his go, and two patches of 4 are already +W | 17:25 |
| bauzas | eventually, the change surface is quite restricted to the ironic driver | 17:25 |
| sean-k-mooney | well more just indrectly noting that today is thehnicall feature freeze | 17:26 |
| sean-k-mooney | but as you note it had +2 across the borad | 17:26 |
| bauzas | so, provided JayF or other ironic folks can hardly stress-test this thing and can provide bugfixes those 2 weeks, I'm good | 17:26 |
| melwitt | this patch has only one +2 but was +W'ed, was that intentional? https://review.opendev.org/c/openstack/nova/+/887151 | 17:26 |
| bauzas | sean-k-mooney: I'm very well aware of today's FF :) | 17:26 |
| johnthetubaguy | in perfect timing, I am going to the beach tomorrow... (obvs to celebrate feature freeze with an ice cream) | 17:27 |
| bauzas | melwitt: doh, my fault ! | 17:27 |
| sean-k-mooney | johnthetubaguy: nice | 17:27 |
| sean-k-mooney | johnthetubaguy: enjoy it | 17:27 |
| sean-k-mooney | im also not planning to be here at this time tomrorrow evening | 17:27 |
| * sean-k-mooney normally works later but im not planning to work late teh firday after FF | 17:28 | |
| bauzas | melwitt: 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 patch | 17:28 |
| johnthetubaguy | Thanks for the reviews, I am back Monday if there is stuff to help push it across the line | 17:28 |
| bauzas | apologies, no pun intended | 17:28 |
| melwitt | ah ok | 17:28 |
| bauzas | https://etherpad.opendev.org/p/nova-bobcat-blueprint-status tells me we are mostly done with all the blueprints we were able to review | 17:29 |
| bauzas | except the large manila series and the ephemeral storage | 17:29 |
| bauzas | so, 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-docs | 17:32 |
| bauzas | for me, my day is almost over | 17:32 |
| bauzas | but I can hang for a bit | 17:33 |
| zer0c00l | I 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-mooney | zer0c00l: with what network backend | 17:39 |
| sean-k-mooney | calico? | 17:39 |
| sean-k-mooney | the intree network backends do not create taps nor does os-vif | 17:40 |
| sean-k-mooney | they are only created by libvirt | 17:40 |
| sean-k-mooney | zer0c00l: i am not aware of any supported backend where the tap is precreated | 17:41 |
| sean-k-mooney | so i dont think this will impact nova | 17:41 |
| zer0c00l | sean-k-mooney: yeah we are running calico | 17:41 |
| sean-k-mooney | well even in the calico case libvirt is ment to create the tap | 17:41 |
| sean-k-mooney | calico just manges the routing | 17:42 |
| sean-k-mooney | calico 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-L66C34 | 17:54 |
| JayF | bauzas: 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-mooney | vif type tap is not supported by our os-vif code so its usign the legacy fallback | 17:54 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L595-L596 | 17:55 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L420-L430 | 17:55 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/designer.py#L44-L55 | 17:56 |
| sean-k-mooney | zer0c00l: 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 been | 17:57 |
| zer0c00l | sean-k-mooney: Thanks for looking into this. :( | 17:57 |
| sean-k-mooney | we could proably correct this with a bug fix | 17:58 |
| sean-k-mooney | jsut setting managed='no' | 17:58 |
| sean-k-mooney | here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L427 | 17:58 |
| sean-k-mooney | the problem is that the there is no way to test this really upstream | 17:59 |
| sean-k-mooney | well beyond unit/fucntional tests | 17:59 |
| sean-k-mooney | but we dont have any calico ci | 17:59 |
| sean-k-mooney | calico should be the only backend using vif_type=tap | 18:00 |
| sean-k-mooney | but im not sure if we woudl need a config option in the workarounds section for this or not | 18:00 |
| zer0c00l | sean-k-mooney: yeah, my team member is doing exactly that. | 18:00 |
| sean-k-mooney | im kind of reluctant to set managed='no' because that might eb a security issue | 18: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-mooney | zer0c00l: can you file a bug for this. | 18:02 |
| zer0c00l | sean-k-mooney: i will do that. thank you. | 18:02 |
| sean-k-mooney | johnthetubaguy: JayF https://review.opendev.org/c/openstack/nova/+/886980/7/nova/conf/ironic.py should that really be mutable? | 18:08 |
| JayF | I think that's an astute observation by you, and it's likely better not mutable | 18:09 |
| JayF | sean-k-mooney: I can add a follow up to flip it back if you wanna land that? | 18:09 |
| sean-k-mooney | im not going to hold the series on that but ya a follow up would likely be a good idea | 18:09 |
| JayF | ack, 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 set | 18:09 |
| JayF | but I am making a note in my calendar to check for it tomorrow morning | 18:10 |
| sean-k-mooney | we have 2 +2 on everything now | 18:10 |
| sean-k-mooney | so ill add a +w to that shortly | 18:10 |
| JayF | exactly why I don't wanna risk accidentally re-pushing them :D | 18:10 |
| sean-k-mooney | im just going to review the one patch i did not look at | 18:10 |
| sean-k-mooney | heheh -R is useful to prevent change but ya we can fix this after FF | 18:10 |
| * JayF goes and looks at a git-review doc | 18:11 | |
| JayF | I'd love to learn that trick anyway :D | 18:11 |
| sean-k-mooney | it just prevents it doing any rebases | 18:12 |
| sean-k-mooney | ok 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 patch | 18:14 |
| sean-k-mooney | so the seriese shoudl have +2w all the way | 18:15 |
| sean-k-mooney | melwitt: ill try and find time later tonight to look at https://review.opendev.org/q/topic:bp%252Funified-limits-nova-tool-and-docs | 18:16 |
| melwitt | Uggla: 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-mooney | if other get to it before me however dont wait for my review | 18:16 |
| sean-k-mooney | o/ | 18:16 |
| melwitt | sean-k-mooney: thanks 🙏 seeya later | 18:19 |
| melwitt | Uggla: 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-session | 18:22 |
| zer0c00l | sean-k-mooney: FYI https://bugs.launchpad.net/nova/+bug/2033681 | 18:55 |
| opendevreview | Merged openstack/nova master: Deprecate ironic.peer_list https://review.opendev.org/c/openstack/nova/+/883346 | 19:06 |
| opendevreview | Merged openstack/nova master: Add function to get all attachments in Cinder.API module https://review.opendev.org/c/openstack/nova/+/881457 | 19:06 |
| opendevreview | Merged openstack/nova master: Reproducer for dangling bdms https://review.opendev.org/c/openstack/nova/+/889947 | 19:06 |
| opendevreview | Merged openstack/nova master: Limit nodes by ironic shard key https://review.opendev.org/c/openstack/nova/+/886980 | 21:00 |
| opendevreview | Merged openstack/placement master: Modify the comment that is confused https://review.opendev.org/c/openstack/placement/+/809948 | 21:00 |
| opendevreview | Merged openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | 21:02 |
| opendevreview | Merged openstack/nova master: Update serial console example client for py3 https://review.opendev.org/c/openstack/nova/+/878080 | 21:02 |
| opendevreview | Merged openstack/nova master: Remove unused mocks https://review.opendev.org/c/openstack/nova/+/867789 | 21:02 |
| opendevreview | Merged openstack/nova master: Fix tox docs target https://review.opendev.org/c/openstack/nova/+/891033 | 21:02 |
| sean-k-mooney | melwitt: o/ | 21:48 |
| melwitt | sean-k-mooney: o/ | 21:49 |
| sean-k-mooney | so the way the doc and porting tool are managing vcpu quota is not correct | 21:50 |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/891646/5/nova/cmd/manage.py#3461 | 21:50 |
| sean-k-mooney | but i think we can adress both of those issues in a followup patch | 21:51 |
| sean-k-mooney | when migrating the quota on flavor.vcpu it should not be mapped 1:1 to the vcpu resouce class limit exclively | 21:51 |
| sean-k-mooney | we need to also set the same limit for PCPUs | 21:51 |
| sean-k-mooney | since flavor.vcpu can be either VCPU alloctions , PCPU alloctions or both in the case of a vm with mixed cpus | 21:52 |
| sean-k-mooney | so while that has the downside of effectivly doubling the qutoa for cpus | 21:52 |
| sean-k-mooney | i think its the only way to avoid over quota issues in general | 21:53 |
| melwitt | sean-k-mooney: hm, ok | 21:53 |
| sean-k-mooney | since we have not made the new quota driver the default | 21:54 |
| sean-k-mooney | just deprecated the old one | 21:54 |
| sean-k-mooney | im fine with doing that in a followup | 21:54 |
| melwitt | sean-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_vcpu | 21:54 |
| melwitt | as well | 21:54 |
| sean-k-mooney | oh ya there is/was | 21:54 |
| melwitt | ack. I'm working on follow up stuff | 21:54 |
| sean-k-mooney | i have approved the two patches as they are since for most peopel it will do what they need | 21:55 |
| melwitt | thank you | 21:56 |
| sean-k-mooney | no worreis i came online to do that before i head to bed so ill chat to you soon o/ | 21:57 |
| melwitt | sean-k-mooney: thanks 🙏 ttyl o/ | 21:57 |
| opendevreview | Merged openstack/nova master: nova-manage: Add 'limits migrate_to_unified_limits' https://review.opendev.org/c/openstack/nova/+/891646 | 23:54 |
| opendevreview | Merged openstack/nova master: Add documentation for unified limits https://review.opendev.org/c/openstack/nova/+/893127 | 23:54 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!