| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-03-31 | |||
| 19:34:18 | openstackgerrit | Merged openstack/nova master: Add new default roles in os-ips policies https://review.opendev.org/715545 | |
| 20:03:23 | openstackgerrit | Merged openstack/nova master: Pass the actual target in os-agents policy https://review.opendev.org/701649 | |
| 20:46:39 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Pass the actual target in unlock override policy https://review.opendev.org/716428 | |
| 20:55:01 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Pass the actual target in limits policy https://review.opendev.org/715761 | |
| 20:55:06 | dansmith | gmann: you around? | |
| 20:55:21 | dansmith | owalsh has a question for you :) | |
| 20:55:37 | gmann | dansmith: hi | |
| 20:55:51 | dansmith | gmann: http://paste.openstack.org/show/kckeKCDgq679ixa2g5IK/ | |
| 20:56:03 | dansmith | gmann: when running nova-manage with an empty policies file | |
| 20:56:44 | owalsh | nova-manage cell_v2 discover_hosts --by-service if it matters | |
| 20:58:22 | dansmith | owalsh: I gotta step away for a bit to get ready for something, but from poking around I feel like gmann is probably your mann :) | |
| 20:58:33 | owalsh | dansmith: ack, thanks | |
| 20:58:34 | gmann | :) | |
| 20:58:40 | openstackgerrit | sean mooney proposed openstack/nova master: [WIP] cyborg evacuate support https://review.opendev.org/715326 | |
| 20:59:31 | owalsh | gmann: already covered by one of the bp/policy-default-refresh patches? | |
| 20:59:38 | gmann | owalsh: those are new defaults and it add the deprecation even you have not override it. but old token keep working as they are maintained as deprecated rule till now. | |
| 20:59:46 | gmann | yeah | |
| 21:00:38 | owalsh | gmann: cool, so should go away once they all merge | |
| 21:00:53 | gmann | owalsh: we are changing all the policy to adopt the system scope and new defaults like read-only etc | |
| 21:03:36 | gmann | owalsh: warnings will stay till we remove the deprecated old rules in 1 or 2 cycle from now | |
| 21:05:07 | owalsh | gmann: ack, thanks, expect I'll get asked about the warning quite often until then :-) | |
| 21:09:23 | gmann | owalsh: basically signal to adopt the scope check, those are configurable for now and disabled as default. our goal is to 1. remove the old deprecated roles 2. enable scope check together. but you can always move to new policy by configuring the enforce_scope=Ture | |
| 21:09:59 | gmann | i am still working on those and should provide a doc on 'how to migrate to new policy' once done. | |
| 21:10:38 | gmann | owalsh: i hope those warnings are not much disturbing (as they are for every rule). | |
| 21:11:12 | melwitt | gmann: ++ I was just gonna say I wonder if we can improve that warning message and link to a doc explaining the details and process for migrating. a main point in it is the user being able to tell whether they need to migrate at all, based on their existing policy | |
| 21:11:43 | melwitt | at a minimum once you have written a doc, we should update the warning message to include the link to the documentation | |
| 21:12:42 | gmann | melwitt: that is good idea. it will be easy to link doc to warning | |
| 21:21:28 | owalsh | gmann: so for default (empty) policy file we will still get warning for 1-2 cycles? | |
| 21:21:49 | owalsh | or just if deprecated roles are used in the policy? | |
| 21:22:03 | dansmith | it definitely sucks to warn about deprecated policy when the deprecated things are defaults | |
| 21:22:21 | dansmith | if we can't tell what is deprecated (defaults vs. overrides) I would argue we should squelch that warning | |
| 21:25:52 | gmann | owalsh: all becasue defaults are deprecated | |
| 21:27:08 | gmann | it is for all rule as we use those defaults rules as check_str for every rule and oslo policy just add warning for those | |
| 21:27:25 | gmann | not sure how to combine those. | |
| 21:30:43 | gmann | one way is disable oslo warning completely and add a single combined warning form nova policy code with link to migration doc. | |
| 21:34:03 | bnemec | The deprecations are warning you that something might break next cycle and you should test with the future defaults now. | |
| 21:35:50 | gmann | bnemec: we test with both old and new but with disable wanring. | |
| 21:36:07 | dansmith | bnemec: sounds like he has deprecated our existing defaults | |
| 21:36:13 | dansmith | which is why I think we should *not* show this to the user | |
| 21:36:20 | dansmith | there's nothing they can do about it, other than ignore | |
| 21:36:46 | dansmith | it teaches our users to ignore deprecation warnings which is majorly uncool | |
| 21:38:01 | bnemec | Ah, this is a different type of deprecation than what I was thinking of. | |
| 21:38:10 | gmann | and each warning teach what is new defaults to that operator can overidde if anything breaking for them | |
| 21:38:32 | gmann | though that info is present in policy doc also | |
| 21:39:21 | dansmith | gmann: I'm not sure what you're saying | |
| 21:39:29 | bnemec | I think the intent when a policy rule name is changing is that if the new policy name is in the policy file then we don't log the warning. | |
| 21:39:39 | dansmith | if the user has made no policy choices and we're logging things telling them that their policy is deprecated, that's a problem | |
| 21:40:25 | dansmith | also, why are we logging this from nova-manage? | |
| 21:40:47 | dansmith | we're just making admin eyes bleed if we complain about something not even related to nova-manage on each command invocation | |
| 21:40:55 | gmann | dansmith: i mean each warning say old default of xyz rule is replaced with one of new default. for example legacy admin to system_reader | |
| 21:41:10 | dansmith | gmann: but we're complaining about our own defaults right? | |
| 21:41:17 | gmann | yeah | |
| 21:41:21 | bnemec | That's fair, we've actually shut off the policy deprecations in some cli tools and unit tests. | |
| 21:41:24 | dansmith | *that* is not okay | |
| 21:42:23 | dansmith | if we have no way to distinguish then we need to squelch the warning until we've fixed all our defaults | |
| 21:42:35 | gmann | i think waning should be added once during api service only | |
| 21:42:45 | dansmith | and, we shouldn't be making those warnings on nova-manage invocations | |
| 21:42:47 | dansmith | gmann: not for our own defaults | |
| 21:43:03 | dansmith | if we're telling the user something is wrong and the thing that is wrong is our default, we're teaching them to ignore our deprecation warnings | |
| 21:43:48 | bnemec | But the user needs to take action. They _shouldn't_ ignore these messages. | |
| 21:44:13 | gmann | own default but still operator rely on those and new default can change the behaviour if they have new roles like read only etc | |
| 21:44:29 | dansmith | gmann: that's what release notes are for | |
| 21:44:30 | gmann | scope is disabled by default so no issue there | |
| 21:44:48 | dansmith | logging that once per startup is totally unreasonable, IMHO | |
| 21:45:14 | owalsh | could nova status upgrade check validate any policy overrides if they exist? | |
| 21:45:38 | dansmith | also that | |
| 21:45:42 | bnemec | If a policy is overridden then you don't get the message. | |
| 21:45:51 | dansmith | nova-status is supposed to be a dynamic release note checker | |
| 21:46:00 | gmann | for this case, i agree on that because it is for every rule and lot of warnings | |
| 21:46:49 | bnemec | cmurphy did have a patch up to further consolidate the deprecation messages, but I feel like there were other concerns with it. | |
| 21:48:14 | gmann | yeah, if rule is override there is no warning for default change. it warn only if rule name change | |
| 21:48:46 | dansmith | we tell people not to override every rule, and almost nobody would override everything, | |
| 21:48:47 | melwitt | gmann: what is happening here is that the defaults are being deprecated and new defaults will be activated in one or two cycles right? maybe a Upgrade release note saying "the default policies are going to change in the W release, please review them" is good enough? | |
| 21:48:52 | dansmith | which means everyone will receive that warning | |
| 21:49:04 | dansmith | melwitt: ++ | |
| 21:49:31 | dansmith | this is precisely what nova-status and renos are for | |
| 21:50:09 | gmann | yeah new defaults are not enforced by default. | |
| 21:50:13 | bnemec | The new defaults are active already. They're just OR'd with the old defaults (if an explicit override is not set) to make sure that the rules are at least as permissive as the old rule so nobody is broken without notice. | |
| 21:50:25 | dansmith | I have to run to a thing, | |
| 21:50:54 | dansmith | but tldr of my opinion is.. there *has* to be a way to make this message go away.. I'd prefer reno/status. If not, then there has to be some way to say "OKAY I GOT IT" | |
| 21:50:55 | melwitt | gmann: and include instructions on how to set enforce_scope = True to see and try the new defaults? also instructions on how to dump the new defaults to review | |
| 21:50:58 | gmann | dansmith: melwitt reno is planned at the end. | |
| 21:51:18 | bnemec | There is. You explicitly set the new rule in your policy file and the deprecation warning goes away. | |
| 21:51:30 | dansmith | bnemec: but we've spent years telling people NOT to do that | |
| 21:51:39 | gmann | bnemec: but that is not expected | |
| 21:51:47 | gmann | yeah what dansmith mentioned | |
| 21:51:48 | melwitt | yeah ... that seems backwards. shouldn't default mean an empty policy file? | |
| 21:51:54 | dansmith | yes | |
| 21:51:55 | bnemec | It's essentially the operator saying, yes, we've looked at this change and it's fine. | |
| 21:52:04 | melwitt | that's what we made a big deal about policy in code back when that was added | |
| 21:52:11 | gmann | true | |
| 21:52:13 | dansmith | copying defaults into the policy file to shut it the eff up moves us backwards many years | |
| 21:52:39 | melwitt | not sure I'd want to confuse users by saying "ok now go set every single rule in your policy file" that seems counterintuitive | |
| 21:53:08 | melwitt | I would imagine their takeaway from that is we've removed policy-in-code and we're back to the old way | |
| 21:53:51 | dansmith | right, because they won't go to the trouble of understanding why, | |
| 21:54:02 | dansmith | they will just hear that is the solution to make it stop warning | |
| 21:54:03 | bnemec | I will say I don't think the policy deprecation mechanism was designed for deprecating the entire policy file at once, like we're doing now. | |
| 21:54:42 | melwitt | yes, I agree this is not the usual thing | |
| 21:54:50 | gmann | yeah, in this case where all defaults are changing warning is too much. | |
| 21:55:00 | bnemec | Also, the redundant rule tool was designed to let them clean up their policy files once the deprecation process is over. | |