14:00:38 <abhishekk> #startmeeting glance
14:00:39 <openstack> Meeting started Thu Dec  3 14:00:38 2020 UTC and is due to finish in 60 minutes.  The chair is abhishekk. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:40 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:42 <openstack> The meeting name has been set to 'glance'
14:00:44 <abhishekk> #topic roll call
14:00:49 <abhishekk> #link https://etherpad.openstack.org/p/glance-team-meeting-agenda
14:00:51 <abhishekk> o/
14:01:06 <Steap> o/
14:01:26 <jokke> o/
14:01:42 <abhishekk> looks like only 3 of us
14:02:19 <abhishekk> rosmaita, will might join late
14:02:26 <abhishekk> Lets start
14:02:36 <abhishekk> #topic release/periodic jobs update
14:02:37 <rosmaita> 0/
14:02:45 <abhishekk> cool, he is here :D
14:03:02 <abhishekk> we have released glance-store for milestone 1
14:03:14 <abhishekk> for glanceclient patch is up
14:03:26 <abhishekk> and for glance I am waiting for Steap's patch to get merge
14:03:34 <abhishekk> it is in merged conflict at the momemnt
14:03:50 * Steap checks
14:04:13 <abhishekk> I am planning to tag glance M1 on Monday around this time
14:04:33 <abhishekk> Periodic jobs all green this week
14:04:53 <abhishekk> First time we are not hitting timeouts during milestone releases \o/
14:05:02 <rosmaita> hang on with Steap's patch
14:05:10 <rosmaita> i think the release note could use a revision
14:05:21 <abhishekk> ack
14:05:44 <jokke> nice
14:06:22 <abhishekk> moving ahead
14:06:43 <abhishekk> #topic Milestone 1 priorities
14:06:55 <abhishekk> Important reviews
14:07:08 <abhishekk> #link https://review.opendev.org/c/openstack/glance/+/763920
14:07:26 <abhishekk> this is Steap's patch which we need to get merged before M1
14:08:00 <abhishekk> Also we need changes/reforms in cluster awareness specs
14:08:10 <abhishekk> and, Lite spec for task API for general user - Needs reviews
14:08:26 <abhishekk> #link https://review.opendev.org/c/openstack/glance-specs/+/763740
14:09:14 <abhishekk> Please review specs whenever you get time
14:09:29 <abhishekk> we need to get them merged within next 2 weeks
14:09:52 <abhishekk> moving to Open discussion
14:09:59 <abhishekk> #topic Open discussion
14:10:36 <jokke> I will revisit the spec next week, Have hands full for today/tomorrow
14:10:44 <abhishekk> jokke, ack
14:10:48 <abhishekk> So I was testing properties protection using policies to find out impact for RBAC
14:11:27 <abhishekk> First impression is we don't have enough document to guide users/operators to use property protection using policies
14:12:19 <abhishekk> 2nd is property protection policies needs to be define in policy.yaml file (it don't recognize separate file at the moment)
14:12:55 <rosmaita> that's weird ... it used to work with separate files
14:13:20 <abhishekk> 3rd there are some low priority bugs which can raise 500 errors if there are any issues in configuration files
14:13:42 <abhishekk> rosmaita, you might be confusing the same with property protection with roles
14:14:01 <abhishekk> you can define property protection with roles in different conf file
14:14:01 <jokke> rosmaita: I have feelin that it got messed up with move to yaml patch which rattled around the policy files and how they gets read
14:14:17 <abhishekk> jokke, nope that's not the case
14:14:27 <jokke> ohh ... just saw the above
14:14:32 <rosmaita> i think abhishek is right
14:15:01 <abhishekk> as far as I remember we never used policies for property protection and that is why there is no documentation to explain the same
14:15:15 <abhishekk> #link https://etherpad.opendev.org/p/property-protection-with-policies
14:15:20 <rosmaita> rackspace used it in their public cloud
14:15:47 <jokke> I think we did too, but can't remember
14:15:48 <abhishekk> NTT also used but with roles
14:17:13 <abhishekk> I will submit documentation to explain property protection with policies
14:19:06 <abhishekk> That's it from me for today
14:19:43 <abhishekk> anything else, Steap, jokke, rosmaita
14:19:50 <Steap> nah, I'm gonna fix my patch
14:19:56 <jokke> quick one abut ^^
14:19:57 <Steap> rosmaita's patch introduced a conflict
14:20:02 <rosmaita> Steap: ha!
14:20:05 <Steap> First he steals my food, then he breaks my patches
14:20:22 <smcginnis> :)
14:20:23 <rosmaita> Steap: hang on a minute, i am making some suggestions for your release note.
14:20:34 <Steap> rosmaita: ok
14:20:36 <abhishekk> :D
14:20:49 <rosmaita> i am still not sorry that i ate the last bowl of goulash
14:20:56 <abhishekk> jokke, you were saying something?
14:21:45 <jokke> So the owner_is_tenant, as it's security thingie, should we detect it in the config and refuse glance-api starting if it still configured to make sure anyone did not upgrade without reading renos etc. and get bitten by it
14:22:39 <abhishekk> I think it will be ignored
14:22:57 <jokke> Like I'm fully committed to get rid of it. Just want to play the change itself safe
14:23:03 <rosmaita> what will happen is that nothing will work
14:23:07 <jokke> abhishekk: currently, yes
14:23:44 <jokke> rosmaita: exactly .. and there is no indication as of now, why
14:23:57 <abhishekk> so you want to add upgrade check?
14:24:07 <rosmaita> that is a good idea
14:24:09 <abhishekk> s/want/suggesting
14:24:10 <jokke> it will just start throwing 403s (I think)
14:25:31 <abhishekk> please add comment so that Steap will get notification as well
14:25:34 <rosmaita> take a look at this and see if we agree that it's a good way to announce this: https://review.opendev.org/c/openstack/glance/+/763920/5/releasenotes/notes/remove-owner_is_tenant-b30150def293effc.yaml#8
14:25:42 <jokke> I don't mind be it upgrade check or just 2loc check if it's still in config, exit and log that the service did not start because it's configured and won't work
14:26:26 <rosmaita> it might be worth sending something to the ML saying that the option has been removed
14:26:31 <rosmaita> as see if anyone screams
14:26:34 <jokke> rosmaita: yeah, I have that reno open. Still commenting on it.
14:27:12 <jokke> rosmaita: I think it's irrelevan it someone screams or not ;) Just lets make sure it's clear why world is broken if someone has ignored all the warnings until now ;P
14:27:16 <abhishekk> I will send out mail
14:27:24 <rosmaita> it's kind of a complicated db migration, you will have to have keystone available to do it
14:27:51 <rosmaita> would have to look up the user_id in the 'owner' column, find out what project they are in, and then replace the value
14:27:52 <rosmaita> but
14:28:00 <rosmaita> i believe users can be in multiple projects
14:28:03 <rosmaita> so it would be a mess
14:28:12 <abhishekk> lot of
14:29:13 <jokke> I think the "no migration path" is pretty clear and like said there is no easy way to do it. I'm not saying we shouldn't remove the config nor that we should postpone it for any reason
14:29:32 <jokke> Just lets be clear why world is broken if you still have it in config
14:29:44 <abhishekk> +1
14:31:14 <jokke> If it was just me glance-api would be logging something like "You fool, should have listened first time. Now ye're fecked" :P
14:31:25 <abhishekk> lol
14:32:00 <rosmaita> well, i think the problem with that is that oslo.config will give you a NoSuchOpt exception when you try to read it, if the option isn't defined (which it won't be, since Steap's patch removes it)
14:32:34 <rosmaita> i think the upgrade check may be the way to go here
14:33:03 <abhishekk> makes sense
14:34:03 <rosmaita> i will take the action item to write an upgrade check for the 2 options we are removing
14:34:11 <rosmaita> (unless someone else is itching to do it)
14:34:25 <abhishekk> you are most welcome :D
14:34:55 <jokke> So one option, which we have used before, is to reintroduce the config key under config.py with the hidden flag so it will not be added by configgen but we can access it and set it to like None so if it's set to True or False we know it was actually set in config
14:35:53 <rosmaita> ok, that's good to know
14:36:05 <abhishekk> In this case we also need to make a note to remove this after couple of cycles?
14:36:27 <rosmaita> no, the upgrade check should continue to work fine if no one has the option
14:36:53 <abhishekk> ack
14:37:00 <jokke> abhishekk: or not ... can be just #TODO ... doesn't hurt have it hanging in there for longer, if someone needs easy quick patch to get started, that'd be good one :D
14:37:21 <abhishekk> sounds good then
14:37:57 <jokke> I can't remember what we used the hidden configs before so might be difficult to find the patch as example but I just remember we have done it somewhere like Juni, Kilo, Liberty range ;)
14:38:11 <abhishekk> So I need to wait for upgrade check + Steap's patch before tagging M1
14:38:11 <rosmaita> i will look, i wasn't aware of that option
14:38:19 <abhishekk> Yes, I remember that as well
14:38:30 <rosmaita> i think the upgrade check can be a follow-up
14:38:39 <rosmaita> i don't think anyone is going to upgrade to M-1 !
14:38:52 <abhishekk> :D
14:39:13 <jokke> rosmaita: very true, and i'd just don't want to rely on that alone either as there is nothing guaranteeing that anyone runs those either
14:39:28 <rosmaita> jokke: that's what i was going to ask
14:39:50 <rosmaita> are you still in favor of the don't start if the option is there patch?
14:40:52 <jokke> I would be yeah, that way it's super clear and can't be missed
14:41:43 <jokke> It's even unlikely that no-one will hit that piece of code, yet if we do not write it, the likelyhood will increase exponentially :P
14:41:55 <rosmaita> should we do that for M-2 or hold M-1 for it?
14:41:58 <abhishekk> I don't think hide option is available now
14:42:19 <rosmaita> ok that will complicate things
14:42:34 <jokke> abhishekk: then, the easiest solution is to create hidden_config.py and not add that to the configgen config
14:42:48 <rosmaita> i was thinking that i could define it only in the upgrade checker and not include it for configgen
14:43:05 <rosmaita> ok what jokke said
14:43:44 <abhishekk> whatever suits best
14:43:54 <rosmaita> either that, or just use regular python (not oslo.config) to read the config file directly
14:44:05 <jokke> rosmaita: just would need to import that hidden_config at the startup so oslo.config actually picks the defenition up, if it's just upgradecheck it won't, I think
14:44:39 <rosmaita> jokke: right, i was just trying to figure out how i could do this in the upgrade check
14:44:50 <abhishekk> Ok to be followup and not holding M1 for it
14:45:25 <jokke> rosmaita: same way, import the hidden_config and check if the value is set, super simple
14:46:21 <jokke> it should be literally like ~10 line addition to the current patch, 20 with the upgrade check (not sure how much boilerplating it needs)
14:47:10 <abhishekk> Let it be in a separate patch as we need upgrade check for 2 deprecated (removed) options
14:47:58 <abhishekk> Anything else ?
14:48:01 <rosmaita> ok, i will work on the upgrade check and jokke can do the api part
14:48:21 <rosmaita> we can resolve the location of removed_config.py once we have patches up
14:49:21 <jokke> ++
14:50:06 <rosmaita> i must say, one nice thing about the new gerrit is having the project name in the gerrit url
14:50:39 <abhishekk> yes
14:51:42 <abhishekk> Lets wrap up
14:51:58 <abhishekk> thank you all
14:52:06 <jokke> thanks!
14:52:14 <abhishekk> have a nice weekend
14:52:15 <rosmaita> bye!
14:52:25 <abhishekk> #endmeeting