Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-04-28
12:52:57 sean-k-mooney this looks correct
12:53:13 sean-k-mooney am you have not turned on scope enforement have you
12:53:34 sean-k-mooney this requires a system scope token
12:53:59 sean-k-mooney trident: you cannot use a project scope token if you have scope enfroment enabled
12:54:55 sean-k-mooney so normal project_member tokens would not work even i fyou udated teh check_str unless you also added 'project' to the scope_types
12:56:23 sean-k-mooney trident: by the way im not sure admin_or_ower shoudl work in this context
12:56:42 sean-k-mooney trident: the resouce does not exsit yet so you cant be an ower of it
12:57:07 sean-k-mooney you woudl want something likse we use for server create
12:57:09 sean-k-mooney https://opendev.org/openstack/nova/src/tag/22.3.0/nova/policies/servers.py#L166-L175
12:57:15 sean-k-mooney which is just project_member
12:58:12 sean-k-mooney actully no
12:58:14 sean-k-mooney https://opendev.org/openstack/nova/src/tag/22.3.0/nova/policies/create_backup.py#L24-L36
12:58:17 sean-k-mooney creat backup is better
12:58:31 sean-k-mooney trident: you woudl want PROJECT_MEMBER_OR_SYSTEM_ADMIN
12:58:44 sean-k-mooney with both scope types
12:59:44 sean-k-mooney again we dont really recommend doing this but if i was to do that personally i woudl create role and then in the custom polciy string allow peopel with admin or the "create_flavor" role to create flavors
13:00:14 sean-k-mooney that way you can atleast limit it to a subset of peopel but admin_or_owner shoudl not work for create
13:01:02 trident Hm, yeah, that makes sense - as you say, it doesn't exist, so owner doesn't make sense at the time of creation. I'm however pretty sure those rules have worked previously. Probably at least on train.
13:01:21 sean-k-mooney admin_or_ower was defiend as 'is_admin:True or project_id:%(project_id)s'
13:01:31 sean-k-mooney but flavors are not part of projects
13:01:39 sean-k-mooney so that woudl not work in anycase
13:02:26 trident Thanks for the advise! I'll do some testing and see what I end up with. :)
13:02:38 sean-k-mooney on train it was more or less the same https://opendev.org/openstack/nova/src/tag/train-em/nova/policies/base.py#L27-L30
13:13:08 gibi sean-k-mooney: I can reproduce the keyerror on master too with a bit of change and I think I see the issue. _create_tracker should take dict that come from the db with id field but _set_hvdevs should take dict that come from the hypervisor (no id field). My new tests (and one existing test) passed db dict to _set_hvdevs causing the db dict to get corrupted
13:13:52 gibi https://github.com/openstack/nova/blob/028b3bca16c750f6c7edf1b389ed6c79a2c9843d/nova/tests/unit/pci/test_manager.py#L361
13:13:52 sean-k-mooney ya so that can work if its a copy
13:14:03 sean-k-mooney since it wont affect the gloabl sate but ya
13:14:25 gibi so we need the copy in L361 as that already corrupts the global state
13:14:34 gibi even on master
13:14:44 sean-k-mooney yep
13:15:02 sean-k-mooney well
13:15:11 sean-k-mooney we shoudl do the copy on line 347
13:15:23 sean-k-mooney or we shoudl hvae _create_tracker do the copy internally
13:15:37 sean-k-mooney and then work on self.fake devs if need
13:16:24 sean-k-mooney but ya in anycase i wonder why we are not seeign this fail in general
13:16:38 sean-k-mooney theses test are not flaky in my experince
13:18:30 gibi we only have one test on master that does the mistake to pass a db dict with id to _sethvdevs. That corrupts the global state but no other test depends on that. Then I added another test that did this mistake and blow if the two test run in the same executor
13:18:41 gibi you can reproduce the issue by duplicating test_set_hvdev_remove_tree_maintained_with_allocations on master
13:18:49 sean-k-mooney ack ok
13:18:57 gibi I will try to clean this up
13:19:25 sean-k-mooney i wonder if we shoudl jsut have _create_tracker deep copy in genreal
13:20:03 sean-k-mooney we can still explictly do it but that would remove the need to do it in the default case
13:20:27 sean-k-mooney only if you call set_hvdevs
13:21:19 sean-k-mooney gibi: glad we caught that before it was merged
13:21:33 sean-k-mooney that kind of think is a pain to debug in the gate when it only failes ocationally
13:21:48 gibi yes, it was a good catch
13:21:59 gibi nobody likes interfeering test cases :)
13:56:01 opendevreview Balazs Gibizer proposed openstack/nova master: Isolate PCI tracker unit tests https://review.opendev.org/c/openstack/nova/+/839766
13:56:20 gibi sean-k-mooney: ^^ I will base the current series on top of this
14:02:46 opendevreview Balazs Gibizer proposed openstack/nova master: Isolate PCI tracker unit tests https://review.opendev.org/c/openstack/nova/+/839766
14:02:46 opendevreview Balazs Gibizer proposed openstack/nova master: Remove unavailable but not reported PCI devices at startup https://review.opendev.org/c/openstack/nova/+/838553
14:02:47 opendevreview Balazs Gibizer proposed openstack/nova master: Simulate bug 1969496 https://review.opendev.org/c/openstack/nova/+/838554
14:02:47 opendevreview Balazs Gibizer proposed openstack/nova master: Allow claiming PCI PF if child VF is unavailable https://review.opendev.org/c/openstack/nova/+/838555
14:13:30 sean-k-mooney gibi: ack ok that makes sesne
15:21:08 opendevreview Elod Illes proposed openstack/nova master: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839781
15:29:36 elodilles gibi bauzas : another stable gate fix to review ^^^ when you have time o:)
15:30:26 elodilles (today's broken oslo.policy release showed an error in our tox docs target as the job is failing for stable branches due to a release on zed)
15:46:05 clarkb elodilles: gibi bauzas email was sent about that problem a few weeks ago http://lists.openstack.org/pipermail/openstack-discuss/2022-April/028016.html there is a good chance that much of openstack needs that sort of update
15:47:04 gibi elodilles, clarkb: thanks I'm +2 on it
15:49:29 sean-k-mooney when did that get remvoed
15:49:42 sean-k-mooney we used to install requiremets.txt
15:50:27 clarkb sean-k-mooney: a while back there was a big push to switch to trimming the doc requirements down so you didn't have to install everything. What that missed was that the doc builds depended on the projects to collect cli command output and such. Basically I think it was docs having their own requirements that introduced the bug
15:50:40 clarkb the intent was good, but no one realized that this flaw existed
15:51:17 sean-k-mooney ah so it was applied genericly
15:51:26 sean-k-mooney i just did not recally this patch going in
15:51:55 sean-k-mooney we might also need test-requiremetns in some cases but in generaly not
15:53:54 elodilles clarkb: thanks, i'll try to check other projects as i've seen +24 broken stable-periodic tox-docs job today (neutron has already a similar patch on the gate right now)
15:54:13 elodilles (this one: https://review.opendev.org/c/openstack/neutron/+/839777 )
15:59:01 sean-k-mooney clarkb: so it would b enice to have included the change id of the change that remvoed it but i dont think we shoudl hold this up for that so ill review it now
15:59:25 clarkb I mean its not my change. I just helped debug a similar problem a few weeks ago and we told everyone about it hoping they would audit and fix their repos
15:59:30 clarkb seems that didn't happen hence the current situation
15:59:50 sean-k-mooney i never new this happend i must have missed the mail
16:02:03 clarkb it was a huge cross openstack effort to change the doc build system
16:02:17 clarkb it was a while ago so I don't remember the details just that it happened and a lot of stuff got updates
17:57:25 opendevreview Merged openstack/nova master: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839781
18:01:08 opendevreview Balazs Gibizer proposed openstack/nova stable/ussuri: Reproduce bug 1953359 https://review.opendev.org/c/openstack/nova/+/822047
18:01:09 opendevreview Balazs Gibizer proposed openstack/nova stable/ussuri: Extend the reproducer for 1953359 and 1952915 https://review.opendev.org/c/openstack/nova/+/822048
18:01:10 opendevreview Balazs Gibizer proposed openstack/nova stable/ussuri: [rt] Apply migration context for incoming migrations https://review.opendev.org/c/openstack/nova/+/822050
18:17:25 opendevreview Elod Illes proposed openstack/nova stable/yoga: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839809
18:22:21 opendevreview Elod Illes proposed openstack/nova stable/xena: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839810
18:23:42 opendevreview Elod Illes proposed openstack/nova stable/wallaby: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839811
18:25:32 opendevreview Elod Illes proposed openstack/nova stable/victoria: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839812
18:26:42 opendevreview Elod Illes proposed openstack/nova stable/ussuri: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839813
19:06:43 melwitt dansmith: yoga fix for docs job is ready https://review.opendev.org/c/openstack/nova/+/839809
19:07:19 dansmith melwitt: I'm going to go out on a limb and say it'd be okay for you to slam those mofos in :)
19:07:40 dansmith you know, IMHO :D
19:08:06 sean-k-mooney i certenly would not object
19:11:15 melwitt haha ok
21:37:26 opendevreview Dan Smith proposed openstack/nova master: DNM: Run against performance.json patch https://review.opendev.org/c/openstack/nova/+/838934
22:11:42 dansmith clarkb: around?
22:15:17 clarkb dansmith: hi
22:15:44 dansmith not even a full devstack run and 57k queries to the keystone db.. seems high, no?
22:16:16 clarkb dansmith: that does seem high. But openstackclient does have to get a new token for everything since there is no token caching
22:16:18 clarkb perhaps related to that?
22:16:30 dansmith still, 57k
22:16:39 dansmith also, they're almost all select
22:16:47 dansmith don't we have to insert when we create a token?
22:16:51 clarkb yes I think so
22:17:24 dansmith https://termbin.com/s2xj
22:17:26 clarkb I wonder if we need to instrument keystoen directly to try and identify that?

Earlier   Later