Monday, 2022-01-17

gmannsean-k-mooney: no, neutron is not forcing scope. it is only warning in logs when enforce_scope is disabled. if enable the it fail with 403 not warning. that is from oslo side.00:52
opendevreviewGhanshyam proposed openstack/nova master: Test PROJECT_ADMIN APIs with no legacy rule case  https://review.opendev.org/c/openstack/nova/+/82484501:33
opendevreviewGhanshyam proposed openstack/nova master: Move rule_if_system() method to base test class  https://review.opendev.org/c/openstack/nova/+/82447502:06
opendevreviewGhanshyam proposed openstack/nova master: Convert SYSTEM_ADMIN|READER to Admin and system scope  https://review.opendev.org/c/openstack/nova/+/81939002:06
opendevreviewGhanshyam proposed openstack/nova master: Server actions APIs scoped to project scope  https://review.opendev.org/c/openstack/nova/+/82435802:06
opendevreviewGhanshyam proposed openstack/nova master: Server actions APIs scoped to project scope  https://review.opendev.org/c/openstack/nova/+/82435802:09
*** EugenMayer1 is now known as EugenMayer03:02
*** tbachman_ is now known as tbachman04:18
opendevreviewMerged openstack/nova master: Fill the AcceleratorRequestBindingFailed exception msg info  https://review.opendev.org/c/openstack/nova/+/81732604:25
tkajinamhello. I'd appreciate your attention to https://review.opendev.org/c/openstack/nova/+/824830 . we need this to fix package dependency in rdo, which is currently blocking CI jobs in some puppet module repos07:49
bauzastkajinam: ack, will look08:03
tkajinambauzas, thank you !08:06
*** akekane_ is now known as abhishekk08:36
bauzasgibi: other cores, 'd appreciate a quick +W on https://review.opendev.org/c/openstack/nova/+/824830 which fixes a critical bug08:51
bauzasstephenfin: also, if you have time08:52
bauzasmelwitt: stephenfin: looks like we had a regression with https://review.opendev.org/c/openstack/nova/+/82428008:52
gibibauzas: I've approved09:26
bauzasta09:27
bauzastkajinam: ^09:27
gibibauzas: but I would like to do a follow up to clean things up09:27
gibias the dependency should be really just in the test09:27
bauzasgibi: I have zero context about the original patch09:27
bauzasbut I saw we add fixtures for monkeypatching eventlet, right?09:27
gibiright09:27
bauzasas we pull nova.utils now, we also pull fixtures09:28
gibiit is used for the fasteners rw lock09:28
bauzasthat's what I saw09:28
gibibut that is only used in test09:28
gibiso the rw lock wrapper can be moved back to the test code tree09:29
gibior at least that is my working assumption now09:29
tkajinambauzas, gibi hmmm... sorry do you mind blocking https://review.opendev.org/c/openstack/nova/+/824830 by -2 or -Workflow ?09:32
tkajinamI'll look into that cleaner approach. we need to update requirements in rdo but it doesn't make much sense if we revert it very soon09:32
bauzastkajinam: no I guess we can move on09:32
bauzasor revert if needed09:33
tkajinamok09:33
bauzasgibi: do you think you could do something before we merge https://review.opendev.org/c/openstack/nova/+/824830 ?09:33
bauzasI'd personnally let the fix be merged09:34
bauzasI've tagged the bug as critical as it's holding other projects09:34
bauzasand once the gate is back happy, we can sort the things up and revert what we need09:34
bauzasbut without any rush09:34
bauzasagreed ?09:35
gibilet land the fix09:35
tkajinamack. thank you for your time and thoughts, bauzas and gibi !09:36
gibitkajinam: thank you for proposing the fix09:40
* gibi is disctracted again with meetings :/09:45
opendevreviewMerged openstack/nova master: Close Glance image if downloading failed.  https://review.opendev.org/c/openstack/nova/+/81534710:31
opendevreviewMerged openstack/nova master: Add fixtures to requirements  https://review.opendev.org/c/openstack/nova/+/82483011:14
sean-k-mooneyfixtures should be in test-requirements not requirements but otherwise adding it makes sense11:35
sean-k-mooneyoh it was moved that is not correct11:35
sean-k-mooneyis see how its used https://review.opendev.org/c/openstack/nova/+/824280/1/nova/utils.py and where the dep comes form11:36
sean-k-mooneybtu we should not use fixtures for that11:37
sean-k-mooneyis there a patch up to remove fixutres form nova.utils. you can do exactly the same with unittest.mock11:40
gibisean-k-mooney: the RW lock is only used in our test code so we move the RW wrapper in our test path and move back the fixtures deps to test-reqs11:40
gibiI'm about to push a patch for it11:40
sean-k-mooneyok11:41
sean-k-mooneyis this a differnt RWlock then the tempest one11:41
sean-k-mooneyim not sure why we woudl need it in nova code11:41
sean-k-mooneyas in i dont think we need the interprocess one11:42
gibisean-k-mooney: hehe, it is the same RW lock11:42
gibihm, no it is not inter process11:42
sean-k-mooneyok just a normal rwlock11:43
gibiyepp11:43
sean-k-mooneyok that makes more sense to me i was like why have we suddenly got multiple processes in our func/unit tests :)11:44
gibi:)11:44
opendevreviewBalazs Gibizer proposed openstack/nova master: Move ReaderWriterLock to the test tree  https://review.opendev.org/c/openstack/nova/+/82493112:35
gibisean-k-mooney, bauzas, melwitt: ^^12:39
bauzasgibi: +2s12:51
sean-k-mooney+1 from me also this is a better way to fix the orginal bug12:53
*** dasm|off is now known as dasm13:13
sean-k-mooneynova/healthcheck/manager.py:48:62: N310: timeutils.utcnow() must be used instead of datetime.now()13:50
sean-k-mooneythat hacking check shoudl proably be fixed to point out it means timeutils form oslo_utils13:50
sean-k-mooneynot the timeutils package13:50
gibisean-k-mooney: good point, it is probably an easy fix13:57
sean-k-mooneyi assume that is commign from hacking based on the N prefix13:58
sean-k-mooneybut i have nto looked is that a nova hackign check or a global one13:59
sean-k-mooneynova 13:59
sean-k-mooneyhttps://github.com/openstack/nova/blob/master/nova/hacking/checks.py#L209-L22113:59
sean-k-mooney so ya trivial fix14:00
gibiN means Nova :)14:00
gibiafaik14:00
sean-k-mooneyH is for genereal hacking14:00
gibiwondering what letter heat and horizon uses :)14:00
sean-k-mooneywell assuming they use hacking14:01
sean-k-mooneymybe l for local14:01
gibiheat uses Heat304 for example, clever, it does not need to be a single letter14:04
gibihorizon uses M14:04
gibisean-k-mooney, bauzas: btw, can I get eyes on this bugfix https://review.opendev.org/q/topic:bug/1952941 I'm still on the hook to backport this to victoria due to downstream upgrade failures.14:07
sean-k-mooneyah that one sure14:09
gibithanks14:15
sean-k-mooneyim +1 on that and the repoducer below it, thanks for following that pattern it makes it supper clear that the fix is correct14:16
sean-k-mooneyyou have a seperate change too  to stop persiting the numa toloplogy in the request spec correct?14:17
sean-k-mooneyhttps://review.opendev.org/c/openstack/nova/+/82021514:17
sean-k-mooneyafter the current backportable change is done i woudl still be happy to proceed with that too14:18
gibisean-k-mooney: yeah, I'm happy to move forward with https://review.opendev.org/c/openstack/nova/+/820215 if there is a consensus14:20
gibithe current state of that patch already show that the idea is feasible14:20
sean-k-mooneyya its not required but since it will prevent this type of error in the future its proably worth it in the long run14:21
bauzasgibi: ack, sorry was on meeting15:01
bauzasgibi: will look at it later today15:01
gibibauzas: thanks, no worries15:01
* bauzas was explaining the ugly sphagetti code from unshelve to Uggla :p15:01
bauzasgibi: gmann: btw. if we need to update a microversion for unshelve (adding a new host), are we OK if we could remove https://github.com/openstack/nova/blob/1ddb8f83adef964a8ca050994a43adc6175994f1/nova/api/openstack/compute/schemas/shelve.py#L31 ?15:02
bauzasavailability_zone is Optional in the doc :)15:03
artomYeah, it's a weird one15:18
gibibauzas: looking15:19
artomAZ isn't actually optional, the entire request body is15:19
sean-k-mooneywell the az is optional15:19
artomIf you have a body, AZ needs to be *something*, could be None15:19
bauzasyou need to either provide no dict or if you provide the dict, you need to add the AZ key15:19
artom... which could be None :P15:19
* artom thinks we should use Uggla's unshelve to specific host RFE microversion to fix that15:19
bauzasso, when adding a new argument like "host", we would also need to add AZ15:20
sean-k-mooney yep but there is no other property you can currently pass in the dict15:20
bauzasyeah, that's why I'm asking to remove this for the new API microversion also adding the new argument15:20
bauzassean-k-mooney: correct, for the moment15:20
sean-k-mooneyartom it would have to be the litral value null potentailly but this is not the only place where its slightly odd15:20
gibiI think both unshelve to an AZ or unshelve to a specific host make sense15:20
bauzassean-k-mooney: but once Uggla will add a new key like "host", that would mean that the AZ key should be required15:21
sean-k-mooneyyep likely you dont need to sepcify both but if you did we shoudl vlaidated the host to az relationship15:21
artomNo...? Having to specify both host and AZ seems weird15:21
artomI'd say one or the other, but not both?15:21
bauzasthat's why I'm saying we should remove the requiring when we modify this API15:21
bauzasin a new microversion of course15:22
sean-k-mooneybauzas: well as currently specified but in generally unshele:{"host":"my-host"}15:22
sean-k-mooneyi think shoudl be valid15:22
artomTo my mind what would be clearest is: either empty body, or AZ with a non-null value, or host with a non-null value15:22
bauzasanyway, let's wait Uggla to provide his spec for Z :)15:22
sean-k-mooneyi dont think we shoudl required Az if the host is passed15:22
gibiartom: ++15:22
bauzassean-k-mooney: agreed, that's my concern15:22
bauzassean-k-mooney: https://github.com/openstack/nova/blob/1ddb8f83adef964a8ca050994a43adc6175994f1/nova/api/openstack/compute/schemas/shelve.py#L31 doesn't accept it for the moment15:22
bauzasso we need to remove this line15:23
bauzaswith a new microversion and when adding a new argument15:23
sean-k-mooneyyes in the new microverion15:23
sean-k-mooneyin the current  one it shoudl remain15:23
bauzasok, anyway, I think we have a consensus15:23
bauzassean-k-mooney: of course, I know it15:23
sean-k-mooneyyep just make sure Uggla captures it in the spec for next cycle15:24
bauzasI was asking whether it was ok to remove the line at the same time we add a new argument15:24
sean-k-mooneyi assume they will start working on the implemantion in parallel15:24
bauzassean-k-mooney: correct, I discussed that with him :)15:24
bauzasanyway, consensus, that's it15:24
bauzasit was just a question15:24
bauzasa simple one15:24
* bauzas goes off for getting his daughter from the school15:25
sean-k-mooneyyep i broght that up as we have not created the new spec dir yet but Uggla can totally do that and we can review the spec15:25
Ugglacool, I'll work on this way.15:26
sean-k-mooneybut ya my vote is for make it not required and supprot null as well15:26
Ugglapassing both option, do you think that passing host should "discard" az15:30
Ugglapassing both options, do you think that passing host should "discard" az15:30
UgglaI mean whatever we will have in az will be ignored if host is passed.15:31
UgglaI mean whatever we will have in az will be ignored if host is passed ?15:31
gibiUggla: no, I think we should not ignore input. we either forbid both to be passed at once, or we should validate that the host is in the az15:32
gibiI more with artom, to forbid to pass both in the same request15:32
sean-k-mooneywell either we validate that the host is in the az or it should be an error to pass both in my view15:33
gibisean-k-mooney: I agree15:33
Ugglaok probably simpler to make it an error.15:34
sean-k-mooneyyep simple 40015:34
sean-k-mooneyno existing client will pass host15:34
sean-k-mooneyand when optinign into the new microversion they can ensure that only one is generated15:34
UgglaIs it possible to do this kind of exclusive or parameters in the schema validation ?15:35
sean-k-mooneyyes15:35
artomRight, but why do more work when less work is enough? :)15:36
sean-k-mooneyim pretty sure there is a way to defeint them as a mutualy exclucive group15:37
sean-k-mooneyi just cant think of a place off the top of my head whwere we actully do that15:37
sean-k-mooneyi suspect this si normally check after the scheme validation15:37
UgglaIs making both options ok in schema validation and then doing the exclusive part in the code, sounds ok ?15:39
sean-k-mooneyhttps://github.com/openstack/nova/blob/1ddb8f83adef964a8ca050994a43adc6175994f1/nova/api/openstack/compute/schemas/servers.py#L172-L17315:39
sean-k-mooneyoneOf15:39
Ugglasean-k-mooney, nice thank you !15:40
sean-k-mooneyas i said we rearly enforce that in teh scheme but no one will object if you do it either15:40
sean-k-mooneythere are some other exampels in the samee file 15:41
sean-k-mooneyhttps://github.com/openstack/nova/blob/1ddb8f83adef964a8ca050994a43adc6175994f1/nova/api/openstack/compute/schemas/servers.py#L281-L29815:41
bauzassean-k-mooney: I'll create the specs directory once we know the Z name :)15:42
bauzasunless I name it 'zombie' directly :p15:43
sean-k-mooneyoh ya good point15:45
sean-k-mooneyi still quite like "zenith" cause its all down hill from there15:46
gmannbauzas: gibi sean-k-mooney Uggla artom yeah, we can remove the AZ from being 'required' from schema now as it will ne extended to accepting 'host' too. AZ was added as 'required' because this API only accept None or AZ and no other field but that is handled by additionalProperties=False so 'required' is not needed as such. 15:53
gmannbauzas: I think you will be right about 'zombie' :) everyone liking it :)15:54
sean-k-mooneyif its is zombie we shoudl make it a base for a downstream realse15:56
sean-k-mooneybecause a decade form now we will still have once custoemr that refuses to move off it and it will never die15:57
sean-k-mooneylike any good zombie :)15:57
gmann:)15:58
sean-k-mooneyosp 18 actuly should either be the z release of a release depending on timeing so maybe that will happen anyway15:59
sean-k-mooney*Z release or A release16:00
bauzasA = Awesome release I guess16:00
bauzaseasy peasy16:01
sean-k-mooneyB = Buffy to help with the zombie problem ?16:01
bauzasC = Chilly16:02
bauzasD = Draughful16:02
UgglaI like B especially if Z is zombie !16:03
UgglaSarah Michelle Gellar was so cute.16:14
opendevreviewMerged openstack/nova master: ensure samples folder exists for microversion  https://review.opendev.org/c/openstack/nova/+/81367216:15
opendevreviewMerged openstack/nova master: libvirt: Ensure all volume drivers log the instance whenever possible  https://review.opendev.org/c/openstack/nova/+/78026017:39
opendevreviewMerged openstack/nova master: functional: Add reproducer for #1907775  https://review.opendev.org/c/openstack/nova/+/76677118:17
ade_leesean-k-mooney, lyarwood hey guys, I updated the fips job to include the full set of tempest tests and there were quite a few more failures -- can you guys take a look?19:44
ade_leehttps://review.opendev.org/c/openstack/nova/+/79051919:44
ade_leecinder folks are looking into other potential problems with volumes / cryptsetup here -- https://review.opendev.org/c/openstack/cinder/+/790535  so some of the failures may be related to that.19:45
sean-k-mooneysure19:46
sean-k-mooneyhave the general centos8 gate issue been fixed19:47
sean-k-mooneyi.e. the fact they broke ping19:47
sean-k-mooneymaking it require root to run19:47
ade_leesean-k-mooney, not sure .. that woud certainly muddy things19:47
ade_leefungi, clarkb ^^ any idea?19:48
fungisean-k-mooney: ade_lee: i don't know if red hat has fixed centos stream 8's packages yet19:49
sean-k-mooneythe fips failrues might be unrealtaed but that is breaking all centos8 stream jobs currently right19:49
fungitechnically, centos stream 8 was (maybe still is) broken/regressed. i recommend not relying on it since it doesn't seem like they actually test it like they did the old centos19:49
sean-k-mooneyfungi: for what its worth i agree with not working aroudn this in our jobs19:50
fungii think there's work underway to get rocky linux added, and we have openeuler though it sounds like it may have a slightly too old kernel for the fips testing19:50
*** artom__ is now known as artom19:50
sean-k-mooneyfungi: really centos 8 stream should be fixed and maintianed as if it was rhel19:50
sean-k-mooneyade_lee:  qemu-kvm: -accel tcg: mprotect of jit buffer: Cannot allocate memory19:51
fungirocky or alma seems like it might be a better option if centos stream is going to be the unstable hopper where free users get to find the bugs and tell red hat what to fix before they tag a new release of their commercial product19:51
sean-k-mooneyfungi: as someone who has to maintain said product if they dont keep centos stream stable i will continue to do all my dev on ubuntu19:52
sean-k-mooneyi really dont like how closely tied our openstack product is to rhel or the rhel lifecycle today. if it is to contibute more value then the bruden it brings they need to keep centos 8 stream stable or make licenses avaiabel for opensrouce to sue for ci on rhel itself19:54
fungibut yeah, maybe this is a one-off, or maybe it'll be the kick needed to get more thorough testing in place for centos stream package updates19:54
sean-k-mooneyif they dont using rocky or alma and not support centos makes complete sense19:54
sean-k-mooneyfungi: we have a log runnign downstream backlog itme to get a centos stream devstack jobs running in our donwstream ci at somepoint but making it work with our patched repos is non tivial19:55
ade_leesean-k-mooney, so -- the mprotect thing -- what does that mean?19:56
sean-k-mooneyade_lee: it looks like the host vm ran out of memory19:56
ade_leesean-k-mooney, so we should be testing using larger vms?  I think I recall folks having to do that somewhere -- checking ..19:57
sean-k-mooneyade_lee: no you need to reduce the concurancy19:58
sean-k-mooneyade_lee: how have you defiend the regex19:58
fungiunless you can get by with smaller flavors for the cirros guests or something19:58
sean-k-mooneythe senario test need to be run serially after the other test19:58
sean-k-mooneyfungi: the vms are using 128mb flavors19:59
sean-k-mooneythat should be ok 19:59
sean-k-mooneyi wonder if the host vm is missing the extended swap 19:59
sean-k-mooneythat we are using to workaround the 1G tcg buffer20:00
ade_leesean-k-mooney, the cinder tests are running with swap .. https://review.opendev.org/c/openstack/cinder/+/790535/24/.zuul.yaml20:00
ade_leeconfigure_swap_size: 409620:00
sean-k-mooneythat proably needs to be increased20:00
sean-k-mooneyfungi: do you know what we set it to for the debian jobs20:01
ade_leesean-k-mooney, ack - well , its not set at all in the nova tests we just ran -- so we need at least that ..20:01
sean-k-mooneydevstack allocates swap by default i belive20:01
sean-k-mooney4096 for centos https://github.com/openstack/devstack/blob/3c98c21fec60da8d2d39df2e1d9b845a51817a0e/.zuul.yaml#L64420:02
sean-k-mooneywhich is the same as debain https://github.com/openstack/devstack/commit/f8e00b86aee9a8f9646bf5aed2c618843307b96320:03
sean-k-mooneyim not sure if the vars section are merged 20:04
sean-k-mooneyhttps://review.opendev.org/c/openstack/nova/+/790519/18/.zuul.yaml20:04
sean-k-mooneyso you might be overwriting it20:04
ade_leesean-k-mooney, we made the same change in swift tests - and got better results ..20:05
sean-k-mooneyya you are https://0d2f8596f517931fcac8-e8ce9722d0830f321adb7d7d98f7ea38.ssl.cf1.rackcdn.com/790519/18/check/nova-centos8-stream-fips/372ef4e/zuul-info/inventory.yaml20:05
sean-k-mooneyade_lee: swift does not spwan vms20:05
ade_leesean-k-mooney, ack20:06
sean-k-mooneythe swap is needed because qemu started allocating a 1GB cache for tcg byty code per qemu process20:06
ade_leesean-k-mooney, so try 8192?20:06
sean-k-mooneymeanign your 128mb vms now takes 1156mb20:06
sean-k-mooneywell right now you have 020:06
sean-k-mooneyso add   configure_swap_size: 409620:07
ade_leeok so lets try 4096 ..20:07
sean-k-mooneyyou can bump it to   configure_swap_size: 8192 but we also only have 80G of disk space in the vms so dont set it too large20:07
sean-k-mooneyfungi: have we considerd using zram by the way 20:07
sean-k-mooneyin the long run that might help in general20:07
opendevreviewAde Lee proposed openstack/nova master: Add check job for FIPS  https://review.opendev.org/c/openstack/nova/+/79051920:08
sean-k-mooneylets see if that helps and we can take another look tomorow20:09
ade_leesean-k-mooney, ack 20:15
fungisean-k-mooney: i'm not familiar with zram, is that on-the-fly memory compression?20:27
sean-k-mooneyfungi: yes basically it creats a compressed ramdisk that you then confirure for swap with a higher priority then normal swap on disk20:53
sean-k-mooneywith zstd compression you can get up to a 4x ratio20:53
sean-k-mooneyfungi: fedora started using it by default for laptops and low memeory isntalls20:53
fungimmm... then why not just compress the ram itself? i guess it's a simple composition to do the same, the extra layers just give me a moment's pause20:54
sean-k-mooneyfungi: it is simpler to integrate in the kernel this way20:55
sean-k-mooneyyou can use zswap which will do it transparently but zram+swap has is more compatiable20:55
sean-k-mooneywindows and osx just compress ram  transparently20:55
sean-k-mooneybut on linux the least intrusive way is to just add the zram device as a higher priority swap device20:56
sean-k-mooneyhttps://fedoraproject.org/wiki/Changes/Scale_ZRAM_to_full_memory_size20:56
sean-k-mooneyhttps://fedoraproject.org/wiki/Changes/SwapOnZRAM20:57
sean-k-mooneythos are the two relevent fedora propsoals20:58
sean-k-mooneyoh ubuntu 22.04 might have it on by defaul https://www.cnx-software.com/2022/01/13/ubuntu-22-04-zswap-raspberry-pi-4-2gb-ram/21:01
sean-k-mooneylooks like they are going with zswap directly which also works21:03
fungineat, well i expect we'll have 22.04 lts beta images available within a couple months21:19
sean-k-mooneythe zram feature has been avaiable in the herel since 2013 so technially we coudl enable it for our existing image by just adding zswap.enabled=1 to the kernel boot line in dib21:20
sean-k-mooneyit might be fun to play with that and see if it helps in general21:21
sean-k-mooneyhttps://ubuntu.com//blog/how-low-can-you-go-running-ubuntu-desktop-on-a-2gb-raspberry-pi-4 is a nice write up on how they used it to make the 2GB rpi work21:21
sean-k-mooneyim pretty sure the same parmaters would work well for us in our ci images21:21
fungiif you want to test with that, the fips mode role in zuul-jobs is an example of rebooting with custom kernel options at the start of a job21:31
fungi(bringing this discussion full-circle)21:32
sean-k-mooneyoh good point. ya i might put a DNM patch on top to try enabling it and see if it helps with the job.21:33
sean-k-mooneyif it does i could take a look at creating a dib element to configure this i guess21:33
sean-k-mooneythe default should speed things up a little but you can tune it a little more by cahnging the algoritim and compresor values21:34
sean-k-mooneyas always its a trade off between speed and compression ratio but lz4 and zstd are both good choices21:34
sean-k-mooneyi think it default to lzo which is not bad but the alternitives are bettter21:35
opendevreviewsean mooney proposed openstack/nova master: [WIP] add initial healthcheck support  https://review.opendev.org/c/openstack/nova/+/82501521:38
sean-k-mooneygibi: bauzas  i have a lot more work todo but that is the basic infra in place more or less. 21:39
sean-k-mooneyill start wiring it into the nova context object and such in follow up patches as well as start working testing in parallel.21:40
ade_leesean-k-mooney, results are in - that worked a lot better ..22:03
ade_leesean-k-mooney, https://review.opendev.org/c/openstack/nova/+/79051922:03
clarkbre working around the centos 8 issue in jobs I think that might be ok as long as you understand other people may not be able to reproduce. From the CI system perspective we won't (and shouldn't) work around it for you as exposing these problems is exactly why we test23:00
*** tbachman_ is now known as tbachman23:01
*** dasm is now known as dasm|off23:11
*** tbachman_ is now known as tbachman23:11

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