Thursday, 2021-10-07

opendevreviewJinsheng Zhang proposed openstack/nova master: Nova support live-migrate instance between different ipxe efi rom  https://review.opendev.org/c/openstack/nova/+/81279606:26
gibio/08:00
gibibauzas: do we have a meeting this morning?08:03
bauzasdang08:03
bauzasgibi: remind me the meeting name ?08:03
gibinova_extra08:04
gibiI always have to look that up from https://meetings.opendev.org/#Nova_Monthly_Asia_Friendly_Team_Meeting08:04
bauzas#startmeeting nova_extra08:04
opendevmeetMeeting started Thu Oct  7 08:04:33 2021 UTC and is due to finish in 60 minutes.  The chair is bauzas. Information about MeetBot at http://wiki.debian.org/MeetBot.08:04
opendevmeetUseful Commands: #action #agreed #help #info #idea #link #topic #startvote.08:04
opendevmeetThe meeting name has been set to 'nova_extra'08:04
bauzasgood moning folks, I have forgotten to open it :)08:04
bauzassorry for having it delayed08:04
bauzasI don't have agenda for this meeting08:05
bauzasit's more something like "if you want to discuss with us, let us know"08:05
bauzasdo we have Asian contributors here ?08:05
bauzasor any other folks wanting to discuss ?08:06
gibiI can rant about the rainy weather 08:07
gibi;)08:07
sean-k-mooney[m]just looking at the patch submitted above by jingsheng zhang https://review.opendev.org/c/openstack/nova/+/81279608:08
sean-k-mooney[m]is that something we even support08:08
sean-k-mooney[m]ipxe boot of nova vms08:09
bauzasgibi: hah08:09
bauzaswe have a good weather here :)08:09
sean-k-mooney[m]i dont know if they are here and could explain there motivation for that08:09
bauzassean-k-mooney: good question08:09
* bauzas looks at the change08:09
sean-k-mooney[m]my understandign is the only way to use ipxe is to enable the qemu boot menu08:10
sean-k-mooney[m]and manually select it08:10
bauzasI'm not really sure it's a nova question08:10
sean-k-mooney[m]also this feels like a spec not a bug since it has  object changes08:10
bauzasmaybe more an image 08:10
bauzasI mean, if you want to have instances calling PXE, then you need to have an image booting by it08:11
sean-k-mooney[m]not really08:11
sean-k-mooney[m]that is provided by the qemu seabios image on the host08:12
bauzassean-k-mooney: but agreed, this isn't a bug, more a new feature08:12
sean-k-mooney[m]its not provide by the glance image08:12
gibiI agree that it feels like a feature08:12
sean-k-mooney[m]we dont officaly support ipxe boot which is why i raised it08:12
gibiwhen I see ipxe boot with nova that was with a special image booted08:12
bauzassean-k-mooney: oh, you mean that the host needs to support iPXE ?08:12
sean-k-mooney[m]gibi you can use an ipxe image form glance08:13
sean-k-mooney[m]but seabios i belive support ipex/network boot too08:13
bauzasis this about having guests calling a pxe server or having a way to have a host supporting pxe ?08:13
gibisean-k-mooney[m]: I see so there is multiple ways08:14
sean-k-mooney[m]its about the guest pxe booting and what happens if you live migrate that guest between hosts08:14
sean-k-mooney[m]if the image was from glance then presumable it would not change between hosts when you live migrate08:15
sean-k-mooney[m]which is why im assumje this is related to teh seabios image form the host08:15
sean-k-mooney[m]it may not be however08:15
sean-k-mooney[m]https://bugs.launchpad.net/nova/+bug/194629808:16
sean-k-mooney[m]this is the relevent bug report08:17
sean-k-mooney[m]anyway it looks like they are not present to talk about it in the meeting08:17
bauzasyeah08:17
sean-k-mooney[m]so we can disucss it later08:17
bauzassean-k-mooney: you should review the change and tell me to at least create a blueprint 08:17
bauzasto exactly explain their needs08:17
bauzasit looks to me we never supported ipxe booting with libvirt08:18
bauzaslibvirt had the knob, but we never exposed it on nova08:19
bauzasso this is fortunate to work, but indeed for move operations, you have to do extra things08:19
sean-k-mooney[m]correct we dont offically support it08:19
sean-k-mooney[m]looking at the bug report then issue is related to the option rom of the network interface which can have different size on diferent hosts08:20
sean-k-mooney[m]but i dd not think we supported enabling that either08:20
gibiI agree with generic sentiment above, so this is a bp at least but most probably a spec08:20
bauzasthat's probably a spec indeed, but let's just ask for the blueprint first08:21
bauzasjust to understand the needs08:21
bauzassean-k-mooney: I can triage the bug as Invalid/Wishlist and explain them to open a blueprint08:22
bauzasagreed ?08:22
sean-k-mooney[m]sure08:22
gibiagreed08:23
bauzasok, anything else to discuss ?08:24
bauzasI'll put a PTG session proposal to discuss the opportunity of the Asian timeslot08:24
bauzasI don't exactly want to kill it, but I wonder how to make it productive if we keep it08:25
bauzasmeetings for the sake of meeting seems a bit irrelevant08:25
gibibauzas: I had the meeting mentioned in the retro part of the etherpad08:25
bauzascool 08:25
bauzasthanks08:25
bauzasit's more or less the question to reach the contributors who don't usually get into IRC 08:26
gibiL11908:26
bauzasat least, if we continue, I'll officially name "office hour"08:26
bauzasthis sounds more reasonable 08:27
bauzasas we don't really have an agenda08:27
bauzasok, anything to add before I close ?08:27
gibi-08:29
bauzas#endmeeting08:32
opendevmeetMeeting ended Thu Oct  7 08:32:13 2021 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)08:32
opendevmeetMinutes:        https://meetings.opendev.org/meetings/nova_extra/2021/nova_extra.2021-10-07-08.04.html08:32
opendevmeetMinutes (text): https://meetings.opendev.org/meetings/nova_extra/2021/nova_extra.2021-10-07-08.04.txt08:32
opendevmeetLog:            https://meetings.opendev.org/meetings/nova_extra/2021/nova_extra.2021-10-07-08.04.log.html08:32
* bauzas can now go to the coffee machine08:32
opendevreviewJinsheng Zhang proposed openstack/nova master: Nova support live-migrate instance between different ipxe efi rom  https://review.opendev.org/c/openstack/nova/+/81279608:39
lyarwoodgibi:  https://bugs.launchpad.net/nova/+bug/1946339 you might be interested in this08:50
gibilooking08:54
gibilyarwood: thanks,yes, that is one of the recently added test case 08:56
gibilyarwood: is it high frequency?08:56
opendevreviewDmitriy Rabotyagov proposed openstack/nova master: Ensure MAC addresses characters are in the same case  https://review.opendev.org/c/openstack/nova/+/81194709:06
lyarwoodgibi: sorry not looked at the frequency, just saw one hit and raised the bug09:17
gibilyarwood: ack, I will check it09:17
gibiI need to juggle multiple things right now downstream so it will take a bit of time to get to it09:18
lyarwoodack np09:20
stephenfinnow that we've forked, any chance we could get these in nice and early? https://review.opendev.org/c/openstack/nova/+/81029109:42
gibistephenfin: I want them, I just need to find the time to review them :/09:44
gibi(there is an ongoing discussion to upgrade a mitaka cloud to victoria downstream in a single step and that discussion is killing meg right now)09:45
opendevreviewLee Yarwood proposed openstack/nova master: compute: Update volume_id within connection_info during swap_volume  https://review.opendev.org/c/openstack/nova/+/80702509:46
opendevreviewLee Yarwood proposed openstack/nova master: fup: Move _wait_for_volume_{attach,detach} to os-volume_attachments  https://review.opendev.org/c/openstack/nova/+/81077509:46
opendevreviewLee Yarwood proposed openstack/nova master: fup: Refactor and simplify Cinder fixture GET volume mock  https://review.opendev.org/c/openstack/nova/+/81077609:46
opendevreviewDmitriy Rabotyagov proposed openstack/nova master: Ensure MAC addresses characters are in the same case  https://review.opendev.org/c/openstack/nova/+/81194710:11
opendevreviewLee Yarwood proposed openstack/nova master: zuul: Move live migration jobs back to voting  https://review.opendev.org/c/openstack/nova/+/81239210:22
opendevreviewLee Yarwood proposed openstack/nova master: Revert "zuul: Skip block migration with attached volumes tests due to bug #1931702"  https://review.opendev.org/c/openstack/nova/+/81247310:22
lyarwood^ also needs the stable/xena backport of my devstack fix to allow grenade to work /o\10:22
opendevreviewsean mooney proposed openstack/nova master: Add autopep8 to tox and pre-commit  https://review.opendev.org/c/openstack/nova/+/80618210:42
sean-k-mooneyi hope people will be open to ^10:42
sean-k-mooneywell this is new "flake8.exceptions.PluginRequestedUnknownParameters: "pycodestyle" requested unknown parameters causing 'FileProcessor' object has no attribute 'indent_size'"10:57
sean-k-mooneyah ok that  with my patch im not capping the deps when installing autopep810:59
sean-k-mooneyhum no thats not the issue11:01
noonedeadpunksean-k-mooney: hey! regarding https://review.opendev.org/c/openstack/nova/+/811947 - setting lower would require adjusting a lot of existing unit tests because all of them have assumption about mac being in upper case11:17
noonedeadpunkHaving that in mind - should I still convert it to lower()?11:17
noonedeadpunkBecause what I didn't want to do - to change tests to fit my change :)11:18
noonedeadpunkBut if you think it's fine - no problems11:18
sean-k-mooneyi kind of dont like sprinkeling random upper and lower cases converstion in the code in general and would prefer to have normalise utility function that is used consitently   11:20
sean-k-mooneyi would prefer to have it normalise to lower case but i think your bug is just a sincel case of thsi11:21
sean-k-mooneyi think there could be other places that woudl still break11:21
sean-k-mooneynoonedeadpunk: with that said i also think this is a neuton bug11:24
sean-k-mooneyi think neutron should be normalising this internally themselves11:24
noonedeadpunkWell... Maybe it should, but, we take xml from libvirt and from neutron api. So these are 2 different sources of truth nova takes info from. And current assumption that they shoudl be equal11:26
sean-k-mooneycorrect 11:27
sean-k-mooneya mac is justa 48bit int11:27
noonedeadpunkAlso, that patch is pretty much simple and backportable11:27
sean-k-mooneyits not a strirng so we shoudl not be doing a case sensitive comparison11:27
noonedeadpunkWhile fixing neutron will require db migrations11:27
sean-k-mooneythat or a conversion on load and normalisation in store11:28
sean-k-mooneybut ya we can normalise in nova but i think neutron should also fix this themseleves11:28
noonedeadpunkI don't disagree here :)11:29
sean-k-mooneyat the end of the day it does not really matter which way we go but lower case is my prefernce we can see what other think in general your patch looks fine11:30
noonedeadpunkyeah, it was mine as well:) but I'd need to fix all other unit tests if change that to lower case11:31
sean-k-mooneythis is really an internal fix in the nova side so i dont think you need a release note but that is the only other thing i was thinking of11:31
sean-k-mooneynoonedeadpunk: yep i done see any issue with fixing the unit tests other then time11:31
sean-k-mooneyi would guess there is a mix of case already11:32
sean-k-mooneyyep https://github.com/openstack/nova/blob/8d9785b965657d42f20e1ad7234f570077a387d7/nova/tests/unit/virt/libvirt/test_vif.py#L8111:32
noonedeadpunkwell, I was also unsure if I haven't break anything else accidentally:) so with current change log I was more sure about zero impact on current functionality11:33
sean-k-mooneyim prettty sure i have always used lower case in our tests11:33
noonedeadpunkah, well, so it's more test_migration thing I guess11:34
sean-k-mooneywhat im more concerneed about is https://github.com/openstack/nova/blob/66574018b517f14dc26e581d0ddaa7788806f83e/nova/objects/virtual_interface.py#L4411:36
sean-k-mooneyand other place we store the adress in our code11:36
sean-k-mooneyi had alway assumed that neutron was already normalising11:36
* bauzas disappears until 1300UTC (dentist appointment)11:37
sean-k-mooneynoonedeadpunk: when we lookup pci device mac adress we do not normalise there currently either 11:39
sean-k-mooneyhttps://github.com/openstack/nova/blob/66574018b517f14dc26e581d0ddaa7788806f83e/nova/pci/utils.py#L173-L19111:39
sean-k-mooneywe just take the value form /sys which should be lower case11:39
sean-k-mooneysean@p50:~/repos/nova$ cat /sys/class/net/enp0s31f6/address 11:40
sean-k-mooney8c:16:45:81:48:b511:40
sean-k-mooneyfor the unique constraint in the db for the virtual interfaces table we are also useing the mac un normalised 11:41
sean-k-mooneyhttps://github.com/openstack/nova/blob/66574018b517f14dc26e581d0ddaa7788806f83e/nova/network/neutron.py#L1307-L130811:41
sean-k-mooneynoonedeadpunk: likely we should just fix your inital case in this patch but we should think about the other cases at some point11:42
noonedeadpunkwell neutron is not normalazing https://paste.opendev.org/show/809839/11:43
sean-k-mooneyyep that would appear to be the case11:43
sean-k-mooneywhcih mean sif you were ever to change the mac on the neutron port it could break things in nova unless we propagate that update in some cases11:44
noonedeadpunkactually I thought that in domain xml mac is also stored non-normalized11:44
noonedeadpunkAnd I saw that issue first when was live-migrating instances from bionic to focal computes11:45
sean-k-mooneylibvirt normalises the xml we provide so the one we provide is not the one that it actully uses11:45
sean-k-mooneyso we likely dont normalise it when we give it to libvirt11:45
noonedeadpunkwas it always like that ?11:45
sean-k-mooneyyep11:46
noonedeadpunkhuh, okay then. Wondering why it's haven't been spotted before then...11:46
sean-k-mooneythe xml you see with virsh dumpxml is not the one that nova provided11:46
sean-k-mooneywell neutron default to lower case11:46
noonedeadpunkYeah, that's what I saw as well11:46
sean-k-mooneyso unless you provdided a mac it wont be an issue11:46
noonedeadpunkyeah, maybe we saw that because we were doing really mass migrations of all vms in our deployments...11:47
sean-k-mooneyso i think this is only an issue for user provided macs11:47
noonedeadpunkyeah, indeed11:47
noonedeadpunkOk, so then I will change that to lower()11:47
sean-k-mooneyif the test fallout of that is no horrific then yes please do 11:48
sean-k-mooneyi suspect it will not be that bad11:49
opendevreviewDmitriy Rabotyagov proposed openstack/nova master: Ensure MAC addresses characters are in the same case  https://review.opendev.org/c/openstack/nova/+/81194712:22
noonedeadpunkyeah you was right, it's not bad at all)12:22
* bauzas 's back12:42
opendevreviewDmitriy Rabotyagov proposed openstack/nova master: Ensure MAC addresses characters are in the same case  https://review.opendev.org/c/openstack/nova/+/81194713:48
bauzasgibi: sean-k-mooney: shit, I remember we need to move the merged specs into implemented https://specs.openstack.org/openstack/nova-specs/specs/xena/15:08
gibibauzas: yeah, we tend to forget that15:09
* gibi just back from the openinfra live event15:09
bauzasgibi: I can do it15:09
gibithere is script for that15:09
gibithans15:09
gibithanks15:09
lyarwoodbauzas: https://review.opendev.org/c/x/gerrit-dash-creator/+/81305016:21
bauzasgibi: oh, I wasn't knowing16:25
melwittbauzas: there's some instructions here too https://docs.openstack.org/nova/latest/contributor/ptl-guide.html16:27
bauzasmelwitt: /me facepalms16:28
lyarwoodhttps://review.opendev.org/c/openstack/nova/+/812392 - bauzas / gibi / melwitt ; this should be good now the stable/xena devstack change is in the gate16:29
bauzasI have this link open as a tab since a month and I forgot about this16:29
melwitt:)16:29
*** ralonsoh_ is now known as ralonsoh16:35
opendevreviewSylvain Bauza proposed openstack/nova-specs master: Move Xena implemented specs  https://review.opendev.org/c/openstack/nova-specs/+/81305116:44
bauzasgibi: melwitt: there there ^16:45
* bauzas ends on this for the day16:52
melwittbauzas: I just remembered you'll want to do the placement repo too. have to do it manually afaik, placement doesn't use launchpad https://docs.openstack.org/placement/latest/specs/index.html#xena they are located under placement/doc/source/specs/16:53
melwittgmann: in case you didn't see my comment on https://review.opendev.org/c/openstack/devstack/+/812092 I think stable/train is also broken in the same way cc elodilles 16:56
gmannmelwitt: checking, stable/train still run tempest master so it should be ok.16:57
melwittgmann: oh, so my patch has jsonschema fail bc of the stable/stein side of grenade /facepalm  ok I understand now16:59
gmannmelwitt: yeah, once stable/stein patch is merged then we can recheck or, we can make depends-on just to be sure16:59
melwittyeah I see. I'm writing up a comment about the novnc issue. I don't understand why it's happening though so can't fix it yet :(17:00
gmannthanks, I will try to dig into that next week or so.17:01
elodillesmelwitt: thanks for the heads up, i will also add this to my TODO list and try to debug it :S17:03
melwittelodilles: to correct myself it's stable/stein that needs a fix but it's failing for a novnc issue that I don't understand how is happening. I'm writing a comment for gmann's patch ^ with what I found so far17:04
elodillesmelwitt: ack, thanks!17:13
melwittgmann: just a fyi, I think I finally figured out why the tempest 20.0.0 patch failed on stable/stein https://review.opendev.org/c/openstack/devstack/+/812092/1#message-348d913f2bbcf909533d62c3cb4e02d6a0100664 cc elodilles 18:54
gmannmelwitt: ah thanks, let me try 24.0.0 which should work as it has old jsonschema in requirement filr https://github.com/openstack/tempest/blob/24.0.0/requirements.txt#L622:21
gmannoh, you already updated, great22:22
melwittgmann: I already updated your patch, I hope you don't mind. I did not update the commit message or code comment yet as it's just for testing. if it works well, we can update the writings22:22
gmannmelwitt: +1.22:23
opendevreviewMerged openstack/nova master: zuul: Move live migration jobs back to voting  https://review.opendev.org/c/openstack/nova/+/81239222:27
melwittgmann: the novnc problem went away but we have a new problem now on the minimum basic scenario test image create: FileNotFoundError: [Errno 2] No such file or directory: '/opt/stack/new/devstack/files/images/cirros-0.3.1-x86_64-uec/cirros-0.3.1-x86_64-vmlinuz'22:37
gmannchecking22:37
gmannmay be that is because we change the configuration of image location in tempest and devstack for tempest 25.0.0, let me check the patch which we can revert.22:38
gmannmelwitt: this one. we did backport it to stable/stein as tempest 26.0.0 was used there and img location changes done in tempest 25.0.0 but now as we have to use tempest 24.0.0 in stable/stein we should revert it https://review.opendev.org/c/openstack/devstack/+/77083922:40
melwittoh I see22:41
gmannlet me do revert and squash the both22:42
melwittkk22:43
opendevreviewmelanie witt proposed openstack/nova master: Make the CellDatabases fixture work with fasteners >= 0.15  https://review.opendev.org/c/openstack/nova/+/81311423:53

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