16:59:12 <knikolla> #startmeeting keystone
16:59:13 <openstack> Meeting started Tue Jul 14 16:59:12 2020 UTC and is due to finish in 60 minutes.  The chair is knikolla. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:59:14 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:59:16 <openstack> The meeting name has been set to 'keystone'
16:59:28 <knikolla> o/
16:59:40 <cmurphy> o/
17:00:31 <vishakha> o/
17:00:44 <knikolla> how's everyone?
17:02:53 <vishakha> Good. How are you?
17:03:00 <cmurphy> not too bad
17:04:19 <knikolla> doing alright.
17:04:37 <knikolla> going to maine the weekend next week.
17:05:46 <knikolla> I guess that's all of us for this meeting.
17:05:53 <knikolla> #topic Review Requests
17:06:43 <vishakha> I have few review requests that closes some bugs
17:07:16 <vishakha> #link https://review.opendev.org/#/c/731087/
17:07:38 <vishakha> #link https://review.opendev.org/#/c/740473/
17:07:56 <vishakha> #link https://review.opendev.org/#/c/737225/
17:08:15 <vishakha> #link https://review.opendev.org/#/c/739784/
17:08:35 <vishakha> #link https://review.opendev.org/#/c/738190/
17:09:57 <knikolla> Thanks vishakha
17:10:34 <knikolla> about https://review.opendev.org/#/c/731087/ i'm not entirely sure about the best time to prune stale assignment, i'
17:10:44 <knikolla> i'm not sure delete role is the best time, but i'd like to hear other opinions
17:11:52 <cmurphy> we should just keep it consistent with how we do it with project and domain assignments right?
17:13:32 <knikolla> do we prune those?
17:14:46 <vishakha> I  did not saw anywhere in keystone pruning assignments
17:14:47 <cmurphy> oh maybe i misunderstood, by stale you mean the ones that exist because of this bug?
17:15:24 <cmurphy> sorry i'm not sure i'm being clear, i haven't looked too deeply at this bug yet
17:16:15 <knikolla> yes, the ones prior to this bug being fixed.
17:17:27 <vishakha> This bug is specific to system assignments, where while deleting system assignments , the assignments are not deleted from the system assignment table
17:20:37 <cmurphy> i don't really think we should be going and retroactively fixing people's deployments and messing with their databases, and leaving in code that does a one-time cleanup but then forever exists in the code base
17:20:55 <cmurphy> i would rather the release note included mysql instruction to help them clean it up one time
17:22:40 <bnemec> I could use another opinion on https://review.opendev.org/#/c/733881/ It's what we agreed to at the PTG, but there was some disagreement on the review. I left more thoughts related to it on https://review.opendev.org/#/c/726929
17:22:40 <bnemec> (for oslo.limit and keystoneauth)
17:23:43 <knikolla> cmurphy: hmmm... unsure if a good idea, but maybe a sql migration?
17:24:39 <cmurphy> knikolla: i'd be more in favor of that than code that runs in the driver
17:25:00 <cmurphy> but that wouldn't fix older deployments
17:25:38 <knikolla> that is true.
17:29:11 <vishakha> Still not sure should I add a note in the document informing the deployers?
17:30:56 <knikolla> yes, that while the bug is fixed moving forward, the previously stale system role assignments will need to be cleaned manually.
17:31:18 <knikolla> we should provide some guidance on how to do that.
17:31:19 <cmurphy> i'm saying i disagree with lance's suggestion to add remove_stale_role_assignments and i would prefer documenting manual steps in the release note
17:32:08 <cmurphy> i'll comment on the review and see what lbragstad thinks
17:32:24 <vishakha> Thanks cmurphy knikolla
17:32:44 * lbragstad walks in late
17:32:49 <knikolla> another possible solution is to catch the exception when a stale assignment is encountered, and clean it then.
17:32:51 <lbragstad> sorry - i got wrapped up reading some docs
17:37:45 <tosky> and also https://review.opendev.org/#/c/733801/ which removes the last legacy job, which already received a +2 from last week
17:39:51 <knikolla> lbragstad, cmurphy: thoughts on cleaning up on exception encountered?
17:40:07 <lbragstad> knikolla cmurphy yep - i just responded on the review, but i can repeat here, too
17:40:27 <lbragstad> i can empathize with not running that code every time we delete a role
17:40:36 <lbragstad> not wanting to run that code*
17:40:49 <lbragstad> but a better alternative wasn't coming to me
17:42:07 <lbragstad> to make sure i understand correctly, cmurphy is your recommendation to provide documentation that tells operators how to fix this with SQL statements?
17:43:10 <cmurphy> lbragstad: yes that is my proposal, it should be a pretty simple sql query, and also a pretty easy test to see if you are affected by it (see if you can run role assignment list --names)
17:44:45 <lbragstad> what about an upgrade check?
17:44:50 <knikolla> That is a better option than running it each time during delete_role, but I would love to avoid adding more work to operators, even if it is a one time thing.
17:45:25 <lbragstad> we typically put things like this in keystone-manage
17:45:45 <lbragstad> instead of asking operators to tinker with the database directly
17:46:11 <lbragstad> (keystone-manage token_flush was an example)
17:46:27 <knikolla> What about adding a try/catch in listing roles with names and doing the cleanup if the exception is caught? That will add more code with limited usefulness, but we would be doing the same by adding documentation.
17:46:28 <cmurphy> token_flush was a task that needed to happen every so often
17:46:35 <cmurphy> this is a one time thing
17:46:50 <knikolla> And the code won't run if the exception is never encountered.
17:49:23 <cmurphy> we have a precedent for asking operators to run db queries to fix a broken state left by a bug
17:49:28 <cmurphy> https://bugs.launchpad.net/keystone/+bug/1848342
17:49:28 <openstack> Launchpad bug 1848342 in OpenStack Identity (keystone) "Duplicated entries in users API" [Medium,Fix released] - Assigned to Pedro Henrique Pereira Martins (pedrohpmartins)
17:49:32 <cmurphy> i don't really see why it's that big of a deal
17:50:36 <cmurphy> it feels really inelegant to leave code in keystone that will last forever that will only ever run once on a deployment in its lifetime
17:51:08 <knikolla> hmm, i wasn't aware of the precedent. interesting.
17:51:17 <lbragstad> i don't recall seeing this bug
17:54:08 <knikolla> i'm okay with a release not for this. given that very few deployments will be using system role assignments at this point in time, it feels like very few people will be running into this.
17:54:38 <knikolla> note*
17:57:03 <lbragstad> if others are ok with the approach, i won't get in the way
17:57:21 <lbragstad> i was unaware of cases where we did this in the past'
17:57:35 <knikolla> i was unaware too
17:58:23 <knikolla> vishakha: you can revise your spec with the new approach, and we can continue talking on the review, or in next weeks meeting
17:58:30 <cmurphy> this may be the only case, if we've come up with another solution for a similar problem in the past we could look at that but i'm unaware of any
17:59:13 <cmurphy> it looks like it was mostly just me involved in the review of that last bugfix so i may be biased
18:00:02 <lbragstad> the only other case that's jogging my memory is asking operators to adjust character encoding in the database... which in my opinion is a bit different than this because it's a database setting out of keystone's control
18:01:07 <cmurphy> yeah
18:01:20 <knikolla> either way, we're out of time
18:01:23 <knikolla> thanks all
18:01:24 <vishakha> Sure, i will wait until we agree on something. Thanks knikolla lbragstad cmurphy
18:01:25 <knikolla> #endmeeting