Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2024-07-10
05:55:01 opendevreview melanie witt proposed openstack/nova master: Config option to treat unified limit of zero as unlimited https://review.opendev.org/c/openstack/nova/+/923813
05:57:40 opendevreview melanie witt proposed openstack/nova master: Config option to treat unified limit of zero as unlimited https://review.opendev.org/c/openstack/nova/+/923813
06:56:29 opendevreview Robert Hoffmann proposed openstack/nova master: Fix: clean up volume attachments https://review.opendev.org/c/openstack/nova/+/923646
08:43:57 opendevreview Abhishek Kekane proposed openstack/nova master: DNM: Test glance new location api https://review.opendev.org/c/openstack/nova/+/891207
08:59:01 frickler sean-k-mooney: did you have a chance to look at the failures on the iso stable backports yet? the grenade failure seems unrelated, I'm more worried about the barbican one
08:59:38 sean-k-mooney frickler: not yet but ill do that shortly, thanks for the heads up
09:00:13 sean-k-mooney frickler: im not really sure how any fo this could be related to barbican given we do not currently have any encyped local storage support in nova
09:00:28 sean-k-mooney at least not in the context of barbican
09:01:50 sean-k-mooney JayF: they do not need to be seperate i just dont blan to work on the metadta feature this cycle. so if its ok with you can you squash your changes into min and take over that patch?
09:03:37 sean-k-mooney frickler: oh interesting i wonder if that was failing in the backport of the orginal cve fix
09:03:44 frickler sean-k-mooney: well this does sound somehow related to me, maybe not your patch but one of the other cve fixes? though not sure why it would only be failing sometimes? "Image ... is unacceptable: Image content does not match disk_format"
09:03:52 frickler https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_f2e/923724/1/check/barbican-tempest-plugin-simple-crypto/f2e7279/testr_results.html
09:03:53 sean-k-mooney its a non voting job and we merged those quite quickly
09:04:26 sean-k-mooney yep it sounds related but i wonder if this is on the glance side or the nova side
09:04:43 sean-k-mooney given its a snapshot related test
09:04:51 sean-k-mooney im going to look into the logs now and see
09:05:07 frickler well that msg is in a 500 response to create_server afaict
09:05:48 sean-k-mooney ya so maybe we have an issue with bootign form a snapshot or somehting with the image signing
09:11:01 sean-k-mooney so i think the instance has a cidner volume associated with it i wonder if its on nfs
09:15:23 sean-k-mooney ok no so cinder is configure for lvm
09:15:50 sean-k-mooney if it was nfs i know there are specific snapshot related issue where the glance and actual image format can differr
09:16:33 sean-k-mooney frickler: i may need to add some more debug logging which might be useful in general
09:17:06 sean-k-mooney nova.exception.ImageUnacceptable: Image 93f91dc1-8c6f-416e-ab0d-abab96d1d03e is unacceptable: Image content does not match disk_format
09:17:31 sean-k-mooney is well an good but it would be nice if it printed the requested and actual format
09:19:02 sean-k-mooney its possibel that this si a bug in the barbican plugin too so ill look at the test
09:21:14 sean-k-mooney frickler: so it using a uec image file img_file = /opt/stack/devstack/files/cirros-0.6.2-x86_64-uec.tar.gz
09:21:18 sean-k-mooney https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_f2e/923724/1/check/barbican-tempest-plugin-simple-crypto/f2e7279/controller/logs/tempest_conf.txt
09:22:02 sean-k-mooney the image format is not set in the config
09:23:34 sean-k-mooney and that default to qcow https://github.com/openstack/tempest/blob/master/tempest/config.py#L1183
09:23:53 sean-k-mooney frickler: so the nova code is correctr the imnage is not what it says it is
09:28:04 sean-k-mooney frickler: this is because we are overriding the image https://github.com/openstack/nova/blob/stable/2024.1/.zuul.yaml#L928
09:28:59 frickler but why doesn't it fail 100%, then? otoh maybe we don't want to care too much about barbican anyway, I just saw that the barbican-grenade job was last passing 2y ago ...
09:29:02 sean-k-mooney frickler: i need to backport https://github.com/openstack/nova/commit/eed3e2b47ffea24d08ad7a85a4e9c36ef56d815e first
09:29:34 sean-k-mooney frickler: i think this is the first backprot that has run on that branch sicne we merge the check
09:29:52 sean-k-mooney the previous two in the seriese did not trigger the barbican job since they were test only
09:30:32 sean-k-mooney ill cherry-pick https://github.com/openstack/nova/commit/eed3e2b47ffea24d08ad7a85a4e9c36ef56d815e back and see if that passes if it does then we can proceed with that and recheck the iso backport
09:38:07 opendevreview sean mooney proposed openstack/nova stable/2024.1: Stop using split UEC image (mostly) https://review.opendev.org/c/openstack/nova/+/923831
09:38:52 sean-k-mooney frickler: ok i expect that should pass, if it does we only need that in 2024.1 as that was the only release use used uec images to work around gate insablitiy
09:40:01 sean-k-mooney they didnt actully fix anything they just chagne the timing slightly which looks like panics happend less often but they still happend so we went back to useing qcow a few months ago
09:43:30 frickler ah, ok, that explains why neither master nor older stable branches were affected
09:44:01 sean-k-mooney ya we ment to revret that after FF but before the final RC and didnt get around to it
09:44:46 sean-k-mooney by revert i mean do what the patch im now backporting does of keep one job with uec images for coverage but go back to qcow for the rest
10:07:24 elodilles sean-k-mooney: sorry for the late response, so about the skip test patch: it's OK to have it separately, maybe a 'related-bug' would be good, but it is good as it is
10:25:47 sean-k-mooney elodilles: ack thanks i can add that to the backports sure
10:26:29 sean-k-mooney i think it merged on master lastnight and i have not created the cherry picks yet so ill do that shortly
10:28:48 elodilles +1
13:59:45 mnaser elodilles: sorry to ping here but I dont se you in glance :) do you mind kicking https://review.opendev.org/c/openstack/glance/+/923310 in and I can work on getting the rest to pass and ready to +W
14:30:29 elodilles mnaser: let me answer at the glance channel o:)
15:58:30 melwitt sean-k-mooney: this is lame and late but fyi I proposed the spec about a config option for being able to treat missing or zero unified limits for resources as unlimited https://review.opendev.org/c/openstack/nova-specs/+/923807
15:59:05 melwitt and super basic WIP code https://review.opendev.org/q/topic:%22bp/unified-limits-nova-zero-as-unlimited%22
17:51:38 sean-k-mooney melwitt: :) better late then never, do you think it could happen thsi cycle or should we premetivly mvoe that to 2025.1
17:51:51 sean-k-mooney or backlog i guess
17:54:55 melwitt sean-k-mooney: I think this cycle. unless I'm massively missing something about it 😛
17:55:29 sean-k-mooney thats more a question of do you think we cna do that and your other feature but if you think we can review both im not agiasnt doign that now
17:55:44 sean-k-mooney i would love to see us make unifed limits the default in 2025.1
17:55:52 sean-k-mooney so making incremental progress to that is good
17:55:53 melwitt the WIP code is all it should take but perhaps people will find other issues about it
17:56:25 sean-k-mooney oh cool i have not looked at that btu the fact it exists is promising
17:57:03 sean-k-mooney yoru not refering to the oslo.limits poc right, you have a nova poc for this
17:57:14 sean-k-mooney ah https://review.opendev.org/c/openstack/nova/+/923813
17:57:58 melwitt it should be only two patches, one for nova-manage limits migrate_to_unified_limits (to make it give more information like advice on what limits need to be created after running it) and one patch to check the config option and consider none/zero limit as unlimited if configured
17:58:08 sean-k-mooney hum that is not quite what i was expecting but it proably would work
17:59:14 sean-k-mooney i have not read the spec yet but i was expecting somethign like, ask keystone for all limit it supprot then check only those limits
17:59:31 melwitt ok, if you can add a comment on the spec or patch I can potentially change it. I proposed what first came to my mind
17:59:46 sean-k-mooney insead of "oh i got zero ignore if config option is set"
18:00:00 melwitt ah ok. that might work too, have to try it
18:00:28 sean-k-mooney so i guess there is only one edge case that matters
18:00:50 sean-k-mooney what happens in these two cases
18:01:01 sean-k-mooney i recuest a resouce and no limit is defied
18:01:10 sean-k-mooney i request a resouce and its defied as 0
18:04:10 sean-k-mooney with your code i think those will be treated the same
18:04:23 sean-k-mooney but im not sure if they should be
18:06:09 sean-k-mooney imo i think no limit defiend shoudl be unlimeted and limit defied as 0 mean you can not use it.
18:06:20 sean-k-mooney but i know others had different views
18:08:13 sean-k-mooney melwitt: ill refect on your spec but not tonight, probaly friday
18:08:55 sean-k-mooney but thats what my intition expects i just dont recall all the context
18:09:44 sean-k-mooney if we take the approch im suggesting im not even sure it requries a cofnig option beyond the quota driver
18:10:25 sean-k-mooney since registering the limits with 0 or any value starts enforcing that limit
18:11:22 sean-k-mooney although i could see use wanting this to be explict too in case anywone is currently using the unifed limits driver in production
18:32:52 opendevreview sean mooney proposed openstack/nova stable/2024.1: fix qemu-img version dependent tests https://review.opendev.org/c/openstack/nova/+/923878
18:51:47 melwitt sean-k-mooney: sorry, was on a call. yeah, I hear you. where I'm coming from ultimately is I'm concerned about the fallout potentially of when we switch the default to unified limits. I keep envisioning the scenario where as soon as they install or upgrade, all server creates start failing because they forgot or missed setting one limit
18:52:32 melwitt so at the core of it I want to prevent operator pain and I'm sure there's other ways to handle it that I haven't thought of
18:55:39 sean-k-mooney yep i want systems to more or less "just work" when we switch but i dont think we shoudl force peole to define limtis for things that previously coudl not have a limit
18:56:14 sean-k-mooney so that why im usggeting treating no registered limit as unlimited and registered with 0 as not allowed
18:56:26 sean-k-mooney or over quota when you try ot request one of that thing
18:57:09 sean-k-mooney the problem of couse if if we do that when we change the default everythign becomes unlimited
18:57:19 sean-k-mooney unless you run the migration tool
18:57:26 melwitt sean-k-mooney: ok, yeah that makes sense
18:57:39 melwitt yeah :/
18:57:42 sean-k-mooney i think thats where the idea was if any limti is regesterd
18:57:50 sean-k-mooney then we know they have started configuring it
18:57:51 melwitt ah, right
18:58:14 sean-k-mooney so there are 3 cases. no limits at all -> they didnt migrate
18:58:34 sean-k-mooney there are some registered limits -> only those resocue classes are enforced
18:58:52 sean-k-mooney all resources classes in the request have limits -> all are enforced
18:59:14 melwitt that makes sense
19:03:03 sean-k-mooney your patch treats no limit and limit of 0 as the same. so that is simpler then i was suggesting above
19:03:21 sean-k-mooney im not sure if that is enough or if we need the extra complexity of above
19:16:46 melwitt sean-k-mooney: it's likely oversimplified ... I thought of it because oslo.limit literally returns 0 for no limit, and doing something smarter will need additional keystone API calls. but not being able to differentiate between intentionally 0 and not is probably too restricting
19:33:01 sean-k-mooney i think an extra call while not ideall is ok

Earlier   Later